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-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.

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?

I'm not very sure about the variable changes... maybe I renamed it for clarity?
If I remember correctly installing gcc-multilib was in response to the script failing locally.

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.

wasm32-wasip1 depends on the wasi sdk, which is based on clang. You can see it installed on line 44-46 of the test-various Dockerfile.

JayAndJef · 2025-07-07T20:21:14Z

how do we feel on this?

tgross35 · 2025-07-07T22:07:51Z

Could you rename the vars in src/ci/docker/host-x86_64/test-various/Dockerfile to WASI_TARGET and WASI_SCRIPT? Not WASIP`. There is also a trailing space on line 63 of that file.

After that please squash and it LGTM, unless @Kobzol do you have anything else?

Kobzol · 2025-07-08T07:07:57Z

I haven't followed all the discussion here - do we have to use emscripten and cannot use clang?

JayAndJef · 2025-07-10T04:52:06Z

I haven't followed all the discussion here - do we have to use emscripten and cannot use clang?

I'm pretty sure so, but to be safe maybe someone who works on that target could answer?

Kobzol · 2025-08-22T18:40:18Z

@bors try

Fix host code appearing in Wasm binaries try-job: test-various try-job: dist-various-1 try-job: dist-various-2 try-job: x86_64-msvc-1

rust-bors · 2025-08-22T21:17:08Z

☀️ Try build successful (CI)
Build commit: a3ad74e (a3ad74e9cd798d375d08e75010c3a7c07b99b8ef, parent: d20509c2a0c71b60aa2b51566e4d14920e8a1661)

Kobzol · 2025-08-22T21:36:29Z

Let's try it.

@bors r+

bors · 2025-08-22T21:36:32Z

📌 Commit dee1b19 has been approved by Kobzol

It is now in the queue for this repository.

ognevny · 2025-08-22T22:30:33Z

src/bootstrap/src/core/sanity.rs

+                || build.config.rust_std_features.contains("compiler-builtins-c"))
+        {
+            let cc_tool = build.cc_tool(*target);
+            if !cc_tool.is_like_clang() && !cc_tool.path().ends_with("emcc") {


just to clarify, will this work on Windows if emcc.bat is used?

No idea 🤷 Are you able to check?

only tomorrow...

Rollup of 28 pull requests Successful merges: - #132087 (Fix overly restrictive lifetime in `core::panic::Location::file` return type) - #137396 (Recover `param: Ty = EXPR`) - #137457 (Fix host code appearing in Wasm binaries) - #142185 (Convert moves of references to copies in ReferencePropagation) - #144648 (Implementation: `#[feature(nonpoison_rwlock)]`) - #144897 (print raw lifetime idents with r#) - #145218 ([Debuginfo] improve enum value formatting in LLDB for better readability) - #145380 (Add codegen-llvm regression tests) - #145573 (Add an experimental unsafe(force_target_feature) attribute.) - #145597 (resolve: Remove `ScopeSet::Late`) - #145633 (Fix some typos in LocalKey documentation) - #145641 (On E0277, point at type that doesn't implement bound) - #145669 (rustdoc-search: GUI tests check for `//` in URL) - #145695 (Introduce ProjectionElem::try_map.) - #145710 (Fix the ABI parameter inconsistency issue in debug.rs for LoongArch64) - #145726 (Experiment: Reborrow trait) - #145731 (Make raw pointers work in type-based search) - #145736 (triagebot: Update style team reviewers) - #145738 (Uplift rustc_mir_transform::coverage::counters::union_find to rustc_data_structures.) - #145742 (rustdoc js: Even more typechecking improvments ) - #145743 (doc: fix some typos in comment) - #145745 (tests: Ignore basic-stepping.rs on LoongArch) - #145747 (Refactor lint buffering to avoid requiring a giant enum) - #145751 (fix(lexer): Allow '-' in the frontmatter infostring continue set) - #145761 (Add aarch64_be-unknown-hermit target) - #145762 (convert strings to symbols in attr diagnostics) - #145763 (Ship LLVM tools for the correct target when cross-compiling) - #145765 (Revert suggestions for missing methods in tuples) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #137457 - JayAndJef:issue-132802-fix, r=Kobzol 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-08-23T13:30:49Z

Could you rename the vars in src/ci/docker/host-x86_64/test-various/Dockerfile to WASI_TARGET and WASI_SCRIPT? Not WASIP`. There is also a trailing space on line 63 of that file.

After that please squash and it LGTM, unless @Kobzol do you have anything else?

This all seems to have been missed (not the biggest deals)

Kobzol · 2025-08-23T14:21:45Z

Oops, sorry, I returned to this after a while and I thought it was done already. Do you want to do it as a follow-up @JayAndJef?

JayAndJef · 2025-08-23T17:21:02Z

Sure, would that entail a new issue and pr?

Kobzol · 2025-08-23T19:35:44Z

PR is enough, no need for an issue.

Rollup of 28 pull requests Successful merges: - rust-lang/rust#132087 (Fix overly restrictive lifetime in `core::panic::Location::file` return type) - rust-lang/rust#137396 (Recover `param: Ty = EXPR`) - rust-lang/rust#137457 (Fix host code appearing in Wasm binaries) - rust-lang/rust#142185 (Convert moves of references to copies in ReferencePropagation) - rust-lang/rust#144648 (Implementation: `#[feature(nonpoison_rwlock)]`) - rust-lang/rust#144897 (print raw lifetime idents with r#) - rust-lang/rust#145218 ([Debuginfo] improve enum value formatting in LLDB for better readability) - rust-lang/rust#145380 (Add codegen-llvm regression tests) - rust-lang/rust#145573 (Add an experimental unsafe(force_target_feature) attribute.) - rust-lang/rust#145597 (resolve: Remove `ScopeSet::Late`) - rust-lang/rust#145633 (Fix some typos in LocalKey documentation) - rust-lang/rust#145641 (On E0277, point at type that doesn't implement bound) - rust-lang/rust#145669 (rustdoc-search: GUI tests check for `//` in URL) - rust-lang/rust#145695 (Introduce ProjectionElem::try_map.) - rust-lang/rust#145710 (Fix the ABI parameter inconsistency issue in debug.rs for LoongArch64) - rust-lang/rust#145726 (Experiment: Reborrow trait) - rust-lang/rust#145731 (Make raw pointers work in type-based search) - rust-lang/rust#145736 (triagebot: Update style team reviewers) - rust-lang/rust#145738 (Uplift rustc_mir_transform::coverage::counters::union_find to rustc_data_structures.) - rust-lang/rust#145742 (rustdoc js: Even more typechecking improvments ) - rust-lang/rust#145743 (doc: fix some typos in comment) - rust-lang/rust#145745 (tests: Ignore basic-stepping.rs on LoongArch) - rust-lang/rust#145747 (Refactor lint buffering to avoid requiring a giant enum) - rust-lang/rust#145751 (fix(lexer): Allow '-' in the frontmatter infostring continue set) - rust-lang/rust#145761 (Add aarch64_be-unknown-hermit target) - rust-lang/rust#145762 (convert strings to symbols in attr diagnostics) - rust-lang/rust#145763 (Ship LLVM tools for the correct target when cross-compiling) - rust-lang/rust#145765 (Revert suggestions for missing methods in tuples) r? `@ghost` `@rustbot` modify labels: rollup

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

tgross35 reviewed Jul 4, 2025

View reviewed changes

Dylan-DPC removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 20, 2025

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Aug 22, 2025

Auto merge of #137457 - JayAndJef:issue-132802-fix, r=<try>

a3ad74e

Fix host code appearing in Wasm binaries try-job: test-various try-job: dist-various-1 try-job: dist-various-2 try-job: x86_64-msvc-1

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 22, 2025

ognevny reviewed Aug 22, 2025

View reviewed changes

jhpratt mentioned this pull request Aug 23, 2025

Rollup of 28 pull requests #145773

Merged

bors merged commit 467c89c into rust-lang:master Aug 23, 2025
12 checks passed

rustbot added this to the 1.91.0 milestone Aug 23, 2025

Fix host code appearing in Wasm binaries #137457

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 3, 2025

Uh oh!

tgross35 commented Jul 3, 2025

Uh oh!

rust-bors bot commented Jul 3, 2025

Uh oh!

rust-bors bot commented Jul 4, 2025

Uh oh!

tgross35 Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

JayAndJef commented Feb 23, 2025 •

edited by Kobzol

Loading

tgross35 Jul 4, 2025 •

edited

Loading