[PATCH] D97834: [WebAssembly] Disable uses of __clang_call_terminate
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG561abd83ffec: [WebAssembly] Disable uses of
__clang_call_terminate (authored by aheejin).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97834/new/
https://reviews.llvm.org/D97834
Files:
clang/lib/CodeGen/CGException.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/wasm-eh.cpp
llvm/lib/CodeGen/WasmEHPrepare.cpp
llvm/lib/Target/WebAssembly/CMakeLists.txt
llvm/lib/Target/WebAssembly/WebAssembly.h
llvm/lib/Target/WebAssembly/WebAssemblyHandleEHTerminatePads.cpp
llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp
llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
llvm/test/CodeGen/WebAssembly/eh-lsda.ll
llvm/test/CodeGen/WebAssembly/exception.ll
llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
Index: llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
===
--- llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
+++ llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
@@ -132,59 +132,6 @@
ret void
}
-; A cleanuppad with a call to __clang_call_terminate().
-;
-; void foo();
-; void test2() {
-; try {
-; foo();
-; } catch (...) {
-; foo();
-; }
-; }
-define void @test2() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
-; CHECK-LABEL: @test2
-entry:
- invoke void @foo()
- to label %try.cont unwind label %catch.dispatch
-
-catch.dispatch: ; preds = %entry
- %0 = catchswitch within none [label %catch.start] unwind to caller
-
-catch.start: ; preds = %catch.dispatch
- %1 = catchpad within %0 [i8* null]
- %2 = call i8* @llvm.wasm.get.exception(token %1)
- %3 = call i32 @llvm.wasm.get.ehselector(token %1)
- %4 = call i8* @__cxa_begin_catch(i8* %2) [ "funclet"(token %1) ]
- invoke void @foo() [ "funclet"(token %1) ]
- to label %invoke.cont1 unwind label %ehcleanup
-
-invoke.cont1: ; preds = %catch.start
- call void @__cxa_end_catch() [ "funclet"(token %1) ]
- catchret from %1 to label %try.cont
-
-try.cont: ; preds = %entry, %invoke.cont1
- ret void
-
-ehcleanup:; preds = %catch.start
- %5 = cleanuppad within %1 []
- invoke void @__cxa_end_catch() [ "funclet"(token %5) ]
- to label %invoke.cont2 unwind label %terminate
-
-invoke.cont2: ; preds = %ehcleanup
- cleanupret from %5 unwind to caller
-
-terminate:; preds = %ehcleanup
- %6 = cleanuppad within %5 []
- %7 = call i8* @llvm.wasm.get.exception(token %6)
- call void @__clang_call_terminate(i8* %7) [ "funclet"(token %6) ]
- unreachable
-; CHECK: terminate:
-; CHECK-NEXT: cleanuppad
-; CHECK-NEXT: %[[EXN:.*]] = call i8* @llvm.wasm.catch
-; CHECK-NEXT: call void @__clang_call_terminate(i8* %[[EXN]])
-}
-
; PHI demotion test. Only the phi before catchswitch should be demoted; the phi
; before cleanuppad should NOT.
;
@@ -194,7 +141,7 @@
; ~Temp() {}
; };
;
-; void test3() {
+; void test2() {
; int num;
; try {
; Temp t;
@@ -214,8 +161,8 @@
; bar(num);
; }
; }
-define void @test3() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
-; CHECK-LABEL: @test3
+define void @test2() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
+; CHECK-LABEL: @test2
entry:
%t = alloca %struct.Temp, align 1
invoke void @foo()
@@ -279,8 +226,8 @@
; Tests if instructions after a call to @llvm.wasm.throw are deleted and the
; BB's dead children are deleted.
-; CHECK-LABEL: @test4
-define i32 @test4(i1 %b, i8* %p) {
+; CHECK-LABEL: @test3
+define i32 @test3(i1 %b, i8* %p) {
entry:
br i1 %b, label %bb.true, label %bb.false
@@ -308,14 +255,22 @@
declare void @bar(i32)
declare %struct.Temp* @_ZN4TempD2Ev(%struct.Temp* returned)
declare i32 @__gxx_wasm_personality_v0(...)
-declare i8* @llvm.wasm.get.exception(token)
-declare i32 @llvm.wasm.get.ehselector(token)
-declare i32 @llvm.eh.typeid.for(i8*)
-declare void @llvm.wasm.throw(i32, i8*)
-declare void @llvm.wasm.rethrow()
+; Function Attrs: nounwind
+declare i8* @llvm.wasm.get.exception(token) #0
+; Function Attrs: nounwind
+declare i32 @llvm.wasm.get.ehselector(token) #0
+; Function Attrs: nounwind
+declare i32 @llvm.eh.typeid.for(i8*) #0
+; Function Attrs: noreturn
+declare void @llvm.wasm.throw(i32, i8*) #1
+; Function Attrs: noreturn
+declare void @llvm.wasm.rethrow() #1
declare i8* @__cxa_begin_catch(i8*)
declare void @__cxa_end_catch()
-declare void @__clang_call_terminate(i8*)
+declare void @_ZSt9terminatev(
[PATCH] D97834: [WebAssembly] Disable uses of __clang_call_terminate
aheejin updated this revision to Diff 328116.
aheejin added a comment.
Address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97834/new/
https://reviews.llvm.org/D97834
Files:
clang/lib/CodeGen/CGException.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/wasm-eh.cpp
llvm/lib/CodeGen/WasmEHPrepare.cpp
llvm/lib/Target/WebAssembly/CMakeLists.txt
llvm/lib/Target/WebAssembly/WebAssembly.h
llvm/lib/Target/WebAssembly/WebAssemblyHandleEHTerminatePads.cpp
llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp
llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
llvm/test/CodeGen/WebAssembly/eh-lsda.ll
llvm/test/CodeGen/WebAssembly/exception.ll
llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
Index: llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
===
--- llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
+++ llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
@@ -132,59 +132,6 @@
ret void
}
-; A cleanuppad with a call to __clang_call_terminate().
-;
-; void foo();
-; void test2() {
-; try {
-; foo();
-; } catch (...) {
-; foo();
-; }
-; }
-define void @test2() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
-; CHECK-LABEL: @test2
-entry:
- invoke void @foo()
- to label %try.cont unwind label %catch.dispatch
-
-catch.dispatch: ; preds = %entry
- %0 = catchswitch within none [label %catch.start] unwind to caller
-
-catch.start: ; preds = %catch.dispatch
- %1 = catchpad within %0 [i8* null]
- %2 = call i8* @llvm.wasm.get.exception(token %1)
- %3 = call i32 @llvm.wasm.get.ehselector(token %1)
- %4 = call i8* @__cxa_begin_catch(i8* %2) [ "funclet"(token %1) ]
- invoke void @foo() [ "funclet"(token %1) ]
- to label %invoke.cont1 unwind label %ehcleanup
-
-invoke.cont1: ; preds = %catch.start
- call void @__cxa_end_catch() [ "funclet"(token %1) ]
- catchret from %1 to label %try.cont
-
-try.cont: ; preds = %entry, %invoke.cont1
- ret void
-
-ehcleanup:; preds = %catch.start
- %5 = cleanuppad within %1 []
- invoke void @__cxa_end_catch() [ "funclet"(token %5) ]
- to label %invoke.cont2 unwind label %terminate
-
-invoke.cont2: ; preds = %ehcleanup
- cleanupret from %5 unwind to caller
-
-terminate:; preds = %ehcleanup
- %6 = cleanuppad within %5 []
- %7 = call i8* @llvm.wasm.get.exception(token %6)
- call void @__clang_call_terminate(i8* %7) [ "funclet"(token %6) ]
- unreachable
-; CHECK: terminate:
-; CHECK-NEXT: cleanuppad
-; CHECK-NEXT: %[[EXN:.*]] = call i8* @llvm.wasm.catch
-; CHECK-NEXT: call void @__clang_call_terminate(i8* %[[EXN]])
-}
-
; PHI demotion test. Only the phi before catchswitch should be demoted; the phi
; before cleanuppad should NOT.
;
@@ -194,7 +141,7 @@
; ~Temp() {}
; };
;
-; void test3() {
+; void test2() {
; int num;
; try {
; Temp t;
@@ -214,8 +161,8 @@
; bar(num);
; }
; }
-define void @test3() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
-; CHECK-LABEL: @test3
+define void @test2() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
+; CHECK-LABEL: @test2
entry:
%t = alloca %struct.Temp, align 1
invoke void @foo()
@@ -279,8 +226,8 @@
; Tests if instructions after a call to @llvm.wasm.throw are deleted and the
; BB's dead children are deleted.
-; CHECK-LABEL: @test4
-define i32 @test4(i1 %b, i8* %p) {
+; CHECK-LABEL: @test3
+define i32 @test3(i1 %b, i8* %p) {
entry:
br i1 %b, label %bb.true, label %bb.false
@@ -308,14 +255,22 @@
declare void @bar(i32)
declare %struct.Temp* @_ZN4TempD2Ev(%struct.Temp* returned)
declare i32 @__gxx_wasm_personality_v0(...)
-declare i8* @llvm.wasm.get.exception(token)
-declare i32 @llvm.wasm.get.ehselector(token)
-declare i32 @llvm.eh.typeid.for(i8*)
-declare void @llvm.wasm.throw(i32, i8*)
-declare void @llvm.wasm.rethrow()
+; Function Attrs: nounwind
+declare i8* @llvm.wasm.get.exception(token) #0
+; Function Attrs: nounwind
+declare i32 @llvm.wasm.get.ehselector(token) #0
+; Function Attrs: nounwind
+declare i32 @llvm.eh.typeid.for(i8*) #0
+; Function Attrs: noreturn
+declare void @llvm.wasm.throw(i32, i8*) #1
+; Function Attrs: noreturn
+declare void @llvm.wasm.rethrow() #1
declare i8* @__cxa_begin_catch(i8*)
declare void @__cxa_end_catch()
-declare void @__clang_call_terminate(i8*)
+declare void @_ZSt9terminatev()
+
+attributes #0 = { nounwind }
+attributes #1 = { noreturn }
; CHECK-DAG: declare void @llvm.wasm.landingpad.index(token, i32 immarg)
; CHECK-D
[PATCH] D97834: [WebAssembly] Disable uses of __clang_call_terminate
dschuff accepted this revision. dschuff added a comment. I agree this is a good approach, and I also like that it's smaller and simpler. In the future we can revisit whether following this particular Itanium convention buys us anything useful or not. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97834/new/ https://reviews.llvm.org/D97834 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D97834: [WebAssembly] Disable uses of __clang_call_terminate
tlively accepted this revision. tlively added a comment. This revision is now accepted and ready to land. Nice, this actually looks like a good simplification in addition to fixing the specific problem. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4650 + // Itanium ABI calls __clang_call_terminate(), which __cxa_begin_catch() on + // the violating exception to mark it handled, but it is currently hard to dl + // with wasm EH instruction structure with catch/catch_all, we just call Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97834/new/ https://reviews.llvm.org/D97834 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D97834: [WebAssembly] Disable uses of __clang_call_terminate
aheejin created this revision.
aheejin added reviewers: dschuff, tlively.
Herald added subscribers: wingo, ecnelises, sunfish, hiraditya,
jgravelle-google, sbc100, mgorny.
aheejin requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
Background:
Wasm EH, while using Windows EH (catchpad/cleanuppad based) IR, uses
Itanium-based libraries and ABIs with some modifications.
`__clang_call_terminate` is a wrapper generated in Clang's Itanium C++
ABI implementation. It contains this code, in C-style pseudocode:
void __clang_call_terminate(void *exn) {
__cxa_begin_catch(exn);
std::terminate();
}
So this function is a wrapper to call `__cxa_begin_catch` on the
exeption pointer before termination.
In Itanium ABI, this function is called when another exception is thrown
while processing an exception. The pointer for this second, violating
exception is passed as the argument of this `__clang_call_terminate`,
which calls `__cxa_begin_catch` with that pointer and calls
`std::terminate` to terminate the program.
The spec (https://libcxxabi.llvm.org/spec.html) says,
When the personality routine encounters a termination condition, it
will call __cxa_begin_catch() to mark the exception as handled and then
call terminate(), which shall not return to its caller.
In wasm EH's Clang implementation, this function is called from
cleanuppads that terminates the program, which we also call terminate
pads. Cleanuppads normally don't access the thrown exception and the
wasm backend converts them to `catch_all` blocks. But because we need
the exception pointer in this cleanuppad, we generate
`wasm.get.exception` intrinsic (which will eventually be lowered to
`catch` instruction) as we do in the catchpads. But because terminate
pads are cleanup pads and should run even when a foreign exception is
thrown, so what we have been doing is:
1. In `WebAssemblyLateEHPrepare::ensureSingleBBTermPads()`, we make sure
terminate pads are in this simple shape:
%exn = catch
call @__clang_call_terminate(%exn)
unreachable
2. In `WebAssemblyHandleEHTerminatePads` pass at the end of the
pipeline, we attach a `catch_all` to terminate pads, so they will be in
this form:
%exn = catch
call @__clang_call_terminate(%exn)
unreachable
catch_all
call @std::terminate()
unreachable
In `catch_all` part, we don't have the exception pointer, so we call
`std::terminate()` directly. The reason we ran HandleEHTerminatePads at
the end of the pipeline, separate from LateEHPrepare, was it was
convenient to assume there was only a single `catch` part per `try`
during CFGSort and CFGStackify.
---
Problem:
While it thinks terminate pads could have been possibly split or calls
to `__clang_call_terminate` could have been duplicated,
`WebAssemblyLateEHPrepare::ensureSingleBBTermPads()` assumes terminate
pads contain no more than calls to `__clang_call_terminate` and
`unreachable` instruction. I assumed that because in LLVM very limited
forms of transformations are done to catchpads and cleanuppads to
maintain the scoping structure. But it turned out to be incorrect;
passes can merge multiple cleanuppads into one, including terminate
pads, as long as the new code has a correct scoping structure. One pass
that does this I observed was SimplifyCFG, but there can be more. After
this transformation, a single cleanuppad can contain any number of other
instructions with the call to `__clang_call_terminate` and can span many
BBs. It wouldn't be practical to duplicate all these BBs within the
cleanuppad to generate the equivalent `catch_all` blocks, only with
calls to `__clang_call_terminate` replaced by calls to `std::terminate`.
Unless we do more complicated transformation to split those calls to
`__clang_call_terminate` into a separate cleanuppad, it is tricky to
solve.
---
Solution (?):
This CL just disables the generation and use of `__clang_call_terminate`
and calls `std::terminate()` directly in its place.
The possible downside of this approach can be, because the Itanium ABI
intended to "mark" the violating exception handled, we don't do that
anymore. What `__cxa_begin_catch` actually does is increment the
exception's handler count and decrement the uncaught exception count,
which in my opinion do not matter much given that we are about to
terminate the program anyway. Also it does not affect info like stack
traces that can be possibly shown to developers.
And while we use a variant of Itanium EH ABI, we can make some
deviations if we choose to; we are already different in that in the
current version of the EH spec we don't support two-phase unwinding. We
can possibly consider a more complicated transformation later to
reenable this, but I don't think that has high priority.
Changes in this CL contains:
- In Clang, we don't generate a call to `wasm.get.exception()` intrinsic and
`__clang_call_terminate` function in terminate pads anymore; we simply
