Skip to content

Support GOOS=js GOARCH=wasm generated code #432

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

Closed
codefromthecrypt opened this issue Apr 2, 2022 · 14 comments · Fixed by #621
Closed

Support GOOS=js GOARCH=wasm generated code #432

codefromthecrypt opened this issue Apr 2, 2022 · 14 comments · Fixed by #621

Comments

@codefromthecrypt
Copy link
Contributor

I was looking at @wuhuizuo's test which uses this source and compiles it like GOOS=js GOARCH=wasm go build -o wasm.wasm

package main

import (
	"syscall/js"

	"github.com/wuhuizuo/go-wasm-go/provider/native"
)

func main() {
	js.Global().Set("Fibonacci", js.FuncOf(Wrap(native.Fibonacci)))
	js.Global().Set("RequestHTTP", js.FuncOf(Wrap(native.RequestHTTP)))
	js.Global().Set("FileIO", js.FuncOf(Wrap(native.FileIO)))
	js.Global().Set("MultiThreads", js.FuncOf(Wrap(native.MultiThreads)))
	js.Global().Set("BytesTest", js.FuncOf(Wrap(native.BytesTest)))
	js.Global().Set("InterfaceTest", js.FuncOf(Wrap(native.InterfaceTest)))
	js.Global().Set("ErrTest", js.FuncOf(Wrap(native.ErrTest)))

	select {}
}

https://github.com/wuhuizuo/go-wasm-go/blob/main/provider/wasm-go/main.go

The resulting wasm isn't exporting those functions. "run" "resume" "getsp" and "mem" are externalized.

You can identify, for example "Fibonacci" if you use the custom name section and get "github.com_wuhuizuo_go_wasm_go_provider_native.Fibonacci". However, again that function is neither the short name, nor is it exported. There's probably some approach to getting the function prefix somewhere

If we want to support this tech we'd need to:

  1. add an API for non-exported functions
  2. somehow detect the go-generated module path prefix to the function or add a utility to scan for it
  • possibly, we could add a second index when this is detected for the short name, and index it twice when doing so doesn't clobber something else. In go-generated it isn't currently possible.

I'm not sure how many use wasm produced by go vs tinygo, but raising it as I noticed there's no way to lookup these functions and use them, yet. We probably need some 👍 and people to say why/how they are using this, so that we don't displace time on things only used for demo projects.

@codefromthecrypt
Copy link
Contributor Author

PS the reason we don't currently add an API for non-exported functions as the spec hints you should avail only exported ones. There's probably a decent security reason for not breaking that seal.

OTOH, the only callers who could access a non-exported function (outside the module) would be host functions. We could avail the api and say "use at your own risk" or add a flag on the runtime that disables anything from ever returning from module.Function(X)

@inkeliz
Copy link
Contributor

inkeliz commented Apr 8, 2022

Maybe wait for golang/go#25612 and golang/go#42372? I think the major blocker is that it doesn't support WASI and doesn't have option to export functions. One of the comments arround calling functions is: "If I call hello_world from the WebAssembly host, on which goroutine will it run?"

The go compiler also requires alot of imported functions (which is defined on https://github.com/golang/go/blob/3a19102de3435a3823126e74cf42c67037ff1890/misc/wasm/wasm_exec.js#L208-L454), the code doesn't run without those functions and it's passing the stack-pointer to the function. Golang can import functions using the magical assembly instruction CallImport. It's possible to create those functions on wazero: but it may change overtime (and it breaks between versions: golang/go#47082).

@codefromthecrypt
Copy link
Contributor Author

@inkeliz thanks for doing the research. there's no current reason to rush before the issues you mentioned!

@codefromthecrypt
Copy link
Contributor Author

We could add a special-cased option to CompileConfig which can understand Go's naming convention and make exports as if it was done correctly in the first place. Ex taking an expression like this: "github.com_wuhuizuo_go_wasm_go_provider_native.Fibonacci" and making an export "Fibonacci".

@inkeliz
Copy link
Contributor

inkeliz commented May 9, 2022

It will also require some custom module, to replace wasm_exec.js, something similar to https://github.com/mattn/gowasmer, so functions like wasmExit will exists (so it can be linked) and also performs the same action.

@mathetake
Copy link
Member

https://github.com/prep/wasmexec this lib closes this issue maybe?

@prep
Copy link

prep commented May 29, 2022

Hey there! I'm the author of ☝️. I'm still tinkering with that package, but it is basically working with the small tests I've been running locally. As per @codefromthecrypt's original question: That isn't working right now, but I've already got it working in a local branch.

For those who are interested, what happens under the hood is:

  1. js.FuncOf() basically adds a new function to the internal values map in wasmexec via _makeFuncWrapper.
  2. This value can be looked up by name, so on the host we can execute the wrapper function.
  3. The wrapper function itself creates a_pendingEvent that is just an object with the id of the function and its parameters.
  4. The wrapper function then calls the resume export on the guest. This signals the guest to read the _pendingEvent value and pass it to our js.FuncOf() function.

The resume function is one of the few exports available and is our only way to signal to the guest that an event is waiting.

I'm struggling a bit to make it into a nice API for the user. It's currently just a Call(name string, args []any) (any, error) function on the ModuleGo struct itself.

That said, my plan is to create a faux-waPC API for wasmexec which can be done by leveraging the above-mentioned mechanism. I'll try to roll this out in the upcoming days if I can spare the time.

@codefromthecrypt
Copy link
Contributor Author

codefromthecrypt commented Jun 2, 2022

I wanted to chime in as things develop.

Firstly, I really appreciate the hints @inkeliz gave including the "no compatibility" promise from go. History has told me that blocking on any specific Go issue to complete can be tantamount to never doing anything. Since folks here and in competing runtimes are doing things, we have stake in this, and also it makes no sense for the go wasm runtime to not support go ;)

