-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
llvm.wasm.throw is inconsistently considered non-unwinding or invocable #124710
Comments
@llvm/issue-subscribers-backend-webassembly Author: Alisa Sireneva (purplesyringa)
1. Miscompiled:
void loud();
void throw_and_catch(void *ex) {
try {
__builtin_wasm_throw(0, ex);
} catch (...) {
loud();
}
} The generated IR is define hidden void @<!-- -->throw_and_catch(void*)(ptr noundef %ex) {
entry:
%ex.addr = alloca ptr, align 4
store ptr %ex, ptr %ex.addr, align 4
%0 = load ptr, ptr %ex.addr, align 4
call void @<!-- -->llvm.wasm.throw(i32 0, ptr %0)
ret void
} without a catchpad. https://godbolt.org/z/Po668Yxaz
void loud();
void my_throw(void *ex) {
__builtin_wasm_throw(0, ex);
}
void throw_and_catch(void *ex) {
try {
my_throw(ex);
} catch (...) {
loud();
}
} InlinerPass replaces invoke void @<!-- -->my_throw(void*)(ptr noundef %ex)
to label %try.cont unwind label %catch.dispatch (correct, working) with invoke void @<!-- -->llvm.wasm.throw(i32 0, ptr %ex)
to label %.noexc unwind label %catch.dispatch (broken, https://godbolt.org/z/rMEd1x79z As far as I'm aware, |
`llvm.wasm.throw` intrinsic can throw but it was not invokable. Not sure what the rationale was when it was first written that way, but I think at least in Emscripten's C++ exception support with the Wasm port of libunwind, `__builtin_wasm_throw`, which is lowered down to `llvm.wasm.rethrow`, is used only within `_Unwind_RaiseException`, which is a one-liner and thus does not need an `invoke`: https://github.com/emscripten-core/emscripten/blob/720e97f76d6f19e0c6a2d6988988cfe23f0517fb/system/lib/libunwind/src/Unwind-wasm.c#L69 (`_Unwind_RaiseException` is called by `__cxa_throw`, which is generated by the `throw` C++ keyword) But this does not address other direct uses of the builtin in C++, whose use I'm not sure about but is not prohibited. Also other language frontends may need to use the builtin in different functions, which has `try`-`catch`es or destructors. This makes `llvm.wasm.throw` invokable in the backend. To do that, this adds a custom lowering routine to `SelectionDAGBuilder::visitInvoke`, like we did for `llvm.wasm.rethrow`. This does not generate `invoke`s for `__builtin_wasm_throw` yet, which will be done by a follow-up PR. Addresses llvm#124710.
Even though `__builtin_wasm_throw`, which is lowered down to `llvm.wasm.throw`, throws, ```cpp try { __builtin_wasm_throw(0, obj); } catch (...) { } ``` does not generate `invoke`. This is because we have assumed the intrinsic cannot be invoked, which doesn't make much sense. (See llvm#128104 for the historical context) llvm#128104 made `llvm.wasm.throw` intrinsic invokable in the backend. This actually generates `invoke`s in Clang for `__builtin_wasm_throw`. While we're at it, this also generates `invoke`s for `__builtin_wasm_rethrow`, which is actually not used anywhere in C++ support. I haven't deleted it just in case in may have uses later. (For example, to support rethrow functionality that carries stack trace with exnref) Depends on llvm#128104 for the CI to pass. Fixes llvm#124710.
The generated IR is
without a catchpad.
https://godbolt.org/z/Po668Yxaz
InlinerPass replaces
(correct, working) with
(broken,
llvm.wasm.throw
cannot be invoked), which halts the backend.https://godbolt.org/z/rMEd1x79z
As far as I'm aware,
llvm.wasm.throw
is the only function that can unwind but cannot be invoked, making this troublesome for external frontends like rustc, not just for optimization passes. Ideally it would become invocable.The text was updated successfully, but these errors were encountered: