golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.92k stars 17.65k forks source link

cmd/compile: add go:wasmexport directive #65199

Closed johanbrandhorst closed 2 months ago

johanbrandhorst commented 9 months ago

Background

38248 defined a new compiler directive, go:wasmimport, for interfacing with host defined functions. This allowed calling from Go code into host functions, but it’s still not possible to call from the WebAssembly (Wasm) host into Go code.

Some applications have adopted the practice of allowing them to be extended by calling into Wasm compiled code according to some well defined ABI. Examples include Envoy, Istio, VS Code and others. Go cannot support compiling code to these applications, as the only exported function in the module compiled by Go is _start, mapping to the main function in a main package.

Despite this, some users are designing custom plugin systems using this interface, utilizing standard in and standard out for communicating with the Wasm binary. This shows a desire for exporting Go functions in the community.

There have been historical discussions on implementing this before (including #42372, #25612 and #41715), but none of them have reached a consensus on a design and implementation. In particular, #42372 had a long discussion (and design doc) that never provided a satisfying answer for how to run executed functions in the Go runtime. Instead of reviving that discussion, this proposal will attempt to build on it and answer the questions posed. This proposal supersedes #42372.

Exporting functions to the wasm host is also a necessity for a hypothetical GOOS=wasip2 targeting preview 2 of the WASI specification. This could be implemented as a special case in the compiler but since this is a feature requested by users it could reuse that functionality (similar to go:wasmimport today).

Proposal

Repurpose the -buildmode build flag value c-shared for the wasip1 port. It now signals to the compiler to replace the _start function with an _initialize function, which performs runtime and package initialization.

Add a new compiler directive, go:wasmexport, which is used to signal to the compiler that a function should be exported using a Wasm export in the resulting Wasm binary. Using the compiler directive will result in a compilation failure unless the target GOOS is wasip1.

There is a single ~optional~ required parameter to the directive, defining the name of the exported function: (UPDATE: make the parameter required, consistent with the //export pragma and easier to implement).

//go:wasmexport name

The directive is only allowed on functions, not methods.

Discussion

Parallel with -buildmode=c-shared and CGO

The proposed implementation is inspired by the implementation of C references to Go functions. When an exported function is called, a new goroutine (G) is created, which executes on a single thread (M), since Wasm is a single threaded architecture. The runtime will wake up and resume scheduling goroutines as necessary, with the exported function being one of the goroutines available for scheduling. Any other goroutines started during package initialization or left over from previous exported function executions will also be available for scheduling.

Why a "-buildmode" option?

The wasi_snapshot_preview1 documentation states that a _start function and an _initialize function are mutually exclusive. Additionally, at the end of the current _start functions as compiled by Go, proc_exit is called. At this point, the module is considered done, and cannot be interacted with. Given these conditions, we need some way for a user to declare that they want to build a binary especially for exporting one or more functions and to include the _initialize function for package and runtime initialization.

We also considered using a GOWASM option instead, but this feels wrong since that environment variable is used to specify options relating to the architecture (existing options are satconv and signext), while this export option is dependent on the behavior of the "OS" (what functions to export, what initialization pattern to expect).

What happens to func main when exports are involved?

Go code compiled to a wasip1 Wasm binary can be either a "Command", which includes the _start function, or a "Reactor/Library", which includes the _initialize function.

When using -buildmode=c-shared, the resulting Wasm binary will not contain a _start function, and will only contain the _initialize function and any exported functions. The Go main function will not be exported to the host. The user can choose to export it like any other function using the //go:wasmexport directive. The _initialize function will not automatically call main. The main function will not initialize the runtime.

When the -buildmode flag is unset, the _start function and any exported functions will be exported to the host. Using //go:wasmexport on the main function in this mode will result in a compilation error. In this mode, only _start will initialize the runtime, and so must be the first export called from the host. Any other exported functions may only be called through calling into host functions that call other exports during the execution of the _start function. Once the _start function has returned, no other exports may be called on the same instance.

Why not reuse //export?

//export is used to export Go functions to C when using buildmode=c-shared. Use of //export puts restrictions on the use of the file, namely that it cannot contain definitions, only declarations. It’s also something of an ugly duckling among compiler directives in that it doesn’t use the now established go: prefix. A new directive removes the need for users to define functions separately from the declaration, has a nice symmetry with go:wasmimport, and uses the well established go: prefix.

Handling Reentrant Calls and Panics

Reentrant calls happen when the Go application calls a host import, and that invocation calls back into an exported function. Reentrant calls are handled by creating a new goroutine. If a panic reaches the top-level of the go:wasmexport call, the program crashes because there are no mechanisms allowing the guest application to propagate the panic to the Wasm host.

Naming exports

When the name of the Go function matches that of the desired Wasm export, the name parameter can be omitted.

For example:

//go:wasmexport add
func add(x, y int) int {
    return x + y
}

Is equivalent to

//go:wasmexport
func add(x, y int) int {
    return x + y
}

The names _start and _initialize are reserved and not available for user exported functions.

Third-party libraries

Third-party libraries will need to be able to define exports, as WASI functionality such as wasi-http requires calling into exported functions, which would be provided by the third party library in a user-friendly wrapper. Any exports defined in third party libraries are compiled to exported Wasm functions.

Module names

The current Wasm architecture doesn’t define a module name of the compiled module, and this proposal does not suggest adding one. Module names are useful to namespace different compiled Wasm binaries, but it can usually be configured by the runtime or using post-processing tools on the binaries. Future proposals may suggest some way to build this into the Go build system, but this proposal suggests not naming it for simplicity.

Conflicting exports

If the compiler detects multiple exports using the same name, a compile error will occur and warn the user that multiple definitions are in conflict. This may have to happen at link time. If this happens in third-party libraries the user has no recourse but to avoid using one of the libraries.

Supported Types

The go:wasmimport directive allows the declaration of host imports by naming the module and function that the application depends on. The directive applies restrictions on the types that can be used in the function signatures, limiting to fixed-size integers and floats, and unsafe.Pointer, which allows simple mapping rules between the Go and Wasm types. The go:wasmexport directive will use the same type restrictions. Any future relaxing of this restriction will be subject to a separate proposal.

Spawning Goroutines from go:wasmexport functions

The proposal considers scenarios where the go:wasmexport call spawns new goroutines. In the absence of threading or stack switching capability in Wasm, the simplest option is to document that all goroutines still running when the invocation of the go:wasmexport function returns will be paused until the control flow re-enters the Go application.

In the future, we anticipate that Wasm will gain the ability to either spawn threads or integrate with the event loop of the host runtime (e.g., via stack-switching) to drive background goroutines to completion after the invocation of a go:wasmexport function has returned.

Blocking in go:wasmexport functions

When the goroutine running the exported function blocks for any reason, the function will yield to the Go runtime. The Go runtime will schedule other goroutines as necessary. If there are no other goroutines, the application will crash with a deadlock, as there is no way to proceed, and Wasm code cannot block.

Authors

@johanbrandhorst, @achille-roussel, @Pryz, @dgryski, @evanphx, @neelance, @mdlayher

Acknowledgements

Thanks to all participants in the go:wasmexport discussion at the Go contributor summit at GopherCon 2023, without which this proposal would not have been possible.

CC @golang/wasm @cherrymui

cherrymui commented 9 months ago

Since the proposal suggests causing compilation errors when //go:wasmexport is used when compiling "Commands"

Based on the discussion above https://github.com/golang/go/issues/65199#issuecomment-1924931088 , I think we agreed that we eventually want to support wasmexport for command. It would be great if we could just support both library and command at same time (personally I think the implementation would not be very different so it shouldn't too hard). But if you prefer supporting library first, then command later, that is probably fine. But maybe we don't want that to cause an error, which complicates things (like you mentioned, you may want a build tag). Maybe we document that wasmexport for command will be supported in the future but is ignored for now.

Given that Wasm's execution model is very different from other architectures, I think either c-archive or c-shared is probably fine. Is it possible for a Wasm module (command) starts running, and while it is running, it dynamically loads another module (library/reactor)? Or all modules have to loaded before any starts to run? If it is the former, I agree that it may be more similar to c-shared.

ydnar commented 8 months ago

Essentially, yes. I'm happy to consider other mechanisms, but I do want it to be explicit, and there is something to be said for the parallel to the existing build mode c-shared.

Maybe GOWASM=reactor?

inliquid commented 8 months ago

@johanbrandhorst

Since the proposal suggests causing compilation errors when //go:wasmexport is used when compiling "Commands" (since we do not export functions to the host in this mode), we'd need some way to exclude files defining these exports for library authors, easiest of which would be a build tag.

I think that would be different from how tinygo works? It allows compiling "Commands" with -target=wasi which then export functions to be used by the embedder. This mechanism is used in the wild for plugins, for instance here: https://github.com/knqyf263/go-plugin

Moreover we already use it in production, and we could eventually switch to go compiler if it behaves in a similar way.

There is main which sets up some globals (in order to "register" plugin) and it effectively compiles to _start which is exported by the module: изображение

Runtime is wazero.

johanbrandhorst commented 8 months ago

Based on the discussion above #65199 (comment) , I think we agreed that we eventually want to support wasmexport for command. It would be great if we could just support both library and command at same time (personally I think the implementation would not be very different so it shouldn't too hard). But if you prefer supporting library first, then command later, that is probably fine. But maybe we don't want that to cause an error, which complicates things (like you mentioned, you may want a build tag). Maybe we document that wasmexport for command will be supported in the future but is ignored for now.

Yeah, this would avoid the build tag question altogether. Reading through that again, I think it would mean that all exports would have to initialize the runtime when called, and all would have to call proc_exit before returning (seeing as a Command can be called at most once). In this form, all exports become equivalent to a main() function. I think my previous interpretation about reentrant calls is wrong given

Command instances may assume that they will be called from the environment at most once.

This seems to imply to me that only a single call from the host will take place. I don't know what that means for reentrant calls. If implemented as described above the reentrant call would call proc_exit before returning. Perhaps that is fine?

I'll think a little more about this and consider making changes to the proposal to remove the restriction of only allowing exports for reactors.

Given that Wasm's execution model is very different from other architectures, I think either c-archive or c-shared is probably fine. Is it possible for a Wasm module (command) starts running, and while it is running, it dynamically loads another module (library/reactor)? Or all modules have to loaded before any starts to run? If it is the former, I agree that it may be more similar to c-shared.

I'll have to ask around to answer this question but I'd think it's the latter.

Maybe GOWASM=reactor?

There's a section in the proposal about this:

We also considered using a GOWASM option instead, but this feels wrong since that environment variable is used to specify options relating to the architecture (existing options are satconv and signext), while this export option is dependent on the behavior of the "OS" (what functions to export, what initialization pattern to expect).

I still don't think that GOWASM is the right option and I'd sooner see us reuse buildmode=c-shared if we have to drop the custom build mode.

I think that would be different from how tinygo works? It allows compiling "Commands" with -target=wasi which then export functions to be used by the embedder.

I'm considering removing this restriction from the proposal, as Cherry suggested. The exact TinyGo implementation is a great source of data on how users are using exports but I don't think compatibility with TinyGo is a high priority for this proposal.

4ad commented 8 months ago

Using c-archive is a mistake. Wasm binaries are not static archives, they are dynamically loaded objects that can not be used at "compile-time" in any sort of meaningful way.

Using c-shared makes sense (but see below). Wasm modules of the type described in this proposal are shared libraries loaded at runtime. The fact that they are for a different ISA (Wasm) instead of the host ISA does not change this fundamental fact.

So we can reuse c-shared for now, but an issue with c-shared is that it is not future proof. When we get component model support, CM modules will be incompatible with core modules, and these core modules described in this proposal will be obsolete.

It seems impractical to support component model without having to introduce a new build mode in the future. Whether that is an argument for using c-shared vs. something custom now, I don't know.

Another argument against c-shared is that it implies some sort of compatibility with C (or its ABI), which is not really the case here.

cherrymui commented 8 months ago

Reading through that again, I think it would mean that all exports would have to initialize the runtime when called, and all would have to call proc_exit before returning (seeing as a Command can be called at most once).

I don't think we want to do this. For a command, it is still expected to call _start to start the command, which will initialize the runtime and call main.main. The exports are used for wasm to call back to Go from a wasmimport host function. It is in the same instance. And calling an exported function before _start (or _initialize) is still considered an error. (For a library, we could consider calling an exported function before _initialize will initialize the runtime first, not sure if this is worth doing.)

Maybe GOWASM=reactor?

I agree with @johanbrandhorst that this is probably not the right approach. GOWASM is for "architecture" features. Reactor/library is not.

For each WASI version, will there be multiple ways for building a "library"? Or there will be predominately one? (It could be different for each WASI version, like, say, reactor for wasip1, component for wasip2?)

johanbrandhorst commented 8 months ago

I don't think we want to do this. For a command, it is still expected to call _start to start the command, which will initialize the runtime and call main.main. The exports are used for wasm to call back to Go from a wasmimport host function. It is in the same instance. And calling an exported function before _start (or _initialize) is still considered an error. (For a library, we could consider calling an exported function before _initialize will initialize the runtime first, not sure if this is worth doing.)

I like this interpretation, but I don't know if it's correct. The exact wording (from https://github.com/WebAssembly/WASI/blob/256b651a3108610c076a12ec1915d9f9ca46e6b9/legacy/application-abi.md#current-unstable-abi) is:

_start is the default export which is called when the user doesn't select a specific function to call. Commands may also export additional functions, (similar to "multi-call" executables), which may be explicitly selected by the user to run instead.

It sounds to me like the user (via the host) can call any exported function, not just _start, and not just through reentrant calls into the same instance. I'd prefer we just support calling _start as you suggest but I'm worried that this would surprise users. What do you think?

For each WASI version, will there be multiple ways for building a "library"? Or there will be predominately one? (It could be different for each WASI version, like, say, reactor for wasip1, component for wasip2?)

For wasip1 I think there will be only one way, which is using _initialize as described in this proposal. It's unclear as yet to me what the exact options will be for wasip2. I don't know of anyone running "Wasm core modules" instead of "component model modules" but the standard is so new it's hard to say how it will be received by the ecosystem. I'd prefer not to make any decisions today about how to solve this in the future. I actually think using c-shared might be the best way to do that, since it leaves us open both to reusing c-shared for wasip2 and inventing our own new build-mode, if we want to use c-shared for Wasm core modules in wasip2.

I intend to update the proposal to support exports in Commands and switching to c-shared for the build mode.

cherrymui commented 8 months ago

I'd prefer we just support calling _start as you suggest but I'm worried that this would surprise users.

I think that is fine. I think usually it is expected that a command has a single entry point. If there is a strong need for multiple entry point, we could add the support later. But a single entry point should be fine for now.

I actually think using c-shared might be the best way to do that, since it leaves us open both to reusing c-shared for wasip2 and inventing our own new build-mode, if we want to use c-shared for Wasm core modules in wasip2.

SGTM. Thanks.

johanbrandhorst commented 8 months ago

I've updated the proposal to support exports in both "Reactors" and "Commands" and using the c-shared build mode to toggle the compilation of a "Reactor". The use of exports in "Commands" is restricted to only be allowed during the execution of the _start function through reentrant calls to host functions. I've removed references to a build tag as the wasip1 build tag is now sufficient to gate any use of go:wasmexport.

inliquid commented 8 months ago

The use of exports in "Commands" is restricted to only be allowed during the execution of the _start function through reentrant calls to host functions.

Not sure if I missed anything or not, but AFAIK atm implementations allow calling exports after _start is done. So for example wazero will call _start (or other start functions depending on config) when you invoke InstantiateModule and you are allowed to call module exports after that. This is basically how the plugin pattern works. What's the point to restrict that?

johanbrandhorst commented 8 months ago

At the end of _start, we call proc_exit. From the point of view of the host, that means the "program is terminated": https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/docs.md#-proc_exitrval-exitcode. The behavior of any calls to the instance at that point should probably be considered "undefined behavior". I'm not sure if we'll have to add something to actually refuse running functions after _start has returned, but it is not expected that users should call anything. Note also how the Application ABI doc explicitly states that

Command instances may assume that they will be called from the environment at most once. Command instances may assume that none of their exports are accessed outside the duration of that call.

If a user wants to call into an instance repeatedly, they will want to compile a "Reactor" though the c-shared build mode. Is there some use case that is not covered by this functionality?

inliquid commented 8 months ago

I think this behavior more or less explained by their RATIONALE.md:

Why do we only return a sys.ExitError on a non-zero exit code?

It is reasonable to think an exit error should be returned, even if the code is success (zero). Even on success, the module is no longer functional. For example, function exports would error later. However, wazero does not. The only time sys.ExitError is on error (non-zero).

This decision was to improve performance and ergonomics for guests that both use WASI (have a _start function), and also allow custom exports. Specifically, Rust, TinyGo and normal wasi-libc, don't exit the module during _start. If they did, it would invalidate their function exports. This means it is unlikely most compilers will change this behavior.

GOOS=waspi1 from Go 1.21 does exit during _start. However, it doesn't support other exports besides _start, and _start is not defined to be called multiple times anyway.

Since sys.ExitError is not always returned, we added Module.IsClosed for defensive checks. This helps integrators avoid calling functions which will always fail.

We use tinygo to compile Go to Wasm with -target=wasi, so that modules can be used as plugins. Not sure if reactor pattern supported by tinygo at all.

johanbrandhorst commented 8 months ago

The plugin pattern I think you're referring to will be perfectly supported by the functionality introduced in this proposal. Note that it's also possible to do with a "Command" today (see the references in the Background section of the proposal), and will remain possible if this proposal is implemented.

rsc commented 8 months ago

Have all remaining concerns about this proposal been addressed?

The proposal is to add support for -buildmode=c-shared in wasm, and to add

//go:wasmexport [name]

as a directive analogous to what //export does when using cgo.

x1unix commented 8 months ago

@rsc I assume that it would work for libraries, but will it work for programs that work in the background?

For example, a program acts as a server worker that accepts requests from a page. Let's assume that client calls a WASM exported function to send a request to a WASM server. The server processes a request and calls the client back to pass a result.

package main

var (
  requests = make(chan []int)
)

//go:wasmimport myNamespace submitResponse
func submitResponse(result int)

//go:wasmexport receiveRequest
func receiveRequest(a, b int) {
   requests <- []int{a, b}
}

func main() {
   ctx := context.TODO()
   requests := make(chan []int)
   go listen(requests)
   <-ctx.Done()
}

func listen() {
   for req := range requests {
       // do some logic and call client.
       submitResponse(42)
   }
}

Will that case work?

johanbrandhorst commented 8 months ago

The proposal makes it clear what can be expected in this situation: When receiveRequest is called from the host (presumably to handle some request), the runtime will wake up (having either previously handled some other export and returned to the host, or been initialized using _initialize and returned to the host), a new goroutine calling receiveRequest will be created, and the scheduler will run. Any of the available goroutines to run at this point (which may include goroutines started by runtime initialization, during previous export invocations, or the goroutine just created to handle the request) may be scheduled.

In this case, because receiveRequest blocks on a channel send, the function would yield back to the runtime, the scheduler would eventually schedule listen, and the request would be handled by submitResponse, which calls back into the host to submit the response. This host call is blocking and no goroutines are scheduled while it is running. Once it returns, listen will go back to blocking on reading from requests and yield back to the runtime, which will schedule receiveRequest, which is now unblocked. Once it returns, the runtime will return back to the host, and no other goroutines will be handled.

That is my understanding of the runtime and scheduler behavior, which is admittedly hazy.

rsc commented 8 months ago

Based on the discussion above, this proposal seems like a likely accept. — rsc for the proposal review group

The proposal is to add support for -buildmode=c-shared in wasm, and to add

//go:wasmexport [name]

as a directive analogous to what //export does when using cgo.

rsc commented 8 months ago

No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — rsc for the proposal review group

The proposal is to add support for -buildmode=c-shared in wasm, and to add

//go:wasmexport [name]

as a directive analogous to what //export does when using cgo.

cherrymui commented 7 months ago

@johanbrandhorst As the proposal has been accepted, do you have any update for implementation or prototype? Thanks.

johanbrandhorst commented 7 months ago

Nothing to share at the moment, just experimenting locally with a prototype for now. We do intend to implement this.

cherrymui commented 6 months ago

@johanbrandhorst Any update on this? As mentioned in https://groups.google.com/g/golang-dev/c/Hy9iaQFhlHw/m/DCbJD7SkBQAJ , we are 4 weeks from the Go 1.23 release freeze. I'd hope we get this in for Go 1.23 release. It would be a good time now to prepare the CLs and start reviewing. Thank you!

johanbrandhorst commented 6 months ago

We've still just been prototyping it, nothing to submit yet unfortunately. We're also hoping we could get something submitted soon but our primary goal is the 32bit port.

cherrymui commented 6 months ago

Thanks for the update. Would it be possible to get this submitted first? I would think this work is simpler whereas for the wasm32 port there are still details need to be fleshed out. Also let us know if there is anything we could help with. Thanks.

johanbrandhorst commented 6 months ago

We're honestly still in the early stages of figuring out the changes necessary. If you have any time to help build a minimal functional prototype we could develop it from there. Unfortunately none of us are working on it full time.

Zxilly commented 3 months ago

I would like to know if there is a current implementation in progress that I would like to contribute to.

johanbrandhorst commented 3 months ago

We don't have a POC to share yet, but if you want to try building something I recommend basing it off the in-process wasm32 branch: https://go-review.googlesource.com/c/go/+/570835/2

cherrymui commented 3 months ago

I'll probably work on the implementation in Go 1.24 cycle. I don't think it depends on the wasm32 port. (Yes, wasm32 will have wasmexport support, which can be added once both this and wasm32 are in.)

achille-roussel commented 3 months ago

I started recording notes when I picked up the work on go:wasmexport earlier this year, didn't have time to move further than that tho https://docs.google.com/document/d/1R14wRHv_UjeFnemvV5hbp75cCRNzVH9ZZS-Yo_zITxg/edit?usp=sharing

Zxilly commented 3 months ago

I'll read the docs and see what I can do.

juliens commented 3 months ago

Hello,

I started to work on an implementation, but I'm stuck with a lots of problem. https://github.com/johanbrandhorst/go/pull/2

Happy if it can help.

stevenzzzz commented 3 months ago

/sub

gopherbot commented 3 months ago

Change https://go.dev/cl/603055 mentions this issue: cmd/compile: add basic wasmexport support

cherrymui commented 3 months ago

I spent a few days to get a basic support of wasmexport work, CL 603055 . Currently it works okay with very simple exported functions, e.g. adding some numbers. More complicated case doesn't work yet as it doesn't support stack unwinding (e.g. goroutine switch, stack growth). I'll look into that.

For now it only supports executable mode, i.e. the Go wasm module calls into the host via wasmimport, which then calls back to Go via wasmexport. Library (c-shared) mode will be done later. Also only wasip1 is implemented for now, not js.

gopherbot commented 3 months ago

Change https://go.dev/cl/603836 mentions this issue: cmd/internal/obj/wasm: handle stack unwinding in wasmexport

cherrymui commented 2 months ago

CL 603836 adds stack unwinding support, including gorouting switch, stack growth, traceback, etc. To test this I need to write a driver/host program that provides the import functions and calls the export funcitons. CL 604235 is an example. Feel free to play with it and report any issues.

By the way, if anyone has a simple way to test this, hopefully with fewer external dependencies, that would be great. I'm still wondering what is the best way to add tests in to the Go repo.

Next I'll take a look at library mode.

Thanks.

johanbrandhorst commented 2 months ago

I sent the question of testing to the #wazero-dev channel on Gophers Slack and @mathetake and Matt Johnson-Pint chimed in on the test CL 604235 with some example code. That could be good for static testing, but it would be great to do some runtime testing too, which would probably require either a pre-built runtime binary with testing hooks that could be installed into the builders, or perhaps a nested go.mod test package (similar to https://github.com/golang/go/blob/master/src/crypto/internal/bigmod/_asm/go.mod?), which could allow a test similar to what you wrote already.

cherrymui commented 2 months ago

Thanks! Yeah, currently there is no wasm binary parser in tree, but it's probably not hard to add one. I'm more concerned about testing run time behavior. Maybe using wazero library with a nested go.mod is okay.

Another possibility is that if wazero's CLI (or some other wasm engine's CLI) supports multiple wasm modules, I probably can give it a Go Wasm module and another module (probably handwritten with textual Wasm), and have them call each other via imports and exports. Our builder (wasip1-wasm_wazero) already has the CLI installed. As far as I can tell, the current cmd/wazero doesn't.

gopherbot commented 2 months ago

Change https://go.dev/cl/604316 mentions this issue: cmd/link, runtime: support library mode on wasip1

vlkv commented 2 months ago

Hello, @johanbrandhorst @cherrymui @juliens It would be great to test (test CL 604235) the new wasmexport feature with wasmtime too. I would like to try and help with this. But I have zero experience in developing/testing golang/go. Can you give some advice? Is all I need to do is to build a Go compiler from some feature branch (what is it's name btw?) and build the test code to wasm target then run it, etc. Can you give some instructions, please?

Thanks.

johanbrandhorst commented 2 months ago

@vlkv this isn't the right forum for this, but I'd be happy to help you in the #webassembly channel on Gophers Slack if you could join. Note that we have automated tests for wasmtime already, and the tests we are talking here are regarding the best way to verify the function-export functionality, which isn't technically hard, but potentially hard to do without introducing undesirable dependencies.

johanbrandhorst commented 2 months ago

I've updated the proposal to make the export name required, as this is consistent with //export and is easier to implement.

cherrymui commented 2 months ago

CL 604316 implements the library (c-shared) mode. I updated the test program CL 604235 to handle both executable and library modes. Feel free to check it out and experiment with it, and see if it satisfies your need. Thanks!

gopherbot commented 2 months ago

Change https://go.dev/cl/604975 mentions this issue: test: add test case for wasmexport parameter types

gopherbot commented 2 months ago

Change https://go.dev/cl/606855 mentions this issue: cmd/link: support wasmexport on js/wasm

cherrymui commented 2 months ago

CL 606855 implemented support on GOOS=js. Also added a test. (It is easier to add a test for js than wasip1. It may still be good to add a test for wasip1.)

Since the proposal doesn't mention a library mode on js, I didn't add one.

With that, I think this is done. Let me know if there is anything else needed. Also please try it out and let me know if there is any issue. Thanks.

johanbrandhorst commented 2 months ago

I agree, I think that CL concludes the implementation of this proposal. Thank you so much!

Zxilly commented 2 months ago

Is that possible to also support lib mode on js platform?

vlkv commented 2 months ago

Hello, @cherrymui

Thank you very much for your work. Unfortunately I couldn't run a test example of my own that uses the go:wasmexport directive successfully. I have created a repo with minimal reproducible example here.

In my example program I am using wasmtime and gotip. I tried both embedding with wasmtime-go and wasmtime CLI to run the code. The guest.go which is compiled to wasm is this:

package main

import "fmt"

func main() {
    fmt.Println("Hello, wasmtime!")
    fmt.Printf("2 + 3 = %d\n", Add(2, 3))
}

//go:wasmexport Add
func Add(a, b int32) int32 {
    fmt.Println("Hello from Add()...")
    result := a + b
    fmt.Printf("Add(%d, %d) result is %d\n", a, b, result)
    return result
}

//go:wasmexport CallAdd
func CallAdd() {
    Add(40, 2)
}

There are some useful targets in Makefile .

Test 1: Invoke CallAdd with wasmtime CLI

vitvlkv@nostromo go_wasmexport_test % make run_wasmtime_cli
wasmtime --invoke CallAdd ./guest/guest.wasm
Error: failed to run main module `./guest/guest.wasm`

Caused by:
    0: failed to invoke `CallAdd`
    1: error while executing at wasm backtrace:
           0: 0x167e9d - <unknown>!CallAdd
    2: memory fault at wasm address 0xfffffff8 in linear memory of size 0x1120000
    3: wasm trap: out of bounds memory access
make: *** [run_wasmtime_cli] Error 134

Test fails. I guess this should not panic, because CallAdd is exported from guest.wasm:

vitvlkv@nostromo go_wasmexport_test % wasmer inspect ./guest/guest.wasm
Type: wasm
Size: 2.4 MB
Imports:
  Functions:
    "wasi_snapshot_preview1"."sched_yield": [] -> [I32]
    "wasi_snapshot_preview1"."proc_exit": [I32] -> []
    "wasi_snapshot_preview1"."args_get": [I32, I32] -> [I32]
    "wasi_snapshot_preview1"."args_sizes_get": [I32, I32] -> [I32]
    "wasi_snapshot_preview1"."clock_time_get": [I32, I64, I32] -> [I32]
    "wasi_snapshot_preview1"."environ_get": [I32, I32] -> [I32]
    "wasi_snapshot_preview1"."environ_sizes_get": [I32, I32] -> [I32]
    "wasi_snapshot_preview1"."fd_write": [I32, I32, I32, I32] -> [I32]
    "wasi_snapshot_preview1"."random_get": [I32, I32] -> [I32]
    "wasi_snapshot_preview1"."poll_oneoff": [I32, I32, I32, I32] -> [I32]
    "wasi_snapshot_preview1"."fd_close": [I32] -> [I32]
    "wasi_snapshot_preview1"."fd_write": [I32, I32, I32, I32] -> [I32]
    "wasi_snapshot_preview1"."fd_fdstat_get": [I32, I32] -> [I32]
    "wasi_snapshot_preview1"."fd_fdstat_set_flags": [I32, I32] -> [I32]
    "wasi_snapshot_preview1"."fd_prestat_get": [I32, I32] -> [I32]
    "wasi_snapshot_preview1"."fd_prestat_dir_name": [I32, I32, I32] -> [I32]
  Memories:
  Tables:
  Globals:
Exports:
  Functions:
    "_start": [] -> []
    "Add": [I32, I32] -> [I32]
    "CallAdd": [] -> []
  Memories:
    "memory": not shared (274 pages..)
  Tables:
  Globals:

Test 2: Invoke Add(40, 2) with embedded wasmtime-go

vitvlkv@nostromo go_wasmexport_test % make run_main        
go run .
panic: error while executing at wasm backtrace:
    0: 0x167e4d - <unknown>!Add

Caused by:
    0: memory fault at wasm address 0xfffffff0 in linear memory of size 0x1120000
    1: wasm trap: out of bounds memory access

goroutine 1 [running]:
main.main()
        /Users/vitvlkv/tp/backend/indicators/tmp/go_wasmexport_test/main.go:20 +0xc8
exit status 2
make: *** [run_main] Error 1

Test fails.

Test 3: Calling wasm _start with wasmtime CLI

vitvlkv@nostromo go_wasmexport_test % wasmtime ./guest/guest.wasm 
Hello, wasmtime!
Hello from Add()...
Add(2, 3) result is 5
2 + 3 = 5

This test looks OK. But, see the Test 4.

Test 4: Calling wasm _start with embedded wasmtime-go

For this test we should uncomment this block of code first. Then run:

vitvlkv@nostromo go_wasmexport_test % make run_main
go run .
Hello, wasmtime!
Hello from Add()...
Add(2, 3) result is 5
2 + 3 = 5
panic: error while executing at wasm backtrace:
    0: 0x7a35b - <unknown>!runtime.exit
    1: 0x85b75 - <unknown>!runtime.main
    2: 0x109ef9 - <unknown>!wasm_pc_f_loop
    3: 0x109ff6 - <unknown>!_rt0_wasm_wasip1

Caused by:
    Exited with i32 exit status 0

goroutine 1 [running]:
main.main()
        /Users/vitvlkv/tp/backend/indicators/tmp/go_wasmexport_test/main.go:14 +0x124
exit status 2
make: *** [run_main] Error 1

Test fails. Here we see, that _start (or main) started to execute normally, but the program panics in the end.

Sorry for very long post, I just want to provide you with as much details as possible. Can you please take a look at this?

cherrymui commented 2 months ago

@vlkv thanks for testing it. From the Makefile

GOOS=wasip1 GOARCH=wasm gotip build -o ./guest/guest.wasm ./guest

You're building an executable, not a library, as it does not set -buildmode=c-shared. This means the entry point is _start, which must be called. This is why "Test 1" and "Test 2" fail.

"Test 4" fails because in executable mode, when _start is called, it runs main.main and then exits. "Exited with i32 exit status 0" just looks like an "error" that represents that the module exits normally, which is expected. And then you cannot call Add, as the module already exited.

To make your test case work, it seems you want to use library mode:

GOOS=wasip1 GOARCH=wasm gotip build -buildmode=c-shared -o ./guest/guest.wasm ./guest

(Note -buildmode=c-shared) Then in your driver code call _initialize first (not _start), then call Add.

We could consider making it fail loud explicitly if an exported function is called before the runtime is initialized (_start or _initialize not called).

cherrymui commented 2 months ago

Is that possible to also support lib mode on js platform?

@Zxilly Technically I think it shouldn't be hard to support. But it is not proposed and specified in this proposal, e.g. how the library is going to be initialized, what the init symbol name is, etc.. If we have consensus on how it should look like, we can do it. Feel free to start a new proposal. Thanks.