From 37b2b0bd40eace71ff872bf5d88eedd3cfc37101 Mon Sep 17 00:00:00 2001 From: Hiroki KUMAZAKI Date: Sun, 6 Sep 2026 00:02:00 +0900 Subject: [PATCH 1/7] purego: respect field offsets in amd64 struct packing tryPlaceRegister packed fields back-to-back and overwrote pending small fields when a 64-bit field followed (val = instead of |=), dropping the first eightbyte and shifting all later arguments by one slot. Small fields also ignored padding, misplacing e.g. the int32 at offset 4 in {int8; int32}. Track each field's in-memory offset: flush the pending eightbyte on crossing, place wide fields directly, and realign the bit cursor for small fields. The recursive place() also clobbered the outer eightbyte index with the inner tail position, misclassifying later sibling fields of nested structs (e.g. StructInStruct lost B and C); save and restore the cursor around recursion. --- struct_amd64.go | 89 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 76 insertions(+), 13 deletions(-) diff --git a/struct_amd64.go b/struct_amd64.go index 85731fa3..338107c8 100644 --- a/struct_amd64.go +++ b/struct_amd64.go @@ -211,8 +211,10 @@ func tryPlaceRegister(v reflect.Value, addFloat func(uintptr), addInt func(uintp shift = 0 class = _NO_CLASS } - var place func(v reflect.Value) - place = func(v reflect.Value) { + var place func(v reflect.Value, base uintptr) + // curEight tracks the eightbyte index of the pending accumulator. + var curEight uintptr + place = func(v reflect.Value, base uintptr) { var numFields int if v.Kind() == reflect.Struct { numFields = v.Type().NumField() @@ -226,57 +228,110 @@ func tryPlaceRegister(v reflect.Value, addFloat func(uintptr), addInt func(uintp } flushed = false var f reflect.Value + var fieldOff uintptr if v.Kind() == reflect.Struct { f = v.Field(i) + fieldOff = base + v.Type().Field(i).Offset } else { f = v.Index(i) + fieldOff = base + uintptr(i)*f.Type().Size() + } + // The System V ABI classifies eightbytes from the in-memory + // image: a field starting in a later eightbyte than the pending + // accumulator ends it. Flush first so a wide field (e.g. the + // int64 in {int8; int64}) never overwrites accumulated smaller + // fields. A field wider than 4 bytes always starts a fresh + // eightbyte in practice (its alignment > remaining space), so + // after flushing we can place it directly without shifting. + needFresh := shift != 0 && fieldOff/8 != curEight + if needFresh { + flushIfNeeded() + } + curEight = fieldOff / 8 + // Small fields accumulate at the in-memory offset within the + // current eightbyte. Realign the bit cursor when the field + // starts later (padding), e.g. the int32 at offset 4 in + // {int8; int32}. + alignTo := func(off uintptr) { + want := byte((off % 8) * 8) + if want > shift { + shift = want + } } switch f.Kind() { case reflect.Struct: - place(f) + savedEight := curEight + place(f, fieldOff) + curEight = savedEight case reflect.Bool: + alignTo(fieldOff) if f.Bool() { val |= 1 << shift } shift += 8 class |= _INTEGER case reflect.Pointer, reflect.UnsafePointer: - val = uint64(f.Pointer()) - shift = 64 + if !needFresh { + val = uint64(f.Pointer()) + shift = 64 + } else { + flushed = false + addInt(uintptr(f.Pointer())) + flushed = true + } class = _INTEGER case reflect.Int8: + alignTo(fieldOff) val |= uint64(f.Int()&0xFF) << shift shift += 8 class |= _INTEGER case reflect.Int16: + alignTo(fieldOff) val |= uint64(f.Int()&0xFFFF) << shift shift += 16 class |= _INTEGER case reflect.Int32: + alignTo(fieldOff) val |= uint64(f.Int()&0xFFFF_FFFF) << shift shift += 32 class |= _INTEGER case reflect.Int64, reflect.Int: - val = uint64(f.Int()) - shift = 64 + if !needFresh { + val = uint64(f.Int()) + shift = 64 + } else { + flushed = false + addInt(uintptr(f.Int())) + flushed = true + } class = _INTEGER case reflect.Uint8: + alignTo(fieldOff) val |= f.Uint() << shift shift += 8 class |= _INTEGER case reflect.Uint16: + alignTo(fieldOff) val |= f.Uint() << shift shift += 16 class |= _INTEGER case reflect.Uint32: + alignTo(fieldOff) val |= f.Uint() << shift shift += 32 class |= _INTEGER case reflect.Uint64, reflect.Uint, reflect.Uintptr: - val = f.Uint() - shift = 64 + if !needFresh { + val = f.Uint() + shift = 64 + } else { + flushed = false + addInt(uintptr(f.Uint())) + flushed = true + } class = _INTEGER case reflect.Float32: + alignTo(fieldOff) val |= uint64(math.Float32bits(float32(f.Float()))) << shift shift += 32 class |= _SSE @@ -285,11 +340,19 @@ func tryPlaceRegister(v reflect.Value, addFloat func(uintptr), addInt func(uintp ok = false return } - val = uint64(math.Float64bits(f.Float())) - shift = 64 + if !needFresh { + val = uint64(math.Float64bits(f.Float())) + shift = 64 + } else { + flushed = false + addFloat(uintptr(math.Float64bits(f.Float()))) + flushed = true + } class = _SSE case reflect.Array: - place(f) + savedArrEight := curEight + place(f, fieldOff) + curEight = savedArrEight default: panic("purego: unsupported kind " + f.Kind().String()) } @@ -304,7 +367,7 @@ func tryPlaceRegister(v reflect.Value, addFloat func(uintptr), addInt func(uintp } } - place(v) + place(v, 0) flushIfNeeded() return ok } From 42eb5c2d98719b123c51673facad8d999aa2ac48 Mon Sep 17 00:00:00 2001 From: Hiroki KUMAZAKI Date: Sun, 6 Sep 2026 02:19:44 +0900 Subject: [PATCH 2/7] struct_test: add regression tests for amd64 struct field offsets Cover the struct argument shapes fixed in tryPlaceRegister: a small field followed by a wide field crossing the eightbyte boundary, a small field after padding, a struct between scalar arguments, and a nested struct followed by a sibling field. These identity round trips fail on main, where the first eightbyte was dropped and fields were packed back-to-back, and pass with the offset-aware packing. --- struct_test.go | 82 +++++++++++++++++++++++++++++++ testdata/structtest/struct_test.c | 36 ++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/struct_test.go b/struct_test.go index f5695932..98433258 100644 --- a/struct_test.go +++ b/struct_test.go @@ -944,6 +944,88 @@ func TestRegisterFunc_structArgs(t *testing.T) { } runtime.KeepAlive(ptr) } + t.Run("CharLong", func(t *testing.T) { + // Small field followed by a wide field crossing the eightbyte + // boundary: the wide field must not overwrite the pending + // small fields. + type CharLong struct { + _ structs.HostLayout + A int8 + B int64 + } + var fn func(CharLong) CharLong + register(&fn, lib, "IdentityCharLong", func(s CharLong) CharLong { + return s + }) + expected := CharLong{A: 0x7f, B: -0x0102030405060708} + if ret := fn(expected); ret != expected { + t.Fatalf("IdentityCharLong returned %+v wanted %+v", ret, expected) + } + }) + t.Run("CharLongBetweenPrims", func(t *testing.T) { + // Same as above but with scalar arguments before and after + // the struct: the struct must consume exactly two register + // slots so the trailing scalar is not shifted. + type CharLong struct { + _ structs.HostLayout + A int8 + B int64 + } + var fn func(int64, CharLong, int64) CharLong + register(&fn, lib, "IdentityCharLongBetweenPrims", func(x int64, s CharLong, y int64) CharLong { + return s + }) + expected := CharLong{A: -1, B: 0x1122334455667788} + if ret := fn(1, expected, 2); ret != expected { + t.Fatalf("IdentityCharLongBetweenPrims returned %+v wanted %+v", ret, expected) + } + }) + t.Run("CharInt", func(t *testing.T) { + // Small field after padding: the int32 at offset 4 must be + // placed at its in-memory offset, not back-to-back with the + // int8. + type CharInt struct { + _ structs.HostLayout + A int8 + B int32 + } + var fn func(CharInt) CharInt + register(&fn, lib, "IdentityCharInt", func(s CharInt) CharInt { + return s + }) + expected := CharInt{A: 0x01, B: 0x02030405} + if ret := fn(expected); ret != expected { + t.Fatalf("IdentityCharInt returned %+v wanted %+v", ret, expected) + } + }) + t.Run("NestedSmallTail", func(t *testing.T) { + // Nested struct with padding followed by a sibling field in + // the next eightbyte. + type NestedSmallTail struct { + _ structs.HostLayout + I struct { + _ structs.HostLayout + A int8 + B int32 + } + C int8 + } + var fn func(NestedSmallTail) NestedSmallTail + register(&fn, lib, "IdentityNestedSmallTail", func(s NestedSmallTail) NestedSmallTail { + return s + }) + expected := NestedSmallTail{ + I: struct { + _ structs.HostLayout + A int8 + B int32 + }{A: 0x11, B: 0x22334455}, + C: 0x66, + } + if ret := fn(expected); ret != expected { + t.Fatalf("IdentityNestedSmallTail returned %+v wanted %+v", ret, expected) + } + }) }) } } diff --git a/testdata/structtest/struct_test.c b/testdata/structtest/struct_test.c index b513b816..d349d608 100644 --- a/testdata/structtest/struct_test.c +++ b/testdata/structtest/struct_test.c @@ -453,3 +453,39 @@ struct Mixed5Args { struct Mixed5Args IdentityMixed5Args(struct Mixed5Args s) { return s; } + +struct CharLong { + int8_t a; + int64_t b; +}; + +struct CharLong IdentityCharLong(struct CharLong s) { + return s; +} + +struct CharLong IdentityCharLongBetweenPrims(int64_t x, struct CharLong s, int64_t y) { + (void) x; + (void) y; + return s; +} + +struct CharInt { + int8_t a; + int32_t b; +}; + +struct CharInt IdentityCharInt(struct CharInt s) { + return s; +} + +struct NestedSmallTail { + struct { + int8_t a; + int32_t b; + } i; + int8_t c; +}; + +struct NestedSmallTail IdentityNestedSmallTail(struct NestedSmallTail s) { + return s; +} From 57d469e262c65525c1c6d8bda13d70e7b3a6fae4 Mon Sep 17 00:00:00 2001 From: Hiroki KUMAZAKI Date: Sun, 6 Sep 2026 13:51:28 +0900 Subject: [PATCH 3/7] purego: flush trailing fields after an alignment flush on arm64 placeRegistersArm64 marked the eightbyte as flushed when a field's alignment pushed the bit cursor past the register boundary, but the field itself was then accumulated into val and never emitted, so the last register of a struct was lost (struct { struct { int8 a; int32 b; }; int8 c } dropped c on linux/arm64). Keep flushed false so the final flush still emits the trailing value. Found by the NestedSmallTail regression test added for the amd64 field offset fix. --- struct_arm64.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/struct_arm64.go b/struct_arm64.go index b776bcd7..3815c6c1 100644 --- a/struct_arm64.go +++ b/struct_arm64.go @@ -134,7 +134,9 @@ func placeRegistersArm64(v reflect.Value, addFloat func(uintptr), addInt func(ui shift = (shift + align) &^ align if shift >= 64 { shift = 0 - flushed = true + // Leave flushed false: the field placed below may + // accumulate into val and still needs the final flush. + flushed = false if class == _FLOAT { addFloat(uintptr(val)) } else { From 3afcd96dccffed9c40effffe416fca6c7140fe49 Mon Sep 17 00:00:00 2001 From: Hiroki KUMAZAKI Date: Sun, 6 Sep 2026 22:52:56 +0900 Subject: [PATCH 4/7] purego: keep the pending eightbyte index consistent after struct recursion tryPlaceRegister restored curEight to the outer field's eightbyte when a nested struct or array recursion returned, but the accumulator is shared across recursion levels. When the nested value spans eightbytes its tail stays pending in the last one it touched; restoring the outer index flushed that tail prematurely and the flushed flag then suppressed the flush of the following sibling field, dropping it from the register arguments (struct { A struct{X,Y,Z int32}; B int32 } and struct { A [3]int32; B int32 } passed six instead of ten to a C sum). Leave curEight pointing at the pending accumulator and add regression coverage for both layouts. --- struct_amd64.go | 9 ++++--- struct_test.go | 42 +++++++++++++++++++++++++++++++ testdata/structtest/struct_test.c | 22 ++++++++++++++++ 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/struct_amd64.go b/struct_amd64.go index 338107c8..f0a4c402 100644 --- a/struct_amd64.go +++ b/struct_amd64.go @@ -260,9 +260,12 @@ func tryPlaceRegister(v reflect.Value, addFloat func(uintptr), addInt func(uintp } switch f.Kind() { case reflect.Struct: - savedEight := curEight + // Recursion shares the accumulator, so it must also share + // curEight: when a nested struct spans eightbytes its tail + // is pending in the last one it touched, and the next + // sibling field belongs to that same eightbyte. Restoring + // the outer index would flush the tail prematurely. place(f, fieldOff) - curEight = savedEight case reflect.Bool: alignTo(fieldOff) if f.Bool() { @@ -350,9 +353,7 @@ func tryPlaceRegister(v reflect.Value, addFloat func(uintptr), addInt func(uintp } class = _SSE case reflect.Array: - savedArrEight := curEight place(f, fieldOff) - curEight = savedArrEight default: panic("purego: unsupported kind " + f.Kind().String()) } diff --git a/struct_test.go b/struct_test.go index 98433258..0caf26b5 100644 --- a/struct_test.go +++ b/struct_test.go @@ -1026,6 +1026,48 @@ func TestRegisterFunc_structArgs(t *testing.T) { t.Fatalf("IdentityNestedSmallTail returned %+v wanted %+v", ret, expected) } }) + t.Run("NestedIntsPlusOne", func(t *testing.T) { + // A nested struct whose tail remains pending in the second + // eightbyte followed by a sibling field in that same + // eightbyte. Recursion must leave the pending eightbyte + // index consistent with the accumulator so the sibling is + // merged into it instead of dropping it. + type inner struct { + _ structs.HostLayout + X int32 + Y int32 + Z int32 + } + type NestedIntsPlusOne struct { + _ structs.HostLayout + A inner + B int32 + } + var sum func(NestedIntsPlusOne) int64 + register(&sum, lib, "SumNestedIntsPlusOne", func(s NestedIntsPlusOne) int64 { + return int64(s.A.X) + int64(s.A.Y) + int64(s.A.Z) + int64(s.B) + }) + if ret := sum(NestedIntsPlusOne{A: inner{X: 1, Y: 2, Z: 3}, B: 4}); ret != 10 { + t.Fatalf("SumNestedIntsPlusOne returned %d wanted 10", ret) + } + }) + t.Run("ArrayIntsPlusOne", func(t *testing.T) { + // The array counterpart of NestedIntsPlusOne: the trailing + // element of [3]int32 is pending in the second eightbyte + // when the sibling field must be merged into it. + type ArrayIntsPlusOne struct { + _ structs.HostLayout + A [3]int32 + B int32 + } + var sum func(ArrayIntsPlusOne) int64 + register(&sum, lib, "SumArrayIntsPlusOne", func(s ArrayIntsPlusOne) int64 { + return int64(s.A[0]) + int64(s.A[1]) + int64(s.A[2]) + int64(s.B) + }) + if ret := sum(ArrayIntsPlusOne{A: [3]int32{1, 2, 3}, B: 4}); ret != 10 { + t.Fatalf("SumArrayIntsPlusOne returned %d wanted 10", ret) + } + }) }) } } diff --git a/testdata/structtest/struct_test.c b/testdata/structtest/struct_test.c index d349d608..04789433 100644 --- a/testdata/structtest/struct_test.c +++ b/testdata/structtest/struct_test.c @@ -489,3 +489,25 @@ struct NestedSmallTail { struct NestedSmallTail IdentityNestedSmallTail(struct NestedSmallTail s) { return s; } + +struct NestedIntsPlusOne { + struct { + int32_t x; + int32_t y; + int32_t z; + } a; + int32_t b; +}; + +int64_t SumNestedIntsPlusOne(struct NestedIntsPlusOne s) { + return (int64_t) s.a.x + s.a.y + s.a.z + s.b; +} + +struct ArrayIntsPlusOne { + int32_t a[3]; + int32_t b; +}; + +int64_t SumArrayIntsPlusOne(struct ArrayIntsPlusOne s) { + return (int64_t) s.a[0] + s.a[1] + s.a[2] + s.b; +} From fed3b2b5589837b27adc6dc5e9697261f70de161 Mon Sep 17 00:00:00 2001 From: Hiroki KUMAZAKI Date: Thu, 10 Sep 2026 23:57:17 +0900 Subject: [PATCH 5/7] purego: emit fields accumulated after a boundary flush On amd64 the eightbyte-crossing flush in tryPlaceRegister left the accumulator marked as flushed, so a small field accumulated afterwards (the trailing int8 in struct { A struct{X int32; Y int8}; B int8 }) was skipped by the final flushIfNeeded and lost from the register arguments; a C function summing 1, 2, 3 returned 3. Reset the flag after that intermediate flush so the fresh accumulator still reaches the flush at the end of the iteration or of place(). The same layout failed on linux/arm64 for a related reason: placeRegistersArm64 positions fields with per-field alignment arithmetic and the recursion ignored a nested composite's trailing padding, so the sibling was packed back-to-back after the last inner field instead of starting at its own in-memory offset. Track the memory offset that bit 0 of the pending register corresponds to and realign the cursor to the composite's end after recursion, emitting the pending register whenever the padding carries past the slot. Add SumNestedPadTail regression coverage; verified on linux/amd64 and linux/arm64 (qemu) for both the direct and callback paths. --- struct_amd64.go | 6 ++++ struct_arm64.go | 48 ++++++++++++++++++++++++++----- struct_test.go | 24 ++++++++++++++++ testdata/structtest/struct_test.c | 12 ++++++++ 4 files changed, 83 insertions(+), 7 deletions(-) diff --git a/struct_amd64.go b/struct_amd64.go index f0a4c402..3832b100 100644 --- a/struct_amd64.go +++ b/struct_amd64.go @@ -246,6 +246,12 @@ func tryPlaceRegister(v reflect.Value, addFloat func(uintptr), addInt func(uintp needFresh := shift != 0 && fieldOff/8 != curEight if needFresh { flushIfNeeded() + // The intermediate flush above ended the pending eightbyte; + // the field placed below starts a fresh accumulator that + // must still reach the flush at the end of this iteration or + // at the end of place(). Leave the flag clear until a field + // is actually emitted. + flushed = false } curEight = fieldOff / 8 // Small fields accumulate at the in-memory offset within the diff --git a/struct_arm64.go b/struct_arm64.go index 3815c6c1..dbe73f40 100644 --- a/struct_arm64.go +++ b/struct_arm64.go @@ -111,8 +111,24 @@ func placeRegistersArm64(v reflect.Value, addFloat func(uintptr), addInt func(ui var shift byte var flushed bool class := _NO_CLASS - var place func(v reflect.Value) - place = func(v reflect.Value) { + // slotOff is the in-memory offset that bit 0 of val corresponds to, so + // the cursor can be realigned to a field's true offset after recursion + // into a composite that carries trailing padding. + var slotOff uintptr + advanceSlot := func() { + if class == _FLOAT { + addFloat(uintptr(val)) + } else { + addInt(uintptr(val)) + } + val = 0 + shift = 0 + class = _NO_CLASS + slotOff += 8 + flushed = true + } + var place func(v reflect.Value, base uintptr) + place = func(v reflect.Value, base uintptr) { var numFields int if v.Kind() == reflect.Struct { numFields = v.Type().NumField() @@ -125,10 +141,13 @@ func placeRegistersArm64(v reflect.Value, addFloat func(uintptr), addInt func(ui } flushed = false var f reflect.Value + var fieldOff uintptr if v.Kind() == reflect.Struct { f = v.Field(k) + fieldOff = base + v.Type().Field(k).Offset } else { f = v.Index(k) + fieldOff = base + uintptr(k)*f.Type().Size() } align := byte(f.Type().Align()*8 - 1) shift = (shift + align) &^ align @@ -144,10 +163,22 @@ func placeRegistersArm64(v reflect.Value, addFloat func(uintptr), addInt func(ui } val = 0 class = _NO_CLASS + slotOff += 8 } switch f.Type().Kind() { - case reflect.Struct: - place(f) + case reflect.Struct, reflect.Array: + place(f, fieldOff) + // A composite occupies its full in-memory size: skip its + // trailing padding so the next sibling is placed at its + // own offset, emitting the pending register whenever that + // carries past the current slot. + for end := fieldOff + f.Type().Size(); end > slotOff+uintptr(shift)/8; { + if bits := (end - slotOff) * 8; bits < 64 { + shift = byte(bits) + break + } + advanceSlot() + } case reflect.Bool: if f.Bool() { val |= 1 << shift @@ -170,6 +201,7 @@ func placeRegistersArm64(v reflect.Value, addFloat func(uintptr), addInt func(ui addInt(uintptr(f.Uint())) shift = 0 flushed = true + slotOff += 8 class = _NO_CLASS case reflect.Int8: val |= uint64(f.Int()&0xFF) << shift @@ -187,12 +219,14 @@ func placeRegistersArm64(v reflect.Value, addFloat func(uintptr), addInt func(ui addInt(uintptr(f.Int())) shift = 0 flushed = true + slotOff += 8 class = _NO_CLASS case reflect.Float32: if class == _FLOAT { addFloat(uintptr(val)) val = 0 shift = 0 + slotOff += 4 } val |= uint64(math.Float32bits(float32(f.Float()))) << shift shift += 32 @@ -201,20 +235,20 @@ func placeRegistersArm64(v reflect.Value, addFloat func(uintptr), addInt func(ui addFloat(uintptr(math.Float64bits(float64(f.Float())))) shift = 0 flushed = true + slotOff += 8 class = _NO_CLASS case reflect.Pointer, reflect.UnsafePointer: addInt(f.Pointer()) shift = 0 flushed = true + slotOff += 8 class = _NO_CLASS - case reflect.Array: - place(f) default: panic("purego: unsupported kind " + f.Kind().String()) } } } - place(v) + place(v, 0) if !flushed { if class == _FLOAT { addFloat(uintptr(val)) diff --git a/struct_test.go b/struct_test.go index 0caf26b5..4d25e60d 100644 --- a/struct_test.go +++ b/struct_test.go @@ -1051,6 +1051,30 @@ func TestRegisterFunc_structArgs(t *testing.T) { t.Fatalf("SumNestedIntsPlusOne returned %d wanted 10", ret) } }) + t.Run("NestedPadTail", func(t *testing.T) { + // The nested struct has trailing padding, so the sibling + // field starts in the next eightbyte while the first one is + // still pending. The boundary flush must not mark the + // accumulator as final: the sibling accumulated afterwards + // still needs the final flush. + type inner struct { + _ structs.HostLayout + X int32 + Y int8 + } + type NestedPadTail struct { + _ structs.HostLayout + A inner + B int8 + } + var sum func(NestedPadTail) int64 + register(&sum, lib, "SumNestedPadTail", func(s NestedPadTail) int64 { + return int64(s.A.X) + int64(s.A.Y) + int64(s.B) + }) + if ret := sum(NestedPadTail{A: inner{X: 1, Y: 2}, B: 3}); ret != 6 { + t.Fatalf("SumNestedPadTail returned %d wanted 6", ret) + } + }) t.Run("ArrayIntsPlusOne", func(t *testing.T) { // The array counterpart of NestedIntsPlusOne: the trailing // element of [3]int32 is pending in the second eightbyte diff --git a/testdata/structtest/struct_test.c b/testdata/structtest/struct_test.c index 04789433..4ed672b3 100644 --- a/testdata/structtest/struct_test.c +++ b/testdata/structtest/struct_test.c @@ -511,3 +511,15 @@ struct ArrayIntsPlusOne { int64_t SumArrayIntsPlusOne(struct ArrayIntsPlusOne s) { return (int64_t) s.a[0] + s.a[1] + s.a[2] + s.b; } + +struct NestedPadTail { + struct { + int32_t x; + int8_t y; + } a; + int8_t b; +}; + +int64_t SumNestedPadTail(struct NestedPadTail s) { + return (int64_t) s.a.x + s.a.y + s.b; +} From e276b854066ef7fb8ed351e330ec70d90ada5be1 Mon Sep 17 00:00:00 2001 From: kumagi Date: Fri, 18 Sep 2026 02:38:55 +0900 Subject: [PATCH 6/7] purego: document that struct field padding is respected RegisterFunc still said that purego could not align struct fields and that callers had to add the padding themselves, which the offset-aware packing made untrue. Document that fields are placed at their in-memory offsets and that only the Go struct declaration has to mirror the C one, and cover a struct that relies on the padding Go inserts. Shorten the comments added with the packing fixes while here. --- func.go | 12 +++++----- struct_amd64.go | 29 +++++++----------------- struct_arm64.go | 15 +++++-------- struct_test.go | 59 ++++++++++++++++++++++++++++--------------------- 4 files changed, 55 insertions(+), 60 deletions(-) diff --git a/func.go b/func.go index 632b3470..c4267e80 100644 --- a/func.go +++ b/func.go @@ -48,8 +48,8 @@ func RegisterLibFunc(fptr any, handle uintptr, name string) { // // These conversions describe how a Go type in the fptr will be used to call // the C function. It is important to note that there is no way to verify that fptr -// matches the C function. This also holds true for struct types where the padding -// needs to be ensured to match that of C; RegisterFunc does not verify this. +// matches the C function. This also holds true for struct types, whose memory layout +// must match the C one; RegisterFunc does not verify this. // // # Type Conversions (Go <=> C) // @@ -101,9 +101,11 @@ func RegisterLibFunc(fptr any, handle uintptr, name string) { // // # Structs // -// Purego can handle the most common structs that have fields of builtin types like int8, uint16, float32, etc. However, -// it does not support aligning fields properly. It is therefore the responsibility of the caller to ensure -// that all padding is added to the Go struct to match the C one. See `BoolStructFn` in struct_test.go for an example. +// Purego can handle the most common structs that have fields of builtin types like int8, uint16, float32, etc. +// Each field is placed at the offset it has in the Go struct's memory image, so the padding the Go compiler +// inserts is preserved and explicit padding fields are not needed. The Go struct must still be declared with the +// same fields, in the same order, as the C one, and should embed [structs.HostLayout] to guarantee that layout. +// Purego does not verify that the two match. // // On Apple ARM64 platforms (macOS and iOS), purego handles proper alignment of struct arguments // when passing them on the stack, following the C ABI's byte-level packing rules. diff --git a/struct_amd64.go b/struct_amd64.go index 3832b100..7bae686f 100644 --- a/struct_amd64.go +++ b/struct_amd64.go @@ -236,28 +236,18 @@ func tryPlaceRegister(v reflect.Value, addFloat func(uintptr), addInt func(uintp f = v.Index(i) fieldOff = base + uintptr(i)*f.Type().Size() } - // The System V ABI classifies eightbytes from the in-memory - // image: a field starting in a later eightbyte than the pending - // accumulator ends it. Flush first so a wide field (e.g. the - // int64 in {int8; int64}) never overwrites accumulated smaller - // fields. A field wider than 4 bytes always starts a fresh - // eightbyte in practice (its alignment > remaining space), so - // after flushing we can place it directly without shifting. + // A field in a later eightbyte than the pending accumulator ends + // it, so flush before a wide field overwrites the small fields + // accumulated so far (e.g. the int64 in {int8; int64}). needFresh := shift != 0 && fieldOff/8 != curEight if needFresh { flushIfNeeded() - // The intermediate flush above ended the pending eightbyte; - // the field placed below starts a fresh accumulator that - // must still reach the flush at the end of this iteration or - // at the end of place(). Leave the flag clear until a field - // is actually emitted. + // The fresh accumulator must still be flushed. flushed = false } curEight = fieldOff / 8 - // Small fields accumulate at the in-memory offset within the - // current eightbyte. Realign the bit cursor when the field - // starts later (padding), e.g. the int32 at offset 4 in - // {int8; int32}. + // Realign the bit cursor over padding, e.g. for the int32 at + // offset 4 in {int8; int32}. alignTo := func(off uintptr) { want := byte((off % 8) * 8) if want > shift { @@ -266,11 +256,8 @@ func tryPlaceRegister(v reflect.Value, addFloat func(uintptr), addInt func(uintp } switch f.Kind() { case reflect.Struct: - // Recursion shares the accumulator, so it must also share - // curEight: when a nested struct spans eightbytes its tail - // is pending in the last one it touched, and the next - // sibling field belongs to that same eightbyte. Restoring - // the outer index would flush the tail prematurely. + // The nested tail stays pending in the accumulator, so + // curEight must not be restored here. place(f, fieldOff) case reflect.Bool: alignTo(fieldOff) diff --git a/struct_arm64.go b/struct_arm64.go index dbe73f40..b13bc803 100644 --- a/struct_arm64.go +++ b/struct_arm64.go @@ -111,9 +111,8 @@ func placeRegistersArm64(v reflect.Value, addFloat func(uintptr), addInt func(ui var shift byte var flushed bool class := _NO_CLASS - // slotOff is the in-memory offset that bit 0 of val corresponds to, so - // the cursor can be realigned to a field's true offset after recursion - // into a composite that carries trailing padding. + // slotOff is the in-memory offset bit 0 of val corresponds to, so that + // the cursor can be realigned after a composite with trailing padding. var slotOff uintptr advanceSlot := func() { if class == _FLOAT { @@ -153,8 +152,8 @@ func placeRegistersArm64(v reflect.Value, addFloat func(uintptr), addInt func(ui shift = (shift + align) &^ align if shift >= 64 { shift = 0 - // Leave flushed false: the field placed below may - // accumulate into val and still needs the final flush. + // Keep flushed false so the field placed below is still + // emitted by the final flush. flushed = false if class == _FLOAT { addFloat(uintptr(val)) @@ -168,10 +167,8 @@ func placeRegistersArm64(v reflect.Value, addFloat func(uintptr), addInt func(ui switch f.Type().Kind() { case reflect.Struct, reflect.Array: place(f, fieldOff) - // A composite occupies its full in-memory size: skip its - // trailing padding so the next sibling is placed at its - // own offset, emitting the pending register whenever that - // carries past the current slot. + // Skip the composite's trailing padding so that the next + // sibling lands at its own in-memory offset. for end := fieldOff + f.Type().Size(); end > slotOff+uintptr(shift)/8; { if bits := (end - slotOff) * 8; bits < 64 { shift = byte(bits) diff --git a/struct_test.go b/struct_test.go index 4d25e60d..8d3e499a 100644 --- a/struct_test.go +++ b/struct_test.go @@ -613,7 +613,7 @@ func TestRegisterFunc_structArgs(t *testing.T) { type BoolFloat struct { _ structs.HostLayout b bool - _ [3]byte // purego won't do padding for you so make sure it aligns properly with C struct + _ [3]byte // redundant with the padding Go inserts, but mirrors the C layout f float32 } var BoolFloatFn func(BoolFloat) float32 @@ -945,9 +945,7 @@ func TestRegisterFunc_structArgs(t *testing.T) { runtime.KeepAlive(ptr) } t.Run("CharLong", func(t *testing.T) { - // Small field followed by a wide field crossing the eightbyte - // boundary: the wide field must not overwrite the pending - // small fields. + // The wide field must not overwrite the pending small field. type CharLong struct { _ structs.HostLayout A int8 @@ -963,9 +961,8 @@ func TestRegisterFunc_structArgs(t *testing.T) { } }) t.Run("CharLongBetweenPrims", func(t *testing.T) { - // Same as above but with scalar arguments before and after - // the struct: the struct must consume exactly two register - // slots so the trailing scalar is not shifted. + // The struct must consume exactly two register slots so the + // trailing scalar is not shifted. type CharLong struct { _ structs.HostLayout A int8 @@ -981,9 +978,8 @@ func TestRegisterFunc_structArgs(t *testing.T) { } }) t.Run("CharInt", func(t *testing.T) { - // Small field after padding: the int32 at offset 4 must be - // placed at its in-memory offset, not back-to-back with the - // int8. + // The int32 must be placed at its padded offset, not back-to-back + // with the int8. type CharInt struct { _ structs.HostLayout A int8 @@ -999,8 +995,7 @@ func TestRegisterFunc_structArgs(t *testing.T) { } }) t.Run("NestedSmallTail", func(t *testing.T) { - // Nested struct with padding followed by a sibling field in - // the next eightbyte. + // A sibling in the next eightbyte after a padded nested struct. type NestedSmallTail struct { _ structs.HostLayout I struct { @@ -1027,11 +1022,8 @@ func TestRegisterFunc_structArgs(t *testing.T) { } }) t.Run("NestedIntsPlusOne", func(t *testing.T) { - // A nested struct whose tail remains pending in the second - // eightbyte followed by a sibling field in that same - // eightbyte. Recursion must leave the pending eightbyte - // index consistent with the accumulator so the sibling is - // merged into it instead of dropping it. + // The sibling must be merged into the eightbyte the nested + // struct left pending. type inner struct { _ structs.HostLayout X int32 @@ -1052,11 +1044,8 @@ func TestRegisterFunc_structArgs(t *testing.T) { } }) t.Run("NestedPadTail", func(t *testing.T) { - // The nested struct has trailing padding, so the sibling - // field starts in the next eightbyte while the first one is - // still pending. The boundary flush must not mark the - // accumulator as final: the sibling accumulated afterwards - // still needs the final flush. + // The sibling after the nested struct's trailing padding must + // still be flushed. type inner struct { _ structs.HostLayout X int32 @@ -1076,9 +1065,7 @@ func TestRegisterFunc_structArgs(t *testing.T) { } }) t.Run("ArrayIntsPlusOne", func(t *testing.T) { - // The array counterpart of NestedIntsPlusOne: the trailing - // element of [3]int32 is pending in the second eightbyte - // when the sibling field must be merged into it. + // The array counterpart of NestedIntsPlusOne. type ArrayIntsPlusOne struct { _ structs.HostLayout A [3]int32 @@ -1092,6 +1079,28 @@ func TestRegisterFunc_structArgs(t *testing.T) { t.Fatalf("SumArrayIntsPlusOne returned %d wanted 10", ret) } }) + t.Run("BoolFloatNoPadding", func(t *testing.T) { + // Fields are placed at their in-memory offsets, so no + // explicit padding is needed after the bool. + type BoolFloat struct { + _ structs.HostLayout + b bool + f float32 + } + var fn func(BoolFloat) float32 + register(&fn, lib, "BoolFloat", func(s BoolFloat) float32 { + if s.b { + return s.f + } + return -s.f + }) + if ret := fn(BoolFloat{b: true, f: 10}); ret != expectedFloat { + t.Fatalf("BoolFloat returned %f wanted %f", ret, expectedFloat) + } + if ret := fn(BoolFloat{b: false, f: 10}); ret != -expectedFloat { + t.Fatalf("BoolFloat returned %f wanted %f", ret, -expectedFloat) + } + }) }) } } From dca47416d6615405f906105c3f98fce67634d104 Mon Sep 17 00:00:00 2001 From: kumagi Date: Sat, 3 Oct 2026 18:00:18 +0900 Subject: [PATCH 7/7] purego: simplify struct register packing with memory images --- func.go | 3 +- struct_amd64.go | 198 ++---------------------------- struct_arm64.go | 196 ++++------------------------- struct_test.go | 34 +++++ testdata/structtest/struct_test.c | 18 +++ 5 files changed, 91 insertions(+), 358 deletions(-) diff --git a/func.go b/func.go index c4267e80..8a3e9871 100644 --- a/func.go +++ b/func.go @@ -104,7 +104,8 @@ func RegisterLibFunc(fptr any, handle uintptr, name string) { // Purego can handle the most common structs that have fields of builtin types like int8, uint16, float32, etc. // Each field is placed at the offset it has in the Go struct's memory image, so the padding the Go compiler // inserts is preserved and explicit padding fields are not needed. The Go struct must still be declared with the -// same fields, in the same order, as the C one, and should embed [structs.HostLayout] to guarantee that layout. +// same fields, in the same order, as the C one, and should include a field named _ of type +// [structs.HostLayout] to guarantee that layout. // Purego does not verify that the two match. // // On Apple ARM64 platforms (macOS and iOS), purego handles proper alignment of struct arguments diff --git a/struct_amd64.go b/struct_amd64.go index 7bae686f..4f2a0ba8 100644 --- a/struct_amd64.go +++ b/struct_amd64.go @@ -4,7 +4,6 @@ package purego import ( - "math" "reflect" "runtime" "unsafe" @@ -137,23 +136,10 @@ func addStruct(v reflect.Value, numInts, numFloats, numStack *int, addInt, addFl return keepAlive } - // if greater than 64 bytes place on stack - if v.Type().Size() > 8*8 { - placeStack(v, addStack) - return keepAlive - } - var ( - savedNumFloats = *numFloats - savedNumInts = *numInts - savedNumStack = *numStack - ) - placeOnStack := postMerger(v.Type()) || !tryPlaceRegister(v, addFloat, addInt) - if placeOnStack { - // reset any values placed in registers - *numFloats = savedNumFloats - *numInts = savedNumInts - *numStack = savedNumStack + if postMerger(v.Type()) { placeStack(v, addStack) + } else { + tryPlaceRegister(v, addFloat, addInt) } return keepAlive } @@ -191,179 +177,17 @@ func postMerger(t reflect.Type) (passInMemory bool) { return true // Go does not have an SSE/SSEUP type so this is always true } -func tryPlaceRegister(v reflect.Value, addFloat func(uintptr), addInt func(uintptr)) (ok bool) { - ok = true - var val uint64 - var shift byte // # of bits to shift - var flushed bool - class := _NO_CLASS - flushIfNeeded := func() { - if flushed { - return - } - flushed = true - if class == _SSE { - addFloat(uintptr(val)) +// tryPlaceRegister passes a struct of at most two eightbytes in its ABI classes. +func tryPlaceRegister(v reflect.Value, addFloat func(uintptr), addInt func(uintptr)) { + var buf [2]uintptr + reflect.NewAt(v.Type(), unsafe.Pointer(&buf[0])).Elem().Set(v) + for i := uintptr(0); i*8 < v.Type().Size(); i++ { + if classifyEightbyte(v.Type(), i*8, i*8+8) == _SSE { + addFloat(buf[i]) } else { - addInt(uintptr(val)) - } - val = 0 - shift = 0 - class = _NO_CLASS - } - var place func(v reflect.Value, base uintptr) - // curEight tracks the eightbyte index of the pending accumulator. - var curEight uintptr - place = func(v reflect.Value, base uintptr) { - var numFields int - if v.Kind() == reflect.Struct { - numFields = v.Type().NumField() - } else { - numFields = v.Type().Len() - } - - for i := range numFields { - if v.Kind() == reflect.Struct && !isABIField(v.Type().Field(i)) { - continue - } - flushed = false - var f reflect.Value - var fieldOff uintptr - if v.Kind() == reflect.Struct { - f = v.Field(i) - fieldOff = base + v.Type().Field(i).Offset - } else { - f = v.Index(i) - fieldOff = base + uintptr(i)*f.Type().Size() - } - // A field in a later eightbyte than the pending accumulator ends - // it, so flush before a wide field overwrites the small fields - // accumulated so far (e.g. the int64 in {int8; int64}). - needFresh := shift != 0 && fieldOff/8 != curEight - if needFresh { - flushIfNeeded() - // The fresh accumulator must still be flushed. - flushed = false - } - curEight = fieldOff / 8 - // Realign the bit cursor over padding, e.g. for the int32 at - // offset 4 in {int8; int32}. - alignTo := func(off uintptr) { - want := byte((off % 8) * 8) - if want > shift { - shift = want - } - } - switch f.Kind() { - case reflect.Struct: - // The nested tail stays pending in the accumulator, so - // curEight must not be restored here. - place(f, fieldOff) - case reflect.Bool: - alignTo(fieldOff) - if f.Bool() { - val |= 1 << shift - } - shift += 8 - class |= _INTEGER - case reflect.Pointer, reflect.UnsafePointer: - if !needFresh { - val = uint64(f.Pointer()) - shift = 64 - } else { - flushed = false - addInt(uintptr(f.Pointer())) - flushed = true - } - class = _INTEGER - case reflect.Int8: - alignTo(fieldOff) - val |= uint64(f.Int()&0xFF) << shift - shift += 8 - class |= _INTEGER - case reflect.Int16: - alignTo(fieldOff) - val |= uint64(f.Int()&0xFFFF) << shift - shift += 16 - class |= _INTEGER - case reflect.Int32: - alignTo(fieldOff) - val |= uint64(f.Int()&0xFFFF_FFFF) << shift - shift += 32 - class |= _INTEGER - case reflect.Int64, reflect.Int: - if !needFresh { - val = uint64(f.Int()) - shift = 64 - } else { - flushed = false - addInt(uintptr(f.Int())) - flushed = true - } - class = _INTEGER - case reflect.Uint8: - alignTo(fieldOff) - val |= f.Uint() << shift - shift += 8 - class |= _INTEGER - case reflect.Uint16: - alignTo(fieldOff) - val |= f.Uint() << shift - shift += 16 - class |= _INTEGER - case reflect.Uint32: - alignTo(fieldOff) - val |= f.Uint() << shift - shift += 32 - class |= _INTEGER - case reflect.Uint64, reflect.Uint, reflect.Uintptr: - if !needFresh { - val = f.Uint() - shift = 64 - } else { - flushed = false - addInt(uintptr(f.Uint())) - flushed = true - } - class = _INTEGER - case reflect.Float32: - alignTo(fieldOff) - val |= uint64(math.Float32bits(float32(f.Float()))) << shift - shift += 32 - class |= _SSE - case reflect.Float64: - if v.Type().Size() > 16 { - ok = false - return - } - if !needFresh { - val = uint64(math.Float64bits(f.Float())) - shift = 64 - } else { - flushed = false - addFloat(uintptr(math.Float64bits(f.Float()))) - flushed = true - } - class = _SSE - case reflect.Array: - place(f, fieldOff) - default: - panic("purego: unsupported kind " + f.Kind().String()) - } - - if shift == 64 { - flushIfNeeded() - } else if shift > 64 { - // Should never happen, but may if we forget to reset shift after flush (or forget to flush), - // better fall apart here, than corrupt arguments. - panic("purego: tryPlaceRegisters shift > 64") - } + addInt(buf[i]) } } - - place(v, 0) - flushIfNeeded() - return ok } func placeStack(v reflect.Value, addStack func(uintptr)) { diff --git a/struct_arm64.go b/struct_arm64.go index b13bc803..2f15c7f1 100644 --- a/struct_arm64.go +++ b/struct_arm64.go @@ -107,152 +107,39 @@ func placeRegisters(v reflect.Value, addFloat func(uintptr), addInt func(uintptr } func placeRegistersArm64(v reflect.Value, addFloat func(uintptr), addInt func(uintptr)) { - var val uint64 - var shift byte - var flushed bool - class := _NO_CLASS - // slotOff is the in-memory offset bit 0 of val corresponds to, so that - // the cursor can be realigned after a composite with trailing padding. - var slotOff uintptr - advanceSlot := func() { - if class == _FLOAT { - addFloat(uintptr(val)) - } else { - addInt(uintptr(val)) - } - val = 0 - shift = 0 - class = _NO_CLASS - slotOff += 8 - flushed = true - } - var place func(v reflect.Value, base uintptr) - place = func(v reflect.Value, base uintptr) { - var numFields int - if v.Kind() == reflect.Struct { - numFields = v.Type().NumField() - } else { - numFields = v.Type().Len() - } - for k := range numFields { - if v.Kind() == reflect.Struct && !isABIField(v.Type().Field(k)) { - continue - } - flushed = false - var f reflect.Value - var fieldOff uintptr - if v.Kind() == reflect.Struct { - f = v.Field(k) - fieldOff = base + v.Type().Field(k).Offset - } else { - f = v.Index(k) - fieldOff = base + uintptr(k)*f.Type().Size() - } - align := byte(f.Type().Align()*8 - 1) - shift = (shift + align) &^ align - if shift >= 64 { - shift = 0 - // Keep flushed false so the field placed below is still - // emitted by the final flush. - flushed = false - if class == _FLOAT { - addFloat(uintptr(val)) - } else { - addInt(uintptr(val)) - } - val = 0 - class = _NO_CLASS - slotOff += 8 - } - switch f.Type().Kind() { - case reflect.Struct, reflect.Array: - place(f, fieldOff) - // Skip the composite's trailing padding so that the next - // sibling lands at its own in-memory offset. - for end := fieldOff + f.Type().Size(); end > slotOff+uintptr(shift)/8; { - if bits := (end - slotOff) * 8; bits < 64 { - shift = byte(bits) - break + if isHFA(v.Type()) { + var place func(reflect.Value) + place = func(v reflect.Value) { + switch v.Kind() { + case reflect.Struct: + for i := range v.NumField() { + if isABIField(v.Type().Field(i)) { + place(v.Field(i)) } - advanceSlot() } - case reflect.Bool: - if f.Bool() { - val |= 1 << shift + case reflect.Array: + for i := range v.Len() { + place(v.Index(i)) } - shift += 8 - class |= _INT - case reflect.Uint8: - val |= f.Uint() << shift - shift += 8 - class |= _INT - case reflect.Uint16: - val |= f.Uint() << shift - shift += 16 - class |= _INT - case reflect.Uint32: - val |= f.Uint() << shift - shift += 32 - class |= _INT - case reflect.Uint64, reflect.Uint, reflect.Uintptr: - addInt(uintptr(f.Uint())) - shift = 0 - flushed = true - slotOff += 8 - class = _NO_CLASS - case reflect.Int8: - val |= uint64(f.Int()&0xFF) << shift - shift += 8 - class |= _INT - case reflect.Int16: - val |= uint64(f.Int()&0xFFFF) << shift - shift += 16 - class |= _INT - case reflect.Int32: - val |= uint64(f.Int()&0xFFFF_FFFF) << shift - shift += 32 - class |= _INT - case reflect.Int64, reflect.Int: - addInt(uintptr(f.Int())) - shift = 0 - flushed = true - slotOff += 8 - class = _NO_CLASS case reflect.Float32: - if class == _FLOAT { - addFloat(uintptr(val)) - val = 0 - shift = 0 - slotOff += 4 - } - val |= uint64(math.Float32bits(float32(f.Float()))) << shift - shift += 32 - class |= _FLOAT + addFloat(uintptr(math.Float32bits(float32(v.Float())))) case reflect.Float64: - addFloat(uintptr(math.Float64bits(float64(f.Float())))) - shift = 0 - flushed = true - slotOff += 8 - class = _NO_CLASS - case reflect.Pointer, reflect.UnsafePointer: - addInt(f.Pointer()) - shift = 0 - flushed = true - slotOff += 8 - class = _NO_CLASS + addFloat(uintptr(math.Float64bits(v.Float()))) default: - panic("purego: unsupported kind " + f.Kind().String()) + panic("purego: unsupported HFA kind " + v.Kind().String()) } } + place(v) + return } - place(v, 0) - if !flushed { - if class == _FLOAT { - addFloat(uintptr(val)) - } else { - addInt(uintptr(val)) - } + + // Non-HFA composites use integer registers, including their padding. + if !v.CanAddr() { + addressable := reflect.New(v.Type()).Elem() + addressable.Set(v) + v = addressable } + copyStruct8ByteChunks(v.Addr().UnsafePointer(), v.Type().Size(), addInt) } func placeStack(v reflect.Value, keepAlive []any, addInt func(uintptr)) []any { @@ -344,11 +231,8 @@ func isHVA(t reflect.Type) bool { } // copyStruct8ByteChunks copies struct memory in 8-byte chunks to the provided callback. -// This is used for Darwin ARM64's byte-level packing of non-HFA/HVA structs. +// This preserves padding for non-HFA composites. func copyStruct8ByteChunks(ptr unsafe.Pointer, size uintptr, addChunk func(uintptr)) { - if !isDarwin { - panic("purego: should only be called on darwin") - } for offset := uintptr(0); offset < size; offset += 8 { var chunk uintptr remaining := size - offset @@ -365,37 +249,9 @@ func copyStruct8ByteChunks(ptr unsafe.Pointer, size uintptr, addChunk func(uintp } } -// placeRegisters implements Darwin ARM64 calling convention for struct arguments. -// -// For HFA/HVA structs, each element must go in a separate register (or stack slot for elements -// that don't fit in registers). We use placeRegistersArm64 for this. -// -// For non-HFA/HVA structs, Darwin uses byte-level packing. We copy the struct memory in -// 8-byte chunks, which works correctly for both register and stack placement. +// placeRegistersDarwin uses the same register layout as other ARM64 platforms. func placeRegistersDarwin(v reflect.Value, addFloat func(uintptr), addInt func(uintptr)) { - if !isDarwin { - panic("purego: placeRegistersDarwin should only be called on darwin") - } - // Check if this is an HFA/HVA - hfa := isHFA(v.Type()) - hva := isHVA(v.Type()) - - // For HFA/HVA structs, use the standard ARM64 logic which places each element separately - if hfa || hva { - placeRegistersArm64(v, addFloat, addInt) - return - } - - // For non-HFA/HVA structs, use byte-level copying - // If the value is not addressable, create an addressable copy - if !v.CanAddr() { - addressable := reflect.New(v.Type()).Elem() - addressable.Set(v) - v = addressable - } - ptr := unsafe.Pointer(v.Addr().Pointer()) - size := v.Type().Size() - copyStruct8ByteChunks(ptr, size, addInt) + placeRegistersArm64(v, addFloat, addInt) } // shouldBundleStackArgs determines if we need to start C-style packing for diff --git a/struct_test.go b/struct_test.go index 8d3e499a..6cf661bf 100644 --- a/struct_test.go +++ b/struct_test.go @@ -960,6 +960,40 @@ func TestRegisterFunc_structArgs(t *testing.T) { t.Fatalf("IdentityCharLong returned %+v wanted %+v", ret, expected) } }) + t.Run("CharDouble", func(t *testing.T) { + // Preserve the small field before the aligned wide field. + type CharDouble struct { + _ structs.HostLayout + A int8 + B float64 + } + var fn func(CharDouble) CharDouble + register(&fn, lib, "IdentityCharDouble", func(s CharDouble) CharDouble { + return s + }) + expected := CharDouble{A: -7, B: -123.5} + if ret := fn(expected); ret != expected { + t.Fatalf("IdentityCharDouble returned %+v wanted %+v", ret, expected) + } + }) + t.Run("CharPointer", func(t *testing.T) { + // Preserve the small field before the aligned wide field. + type CharPointer struct { + _ structs.HostLayout + A int8 + B unsafe.Pointer + } + var fn func(CharPointer) CharPointer + register(&fn, lib, "IdentityCharPointer", func(s CharPointer) CharPointer { + return s + }) + value := int64(0x12345678) + expected := CharPointer{A: -7, B: unsafe.Pointer(&value)} + if ret := fn(expected); ret != expected { + t.Fatalf("IdentityCharPointer returned %+v wanted %+v", ret, expected) + } + runtime.KeepAlive(&value) + }) t.Run("CharLongBetweenPrims", func(t *testing.T) { // The struct must consume exactly two register slots so the // trailing scalar is not shifted. diff --git a/testdata/structtest/struct_test.c b/testdata/structtest/struct_test.c index 4ed672b3..257c4c67 100644 --- a/testdata/structtest/struct_test.c +++ b/testdata/structtest/struct_test.c @@ -523,3 +523,21 @@ struct NestedPadTail { int64_t SumNestedPadTail(struct NestedPadTail s) { return (int64_t) s.a.x + s.a.y + s.b; } + +struct CharDouble { + int8_t a; + double b; +}; + +struct CharDouble IdentityCharDouble(struct CharDouble s) { + return s; +} + +struct CharPointer { + int8_t a; + void * b; +}; + +struct CharPointer IdentityCharPointer(struct CharPointer s) { + return s; +}