From fe341ee95c7b76828dd8f4bd3cad94fd8414d1d0 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sun, 18 May 2025 15:18:37 -0700 Subject: [PATCH 1/4] cm, wit: fix record size calculation Ensure the final size is aligned to the record alignment. See here for more context: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size --- cm/result_test.go | 34 ++++++++++++++++++++++++++++++++++ wit/record.go | 2 +- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/cm/result_test.go b/cm/result_test.go index 70baf408..092bfa31 100644 --- a/cm/result_test.go +++ b/cm/result_test.go @@ -387,3 +387,37 @@ func TestIssue344BoolResult(t *testing.T) { t.Errorf("*v.OK(): %v, expected %v", got, want) } } + +func TestIssue350(t *testing.T) { + type Shape struct { + _ HostLayout + shape [unsafe.Sizeof(Tuple3[uint16, Result[uint64, uint64, struct{}], uint8]{})]byte + // Previously, with bug in (*Record).Size() algorithm: + // shape [unsafe.Sizeof(Option[[3]string]{})]byte + } + type O Option[[3]string] + type E Tuple3[uint16, Result[uint64, uint64, struct{}], uint8] + type T Result[Shape, O, E] + + shapeSize := unsafe.Sizeof(Shape{}) + errSize := unsafe.Sizeof(E{}) + + if errSize > shapeSize { + t.Errorf("size of err type (%d) > size of shape type (%d)", errSize, shapeSize) + + var e E + base := uintptr(unsafe.Pointer(&e)) + f0 := uintptr(unsafe.Pointer(&e.F0)) - base + f1 := uintptr(unsafe.Pointer(&e.F1)) - base + f2 := uintptr(unsafe.Pointer(&e.F2)) - base + t.Logf("Offsets: F0: %d F1: %d F2 %d", f0, f1, f2) + } + + // _ = Err[T]( + // Err, uint8]{ + // F0: 0, + // F1: OK[Result[uint64, uint64, struct{}]](0), + // F2: 0, + // }, + // ) +} diff --git a/wit/record.go b/wit/record.go index ebf9afcf..443748c6 100644 --- a/wit/record.go +++ b/wit/record.go @@ -18,7 +18,7 @@ func (r *Record) Size() uintptr { s = Align(s, f.Type.Align()) s += f.Type.Size() } - return s + return Align(s, r.Align()) } // Align returns the [ABI byte alignment] for [Record] r. From 831ae3626457f7efbf1c2e81642c5b2bd7f1b2bd Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sun, 18 May 2025 15:33:01 -0700 Subject: [PATCH 2/4] wit: update links to element size documentation --- wit/abi.go | 2 +- wit/enum.go | 2 +- wit/flags.go | 2 +- wit/future.go | 2 +- wit/list.go | 2 +- wit/option.go | 2 +- wit/pointer.go | 2 +- wit/record.go | 2 +- wit/resource.go | 4 ++-- wit/result.go | 2 +- wit/stream.go | 2 +- wit/tuple.go | 2 +- wit/variant.go | 2 +- 13 files changed, 14 insertions(+), 14 deletions(-) diff --git a/wit/abi.go b/wit/abi.go index d8dae5b1..e654439a 100644 --- a/wit/abi.go +++ b/wit/abi.go @@ -9,7 +9,7 @@ import ( // [Canonical ABI] [size], [alignment], and [flat] representation. // // [Canonical ABI]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md -// [size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size // [alignment]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#alignment // [flat]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#flattening type ABI interface { diff --git a/wit/enum.go b/wit/enum.go index a37b0980..20006ae3 100644 --- a/wit/enum.go +++ b/wit/enum.go @@ -29,7 +29,7 @@ func (e *Enum) Despecialize() TypeDefKind { // type that can represent 0...len(e.Cases). // It is first [despecialized] into a [Variant] with no associated types, then sized. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size // [despecialized]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#despecialization func (e *Enum) Size() uintptr { return e.Despecialize().Size() diff --git a/wit/flags.go b/wit/flags.go index 9ed8d058..7fc93ae0 100644 --- a/wit/flags.go +++ b/wit/flags.go @@ -11,7 +11,7 @@ type Flags struct { // Size returns the [ABI byte size] of [Flags] f. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size func (f *Flags) Size() uintptr { n := len(f.Flags) switch { diff --git a/wit/future.go b/wit/future.go index 2fa95e64..ad499c7c 100644 --- a/wit/future.go +++ b/wit/future.go @@ -12,7 +12,7 @@ type Future struct { // Size returns the [ABI byte size] for a [Future]. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size func (*Future) Size() uintptr { return 4 } // Align returns the [ABI byte alignment] a [Future]. diff --git a/wit/list.go b/wit/list.go index f7e26832..8c89c609 100644 --- a/wit/list.go +++ b/wit/list.go @@ -11,7 +11,7 @@ type List struct { // Size returns the [ABI byte size] for a [List]. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size func (*List) Size() uintptr { return 8 } // [2]int32 // Align returns the [ABI byte alignment] a [List]. diff --git a/wit/option.go b/wit/option.go index 0d5d2a28..e73c12af 100644 --- a/wit/option.go +++ b/wit/option.go @@ -27,7 +27,7 @@ func (o *Option) Despecialize() TypeDefKind { // Size returns the [ABI byte size] for [Option] o. // It is first [despecialized] into a [Variant] with two cases, "none" and "some(T)", then sized. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size // [despecialized]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#despecialization func (o *Option) Size() uintptr { return o.Despecialize().Size() diff --git a/wit/pointer.go b/wit/pointer.go index 53828960..e00d4fbc 100644 --- a/wit/pointer.go +++ b/wit/pointer.go @@ -14,7 +14,7 @@ type Pointer struct { // Size returns the [ABI byte size] for [Pointer]. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size func (*Pointer) Size() uintptr { return 4 } // Align returns the [ABI byte alignment] for [Pointer]. diff --git a/wit/record.go b/wit/record.go index 443748c6..c3c6344c 100644 --- a/wit/record.go +++ b/wit/record.go @@ -11,7 +11,7 @@ type Record struct { // Size returns the [ABI byte size] for [Record] r. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size func (r *Record) Size() uintptr { var s uintptr for _, f := range r.Fields { diff --git a/wit/resource.go b/wit/resource.go index 546c7e1f..8f4d0f5d 100644 --- a/wit/resource.go +++ b/wit/resource.go @@ -8,7 +8,7 @@ type Resource struct{ _typeDefKind } // Size returns the [ABI byte size] for [Resource]. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size func (*Resource) Size() uintptr { return 4 } // Align returns the [ABI byte alignment] for [Resource]. @@ -45,7 +45,7 @@ func (_handle) isHandle() {} // Size returns the [ABI byte size] for this [Handle]. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size func (_handle) Size() uintptr { return 4 } // Align returns the [ABI byte alignment] for this [Handle]. diff --git a/wit/result.go b/wit/result.go index f4787fa1..9ac9598b 100644 --- a/wit/result.go +++ b/wit/result.go @@ -40,7 +40,7 @@ func (r *Result) Types() []Type { // Size returns the [ABI byte size] for [Result] r. // It is first [despecialized] into a [Variant] with two cases "ok" and "error", then sized. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size // [despecialized]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#despecialization func (r *Result) Size() uintptr { return r.Despecialize().Size() diff --git a/wit/stream.go b/wit/stream.go index 609967ff..298ecca8 100644 --- a/wit/stream.go +++ b/wit/stream.go @@ -12,7 +12,7 @@ type Stream struct { // Size returns the [ABI byte size] for a [Stream]. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size func (*Stream) Size() uintptr { return 4 } // Align returns the [ABI byte alignment] a [Stream]. diff --git a/wit/tuple.go b/wit/tuple.go index 30d51403..af819104 100644 --- a/wit/tuple.go +++ b/wit/tuple.go @@ -48,7 +48,7 @@ func (t *Tuple) Despecialize() TypeDefKind { // Size returns the [ABI byte size] for [Tuple] t. // It is first [despecialized] into a [Record] with 0-based integer field names, then sized. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size // [despecialized]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#despecialization func (t *Tuple) Size() uintptr { return t.Despecialize().Size() diff --git a/wit/variant.go b/wit/variant.go index efdcbee6..37a9b64b 100644 --- a/wit/variant.go +++ b/wit/variant.go @@ -46,7 +46,7 @@ func (v *Variant) Types() []Type { // Size returns the [ABI byte size] for [Variant] v. // -// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#size +// [ABI byte size]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size func (v *Variant) Size() uintptr { s := Discriminant(len(v.Cases)).Size() s = Align(s, v.maxCaseAlign()) From bb7256fa54f88a711fe17f23018ce9bafde81989 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sun, 18 May 2025 15:45:39 -0700 Subject: [PATCH 3/4] wit: add additional ABI size and alignment tests for record and variant types --- wit/abi_test.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/wit/abi_test.go b/wit/abi_test.go index a6b89a01..66c98001 100644 --- a/wit/abi_test.go +++ b/wit/abi_test.go @@ -99,7 +99,7 @@ func TestDiscriminant(t *testing.T) { } } -func TestTypeSize(t *testing.T) { +func TestTypeSizeAndAlign(t *testing.T) { tests := []struct { name string v Type @@ -119,6 +119,10 @@ func TestTypeSize(t *testing.T) { {"f64", F64{}, 8, 8}, {"char", Char{}, 4, 4}, {"string", String{}, 8, 4}, + {"option", &TypeDef{Kind: &Option{Type: String{}}}, 12, 4}, + {"option", &TypeDef{Kind: &Option{Type: F32{}}}, 8, 4}, + {"variant", &TypeDef{Kind: &Variant{Cases: []Case{{Type: String{}}, {Type: F64{}}}}}, 16, 8}, + {"record", &TypeDef{Kind: &Record{Fields: []Field{{Type: U16{}}, {Type: &TypeDef{Kind: &Result{OK: U64{}}}}, {Type: U8{}}}}}, 32, 8}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -156,6 +160,7 @@ func TestTypeFlat(t *testing.T) { {"option", &TypeDef{Kind: &Option{Type: String{}}}, []Type{U32{}, PointerTo(U8{}), U32{}}}, {"option", &TypeDef{Kind: &Option{Type: F32{}}}, []Type{U32{}, F32{}}}, {"variant", &TypeDef{Kind: &Variant{Cases: []Case{{Type: String{}}, {Type: F64{}}}}}, []Type{U32{}, U64{}, U32{}}}, + {"record", &TypeDef{Kind: &Record{Fields: []Field{{Type: U16{}}, {Type: &TypeDef{Kind: &Result{OK: U64{}}}}, {Type: U8{}}}}}, []Type{U32{}, U32{}, U64{}, U32{}}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -177,9 +182,6 @@ func witFor[T Node](nodes ...T) []string { // TestHasBorrow verifies that HasBorrow returns true for WIT types that contain a Borrow type. func TestHasBorrow(t *testing.T) { - makeBorrow := func() *TypeDef { return &TypeDef{Kind: &Borrow{}} } - makeTypeDef := func(kind TypeDefKind) *TypeDef { return &TypeDef{Kind: kind} } - testCases := []struct { name string typeDef *TypeDef @@ -211,3 +213,11 @@ func TestHasBorrow(t *testing.T) { }) } } + +func makeTypeDef(kind TypeDefKind) *TypeDef { + return &TypeDef{Kind: kind} +} + +func makeBorrow() *TypeDef { + return &TypeDef{Kind: &Borrow{}} +} From cf9588bd42822ebfab49928c661c51cc499c1fbb Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sun, 18 May 2025 15:47:54 -0700 Subject: [PATCH 4/4] CHANGELOG: note fix to #350 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 383ba3eb..4c2492c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed - [#306](https://github.com/bytecodealliance/go-modules/issues/306): do not emit incompatible version feature gates when serializing WIT. For example, if a world with version `0.1.0` *includes* a world from another package with a `@since(version = 0.2.0)` feature gate, then strip that feature gate when serializing to WIT if the version is greater than the world’s package version. +- [#350](https://github.com/bytecodealliance/go-modules/issues/350): correctly align `record` size to the highest alignment of its fields, per [specification](https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#element-size). ## [v0.6.2] — 2025-03-16