[llvm] 4cd3f4b - [WebAssembly] Fix a bug in finding matching EH pad
Heejin Ahn via llvm-commits
llvm-commits at lists.llvm.org
Thu May 28 19:48:01 PDT 2020
Author: Heejin Ahn
Date: 2020-05-28T19:46:11-07:00
New Revision: 4cd3f4b31b0bd19f3b63f53888a5a2afea68e109
URL: https://github.com/llvm/llvm-project/commit/4cd3f4b31b0bd19f3b63f53888a5a2afea68e109
DIFF: https://github.com/llvm/llvm-project/commit/4cd3f4b31b0bd19f3b63f53888a5a2afea68e109.diff
LOG: [WebAssembly] Fix a bug in finding matching EH pad
Summary:
`getMatchingEHPad()` in LateEHPrepare is a function to find the nearest
EH pad that dominates the given instruction. This intends to be
lightweight so it does not use full WebAssemblyException scope analysis
or dominator analysis. It simply does backward BFS to its predecessors
and stops at the first EH pad each search path encounters. All search
should end up at the same EH pad, and if not, it returns null.
But it didn't take into account that when there are inner scopes within
the current scope, some path in BFS can hit an inner EH pad first. For
example, in the given diagram, `Inst` belongs to the outer scope and
`getMathingEHPad()` should return 'EHPad 1', but some search path can go
into the inner scope and end up with 'EHPad 2'. The search will return
null because different paths end up with different EH pads.
```
--- EHPad 1 ---
| - EHPad 2 - |
| | | |
| ----------- |
| Inst |
---------------
```
So far this was OK because we haven't tested a case in which a given
instruction is far from its EH pad. Also, this bug does not happen when
the inner EH scope is a cleanup scope, because a cleanup scope ends with
a `cleanupret` whose successor is an EH pad, so the search encounters
that EH pad first before going into the child scope. But this can happen
when the child scope is a catch scope that ends with `catchret`. So this
patch, when doing backward BFS, does not search predecessors that ends
with `catchret`. Because `catchret`s are replaced with `br`s during this
pass, this records BBs that have `catchret`s in the beginning, before
doing any other transformations.
Reviewers: dschuff
Subscribers: sbc100, jgravelle-google, hiraditya, sunfish, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D80571
Added:
Modified:
llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
llvm/test/CodeGen/WebAssembly/exception.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
index 54115849df18..2280ec38e5b9 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
@@ -32,12 +32,16 @@ class WebAssemblyLateEHPrepare final : public MachineFunctionPass {
}
bool runOnMachineFunction(MachineFunction &MF) override;
+ void recordCatchRetBBs(MachineFunction &MF);
bool addCatches(MachineFunction &MF);
bool replaceFuncletReturns(MachineFunction &MF);
bool removeUnnecessaryUnreachables(MachineFunction &MF);
bool addExceptionExtraction(MachineFunction &MF);
bool restoreStackPointer(MachineFunction &MF);
+ MachineBasicBlock *getMatchingEHPad(MachineInstr *MI);
+ SmallSet<MachineBasicBlock *, 8> CatchRetBBs;
+
public:
static char ID; // Pass identification, replacement for typeid
WebAssemblyLateEHPrepare() : MachineFunctionPass(ID) {}
@@ -58,7 +62,8 @@ FunctionPass *llvm::createWebAssemblyLateEHPrepare() {
// possible search paths should be the same.
// Returns nullptr in case it does not find any EH pad in the search, or finds
// multiple
diff erent EH pads.
-static MachineBasicBlock *getMatchingEHPad(MachineInstr *MI) {
+MachineBasicBlock *
+WebAssemblyLateEHPrepare::getMatchingEHPad(MachineInstr *MI) {
MachineFunction *MF = MI->getParent()->getParent();
SmallVector<MachineBasicBlock *, 2> WL;
SmallPtrSet<MachineBasicBlock *, 2> Visited;
@@ -77,7 +82,9 @@ static MachineBasicBlock *getMatchingEHPad(MachineInstr *MI) {
}
if (MBB == &MF->front())
return nullptr;
- WL.append(MBB->pred_begin(), MBB->pred_end());
+ for (auto *Pred : MBB->predecessors())
+ if (!CatchRetBBs.count(Pred)) // We don't go into child scopes
+ WL.push_back(Pred);
}
return EHPad;
}
@@ -111,6 +118,7 @@ bool WebAssemblyLateEHPrepare::runOnMachineFunction(MachineFunction &MF) {
bool Changed = false;
if (MF.getFunction().hasPersonalityFn()) {
+ recordCatchRetBBs(MF);
Changed |= addCatches(MF);
Changed |= replaceFuncletReturns(MF);
}
@@ -122,6 +130,21 @@ bool WebAssemblyLateEHPrepare::runOnMachineFunction(MachineFunction &MF) {
return Changed;
}
+// Record which BB ends with 'CATCHRET' instruction, because this will be
+// replaced with BRs later. This set of 'CATCHRET' BBs is necessary in
+// 'getMatchingEHPad' function.
+void WebAssemblyLateEHPrepare::recordCatchRetBBs(MachineFunction &MF) {
+ CatchRetBBs.clear();
+ for (auto &MBB : MF) {
+ auto Pos = MBB.getFirstTerminator();
+ if (Pos == MBB.end())
+ continue;
+ MachineInstr *TI = &*Pos;
+ if (TI->getOpcode() == WebAssembly::CATCHRET)
+ CatchRetBBs.insert(&MBB);
+ }
+}
+
// Add catch instruction to beginning of catchpads and cleanuppads.
bool WebAssemblyLateEHPrepare::addCatches(MachineFunction &MF) {
bool Changed = false;
diff --git a/llvm/test/CodeGen/WebAssembly/exception.ll b/llvm/test/CodeGen/WebAssembly/exception.ll
index bfd4e1720e28..d6a636019838 100644
--- a/llvm/test/CodeGen/WebAssembly/exception.ll
+++ b/llvm/test/CodeGen/WebAssembly/exception.ll
@@ -74,7 +74,7 @@ rethrow: ; preds = %catch.start
call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
unreachable
-try.cont: ; preds = %entry, %catch
+try.cont: ; preds = %catch, %entry
ret void
}
@@ -169,7 +169,7 @@ 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
+try.cont: ; preds = %invoke.cont1, %entry
ret void
ehcleanup: ; preds = %catch.start
@@ -262,7 +262,7 @@ rethrow: ; preds = %catch.start
call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ]
unreachable
-try.cont: ; preds = %entry, %invoke.cont1
+try.cont: ; preds = %invoke.cont1, %entry
ret void
ehcleanup: ; preds = %catch
@@ -303,11 +303,11 @@ 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) #2 [ "funclet"(token %1) ]
+ %4 = call i8* @__cxa_begin_catch(i8* %2) [ "funclet"(token %1) ]
call void @__cxa_end_catch() [ "funclet"(token %1) ]
catchret from %1 to label %try.cont
-try.cont: ; preds = %entry, %catch.start
+try.cont: ; preds = %catch.start, %entry
ret void
}
@@ -327,8 +327,40 @@ catch.start: ; preds = %catch.dispatch
%3 = call i32 @llvm.wasm.get.ehselector(token %1)
catchret from %1 to label %try.cont
-try.cont: ; preds = %entry, %catch.start
+try.cont: ; preds = %catch.start, %entry
+ ret void
+}
+
+; Tests a case when a cleanup region (cleanuppad ~ clanupret) contains another
+; catchpad
+define void @test_complex_cleanup_region() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
+entry:
+ invoke void @foo()
+ to label %invoke.cont unwind label %ehcleanup
+
+invoke.cont: ; preds = %entry
ret void
+
+ehcleanup: ; preds = %entry
+ %0 = cleanuppad within none []
+ invoke void @foo() [ "funclet"(token %0) ]
+ to label %ehcleanupret unwind label %catch.dispatch
+
+catch.dispatch: ; preds = %ehcleanup
+ %1 = catchswitch within %0 [label %catch.start] unwind label %ehcleanup.1
+
+catch.start: ; preds = %catch.dispatch
+ %2 = catchpad within %1 [i8* null]
+ %3 = call i8* @llvm.wasm.get.exception(token %2)
+ %4 = call i32 @llvm.wasm.get.ehselector(token %2)
+ catchret from %2 to label %ehcleanupret
+
+ehcleanup.1: ; preds = %catch.dispatch
+ %5 = cleanuppad within %0 []
+ unreachable
+
+ehcleanupret: ; preds = %catch.start, %ehcleanup
+ cleanupret from %0 unwind to caller
}
declare void @foo()
More information about the llvm-commits
mailing list