[PATCH] Fix crash when resolving branch conditions for temporary destructor condition blocks.

Manuel Klimek klimek at google.com
Wed Apr 30 01:31:12 PDT 2014


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1355
@@ -1354,1 +1354,3 @@
 
+const Stmt* GetRightmostLeaf(const Stmt* Condition) {
+  while (Condition) {
----------------
Jordan Rose wrote:
> Please follow the new LLVM style with lowercase function names. This can also be static. (And the asterisks go on the right of the space.)
Done.

================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1392
@@ +1391,3 @@
+  // only have a terminator (and no statements). These blocks violate the
+  // invariant this funcion assumes.
+  if (B->getTerminator().isTemporaryDtorsBranch())
----------------
Alex McCarthy wrote:
> funcion -> function
Done.

================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1410
@@ -1377,13 +1409,3 @@
       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 clang::Stmt* LastStmt = CS->getStmt();
+    assert(LastStmt == Condition || LastStmt == GetRightmostLeaf(Condition));
----------------
Jordan Rose wrote:
> Style nitpicks: no "clang::", asterisk next to the variable name.
Done.

================
Comment at: test/Analysis/temporaries.cpp:193
@@ -194,3 +192,3 @@
 
-    FIXME: This shouldn't cause a warning.
-    if (compute(i == 5 &&
+    if (check(NoReturnDtor()) &&
+        compute(i == 5 &&
----------------
Jordan Rose wrote:
> Alex McCarthy wrote:
> > Out of curiosity, why the change to this test case?
> Seconded; all the existing tests are designed to tickle past bugs, and I'd rather not change any of the conditions.
Good catch - that was a leftover from debugging, I didn't intend to change it.
Changed the test back (it still breaks, so I added the wrong expectation and a FIXME).

http://reviews.llvm.org/D3552






More information about the cfe-commits mailing list