Skip to content

tests: check wasm compiler_builtins object architecture#156230

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
Vastargazing:tests/wasm-compiler-builtins-object-arch
Jun 30, 2026
Merged

tests: check wasm compiler_builtins object architecture#156230
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
Vastargazing:tests/wasm-compiler-builtins-object-arch

Conversation

@Vastargazing

@Vastargazing Vastargazing commented May 6, 2026

Copy link
Copy Markdown
Contributor

View all comments

See #132802

This adds a run-make test for the wasm sysroot regression fixed in #137457

The test checks that the .o members in the prebuilt
libcompiler_builtins rlib for wasm32-wasip1 are wasm objects rather than
host ELF objects. Before that fix, bootstrap could use the host C compiler for
compiler-rt fallbacks on wasm targets and end up embedding host objects into
the wasm sysroot.

I used wasm32-wasip1 because that's the wasm target covered by the existing
tests/run-make CI setup, and the test asserts that it actually saw .o
members in the archive.

Closes: #132802

r? @tgross35

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 6, 2026
@rustbot

rustbot commented May 6, 2026

Copy link
Copy Markdown
Collaborator

tgross35 is currently at their maximum review capacity.
They may take a while to respond.

@rust-log-analyzer

This comment has been minimized.

@Vastargazing Vastargazing force-pushed the tests/wasm-compiler-builtins-object-arch branch from 211904e to b25f8a1 Compare May 6, 2026 11:17

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for picking this up!

View changes since this review

Comment thread tests/run-make/wasm-compiler-builtins-object-arch/rmake.rs
Comment thread tests/run-make/wasm-compiler-builtins-object-arch/rmake.rs Outdated
Comment thread tests/run-make/wasm-compiler-builtins-object-arch/rmake.rs Outdated
Comment thread tests/run-make/wasm-compiler-builtins-object-arch/rmake.rs
Comment thread tests/run-make/wasm-compiler-builtins-object-arch/rmake.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 18, 2026
@rustbot

rustbot commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@Vastargazing Vastargazing force-pushed the tests/wasm-compiler-builtins-object-arch branch from b25f8a1 to 962838f Compare May 21, 2026 04:42
@Vastargazing

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback! I've just force-pushed the fixes to the branch.

Addresssed everything we discussed: added the module-level docs with links, and broadened the directive to only-wasm to cover other targets. I also flipped the filter to skip only .rmeta so unexpected files will actually trigger a failure now.

For the wasm check, i dropped the raw magic bytes and went with object::File::parse + matching on Wasm32/Wasm64. Also cleaned up the misleading assert message about compiler-builtins-c and dropped the trailing eprintln!.

Let me know what you think!

@rust-log-analyzer

This comment has been minimized.

@Vastargazing Vastargazing force-pushed the tests/wasm-compiler-builtins-object-arch branch from 962838f to 50ad258 Compare May 21, 2026 18:08
@tgross35

Copy link
Copy Markdown
Contributor

@bors try jobs=test-various*

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 11, 2026
…ect-arch, r=<try>

tests: check wasm compiler_builtins object architecture


try-job: test-various*
@tgross35

Copy link
Copy Markdown
Contributor

@bors try jobs=test-various*

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 11, 2026
…ect-arch, r=<try>

tests: check wasm compiler_builtins object architecture


try-job: test-various*

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates, r=me with the passing try job

(Fyi you may have missed the above rustbot comment about how to update the PR status, this was still labeled waiting-on-author)

View changes since this review

@rust-bors

rust-bors Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

💔 Test for 30c565b failed: CI. Failed job:

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

failed to parse member lib.rmeta-link in compiler_builtins rlib: Unknown file magic

Looks like another extension needs to be skipped

View changes since this review

@rust-log-analyzer

This comment has been minimized.

@Vastargazing Vastargazing force-pushed the tests/wasm-compiler-builtins-object-arch branch from 50ad258 to 266e90f Compare June 11, 2026 06:19
@rustbot

