-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
cmd/compile: allow the use of go:wasmimport globally #59149
Comments
Currently the implementation uses 32-bit linear memory. But that might change in the future? Does this have any impact to this? Thanks. |
@cherrymui There is the 64bit memory proposal for web assembly, so in the future yes. One reason we used i32 here is to remain similar to what clang and friends use for pointers. We certainly could use i64 today to avoid future concerns though. |
Thanks. Using i32 for now is probably fine. The concern is that if later we change to 64-bit address, will this break any user? Maybe it will not if the Wasm engine is backward compatible? Do you think there is any compatibility issue? If not, this is probably okay. |
@cherrymui Ah great question. Let me dig into the 64bit memory proposal and see what they suggest, since this would effect everyone that generates wasm if they all need to compile different 32 vs 64 bit versions. |
I've added a section to the proposal that disallows any parameter types other than those that easily map to Wasm types. This restriction could be relaxed in a hypothetical future |
Thanks. SGTM. Maybe add a note that if Wasm moves to 64-bit pointers, the ABI translation rule for unsafe.Pointer might change? A minor question: if the directive is seen when compiling to a non-Wasm platform, is it an error or just silently ignored? |
Thank you, I updated the proposal with a section on future 64 bit Wasm runtimes. The current behavior is to cause a compile time error when using the directive on a non-Wasm platform, and the current plan is to keep that behavior. |
One thing to note is that there will be some users who tries to put Support for Importing global variables will be far more complex than just functions, and standard compilers usually disable it AFAIK. So maybe we might want to add safeguard on this. |
Also, two things to consider:
|
Yeah, if we support
Either way it is fine for me. If at link time we don't naturally check for duplications when generating the table, I think it is fine not to. Does other linker check this?
I guess that will require changing the syntax of the directive? As noted above, we don't support space in module names, so I guess it is okay to leave empty name out as well. If there is a need in the future we can extended the syntax. Empty names seem very confusing, though. How does the other side provide the symbol? |
Hm, theoretically, you could have libraries that import the same host function independently, right? Could you clarify what sort of problem this might cause?
No, we don't want to allow this, and the current definition makes it impossible. Thanks for raising this! |
well, what if two different packages import the functions with different signatures but with the same import names? E.g. package foo
//go:wasmimport env hostfn
func hostfn(uint32) package bar
//go:wasmimport env hostfn
func hostfn(float32) if we have objects like ^^, then the resulted wasm binary would be like: (module
(type (;0;) (func (param i32)))
(type (;1;) (func (param f32)))
(import "env" "hostfn" (func (;0;) (type 0)))
(import "env" "hostfn" (func (;1;) (type 1)))
) which is somehow valid in the sense of Wasm spec, but in reality the binary is unusable. So my question is whether or not we want to raise an error when linking such objects. But I guess as @cherrymui mentioned, we don't need to do anything. edited: seems like clang/wasm-ld doesn't emit an error on this. |
in Wasm spec, names are arbitrary length utf-8 strings, so technically the hosts can define an empty module instance (zero length utf-8), and export the empty name entries (e.g. function). agreed it's better to just rule out the empty names for now as it seems to require syntax changes as opposed to the practical use cases. |
This proposal has been added to the active column of the proposals project |
Since there will be a single signature eventually available at runtime, the program will likely fail at instantiation when resolving imports. The compiler could ensure that all uses of Thanks for raising this point! |
For the example with conflicting signatures, isn't one correct and one wrong? And if so, doesn't the wrong one cause a problem at runtime by itself? Who are the maintainers of the wasm code that should weigh in? @cherrymui? @neelance? Anyone else? |
As commented above #59149 (comment) I'm fine either way. I think we don't need to check if other toolchains don't. Yeah, I'd think it will result in some kind of run time (or instantiation time) error. |
Agree that it's ok to defer this error checking to the WASM runtime if other toolchains do the same. As #59149 (comment) says, we can only check a subset of issues at compile time anyway. Runtime mismatches between imported symbols and actual host signatures are likely a much common problem, and this is something the compiler can't do anything about. |
Change https://go.dev/cl/489255 mentions this issue: |
Based on the discussion above, this proposal seems like a likely accept. |
No change in consensus, so accepted. 🎉 |
This is for compatibility with upstream Go. See golang/go#59149 for more context.
This is for compatibility with upstream Go. See golang/go#59149 for more context.
This is for compatibility with upstream Go. See golang/go#59149 for more context.
This is for compatibility with upstream Go. See golang/go#59149 for more context.
This is for compatibility with upstream Go. See golang/go#59149 for more context.
This is for compatibility with upstream Go. See golang/go#59149 for more context.
This is for compatibility with upstream Go. See golang/go#59149 for more context.
Background
#38248 defined a new compiler directive,
go:wasmimport
, for declaring a dependency on a function provided by the runtime environment. It also defined a restriction on the use of this compiler directive, such that it cannot be used in code outside of theruntime
andsyscall/js
standard library packages. #58141 relaxed this restriction to allow the use of the directive in thesyscall
package to allow accessing WASI defined syscall APIs necessary to support WASI.The
go:wasmimport
directive is defined as follows:Where
importmodule
is the Wasm module of the function andimportname
is the name of the function within the module. The effect of this compiler directive is that any Go function calls of the function will be replaced with a call instruction to the function identified by the module and function name during compilation. Parameters and return values to the Go function are translated to Wasm according to the following table:Any other parameter types are disallowed by the compiler.
For more information see the ssagen compiler package.
Proposal
We propose that the package restriction on the use of the
go:wasmimport
directive be removed entirely.Rationale
Much of the discussion around the WASI proposal (#58141) revolved around the question of how and when the WASI target should move between syscall APIs (i.e. from depending on the
wasm_snapshot_preview1
API to thewasm_snapshot_preview2
API and beyond). A major reason for this anxiety is because of the conflict between the desire for stability and the desire for access to newer runtime features afforded by newer syscall APIs.One way to help avoid this conflict would be to provide access to newer APIs through purpose built third-party packages (e.g.
golang.org/x/wasi/preview2/net
) implementing various syscall level APIs through the use ofgo:wasmimport
and exposing Go native types implementing interfaces such asnet.Listener
andnet.Conn
. Users who want to opt-in to new functionality can import one of these packages with the understanding that it may be unstable. Users who prefer to wait to use this functionality until it’s in the standard library can stay on the older API version. We’re exploring adding functionality to wit-bindgen to generate Go files withgo:wasmimport
directives and function signatures.In addition to this, it would also allow users or vendors to create their own packages around vendor specific runtime extensions (e.g. WasmEdge defines its own sockets API, Fermyon defines a custom HTTP handler API) and expose them as easily consumable Go modules. This would help make Go more flexible and useful in different Wasm runtime environments.
Discussion
Rationale for the existing restriction
The existing restriction was added as a last minute addition to the original proposal. The primary rationale for this seems to have been an anxiety around exposing this widely without having had any experience of using it. If changes had to be made, it could break existing users.
We believe this concern should be smaller now that we have experience of using the directive while implementing
go:wasmimport
and working on implementing the WASI target.Regardless, we do want to reserve the right to make breaking changes to this directive until such a point as the wasm architecture itself is declared stable. This is in line with the reservations made in #38248.
Effect on js/wasm
The
js/wasm
target has similar functionality exposed via the use of thesyscall/js
standard library package. The existing standard library interfaces will remain the preferred way of accessing functions defined on the JavaScript host. The primary use case within thejs/wasm
target would be as a means of accessing functions defined by other Wasm modules loaded into the same environment.A note on wasm module names
The Wasm Core 1.0 spec defines a name as used in module and function names as a sequence of UTF-8 codepoints. The current definition of the
go:wasmimport
directive uses a space to separate the module name from the function name, making it impossible to specify a module or function name with a space in it. This proposal does not suggest changing the definition to support this use case.Other language implementations
TinyGo
TinyGo uses the
export
pragma both for exporting to and importing methods from the environment. It also allows the user to specify the module to import from using the//go:wasm-module
pragma. An example of this in the wild is the Fastly Compute@Edge SDK:TinyGo similarly restricts the types supported in parameters though I have struggled to find documentation for this exact limitation.
Rust
Rust uses the
extern "C"
raw interface pattern to define Wasm host imports. This is similar to our proposal in that it allows the user to specify the module and function name.Rust supports using rich types in arguments but does not provide guarantees of the stability and encourages the use of integer/float values only.
Future work
Richer type support for imported functions
The current implementation of go:wasmimport supports only functions with integer, floating point or pointer parameters and a single return value. In the future, we would like to support types such as strings and structs and multiple return values. This would require defining a more elaborate calling convention for the imported functions than the one currently used.
The current mechanism follows the same semantics in use by other Wasm compilers such as Clang, which effectively treat Wasm the same as C, requiring the caller and callee to share the same ABI to be able to decode values in memory.
A 32 bit Wasm port
During the discussion of #59156, it became evident that many of the UX issues with the directive would disappear if the Wasm port was a 32 bit port, since all existing Wasm runtimes use 32 bit pointers, and the mismatch between the memory layout of Go and Wasm would disappear. There are still some concerns around whether the Go struct layout may change in the future, which would prevent us from guaranteeing direct memory mapping.
In the case of a future 32 bit Wasm port, the restriction on types allowed in the
go:wasmimport
parameters would be relaxed in a way that doesn't risk introducing memory corruption, but provides significant UX improvements to the compiler directive. The currentwasm
architecture, which is a 64 bit port, will likely retain the parameter limitations for the forseeable future.64 bit Wasm runtimes
In the future, some runtimes may add support for 64 bit pointers. In such a runtime, the ABI translation rules for the use of the
unsafe.Pointer
type may change. We would evaluate the effects of this when/if adding support for 64 bit pointers.Authors
@johanbrandhorst, @Pryz, @evanphx, @achille-roussel
The text was updated successfully, but these errors were encountered: