Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Support wasm exception handling for Emscripten target #112195

Open
hoodmane opened this issue Jun 2, 2023 · 13 comments
Open

Support wasm exception handling for Emscripten target #112195

hoodmane opened this issue Jun 2, 2023 · 13 comments
Labels
A-panic Area: Panicking machinery O-emscripten Target: 50% off wasm32-unknown-musl. the savings come out of stdio.h, but hey, you get SDL! O-wasm Target: WASM (WebAssembly), http://webassembly.org/

Comments

@hoodmane
Copy link
Contributor

hoodmane commented Jun 2, 2023

Currently the Rust Emscripten target supports js exception handling. I've been trying to get it to support also wasm exception handling. The patch to get it to exclusively use wasm exception handling is quite small. The following suffices:

Patch
diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs
index a0a640473eb..cd88648911a 100644
--- a/compiler/rustc_codegen_llvm/src/llvm_util.rs
+++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs
@@ -110,7 +110,7 @@ fn llvm_arg_to_arg_name(full_arg: &str) -> &str {
         }
 
         if sess.target.os == "emscripten" && sess.panic_strategy() == PanicStrategy::Unwind {
-            add("-enable-emscripten-cxx-exceptions", false);
+            add("-wasm-enable-sjlj", false);
         }
 
         // HACK(eddyb) LLVM inserts `llvm.assume` calls to preserve align attributes
diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs
index 94acdea894b..2909401836d 100644
--- a/compiler/rustc_codegen_ssa/src/back/link.rs
+++ b/compiler/rustc_codegen_ssa/src/back/link.rs
@@ -2057,12 +2057,11 @@ fn add_order_independent_options(
     }
 
     if sess.target.os == "emscripten" {
-        cmd.arg("-s");
-        cmd.arg(if sess.panic_strategy() == PanicStrategy::Abort {
-            "DISABLE_EXCEPTION_CATCHING=1"
+        if sess.panic_strategy() == PanicStrategy::Abort {
+            cmd.arg("-sDISABLE_EXCEPTION_CATCHING=1");
         } else {
-            "DISABLE_EXCEPTION_CATCHING=0"
-        });
+            cmd.arg("-fwasm-exceptions");
+        }
     }
 
     if flavor == LinkerFlavor::PtxLinker {

I'm not sure what the best way is to add support for a choice between these two options.

@aheejin Is there any way to turn -enable-emscripten-cxx-exceptions back off by adding another argument? It would be convenient to be able to inject -mllvm -disable-emscripten-cxx-exceptions -mllvm -wasm-enable-sjlj and get wasm exceptions to work...

@hoodmane
Copy link
Contributor Author

hoodmane commented Jun 2, 2023

If a target has more than one stack unwinding ABI is there some standard way to switch between them?

@hoodmane
Copy link
Contributor Author

hoodmane commented Jun 2, 2023

Ah it seems that the following may work to disable js exceptions and enable wasm ones:

	-C link-arg=-fwasm-exceptions \
	-C link-arg=-sDISABLE_EXCEPTION_CATCHING=1 \
	-C llvm-args=-enable-emscripten-cxx-exceptions=0 \
    -C llvm-args=-wasm-enable-sjlj 

@Jules-Bertholet
Copy link
Contributor

Jules-Bertholet commented Jun 2, 2023

@rustbot label O-wasm

@rustbot
Copy link
Collaborator

rustbot commented Jun 2, 2023

Error: Label o-wasm can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@rustbot rustbot added the O-wasm Target: WASM (WebAssembly), http://webassembly.org/ label Jun 2, 2023
@aheejin
Copy link

aheejin commented Jun 3, 2023

@aheejin Is there any way to turn -enable-emscripten-cxx-exceptions back off by adding another argument? It would be convenient to be able to inject -mllvm -disable-emscripten-cxx-exceptions -mllvm -wasm-enable-sjlj and get wasm exceptions to work...

Which tools do you call? clang? And the option you want to use is either -enable-emscripten-cxx-exceptions -mllvm -enable-emscripten-sjlj (for Emscripten (JS) exceptions/SjLj) or -fwasm-exceptions -mllvm -wasm-enable-sjlj (for Wasm exceptions/SjLj). In which case do you need -disable-emscripten-cxx-exceptions? Only specifying -fwasm-exceptions will enable Wasm EH and disable Emscripten EH, and specifying -enable-emscripten-cxx-exceptions will enable Emscripten EH and disable Wasm EH. If you give both -enable-emscripten-cxx-exceptions and -fwasm-exceptions at the same time it will error out.

@hoodmane
Copy link
Contributor Author

hoodmane commented Jun 3, 2023

The issue is that Rust always adds -enable-emscripten-cxx-exceptions, so if I want to use wasm-exceptions I have to turn it back off somehow. But -enable-emscripten-cxx-exceptions=0 seems to do that... at least some of the time.

@hoodmane
Copy link
Contributor Author

I made a bit more progress over here:
emscripten-core/emscripten#19612

It seems that we need to adjust the ir generated for the try intrinsic and I have a guess of what the IR should be. But I would probably need help from someone who understands rustc_codegen_llvm a bit better.

@purplesyringa
Copy link
Contributor

Are there any plans on how wasm vs JS exceptions are going to be configured? (-Z emscripten_wasm_eh is funny, but I suppose there's going to be a -C option?) Would this option be readable via a cfg?

@hoodmane
Copy link
Contributor Author

Well if you look at the MCP and [the PR] (#131830) you'll see in the discussion that we aren't really thinking about stabilizing it to a -C flag but eventually turning it on always and then later removing the option to turn it off. I think we'll need more discussion before each of these changes though. Do you expect to need the legacy exception handling going forward?

There is a cfg flag for it but you have to enable an internal feature to access it. I'm not sure if you can enable internal features outside the standard library though.

@purplesyringa
Copy link
Contributor

I do not necessarily need legacy exception handling myself, but I'm maintaining a crate that needs to use different code for different EH ABIs, hence I'm interested in knowing what the way forward is going to be so that I can update the code preemptively. I understand this whole crate idea is based on thin ice, so I'll just hardcode cfg(emscripten_wasm_eh) for the time being, but if there's a public cfg, I'll happily use it instead.

@hoodmane
Copy link
Contributor Author

Cool! I appreciate that you are making the effort to support emscripten.

@workingjubilee workingjubilee added A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows A-panic Area: Panicking machinery and removed A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows labels Jan 22, 2025
@hoodmane
Copy link
Contributor Author

@workingjubilee should we close this as resolved via the -Zemscripten-wasm-eh flag?

@purplesyringa
Copy link
Contributor

Wasm EH is still utterly broken, see e.g. #135665. Please don't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-panic Area: Panicking machinery O-emscripten Target: 50% off wasm32-unknown-musl. the savings come out of stdio.h, but hey, you get SDL! O-wasm Target: WASM (WebAssembly), http://webassembly.org/
Projects
None yet
Development

No branches or pull requests

6 participants