-
Notifications
You must be signed in to change notification settings - Fork 20
WIP: Support wasip2 in Go 1.24 #367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
WIP: Support wasip2 in Go 1.24 #367
Conversation
Signed-off-by: Till Schneidereit <[email protected]>
I'm not entirely sure what changed these files, but it's a result of running tests via the Makefile, as best I can tell. Signed-off-by: Till Schneidereit <[email protected]>
Otherwise no `cabi_realloc` exists in Go. Signed-off-by: Till Schneidereit <[email protected]>
…,ex}port` Go has slightly tighter constraints on `wasmimport` and `wasmexport` than TinyGo, which this commit works around: - It disallows use of `*string` (as opposed to `string`) - It disallows use of `*cm.List` (probably because of the `[any]`, but I'm not sure at all) Signed-off-by: Till Schneidereit <[email protected]>
Signed-off-by: Till Schneidereit <[email protected]>
Signed-off-by: Till Schneidereit <[email protected]>
|
The basic approach looks good to me! I'm actually solving a very similar problem over in fastly/Viceroy#491, except for TinyGo, and with the added constraint that it must not require rebuilding existing TinyGo programs (for the sake of Fastly Compute's customers). Basically, I'm injecting a |
|
Thanks! I'm thrilled to see this work. Have you looked at the GOARCH=wasm32 port yet? It would cleanly address the underlying ABI issues here pointing to structs with pointers. It needs someone to complete the work and land it in big Go. |
| type option[T any] struct { | ||
| _ HostLayout | ||
| isSome uint8 | ||
| isSome uint32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
This will break ABI compatibility on TinyGo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically the uint8 isn't correct WRT the CM's canonical ABI, hence the change. Can you say how this breaks things on TinyGo? I'd assume that new code using newly generated bindings would continue to work as before?
(To be clear, I think it's fine to revert this, since the relevant bit will always reside in the same byte, whether the rest is filled with padding or is represented by the uint32.)
| return nil | ||
| } | ||
|
|
||
| func (o option[T]) IsSome() bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This grows the public API of Option[T]. Would consider this as a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Entirely okay to revert.
| b.WriteString(p.name) | ||
| if wit.HasPointer(t) { | ||
| file.Import("unsafe") | ||
| stringio.Write(&b, "unsafe.Pointer(&", p.name, ")") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use the return value of file.Import. The short package name might not be unsafe.
Excellent!
I'm aware of that, yes. In various conversations I had gotten the strong impression that people saw component model support as being hard-blocked on that port; this PR is the result of me trying to understand why that'd be the case—and concluding that it's not. The reason I was interested in it is that it seems like no progress has been made on it for a long while now, and AFAICT, it's not currently on track for making the February release. So it seems like waiting for that port to materialize puts us on an August 2026 timeline.
Agreed, yes. Same as with turning this PR from a PoC into a solution, that someone will unfortunately not be me: I have neither the bandwidth, nor even remotely the expertise to be particularly efficient at it. |
This adds some more Options that can be handled by the factory, such as: - providing custom Wasm bytes, rather than using the embedded ones - instantiating the WASI snapshot module in the runtime, required for WASI - providing a custom module config, e.g. to enable stdin/stdout when using WASI, or to customise the 'Start' function used by Wazero It also adds a new example which uses a guest module built using Go (big Go, not TinyGo, although that would also work). This example requires a few of these options to be configured - see the new basic_test.go file for details. Specifically it requires: - WASI enabled - architecture set to wasm64 (because big Go compiles to wasm64) - a custom module config with the `_initialize` function added to its list of start functions The Go guest module is generated using [this branch] of `wit-bindgen-go`. The custom branch is required because otherwise the generated Go code doesn't compile, of limitations with Go's //go:wasmexport functionality. There's a script (`gen-guest.sh`) that runs the `wit-bindgen-go` command. Go compilation to Wasm is then done in the same generate.go file as all the other examples. [this branch]: bytecodealliance/go-modules#367
…ibility This implements a complete fix for the wasm64 pointer size mismatch issue identified in PR bytecodealliance#367. On wasm64, Go uses 64-bit pointers but the Component Model ABI uses 32-bit values exclusively. When functions return pointer- containing types (strings, lists, options, results), the mismatch causes data corruption and "unknown handle index 0" errors. - area.go: Add RetArea1-16 types with uint32 fields for receiving 32-bit ABI return values with proper HostLayout alignment - lift.go: Implement lift/lower functions for converting between 32-bit ABI values and Go types: - LiftString32, LiftList32: Convert i32 pointers to native pointers - LiftOptionString32, LiftOptionList32: Lift option<T> from ABI - LiftListWithConverter32: Deep conversion for nested pointer types - LiftTupleWithPointers32: Convert tuples with 32-bit pointer fields - Result lifting helpers for proper discriminant/payload handling - wasm64.go: Add IsWasm64 constant for platform detection - lift_test.go: Comprehensive tests for all lift/lower functions - Add wasm64 RetArea detection and code generation: - needsWasm64RetArea(): Detect pointer-containing return types - canUseRetAreaForType(): Validate types work with uint32 fields - calculateRetAreaSize(): Count fields needed including padding - generateWasm64LiftCall(): Generate specialized lift code - Support for complex nested types: - Deep conversion for list<tuple<T,U>> with nested pointers - Proper handling of monotypic vs heterogeneous tuples - Inline converters for compound types in lists - Result type support with proper alignment: - Remove Result exclusion from RetArea - Implement generateResultLiftCall() for result<T,E> types - Calculate payload offset based on Canonical ABI alignment rules - Handle simple variants (<=2 cases) with inline if/else - Use Reinterpret fallback for complex variants (>2 cases) - Support for empty payloads, enums, resources, and nested types Before (broken on wasm64): wasmimport_Function(unsafe.Pointer(&result)) return After (correct for wasm64): wasmimport_Function(unsafe.Pointer(&cm.Area3)) return cm.LiftString32[string](cm.Area3.F1, cm.Area3.F2) For complex types like result<list<u8>, stream-error>: return func() cm.Result[...] { disc := cm.Area4.F1 if disc == 0 { return cm.OK[...](cm.LiftList32[uint8](cm.Area4.F2, cm.Area4.F3)) } else { return cm.Err[...](/* variant lifting */) } }() The fix implements the "Area/RetArea" pattern suggested in PR bytecodealliance#367: 1. Global RetArea variables receive 32-bit ABI values from host 2. Specialized lift functions convert from 32-bit to native layout 3. Proper alignment calculation based on Canonical ABI spec 4. Deep conversion for nested structures with pointer fields Alignment calculation follows Component Model spec: - For Result types: payload offset = align_to(discriminant_size, max_case_align) - Discriminant is u8 (1 byte) for 2-case results - Payload aligned to max(align(OK), align(Err)) - Converts byte offset to uint32 field index Works on ALL platforms: - wasm32 (TinyGo): Correct, small overhead (~5%) - wasm64 (big Go): Correct, fixes critical bugs - Native (testing): Correct, tests pass The Component Model ABI uses i32 pointers even on wasm64, so RetArea with uint32 fields is correct for all platforms. - All 100+ generator tests pass - All cm package tests pass - Real-world validation: HTTP requests to example.com succeed on wasm64 (previously failed with "unknown handle index 0") - No regressions on wasm32 Fixes the wasm64 pointer size mismatch issue from PR bytecodealliance#367. Co-authored-by: Claude <[email protected]>
…ibility This implements a complete fix for the wasm64 pointer size mismatch issue identified in PR bytecodealliance#367. On wasm64, Go uses 64-bit pointers but the Component Model ABI uses 32-bit values exclusively. When functions return pointer- containing types (strings, lists, options, results), the mismatch causes data corruption and "unknown handle index 0" errors. - area.go: Add RetArea1-16 types with uint32 fields for receiving 32-bit ABI return values with proper HostLayout alignment - lift.go: Implement lift/lower functions for converting between 32-bit ABI values and Go types: - LiftString32, LiftList32: Convert i32 pointers to native pointers - LiftOptionString32, LiftOptionList32: Lift option<T> from ABI - LiftListWithConverter32: Deep conversion for nested pointer types - LiftTupleWithPointers32: Convert tuples with 32-bit pointer fields - Result lifting helpers for proper discriminant/payload handling - wasm64.go: Add IsWasm64 constant for platform detection - lift_test.go: Comprehensive tests for all lift/lower functions - Add wasm64 RetArea detection and code generation: - needsWasm64RetArea(): Detect pointer-containing return types - canUseRetAreaForType(): Validate types work with uint32 fields - calculateRetAreaSize(): Count fields needed including padding - generateWasm64LiftCall(): Generate specialized lift code - Support for complex nested types: - Deep conversion for list<tuple<T,U>> with nested pointers - Proper handling of monotypic vs heterogeneous tuples - Inline converters for compound types in lists - Result type support with proper alignment: - Remove Result exclusion from RetArea - Implement generateResultLiftCall() for result<T,E> types - Calculate payload offset based on Canonical ABI alignment rules - Handle simple variants (<=2 cases) with inline if/else - Use Reinterpret fallback for complex variants (>2 cases) - Support for empty payloads, enums, resources, and nested types Before (broken on wasm64): wasmimport_Function(unsafe.Pointer(&result)) return After (correct for wasm64): wasmimport_Function(unsafe.Pointer(&cm.Area3)) return cm.LiftString32[string](cm.Area3.F1, cm.Area3.F2) For complex types like result<list<u8>, stream-error>: return func() cm.Result[...] { disc := cm.Area4.F1 if disc == 0 { return cm.OK[...](cm.LiftList32[uint8](cm.Area4.F2, cm.Area4.F3)) } else { return cm.Err[...](/* variant lifting */) } }() The fix implements the "Area/RetArea" pattern suggested in PR bytecodealliance#367: 1. Global RetArea variables receive 32-bit ABI values from host 2. Specialized lift functions convert from 32-bit to native layout 3. Proper alignment calculation based on Canonical ABI spec 4. Deep conversion for nested structures with pointer fields Alignment calculation follows Component Model spec: - For Result types: payload offset = align_to(discriminant_size, max_case_align) - Discriminant is u8 (1 byte) for 2-case results - Payload aligned to max(align(OK), align(Err)) - Converts byte offset to uint32 field index Works on ALL platforms: - wasm32 (TinyGo): Correct, small overhead (~5%) - wasm64 (big Go): Correct, fixes critical bugs - Native (testing): Correct, tests pass The Component Model ABI uses i32 pointers even on wasm64, so RetArea with uint32 fields is correct for all platforms. - All 100+ generator tests pass - All cm package tests pass - Real-world validation: HTTP requests to example.com succeed on wasm64 (previously failed with "unknown handle index 0") - No regressions on wasm32 Fixes the wasm64 pointer size mismatch issue from PR bytecodealliance#367. Co-authored-by: Claude <[email protected]>
This set of commits allows me to successfully build and start, but not actually use without blocking issues, the examples from https://github.com/ydnar/wasi-http-go.
The blocking issues by now should all be solvable with changes to bindings generation, as described below. In particular, I was able to get the
basicexample fromwasi-http-goworking with light manual changes to the generated bindings.Big caveat upfront: this is the first Go code I ever wrote, and I have basically no idea whatsoever what I'm doing. That means I'm decidedly not the right person to make the required changes to bindings generation.
[I updated this description since opening the PR, since I worked through the key blocking issues.]
Where before, issues around
cabi_reallocwere blocking startup of any component, the new commits from today solve those. They require two things though, one of which is unfortunate:go buildneeds to be invoked with-ldflags=-checklinkname=0to be able to use the internalruntime.sbrkfunction. The reason for this is documented inx/realloc.go.x/cabineeds to be imported somewhere forcabi_reallocto be available at runtime. I think this can easily be addressed by adding that import to any*.exports.gofile during bindings generation.Other remaining issues
Go runtime
As described in the comments in
realloc.go, the Go runtime makes calls to CM imports during its own initialization, before the GC subsystem has been initialized. The two calls underruntime.initthat cause calls to imports areticks.init()andrandinit(). Given the constraints documented there, the ordering seems pretty fundamental, unfortunately.So to be able to build without
-ldflags=-checklinkname=0, options I see, roughly in ascending order of complexity, aresbrkto be linked by adding a//go:linknameannotation to it. This would only be needed for the wasm port, so might be acceptable.cabi_malloc, and nothing else, only usable during runtime initialization.cabi_mallocitself to the runtime.Bindings
The generated bindings still assume in various places that pointers are 32-bit. This is actually okay for any pointers directly passed as arguments or returned as top-level values. Where it breaks down is when a pointer is embedded into another structure, as is the case for
string, or any kind of struct, list, etc.To address this, the bindings need to be changed to convert any data structures that embed pointers into equivalent ones containing 32-bit versions of the pointers—and the inverse for return values.
This can be done with reused slices for most cases (basically all except for lists of structures with pointers, such as
List[string], since those require an unbounded number of slices) by creating slices to use for lifting/lowering at initialization time and reusing them across calls.Here's a PoC of how this can work for return values, with the
authoritymethod onhttp/incoming-requestas the example:With the above applied on top of re-generating the bindings with this PR's changes, the
basicexample successfully returns"Hello world"for me. It doesn't properly apply thex-goheader, since that requires loweringList[FieldValue]in the right way.STR
As mentioned, with these patches
wasi-http-gocompiles successfully as long as-ldflags=-checklinkname=0is passed togo build. To actually run it, two more steps are needed:wasm-tools component embedfor thiswasm-tools component newfor this, with the p1 adapter from the Wasmtime 33 release for this.The result of these steps is successfully loaded with
wasmtime serve -S cli.