diff --git a/CHANGELOG.md b/CHANGELOG.md index dbd87a60..1509f5c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), - Breaking: generated `*.wasm.go` files will now have correct WIT kebab-case base name. Interfaces or worlds with `-` in their name will require removal of the previous `*.wasm.go` files. - Dropped support for TinyGo v0.32.0. +### Fixed + +- [#284](https://github.com/bytecodealliance/go-modules/issues/284): do not use `bool` for `variant` or `result` GC shapes. TinyGo returns `result` and `variant` values with `bool` as 0 or 1, which breaks the memory representation of tagged unions (variants). + ## [v0.5.0] — 2024-12-14 ### Changed diff --git a/Makefile b/Makefile index 954b76ef..5b0f0bb2 100644 --- a/Makefile +++ b/Makefile @@ -53,3 +53,7 @@ test: tinygo test -target=wasip1 $(GOTESTARGS) $(GOTESTMODULES) tinygo test -target=wasip2 $(GOTESTARGS) $(GOTESTMODULES) tinygo test -target=wasip2 $(GOTESTARGS) ./tests/... + +.PHONY: tests +tests: + tinygo test -target=wasip2 $(GOTESTARGS) ./tests/... diff --git a/cm/result_test.go b/cm/result_test.go index 759c03ea..ee716bc6 100644 --- a/cm/result_test.go +++ b/cm/result_test.go @@ -1,6 +1,7 @@ package cm import ( + "fmt" "runtime" "testing" "unsafe" @@ -238,3 +239,98 @@ func TestIssue95BoolInt64(t *testing.T) { t.Errorf("*res.OK(): %v, expected %v", got, want) } } + +func TestIssue284(t *testing.T) { + // Using int8 instead of bool for shape works on Go and TinyGo. + // See https://github.com/bytecodealliance/go-modules/issues/284 + type BoolS8Result Result[int8, bool, int8] + + tests := []struct { + isErr bool + ok bool + err int8 + }{ + {false, false, 0}, + {false, true, 0}, + {true, false, 0}, + {true, false, 1}, + {true, false, 5}, + {true, false, 126}, + {true, false, 127}, + } + for _, tt := range tests { + if !tt.isErr { + t.Run(fmt.Sprintf("OK[BoolS8Result](%t)", tt.ok), func(t *testing.T) { + r := OK[BoolS8Result](tt.ok) + ok, _, isErr := r.Result() + if isErr != tt.isErr { + t.Errorf("isErr == %t, expected %t", isErr, tt.isErr) + } + if ok != tt.ok { + t.Errorf("ok == %t, expected %t", ok, tt.ok) + } + }) + } else { + t.Run(fmt.Sprintf("Err[BoolS8Result](%d)", tt.err), func(t *testing.T) { + r := Err[BoolS8Result](tt.err) + _, err, isErr := r.Result() + if isErr != tt.isErr { + t.Errorf("isErr == %t, expected %t", isErr, tt.isErr) + } + if err != tt.err { + t.Errorf("err == %d, expected %d", err, tt.err) + } + }) + } + } +} + +func TestIssue284NotTinyGo(t *testing.T) { + if runtime.Compiler == "tinygo" { + return + } + + // Using bool as result shape changes how [OK] returns the result by value. + // This works on Go, but breaks on TinyGo / LLVM. + // See https://github.com/bytecodealliance/go-modules/issues/284 + type BoolS8Result Result[bool, bool, int8] + + tests := []struct { + isErr bool + ok bool + err int8 + }{ + {false, false, 0}, + {false, true, 0}, + {true, false, 0}, + {true, false, 1}, + {true, false, 5}, + {true, false, 126}, + {true, false, 127}, + } + for _, tt := range tests { + if !tt.isErr { + t.Run(fmt.Sprintf("OK[BoolS8Result](%t)", tt.ok), func(t *testing.T) { + r := OK[BoolS8Result](tt.ok) + ok, _, isErr := r.Result() + if isErr != tt.isErr { + t.Errorf("isErr == %t, expected %t", isErr, tt.isErr) + } + if ok != tt.ok { + t.Errorf("ok == %t, expected %t", ok, tt.ok) + } + }) + } else { + t.Run(fmt.Sprintf("Err[BoolS8Result](%d)", tt.err), func(t *testing.T) { + r := Err[BoolS8Result](tt.err) + _, err, isErr := r.Result() + if isErr != tt.isErr { + t.Errorf("isErr == %t, expected %t", isErr, tt.isErr) + } + if err != tt.err { + t.Errorf("err == %d, expected %d", err, tt.err) + } + }) + } + } +} diff --git a/tests/generated/issues/issue284/i/i.wit.go b/tests/generated/issues/issue284/i/i.wit.go new file mode 100755 index 00000000..38ba1eb2 --- /dev/null +++ b/tests/generated/issues/issue284/i/i.wit.go @@ -0,0 +1,13 @@ +// Code generated by wit-bindgen-go. DO NOT EDIT. + +// Package i represents the imported interface "issues:issue284/i". +package i + +import ( + "go.bytecodealliance.org/cm" +) + +// BoolS8Result represents the result "issues:issue284/i#bool-s8-result". +// +// type bool-s8-result = result +type BoolS8Result cm.Result[int8, bool, int8] diff --git a/tests/generated/issues/issue284/w/w.wit.go b/tests/generated/issues/issue284/w/w.wit.go new file mode 100755 index 00000000..8d8f46ab --- /dev/null +++ b/tests/generated/issues/issue284/w/w.wit.go @@ -0,0 +1,4 @@ +// Code generated by wit-bindgen-go. DO NOT EDIT. + +// Package w represents the world "issues:issue284/w". +package w diff --git a/tests/generated/wasi/sockets/v0.2.0/tcp/abi.go b/tests/generated/wasi/sockets/v0.2.0/tcp/abi.go index 2788dbeb..eb335d00 100755 --- a/tests/generated/wasi/sockets/v0.2.0/tcp/abi.go +++ b/tests/generated/wasi/sockets/v0.2.0/tcp/abi.go @@ -23,7 +23,7 @@ type TupleInputStreamOutputStreamShape struct { // IPSocketAddressShape is used for storage in variant or result types. type IPSocketAddressShape struct { _ cm.HostLayout - shape [unsafe.Sizeof(network.IPSocketAddress{})]byte + shape [unsafe.Sizeof(IPSocketAddress{})]byte } func lower_IPv4Address(v network.IPv4Address) (f0 uint32, f1 uint32, f2 uint32, f3 uint32) { diff --git a/tests/generated/wasi/sockets/v0.2.0/tcp/tcp.wasm.go b/tests/generated/wasi/sockets/v0.2.0/tcp/tcp.wasm.go index 56247525..d6e56c26 100755 --- a/tests/generated/wasi/sockets/v0.2.0/tcp/tcp.wasm.go +++ b/tests/generated/wasi/sockets/v0.2.0/tcp/tcp.wasm.go @@ -46,7 +46,7 @@ func wasmimport_TCPSocketKeepAliveCount(self0 uint32, result *cm.Result[uint32, //go:wasmimport wasi:sockets/tcp@0.2.0 [method]tcp-socket.keep-alive-enabled //go:noescape -func wasmimport_TCPSocketKeepAliveEnabled(self0 uint32, result *cm.Result[bool, bool, ErrorCode]) +func wasmimport_TCPSocketKeepAliveEnabled(self0 uint32, result *cm.Result[ErrorCode, bool, ErrorCode]) //go:wasmimport wasi:sockets/tcp@0.2.0 [method]tcp-socket.keep-alive-idle-time //go:noescape diff --git a/tests/generated/wasi/sockets/v0.2.0/tcp/tcp.wit.go b/tests/generated/wasi/sockets/v0.2.0/tcp/tcp.wit.go index ff669440..f43fd44f 100755 --- a/tests/generated/wasi/sockets/v0.2.0/tcp/tcp.wit.go +++ b/tests/generated/wasi/sockets/v0.2.0/tcp/tcp.wit.go @@ -285,7 +285,7 @@ func (self TCPSocket) KeepAliveCount() (result cm.Result[uint32, uint32, ErrorCo // keep-alive-enabled: func() -> result // //go:nosplit -func (self TCPSocket) KeepAliveEnabled() (result cm.Result[bool, bool, ErrorCode]) { +func (self TCPSocket) KeepAliveEnabled() (result cm.Result[ErrorCode, bool, ErrorCode]) { self0 := cm.Reinterpret[uint32](self) wasmimport_TCPSocketKeepAliveEnabled((uint32)(self0), &result) return diff --git a/tests/generated/wasi/sockets/v0.2.0/udp/abi.go b/tests/generated/wasi/sockets/v0.2.0/udp/abi.go index 7e44e2f2..3dfaa27f 100755 --- a/tests/generated/wasi/sockets/v0.2.0/udp/abi.go +++ b/tests/generated/wasi/sockets/v0.2.0/udp/abi.go @@ -11,7 +11,7 @@ import ( // IPSocketAddressShape is used for storage in variant or result types. type IPSocketAddressShape struct { _ cm.HostLayout - shape [unsafe.Sizeof(network.IPSocketAddress{})]byte + shape [unsafe.Sizeof(IPSocketAddress{})]byte } func lower_IPv4Address(v network.IPv4Address) (f0 uint32, f1 uint32, f2 uint32, f3 uint32) { diff --git a/wit/bindgen/abi.go b/wit/bindgen/abi.go index 5b4f4d37..85fd28f4 100644 --- a/wit/bindgen/abi.go +++ b/wit/bindgen/abi.go @@ -6,7 +6,7 @@ import ( "go.bytecodealliance.org/wit" ) -// variantShape returns the type with the greatest size. +// variantShape returns the type with the greatest size that is not a bool. // If there are multiple types with the same size, it returns // the first type that contains a pointer. func variantShape(types []wit.Type) wit.Type { @@ -19,6 +19,12 @@ func variantShape(types []wit.Type) wit.Type { return -1 case a.Size() < b.Size(): return 1 + case !isBool(a) && isBool(b): + // bool cannot be used as variant shape + // See https://github.com/bytecodealliance/go-modules/issues/284 + return -1 + case isBool(a) && !isBool(b): + return 1 case wit.HasPointer(a) && !wit.HasPointer(b): return -1 case !wit.HasPointer(a) && wit.HasPointer(b): @@ -30,6 +36,16 @@ func variantShape(types []wit.Type) wit.Type { return types[0] } +func isBool(t wit.TypeDefKind) bool { + switch t := t.(type) { + case wit.Bool: + return true + case *wit.TypeDef: + return isBool(t.Root().Kind) + } + return false +} + // variantAlign returns the type with the largest alignment. func variantAlign(types []wit.Type) wit.Type { if len(types) == 0 { diff --git a/wit/bindgen/generator.go b/wit/bindgen/generator.go index 220dcad3..553b42f4 100644 --- a/wit/bindgen/generator.go +++ b/wit/bindgen/generator.go @@ -1018,7 +1018,7 @@ func (g *generator) typeShape(file *gen.File, dir wit.Direction, t wit.Type) str } func (g *generator) typeDefShape(file *gen.File, dir wit.Direction, t *wit.TypeDef) string { - switch kind := t.Kind.(type) { + switch kind := t.Root().Kind.(type) { case wit.Type: return g.typeShape(file, dir, kind) case *wit.Variant: