Official binaries for wasm32-unknown-unknown (and potentially other WASM platforms?) contain code for the wrong architecture

[emilazy]

The compiler-builtins crate compiles C code for WASM platforms since rust-lang/compiler-builtins#566. This works if the C compiler is Clang, as it passes the appropriate -target. However, in a GCC build environment this means that the cc crate will end up silently compiling code for the wrong architecture entirely. This means that, for example, the compiler-builtins shipping for wasm32-unknown-unknown via Rustup contains object files like the following:

45c91108d938afe8-cmpti2.o: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), with debug_info, not stripped

I suppose that the build for these needs to arrange for Clang to be present, or perhaps even specified explicitly in the target.*.{cc,cxx,linker} settings.

(Was redirected here from rust-lang/compiler-builtins#732.)

[jyn514]

@emilazy are you interested in fixing this? i think there are three main steps:

1. give a hard error in bootstrap when using gcc to compile for wasm
2. change our CI to use clang instead of gcc
3. add a test that compiling a sample program for wasm32-unknown doesn't give any linker warnings

optionally, you could also change bootstrap to default to clang instead of cc for wasm32-unknown.

i'm happy to mentor this.

@rustbot label E-mentor

[JayAndJef]

Hi @jyn514! If @emilazy doesn't want to take on this issue, I would be glad to. I'm not sure if this is beyond the scope of mine or yours, however, as i'm looking for a good first issue in rustc to resolve - would you be willing to mentor?

[jyn514]

I'm still willing to mentor. I think this would be a challenging first issue but you could do it if you're motivated. There are lots of small things to change, but you will (mostly) have short feedback loops, and finding which parts of the code are relevant will not be too hard I think. Take a look at src/bootstrap, at the dev guide for bootstrap, and at the wasm32-unknown platform docs. This check should probably (?) live in sanity.rs, which has flaws, but it's what we have right now.

[JayAndJef]

I think I'll take this issue on. Are there any steps I need to take besides forking the repository, such as claiming this issue or moving to a different platform to communicate with you?

[jyn514]

both those, yeah - i think @rustbot claim is the current syntax?

i'm on zulip as "jyn".

[JayAndJef]

Great, I'll scope out the bootstrap module

@rustbot claim

[tgross35]

This means that, for example, the compiler-builtins shipping for wasm32-unknown-unknown via Rustup contains object files like the following:

45c91108d938afe8-cmpti2.o: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), with debug_info, not stripped

That seems like the sort of thing that would have gotten noticed almost immediately, compiler-builtins is always provided at linktime. How is this working currently - are builtins never actually needed or is it somehow possible to link wasm and x86 objects together?

[jyn514]

That seems like the sort of thing that would have gotten noticed almost immediately, compiler-builtins is always provided at linktime. How is this working currently - are builtins never actually needed or is it somehow possible to link wasm and x86 objects together?

bootstrap supports disabling optimized-compiler-builtins (explicitly, since #102579). in that case it uses the rust symbols defined in https://github.com/rust-lang/compiler-builtins/ directly, not the ones defined in LLVM.

you can see in #136096 that LLD unconditionally warns when compiling to wasm32-unknown, but it is silenced by rustc so you can't see the warning. I suspect what has happened is:

• someone turned on optimized-compiler-builtins = true for wasm32-unknown in rl/r CI and just assumed that would work.
• that had absolutely no effect, because gcc doesn't support wasm and CI uses gcc to build llvm for this target. instead it fell back to using the weak symbols defined in rust, not the strong symbols defined in llvm.
• because rustc silenced the warning, no one noticed until Emily actually inspected the final binary and noticed that it wasn't using the optimized builtins.

cc provides is_like_clang. Would things work better if we just disabled the C build if the target is wasm and not is_like_clang?

@tgross35 i think doing that in compiler-builtins would be a good step, yeah. but it still requires work downstream in rust-lang/rust - all the steps in #132802 (comment) are still required; maybe we could get away with delaying step 1 until bootstrap actually builds compiler-builtins, but it would ideally be nice to catch it early.

[jyn514]

cc #105065

[trevyn]

That seems like the sort of thing that would have gotten noticed almost immediately, compiler-builtins is always provided at linktime. How is this working currently - are builtins never actually needed or is it somehow possible to link wasm and x86 objects together?

I think this is a result of rust-lang/compiler-builtins#566

Basically, this doesn’t affect any builtins that are called by Rust, so if you’re only building Rust code for wasm, you’ll never run into this.

[jyn514]

Basically, this doesn’t affect any builtins that are only called by Rust, so if you’re only building Rust code for wasm, you’ll never run into this.

i am confused - i thought the builtins have the same mangled names at runtime, regardless of whether you use the C builtins or the rust builtins, the only difference is that the rust builtins are weak symbols and the c builtins are strong.

looking at rust-lang/compiler-builtins#566, it seems it doesn't make any changes other than to compile the C builtins? so it makes sense to me it's falling back to the rust ones. but i don't understand what "only called by rust" has to do with anything.

[trevyn]

Sorry, I added a spurious “only” there and fixed it in edit.

My understanding is that the only builtins that are miscompiled are ones that Rust never calls.

But if you use clang to compile some C to wasm (to ultimately link with your Rust), it may use these extra builtins.

So it might be useful to make these builtins available.

Which it sounds like you have a plan to make possible. :)