This doesn't remove the issues like compatibility risk, though we are starting from a later version of Go anyway. For example, our minimum is 1.17, so we don't need to think about past incompat, just knowing it can happen again. When we opened this issue, it wasn't that long ago, but also we are much more mature for the age. We can figure out how to do this without a lot of liability.

In both looking at go impl, as well its wiki I've noticed the following things:

Exporting of functions is different than working with go generated wasm in general

You can today use "run" to kick off wasm similar to a WASI command with "_start'. Tracking of exports is a separate topic and can progress independently.

The go function imports should be two modules, but it is packed into one "go".

We have the ability to rename imports now, so we can actually create two implementation modules for the "go" module imported by go-generated wasm. ex. debug, runtime.* functions can and likely should be separately implemented from syscall/* functions.

debug, runtime.* functions are just like the assemblyscript magic imports and WASI and should be implemented in wazero.

The debug, runtime.* are just like WASI or assembly script and should be implemented here, so that they can take advantage of the state we already manage in SysContext, such as RNG and clocks.

We aren't likely to export SysContext prior to elaborating WASI 2 because we should think about capabilities and how or if we want to control that. Meanwhile, "go.runtime.*" are just like WASI functions. Implementing these outside the wazero repo means duplicating the problem and any state around things like clocks. This is not a good thing, so we should provide a default partial module for this.

The "go.syscall/*" functions, and how to invoke "run" is trickier and can be deferred

Porting the rest of the imports and the "run" logic wasm_exec is something we can lean on @prep for, since work is already begun on prep/wasmexec. We can hand over parts such as the runtime imports to reduce logic in that project, at least for the wazero side of it.

We will eventually complete support here

We should be clear that this is a deferment not a permanent punt. It doesn't make sense for a dependency free solution for go, to intentionally not support go. Rather, this is a way to build experience and also deal with the realities of hands available. At some point, we will complete wasm_exec natively here, and that will be more efficient than working through an abstraction that has both dependencies and abstraction concerns only needed for wasmtime and wasmer.

Next action is to implement the syscall/* imports

As mentioned in the earlier section the effort dispatch is easier if we split responsibilities and enable @prep to continue but also taking advantage of things like what we are already doing. I'll help with this by implementing the syscall imports. Doing so is helpful for another reason which is solidifying the three users of clock interfaces: WASI, WASI 2, and wasm_exec

@prep
Copy link

prep commented Jun 4, 2022

I've just pushed some changes to wasmexec that makes the whole js.FuncOf() thing work. I've also built a fake-waPC thingy on top of it for convenience.

@codefromthecrypt as for porting things over, I'd be happy to help where I can. I think there is value in keeping wasmexec separate to support the other runtimes but once things stabilise we could port things in and keep it in sync if you feel that is valuable.

@codefromthecrypt
Copy link
Contributor Author

codefromthecrypt commented Jun 4, 2022

@prep sounds good. FYI to test portability of the sys interfaces needed (ex nanotime etc), I started porting the exec_wasm.js yesterday. I'll keep going on this and once done compile a list of suggestions and/or PR to your repo. You can ping me on yours when you like also.

@codefromthecrypt
Copy link
Contributor Author

@cherrymui @aclements I think most of the ecosystem are stumped about the purposed of the "go" "debug" function import when targeting GOOS=js GOARCH=wasm. Best I can tell, it accepts the same signature as other CallImport functions. I can't seem to generate any code that uses it, and wonder if it is just here for ad-hoc use? https://github.com/golang/go/blob/master/src/cmd/link/internal/wasm/asm.go#L133-L138

@prep
Copy link

prep commented Jun 6, 2022

On a somewhat related topic, the current implementation requires us to implement the runtime.getRandomData import while at the same time needing crypto.getRandomValues in the JS runtime for rand.Read. Both doing the exact same thing.

I wonder if it's too cheeky to do something like this in crypto/rand:

//go:linkname getRandomData runtime.getRandomData
func getRandomData(r []byte)

To funnel those requests through the same import. I'm unsure if this works, considering runtime.getRandomData() is an export itself.

@codefromthecrypt
Copy link
Contributor Author

FYI the host side implemented here needn't do any goroutine safely, and really should highly discourage anyone from trying to do that.

It may seem odd because Go makes some of the _js files use mutexes. For example, you might think because function wrappers are implemented with locks, then so should the host side.

However, it is misleading because the atomic primitives which would implement the mutexes are left a TODO. This means there's no safety provided in practice, and function wrappers if allocated concurrently would have unpredictable results.

In summary, go 1.19 won't be able to do concurrency in its wasm, so there's no point cluttering the code with anything that fakes as if it could. If this every changes, we can adjust after that.

@codefromthecrypt
Copy link
Contributor Author

#621 should solve this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants