[cfe-commits] r139586 - in /cfe/trunk: INPUTS/cfg-nested-var-scopes.cpp include/clang/Analysis/CFG.h lib/Analysis/CFG.cpp test/SemaCXX/return-noreturn.cpp
Chandler Carruth
chandlerc at gmail.com
Mon Sep 12 23:09:01 PDT 2011
Author: chandlerc
Date: Tue Sep 13 01:09:01 2011
New Revision: 139586
URL: http://llvm.org/viewvc/llvm-project?rev=139586&view=rev
Log:
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.
Added:
cfe/trunk/INPUTS/cfg-nested-var-scopes.cpp
Modified:
cfe/trunk/include/clang/Analysis/CFG.h
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/test/SemaCXX/return-noreturn.cpp
Added: cfe/trunk/INPUTS/cfg-nested-var-scopes.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/INPUTS/cfg-nested-var-scopes.cpp?rev=139586&view=auto
==============================================================================
--- cfe/trunk/INPUTS/cfg-nested-var-scopes.cpp (added)
+++ cfe/trunk/INPUTS/cfg-nested-var-scopes.cpp Tue Sep 13 01:09:01 2011
@@ -0,0 +1,59 @@
+// Hammer the CFG with large numbers of overlapping variable scopes, which
+// implicit destructors triggered at each edge.
+
+#define EXPAND_BASIC_STRUCT(i) struct X##i { X##i(int); ~X##i(); };
+#define EXPAND_NORET_STRUCT(i) struct X##i { X##i(int); ~X##i() __attribute__((noreturn)); };
+EXPAND_BASIC_STRUCT(0000); EXPAND_NORET_STRUCT(0001);
+EXPAND_BASIC_STRUCT(0010); EXPAND_BASIC_STRUCT(0011);
+EXPAND_BASIC_STRUCT(0100); EXPAND_NORET_STRUCT(0101);
+EXPAND_NORET_STRUCT(0110); EXPAND_BASIC_STRUCT(0111);
+EXPAND_BASIC_STRUCT(1000); EXPAND_NORET_STRUCT(1001);
+EXPAND_BASIC_STRUCT(1010); EXPAND_BASIC_STRUCT(1011);
+EXPAND_NORET_STRUCT(1100); EXPAND_NORET_STRUCT(1101);
+EXPAND_BASIC_STRUCT(1110); EXPAND_BASIC_STRUCT(1111);
+
+#define EXPAND_2_VARS(c, i, x) const X##i var_##c##_##i##0(x), &var_##c##_##i##1 = X##i(x)
+#define EXPAND_4_VARS(c, i, x) EXPAND_2_VARS(c, i##0, x); EXPAND_2_VARS(c, i##1, x)
+#define EXPAND_8_VARS(c, i, x) EXPAND_4_VARS(c, i##0, x); EXPAND_4_VARS(c, i##1, x)
+#define EXPAND_16_VARS(c, i, x) EXPAND_8_VARS(c, i##0, x); EXPAND_8_VARS(c, i##1, x)
+#define EXPAND_32_VARS(c, x) EXPAND_16_VARS(c, 0, x); EXPAND_16_VARS(c, 1, x)
+
+#define EXPAND_2_INNER_CASES(i, x, y) INNER_CASE(i, x, y); INNER_CASE(i + 1, x, y);
+#define EXPAND_4_INNER_CASES(i, x, y) EXPAND_2_INNER_CASES(i, x, y) EXPAND_2_INNER_CASES(i + 2, x, y)
+#define EXPAND_8_INNER_CASES(i, x, y) EXPAND_4_INNER_CASES(i, x, y) EXPAND_4_INNER_CASES(i + 4, x, y)
+#define EXPAND_16_INNER_CASES(i, x, y) EXPAND_8_INNER_CASES(i, x, y) EXPAND_8_INNER_CASES(i + 8, x, y)
+#define EXPAND_32_INNER_CASES(i, x, y) EXPAND_16_INNER_CASES(i, x, y) EXPAND_16_INNER_CASES(i + 16, x, y)
+
+#define EXPAND_2_OUTER_CASES(i, x, y) OUTER_CASE(i, x, y); OUTER_CASE(i + 1, x, y);
+#define EXPAND_4_OUTER_CASES(i, x, y) EXPAND_2_OUTER_CASES(i, x, y) EXPAND_2_OUTER_CASES(i + 2, x, y)
+#define EXPAND_8_OUTER_CASES(i, x, y) EXPAND_4_OUTER_CASES(i, x, y) EXPAND_4_OUTER_CASES(i + 4, x, y)
+#define EXPAND_16_OUTER_CASES(i, x, y) EXPAND_8_OUTER_CASES(i, x, y) EXPAND_8_OUTER_CASES(i + 8, x, y)
+#define EXPAND_32_OUTER_CASES(i, x, y) EXPAND_16_OUTER_CASES(i, x, y) EXPAND_16_OUTER_CASES(i + 16, x, y)
+
+unsigned cfg_nested_vars(int x) {
+ int y = 0;
+ while (x > 0) {
+ EXPAND_32_VARS(a, x);
+ switch (x) {
+#define INNER_CASE(i, x, y) \
+ case i: { \
+ int case_var = 3*x + i; \
+ EXPAND_32_VARS(c, case_var); \
+ y += case_var - 1; \
+ break; \
+ }
+#define OUTER_CASE(i, x, y) \
+ case i: { \
+ int case_var = y >> 8; \
+ EXPAND_32_VARS(b, y); \
+ switch (case_var) { \
+ EXPAND_32_INNER_CASES(0, x, y); \
+ } \
+ break; \
+ }
+EXPAND_32_OUTER_CASES(0, x, y);
+ }
+ --x;
+ }
+ return y;
+}
Modified: cfe/trunk/include/clang/Analysis/CFG.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CFG.h?rev=139586&r1=139585&r2=139586&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/CFG.h (original)
+++ cfe/trunk/include/clang/Analysis/CFG.h Tue Sep 13 01:09:01 2011
@@ -506,6 +506,10 @@
Elements.push_back(CFGTemporaryDtor(E), C);
}
+ void appendAutomaticObjDtor(VarDecl *VD, Stmt *S, BumpVectorContext &C) {
+ Elements.push_back(CFGAutomaticObjDtor(VD, S), C);
+ }
+
// Destructors must be inserted in reversed order. So insertion is in two
// steps. First we prepare space for some number of elements, then we insert
// the elements beginning at the last position in prepared space.
Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=139586&r1=139585&r2=139586&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Tue Sep 13 01:09:01 2011
@@ -413,11 +413,10 @@
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 @@
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 @@
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 @@
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;
}
Modified: cfe/trunk/test/SemaCXX/return-noreturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/return-noreturn.cpp?rev=139586&r1=139585&r2=139586&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/return-noreturn.cpp (original)
+++ cfe/trunk/test/SemaCXX/return-noreturn.cpp Tue Sep 13 01:09:01 2011
@@ -8,6 +8,8 @@
~pr6884_abort_struct() __attribute__((noreturn)) { pr6884_abort(); }
};
+struct other { ~other() {} };
+
// Ensure that destructors from objects are properly modeled in the CFG despite
// the presence of switches, case statements, labels, and blocks. These tests
// try to cover bugs reported in both PR6884 and PR10063.
@@ -37,9 +39,63 @@
switch (x) default: L1: L2: case 4: { pr6884_abort_struct(); }
}
- int h(int x) {
+ // Test that these constructs work even when extraneous blocks are created
+ // before and after the switch due to implicit destructors.
+ int g1(int x) {
+ other o;
+ switch (x) default: pr6884_abort_struct();
+ }
+ int g2(int x) {
+ other o;
+ switch (x) { default: pr6884_abort_struct(); }
+ }
+ int g2_positive(int x) {
+ other o;
+ switch (x) { default: ; }
+ } // expected-warning {{control reaches end of non-void function}}
+ int g3(int x) {
+ other o;
+ switch (x) { default: { pr6884_abort_struct(); } }
+ }
+ int g4(int x) {
+ other o;
+ switch (x) default: L1: L2: case 4: pr6884_abort_struct();
+ }
+ int g5(int x) {
+ other o;
+ switch (x) default: L1: { L2: case 4: pr6884_abort_struct(); }
+ }
+ int g6(int x) {
+ other o;
+ switch (x) default: L1: L2: case 4: { pr6884_abort_struct(); }
+ }
+
+ // Test that these constructs work even with variables carrying the no-return
+ // destructor instead of temporaries.
+ int h1(int x) {
+ other o;
+ switch (x) default: pr6884_abort_struct a;
+ }
+ int h2(int x) {
+ other o;
+ switch (x) { default: pr6884_abort_struct a; }
+ }
+ int h3(int x) {
+ other o;
switch (x) { default: { pr6884_abort_struct a; } }
}
+ int h4(int x) {
+ other o;
+ switch (x) default: L1: L2: case 4: pr6884_abort_struct a;
+ }
+ int h5(int x) {
+ other o;
+ switch (x) default: L1: { L2: case 4: pr6884_abort_struct a; }
+ }
+ int h6(int x) {
+ other o;
+ switch (x) default: L1: L2: case 4: { pr6884_abort_struct a; }
+ }
}
// PR9380
More information about the cfe-commits
mailing list