Skip to content

Commit 492812f

Browse files
authored
[WebAssembly] Fix rethrow's index calculation (#114693)
So far we have assumed that we only rethrow the exception caught in the innermost EH pad. This is true in code we directly generate, but after inlining this may not be the case. For example, consider this code: ```ll ehcleanup: %0 = cleanuppad ... call @Destructor cleanupret from %0 unwind label %catch.dispatch ``` If `destructor` gets inlined into this function, the code can be like ```ll ehcleanup: %0 = cleanuppad ... invoke @throwing_func to label %unreachale unwind label %catch.dispatch.i catch.dispatch.i: catchswitch ... [ label %catch.start.i ] catch.start.i: %1 = catchpad ... invoke @some_function to label %invoke.cont.i unwind label %terminate.i invoke.cont.i: catchret from %1 to label %destructor.exit destructor.exit: cleanupret from %0 unwind label %catch.dispatch ``` We lower a `cleanupret` into `rethrow`, which assumes it rethrows the exception caught by the nearest dominating EH pad. But after the inlining, the nearest dominating EH pad is not `ehcleanup` but `catch.start.i`. The problem exists in the same manner in the new (exnref) EH, because it assumes the exception comes from the nearest EH pad and saves an exnref from that EH pad and rethrows it (using `throw_ref`). This problem can be fixed easily if `cleanupret` has the basic block where its matching `cleanuppad` is. The bitcode instruction `cleanupret` kind of has that info (it has a token from the `cleanuppad`), but that info is lost when when we enter ISel, because `TargetSelectionDAG.td`'s `cleanupret` node does not have any arguments: https://github.com/llvm/llvm-project/blob/5091a359d9807db8f7d62375696f93fc34226969/llvm/include/llvm/Target/TargetSelectionDAG.td#L700 Note that `catchret` already has two basic block arguments, even though neither of them means `catchpad`'s BB. This PR adds the `cleanuppad`'s BB as an argument to `cleanupret` node in ISel and uses it in the Wasm backend. Because this node is also used in X86 backend we need to note its argument there too but nothing more needs to change there as long as X86 doesn't need it. --- - Details about changes in the Wasm backend: After this PR, our pseudo `RETHROW` instruction takes a BB, which means the EH pad whose exception it needs to rethrow. There are currently two ways to generate a `RETHROW`: one is from `llvm.wasm.rethrow` intrinsic and the other is from `CLEANUPRET` we discussed above. In case of `llvm.wasm.rethrow`, we add a '0' as a placeholder argument when it is lowered to a `RETHROW`, and change it to a BB in LateEHPrepare. As written in the comments, this PR doesn't change how this BB is computed. The BB argument will be converted to an immediate argument as with other control flow instructions in CFGStackify. In case of `CLEANUPRET`, it already has a BB argument pointing to an EH pad, so it is just converted to a `RETHROW` with the same BB argument in LateEHPrepare. This will also be lowered to an immediate in CFGStackify with other control flow instructions. --- Fixes #114600.
1 parent cbc7812 commit 492812f

12 files changed

+305
-73
lines changed

llvm/include/llvm/Target/TargetSelectionDAG.td

+5-1
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,10 @@ def SDTCatchret : SDTypeProfile<0, 2, [ // catchret
234234
SDTCisVT<0, OtherVT>, SDTCisVT<1, OtherVT>
235235
]>;
236236

237+
def SDTCleanupret : SDTypeProfile<0, 1, [ // cleanupret
238+
SDTCisVT<0, OtherVT>
239+
]>;
240+
237241
def SDTNone : SDTypeProfile<0, 0, []>; // ret, trap
238242

239243
def SDTUBSANTrap : SDTypeProfile<0, 1, []>; // ubsantrap
@@ -697,7 +701,7 @@ def brind : SDNode<"ISD::BRIND" , SDTBrind, [SDNPHasChain]>;
697701
def br : SDNode<"ISD::BR" , SDTBr, [SDNPHasChain]>;
698702
def catchret : SDNode<"ISD::CATCHRET" , SDTCatchret,
699703
[SDNPHasChain, SDNPSideEffect]>;
700-
def cleanupret : SDNode<"ISD::CLEANUPRET" , SDTNone, [SDNPHasChain]>;
704+
def cleanupret : SDNode<"ISD::CLEANUPRET" , SDTCleanupret, [SDNPHasChain]>;
701705

