Skip to content
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

Document wasm-bindgen incompatibility with C/C++ #2209

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tlively
Copy link

@tlively tlively commented Jun 17, 2020

cc @RReverser @alexcrichton. I'm not sure if there would be a better or more prominent place to document this problem.

cc @RReverser @alexcrichton. I'm not sure if there would be a better or more prominent place to document this problem.
@DzmitryFil
Copy link

Huge problem that comes out of this incompatibility is absence of high performance memory allocators for wasm32-unknown-unknown target, as all of the currently available are written in c/c++.

@alexcrichton
Copy link
Contributor

Thanks for the PR! I do think it's worthwhile to document the interactions with C/C++, but I don't think we should say that wasm-bindgen is entirely incompatible with C/C++. I think if we document it we'd want to be pretty precise, along the lines of:

  • It's pretty difficult to do and there's little prior art of getting it done. Mixing is recommended to use the Emscripten target.
  • Otherwise users just need a native C/C++ toolchain target wasm32-unknown-unknown, although this is typically difficult to acquire.
  • Even if all that is acquired there's still an ABI difference to watch out for, but that's not required for correctness of specific programs.

@tlively
Copy link
Author

tlively commented Jun 18, 2020

@alexcrichton, it sounds like the most straightforward path that is most likely to work for folks who want to use both wasm-bindgen and C/C++ is to use the wasi target for both Rust and C/C++ and make sure no structs are passed or returned by value across the boundary. Then to run on the web they should use the WASI web polyfill. Does that sound right?

@RReverser
Copy link
Member

make sure no structs are passed or returned by value across the boundary

I think with this restrictions many libraries will sufficiently work on wasm32-unknown-unknown too.

WASI target is problematic in its own ways on the Web - namely, as mentioned on the original issue, you usually want either Emscripten or wasm-bindgen to generate JS bindings for you if you're interacting with JavaScript APIs in any meaningful way, and WASI target won't work with either of those.

@tlively
Copy link
Author

tlively commented Jun 18, 2020

My bad, my ignorance of wasm-bindgen is showing :)

I'm trying to figure out what we can tell users to do that doesn't involve building their own C/C++ wasm32-unknown-unknown toolchain, since that seems difficult enough that it's not worth mentioning. They can't use Emscripten because its JS will interfere with wasm-bindgen's JS, so that really only leaves WASI. Could users use WASI for their C/C++ and wasm32-unknown-unknown for their Rust? That was wasm-bindgen will still give them JS, but I guess they would still have to supply some WASI implementations somehow.

@RReverser
Copy link
Member

Could users use WASI for their C/C++ and wasm32-unknown-unknown for their Rust?

That's what I've done on few occasions, yeah - but note that they can't use WASI JS polyfill, instead they just need to make sure that library doesn't do any I/O, which is often sufficient.

On other occasions, where library can support -nostdlib, like with libdeflate, I've just used wasm32-unknown-unknown - for these occasions the headers Clang provides are sufficient.

But yeah, we (as in community) definitely lack some target that would make C / C++ "just compile". I'd imagine such target having a sysroot that is similar in functionality to Rust's wasm32-unknown-unknown target in that it allows any APIs to compile with stubs for I/O that error out if ever reached at runtime.

In most cases, this would be sufficient and much simpler to port to than either target/solution that exists today.

@alexcrichton
Copy link
Contributor

Practically speaking wasm-bindgen is incompatible with C/C++. Technically though there's nothing incompatible, it's just how toolchains are setup. That's the "practically incompatible" part though where there is no official toolchain for C/C++ targeting wasm32-unknown-unknown. There is for wasm32-wasi but running WASI code on the web also isn't the greatest today unfortunately.

I basically just want to be clear in the docs that it's unlikely to get work and Emscripten is the recommended solution for mixing C/C++ and Rust on the web. I'd rather not document wasm-bindgen as incompatible with C/C++ though because there's nothing it does that's inherently incompatible, we just aren't also a project for getting toolchains from.

@RReverser
Copy link
Member

Practically speaking wasm-bindgen is incompatible with C/C++.

I don't think the original concern is as much about wasm-bindgen itself, as about wasm32-unknown-unknown Rust target being incompatible with wasm32-unknown-unknown C/C++ (Clang) target due to usage of https://github.com/rust-lang/rust/blob/master/src/librustc_target/abi/call/wasm32_bindgen_compat.rs.

This is not something currently documented, and might catch people by surprise. Personally, I've used C/C++ libraries wrapped in Rust crates in a code using wasm-bindgen without issues, but as I've learned from @tlively yesterday, I was lucky and could've run into these ABI incompatibilities.

If I did, I might have gotten to the root of the issue, but it would take a lot of time, and might catch other users by surprise as well, especially because such a library can be wrapped in a crate somewhere deep down the dependency tree.

@alexcrichton
Copy link
Contributor

Yeah that's for sure something to document, but that is not "wasm-bindgen and C/C+ are fundamentally incompatible", that's just a gotcha to watch out for. Most C/C++ FFI bindings don't pass structs-by-value so it's a moot point anyway.

@RReverser
Copy link
Member

"wasm-bindgen and C/C+ are fundamentally incompatible", that's just a gotcha to watch out for

Yeah it's more like "wasm-bindgen ABI" or whatever we call that compat layer file.

Most C/C++ FFI bindings don't pass structs-by-value so it's a moot point anyway.

I've definitely used (and wrote bindgen and C/C+ are fundamentally incompatible", that's just a gotcha to watch out for

Yeah it's more like "wasm-bindgen ABI" or whatever we call that compat layer file.

Most C/C++ FFI bindings don't pass structs-by-value so it's a moot point anyway.

I've definitely used (and wrote 😬) a few that did in the past, so it might be important to some other users.

@RReverser
Copy link
Member

I wonder how hard is it to just fix this on wasm-bindgen side? Does it require major version bump because older wasm-bindgen is expected to work against newer projects or something like that?

@alexcrichton
Copy link
Contributor

I have not personally put that much effort into fixing this. The ABI was likely to change anyway with multi-value now stabilizing so it didn't seem to pressing to tackle.

@elichai
Copy link

elichai commented Jun 21, 2020

FWIW in rust-secp256k1, we were scared of ABI problems in wasm32-unknown-unknown so we test at runtime that all the types used in the FFI have matching size and alignment:
The runtime call: https://github.com/rust-bitcoin/rust-secp256k1/blob/a5147bbf015ca61bb0e4e09adb8e9b324965c291/src/context.rs#L110
The things it check: https://github.com/rust-bitcoin/rust-secp256k1/blob/a5147bbf015ca61bb0e4e09adb8e9b324965c291/secp256k1-sys/src/types.rs#L47
The C part: https://github.com/rust-bitcoin/rust-secp256k1/blob/a5147bbf015ca61bb0e4e09adb8e9b324965c291/secp256k1-sys/wasm-sysroot/stdio.h

@RReverser
Copy link
Member

I have not personally put that much effort into fixing this. The ABI was likely to change anyway with multi-value now stabilizing so it didn't seem to pressing to tackle.

Are you saying that with multi-value becoming part of ABI now / soon, this issue might be fixed in one sweep as well?

@tlively
Copy link
Author

tlively commented Jun 21, 2020

I have not personally put that much effort into fixing this. The ABI was likely to change anyway with multi-value now stabilizing so it didn't seem to pressing to tackle.

Are you saying that with multi-value becoming part of ABI now / soon, this issue might be fixed in one sweep as well?

That's been the plan since we discovered the ABI mismatch. I didn't expect that multivalue support would take as long as it has, but this is still the plan and I'm still making progress towards a new multivalue C ABI.

@chayleaf
Copy link

chayleaf commented Dec 1, 2020

Emscripten is the recommended solution for mixing C/C++ and Rust on the web

@alexcrichton However wasm-bindgen doesn't support the wasm32-unknown-emscripten target, and doing it via JS wrappers has way too much overhead and boilerplate