rustbot commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Warning ⚠️

@Vastargazing

Copy link
Copy Markdown
Contributor Author

Pushed a fix - lib.rmeta-link (wasm-specific link-time metadata member) was slipping past the .rmeta skip filter and tripping object::File::parse on the try build. Added it to the skip list.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 11, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 12, 2026
@tgross35

Copy link
Copy Markdown
Contributor

I don't think that's a better solution since if there's a problem reading the entry (e.g. if object is compiled without wasm support) then the test quietly does nothing. So I'd prefer sticking with the ignored list, or at least the original where everything ending with .o is checked.

But that is an interesting point - any idea why .rcgu.o isn't parsing? They should be valid object files.

(If you can't run locally then to get a more useful output message in CI, you could keep the current version but push the name to a vec if it fails to parse. Then after the loop, print the whole vec and fail if any entries don't match .rmeta and other expected parse failures. That way we get the whole list of what didn't parse at once).

@Vastargazing Vastargazing force-pushed the tests/wasm-compiler-builtins-object-arch branch from d5d20bf to 91d8c56 Compare June 13, 2026 08:41
@Vastargazing

Vastargazing commented Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

Found the issue

run-make-support builds the object crate without the wasm feature. So FileKind::Wasm isn't compiled in, it misses the \0asm magic, and throws "Unknown file magic". The objects are completely fine, our parser just can't see them.

Went back to the original .o + \0asm magic-byte check - dependency-free, and a host ELF/Mach-O can't start with \0asm anyway so it still catches the regression. Dropped the silent skip.

(If you'd rather keep the object::File + architecture() check, that just needs the wasm feature enabled in run-make-support - wasmparser is already in the tree. Happy to switch.)

@rustbot ready

@tgross35

Copy link
Copy Markdown
Contributor

(If you'd rather keep the object::File + architecture() check, that just needs the wasm feature enabled in run-make-support - wasmparser is already in the tree. Happy to switch.)

Yes please - we'll probably want it at some other point anyway :) Sorry this has had so much back and forth

Add a run-make test that checks the `.o` members in the prebuilt
`libcompiler_builtins` archive for `wasm32-wasip1` are wasm objects.

This guards against building wasm compiler-rt fallbacks with the host C
toolchain.
@Vastargazing Vastargazing force-pushed the tests/wasm-compiler-builtins-object-arch branch from 91d8c56 to e101c8d Compare June 28, 2026 16:14
@rustbot

rustbot commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

The run-make-support library was changed

cc @jieyouxu

@Vastargazing

Copy link
Copy Markdown
Contributor Author

Thanks for the patience (: ! Enabled the wasm feature on object in run-make-support and put the object::File + architecture() check back. .rcgu.o parse as Wasm32 now, and .rmeta / .rmeta-link are skipped. Cargo.lock didn't need any change since wasmparser was already pinned in the tree.

@rustbot ready

@tgross35

Copy link
Copy Markdown
Contributor

@bors try jobs=test-various

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 28, 2026
…ect-arch, r=<try>

tests: check wasm compiler_builtins object architecture


try-job: test-various
@rust-bors

rust-bors Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 4356fe3 (4356fe380756a7d9b7cd095f42f04cca8a0a93f6)
Base parent: 2574810 (2574810b228d53518cc2a13f2ee473195932a999)

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you for sticking with this!

@bors r+

View changes since this review

@rust-bors

rust-bors Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

📌 Commit e101c8d has been approved by tgross35

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 1. This pull request will be tested once the tree is reopened.

@rust-bors rust-bors Bot 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 Jun 29, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jun 29, 2026
…uiltins-object-arch, r=tgross35

tests: check wasm compiler_builtins object architecture

See rust-lang#132802

This adds a run-make test for the wasm sysroot regression fixed in rust-lang#137457

The test checks that the `.o` members in the prebuilt
`libcompiler_builtins` rlib for `wasm32-wasip1` are wasm objects rather than
host ELF objects. Before that fix, bootstrap could use the host C compiler for
compiler-rt fallbacks on wasm targets and end up embedding host objects into
the wasm sysroot.

I used `wasm32-wasip1` because that's the wasm target covered by the existing
`tests/run-make` CI setup, and the test asserts that it actually saw `.o`
members in the archive.

Closes: rust-lang#132802

r? @tgross35
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jun 29, 2026
…uiltins-object-arch, r=tgross35

tests: check wasm compiler_builtins object architecture

See rust-lang#132802

This adds a run-make test for the wasm sysroot regression fixed in rust-lang#137457

The test checks that the `.o` members in the prebuilt
`libcompiler_builtins` rlib for `wasm32-wasip1` are wasm objects rather than
host ELF objects. Before that fix, bootstrap could use the host C compiler for
compiler-rt fallbacks on wasm targets and end up embedding host objects into
the wasm sysroot.

I used `wasm32-wasip1` because that's the wasm target covered by the existing
`tests/run-make` CI setup, and the test asserts that it actually saw `.o`
members in the archive.

Closes: rust-lang#132802

r? @tgross35
rust-bors Bot pushed a commit that referenced this pull request Jun 29, 2026
…uwer

Rollup of 16 pull requests

Successful merges:

 - #155722 (Introduce aarch64-unknown-linux-pauthtest target)
 - #156230 (tests: check wasm compiler_builtins object architecture)
 - #158073 (bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS)
 - #158169 (Fix debuginfo compression in bootstrap)
 - #158256 (Avoid parser panics bubbling out to proc macros)
 - #158375 (Support `DefKind::InlineConst` in `ConstKind::Unevaluated`)
 - #158556 (delegation: store child segment flag in `PathSegment`)
 - #158561 (Avoid building rustdoc for tests without doctests)
 - #158562 (Improve tracing of steps in bootstrap)
 - #157445 (Allow section override when using patchable-function-entries)
 - #158081 (trait-system: Recover deferred closure calls after errors)
 - #158327 (Move attribute and keyword docs from `std` to `core`)
 - #158468 (Include default-stability info in rustdoc JSON.)
 - #158564 (fix `-Z min-recursion-limit` unstable chapter name)
 - #158568 (llvm-wrapper: use accessors for private fields in LLVM 23+)
 - #158582 (Comment on needed RAM in huge-stacks.rs)
rust-bors Bot pushed a commit that referenced this pull request Jun 30, 2026
…uwer

Rollup of 16 pull requests

Successful merges:

 - #155722 (Introduce aarch64-unknown-linux-pauthtest target)
 - #156230 (tests: check wasm compiler_builtins object architecture)
 - #158073 (bootstrap: fix panic when repo path contains spaces by switching to CARGO_ENCODED_RUSTFLAGS)
 - #158169 (Fix debuginfo compression in bootstrap)
 - #158256 (Avoid parser panics bubbling out to proc macros)
 - #158375 (Support `DefKind::InlineConst` in `ConstKind::Unevaluated`)
 - #158556 (delegation: store child segment flag in `PathSegment`)
 - #158561 (Avoid building rustdoc for tests without doctests)
 - #158562 (Improve tracing of steps in bootstrap)
 - #157445 (Allow section override when using patchable-function-entries)
 - #158081 (trait-system: Recover deferred closure calls after errors)
 - #158327 (Move attribute and keyword docs from `std` to `core`)
 - #158468 (Include default-stability info in rustdoc JSON.)
 - #158564 (fix `-Z min-recursion-limit` unstable chapter name)
 - #158568 (llvm-wrapper: use accessors for private fields in LLVM 23+)
 - #158582 (Comment on needed RAM in huge-stacks.rs)
rust-bors Bot pushed a commit that referenced this pull request Jun 30, 2026
…uwer

Rollup of 11 pull requests

Successful merges:

 - #155722 (Introduce aarch64-unknown-linux-pauthtest target)
 - #156230 (tests: check wasm compiler_builtins object architecture)
 - #156295 (Pass the whole `GenericArgs` to `Interner::for_each_relevant_impl()`)
 - #158375 (Support `DefKind::InlineConst` in `ConstKind::Unevaluated`)
 - #158556 (delegation: store child segment flag in `PathSegment`)
 - #158081 (trait-system: Recover deferred closure calls after errors)
 - #158468 (Include default-stability info in rustdoc JSON.)
 - #158543 (Note usage of documentation hard links in `core::io`)
 - #158564 (fix `-Z min-recursion-limit` unstable chapter name)
 - #158568 (llvm-wrapper: use accessors for private fields in LLVM 23+)
 - #158582 (Comment on needed RAM in huge-stacks.rs)
rust-bors Bot pushed a commit that referenced this pull request Jun 30, 2026
…uwer

Rollup of 11 pull requests

Successful merges:

 - #155722 (Introduce aarch64-unknown-linux-pauthtest target)
 - #156230 (tests: check wasm compiler_builtins object architecture)
 - #156295 (Pass the whole `GenericArgs` to `Interner::for_each_relevant_impl()`)
 - #158375 (Support `DefKind::InlineConst` in `ConstKind::Unevaluated`)
 - #158556 (delegation: store child segment flag in `PathSegment`)
 - #158081 (trait-system: Recover deferred closure calls after errors)
 - #158468 (Include default-stability info in rustdoc JSON.)
 - #158543 (Note usage of documentation hard links in `core::io`)
 - #158564 (fix `-Z min-recursion-limit` unstable chapter name)
 - #158568 (llvm-wrapper: use accessors for private fields in LLVM 23+)
 - #158582 (Comment on needed RAM in huge-stacks.rs)
rust-bors Bot pushed a commit that referenced this pull request Jun 30, 2026
…uwer

Rollup of 11 pull requests

Successful merges:

 - #155722 (Introduce aarch64-unknown-linux-pauthtest target)
 - #156230 (tests: check wasm compiler_builtins object architecture)
 - #156295 (Pass the whole `GenericArgs` to `Interner::for_each_relevant_impl()`)
 - #158375 (Support `DefKind::InlineConst` in `ConstKind::Unevaluated`)
 - #158556 (delegation: store child segment flag in `PathSegment`)
 - #158081 (trait-system: Recover deferred closure calls after errors)
 - #158468 (Include default-stability info in rustdoc JSON.)
 - #158543 (Note usage of documentation hard links in `core::io`)
 - #158564 (fix `-Z min-recursion-limit` unstable chapter name)
 - #158568 (llvm-wrapper: use accessors for private fields in LLVM 23+)
 - #158582 (Comment on needed RAM in huge-stacks.rs)
@rust-bors rust-bors Bot merged commit 5aa6dfb into rust-lang:main Jun 30, 2026
14 checks passed
@rustbot rustbot added this to the 1.98.0 milestone Jun 30, 2026
rust-timer added a commit that referenced this pull request Jun 30, 2026
Rollup merge of #156230 - Vastargazing:tests/wasm-compiler-builtins-object-arch, r=tgross35

tests: check wasm compiler_builtins object architecture

See #132802

This adds a run-make test for the wasm sysroot regression fixed in #137457

The test checks that the `.o` members in the prebuilt
`libcompiler_builtins` rlib for `wasm32-wasip1` are wasm objects rather than
host ELF objects. Before that fix, bootstrap could use the host C compiler for
compiler-rt fallbacks on wasm targets and end up embedding host objects into
the wasm sysroot.

I used `wasm32-wasip1` because that's the wasm target covered by the existing
`tests/run-make` CI setup, and the test asserts that it actually saw `.o`
members in the archive.

Closes: #132802

r? @tgross35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

4 participants