702706
def trap : SDNode<"ISD::TRAP" , SDTNone,
703707
[SDNPHasChain, SDNPSideEffect]>;

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -2151,8 +2151,10 @@ void SelectionDAGBuilder::visitCleanupRet(const CleanupReturnInst &I) {
21512151
FuncInfo.MBB->normalizeSuccProbs();
21522152

21532153
// Create the terminator node.
2154-
SDValue Ret =
2155-
DAG.getNode(ISD::CLEANUPRET, getCurSDLoc(), MVT::Other, getControlRoot());
2154+
MachineBasicBlock *CleanupPadMBB =
2155+
FuncInfo.getMBB(I.getCleanupPad()->getParent());
2156+
SDValue Ret = DAG.getNode(ISD::CLEANUPRET, getCurSDLoc(), MVT::Other,
2157+
getControlRoot(), DAG.getBasicBlock(CleanupPadMBB));
21562158
DAG.setRoot(Ret);
21572159
}
21582160

llvm/lib/Target/AArch64/AArch64InstrInfo.td

+1-1
Original file line numberDiff line numberDiff line change
@@ -5372,7 +5372,7 @@ let isPseudo = 1 in {
53725372
//===----------------------------------------------------------------------===//
53735373
let isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1,
53745374
isCodeGenOnly = 1, isReturn = 1, isEHScopeReturn = 1, isPseudo = 1 in {
5375-
def CLEANUPRET : Pseudo<(outs), (ins), [(cleanupret)]>, Sched<[]>;
5375+
def CLEANUPRET : Pseudo<(outs), (ins), [(cleanupret bb)]>, Sched<[]>;
53765376
let usesCustomInserter = 1 in
53775377
def CATCHRET : Pseudo<(outs), (ins am_brcond:$dst, am_brcond:$src), [(catchret bb:$dst, bb:$src)]>,
53785378
Sched<[]>;

llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp

+7-41
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,8 @@ class WebAssemblyCFGStackify final : public MachineFunctionPass {
9999
const MachineBasicBlock *MBB);
100100
unsigned getDelegateDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
101101
const MachineBasicBlock *MBB);
102-
unsigned
103-
getRethrowDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
104-
const SmallVectorImpl<const MachineBasicBlock *> &EHPadStack);
102+
unsigned getRethrowDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
103+
const MachineBasicBlock *EHPadToRethrow);
105104
void rewriteDepthImmediates(MachineFunction &MF);
106105
void fixEndsAtEndOfFunction(MachineFunction &MF);
107106
void cleanupFunctionData(MachineFunction &MF);
@@ -2458,34 +2457,13 @@ unsigned WebAssemblyCFGStackify::getDelegateDepth(
24582457

24592458
unsigned WebAssemblyCFGStackify::getRethrowDepth(
24602459
const SmallVectorImpl<EndMarkerInfo> &Stack,
2461-
const SmallVectorImpl<const MachineBasicBlock *> &EHPadStack) {
2460+
const MachineBasicBlock *EHPadToRethrow) {
24622461
unsigned Depth = 0;
2463-
// In our current implementation, rethrows always rethrow the exception caught
2464-
// by the innermost enclosing catch. This means while traversing Stack in the
2465-
// reverse direction, when we encounter END_TRY, we should check if the
2466-
// END_TRY corresponds to the current innermost EH pad. For example:
2467-
// try
2468-
// ...
2469-
// catch ;; (a)
2470-
// try
2471-
// rethrow 1 ;; (b)
2472-
// catch ;; (c)
2473-
// rethrow 0 ;; (d)
2474-
// end ;; (e)
2475-
// end ;; (f)
2476-
//
2477-
// When we are at 'rethrow' (d), while reversely traversing Stack the first
2478-
// 'end' we encounter is the 'end' (e), which corresponds to the 'catch' (c).
2479-
// And 'rethrow' (d) rethrows the exception caught by 'catch' (c), so we stop
2480-
// there and the depth should be 0. But when we are at 'rethrow' (b), it
2481-
// rethrows the exception caught by 'catch' (a), so when traversing Stack
2482-
// reversely, we should skip the 'end' (e) and choose 'end' (f), which
2483-
// corresponds to 'catch' (a).
24842462
for (auto X : reverse(Stack)) {
24852463
const MachineInstr *End = X.second;
24862464
if (End->getOpcode() == WebAssembly::END_TRY) {
24872465
auto *EHPad = TryToEHPad[EndToBegin[End]];
2488-
if (EHPadStack.back() == EHPad)
2466+
if (EHPadToRethrow == EHPad)
24892467
break;
24902468
}
24912469
++Depth;
@@ -2497,7 +2475,6 @@ unsigned WebAssemblyCFGStackify::getRethrowDepth(
24972475
void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
24982476
// Now rewrite references to basic blocks to be depth immediates.
24992477
SmallVector<EndMarkerInfo, 8> Stack;
2500-
SmallVector<const MachineBasicBlock *, 8> EHPadStack;
25012478

25022479
auto RewriteOperands = [&](MachineInstr &MI) {
25032480
// Rewrite MBB operands to be depth immediates.
@@ -2508,6 +2485,8 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
25082485
if (MO.isMBB()) {
25092486
if (MI.getOpcode() == WebAssembly::DELEGATE)
25102487
MO = MachineOperand::CreateImm(getDelegateDepth(Stack, MO.getMBB()));
2488+
else if (MI.getOpcode() == WebAssembly::RETHROW)
2489+
MO = MachineOperand::CreateImm(getRethrowDepth(Stack, MO.getMBB()));
25112490
else
25122491
MO = MachineOperand::CreateImm(getBranchDepth(Stack, MO.getMBB()));
25132492
}
@@ -2539,12 +2518,8 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
25392518
Stack.pop_back();
25402519
break;
25412520

2542-
case WebAssembly::END_TRY: {
2543-
auto *EHPad = TryToEHPad[EndToBegin[&MI]];
2544-
EHPadStack.push_back(EHPad);
2545-
[[fallthrough]];
2546-
}
25472521
case WebAssembly::END_BLOCK:
2522+
case WebAssembly::END_TRY:
25482523
case WebAssembly::END_TRY_TABLE:
25492524
Stack.push_back(std::make_pair(&MBB, &MI));
25502525
break;
@@ -2553,15 +2528,6 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
25532528
Stack.push_back(std::make_pair(EndToBegin[&MI]->getParent(), &MI));
25542529
break;
25552530

2556-
case WebAssembly::CATCH_LEGACY:
2557-
case WebAssembly::CATCH_ALL_LEGACY:
2558-
EHPadStack.pop_back();
2559-
break;
2560-
2561-
case WebAssembly::RETHROW:
2562-
MI.getOperand(0).setImm(getRethrowDepth(Stack, EHPadStack));
2563-
break;
2564-
25652531
case WebAssembly::DELEGATE:
25662532
RewriteOperands(MI);
25672533
Stack.push_back(std::make_pair(&MBB, &MI));

llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp

+13
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,19 @@ void WebAssemblyDAGToDAGISel::Select(SDNode *Node) {
248248
ReplaceNode(Node, Throw);
249249
return;
250250
}
251+
case Intrinsic::wasm_rethrow: {
252+
// RETHROW's BB argument will be populated in LateEHPrepare. Just use a
253+
// '0' as a placeholder for now.
254+
MachineSDNode *Rethrow = CurDAG->getMachineNode(
255+
WebAssembly::RETHROW, DL,
256+
MVT::Other, // outchain type
257+
{
258+
CurDAG->getConstant(0, DL, MVT::i32), // placeholder
259+
Node->getOperand(0) // inchain
260+
});
261+
ReplaceNode(Node, Rethrow);
262+
return;
263+
}
251264
}
252265
break;
253266
}

llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td

+4-5
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,8 @@ defm CATCH_ALL_REF : I<(outs EXNREF:$dst), (ins), (outs), (ins), []>;
168168
// Pseudo instructions: cleanupret / catchret
169169
let isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1,
170170
isPseudo = 1, isEHScopeReturn = 1 in {
171-
defm CLEANUPRET : NRI<(outs), (ins), [(cleanupret)], "cleanupret", 0>;
171+
defm CLEANUPRET : NRI<(outs), (ins bb_op:$ehpad), [(cleanupret bb:$ehpad)],
172+
"cleanupret", 0>;
172173
defm CATCHRET : NRI<(outs), (ins bb_op:$dst, bb_op:$from),
173174
[(catchret bb:$dst, bb:$from)], "catchret", 0>;
174175
} // isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1,
@@ -180,11 +181,9 @@ let isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1,
180181
// Rethrowing an exception: rethrow
181182
// The new exnref proposal also uses this instruction as an interim pseudo
182183
// instruction before we convert it to a THROW_REF.
184+
// $ehpad is the EH pad where the exception to rethrow has been caught.
183185
let isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in
184-
defm RETHROW : NRI<(outs), (ins i32imm:$depth), [], "rethrow \t$depth", 0x09>;
185-
// The depth argument will be computed in CFGStackify. We set it to 0 here for
186-
// now.
187-
def : Pat<(int_wasm_rethrow), (RETHROW 0)>;
186+
defm RETHROW : NRI<(outs), (ins bb_op:$ehpad), [], "rethrow \t$ehpad", 0x09>;
188187

189188
// Region within which an exception is caught: try / end_try
190189
let Uses = [VALUE_STACK], Defs = [VALUE_STACK] in {

llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp

+37-17
Original file line numberDiff line numberDiff line change
@@ -251,15 +251,39 @@ bool WebAssemblyLateEHPrepare::replaceFuncletReturns(MachineFunction &MF) {
251251
Changed = true;
252252
break;
253253
}
254+
case WebAssembly::RETHROW:
255+
// These RETHROWs here were lowered from llvm.wasm.rethrow() intrinsics,
256+
// generated in Clang for when an exception is not caught by the given
257+
// type (e.g. catch (int)).
258+
//
259+
// RETHROW's BB argument is the EH pad where the exception to rethrow has
260+
// been caught. (Until this point, RETHROW has just a '0' as a placeholder
261+
// argument.) For these llvm.wasm.rethrow()s, we can safely assume the
262+
// exception comes from the nearest dominating EH pad, because catch.start
263+
// EH pad is structured like this:
264+
//
265+
// catch.start:
266+
// catchpad ...
267+
// %matches = compare ehselector with typeid
268+
// br i1 %matches, label %catch, label %rethrow
269+
//
270+
// rethrow:
271+
// ;; rethrows the exception caught in 'catch.start'
272+
// call @llvm.wasm.rethrow()
273+
TI->removeOperand(0);
274+
TI->addOperand(MachineOperand::CreateMBB(getMatchingEHPad(TI)));
275+
Changed = true;
276+
break;
254277
case WebAssembly::CLEANUPRET: {
255-
// Replace a cleanupret with a rethrow. For C++ support, currently
256-
// rethrow's immediate argument is always 0 (= the latest exception).
278+
// CLEANUPRETs have the EH pad BB the exception to rethrow has been caught
279+
// as an argument. Use it and change the instruction opcode to 'RETHROW'
280+
// to make rethrowing instructions consistent.
257281
//
258-
// Even when -wasm-enable-exnref is true, we use a RETHROW here for the
259-
// moment. This will be converted to a THROW_REF in
260-
// addCatchRefsAndThrowRefs.
282+
// This is because we cannot safely assume that it is always the nearest
283+
// dominating EH pad, in case there are code transformations such as
284+
// inlining.
261285
BuildMI(MBB, TI, TI->getDebugLoc(), TII.get(WebAssembly::RETHROW))
262-
.addImm(0);
286+
.addMBB(TI->getOperand(0).getMBB());
263287
TI->eraseFromParent();
264288
Changed = true;
265289
break;
@@ -272,21 +296,17 @@ bool WebAssemblyLateEHPrepare::replaceFuncletReturns(MachineFunction &MF) {
272296
// Add CATCH_REF and CATCH_ALL_REF pseudo instructions to EH pads, and convert
273297
// RETHROWs to THROW_REFs.
274298
bool WebAssemblyLateEHPrepare::addCatchRefsAndThrowRefs(MachineFunction &MF) {
275-
bool Changed = false;
276299
const auto &TII = *MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
277300
auto &MRI = MF.getRegInfo();
278301
DenseMap<MachineBasicBlock *, SmallVector<MachineInstr *, 2>> EHPadToRethrows;
279302

280303
// Create a map of <EH pad, a vector of RETHROWs rethrowing its exception>
281-
for (auto &MBB : MF) {
282-
for (auto &MI : MBB) {
283-
if (MI.getOpcode() == WebAssembly::RETHROW) {
284-
Changed = true;
285-
auto *EHPad = getMatchingEHPad(&MI);
286-
EHPadToRethrows[EHPad].push_back(&MI);
287-
}
288-
}
289-
}
304+
for (auto &MBB : MF)
305+
for (auto &MI : MBB)
306+
if (MI.getOpcode() == WebAssembly::RETHROW)
307+
EHPadToRethrows[MI.getOperand(0).getMBB()].push_back(&MI);
308+
if (EHPadToRethrows.empty())
309+
return false;
290310

291311
// Convert CATCH into CATCH_REF and CATCH_ALL into CATCH_ALL_REF, when the
292312
// caught exception is rethrown. And convert RETHROWs to THROW_REFs.
@@ -325,7 +345,7 @@ bool WebAssemblyLateEHPrepare::addCatchRefsAndThrowRefs(MachineFunction &MF) {
325345
}
326346
}
327347

328-
return Changed;
348+
return true;
329349
}
330350

331351
// Remove unnecessary unreachables after a throw/rethrow/throw_ref.

llvm/lib/Target/X86/X86InstrCompiler.td

+2-1
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,8 @@ def EH_RETURN64 : I<0xC3, RawFrm, (outs), (ins GR64:$addr),
195195

196196
let isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1,
197197
isCodeGenOnly = 1, isReturn = 1, isEHScopeReturn = 1 in {
198-
def CLEANUPRET : I<0, Pseudo, (outs), (ins), "# CLEANUPRET", [(cleanupret)]>;
198+
def CLEANUPRET : I<0, Pseudo, (outs), (ins), "# CLEANUPRET",
199+
[(cleanupret bb)]>;
199200

200201
// CATCHRET needs a custom inserter for SEH.
201202
let usesCustomInserter = 1 in

llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.mir

+7-4
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,16 @@ body: |
4040
; CHECK: RETHROW 1
4141
EH_LABEL <mcsymbol .Ltmp2>
4242
%0:i32 = CATCH_LEGACY &__cpp_exception, implicit-def dead $arguments
43-
RETHROW 0, implicit-def dead $arguments
43+
RETHROW %bb.1, implicit-def dead $arguments
4444
4545
bb.2 (landing-pad):
46+
successors: %bb.3
4647
; CHECK: bb.2 (landing-pad):
4748
; CHECK: CATCH_LEGACY
4849
; CHECK: RETHROW 0
4950
EH_LABEL <mcsymbol .Ltmp3>
5051
%1:i32 = CATCH_LEGACY &__cpp_exception, implicit-def dead $arguments
51-
RETHROW 0, implicit-def dead $arguments
52+
RETHROW %bb.2, implicit-def dead $arguments
5253
5354
bb.3:
5455
; CHECK: bb.3:
@@ -105,12 +106,14 @@ body: |
105106
RETURN %0:i32, implicit-def dead $arguments
106107
107108
bb.3 (landing-pad):
109+
successors:
108110
EH_LABEL <mcsymbol .Ltmp4>
109111
%0:i32 = CATCH_LEGACY &__cpp_exception, implicit-def dead $arguments
110-
RETHROW 0, implicit-def dead $arguments
112+
RETHROW %bb.3, implicit-def dead $arguments
111113
112114
bb.4 (landing-pad):
115+
successors:
113116
EH_LABEL <mcsymbol .Ltmp5>
114117
%1:i32 = CATCH_LEGACY &__cpp_exception, implicit-def dead $arguments
115-
RETHROW 0, implicit-def dead $arguments
118+
RETHROW %bb.4, implicit-def dead $arguments
116119
...

0 commit comments

Comments
 (0)