Auto merge of #148291 - purplesyringa:move-throw-to-unwind, r=bjorn3

Move wasm `throw` intrinsic back to `unwind` Fixes #148246, less invasive than the previously proposed #148269. Removes the publicly visible unstable intrinsic tracked in #122465 since it's not clear how to export it in a sound manner. r? `@bjorn3` --- rustc assumes that regular `extern "Rust"` functions unwind only if the `unwind` panic runtime is linked. `throw` was annotated as such, but unwound unconditionally. This could cause UB when a crate built with `-C panic=abort` called `throw` from `core` built with `-C panic=unwind`, since no terminator was added to handle the panic arising from calling an allegedly non-unwinding `extern "Rust"` function. rustc was taught to recognize this condition since #144225 and prevented such linkage, but this caused regressions in #148246, since this meant that Emscripten projects could not be built with `-C panic=abort` without recompiling std. The most straightforward solution would be to move `throw` into the `panic_unwind` crate, so that it's only compiled if the panic runtime is guaranteed to be `unwind`, but this is messy due to our architecture. Instead, move it into `unwind::wasm`, which is only compiled for bare-metal targets that default to `panic = "abort"`, rendering the issue moot.

Author: bors

library/stdarch/crates/core_arch/src/wasm32/mod.rs

@@ -173,35 +173,3 @@ pub fn f64_nearest(a: f64) -> f64 {
 pub fn f64_sqrt(a: f64) -> f64 {
  crate::intrinsics::sqrtf64(a)
 }
-
-unsafe extern "C-unwind" {
- #[link_name = "llvm.wasm.throw"]
- fn wasm_throw(tag: i32, ptr: *mut u8) -> !;
-}
-
-/// Generates the [`throw`] instruction from the [exception-handling proposal] for WASM.
-///
-/// This function is unlikely to be stabilized until codegen backends have better support.
-///
-/// [`throw`]: https://webassembly.github.io/exception-handling/core/syntax/instructions.html#syntax-instr-control
-/// [exception-handling proposal]: https://github.com/WebAssembly/exception-handling
-// FIXME: wasmtime does not currently support exception-handling, so cannot execute
-// a wasm module with the throw instruction in it. once it does, we can
-// reenable this attribute.
-// #[cfg_attr(test, assert_instr(throw, TAG = 0, ptr = core::ptr::null_mut()))]
-#[inline]
-#[unstable(feature = "wasm_exception_handling_intrinsics", issue = "122465")]
-// FIXME: Since this instruction unwinds, `core` built with `-C panic=unwind`
-// cannot be linked with `-C panic=abort` programs. But that's not
-// entirely supported anyway, because runtimes without EH support won't
-// be able to handle `try` blocks in `-C panic=unwind` crates either.
-// We ship `-C panic=abort` `core`, so this doesn't affect users
-// directly. Resolving this will likely require patching out both `try`
-// and `throw` instructions, at which point we can look into whitelisting
-// this function in the compiler to allow linking.
-// See https://github.com/rust-lang/rust/issues/118168.
-#[allow(ffi_unwind_calls)]
-pub unsafe fn throw<const TAG: i32>(ptr: *mut u8) -> ! {
- static_assert!(TAG == 0); // LLVM only supports tag 0 == C++ right now.
- wasm_throw(TAG, ptr)
-}

library/unwind/src/lib.rs

@@ -7,7 +7,7 @@
 #![cfg_attr(not(target_env = "msvc"), feature(libc))]
 #![cfg_attr(
  all(target_family = "wasm", any(not(target_os = "emscripten"), emscripten_wasm_eh)),
- feature(simd_wasm64, wasm_exception_handling_intrinsics)
+ feature(link_llvm_intrinsics, simd_wasm64)
 )]
 #![allow(internal_features)]
 #![deny(unsafe_op_in_unsafe_fn)]

library/unwind/src/wasm.rs

