wasm-bindgen icon indicating copy to clipboard operation
wasm-bindgen copied to clipboard

Implemented compatibility mode with wasm32-wasi

Open Rob2309 opened this issue 2 years ago • 24 comments

This PR adds the new --wasi-abi flag to wasm-bindgen. With this flag, wasm-bindgen generates bindings that are compatible with the ABI used by the rust target wasm32-wasi. The changes should not affect the behavior of wasm-bindgen when the flag is not specified.

For implementing the behavior of wasm32-wasi, I introduced a few new Instructions to the "Standard". They are PackSlice, UnpackSlice, PackOption and UnpackOption. These instructions translate between the passed-by-pointer structs used in wasm32-wasi and the format expected by the rest of wasm-bindgen. Since I think dereferencing pointers is not quite possible in WIT, I just assumed that the wasi ABI is incompatible with most WIT stuff. If I am wrong about this I am happy to change this.

I tried to make the existing tests work with the wasi target, but for this I needed to add a few "mock" system calls in from of empty javascript functions, since there doesn't seem to be a way to disable the std lib with this target.

I am very open to suggestions on this.

Rob2309 avatar Feb 19 '23 17:02 Rob2309

I am thinking about implementing the WASI systemcalls as optional intrinsics, so that a wasi environment is not needed when running the generated module.

Rob2309 avatar Feb 21 '23 12:02 Rob2309

I just added the environment variable flag "WASM_BINDGEN_EMULATE_WASI" that introduces many WASI systemcalls as intrinsics. While not all syscalls are implemented yet, the tests run fine on wasm32-wasi without any external wasi shims.

Rob2309 avatar Feb 21 '23 18:02 Rob2309

What is the exact use-case of wasm-bindgen for WASI? As far as I understood https://github.com/rustwasm/wasm-bindgen/issues/1841, it was for things like using the interface proposal, but that never came into fruition.

So currently I'm not sure what the purpose of wasm-bindgen is without access to JS. Right now I'm more in favor of https://github.com/rustwasm/wasm-bindgen/pull/3233, which seeks to disable the functionality of wasm-bindgen when targeting WASI, like it works for non-Wasm targets right now.

I'm wholly ignorant about WASI btw, so I'm probably missing the big-picture here.

daxpedda avatar May 11 '23 13:05 daxpedda

What is the exact use-case of wasm-bindgen for WASI? As far as I understood #1841, it was for things like using the interface proposal, but that never came into fruition.

So currently I'm not sure what the purpose of wasm-bindgen is without access to JS. Right now I'm more in favor of #3233, which seeks to disable the functionality of wasm-bindgen when targeting WASI, like it works for non-Wasm targets right now.

I'm wholly ignorant about WASI btw, so I'm probably missing the big-picture here.

The main point for doing this was to support compiling C code (e.g. with the cc crate) and linking it into a library that uses wasm-bindgen. Since C does not support the wasm-bindgen calling convention, I instead opted for supporting the wasi calling convention in wasm-bindgen.

Rob2309 avatar May 16 '23 07:05 Rob2309

The main point for doing this was to support compiling C code (e.g. with the cc crate) and linking it into a library that uses wasm-bindgen.

Apologies for any cluelessness here: would it be possible to compile Rust with the usual wasm32-unknown-unknown target while linking to a C library compiled with wasm32-wasi?

daxpedda avatar May 16 '23 09:05 daxpedda

Apologies for any cluelessness here: would it be possible to compile Rust with the usual wasm32-unknown-unknown target while linking to a C library compiled with wasm32-wasi?

It should be possible, but currently isn't, mainly because Rust's "C" ABI on wasm32-unknown-unknown doesn't match the one used by clang (which is defined in https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md). There are a lot of different issues opened about this, but rustwasm/team#291 seems to be the main one.

The difference between them is that Rust flattens structs out into one parameter per field, whereas clang passes them via. a pointer. The wasm32-wasi target does match, which is why it works for linking to C code.

There's also then the issue of somehow providing the WASI imports needed by the C library, which would probably involve having to add a way to pass wasm imports to init. I'm not sure how you could get that to work on targets without init, though.

Liamolucko avatar May 19 '23 02:05 Liamolucko

There's also then the issue of somehow providing the WASI imports needed by the C library, which would probably involve having to add a way to pass wasm imports to init. I'm not sure how you could get that to work on targets without init, though.

Maybe that's the part we could provide here in wasm-bindgen, but that's useless right now if the problem is still in Rust.

daxpedda avatar May 19 '23 11:05 daxpedda

As I understand it, this will allow compiling Rust code to wasm32-wasi and executing it in browser, right? I'm in favor of this as long as the compilation is for wasm32-unknown-unknown target but the generated code is compatible with wasm32-wasi ABI. That should allow C code.

That combined with https://github.com/rustwasm/wasm-bindgen/pull/3233, means that wasm32-wasi is explicitly incompatible with wasm-bindgen but there is an option to use an ABI, that is the same as clang, thus allowing linking with C code

