[WebAssembly] Fix unwind mismatches in new EH #114361
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes unwind mismatches for the new EH spec.
The main flow is similar to that of the legacy EH's unwind mismatch
fixing. The new EH shared `fixCallUnwindMismatches` and
`fixCatchUnwindMismatches` functions, which gather the range of
instructions we need to fix their unwind destination for, with the
legacy EH. But unlike the legacy EH that uses `try`-`delegate`s to fix
them, the new EH wrap those instructions with nested
`try_table`-`end_try_table`s that jump to a "trampoline" BB, where we
rethrow (using a `throw_ref`) the exception to the correct `try_table`.
For a simple example of a call unwind mismatch, suppose if `call foo`
should unwind to the outer `try_table` but is wrapped in another
`try_table` (not shown here):
```wast
try_table
...
call foo ;; Unwind mismatch. Should unwind to the outer try_table
...
end_try_table
```
Then we wrap the call with a new nested `try_table`-`end_try_table`, add
a `block` / `end_block` right inside the target `try_table`, and make
the nested `try_table` jump to it using a `catch_all_ref` clause, and
rethrow the exception using a `throw_ref`:
```wast
try_table
block $l0 exnref
...
try_table (catch_all_ref $l0)
call foo
end_try_table
...
end_block ;; Trampoline BB
throw_ref
end_try_table
```
---
This fixes two existing bugs. These are not easy to test independently
without the unwind mismatch fixing. The first one is how we calculate
`ScopeTops`. Turns out, we should do it in the same way as in the legacy
EH even though there is no `end_try` at the end of `catch` block
anymore. `nested_try` in `cfg-stackify-eh.ll` tests this case.
The second bug is in `rewriteDepthImmediates`. `try_table`'s immediates
should be computed without the `try_table` itself, meaning
```wast
block
try_table (catch ... 0)
end_try_table
end_block
```
Here 0 should target not `end_try_table` but `end_block`. This bug
didn't crash the program because `placeTryTableMarker` generated only
the simple form of `try_table` that has a single catch clause and an
`end_block` follows right after the `end_try_table` in the same BB, so
jumping to an `end_try_table` is the same as jumping to the `end_block`.
But now we generate `catch` clauses with depths greater than 0 with when
fixing unwind mismatches, which uncovered this bug.
---
One case that needs a special treatment was when `end_loop` precedes an
`end_try_table` within a BB and this BB is a (true) unwind destination
when fixing unwind mismatches. In this case we need to split this
`end_loop` into a predecessor BB. This case is tested in
`unwind_mismatches_with_loop` in `cfg-stackify-eh.ll`.
---
`cfg-stackify-eh.ll` contains mostly the same set of tests with the
existing `cfg-stackify-eh-legacy.ll` with the updated FileCheck
expectations. As in `cfg-stackify-eh-legacy.ll`, the FileCheck lines
mostly only contain control flow instructions and calls for
readability.
- `nested_try` and `unwind_mismatches_with_loop` are added to test newly
found bugs in the new EH.
- Some tests in `cfg-stackify-eh-legacy.ll` about the legacy-EH-specific
asepcts have not been added to `cfg-stackify-eh.ll`.
(`remove_unnecessary_instrs`, `remove_unnecessary_br`,
`fix_function_end_return_type_with_try_catch`, and
`branch_remapping_after_fixing_unwind_mismatches_0/1`)
|
@llvm/pr-subscribers-backend-webassembly Author: Heejin Ahn (aheejin) ChangesThis fixes unwind mismatches for the new EH spec. The main flow is similar to that of the legacy EH's unwind mismatch fixing. The new EH shared For a simple example of a call unwind mismatch, suppose if try_table
...
call foo ;; Unwind mismatch. Should unwind to the outer try_table
...
end_try_tableThen we wrap the call with a new nested try_table
block $l0 exnref
...
try_table (catch_all_ref $l0)
call foo
end_try_table
...
end_block ;; Trampoline BB
throw_ref
end_try_tableThis fixes two existing bugs. These are not easy to test independently without the unwind mismatch fixing. The first one is how we calculate The second bug is in block
try_table (catch ... 0)
end_try_table
end_blockHere 0 should target not One case that needs a special treatment was when
Patch is 103.16 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114361.diff 3 Files Affected:
diff --git a/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h b/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
index e3a60fa4812d8f..3900d4a0aa7044 100644
--- a/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
+++ b/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
@@ -478,6 +478,22 @@ inline bool isMarker(unsigned Opc) {
}
}
+inline bool isEndMarker(unsigned Opc) {
+ switch (Opc) {
+ case WebAssembly::END_BLOCK:
+ case WebAssembly::END_BLOCK_S:
+ case WebAssembly::END_LOOP:
+ case WebAssembly::END_LOOP_S:
+ case WebAssembly::END_TRY:
+ case WebAssembly::END_TRY_S:
+ case WebAssembly::END_TRY_TABLE:
+ case WebAssembly::END_TRY_TABLE_S:
+ return true;
+ default:
+ return false;
+ }
+}
+
inline bool isTry(unsigned Opc) {
switch (Opc) {
case WebAssembly::TRY:
@@ -510,6 +526,20 @@ inline bool isCatch(unsigned Opc) {
}
}
+inline bool isCatchAll(unsigned Opc) {
+ switch (Opc) {
+ case WebAssembly::CATCH_ALL_LEGACY:
+ case WebAssembly::CATCH_ALL_LEGACY_S:
+ case WebAssembly::CATCH_ALL:
+ case WebAssembly::CATCH_ALL_S:
+ case WebAssembly::CATCH_ALL_REF:
+ case WebAssembly::CATCH_ALL_REF_S:
+ return true;
+ default:
+ return false;
+ }
+}
+
inline bool isLocalGet(unsigned Opc) {
switch (Opc) {
case WebAssembly::LOCAL_GET_I32:
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
index a5f73fabca3542..6800fb614300fa 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
@@ -78,13 +78,19 @@ class WebAssemblyCFGStackify final : public MachineFunctionPass {
void placeTryMarker(MachineBasicBlock &MBB);
void placeTryTableMarker(MachineBasicBlock &MBB);
- // Exception handling related functions
+ // Unwind mismatch fixing for exception handling
+ // - Common functions
bool fixCallUnwindMismatches(MachineFunction &MF);
bool fixCatchUnwindMismatches(MachineFunction &MF);
+ void recalculateScopeTops(MachineFunction &MF);
+ // - Legacy EH
void addNestedTryDelegate(MachineInstr *RangeBegin, MachineInstr *RangeEnd,
MachineBasicBlock *UnwindDest);
- void recalculateScopeTops(MachineFunction &MF);
void removeUnnecessaryInstrs(MachineFunction &MF);
+ // - Standard EH (exnref)
+ void addNestedTryTable(MachineInstr *RangeBegin, MachineInstr *RangeEnd,
+ MachineBasicBlock *UnwindDest);
+ MachineBasicBlock *getTrampolineBlock(MachineBasicBlock *UnwindDest);
// Wrap-up
using EndMarkerInfo =
@@ -111,6 +117,9 @@ class WebAssemblyCFGStackify final : public MachineFunctionPass {
// <EH pad, TRY marker> map
DenseMap<const MachineBasicBlock *, MachineInstr *> EHPadToTry;
+ DenseMap<const MachineBasicBlock *, MachineBasicBlock *>
+ UnwindDestToTrampoline;
+
// We need an appendix block to place 'end_loop' or 'end_try' marker when the
// loop / exception bottom block is the last block in a function
MachineBasicBlock *AppendixBB = nullptr;
@@ -119,11 +128,27 @@ class WebAssemblyCFGStackify final : public MachineFunctionPass {
AppendixBB = MF.CreateMachineBasicBlock();
// Give it a fake predecessor so that AsmPrinter prints its label.
AppendixBB->addSuccessor(AppendixBB);
- MF.push_back(AppendixBB);
+ // If the caller trampoline BB exists, insert the appendix BB before it.
+ // Otherwise insert it at the end of the function.
+ if (CallerTrampolineBB)
+ MF.insert(CallerTrampolineBB->getIterator(), AppendixBB);
+ else
+ MF.push_back(AppendixBB);
}
return AppendixBB;
}
+ // Create a caller-dedicated trampoline BB to be used for fixing unwind
+ // mismatches where the unwind destination is the caller.
+ MachineBasicBlock *CallerTrampolineBB = nullptr;
+ MachineBasicBlock *getCallerTrampolineBlock(MachineFunction &MF) {
+ if (!CallerTrampolineBB) {
+ CallerTrampolineBB = MF.CreateMachineBasicBlock();
+ MF.push_back(CallerTrampolineBB);
+ }
+ return CallerTrampolineBB;
+ }
+
// Before running rewriteDepthImmediates function, 'delegate' has a BB as its
// destination operand. getFakeCallerBlock() returns a fake BB that will be
// used for the operand when 'delegate' needs to rethrow to the caller. This
@@ -691,12 +716,20 @@ void WebAssemblyCFGStackify::placeTryTableMarker(MachineBasicBlock &MBB) {
if (!Header)
return;
- assert(&MBB != &MF.front() && "Header blocks shouldn't have predecessors");
- MachineBasicBlock *LayoutPred = MBB.getPrevNode();
+ // Unlike the end_try marker, we don't place an end marker at the end of
+ // exception bottom, i.e., at the end of the old 'catch' block. But we still
+ // consider the try-catch part as a scope when computing ScopeTops.
+ WebAssemblyException *WE = WEI.getExceptionFor(&MBB);
+ assert(WE);
+ MachineBasicBlock *Bottom = SRI.getBottom(WE);
+ auto Iter = std::next(Bottom->getIterator());
+ if (Iter == MF.end())
+ Iter--;
+ MachineBasicBlock *Cont = &*Iter;
// If the nearest common dominator is inside a more deeply nested context,
// walk out to the nearest scope which isn't more deeply nested.
- for (MachineFunction::iterator I(LayoutPred), E(Header); I != E; --I) {
+ for (MachineFunction::iterator I(Bottom), E(Header); I != E; --I) {
if (MachineBasicBlock *ScopeTop = ScopeTops[I->getNumber()]) {
if (ScopeTop->getNumber() > Header->getNumber()) {
// Skip over an intervening scope.
@@ -905,14 +938,52 @@ void WebAssemblyCFGStackify::placeTryTableMarker(MachineBasicBlock &MBB) {
BuildMI(MBB, InsertPos, MBB.findPrevDebugLoc(InsertPos),
TII.get(WebAssembly::END_BLOCK));
registerScope(Block, EndBlock);
+
// Track the farthest-spanning scope that ends at this point.
- updateScopeTops(Header, &MBB);
+ // Unlike the end_try, even if we don't put a end marker at the end of catch
+ // block, we still have to create two mappings: (BB with 'end_try_table' -> BB
+ // with 'try_table') and (BB after the (conceptual) catch block -> BB with
+ // 'try_table').
+ //
+ // This is what can happen if we don't create the latter mapping:
+ //
+ // Suppoe in the legacy EH we have this code:
+ // try
+ // try
+ // code1
+ // catch (a)
+ // end_try
+ // code2
+ // catch (b)
+ // end_try
+ //
+ // If we don't create the latter mapping, try_table markers would be placed
+ // like this:
+ // try_table
+ // code1
+ // end_try_table (a)
+ // try_table
+ // code2
+ // end_try_table (b)
+ //
+ // This does not reflect the original structure, and more important problem
+ // is, in case 'code1' has an unwind mismatch and should unwind to
+ // 'end_try_table (b)' rather than 'end_try_table (a)', we don't have a way to
+ // make it jump after 'end_try_table (b)' without creating another block. So
+ // even if we don't place 'end_try' marker at the end of 'catch' block
+ // anymore, we create ScopeTops mapping the same way as the legacy exception,
+ // so the resulting code will look like:
+ // try_table
+ // try_table
+ // code1
+ // end_try_table (a)
+ // code2
+ // end_try_table (b)
+ for (auto *End : {&MBB, Cont})
+ updateScopeTops(Header, End);
}
void WebAssemblyCFGStackify::removeUnnecessaryInstrs(MachineFunction &MF) {
- if (WebAssembly::WasmEnableExnref)
- return;
-
const auto &TII = *MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
// When there is an unconditional branch right before a catch instruction and
@@ -1215,7 +1286,291 @@ void WebAssemblyCFGStackify::addNestedTryDelegate(
registerTryScope(Try, Delegate, nullptr);
}
+// Given an unwind destination, return a trampoline BB. A trampoline BB is a
+// destination of a nested try_table inserted to fix an unwind mismatch. It
+// contains an end_block, which is the target of the try_table, and a throw_ref,
+// to rethrow the exception to the right try_table.
+// try_table (catch ... )
+// block exnref
+// ...
+// try_table (catch_all_ref N)
+// some code
+// end_try_table
+// ...
+// end_block ;; Trampoline BB
+// throw_fef
+// end_try_table
+MachineBasicBlock *
+WebAssemblyCFGStackify::getTrampolineBlock(MachineBasicBlock *UnwindDest) {
+ // We need one trampoline BB per an unwind destination, even though there are
+ // multiple try_tables target the same unwind destination. If we have already
+ // created one for the given UnwindDest, return it.
+ auto It = UnwindDestToTrampoline.find(UnwindDest);
+ if (It != UnwindDestToTrampoline.end())
+ return It->second;
+
+ auto &MF = *UnwindDest->getParent();
+ auto &MRI = MF.getRegInfo();
+ const auto &TII = *MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
+
+ MachineInstr *Block = nullptr;
+ MachineBasicBlock *TrampolineBB = nullptr;
+ DebugLoc EndDebugLoc;
+
+ if (UnwindDest == getFakeCallerBlock(MF)) {
+ // If the unwind destination is the caller, create a caller-dedicated
+ // trampoline BB at the end of the function and wrap the whole function with
+ // a block.
+ auto BeginPos = MF.begin()->begin();
+ while (WebAssembly::isArgument(BeginPos->getOpcode()))
+ BeginPos++;
+ Block = BuildMI(*MF.begin(), BeginPos, MF.begin()->begin()->getDebugLoc(),
+ TII.get(WebAssembly::BLOCK))
+ .addImm(int64_t(WebAssembly::BlockType::Exnref));
+ TrampolineBB = getCallerTrampolineBlock(MF);
+ MachineBasicBlock *PrevBB = &*std::prev(CallerTrampolineBB->getIterator());
+ EndDebugLoc = PrevBB->findPrevDebugLoc(PrevBB->end());
+ } else {
+ // If the unwind destination is another EH pad, create a trampoline BB for
+ // the unwind dest and insert a block instruction right after the target
+ // try_table.
+ auto *TargetBeginTry = EHPadToTry[UnwindDest];
+ auto *TargetEndTry = BeginToEnd[TargetBeginTry];
+ auto *TargetBeginBB = TargetBeginTry->getParent();
+ auto *TargetEndBB = TargetEndTry->getParent();
+
+ Block = BuildMI(*TargetBeginBB, std::next(TargetBeginTry->getIterator()),
+ TargetBeginTry->getDebugLoc(), TII.get(WebAssembly::BLOCK))
+ .addImm(int64_t(WebAssembly::BlockType::Exnref));
+ TrampolineBB = MF.CreateMachineBasicBlock();
+ EndDebugLoc = TargetEndTry->getDebugLoc();
+ MF.insert(TargetEndBB->getIterator(), TrampolineBB);
+ TrampolineBB->addSuccessor(UnwindDest);
+ }
+
+ // Insert an end_block, catch_all_ref (pseudo instruction), and throw_ref
+ // instructions in the trampoline BB.
+ MachineInstr *EndBlock =
+ BuildMI(TrampolineBB, EndDebugLoc, TII.get(WebAssembly::END_BLOCK));
+ auto ExnReg = MRI.createVirtualRegister(&WebAssembly::EXNREFRegClass);
+ BuildMI(TrampolineBB, EndDebugLoc, TII.get(WebAssembly::CATCH_ALL_REF))
+ .addDef(ExnReg);
+ BuildMI(TrampolineBB, EndDebugLoc, TII.get(WebAssembly::THROW_REF))
+ .addReg(ExnReg);
+
+ registerScope(Block, EndBlock);
+ UnwindDestToTrampoline[UnwindDest] = TrampolineBB;
+ return TrampolineBB;
+}
+
+// Wrap the given range of instructions with a try_table-end_try_table that
+// targets 'UnwindDest'. RangeBegin and RangeEnd are inclusive.
+void WebAssemblyCFGStackify::addNestedTryTable(MachineInstr *RangeBegin,
+ MachineInstr *RangeEnd,
+ MachineBasicBlock *UnwindDest) {
+ auto *BeginBB = RangeBegin->getParent();
+ auto *EndBB = RangeEnd->getParent();
+
+ MachineFunction &MF = *BeginBB->getParent();
+ const auto &MFI = *MF.getInfo<WebAssemblyFunctionInfo>();
+ const auto &TII = *MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
+
+ // Get the trampoline BB that the new try_table will unwind to.
+ auto *TrampolineBB = getTrampolineBlock(UnwindDest);
+
+ // Local expression tree before the first call of this range should go
+ // after the nested TRY_TABLE.
+ SmallPtrSet<const MachineInstr *, 4> AfterSet;
+ AfterSet.insert(RangeBegin);
+ for (auto I = MachineBasicBlock::iterator(RangeBegin), E = BeginBB->begin();
+ I != E; --I) {
+ if (std::prev(I)->isDebugInstr() || std::prev(I)->isPosition())
+ continue;
+ if (WebAssembly::isChild(*std::prev(I), MFI))
+ AfterSet.insert(&*std::prev(I));
+ else
+ break;
+ }
+
+ // Create the nested try_table instruction.
+ auto TryTablePos = getLatestInsertPos(
+ BeginBB, SmallPtrSet<const MachineInstr *, 4>(), AfterSet);
+ MachineInstr *TryTable =
+ BuildMI(*BeginBB, TryTablePos, RangeBegin->getDebugLoc(),
+ TII.get(WebAssembly::TRY_TABLE))
+ .addImm(int64_t(WebAssembly::BlockType::Void))
+ .addImm(1) // # of catch clauses
+ .addImm(wasm::WASM_OPCODE_CATCH_ALL_REF)
+ .addMBB(TrampolineBB);
+
+ // Create a BB to insert the 'end_try_table' instruction.
+ MachineBasicBlock *EndTryTableBB = MF.CreateMachineBasicBlock();
+ EndTryTableBB->addSuccessor(TrampolineBB);
+
+ auto SplitPos = std::next(RangeEnd->getIterator());
+ if (SplitPos == EndBB->end()) {
+ // If the range's end instruction is at the end of the BB, insert the new
+ // end_try_table BB after the current BB.
+ MF.insert(std::next(EndBB->getIterator()), EndTryTableBB);
+ EndBB->addSuccessor(EndTryTableBB);
+
+ } else {
+ // When the split pos is in the middle of a BB, we split the BB into two and
+ // put the 'end_try_table' BB in between. We normally create a split BB and
+ // make it a successor of the original BB (CatchAfterSplit == false), but in
+ // case the BB is an EH pad and there is a 'catch' after split pos
+ // (CatchAfterSplit == true), we should preserve the BB's property,
+ // including that it is an EH pad, in the later part of the BB, where the
+ // 'catch' is.
+ bool CatchAfterSplit = false;
+ if (EndBB->isEHPad()) {
+ for (auto I = MachineBasicBlock::iterator(SplitPos), E = EndBB->end();
+ I != E; ++I) {
+ if (WebAssembly::isCatch(I->getOpcode())) {
+ CatchAfterSplit = true;
+ break;
+ }
+ }
+ }
+
+ MachineBasicBlock *PreBB = nullptr, *PostBB = nullptr;
+ if (!CatchAfterSplit) {
+ // If the range's end instruction is in the middle of the BB, we split the
+ // BB into two and insert the end_try_table BB in between.
+ // - Before:
+ // bb:
+ // range_end
+ // other_insts
+ //
+ // - After:
+ // pre_bb: (previous 'bb')
+ // range_end
+ // end_try_table_bb: (new)
+ // end_try_table
+ // post_bb: (new)
+ // other_insts
+ PreBB = EndBB;
+ PostBB = MF.CreateMachineBasicBlock();
+ MF.insert(std::next(PreBB->getIterator()), PostBB);
+ MF.insert(std::next(PreBB->getIterator()), EndTryTableBB);
+ PostBB->splice(PostBB->end(), PreBB, SplitPos, PreBB->end());
+ PostBB->transferSuccessors(PreBB);
+ } else {
+ // - Before:
+ // ehpad:
+ // range_end
+ // catch
+ // ...
+ //
+ // - After:
+ // pre_bb: (new)
+ // range_end
+ // end_try_table: (new)
+ // end_try_table
+ // post_bb: (previous 'ehpad')
+ // catch
+ // ...
+ assert(EndBB->isEHPad());
+ PreBB = MF.CreateMachineBasicBlock();
+ PostBB = EndBB;
+ MF.insert(PostBB->getIterator(), PreBB);
+ MF.insert(PostBB->getIterator(), EndTryTableBB);
+ PreBB->splice(PreBB->end(), PostBB, PostBB->begin(), SplitPos);
+ // We don't need to transfer predecessors of the EH pad to 'PreBB',
+ // because an EH pad's predecessors are all through unwind edges and they
+ // should still unwind to the EH pad, not PreBB.
+ }
+ unstackifyVRegsUsedInSplitBB(*PreBB, *PostBB);
+ PreBB->addSuccessor(EndTryTableBB);
+ PreBB->addSuccessor(PostBB);
+ }
+
+ // Add a 'try_table' instruction in the delegate BB created above.
+ MachineInstr *EndTryTable = BuildMI(EndTryTableBB, RangeEnd->getDebugLoc(),
+ TII.get(WebAssembly::END_TRY_TABLE));
+ registerTryScope(TryTable, EndTryTable, nullptr);
+}
+
+// In the standard (exnref) EH, we fix unwind mismatches by adding a new
+// block~end_block inside of the unwind destination try_table~end_try_table:
+// try_table ...
+// block exnref ;; (new)
+// ...
+// try_table (catch_all_ref N) ;; (new) to trampoline BB
+// code
+// end_try_table ;; (new)
+// ...
+// end_block ;; (new) trampoline BB
+// throw_ref ;; (new)
+// end_try_table
+//
+// To do this, we will create a new BB that will contain the new 'end_block' and
+// 'throw_ref' and insert it before the 'end_try_table' BB.
+//
+// But there are cases when there are 'end_loop'(s) before the 'end_try_table'
+// in the same BB. (There can't be 'end_block' before 'end_try_table' in the
+// same BB because EH pads can't be directly branched to.) Then after fixing
+// unwind mismatches this will create the mismatching markers like below:
+// bb0:
+// try_table
+// block exnref
+// ...
+// loop
+// ...
+// new_bb:
+// end_block
+// end_try_table_bb:
+// end_loop
+// end_try_table
+//
+// So if the unwind dest BB has a end_loop before an end_try_table, we split the
+// BB with the end_loop as a separate BB before the end_try_table BB, so that
+// after we fix the unwind mismatch, the code will be like:
+// bb0:
+// try_table
+// block exnref
+// ...
+// loop
+// ...
+// end_loop_bb:
+// end_loop
+// new_bb:
+// end_block
+// end_try_table_bb:
+// end_try_table
+static void splitEndLoopBB(MachineBasicBlock *UnwindDest) {
+ auto &MF = *UnwindDest->getParent();
+ MachineInstr *EndTryTable = nullptr, *EndLoop = nullptr;
+ for (auto &MI : reverse(*UnwindDest)) {
+ if (MI.getOpcode() == WebAssembly::END_TRY_TABLE) {
+ EndTryTable = &MI;
+ continue;
+ }
+ if (EndTryTable && MI.getOpcode() == WebAssembly::END_LOOP) {
+ EndLoop = &MI;
+ break;
+ }
+ }
+ if (!EndLoop)
+ return;
+
+ auto *EndLoopBB = MF.CreateMachineBasicBlock();
+ MF.insert(UnwindDest->getIterator(), EndLoopBB);
+ auto SplitPos = std::next(EndLoop->getIterator());
+ EndLoopBB->splice(EndLoopBB->end(), UnwindDest, UnwindDest->begin(),
+ SplitPos);
+ EndLoopBB->addSuccessor(UnwindDest);
+}
+
bool WebAssemblyCFGStackify::fixCallUnwindMismatches(MachineFunction &MF) {
+ // This function is used for both the legacy EH and the standard (exnref) EH,
+ // and the reason we have unwind mismatches is the same for the both of them,
+ // but the code examples in the comments are going to be different. To make
+ // the description less confusing, we write the basically same comments twice,
+ // once for the legacy EH and the standard EH.
+ //
+ // -- Legacy EH --------------------------------------------------------------
+ //
// Linearizing the control flow by placing TRY / END_TRY markers can create
// mismatches in unwind destinations for throwing instructions, such as calls.
//
@@ -1334,12 +1689,128 @@ bool WebAssemblyCFGStackify::fixCallUnwindMismatches(MachineFunction &MF) {
// couldn't happen, because may-throwing instruction there had an unwind
// destination, i.e., it was an invoke before, and there could be only one
// invoke within a BB.)
+ //
+ // -- Standard EH ------------------------------------------------------------
+ //
+ // Linearizing the control flow by placing TRY / END_TRY_TABLE markers can
+ // create mismatches in unwind destinations for throwing instructions, such as
+ // calls.
+ //
+ // We use the a nested 'try_table'~'end_try_table' instruction to fix the
+ // unwind mismatches. try_table's catch clauses take an immediate argument
+ // that specifics which block we should branch to.
+ //
+ // 1. When an instruction may throw, but the EH pad it will unwind to can be
+ // different from the original CFG.
+ //
+ // Example: we have the following CFG:
+ // bb0:
+ // call @foo ; if it throws, unwind to bb2
+ // bb1:
+ // call @bar ; if it throws, unwind to bb3
+ // bb2 (ehpad):
+ // catch
+ // ...
+ // bb3 (ehpad)
+ // catch
+ // ...
+ //
+ // And the CFG is so...
[truncated]
|
aheejin
added a commit
to aheejin/llvm-project
that referenced
this pull request
Oct 31, 2024
These tests are added to match the standard EH tests in llvm#114361: - nested_try - unwind_mismatches_with_loop These tests are useful to test certain aspects of the new EH but I think they add more coverage to the legaacy tests as well. And `unstackify_when_fixing_unwind_mismatch` and `unwind_mismatches_5` have not changed; they are just moved. This also fixes some comments.
aheejin
added a commit
to aheejin/llvm-project
that referenced
this pull request
Oct 31, 2024
These tests are added to match the standard EH tests in llvm#114361: - nested_try - unwind_mismatches_with_loop These tests are useful to test certain aspects of the new EH but I think they add more coverage to the legaacy tests as well. And `unstackify_when_fixing_unwind_mismatch` and `unwind_mismatches_5` have not changed; they have been just moved. This also fixes some comments.
dschuff
approved these changes
Nov 4, 2024
aheejin
added a commit
that referenced
this pull request
Nov 5, 2024
These tests are added to match the standard EH tests in #114361: - `nested_try` - `unwind_mismatches_with_loop` These tests are useful to test certain aspects of the new EH but I think they add more coverage to the legaacy tests as well. And `unstackify_when_fixing_unwind_mismatch` and `unwind_mismatches_5` have not changed; they have been just moved. This also fixes some comments.
aheejin
added a commit
to aheejin/llvm-project
that referenced
this pull request
Nov 5, 2024
PhilippRados
pushed a commit
to PhilippRados/llvm-project
that referenced
this pull request
Nov 6, 2024
These tests are added to match the standard EH tests in llvm#114361: - `nested_try` - `unwind_mismatches_with_loop` These tests are useful to test certain aspects of the new EH but I think they add more coverage to the legaacy tests as well. And `unstackify_when_fixing_unwind_mismatch` and `unwind_mismatches_5` have not changed; they have been just moved. This also fixes some comments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes unwind mismatches for the new EH spec.
The main flow is similar to that of the legacy EH's unwind mismatch fixing. The new EH shared
fixCallUnwindMismatchesandfixCatchUnwindMismatchesfunctions, which gather the range of instructions we need to fix their unwind destination for, with the legacy EH. But unlike the legacy EH that usestry-delegates to fix them, the new EH wrap those instructions with nestedtry_table-end_try_tables that jump to a "trampoline" BB, where we rethrow (using athrow_ref) the exception to the correcttry_table.For a simple example of a call unwind mismatch, suppose if
call fooshould unwind to the outertry_tablebut is wrapped in anothertry_table(not shown here):Then we wrap the call with a new nested
try_table-end_try_table, add ablock/end_blockright inside the targettry_table, and make the nestedtry_tablejump to it using acatch_all_refclause, and rethrow the exception using athrow_ref:This fixes two existing bugs. These are not easy to test independently without the unwind mismatch fixing. The first one is how we calculate
ScopeTops. Turns out, we should do it in the same way as in the legacy EH even though there is noend_tryat the end ofcatchblock anymore.nested_tryincfg-stackify-eh.lltests this case.The second bug is in
rewriteDepthImmediates.try_table's immediates should be computed without thetry_tableitself, meaningHere 0 should target not
end_try_tablebutend_block. This bug didn't crash the program becauseplaceTryTableMarkergenerated only the simple form oftry_tablethat has a single catch clause and anend_blockfollows right after theend_try_tablein the same BB, so jumping to anend_try_tableis the same as jumping to theend_block. But now we generatecatchclauses with depths greater than 0 with when fixing unwind mismatches, which uncovered this bug.One case that needs a special treatment was when
end_loopprecedes anend_try_tablewithin a BB and this BB is a (true) unwind destination when fixing unwind mismatches. In this case we need to split thisend_loopinto a predecessor BB. This case is tested inunwind_mismatches_with_loopincfg-stackify-eh.ll.cfg-stackify-eh.llcontains mostly the same set of tests with the existingcfg-stackify-eh-legacy.llwith the updated FileCheck expectations. As incfg-stackify-eh-legacy.ll, the FileCheck lines mostly only contain control flow instructions and calls for readability.nested_tryandunwind_mismatches_with_loopare added to test newly found bugs in the new EH.cfg-stackify-eh-legacy.llabout the legacy-EH-specific asepcts have not been added tocfg-stackify-eh.ll. (remove_unnecessary_instrs,remove_unnecessary_br,fix_function_end_return_type_with_try_catch, andbranch_remapping_after_fixing_unwind_mismatches_0/1)