-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[EH] Fix inconsistency of in/decrementing refcounts #25988
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
base: main
Are you sure you want to change the base?
Conversation
This fixes inconsistency of incrementing/decrementing refcounts between Wasm EH and Emscripten EH. Previously, in Emscripten EH, we incremented the refcount in `__cxa_begin_catch`, while Wasm EH incremented it in `__cxa_throw`. This PR moves the incrementing call from `__cxa_begin_catch` to `__cxa_throw` as well. This also increments the refcount in `__cxa_rethrow`. These incrementing calls are guarded with `+#if !DISABLE_EXCEPTION_CATCHING`, because without that, `std::terminate` will run: https://github.com/emscripten-core/emscripten/blob/d1251798144df813c52934768964a1223504c440/system/lib/libcxxabi/src/cxa_noexception.cpp#L25-L35 Fixes emscripten-core#17115.
|
Great! It looks like these mentions can also be removed from the docs now: Lines 1837 to 1838 in d125179
|
Thanks! Removed: 9fee482 |
|
This fails LTO tests due to llvm/llvm-project#173235. Before, We have two choices:
Maybe 2 is better for safety anyway. |
This fixes inconsistency of incrementing/decrementing refcounts between Wasm EH and Emscripten EH.
Previously, in Emscripten EH, we incremented the refcount in
__cxa_begin_catch, while Wasm EH incremented it in__cxa_throw. This PR moves the incrementing call from__cxa_begin_catchto__cxa_throwas well. This also increments the refcount in__cxa_rethrow.These incrementing calls are guarded with
+#if !DISABLE_EXCEPTION_CATCHING, because without that,std::terminatewill run:emscripten/system/lib/libcxxabi/src/cxa_noexception.cpp
Lines 25 to 35 in d125179
Fixes #17115.