Skip to content

Commit d8b3dc5

Browse files
committed
[WebAssembly] Fix remapping branch dests in fixCatchUnwindMismatches
This is a case D97178 tried to solve but missed. D97178 could not handle the case when multiple consecutive delegates are generated: - Before: ``` block br (a) try catch end_try end_block <- (a) ``` - After ``` block br (a) try ... try try catch end_try <- (a) delegate delegate end_block <- (b) ``` (The `br` should point to (b) now) D97178 assumed `end_block` exists two BBs later than `end_try`, because it assumed the order as `end_try` BB -> `delegate` BB -> `end_block` BB. But it turned out there can be multiple `delegate`s in between. This patch changes the logic so we just search from `end_try` BB until we find `end_block`. Fixes emscripten-core/emscripten#13515. (More precisely, fixes emscripten-core/emscripten#13515 (comment).) Reviewed By: dschuff, tlively Differential Revision: https://reviews.llvm.org/D97569
1 parent 83bc781 commit d8b3dc5

File tree

2 files changed

+125
-12
lines changed

2 files changed

+125
-12
lines changed

llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp

+21-12
Original file line numberDiff line numberDiff line change
@@ -1368,18 +1368,15 @@ bool WebAssemblyCFGStackify::fixCatchUnwindMismatches(MachineFunction &MF) {
13681368
if (EHPadToUnwindDest.empty())
13691369
return false;
13701370
NumCatchUnwindMismatches += EHPadToUnwindDest.size();
1371-
// <current branch dest, future branch dest> map, because fixing catch unwind
1372-
// mismatches can invalidate branch destinations
1373-
DenseMap<MachineBasicBlock *, MachineBasicBlock *> BrDestMap;
1371+
SmallPtrSet<MachineBasicBlock *, 4> NewEndTryBBs;
13741372

13751373
for (auto &P : EHPadToUnwindDest) {
13761374
MachineBasicBlock *EHPad = P.first;
13771375
MachineBasicBlock *UnwindDest = P.second;
13781376
MachineInstr *Try = EHPadToTry[EHPad];
13791377
MachineInstr *EndTry = BeginToEnd[Try];
13801378
addTryDelegate(Try, EndTry, UnwindDest);
1381-
BrDestMap[EndTry->getParent()] =
1382-
EndTry->getParent()->getNextNode()->getNextNode();
1379+
NewEndTryBBs.insert(EndTry->getParent());
13831380
}
13841381

13851382
// Adding a try-delegate wrapping an existing try-catch-end can make existing
@@ -1423,17 +1420,29 @@ bool WebAssemblyCFGStackify::fixCatchUnwindMismatches(MachineFunction &MF) {
14231420
// As we can see in this case, when branches target a BB that has both
14241421
// 'end_try' and 'end_block' and the BB is split to insert a 'delegate', we
14251422
// have to remap existing branch destinations so that they target not the
1426-
// 'end_try' BB but the new 'end_block' BB, which should be the second next BB
1427-
// of 'end_try' (because there is a 'delegate' BB in between). In this
1428-
// example, the 'br bb3' instruction should be remapped to 'br split_bb'.
1423+
// 'end_try' BB but the new 'end_block' BB. There can be multiple 'delegate's
1424+
// in between, so we try to find the next BB with 'end_block' instruction. In
1425+
// this example, the 'br bb3' instruction should be remapped to 'br split_bb'.
14291426
for (auto &MBB : MF) {
14301427
for (auto &MI : MBB) {
14311428
if (MI.isTerminator()) {
14321429
for (auto &MO : MI.operands()) {
1433-
if (MO.isMBB()) {
1434-
auto It = BrDestMap.find(MO.getMBB());
1435-
if (It != BrDestMap.end())
1436-
MO.setMBB(It->second);
1430+
if (MO.isMBB() && NewEndTryBBs.count(MO.getMBB())) {
1431+
auto *BrDest = MO.getMBB();
1432+
bool FoundEndBlock = false;
1433+
for (; std::next(BrDest->getIterator()) != MF.end();
1434+
BrDest = BrDest->getNextNode()) {
1435+
for (const auto &MI : *BrDest) {
1436+
if (MI.getOpcode() == WebAssembly::END_BLOCK) {
1437+
FoundEndBlock = true;
1438+
break;
1439+
}
1440+
}
1441+
if (FoundEndBlock)
1442+
break;
1443+
}
1444+
assert(FoundEndBlock);
1445+
MO.setMBB(BrDest);
14371446
}
14381447
}
14391448
}

llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll

+104
Original file line numberDiff line numberDiff line change
@@ -1196,6 +1196,108 @@ try.cont: ; preds = %catch.start1, %catc
11961196
ret void
11971197
}
11981198

1199+
; The similar case with test20, but multiple consecutive delegates are
1200+
; generated:
1201+
; - Before:
1202+
; block
1203+
; br (a)
1204+
; try
1205+
; catch
1206+
; end_try
1207+
; end_block
1208+
; <- (a)
1209+
;
1210+
; - After
1211+
; block
1212+
; br (a)
1213+
; try
1214+
; ...
1215+
; try
1216+
; try
1217+
; catch
1218+
; end_try
1219+
; <- (a)
1220+
; delegate
1221+
; delegate
1222+
; end_block
1223+
; <- (b) The br destination should be remapped to here
1224+
;
1225+
; The test was reduced by bugpoint and should not crash.
1226+
define void @test21() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
1227+
entry:
1228+
br i1 undef, label %if.then, label %if.end12
1229+
1230+
if.then: ; preds = %entry
1231+
invoke void @__cxa_throw() #1
1232+
to label %unreachable unwind label %catch.dispatch
1233+
1234+
catch.dispatch: ; preds = %if.then
1235+
%0 = catchswitch within none [label %catch.start] unwind to caller
1236+
1237+
catch.start: ; preds = %catch.dispatch
1238+
%1 = catchpad within %0 [i8* bitcast (i8** @_ZTIi to i8*)]
1239+
%2 = call i8* @llvm.wasm.get.exception(token %1)
1240+
%3 = call i32 @llvm.wasm.get.ehselector(token %1)
1241+
catchret from %1 to label %catchret.dest
1242+
1243+
catchret.dest: ; preds = %catch.start
1244+
invoke void @foo()
1245+
to label %invoke.cont unwind label %catch.dispatch4
1246+
1247+
invoke.cont: ; preds = %catchret.dest
1248+
invoke void @__cxa_throw() #1
1249+
to label %unreachable unwind label %catch.dispatch4
1250+
1251+
catch.dispatch4: ; preds = %invoke.cont, %catchret.dest
1252+
%4 = catchswitch within none [label %catch.start5] unwind to caller
1253+
1254+
catch.start5: ; preds = %catch.dispatch4
1255+
%5 = catchpad within %4 [i8* bitcast (i8** @_ZTIi to i8*)]
1256+
%6 = call i8* @llvm.wasm.get.exception(token %5)
1257+
%7 = call i32 @llvm.wasm.get.ehselector(token %5)
1258+
unreachable
1259+
1260+
if.end12: ; preds = %entry
1261+
invoke void @foo()
1262+
to label %invoke.cont14 unwind label %catch.dispatch16
1263+
1264+
catch.dispatch16: ; preds = %if.end12
1265+
%8 = catchswitch within none [label %catch.start17] unwind label %ehcleanup
1266+
1267+
catch.start17: ; preds = %catch.dispatch16
1268+
%9 = catchpad within %8 [i8* bitcast (i8** @_ZTIi to i8*)]
1269+
%10 = call i8* @llvm.wasm.get.exception(token %9)
1270+
%11 = call i32 @llvm.wasm.get.ehselector(token %9)
1271+
br i1 undef, label %catch20, label %rethrow19
1272+
1273+
catch20: ; preds = %catch.start17
1274+
catchret from %9 to label %catchret.dest22
1275+
1276+
catchret.dest22: ; preds = %catch20
1277+
br label %try.cont23
1278+
1279+
rethrow19: ; preds = %catch.start17
1280+
invoke void @llvm.wasm.rethrow() #1 [ "funclet"(token %9) ]
1281+
to label %unreachable unwind label %ehcleanup
1282+
1283+
try.cont23: ; preds = %invoke.cont14, %catchret.dest22
1284+
invoke void @foo()
1285+
to label %invoke.cont24 unwind label %ehcleanup
1286+
1287+
invoke.cont24: ; preds = %try.cont23
1288+
ret void
1289+
1290+
invoke.cont14: ; preds = %if.end12
1291+
br label %try.cont23
1292+
1293+
ehcleanup: ; preds = %try.cont23, %rethrow19, %catch.dispatch16
1294+
%12 = cleanuppad within none []
1295+
cleanupret from %12 unwind to caller
1296+
1297+
unreachable: ; preds = %if.then, %invoke.cont, %rethrow19
1298+
unreachable
1299+
}
1300+
11991301
; Check if the unwind destination mismatch stats are correct
12001302
; NOSORT: 20 wasm-cfg-stackify - Number of call unwind mismatches found
12011303
; NOSORT: 4 wasm-cfg-stackify - Number of catch unwind mismatches found
@@ -1225,6 +1327,8 @@ declare i8* @llvm.wasm.get.exception(token) #0
12251327
; Function Attrs: nounwind
12261328
declare i32 @llvm.wasm.get.ehselector(token) #0
12271329
; Function Attrs: noreturn
1330+
declare void @__cxa_throw() #1
1331+
; Function Attrs: noreturn
12281332
declare void @llvm.wasm.rethrow() #1
12291333
; Function Attrs: nounwind
12301334
declare i32 @llvm.eh.typeid.for(i8*) #0

0 commit comments

Comments
 (0)