Skip to content

Fix emscripten-wasm-eh with unwind=abort #135450

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Jan 13, 2025

If we build the standard library with wasm-eh then we need to link with -fwasm-exceptions even if we compile with panic=abort.

Without this change, linking a panic=abort crate fails with: undefined symbol: __cpp_exception.

Followup to #131830.

r? workingjubilee

@rustbot
Copy link
Collaborator

rustbot commented Jan 13, 2025

Could not assign reviewer from: workingjubilee.
User(s) workingjubilee are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

@rustbot
Copy link
Collaborator

rustbot commented Jan 13, 2025

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added 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 Jan 13, 2025
hoodmane added a commit to hoodmane/pyodide-build that referenced this pull request Jan 13, 2025
For now, we need to use a custom rust toolchain for this. Apparently all
alternatives to this don't work. -Zbuild-std doesn't work with panic=abort
(rust-lang/cargo#7359)
and my attempts to use a custom sysroot with either
https://github.com/RalfJung/rustc-build-sysroot/ or
https://github.com/DianaNites/cargo-sysroot/  seem to hit the same problem
as with `-Zbuild-std`. Thus, I think the only reasonable way to go is to
build the sysroot from the rust source directory. Perhaps we can eventually
approach this by copying the `lib/rustlib/wasm32-unknown-emscripten/lib/`
folder out of the build of the rust compiler on top of a nightly install of
the compiler. For now, there is the additional problem that we need this
patch to fix unwind=abort:
rust-lang/rust#135450

I got my copy of the rust compiler by checking out this commit:
hoodmane/rust@052ba16
two commits ahead of the rust main branch and running:
```
./x build --stage 2 --target x86_64-unknown-linux-gnu,wasm32-unknown-emscripten
```
@workingjubilee
Copy link
Member

@hoodmane Can you comment that this being unconditional is needed for -Zbuild-std to work?

@hoodmane
Copy link
Contributor Author

It's not needed for -Zbuild-std to work, -Zbuild-std apparently never has worked.

@hoodmane
Copy link
Contributor Author

The problem with -Zbuild-std also has nothing to do with Emscripten, you can't use -Zbuild-std with panic=abort at all:
rust-lang/cargo#15058

@workingjubilee
Copy link
Member

... 🫠 right, so, can the diff of this PR include a comment on why we do this

@hoodmane
Copy link
Contributor Author

If we don't, then linking fails a panic=abort binary fails with undefined symbol: __cpp_exception. I will add this to the commit message.

If we build the standard library with wasm-eh then we need to link
with `-fwasm-exceptions` even if we compile with `panic=abort`
Without this change, linking a `panic=abort` crate fails with:
`undefined symbol: __cpp_exception`.

Followup to rust-lang#131830.
@hoodmane
Copy link
Contributor Author

@workingjubilee I added to the commit message:

Without this change, linking a panic=abort crate fails with: undefined symbol: __cpp_exception.

Clear enough?

@workingjubilee
Copy link
Member

cool!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jan 13, 2025

📌 Commit 4d0a838 has been approved by workingjubilee

It is now in the queue for this repository.

@bors 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 Jan 13, 2025
@hoodmane
Copy link
Contributor Author

hoodmane commented Jan 13, 2025

Thanks for reviewing @workingjubilee!

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 14, 2025
Rollup of 10 pull requests

Successful merges:

 - rust-lang#134498 (Fix cycle error only occurring with -Zdump-mir)
 - rust-lang#134977 (Detect `mut arg: &Ty` meant to be `arg: &mut Ty` and provide structured suggestion)
 - rust-lang#135390 (Re-added regression test for rust-lang#122638)
 - rust-lang#135393 (uefi: helpers: Introduce OwnedDevicePath)
 - rust-lang#135440 (rm unnecessary `OpaqueTypeDecl` wrapper)
 - rust-lang#135441 (Make sure to mark `IMPL_TRAIT_REDUNDANT_CAPTURES` as `Allow` in edition 2024)
 - rust-lang#135444 (Update books)
 - rust-lang#135450 (Fix emscripten-wasm-eh with unwind=abort)
 - rust-lang#135452 (bootstrap: fix outdated feature name in comment)
 - rust-lang#135454 (llvm: Allow sized-word rather than ymmword in tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 05ae6bf into rust-lang:master Jan 14, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 14, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 14, 2025
Rollup merge of rust-lang#135450 - hoodmane:wasm-eh-abort-fix, r=workingjubilee

Fix emscripten-wasm-eh with unwind=abort

If we build the standard library with wasm-eh then we need to link with `-fwasm-exceptions` even if we compile with `panic=abort`.

Without this change, linking a `panic=abort` crate fails with: `undefined symbol: __cpp_exception`.

Followup to rust-lang#131830.

r? workingjubilee
@hoodmane hoodmane deleted the wasm-eh-abort-fix branch January 14, 2025 08:09
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 11, 2025
Rollup of 10 pull requests

Successful merges:

 - rust-lang#134498 (Fix cycle error only occurring with -Zdump-mir)
 - rust-lang#134977 (Detect `mut arg: &Ty` meant to be `arg: &mut Ty` and provide structured suggestion)
 - rust-lang#135390 (Re-added regression test for rust-lang#122638)
 - rust-lang#135393 (uefi: helpers: Introduce OwnedDevicePath)
 - rust-lang#135440 (rm unnecessary `OpaqueTypeDecl` wrapper)
 - rust-lang#135441 (Make sure to mark `IMPL_TRAIT_REDUNDANT_CAPTURES` as `Allow` in edition 2024)
 - rust-lang#135444 (Update books)
 - rust-lang#135450 (Fix emscripten-wasm-eh with unwind=abort)
 - rust-lang#135452 (bootstrap: fix outdated feature name in comment)
 - rust-lang#135454 (llvm: Allow sized-word rather than ymmword in tests)

r? `@ghost`
`@rustbot` modify labels: rollup
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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.

5 participants