Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compiler-builtins/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,7 @@ pub mod x86;
#[cfg(target_arch = "x86_64")]
pub mod x86_64;

#[cfg(target_family = "wasm")]
pub mod wasm;

pub mod probestack;
23 changes: 23 additions & 0 deletions compiler-builtins/src/wasm.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//! Builtins for WebAssembly targets.

// Define the __cpp_exception tag that LLVM's wasm exception handling requires.
// This must be provided since LLVM commit
// aee99e8015daa9f53ab1fd4e5b24cc4c694bdc4a which changed the tag from being
// weakly defined in each object file to being an external reference that must
// be linked from somewhere.
//
// In LLVM's compiler-rt this is provided by
// compiler-rt/lib/builtins/wasm/__cpp_exception.S
//
// Emscripten provides this tag in its runtime libraries, so we don't need to
// define it for Emscripten targets. Including this in Emscripten would break
// the wasm-cpp-exception-tag test.
Copy link
Member

Choose a reason for hiding this comment

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

For wasi should this be provided by wasi-libc in case C++ code is used?

Copy link
Author

Choose a reason for hiding this comment

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

I am not sure what is happening with wasi cc @alexcrichton . But I'm happy to guard this to also be skipped on wasi if we think that's the right choice. I think the only case where there could be problems is with dynamic linking.

Copy link
Member

Choose a reason for hiding this comment

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

I'd agree with bjorn3 that this shouldn't be defined here, but in general with dynamic linking I don't think this should be defined in compiler-builtins either because that would mean that there'd be an exception-type-per-shared-object which I don't think is what's desired.

Is this just for Emscripten targets? If so, shouldn't Emscripten be the one providing this definition? If this isn't for Emscripten, is it for wasm32-unknown-unknown? If so where does Rust end up needing __cpp_exception from?

Copy link
Author

Choose a reason for hiding this comment

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

It is for wasm32-unknown-unknown. I have a cfg on this code that excludes Emscripten because Emscripten is already handling it. Rust uses cpp_exception for panics and catch_unwind. It is emitted by llvm, and by default it will import it from env.__cpp_exception unless you define the symbol explicitly. wasm-bindgen is currently using walrus to transform the generated module and convert the __cpp_exception import into a tag definition:
https://github.com/wasm-bindgen/wasm-bindgen/pull/4938/changes#diff-8d702d8da596b2105e4e94a3855489a49792843bd621954a3feacf845b6d42afR375-R389

Copy link
Author

Choose a reason for hiding this comment

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

Specifically, panic uses the wasm_throw llvm intrinsic and catch_unwind uses the wasm_catch llvm intrinsic. Both of these intrinsics generate a reference to this symbol:
wasm_throw definition
wasm_catch definition
__cpp_exception symbol generated here

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, thanks for the info. In that situation though this definition I think might be best to reside in the unwind crate in the standard library? There's no need to have it compiler-builtins specifically and unwinding is the true source of the symbol as well?

Additionally I think it would be good to restrict it to target_os="unknown" to only affect wasm32-unknown-unknown. The responsibility for defining this symbol on other toolchains lies elsewhere (e.g. with Emscripten or WASI targets). External toolchains will have more knowledge of what to do in the face of shared libraries, for example.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, that's fine with me.

#[cfg(not(target_os = "emscripten"))]
core::arch::global_asm!(
".globl __cpp_exception",
#[cfg(target_pointer_width = "64")]
".tagtype __cpp_exception i64",
#[cfg(target_pointer_width = "32")]
".tagtype __cpp_exception i32",
"__cpp_exception:",
);