From 992b0b3fe7006f72f00ce8d673a3fca600119555 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 26 Aug 2024 14:34:24 -0700 Subject: [PATCH 1/2] Document the broken C ABI of `wasm32-unknown-unknown` Inspired by discussion on https://github.com/rust-lang/rust/issues/129486 this is intended to at least document the current state of the world in a more public location than throughout a series of issues. --- .../wasm32-unknown-unknown.md | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md b/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md index 12b7817c38277..6ce022e5ebba2 100644 --- a/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md +++ b/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md @@ -195,3 +195,116 @@ conditionally compile code instead. This is notably different to the way native platforms such as x86\_64 work, and this is due to the fact that WebAssembly binaries must only contain code the engine understands. Native binaries work so long as the CPU doesn't execute unknown code dynamically at runtime. + +## Broken `extern "C"` ABI + +This target has what is considered a broken `extern "C"` ABI implementation at +this time. Notably the same signature in Rust and C will compile to different +WebAssembly functions and be incompatible. This is considered a bug and it will +be fixed in a future version of Rust. + +For example this Rust code: + +```rust +#[repr(C)] +struct MyPair { + a: u32, + b: u32, +} + +extern "C" { + fn take_my_pair(pair: MyPair) -> u32; +} + +#[no_mangle] +pub unsafe extern "C" fn call_c() -> u32 { + take_my_pair(MyPair { a: 1, b: 2 }) +} +``` + +compiles to a WebAssembly module that looks like: + +```wasm +(module + (import "env" "take_my_pair" (func $take_my_pair (param i32 i32) (result i32))) + (func $call_c + i32.const 1 + i32.const 2 + call $take_my_pair + ) +) +``` + +The function when defined in C, however, looks like + +```c +struct my_pair { + unsigned a; + unsigned b; +}; + +unsigned take_my_pair(struct my_pair pair) { + return pair.a + pair.b; +} +``` + +```wasm +(module + (import "env" "__linear_memory" (memory 0)) + (func $take_my_pair (param i32) (result i32) + local.get 0 + i32.load offset=4 + local.get 0 + i32.load + i32.add + ) +) +``` + +Notice how Rust thinks `take_my_pair` takes two `i32` parameters but C thinks it +only takes one. + +The correct definition of the `extern "C"` ABI for WebAssembly is located in the +[WebAssembly/tool-conventions](https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md) +repository. The `wasm32-unknown-unknown` target (and only this target, not other +WebAssembly targets Rust support) does not correctly follow this document. + +Example issues in the Rust repository about this bug are: + +* [#115666](https://github.com/rust-lang/rust/issues/115666) +* [#129486](https://github.com/rust-lang/rust/issues/129486) + +This current state of the `wasm32-unknown-unknown` backend is due to an +unfortunate accident which got relied on. The `wasm-bindgen` project prior to +0.2.89 was incompatible with the "correct" definition of `extern "C"` and it was +seen as not worth the tradeoff of breaking `wasm-bindgen` historically to fix +this issue in the compiler. + +Thanks to the heroic efforts of many involved in this, however, the nightly +compiler currently supports a `-Zwasm-c-abi` implemented in +[#117919](https://github.com/rust-lang/rust/pull/117919). This nightly-only flag +can be used to indicate whether the spec-defined version of `extern "C"` should +be used instead of the "legacy" version of +whatever-the-Rust-target-originally-implemented. For example using the above +code you can see (lightly edited for clarity): + +``` +$ rustc +nightly -Zwasm-c-abi=spec foo.rs --target wasm32-unknown-unknown --crate-type lib --emit obj -O +$ wasm-tools print foo.o +(module + (import "env" "take_my_pair" (func $take_my_pair (param i32) (result i32))) + (func $call_c (result i32) + ) + ;; ... +) +``` + +which shows that the C and Rust definitions of the same function now agree like +they should. + +The `-Zwasm-c-abi` compiler flag is tracked in +[#122532](https://github.com/rust-lang/rust/issues/122532) and a lint was +implemented in [#117918](https://github.com/rust-lang/rust/issues/117918) to +help warn users about the transition. The current plan is to, in the future, +switch `-Zwasm-c-api=spec` to being the default. Some time after that the +`-Zwasm-c-abi` flag and the "legacy" implementation will all be removed. From 2d6d6a84df371390046269fe4955d2e4300f118f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 27 Aug 2024 07:33:49 -0700 Subject: [PATCH 2/2] Updates/clarifications --- .../platform-support/wasm32-unknown-unknown.md | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md b/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md index 6ce022e5ebba2..0e06a820a22c3 100644 --- a/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md +++ b/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md @@ -205,7 +205,7 @@ be fixed in a future version of Rust. For example this Rust code: -```rust +```rust,ignore (does-not-link) #[repr(C)] struct MyPair { a: u32, @@ -280,20 +280,22 @@ unfortunate accident which got relied on. The `wasm-bindgen` project prior to seen as not worth the tradeoff of breaking `wasm-bindgen` historically to fix this issue in the compiler. -Thanks to the heroic efforts of many involved in this, however, the nightly -compiler currently supports a `-Zwasm-c-abi` implemented in +Thanks to the heroic efforts of many involved in this, however, `wasm-bindgen` +0.2.89 and later are compatible with the correct definition of `extern "C"` and +the nightly compiler currently supports a `-Zwasm-c-abi` implemented in [#117919](https://github.com/rust-lang/rust/pull/117919). This nightly-only flag can be used to indicate whether the spec-defined version of `extern "C"` should be used instead of the "legacy" version of whatever-the-Rust-target-originally-implemented. For example using the above code you can see (lightly edited for clarity): -``` +```shell $ rustc +nightly -Zwasm-c-abi=spec foo.rs --target wasm32-unknown-unknown --crate-type lib --emit obj -O $ wasm-tools print foo.o (module (import "env" "take_my_pair" (func $take_my_pair (param i32) (result i32))) (func $call_c (result i32) + ;; ... ) ;; ... ) @@ -305,6 +307,8 @@ they should. The `-Zwasm-c-abi` compiler flag is tracked in [#122532](https://github.com/rust-lang/rust/issues/122532) and a lint was implemented in [#117918](https://github.com/rust-lang/rust/issues/117918) to -help warn users about the transition. The current plan is to, in the future, -switch `-Zwasm-c-api=spec` to being the default. Some time after that the -`-Zwasm-c-abi` flag and the "legacy" implementation will all be removed. +help warn users about the transition if they're using `wasm-bindgen` 0.2.88 or +prior. The current plan is to, in the future, switch `-Zwasm-c-api=spec` to +being the default. Some time after that the `-Zwasm-c-abi` flag and the +"legacy" implementation will all be removed. During this process users on a +sufficiently updated version of `wasm-bindgen` should not experience breakage.