From 10f01bd4d3aca9472b792b3ded3e6bf75ae90a7b Mon Sep 17 00:00:00 2001 From: Chen Li Date: Tue, 11 Aug 2015 18:12:26 +0000 Subject: [LowerSwitch] Fix a bug when LowerSwitch deletes the default block Summary: LowerSwitch crashed with the attached test case after deleting the default block. This happened because the current implementation of deleting dead blocks is wrong. After the default block being deleted, it contains no instruction or terminator, and it should no be traversed anymore. However, since the iterator is advanced before processSwitchInst() function is executed, the block advanced to could be deleted inside processSwitchInst(). The deleted block would then be visited next and crash dyn_cast(Cur->getTerminator()) because Cur->getTerminator() returns a nullptr. This patch fixes this problem by recording dead default blocks into a list, and delete them after all processSwitchInst() has been done. It still possible to visit dead default blocks and waste time process them. But it is a compile time issue, and I plan to have another patch to add support to skip dead blocks. Reviewers: kariddi, resistor, hans, reames Subscribers: llvm-commits Differential Revision: http://reviews.llvm.org/D11852 llvm-svn: 244642 --- llvm/lib/Transforms/Utils/LowerSwitch.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'llvm/lib/Transforms/Utils/LowerSwitch.cpp') diff --git a/llvm/lib/Transforms/Utils/LowerSwitch.cpp b/llvm/lib/Transforms/Utils/LowerSwitch.cpp index 3b447098af1..0e47626ce53 100644 --- a/llvm/lib/Transforms/Utils/LowerSwitch.cpp +++ b/llvm/lib/Transforms/Utils/LowerSwitch.cpp @@ -78,7 +78,7 @@ namespace { typedef std::vector CaseVector; typedef std::vector::iterator CaseItr; private: - void processSwitchInst(SwitchInst *SI); + void processSwitchInst(SwitchInst *SI, SmallVectorImpl &DeleteList); BasicBlock *switchConvert(CaseItr Begin, CaseItr End, ConstantInt *LowerBound, ConstantInt *UpperBound, @@ -116,16 +116,21 @@ FunctionPass *llvm::createLowerSwitchPass() { bool LowerSwitch::runOnFunction(Function &F) { bool Changed = false; + SmallVector DeleteList; for (Function::iterator I = F.begin(), E = F.end(); I != E; ) { BasicBlock *Cur = I++; // Advance over block so we don't traverse new blocks if (SwitchInst *SI = dyn_cast(Cur->getTerminator())) { Changed = true; - processSwitchInst(SI); + processSwitchInst(SI, DeleteList); } } + for (BasicBlock* BB: DeleteList) { + DeleteDeadBlock(BB); + } + return Changed; } @@ -397,7 +402,7 @@ unsigned LowerSwitch::Clusterify(CaseVector& Cases, SwitchInst *SI) { // processSwitchInst - Replace the specified switch instruction with a sequence // of chained if-then insts in a balanced binary search. // -void LowerSwitch::processSwitchInst(SwitchInst *SI) { +void LowerSwitch::processSwitchInst(SwitchInst *SI, SmallVectorImpl &DeleteList) { BasicBlock *CurBlock = SI->getParent(); BasicBlock *OrigBlock = CurBlock; Function *F = CurBlock->getParent(); @@ -518,7 +523,7 @@ void LowerSwitch::processSwitchInst(SwitchInst *SI) { BasicBlock *OldDefault = SI->getDefaultDest(); CurBlock->getInstList().erase(SI); - // If the Default block has no more predecessors just remove it. + // If the Default block has no more predecessors just add it to DeleteList. if (pred_begin(OldDefault) == pred_end(OldDefault)) - DeleteDeadBlock(OldDefault); + DeleteList.push_back(OldDefault); } -- cgit v1.2.3