diff options
author | Chandler Carruth <chandlerc@gmail.com> | 2011-09-13 06:09:01 +0000 |
---|---|---|
committer | Chandler Carruth <chandlerc@gmail.com> | 2011-09-13 06:09:01 +0000 |
commit | c8cfc74bdcc999828bc232294d937fb191940d5b (patch) | |
tree | 7a8b66ff927fe4f6e10e4d1610dc1537fa6ac4cd /lib/Analysis/CFG.cpp | |
parent | 6c11f0b3106263600af2ea0438ebb372821514aa (diff) |
Enhance the CFG construction to detect no-return destructors for
temporary objects and local variables. When detected, these split the
block, marking the new one as having only the exit block as a successor.
This prevents a large number of false positives in warnings sensitive to
no-return constructs such as -Wreturn-type, and fixes the remainder of
PR10063 along with several variations of this bug that had not been
reported. The test cases are extended across the board to cover these
patterns.
This also checks in a stress test for these types of CFGs. The stress
test declares some 32k variables, a mixture of no-return and normal
destructors. Previously, this resulted in roughly 2500 CFG blocks, but
didn't model any of the no-return destructors. With this patch, it
results in over 33k blocks, many of them now unreachable.
The nice thing about how the analyzer is set up? This causes *no*
regression in performance of building the CFG. It actually in some cases
makes it faster, as best I can benchmark. The analysis for -Wreturn-type
(and any other that cares about no-return code paths) is technically
slower now as it has to look at many more candidate blocks, but it
computes the correct answer. I have more test cases to follow, I think
they all work now. Also I have further work that should dramatically
simplify analyses in the presence of no-return.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@139586 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'lib/Analysis/CFG.cpp')
-rw-r--r-- | lib/Analysis/CFG.cpp | 89 |
1 files changed, 61 insertions, 28 deletions
diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp index d385420a56..69399f3bcd 100644 --- a/lib/Analysis/CFG.cpp +++ b/lib/Analysis/CFG.cpp @@ -413,11 +413,10 @@ private: void appendTemporaryDtor(CFGBlock *B, CXXBindTemporaryExpr *E) { B->appendTemporaryDtor(E, cfg->getBumpVectorContext()); } + void appendAutomaticObjDtor(CFGBlock *B, VarDecl *VD, Stmt *S) { + B->appendAutomaticObjDtor(VD, S, cfg->getBumpVectorContext()); + } - void insertAutomaticObjDtors(CFGBlock *Blk, CFGBlock::iterator I, - LocalScope::const_iterator B, LocalScope::const_iterator E, Stmt *S); - void appendAutomaticObjDtors(CFGBlock *Blk, LocalScope::const_iterator B, - LocalScope::const_iterator E, Stmt *S); void prependAutomaticObjDtorsWithTerminator(CFGBlock *Blk, LocalScope::const_iterator B, LocalScope::const_iterator E); @@ -647,8 +646,40 @@ void CFGBuilder::addAutomaticObjDtors(LocalScope::const_iterator B, if (B == E) return; - autoCreateBlock(); - appendAutomaticObjDtors(Block, B, E, S); + CFGBlock::iterator InsertPos; + + // We need to append the destructors in reverse order, but any one of them + // may be a no-return destructor which changes the CFG. As a result, buffer + // this sequence up and replay them in reverse order when appending onto the + // CFGBlock(s). + SmallVector<VarDecl*, 10> Decls; + Decls.reserve(B.distance(E)); + for (LocalScope::const_iterator I = B; I != E; ++I) + Decls.push_back(*I); + + for (SmallVectorImpl<VarDecl*>::reverse_iterator I = Decls.rbegin(), + E = Decls.rend(); + I != E; ++I) { + // If this destructor is marked as a no-return destructor, we need to + // create a new block for the destructor which does not have as a successor + // anything built thus far: control won't flow out of this block. + QualType Ty = (*I)->getType().getNonReferenceType(); + if (const ArrayType *AT = Context->getAsArrayType(Ty)) + Ty = AT->getElementType(); + const CXXDestructorDecl *Dtor = Ty->getAsCXXRecordDecl()->getDestructor(); + if (cast<FunctionType>(Dtor->getType())->getNoReturnAttr()) { + Block = createBlock(/*add_successor=*/false); + // Wire up this block directly to the exit block if we're in the + // no-return case. We pruned any other successors because control flow + // won't actually exit this block, but we want to be able to find all of + // these entries in the CFG when doing analyses. + addSuccessor(Block, &cfg->getExit()); + } else { + autoCreateBlock(); + } + + appendAutomaticObjDtor(Block, *I, S); + } } /// addImplicitDtorsForDestructor - Add implicit destructors generated for @@ -807,34 +838,21 @@ void CFGBuilder::addLocalScopeAndDtors(Stmt *S) { addAutomaticObjDtors(ScopePos, scopeBeginPos, S); } -/// insertAutomaticObjDtors - Insert destructor CFGElements for variables with -/// automatic storage duration to CFGBlock's elements vector. Insertion will be -/// performed in place specified with iterator. -void CFGBuilder::insertAutomaticObjDtors(CFGBlock *Blk, CFGBlock::iterator I, - LocalScope::const_iterator B, LocalScope::const_iterator E, Stmt *S) { - BumpVectorContext &C = cfg->getBumpVectorContext(); - I = Blk->beginAutomaticObjDtorsInsert(I, B.distance(E), C); - while (B != E) - I = Blk->insertAutomaticObjDtor(I, *B++, S); -} - -/// appendAutomaticObjDtors - Append destructor CFGElements for variables with -/// automatic storage duration to CFGBlock's elements vector. Elements will be -/// appended to physical end of the vector which happens to be logical -/// beginning. -void CFGBuilder::appendAutomaticObjDtors(CFGBlock *Blk, - LocalScope::const_iterator B, LocalScope::const_iterator E, Stmt *S) { - insertAutomaticObjDtors(Blk, Blk->begin(), B, E, S); -} - /// prependAutomaticObjDtorsWithTerminator - Prepend destructor CFGElements for /// variables with automatic storage duration to CFGBlock's elements vector. /// Elements will be prepended to physical beginning of the vector which /// happens to be logical end. Use blocks terminator as statement that specifies /// destructors call site. +/// FIXME: This mechanism for adding automatic destructors doesn't handle +/// no-return destructors properly. void CFGBuilder::prependAutomaticObjDtorsWithTerminator(CFGBlock *Blk, LocalScope::const_iterator B, LocalScope::const_iterator E) { - insertAutomaticObjDtors(Blk, Blk->end(), B, E, Blk->getTerminator()); + BumpVectorContext &C = cfg->getBumpVectorContext(); + CFGBlock::iterator InsertPos + = Blk->beginAutomaticObjDtorsInsert(Blk->end(), B.distance(E), C); + for (LocalScope::const_iterator I = B; I != E; ++I) + InsertPos = Blk->insertAutomaticObjDtor(InsertPos, *I, + Blk->getTerminator()); } /// Visit - Walk the subtree of a statement and add extra @@ -2861,7 +2879,22 @@ CFGBlock *CFGBuilder::VisitCXXBindTemporaryExprForTemporaryDtors( if (!BindToTemporary) { // If lifetime of temporary is not prolonged (by assigning to constant // reference) add destructor for it. - autoCreateBlock(); + + // If the destructor is marked as a no-return destructor, we need to create + // a new block for the destructor which does not have as a successor + // anything built thus far. Control won't flow out of this block. + const CXXDestructorDecl *Dtor = E->getTemporary()->getDestructor(); + if (cast<FunctionType>(Dtor->getType())->getNoReturnAttr()) { + Block = createBlock(/*add_successor=*/false); + // Wire up this block directly to the exit block if we're in the + // no-return case. We pruned any other successors because control flow + // won't actually exit this block, but we want to be able to find all of + // these entries in the CFG when doing analyses. + addSuccessor(Block, &cfg->getExit()); + } else { + autoCreateBlock(); + } + appendTemporaryDtor(Block, E); B = Block; } |