-
Notifications
You must be signed in to change notification settings - Fork 16
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. |
@@ -29,7 +29,7 @@ func Some[T any](v T) Option[T] { | |||
// followed by storage for the associated type T. | |||
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
.)
@@ -47,6 +47,10 @@ func (o *option[T]) Some() *T { | |||
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 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
basic
example fromwasi-http-go
working 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_realloc
were blocking startup of any component, the new commits from today solve those. They require two things though, one of which is unfortunate:go build
needs to be invoked with-ldflags=-checklinkname=0
to be able to use the internalruntime.sbrk
function. The reason for this is documented inx/realloc.go
.x/cabi
needs to be imported somewhere forcabi_realloc
to be available at runtime. I think this can easily be addressed by adding that import to any*.exports.go
file 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.init
that 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, aresbrk
to be linked by adding a//go:linkname
annotation 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_malloc
itself 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
authority
method onhttp/incoming-request
as the example:With the above applied on top of re-generating the bindings with this PR's changes, the
basic
example successfully returns"Hello world"
for me. It doesn't properly apply thex-go
header, since that requires loweringList[FieldValue]
in the right way.STR
As mentioned, with these patches
wasi-http-go
compiles successfully as long as-ldflags=-checklinkname=0
is passed togo build
. To actually run it, two more steps are needed:wasm-tools component embed
for thiswasm-tools component new
for 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
.