Change default panic strategy to abort for wasm32-unknown-emscripten

[nedv-eu]

Emscripten v2.0.10 removed __gxx_personality_v0 function stub that panic-unwind in wam32-unknown-emscripten target depends on. This causes linker error when using newer versions of emscripten compiler. As mentioned in #85821 (comment) the __gxx_personality_v0 function was just a stub in emscripten for several years and therefor the panic-unwind strategy was broken all the time. Changing default to abort fixes builds (issue 85821) with recent version of emscripten yet we are not loosing any functionality as the panic-unwind was broken anyway. Fixes #85821

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @michaelwoerister (or someone else) soon.

Please see the contribution instructions for more information.

[rust-highfive]

⚠️ Warning ⚠️

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

[michaelwoerister]

Thanks for the PR, @nedv-eu!

I don't feel qualified to review this. Does anyone in @rust-lang/compiler-contributors know more about this topic?

[tmandry]

The change makes sense to me, but I'm not familiar with emscripten specifically. Do we have a maintainer for that target?

[michaelwoerister]

r? @pnkfelix for re-assignment -- maybe we should discuss in the triage meeting?

[nikic]

Based on

rust/compiler/rustc_codegen_ssa/src/back/link.rs

Lines 1950 to 1957 in bb918d0

	if sess.target.is_like_emscripten {
	cmd.arg("-s");
	cmd.arg(if sess.panic_strategy() == PanicStrategy::Abort {
	"DISABLE_EXCEPTION_CATCHING=1"
	} else {
	"DISABLE_EXCEPTION_CATCHING=0"
	});
	}

and https://github.com/rust-lang/rust/blob/673d0db5e393e9c64897005b470bfeb6d5aec61b/library/panic_unwind/src/emcc.rs
it looks like we do try to support exceptions with emscripten.

[nedv-eu]

Yes, seems like there was an attempt to support this. However since it is broken and the fact that enabling exceptions involves a lot of overhead (see https://emscripten.org/docs/porting/exceptions.html) I believe it is the better option to use abort panic strategy by default - as is the case for other wasm targets.

[pnkfelix]

nominating for discussion in T-compiler meeting about whether we should just adopt approach given here, or if we should expend effort figuring out the exception situation for emscripten.

[wesleywiser]

Waiting on @pnkfelix for review and approval per last weeks' compiler team meeting.

[tmarkovski]

Alternatively, if team doesn't feel the default strategy can be changed, this can be fixed by allowing the emscripten arguments to be passed to the linker, instead overriden. Related bug - #41750
That should fix most of the bugs related to undefined symbol __gxx_personality_v0

[nedv-eu]

The problem is that __gxx_personality_v0 symbol actually is undefined and panic unwind code is using it. Disabling errors on undefined symbols only masks the problem. Panic unwind at runtime will still crash on undefined symbol. And not only this one but any other undefined symbol error will be caught not during link time but at runtime. That is unacceptable as per fail early rule.

[pnkfelix]

@bors r+

[bors]

📌 Commit 8b8bbf0 has been approved by pnkfelix

[matthiaskrgr]

@bors r-
failed in a rollup: #91862 (comment)

[matthiaskrgr]

@bors rollup=iffy

[Dylan-DPC]

@nedv-eu any updates on the rollup failure?

[nedv-eu]

Sorry for delay. For some reason I missed previous notifications in this topic.

I went through the failing tests and it seems they are not supposed to be run in this case. There is always a comment "ignore-wasm32-bare compiled with panic=abort by default"

[bjorn3]

I suspect ignore-wasm32-bare only applies to wasm32-unknown-unknown and not -emscripten. Try ignore-wasm32 without -bare.

[Dylan-DPC]

@nedv-eu any updates on this?

[nedv-eu]

In the end I based my project on wasm32-unknown-unknown. Unfortunately I cannot invest more effort in fixing emscripten target. Feel free to close this unless there is someone else interested in using wasm32-unknown-emscripten target. Since it is broken for quite a along time (at least with recent version of emcc) maybe it is worth considering removing the target from Tier 2?

[Dylan-DPC]

Closing this based on the last comment