Warn when target is wasm32-not-wasi

[TheBlueMatt]

Because of the ABI incompatibility between Rust's wasm32-unknown target and upstream LLVM/clang's wasm32 target, linking Rust and C code (on Rust targets other than wasm32-wasi) may be unsafe (see rustwasm/team#291 for more details). However, in many cases there are no warnings at all and undefined behavior may instead simply be invoked. It is likely prudent for cc-rs to generate such a warning, given its in the best position to "see" the issue, simply printing something to stderr if the target is wasm32 and not wasi.

[dot-asm]

It's not like nothing works, right? If you know how to play your cards, you can have Rust's wasm32-unknown interoperate with clang. So the question is really about identifying the incompatibilities and the assertion is that the toolchain will let you through without so much as a warning. Well, it doesn't seem to hold upon inspection. I've tried to trigger the problem that would be a result of the said incompatibility and have got a very informative error message from rust-lld which in fact illuminated the incompatibility.

Just in case for reference. Here is the example. On the C side:

typedef struct { int a, b; } ab;
int sum(ab x) { return x.a + x.b; }

On the Rust side (which is the right way to go about on all platforms but wasm32-unknown):

#[repr(C)]
pub struct ab {
    pub a: i32,
    pub b: i32,
}
extern "C" {
    pub fn sum(x: ab) -> i32;
}
fn main() {
    let x = ab{a: 1, b: 2};
    println!("{}", unsafe { sum(x) });
}

The error message from rust-lld:

rust-lld: error: function signature mismatch: sum
          >>> defined as (i32, i32) -> i32 in /.../target/wasm32-unknown-unknown/release/deps/wasm_test-9cf57e2afcff62b1.wasm_test.9391aa4e-cgu.0.rcgu.o
          >>> defined as (i32) -> i32 in /.../target/wasm32-unknown-unknown/release/build/wasm-test-92361b6eef4349d8/out/liba.a(a.o)

[dot-asm]

BTW

On the Rust side (which is the right way to go about on all platforms but wasm32-unknown):

It's actually "[all] but wasm32-unknown-unknown." I mean among the three Rust wasm targets, wasm32-unknown-emscripten, wasm32-unknown-unknown and wasm32-wasi, only one [in the middle] appears to be problematic. [As opposed to the originally suggested "all are problematic but wasm32-wasi".]

[TheBlueMatt]

the number in the readme is descriptive, not a policy; i'll update it

Right, I believe some crates have thought about this and do theoretically support it, so you're right it shouldn't be a hard-error, or at least should have an override.

assertion is that the toolchain will let you through without so much as a warning

I don't think that was quite my assertion, but a few notes:
(a) ideally we'd make this a hard error, it's obviously very unsafe to run that code,
(b) it's unclear to me if we're guaranteed to get a warning - the linker can only detect the error if the parameters are different once converted to a list of basic integer types, not whether the parameters are being interpreted differently (IIUC the issue is pointer-vs-struct-as-values). I admit I haven't dug into it fully, but I'd really like to see a strong argument for the linker always warning on any incompatibility.
(c) IME cargo likes to hide any warnings generated by the host toolchain, did you receive that warning with default cargo compilation or rustc?

[TheBlueMatt]

It's actually "[all] but wasm32-unknown-unknown." I mean among the three Rust wasm targets, wasm32-unknown-emscripten, wasm32-unknown-unknown and wasm32-wasi, only one [in the middle] appears to be problematic. [As opposed to the originally suggested "all are problematic but wasm32-wasi".]

Ah, right, it's a bit more subtle, I believe, in that 'em scripted and wasi have different ABIs, but both are compatible with their respective C compilers.

[dot-asm]

(a) ideally we'd make this a hard error, it's obviously very unsafe to run that code,

This is not something that cc-rs would be able to pull off. It would be the job for the Rust compiler, one that passes structures by value.

(b) it's unclear to me if we're guaranteed to get a warning - the linker can only detect the error if the parameters are different once converted to a list of basic integer types, not whether the parameters are being interpreted differently (IIUC the issue is pointer-vs-struct-as-values). I admit I haven't dug into it fully, but I'd really like to see a strong argument for the linker always warning on any incompatibility.

It wasn't a warning, but a hard failure. As for parameter mismatches. Concern essentially is that it won't catch the cases when the size of the structure argument is not larger than 32 bits, right? Well, this actually seems to work out. As it turns out, if the structure is not larger than 32 bits, then C expects it to be passed by value, and Rust does exactly that. In other words the linker should arguably catch all the mismatches...

(c) IME cargo likes to hide any warnings generated by the host toolchain, did you receive that warning with default cargo compilation or rustc?

As already mentioned, it was a hard failure, so there was no obfuscation on cargo part or ambiguity.

[TheBlueMatt]

It wasn't a warning, but a hard failure.

Oh! That's the source of a lot of confusion here, I think. What version of wasm-ld are you using? Locally that's a warning, not a failure.

if the structure is not larger than 32 bits, then C expects it to be passed by value, and Rust does exactly that.

Ah, I had understood that the clang wasm ABI did not have that behavior. If that's true then indeed as long as wasm-ld hard-fails then it seems like there should be no issue.

[dot-asm]

Oh! That's the source of a lot of confusion here, I think. What version of wasm-ld are you using? Locally that's a warning, not a failure.

As mentioned, it was rust-lld that failed on me. The one that is part of the stock Rust 1.6x(*) installation. For example mine is called ~/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin/rust-lld. Is it possible that it depends on the fact that I don't have wasm-ld on the program search path?

(*) I'm writing 1.6x, because I'm relatively sure that it's not something that was introduced in the most recent 1.63. But I don't really know how far back does it go.

[dot-asm]

I've tried to configure wasm-ld as linker through .cargo/config.toml and the compilation fails with "error: unknown argument: --rsp-quoting=posix" [from wasm-ld]. So wasm-ld does not appear to be a viable option, at least not for 1.63.

[TheBlueMatt]

As mentioned, it was rust-lld that failed on me

Ah, I missed that, thanks for calling it out. IIUC rust-lld is the same as system-lld, but built from the same LLVM version as rust, rather than whatever system LLVM version is built into system lld. I could be wrong here, though.

I've tried to configure wasm-ld as linker through .cargo/config.toml and the compilation fails with "error: unknown argument: --rsp-quoting=posix" [from wasm-ld]. So wasm-ld does not appear to be a viable option, at least not for 1.63.

lld is a thing wrapper around relevant ld's, and calls out to wasm-ld if the target is wasm, or at least that's what it does for my system lld.

In any case, given the latest rust-lld hard-errors on these issues, and we believe its not possible to have the two ABIs generate the same function signature in an incompatible way, I think we can close this. It'd be nice to dig into it more, but it sounds like maybe newer LLVM simply changed the behavior and now errors.

[dot-asm]

lld is a thin wrapper around relevant ld's, and calls out to wasm-ld if the target is wasm, or at least that's what it does for my system lld.

Hmm... I can't confirm this. If I strace -f -e trace=process ... I don't see that rust-lld would call any other linker...

[TheBlueMatt]

$ lld
lld is a generic driver.
Invoke ld.lld (Unix), ld64.lld (macOS), lld-link (Windows), wasm-ld (WebAssembly) instead

Its possible rust-lld is actually completely different, then, or its just ld.lld or wasm-ld with lld's argument parsing somehow.

[dot-asm]

It can't be just "or wasm-ld", because as far as I understand it's used for all bare-metal/no-std platforms. And if I run ldd rust-lld it says it's linked with libLLVM-14-rust-1.63.0-stable.so. So it appears to be a special-purpose independent tool...

[dot-asm]

There might be an explanation for why our experiences appear to differ. I'm experimenting directly with cargo build --target=wasm32-unknown-unknown and disassemble the resulting .wasm blob to examine what's going on [on the Rust-C boundary]. While you might be using wasm-pack, which might override rustc default and direct it toward wasm-ld, which in turn might let mismatching signatures through, maybe with a warning. (I'm not actually proficient at wasm-pack-ing, I tried it only once, with partial success as far as I recall.) Would it make it appropriate to have cc-rs issue a warning? I'd argue no. It would be more appropriate to make wasm-ld reject the mismatching signatures...

[TheBlueMatt]

I was actually using clang for linking for cross-language LTO, not building directly with cargo, so my experience is definitely not 100% representative (and I originally pushed for the wasi ABI switch, primarily so that I could do x-lang LTO here, so its not so much an issue for me, more because I've seen people showing up trying to build a crate I help maintain in wasm-pack and my initial response was "lol, no way that works, that's probably gonna crash, or corrupt memory").

I agree if wasm-ld can 100% detect this issue the correct path is to have it fail, not to have cc try to work around it. Because it sounds like it can, and it sounds like at least in default configurations things will already fail, I agree this issus is likely not necessary. It'd be nice to test wasm-pack to double-check (since that was my original concern), but if it fails, we can probably close and move on.

[dot-asm]

An additional bit of information. wasm32-unknown-emscripten does call wasm-ld, and it still treats mismatching signatures as a warning, not an error. But it's not a problem in the context of this query. Because there is no ABI mismatch in emscripten target. So that if wasm-ld issues the warning, then it's for an unrelated reason. One can make a case for having cc-rs always pass -Wl,--fatal-warnings to guard against situations when the signature mismatch is a result of an actually buggy declaration on the Rust side.

[thomcc]

I think a warning would be appropriate. There are a couple cases where this is fine ("I don't plan on calling it from Rust", "I've manually ensured the ABI issues are avoided", ...), but that's fine for warnings.

[dot-asm]

To summarize. This is about matching ABIs on the Rust and clang sides, and the only problematic target is wasm32-unknown-unknown. And for this target Rust invokes rust-lld which refuses to link problematic code with an informative message. With this in mind it's argued that a general vague warning is not actually necessary.

[thomcc]

Ah, right, thanks. That's not really enough in all cases, as the linker does not see everything -- for example, there still would be issues in the case of C APIs which take/return function pointers (possibly indirectly e.g. via a vtable or something). I don't see how the linker could detect this, but maybe there's some way.

Thinking more though, I guess libraries with APIs that use function pointers with problematic signatures are fairly likely to have the same patterns occur in their public APIs too, so it might be very rare. That is, the question isn't "how many C APIs have function pointers that would hit this ABI issue", it's "how many C APIs have function poijnters that would hit this ABI issue, and don't also have functions that would".

I tend to think a warning couldn't hurt, to help ensure people doing this are at least aware of this issue.

[dot-asm]

... in the case of C APIs which take/return function pointers... I don't see how the linker could detect this, but maybe there's some way.

No, linker won't detect this. However! The [said] signature mismatches would be caught at run time. How informative the message is naturally depends on the vm, but at least node reports "function signature mismatch" [in non-release build].

I tend to think a warning couldn't hurt, to help ensure people doing this are at least aware of this issue.

If cc-rs maintainers choose this path, I'd advocate to for providing a way to suppress the warning. So that those crate maintainers who did the work to ensure that their code works at wasm32-unknown-unknown platform would be excused from answering "it's safe to ignore" over and over :-)