Review comment changes.
http://reviews.llvm.org/D3552
Files:
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/temporaries.cpp
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1352,6 +1352,31 @@
return state->getSVal(Ex, LCtx);
}
+const Stmt *getRightmostLeaf(const Stmt *Condition) {
+ while (Condition) {
+ const BinaryOperator *BO = dyn_cast<BinaryOperator>(Condition);
+ if (!BO || !BO->isLogicalOp()) {
+ return Condition;
+ }
+ Condition = BO->getRHS()->IgnoreParens();
+ }
+ return nullptr;
+}
+
+// Returns the condition the branch at the end of 'B' depends on and whose value
+// has been evaluated within 'B'.
+// In most cases, the terminator condition of 'B' will be evaluated fully in
+// the last statement of 'B'; in those cases, the resolved condition is the
+// given 'Condition'.
+// If the condition of the branch is a logical binary operator tree, the CFG is
+// optimized: in that case, we know that the expression formed by all but the
+// rightmost leaf of the logical binary operator tree must be true, and thus
+// the branch condition is at this point equivalent to the truth value of that
+// rightmost leaf; the CFG block thus only evaluates this rightmost leaf
+// expression in its final statement. As the full condition in that case was
+// not evaluated, and is thus not in the SVal cache, we need to use that leaf
+// expression to evaluate the truth value of the condition in the current state
+// space.
static const Stmt *ResolveCondition(const Stmt *Condition,
const CFGBlock *B) {
if (const Expr *Ex = dyn_cast<Expr>(Condition))
@@ -1361,6 +1386,12 @@
if (!BO || !BO->isLogicalOp())
return Condition;
+ // FIXME: This is a workaround until we handle temporary destructor branches
+ // correctly; currently, temporary destructor branches lead to blocks that
+ // only have a terminator (and no statements). These blocks violate the
+ // invariant this function assumes.
+ if (B->getTerminator().isTemporaryDtorsBranch()) return Condition;
+
// For logical operations, we still have the case where some branches
// use the traditional "merge" approach and others sink the branch
// directly into the basic blocks representing the logical operation.
@@ -1375,18 +1406,9 @@
Optional<CFGStmt> CS = Elem.getAs<CFGStmt>();
if (!CS)
continue;
- if (CS->getStmt() != Condition)
- break;
- return Condition;
- }
-
- assert(I != E);
-
- while (Condition) {
- BO = dyn_cast<BinaryOperator>(Condition);
- if (!BO || !BO->isLogicalOp())
- return Condition;
- Condition = BO->getRHS()->IgnoreParens();
+ const Stmt *LastStmt = CS->getStmt();
+ assert(LastStmt == Condition || LastStmt == getRightmostLeaf(Condition));
+ return LastStmt;
}
llvm_unreachable("could not resolve condition");
}
Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -118,13 +118,11 @@
extern bool coin();
extern bool check(const Dtor &);
-#ifndef TEMPORARY_DTORS
- // FIXME: Don't assert here when tmp dtors are enabled.
+ // Regression test: we used to assert here when tmp dtors are enabled.
// PR16664 and PR18159
if (coin() && (coin() || coin() || check(Dtor()))) {
Dtor();
}
-#endif
}
#ifdef TEMPORARY_DTORS
@@ -170,36 +168,35 @@
clang_analyzer_eval(true); // no warning, unreachable code
}
-/*
+ // Regression test: we used to assert here.
// PR16664 and PR18159
- FIXME: Don't assert here.
void testConsistencyNested(int i) {
extern bool compute(bool);
-
+
if (i == 5 && (i == 4 || i == 5 || check(NoReturnDtor())))
- clang_analyzer_eval(true); // expected TRUE
-
+ clang_analyzer_eval(true); // expected-warning{{TRUE}}
+
if (i == 5 && (i == 4 || i == 5 || check(NoReturnDtor())))
- clang_analyzer_eval(true); // expected TRUE
+ clang_analyzer_eval(true); // expected-warning{{TRUE}}
if (i != 5)
return;
if (compute(i == 5 &&
(i == 4 || compute(true) ||
compute(i == 5 && (i == 4 || check(NoReturnDtor()))))) ||
i != 4) {
- clang_analyzer_eval(true); // expected TRUE
+ clang_analyzer_eval(true); // expected-warning{{TRUE}}
}
- FIXME: This shouldn't cause a warning.
if (compute(i == 5 &&
(i == 4 || i == 4 ||
compute(i == 5 && (i == 4 || check(NoReturnDtor()))))) ||
i != 4) {
- clang_analyzer_eval(true); // no warning, unreachable code
+ // FIXME: This shouldn't cause a warning.
+ clang_analyzer_eval(true); // expected-warning{{TRUE}}
}
- }*/
+ }
// PR16664 and PR18159
void testConsistencyNestedSimple(bool value) {
@@ -229,6 +226,14 @@
}
}
+ void testBinaryOperatorShortcut(bool value) {
+ if (value) {
+ if (false && false && check(NoReturnDtor()) && true) {
+ clang_analyzer_eval(true);
+ }
+ }
+ }
+
#endif // TEMPORARY_DTORS
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