Question: assuming my understanding is correct (please correct me if it isn't), why not make this the default?

ranile avatar May 19 '23 15:05 ranile

Question: assuming my understanding is correct (please correct me if it isn't), why not make this the default?

What do you mean with "default"?

daxpedda avatar May 19 '23 15:05 daxpedda

Question: assuming my understanding is correct (please correct me if it isn't), why not make this the default?

What do you mean with "default"?

The wasi-abi flag this PR introduces. Why not make it the default?

ranile avatar May 19 '23 16:05 ranile

Yeah, that sounds reasonable to me too.

daxpedda avatar May 19 '23 16:05 daxpedda

The wasi-abi flag this PR introduces. Why not make it the default?

The problem I see with this is that this ABI is currently completely incompatible with any wasm target except wasm32-wasi. I don't think there is a good way to make this the default without breaking most stuff.

The main usecase I had for this PR is allowing to run a rust program linked to C code in a browser. For this, the PR also introduces basic wasi shims that do enough to make programs work that expect correct wasi system calls.

Rob2309 avatar May 19 '23 19:05 Rob2309

1+ for making wasm32-wasi the defaut. It will allow Rust wasm modules to use many things that wasn't possible before. Things like std::thread or std::time::Instant

temeddix avatar Oct 25 '23 16:10 temeddix

1+ for making wasm32-wasi the defaut. It will allow Rust wasm modules to use many things that wasn't possible before. Things like std::thread or std::time::Instant

wasm-bindgen doesn't control how you compile your Rust code. We were discussing the default for adding the shim/ABI compatibility, not which target is chosen.

The whole point here is to build on what Hamza said:

I'm in favor of this as long as the compilation is for wasm32-unknown-unknown target but the generated code is compatible with wasm32-wasi ABI. That should allow C code.

Though I guess making wasm-bindgen compatible with Rust code that is compiled to wasm32-wasi is possible as well, it is a different discussion.

daxpedda avatar Oct 25 '23 16:10 daxpedda

Though I guess making wasm-bindgen compatible with Rust code that is compiled to wasm32-wasi is possible as well, it is a different discussion.

If I remember correctly, this is exactly what this PR does. I am honestly not quite sure anymore since so much time has passed.

Rob2309 avatar Oct 25 '23 17:10 Rob2309

Thanks for the correction :)

I hope this gets reviewed by the authors, as this PR basically fixes some limitations that arise from wasm32-unknown-unknown. wasm32-wasi is actively being developed and popular crates like tokio are adding support for it. Would this change make into the master? @Rob2309 mentioned that he's open to suggestions.

temeddix avatar Oct 25 '23 17:10 temeddix

As mentioned above, I agree mostly with Hamza that I would prefer a solution that adds WASI compatibility by adding a shim to make linked WASI libraries work. This means Rust should be compiled with wasm32-unknown-unknown but linked C/C++ libraries can be compiled with wasm32-wasi.

This isn't a solution that can be applied today because of #3454, so I would argue that fixing #3454 is the way forward right now.

Supporting wasm32-wasi directly can be discussed here: https://github.com/rustwasm/wasm-bindgen/issues/3421. Though I would generally prefer not to do that for reasons I wrote up there, but I'm happy to hear more arguments and views.

daxpedda avatar Oct 25 '23 17:10 daxpedda

To add to what I said previously, I would rather have wasm32-unknown-unknown be compatible with C ABI than wasm32-wasi be supported. wasm32-wasi has functions like file system access which obviously can't be provided in a browser environment so I'm a little hesitant to add wasm32-wasi compatibility mode. We would effectively be shipping a shim like browser_wasi_shim if we add this support

ranile avatar Oct 25 '23 20:10 ranile

We would effectively be shipping a shim like browser_wasi_shim if we add this support

That's the idea. But if this can be done without the help of wasm-bindgen, or make adjustments to enable that, even better.

daxpedda avatar Oct 25 '23 20:10 daxpedda

wasmer-js claimed that it supports polyfills for system I/O, and that threads polyfill(using webworkers) is also coming soon.

  • https://github.com/wasmerio/wasmer-js#usage
  • https://github.com/wasmerio/wasmer-js/issues/332
  • https://github.com/wasmerio/wasmer-js/pull/328

IMHO, maybe wasmer-js can be the standard WASI shim for wasm-bindgen. Perhaps a comparison between browser_wasi_shim and wasmer-js can be done.

image

Either way, this change will allow std::fs, std::time::Instant, std::thread work on the web as it is, allowing so many crates work on the web just like they do on native.

temeddix avatar Oct 25 '23 20:10 temeddix

I haven't really been paying attention on the wasm progress in rust. Is there still a realistic usecase for this PR?

Rob2309 avatar Apr 24 '24 17:04 Rob2309

Now that https://github.com/rust-lang/rust/pull/117919 was merged, we could start thinking about the proposal I outlined in https://github.com/rustwasm/wasm-bindgen/pull/3324#issuecomment-1779753420.

daxpedda avatar May 20 '24 18:05 daxpedda

Now that rust-lang/rust#117919 was merged, we could start thinking about the proposal I outlined in #3324 (comment).

If I understand correctly, one would simply compile a crate with wasm32-unknown-unknown with wasm-bindgen and import c functions with extern "wasm-c"? This would render this PR unnecessary right?

Rob2309 avatar May 21 '24 12:05 Rob2309

If I understand correctly, one would simply compile a crate with wasm32-unknown-unknown with wasm-bindgen and import c functions with extern "wasm-c"?

The PR didn't introduce a new ABI, just adjusted the "C" ABI to behave correctly, ergo compatible with wasm32-wasi.

wasm-bindgen should allow adding the WASI shim, whether this requires any changes to wasm-bindgen or not is whats missing here. Preferably a test should be added. I guess it might be best to do this in a new PR.

daxpedda avatar May 21 '24 13:05 daxpedda