Edit: What I meant is when the entire Rust wasm ecosystem depends on wasm-bindgen, it isn't that easy to switch to Emscripten

@coderedart
Copy link

its been a couple years, but is there any progress in regards to this issue? I would like to embed lua into my rust wasm app for scripting, but the bindings are written in c/cpp. :(

@tlively
Copy link
Author

tlively commented Jul 23, 2022

Nope, there hasn't been any progress :( Everyone working in this space has been busy with other things.

@rafaelbeckel
Copy link

I think we can close this because the information in the PR is not accurate anymore. It's now possible to link Rust + C together within wasm32-unknown-unknown by using the new compiler flag in Rust nightly. This will be the default behavior in the near future.

@RReverser
Copy link
Member

I think we can close this because the information in the PR is not accurate anymore.

Well it's still accurate. Having a new experimental flag that can fix the issue is all the more reason to document that the issue exists in the first place :)

@rafaelbeckel
Copy link

@RReverser thanks for the feedback. I was referring to the wording in the changed file, which is not entirely accurate anymore, but I agree I was hasty in suggesting to close the issue. I should have suggested to rephrase it instead.

For example,

"note that only the wasm32-unknown-emscripten target supports linking Rust and C or C++ together"

wasm32-wasi also supports it, and now wasm32-unknown-unknown by using the new flag.

"There is currently no way to both use wasm-bindgenand also safely link with C or C++. There is a general long-term plan to fix this, but there is no immediate solution"

While there is a way now, one could argue the word "safely" makes this phrase accurate. Still, it would be helpful to be more specific here.

Anyway, I'm experimenting with the limits of what can be done with wasm-bindgen and exported C types. I managed to compile a Rust+C binary with wasm-bindgen and a static libc (so I can use malloc and free from C), but making it work at runtime has varying cases of success, depending on what I do with C.

For primitive data types, it works flawlessly, but when I export a function from C that returns a pointer, it compiles, but wasm-bindgen expects such functions to be provided by the env module, which doesn't exist.

I can make it work by manually editing the generated .js wrapper, by removing the import * as __wbg_star0 from "env" and manually defining the __wbg_init_memory function (which is exported empty).

I'm documenting everything and I'll share the results in this thread.

@rafaelbeckel
Copy link

rafaelbeckel commented Jun 29, 2024

As promised, here are my experiments so far: https://github.com/rafaelbeckel/test-c-rust-wasm

The working examples with wasm-bindgen are 4 and 5.

I'd say we can go pretty far for a lot of projects, but in some cases it takes a bit of manual tweaks of the generated JS wrapper. The main limitations come from the Rust compiler itself when dealing with cdylib, not from wasm-bindgen.

@aviplayer
Copy link

@rafaelbeckel I was experimenting with CloudFlare. I tried your examples but it doesn't work.
made a simple example:
let calculator = Calculator::new();
let res = calculator.add(3, 2);
console_log!("Result: {}", res);

Added lib like this: rbg_calculator = { path = "../test-c-rust-wasm/crates/5_rust_bindgen" }

But CloudFlare worker fails:  service core:user:memgraph: Uncaught TypeError: WebAssembly.Instance(): Import #6 "env": module is not an object or function

@rafaelbeckel
Copy link

@aviplayer, sorry for my late answer; I was on vacation and didn't see your message.

This issue arises because C symbols are not exported to a dylib unless you wrap them explicitly in Rust. This thread can give you some context: rust-lang/rfcs#2771.

In the case of WASM, all missing symbols are expected to be provided from the env variable in the WASM imports table.

In my example, the memory.js file injects the missing symbols and provides a simple bump allocator from JS for malloc() and free() calls. Check out the build_with_wasm_pack.sh file in my example.

This was my first iteration for a POC and quite a hack-ish approach. In production, I'm using musl-libc (instead of the OpenBSD subset in the calculator) and have malloc() and free() implemented in Rust directly from the tinylibc crate, so we don't have to provide it from the imports table.

I will update the examples later on.

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 this pull request may close these issues.

9 participants