all: use structs.HostLayout on structs that must match the C ABI - #502
all: use structs.HostLayout on structs that must match the C ABI#502TotallyGamerJet wants to merge 3 commits into
Conversation
Go 1.23 added structs.HostLayout, a zero-sized marker field that tells the compiler a struct must be laid out the way the host's C ABI expects rather than by Go's own rules. Until now purego relied on Go's current layout happening to match C. Declare the marker on every struct that stands in for a C one: objc_super, MethodDescription, PropertyAttribute, blockDescriptor and blockLayout in package objc; G, ThreadStart, the Darwin pthread types, the NetBSD stack_t and argset in internal/fakecgo; syscallArgs, which internal/cgo hands to C as a struct syscallArgs; the window example's NSPoint/NSSize/NSRect and WNDCLASSEX/RECT/POINT/MSG; and the C-representative structs in the tests. Structs that never cross into C, such as MethodDef, FieldDef and the callback argument block filled in by Go assembly, are left alone. The marker field is zero-sized, so it takes no part in the C ABI. Teach the argument classification to skip zero-sized fields, otherwise a struct that declares it is misclassified: isAllSameFloat would panic descending into the marker, isHFA/isHVA would stop recognizing homogeneous aggregates, and objc's @encode would emit the marker as a member. Updates ebitengine#259 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hajimehoshi
left a comment
There was a problem hiding this comment.
I think adding a check of isABIField can be a separate change from introducing LayoutHost, as isABIField seems to be related to other zero-sized structs. What do you think?
There was a problem hiding this comment.
Pull request overview
This PR adopts Go’s structs.HostLayout marker for structs that must match the host C ABI (including internal syscall argument blocks, fakecgo runtime structs, Objective-C runtime/block structs, examples, and ABI-related tests), and updates ABI/encoding logic to ignore zero-sized marker fields during argument/return classification and Objective-C @encode.
Changes:
- Add
_ structs.HostLayoutto C-representative structs across core code, internal/fakecgo, objc, tests, and window examples. - Teach ABI struct classification/helpers to skip zero-sized fields so HostLayout does not affect register/stack decisions or cause panics.
- Update Objective-C type encoding and example outputs to ignore/avoid printing the marker field.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| syscall.go | Marks syscallArgs as host-layout to match C ABI. |
| syscall_32bit.go | Marks syscallArgs as host-layout to match C ABI (32-bit). |
| func.go | Adds ABI-field helpers and updates homogeneous-float detection to skip zero-sized fields. |
| struct_amd64.go | Updates struct return/float detection to use ABI-field indexing (skip marker). |
| struct_arm64.go | Updates HFA/HVA-related logic to count ABI fields and skip marker fields when packing. |
| struct_s390x.go | Updates homogeneous-float handling to use ABI-field indexing. |
| struct_riscv64.go | Updates homogeneous-float handling to use ABI-field indexing. |
| struct_test.go | Updates struct ABI tests to include HostLayout and use keyed literals. |
| func_test.go | Updates ABI argument-passing tests to include HostLayout in struct types. |
| objc/objc_runtime_darwin.go | Adds HostLayout to objc structs and skips zero-sized fields in @encode. |
| objc/objc_runtime_darwin_test.go | Adds HostLayout to example structs and updates expected output formatting. |
| objc/objc_block_darwin.go | Adds HostLayout to Block ABI structs. |
| objc/objc_block_darwin_test.go | Adds HostLayout to test/example structs. |
| objc/encoding_darwin_test.go | Adds HostLayout to encode test struct. |
| internal/fakecgo/linux.go | Adds HostLayout to argset to match runtime/cgo ABI. |
| internal/fakecgo/libcgo.go | Adds HostLayout to G and ThreadStart to match runtime/cgo ABI. |
| internal/fakecgo/libcgo_netbsd.go | Adds HostLayout to NetBSD stack_t. |
| internal/fakecgo/libcgo_darwin.go | Adds HostLayout to Darwin pthread struct representations. |
| examples/window/main_windows.go | Adds HostLayout to Win32 ABI structs in the window example. |
| examples/window/main_darwin.go | Adds HostLayout to Cocoa geometry structs and uses keyed literals. |
Suppressed comments (1)
func.go:618
- In isAllSameFloat, the struct-field recursion overwrites the running allFloats result (
allFloats, structNumFields = isAllSameFloat(f)). This can incorrectly turn a previously-detected non-float aggregate back intotrueif a later nested struct is all-floats. Combine the nested result with the existing state instead of overwriting it.
if f.Kind() == reflect.Struct {
var structNumFields int
allFloats, structNumFields = isAllSameFloat(f)
numFields += structNumFields
continue
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| case reflect.Struct: | ||
| for i := 0; i < first.Type.NumField(); i++ { | ||
| for i := 0; i < numABIFields(first.Type); i++ { | ||
| if !isHFA(first.Type) { | ||
| return false | ||
| } |
Narrow the skip from "any zero-sized field" to the structs.HostLayout
marker itself, in answer to the review question about splitting it out.
Zero-sized is not the right rule. clang treats a zero-length array member
as a member: on arm64 `struct { char pad[0]; double x, y; }` is not a
homogeneous float aggregate and is passed in x0/x1 rather than d0/d1, and
@encode reports the member as [0c]. Skipping every zero-sized field would
have quietly changed how such a struct is passed. Only the marker has no
C counterpart, so only the marker may be skipped -- which is also why
this cannot land ahead of the marker itself.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
We do need to exclude HostLayout from the calculated ABI otherwise we get incorrect argument passing. It's now specific only to HostLayout though |
| encoding.WriteString("=") | ||
| for i := 0; i < typ.NumField(); i++ { | ||
| f := typ.Field(i) | ||
| if f.Type.ConvertibleTo(hostLayoutType) { |
| var i int | ||
| for i = 0; i < outType.NumField(); i++ { | ||
| if outType.Field(i).Offset == 8 { | ||
| for i = 0; i < numFields; i++ { |
There was a problem hiding this comment.
Optional: for i := range numFields
(There are a lot of for-range candidates, so we can do them once later)
What issue is this addressing?
Closes #259
What type of issue is this addressing?
feature
What this PR does | solves
Go 1.23 added structs.HostLayout, a zero-sized marker field that tells the compiler a struct must be laid out the way the host's C ABI expects rather than by Go's own rules. Until now purego relied on Go's current layout happening to match C.
Declare the marker on every struct that stands in for a C one: objc_super, MethodDescription, PropertyAttribute, blockDescriptor and blockLayout in package objc; G, ThreadStart, the Darwin pthread types, the NetBSD stack_t and argset in internal/fakecgo; syscallArgs, which internal/cgo hands to C as a struct syscallArgs; the window example's NSPoint/NSSize/NSRect and WNDCLASSEX/RECT/POINT/MSG; and the C-representative structs in the tests. Structs that never cross into C, such as MethodDef, FieldDef and the callback argument block filled in by Go assembly, are left alone.
The marker field is zero-sized, so it takes no part in the C ABI. Teach the argument classification to skip zero-sized fields, otherwise a struct that declares it is misclassified: isAllSameFloat would panic descending into the marker, isHFA/isHVA would stop recognizing homogeneous aggregates, and objc's @encode would emit the marker as a member.