Support -Cpanic=unwind on WASI targets#156061
Support -Cpanic=unwind on WASI targets#156061alexcrichton wants to merge 1 commit intorust-lang:mainfrom
-Cpanic=unwind on WASI targets#156061Conversation
|
These commits modify compiler targets. |
|
r? @marcoieni rustbot has assigned @marcoieni. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
a960b56 to
3661ec1
Compare
| // actually turned on, which it's not by default on this target. For | ||
| // `-Zbuild-std` builds, however, this affects when rebuilding libstd | ||
| // with unwinding. | ||
| llvm_args: cvs!["-wasm-use-legacy-eh=false"], |
There was a problem hiding this comment.
Isn't
rust/compiler/rustc_codegen_ssa/src/base.rs
Lines 369 to 375 in 54f67d2
Edit: Might be confused with wasm vs js exception handling.
There was a problem hiding this comment.
I believe, yeah, that's related to the Emscripten-specific scheme of exceptions pre-wasm-exceptions of using JS instead. Effectively there's 3 modes of exceptions on Wasm:
- Historical JS-based support for Emscripten. That's used when
wants_wasm_ehisfalseand is only applicable to historical versions of the Emscripten target (or-Zemscripten-wasm-eh=false) - Historical wasm support. This uses the now-legacy exception-handling proposal that shipped in browsers but was never standardized. This uses wasm instructions and has
wants_wasm_ehistrue. LLVM by default emits this today. - Modern wasm support. This uses the now-standard exception-handling proposal as the successor to the "legacy exceptions" of above. This is
wants_wasm_ehistrueplus an option to LLVM to use the new instructions.
For (1) the LLVM IR is different, and for (2) and (3) they're the same IR. I don't know much about (1) myself. For WASI this PR only exposes (3)
|
The changes in |
|
@bjorn3 would you be up for reviewing the non-infra-related bits? |
| && let Ok(mut path) = path.into_os_string().into_string() | ||
| { | ||
| path.push_str(" run -C cache=n --dir ."); | ||
| path.push_str(" run -Wexceptions -C cache=n --dir ."); |
There was a problem hiding this comment.
There are no tests added that use exceptions, right?
There was a problem hiding this comment.
Almost yeah, but there's one test that does extern crate panic_unwind; which pulls in libunwind which pulls in a throw instruction which then needs this to validate
There was a problem hiding this comment.
I'm surprised that even works without //@ needs-unwind to only run the test for panic=unwind. I thought rustc refused to link a panic runtime that doesn't match the -Cpanic=..., but I guess that check is only done when the panic runtime dependency is injected by rustc, not when it is explicitly done by the user.
This commit is some minor updates/restructuring in a few locations with
the end result being supporting `-Cpanic=unwind` on WASI targets. This
continues to be off-by-default insofar as WASI targets default to
`-Cpanic=abort`, meaning that actually using anything in this commit
requires `-Zbuild-std`. Specifically the changes made here are:
* The self-contained sysroot for WASI targets now contains a copy of
`libunwind.a` from wasi-sdk, first shipped with wasi-sdk-33 (also
updated here).
* The `unwind` crate here in this repository uses the `libunwind` module
instead of the custom bare-metal wasm implementation of exceptions.
This means that Rust uses the `_Unwind_*` symbols which allows it to
interoperate with C/C++/etc.
* Wasm targets are all updated to pass the LLVM argument
`-wasm-use-legacy-eh=false` to differ from LLVM's/clang's default of
using the legacy exception handling proposal for WebAssembly. This has
no effect by default because `panic=abort` is used on most targets.
Emscripten is exempted from this as the Emscripten target is
explicitly intended to follow LLVM's/clang's defaults.
* There's a single test in the test suite that links to the
`panic_unwind` crate which ended up requiring `-Wexceptions` from
Wasmtime, so the test parts were updated and Wasmtime was updated in
CI, too.
The net result of all of this is that this should not actually affect
any WebAssembly target's default behavior. Optionally, though, WASI
programs can be built with exception handling via:
RUSTFLAGS='-Cpanic=unwind' cargo +nightly run -Z build-std --target wasm32-wasip2
Effectively `-Zbuild-std` and `-Cpanic=unwind` is all that's necessary
to enable this support on wasm targets.
Finally, this ends up closing 154593 as well. The WASI targets are now
defined to use `-lunwind` to implement unwinding. This means that the
in-tree definition of `__cpp_exception` is no longer of concern and the
definition is always sourced externally. If Rust is linked with other
C/C++ code using WASI then these idioms are compatible with wasi-sdk,
for example, to use that as a linker. The main caveat is that when using
an external linker the `-fwasm-exceptions` argument needs to be passed
to `clang` for it to be able to find the `libunwind.a` library to link
against.
Closes 154593
3661ec1 to
13c6e60
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@bors r+ |
This commit is some minor updates/restructuring in a few locations with the end result being supporting
-Cpanic=unwindon WASI targets. This continues to be off-by-default insofar as WASI targets default to-Cpanic=abort, meaning that actually using anything in this commit requires-Zbuild-std. Specifically the changes made here are:libunwind.afrom wasi-sdk, first shipped with wasi-sdk-33 (also updated here).unwindcrate here in this repository uses thelibunwindmodule instead of the custom bare-metal wasm implementation of exceptions. This means that Rust uses the_Unwind_*symbols which allows it to interoperate with C/C++/etc.-wasm-use-legacy-eh=falseto differ from LLVM's/clang's default of using the legacy exception handling proposal for WebAssembly. This has no effect by default becausepanic=abortis used on most targets. Emscripten is exempted from this as the Emscripten target is explicitly intended to follow LLVM's/clang's defaults.panic_unwindcrate which ended up requiring-Wexceptionsfrom Wasmtime, so the test parts were updated and Wasmtime was updated in CI, too.The net result of all of this is that this should not actually affect any WebAssembly target's default behavior. Optionally, though, WASI programs can be built with exception handling via:
Effectively
-Zbuild-stdand-Cpanic=unwindis all that's necessary to enable this support on wasm targets.Finally, this ends up closing #154593 as well. The WASI targets are now defined to use
-lunwindto implement unwinding. This means that the in-tree definition of__cpp_exceptionis no longer of concern and the definition is always sourced externally. If Rust is linked with other C/C++ code using WASI then these idioms are compatible with wasi-sdk, for example, to use that as a linker. The main caveat is that when using an external linker the-fwasm-exceptionsargument needs to be passed toclangfor it to be able to find thelibunwind.alibrary to link against.Closes #154593