@@ -40,22 +40,40 @@ pub unsafe fn _Unwind_DeleteException(exception: *mut _Unwind_Exception) {
 }
 
 pub unsafe fn _Unwind_RaiseException(exception: *mut _Unwind_Exception) -> _Unwind_Reason_Code {
- // The wasm `throw` instruction takes a "tag", which differentiates certain
- // types of exceptions from others. LLVM currently just identifies these
- // via integers, with 0 corresponding to C++ exceptions and 1 to C setjmp()/longjmp().
- // Ideally, we'd be able to choose something unique for Rust, but for now,
- // we pretend to be C++ and implement the Itanium exception-handling ABI.
+ // This implementation is only used for `wasm*-unknown-unknown` targets. Such targets are not
+ // guaranteed to support exceptions, and they default to `-C panic=abort`. Because an unknown
+ // instruction is a load-time error on wasm, instead of a runtime error like on traditional
+ // architectures, we never want to codegen a `throw` instruction unless the user explicitly
+ // enabled exceptions via `-Z build-std` with `-C panic=unwind`.
  cfg_select! {
- // panic=abort is default for wasm targets. Because an unknown instruction is a load-time
- // error on wasm, instead of a runtime error like on traditional architectures, we never
- // want to codegen a `throw` instruction, as that would break users using runtimes that
- // don't yet support exceptions. The only time this first branch would be selected is if
- // the user explicitly opts in to wasm exceptions, via -Zbuild-std with -Cpanic=unwind.
  panic = "unwind" => {
+ // It's important that this intrinsic is defined here rather than in `core`. Since it
+ // unwinds, invoking it from Rust code compiled with `-C panic=unwind` immediately
+ // forces `panic_unwind` as the required panic runtime.
+ //
+ // We ship unwinding `core` on Emscripten, so making this intrinsic part of `core` would
+ // prevent linking precompiled `core` into `-C panic=abort` binaries. Unlike `core`,
+ // this particular module is never precompiled with `-C panic=unwind` because it's only
+ // used for bare-metal targets, so an error can only arise if the user both manually
+ // recompiles `std` with `-C panic=unwind` and manually compiles the binary crate with
+ // `-C panic=abort`, which we don't care to support.
+ //
+ // See https://github.com/rust-lang/rust/issues/148246.
+ unsafe extern "C-unwind" {
+ /// LLVM lowers this intrinsic to the `throw` instruction.
+ #[link_name = "llvm.wasm.throw"]
+ fn wasm_throw(tag: i32, ptr: *mut u8) -> !;
+ }
+
+ // The wasm `throw` instruction takes a "tag", which differentiates certain types of
+ // exceptions from others. LLVM currently just identifies these via integers, with 0
+ // corresponding to C++ exceptions and 1 to C setjmp()/longjmp(). Ideally, we'd be able
+ // to choose something unique for Rust, but for now, we pretend to be C++ and implement
+ // the Itanium exception-handling ABI.
  // corresponds with llvm::WebAssembly::Tag::CPP_EXCEPTION
  // in llvm-project/llvm/include/llvm/CodeGen/WasmEHFuncInfo.h
  const CPP_EXCEPTION_TAG: i32 = 0;
- core::arch::wasm::throw::<CPP_EXCEPTION_TAG>(exception.cast())
+ wasm_throw(CPP_EXCEPTION_TAG, exception.cast())
  }
  _ => {
  let _ = exception;

tests/codegen-llvm/wasm_exceptions.rs

@@ -3,7 +3,7 @@
 //@ [WASMEXN] compile-flags: -C panic=unwind -Z emscripten-wasm-eh
 
 #![crate_type = "lib"]
-#![feature(core_intrinsics, wasm_exception_handling_intrinsics, link_llvm_intrinsics)]
+#![feature(core_intrinsics, link_llvm_intrinsics)]
 
 extern "C-unwind" {
  fn may_panic();

tests/ui/wasm/wasm-link-to-panic-abort-issue-148246.rs

@@ -0,0 +1,7 @@
+//@ only-wasm32
+//@ compile-flags: -C panic=abort
+//@ build-pass
+
+// Test that a `-C panic=abort` binary crate can link to a `-C panic=unwind` core.
+
+fn main() {}