Disable unwinding for emscripten again

[Jules-Bertholet]

Unwinding on wasm32-unknown-emscripten seems to be broken again, so this PR disables it. Undoes 62c3443, fixes #85821, redux of #89762 which was abandoned by its author.

[rust-highfive]

Thank you for submitting a new PR for the library teams! If this PR contains a stabilization of a library feature that has not already completed FCP in its tracking issue, introduces new or changes existing unstable library APIs, or changes our public documentation in ways that create new stability guarantees then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary

[rust-highfive]

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

[rust-highfive]

⚠️ Warning ⚠️

• These commits modify compiler targets. (See the Target Tier Policy.)

[nagisa]

Given that this is not the first time this default changes, I think we should have a write-up detailing a final decision and stick with it, instead of switching it back and forth every time the upstream project releases a new version.

[vincenzopalazzo]

LGTM, but lets wait the final decision on that problem

[Jules-Bertholet]

From my limited sleuthing:
According to #85821 (comment), apparently Rust unwinding has never really worked properly on Emscripten. Emscripten offers two alternative exception handing methods: via JS and natively via WASM. The JS method is slow, requires JS (which not all WASM environments have), and it's not clear how easy it would be to support with Rust. The WASM exceptions seem much more straightforward, but they rely on a not-yet-finalized proposal that most WASM engines don't support.

[nagisa]

I'm going to nominate this for a T-compiler discussion.

I think it is a correct thing to disable unwinding here, but I worry that doing so will break somebody's CI or something. Probably is worth it over a target that doesn't work at all, but also if we make a decision like this now, I feel like we should make it such that it is permanent. That is, we would commit to wasm32-unknown-emscripten always using the abort panic scheme. We'd commit here so that users of this target can actually rely on a specific panicking mechanism here rather than being forced to specify --panic=abort to get any semblance of stability here.

[Jules-Bertholet]

rust/compiler/rustc_target/src/spec/wasm_base.rs

Lines 83 to 87 in 2fa9789

	// Unwinding doesn't work right now, so the whole target unconditionally
	// defaults to panic=abort. Note that this is guaranteed to change in
	// the future once unwinding is implemented. Don't rely on this as we're
	// basically guaranteed to change it once WebAssembly supports
	// exceptions.

I don't think committing to never ever having any form of unwinding for the Emscripten target makes sense, given that Rust doesn't commit to that for the other Wasm targets; once the Wasm exception handling proposal is finalized and stabilized everywhere, it can be used to implement unwinding. A weaker commitment which I think does make sense is that unwinding won't be implemented with any mechanism other than native Wasm exceptions—Emscripten's homegrown exception handling doesn't provide a API that Rust can rely on.

[Jules-Bertholet]

Another option is to gate WebAssembly unwinding behind an exceptions target feature, or some similar mechanism. See also #77839

[apiraino]

The topic of disabling unwind has been discussed during T-compiler meeting on Zulip (notes). One idea was to disable unwind (so this PR can move forward) and define a process involving a writeup to reinstate it back if/when deemed necessary.

@rustbot label -I-compiler-nominated

[Jules-Bertholet]

@rustbot ready

[bors]

☔ The latest upstream changes (presumably #97512) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #97939) made this pull request unmergeable. Please resolve the merge conflicts.

[Jules-Bertholet]

This should not be merged until the discussion around #97888 is resolved.

[bors]

☔ The latest upstream changes (presumably #97968) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #98037) made this pull request unmergeable. Please resolve the merge conflicts.

[tlively]

Hi, I work on Emscripten and have historically worked on the Emscripten beckends in Rust, too. I am the one who previously enabled unwinding for the Emscripten target. From briefly looking at the issues that motivated this, it looks like the reporters were trying to use versions of Emscripten that use versions of LLVM that are incompatible with Rust's version of LLVM. In other words, I think the root problem here is a lack of documentation or automation about what version of Emscripten should be used with Rust, not that there is an actual bug with Emscripten or Rust.

[Jules-Bertholet]

No, this wasn't an LLVM compatibility issue as far as I can tell, this was due to a change in Emscripten's libunwind.

[tlively]

Sure, but the principle is the same. Using Emscripten versions from before that change to Emscripten's libunwind is the "correct" fix to this problem, since the latest version (or any other recent version) of Emscripten is not compatible with Rust in general.

[Jules-Bertholet]

I would think that the "correct" fix would be to rely only on APIs that the Emscripten project makes stability guarantees for, so users don't need to be bound to a specific version.

[tlively]

I agree that that would be even better, but Emscripten does not currently make stability guarantees for anything beyond the basic C ABI, so unfortunately that is not realistic right now. We're aiming to stabilize our C++ ABI soon, which would include libunwind, but it is not stable yet. Relatedly, we would like to stabilize a dynamic linking ABI soon, but again, that's not stable yet.