Fix bad CFG construction bug when handling C++ 'try' statements.
This code assigned the last created CFGBlock* to the variable 'Block', which is a scratch variable which is null'ed out after a block is completed. By assigning the last created block to 'Block', we start editing a completed block, inserting CFGStmts that should be in another block. This was the case with 'try'. The test case that showed this had a while loop inside a 'try', and the logic before the while loop was being included as part of the "condition block" for the loop. This showed up as a bogus dead store, but could have lots of implications. Turns out this bug was replicated a few times within CFG.cpp, so I went and fixed up those as well. llvm-svn: 167788
This commit is contained in:
parent
c94c3bb5d0
commit
e6ee671e16
|
@ -1648,8 +1648,10 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) {
|
|||
|
||||
// If the type of VD is a VLA, then we must process its size expressions.
|
||||
for (const VariableArrayType* VA = FindVA(VD->getType().getTypePtr());
|
||||
VA != 0; VA = FindVA(VA->getElementType().getTypePtr()))
|
||||
Block = addStmt(VA->getSizeExpr());
|
||||
VA != 0; VA = FindVA(VA->getElementType().getTypePtr())) {
|
||||
if (CFGBlock *newBlock = addStmt(VA->getSizeExpr()))
|
||||
LastBlock = newBlock;
|
||||
}
|
||||
|
||||
// Remove variable from local scope.
|
||||
if (ScopePos && VD == *ScopePos)
|
||||
|
@ -1767,7 +1769,7 @@ CFGBlock *CFGBuilder::VisitIfStmt(IfStmt *I) {
|
|||
// Add the condition as the last statement in the new block. This may create
|
||||
// new blocks as the condition may contain control-flow. Any newly created
|
||||
// blocks will be pointed to be "Block".
|
||||
Block = addStmt(I->getCond());
|
||||
CFGBlock *LastBlock = addStmt(I->getCond());
|
||||
|
||||
// Finally, if the IfStmt contains a condition variable, add both the IfStmt
|
||||
// and the condition variable initialization to the CFG.
|
||||
|
@ -1775,11 +1777,11 @@ CFGBlock *CFGBuilder::VisitIfStmt(IfStmt *I) {
|
|||
if (Expr *Init = VD->getInit()) {
|
||||
autoCreateBlock();
|
||||
appendStmt(Block, I->getConditionVariableDeclStmt());
|
||||
addStmt(Init);
|
||||
LastBlock = addStmt(Init);
|
||||
}
|
||||
}
|
||||
|
||||
return Block;
|
||||
return LastBlock;
|
||||
}
|
||||
|
||||
|
||||
|
@ -2611,7 +2613,7 @@ CFGBlock *CFGBuilder::VisitSwitchStmt(SwitchStmt *Terminator) {
|
|||
// Add the terminator and condition in the switch block.
|
||||
SwitchTerminatedBlock->setTerminator(Terminator);
|
||||
Block = SwitchTerminatedBlock;
|
||||
Block = addStmt(Terminator->getCond());
|
||||
CFGBlock *LastBlock = addStmt(Terminator->getCond());
|
||||
|
||||
// Finally, if the SwitchStmt contains a condition variable, add both the
|
||||
// SwitchStmt and the condition variable initialization to the CFG.
|
||||
|
@ -2619,11 +2621,11 @@ CFGBlock *CFGBuilder::VisitSwitchStmt(SwitchStmt *Terminator) {
|
|||
if (Expr *Init = VD->getInit()) {
|
||||
autoCreateBlock();
|
||||
appendStmt(Block, Terminator->getConditionVariableDeclStmt());
|
||||
addStmt(Init);
|
||||
LastBlock = addStmt(Init);
|
||||
}
|
||||
}
|
||||
|
||||
return Block;
|
||||
return LastBlock;
|
||||
}
|
||||
|
||||
static bool shouldAddCase(bool &switchExclusivelyCovered,
|
||||
|
@ -2807,8 +2809,7 @@ CFGBlock *CFGBuilder::VisitCXXTryStmt(CXXTryStmt *Terminator) {
|
|||
|
||||
assert(Terminator->getTryBlock() && "try must contain a non-NULL body");
|
||||
Block = NULL;
|
||||
Block = addStmt(Terminator->getTryBlock());
|
||||
return Block;
|
||||
return addStmt(Terminator->getTryBlock());
|
||||
}
|
||||
|
||||
CFGBlock *CFGBuilder::VisitCXXCatchStmt(CXXCatchStmt *CS) {
|
||||
|
@ -2949,15 +2950,15 @@ CFGBlock *CFGBuilder::VisitCXXForRangeStmt(CXXForRangeStmt *S) {
|
|||
addLocalScopeAndDtors(S->getLoopVarStmt());
|
||||
|
||||
// Populate a new block to contain the loop body and loop variable.
|
||||
Block = addStmt(S->getBody());
|
||||
addStmt(S->getBody());
|
||||
if (badCFG)
|
||||
return 0;
|
||||
Block = addStmt(S->getLoopVarStmt());
|
||||
CFGBlock *LoopVarStmtBlock = addStmt(S->getLoopVarStmt());
|
||||
if (badCFG)
|
||||
return 0;
|
||||
|
||||
// This new body block is a successor to our condition block.
|
||||
addSuccessor(ConditionBlock, KnownVal.isFalse() ? 0 : Block);
|
||||
addSuccessor(ConditionBlock, KnownVal.isFalse() ? 0 : LoopVarStmtBlock);
|
||||
}
|
||||
|
||||
// Link up the condition block with the code that follows the loop (the
|
||||
|
|
|
@ -126,3 +126,26 @@ int test_5() {
|
|||
return 1;
|
||||
}
|
||||
|
||||
|
||||
int test_6_aux(unsigned x);
|
||||
|
||||
void test_6() {
|
||||
unsigned currDestLen = 0; // no-warning
|
||||
try {
|
||||
while (test_6_aux(currDestLen)) {
|
||||
currDestLen += 2; // no-warning
|
||||
}
|
||||
}
|
||||
catch (void *) {}
|
||||
}
|
||||
|
||||
void test_6b() {
|
||||
unsigned currDestLen = 0; // no-warning
|
||||
try {
|
||||
while (test_6_aux(currDestLen)) {
|
||||
currDestLen += 2; // expected-warning {{Value stored to 'currDestLen' is never read}}
|
||||
break;
|
||||
}
|
||||
}
|
||||
catch (void *) {}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue