Fix host code appearing in Wasm binaries #137457

JayAndJef · 2025-02-23T04:43:56Z

This is a direct fix for issue 132802.
Followed the outline as follows:

give a hard error in bootstrap when using gcc to compile for wasm

change our CI to use clang instead of gcc

add a test that compiling a sample program for wasm32-unknown doesn't give any linker warnings

The test-various ci job was also changed.

try-job: test-various
try-job: dist-various-1
try-job: dist-various-2
try-job: x86_64-msvc-1

rustbot · 2025-02-23T04:44:05Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Kobzol (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

rustbot · 2025-02-23T04:44:08Z

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

jyn514 · 2025-02-23T04:49:03Z

src/bootstrap/src/core/sanity.rs

+
+fn is_gcc_compiler(path: PathBuf, build: &Build) -> bool {
+    let cc_output = command(&path).arg("--version").run_capture_stdout(build).stdout();
+    cc_output.contains("GCC")


i don't think this does what you want. on my machine, cc --version prints "cc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0", even though it's really gcc.

i am not sure how to check this ... maybe the rust equivalent of realpath $(which cc) | rg gcc? or we could check cc.is_like_clang(): https://docs.rs/cc/latest/cc/struct.Tool.html#method.is_like_clang

I'll try to find a gcc-specific flag to run. If all else fails, we can try compiling a C program and check if some headers are defined.

When going down this path I might recommend something like clang -E -dM -x c /dev/null | rg -i clang which will print #define __clang__ 1. That's a test for clang rather than a test for "not gcc" but at this time only clang has support for wasm, so it might work as a detection mechanism nonetheless?

Although IMO rejecting compilers like this is a bit of a tricky business that's bound to have false positives and negatives so personally I'd probably just skip this part and rely on the test you added instead.

tgross35 · 2025-02-24T04:20:53Z

tests/run-make/wasm-builtins-no-linker-warning/rmake.rs

This can just be a UI test (tests/ui), which are cheaper to run than rmake (one thing to compile rather than 2-3). You'll just need the directives //@ only-wasm32 and //@ compile-flags: -Dlinker-messages.

Please make sure tests have a doc comment describing what they are testing as well.

Got it. I'll rewrite this next week - is there anything else?

Just to be clear - this should be a //@ build-pass test without an expected output, right?

That sounds correct 👍

Kobzol

Detecting GCC would be also useful for other things, but it sounds a bit tricky. Left one nit, otherwise the bootstrap changes (modulo GCC detection, which isn't working yet) look fine. I don't know about WASM though, so someone else should look at that.

Kobzol · 2025-02-25T09:33:25Z

src/bootstrap/src/core/sanity.rs

@@ -314,6 +314,17 @@ than building it.
            .entry(*target)
            .or_insert_with(|| Target::from_triple(&target.triple));

+        // compiler-rt c fallbacks for wasm cannot be built with gcc
+        if (target.triple == "wasm32-unknown-unknown" || target.triple == "wasm32v1-none") // bare metal targets without wasi sdk


Could you please add a method to TargetSelection named e.g. is_bare_wasm, or is_wasm_without_wasi or something like that that describes this family of targets?

If going down this compiler-detection route I'd recommend enabling this for all wasm targets, not just the unknown/none targets. The WASI targets have some auto-configuration but the same principle applies here where if gcc is used then that's a bug for WASI as well.

If going down this compiler-detection route I'd recommend enabling this for all wasm targets, not just the unknown/none targets.

Would this entail some sort of check to see if the wasi sdk is using gcc?

In theory (unless I'm misremembering something) nothing more than what's already here other than updating this conditional. The detected compiler for WASI targets should make its way to here and always be clang (wasi-sdk doesn't have gcc and won't for the forseeable future)

jieyouxu · 2025-02-25T11:49:57Z

cc @alexcrichton (target maintainer for wasm32-unknown-unknown) in case you have any concerns/feedback

tgross35 · 2025-02-25T18:17:37Z

Detecting GCC would be also useful for other things, but it sounds a bit tricky. Left one nit, otherwise the bootstrap changes (modulo GCC detection, which isn't working yet) look fine. I don't know about WASM though, so someone else should look at that.

cc exposes this functionality, maybe we could wrap that rather than recreating it here?

jieyouxu · 2025-02-25T18:43:51Z

cc exposes this functionality, maybe we could wrap that rather than recreating it here?

Do note that bootstrap cc unfortunately has to be pinned to a quite an old version, FWIW.

JayAndJef · 2025-02-25T18:58:36Z

Hey yall, just wanted everyone to be aware I'm in a power outage so I probably can't work on this today.

tgross35 · 2025-02-25T19:06:46Z

Hey yall, just wanted everyone to be aware I'm in a power outage so I probably can't work on this today.

There is never any requirement to work on a PR every day, thanks for being active but take your time :)

JayAndJef · 2025-02-27T18:26:01Z

There is never any requirement to work on a PR every day, thanks for being active but take your time :)

Thanks, I'll probably finish the changes on Sunday or so then

JayAndJef · 2025-03-03T00:46:26Z

I've changed the test to a UI test, and changed the compiler detection. I'm not sure about the consensus on that - are we still going use it? The commits for that can be scrapped if so

Kobzol · 2025-03-03T07:56:28Z

The UI and runtime tests sadly won't help that much for this issue, because it's possible that we might use Clang on test builders, but GCC on dist builders. But the bootstrap sanity check should hopefully do the trick.

alexcrichton · 2025-03-03T15:47:08Z

For wasm specifically targets I think that Clang should be used everywhere because gcc doesn't have support for wasm. If any builder is using gcc for wasm that's definitely a bug that needs fixing (which the tests might help detect?)

Kobzol · 2025-03-03T15:57:33Z

Well, the dist builders only build stuff, it's not actually tested on most dist builders, which is why #132802 was a thing. So the UI/run-make tests are nice to have, but wouldn't help for this situation. But with the bootstrap sanity test I think it's fine.

You can r=me, unless you want to do more changes.

alexcrichton · 2025-03-03T16:00:31Z

Oh, right, yes! (sorry your original comment is clearer now that I've woken up more...)

JayAndJef · 2025-03-03T16:02:54Z

Great, so the sanity test is going to be kept?

Kobzol · 2025-03-03T16:21:56Z

Yeah, let's keep the tests. Let's see if CI works.

@bors try

fix for issue 132802: x86 code in `wasm32-unknown-unknown` binaries This is a direct fix for issue [132802](rust-lang#132802). Followed the outline as follows: > * give a hard error in bootstrap when using gcc to compile for wasm > * change our CI to use clang instead of gcc > * add a test that compiling a sample program for wasm32-unknown doesn't give any linker warnings The `test-various` ci job was also changed. try-job: test-various try-job: dist-various-2

JayAndJef · 2025-07-01T21:13:20Z

I'm having some trouble reproducing the error, and the original logs are deleted. Perhaps we could let bors try it again so I could have a better idea of what's happening?

Kobzol · 2025-07-01T21:15:43Z

@bors2 try

rust-bors · 2025-07-01T21:15:48Z

⌛ Trying commit 8b1dbac with merge 8d45e10…

To cancel the try build, run the command @bors2 try cancel.

Fix host code appearing in Wasm binaries    This is a direct fix for issue [132802](#132802). Followed the outline as follows: > * give a hard error in bootstrap when using gcc to compile for wasm > * change our CI to use clang instead of gcc > * add a test that compiling a sample program for wasm32-unknown doesn't give any linker warnings The `test-various` ci job was also changed. try-job: test-various try-job: dist-various-1 try-job: dist-various-2 try-job: x86_64-msvc-1

tgross35 · 2025-07-01T21:22:08Z

I'm having some trouble reproducing the error, and the original logs are deleted. Perhaps we could let bors try it again so I could have a better idea of what's happening?

For future reference, rust-log-analyzer saves a snip of the error in a comment that doesn't go away #137457 (comment). The problem here was that it's hitting the panic message that's added in this PR, meaning it's not trying to build with Clang, so you'll probably need what I mentioned here #137457 (comment).

No harm in getting fresh results though!

rust-bors · 2025-07-01T21:43:36Z

💔 Test failed

CI ❌

JayAndJef · 2025-07-02T19:11:54Z

I'm having some trouble reproducing the error, and the original logs are deleted. Perhaps we could let bors try it again so I could have a better idea of what's happening?

For future reference, rust-log-analyzer saves a snip of the error in a comment that doesn't go away #137457 (comment). The problem here was that it's hitting the panic message that's added in this PR, meaning it's not trying to build with Clang, so you'll probably need what I mentioned here #137457 (comment).

No harm in getting fresh results though!

It's strange though - the only wasm target in dist-various-1 is the emscripten one, which should use emcc as its c compiler. It's based off of clang, but if you check a few lines above the error in that comment, it says that it failed to run the compiler detection tool with emcc. Perhaps that might be the issue?

tgross35 · 2025-07-02T19:43:06Z

Good point, cc should be using emcc by default. I guess maybe fix the current error first, then we can rerun tests? Now that it prints cc_tool.path().display(), we should get a bit more info.

JayAndJef · 2025-07-02T21:33:16Z

test-various works when ran locally, and it seems like the error was that the gnu mirror kept timing out. Maybe it was a one-off network issue when running CI?

#13 329.5 failed: Connection timed out.
#13 329.5 Connecting to gitweb.git.savannah.gnu.org (gitweb.git.savannah.gnu.org)|135.148.138.35|:443... failed: Connection timed out.
#13 329.5 Connecting to gitweb.git.savannah.gnu.org (gitweb.git.savannah.gnu.org)|38.102.85.135|:443... connected.
--- ERROR ---
#13 329.5 HTTP request sent, awaiting response... 502 Bad Gateway
#13 329.5 2025-07-01 21:42:40 ERROR 502: Bad Gateway.
#13 329.5
#13 329.5 make: *** [Makefile:79: sources/config.sub] Error 8
--- ERROR ---
#13 ERROR: process "/bin/sh -c bash musl-toolchain.sh x86_64 && rm -rf build" did not complete successfully: exit code: 1

tgross35 · 2025-07-02T22:16:38Z

@bors2 try

rust-bors · 2025-07-02T22:16:42Z

⌛ Trying commit 8b1dbac with merge 71637f3…

To cancel the try build, run the command @bors2 try cancel.

Fix host code appearing in Wasm binaries    This is a direct fix for issue [132802](#132802). Followed the outline as follows: > * give a hard error in bootstrap when using gcc to compile for wasm > * change our CI to use clang instead of gcc > * add a test that compiling a sample program for wasm32-unknown doesn't give any linker warnings The `test-various` ci job was also changed. try-job: test-various try-job: dist-various-1 try-job: dist-various-2 try-job: x86_64-msvc-1

rust-bors · 2025-07-02T22:27:38Z

💔 Test failed

CI ❌

JayAndJef · 2025-07-03T06:51:22Z

Will look into soon

tgross35 · 2025-07-03T08:04:16Z

For lack of a better idea about that seemingly spurious CI failure, let's just see where the other jobs get

@bors2 try jobs=dist-various*

rust-bors · 2025-07-03T08:04:21Z

⌛ Trying commit 8b1dbac with merge c25f9bd…

To cancel the try build, run the command @bors2 try cancel.

Fix host code appearing in Wasm binaries    This is a direct fix for issue [132802](#132802). Followed the outline as follows: > * give a hard error in bootstrap when using gcc to compile for wasm > * change our CI to use clang instead of gcc > * add a test that compiling a sample program for wasm32-unknown doesn't give any linker warnings The `test-various` ci job was also changed. try-job: test-various try-job: dist-various-1 try-job: dist-various-2 try-job: x86_64-msvc-1 try-job: dist-various*

tgross35 · 2025-07-03T08:13:22Z

Seems like the change to the dockerfile just busts cache and we're hitting a spurious download. Discussion at #t-infra > test-various

rust-bors · 2025-07-03T09:15:22Z

💔 Test failed

CI ❌

tgross35 · 2025-07-03T09:23:05Z

That's more like it!

2025-07-03T09:14:40.5579568Z thread 'main' panicked at src/bootstrap/src/core/sanity.rs:336:17:
2025-07-03T09:14:40.5581953Z Clang is required to build C code for Wasm targets, got `emcc` instead
2025-07-03T09:14:40.5582721Z this is because compiler-builtins is configured to build C source. Either ensure Clang is used, or adjust this configuration.
2025-07-03T09:14:40.5583366Z note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I guess you might need to check .path() for emcc if .is_like_clang() isn't true.

JayAndJef · 2025-07-03T22:18:33Z

makes me wonder if there's a better way to detect emcc specifically, or group it into clang

tgross35 · 2025-07-03T22:43:32Z

That's probably worth an issue on the cc repo if you're up for writing one up.

@bors2 try

rust-bors · 2025-07-03T22:43:35Z

⌛ Trying commit dee1b19 with merge a2d8ab0…

To cancel the try build, run the command @bors2 try cancel.

Fix host code appearing in Wasm binaries    This is a direct fix for issue [132802](#132802). Followed the outline as follows: > * give a hard error in bootstrap when using gcc to compile for wasm > * change our CI to use clang instead of gcc > * add a test that compiling a sample program for wasm32-unknown doesn't give any linker warnings The `test-various` ci job was also changed. try-job: test-various try-job: dist-various-1 try-job: dist-various-2 try-job: x86_64-msvc-1

rust-bors · 2025-07-04T01:14:17Z

☀️ Try build successful (CI)
Build commit: a2d8ab0 (a2d8ab005c442519d658a5e5c0a932f20c44071b, parent: 48aee7e383503c234cce4206dee9f19f57edb617)

tgross35 · 2025-07-04T01:30:19Z

src/ci/docker/host-x86_64/test-various/Dockerfile

-ENV WASM_TARGETS=wasm32-wasip1
-ENV WASM_SCRIPT python3 /checkout/x.py --stage 2 test --host='' --target $WASM_TARGETS \
+ENV WASM_WASIP_TARGET=wasm32-wasip1 
+ENV WASM_WASIP_SCRIPT python3 /checkout/x.py --stage 2 test --host='' --target $WASM_WASIP_TARGET \


What exactly is going on with the changes in this file btw? It looks like a variable rename, which should be inconsequential, plus installing gcc-multilib?

Actually, why is this wasi build succeeding? We don't set CC_wasm32_wasip1 to Clang anywhere, which would mean GCC is the default.

I think it would be good to set the CC_* variables even if it works for some reason, to make it explicit that we want clang.

rustbot assigned Kobzol Feb 23, 2025

jyn514 reviewed Feb 23, 2025

View reviewed changes

tgross35 reviewed Feb 24, 2025

View reviewed changes

Kobzol reviewed Feb 25, 2025

View reviewed changes

jieyouxu added the O-wasm Target: WASM (WebAssembly), http://webassembly.org/ label Feb 27, 2025

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

fix: remove unneeded(?) install script

8b1dbac

JayAndJef marked this pull request as ready for review July 1, 2025 21:13

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 1, 2025

fix: allow emcc

dee1b19

tgross35 reviewed Jul 4, 2025

View reviewed changes

Fix host code appearing in Wasm binaries #137457

Are you sure you want to change the base?

Fix host code appearing in Wasm binaries #137457

Conversation

JayAndJef commented Feb 23, 2025 • edited by Kobzol Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Feb 23, 2025

Uh oh!

rustbot commented Feb 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kobzol left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu commented Feb 25, 2025

Uh oh!

tgross35 commented Feb 25, 2025

Uh oh!

jieyouxu commented Feb 25, 2025

Uh oh!

JayAndJef commented Feb 25, 2025

Uh oh!

tgross35 commented Feb 25, 2025

Uh oh!

JayAndJef commented Feb 27, 2025

Uh oh!

JayAndJef commented Mar 3, 2025

Uh oh!

Kobzol commented Mar 3, 2025

Uh oh!

alexcrichton commented Mar 3, 2025

Uh oh!

Kobzol commented Mar 3, 2025

Uh oh!

alexcrichton commented Mar 3, 2025

Uh oh!

JayAndJef commented Mar 3, 2025

Uh oh!

Kobzol commented Mar 3, 2025

Uh oh!

This comment was marked as outdated.

This comment has been minimized.

This comment was marked as outdated.

JayAndJef commented Jul 1, 2025

Uh oh!

Kobzol commented Jul 1, 2025

Uh oh!

rust-bors bot commented Jul 1, 2025

Uh oh!

tgross35 commented Jul 1, 2025

Uh oh!

rust-bors bot commented Jul 1, 2025

Uh oh!

JayAndJef commented Jul 2, 2025

Uh oh!

tgross35 commented Jul 2, 2025

Uh oh!

JayAndJef commented Jul 2, 2025

JayAndJef commented Feb 23, 2025 •

edited by Kobzol

Loading

tgross35 commented Jul 3, 2025 •

edited

Loading

tgross35 Jul 4, 2025 •

edited

Loading