[cfe-commits] r167788 - in /cfe/trunk: lib/Analysis/CFG.cpp test/Analysis/dead-stores.cpp
Ted Kremenek
kremenek at apple.com
Mon Nov 12 16:12:13 PST 2012
Author: kremenek
Date: Mon Nov 12 18:12:13 2012
New Revision: 167788
URL: http://llvm.org/viewvc/llvm-project?rev=167788&view=rev
Log:
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.
Modified:
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/test/Analysis/dead-stores.cpp
Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=167788&r1=167787&r2=167788&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Mon Nov 12 18:12:13 2012
@@ -1648,8 +1648,10 @@
// 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 @@
// 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 @@
if (Expr *Init = VD->getInit()) {
autoCreateBlock();
appendStmt(Block, I->getConditionVariableDeclStmt());
- addStmt(Init);
+ LastBlock = addStmt(Init);
}
}
- return Block;
+ return LastBlock;
}
@@ -2611,7 +2613,7 @@
// 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 @@
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 @@
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 @@
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
Modified: cfe/trunk/test/Analysis/dead-stores.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dead-stores.cpp?rev=167788&r1=167787&r2=167788&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/dead-stores.cpp (original)
+++ cfe/trunk/test/Analysis/dead-stores.cpp Mon Nov 12 18:12:13 2012
@@ -126,3 +126,26 @@
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 *) {}
+}
More information about the cfe-commits
mailing list