[PATCH] WIP Fix for temporary destructors in conditionals

[Pavel Labath]

  Actually, it turns out that the SymbolReaper is working just fine and needs no
  modifications. So, all I needed to do was bind the values to the conditional
  sub-expressions. I will take one more look at this one Monday, but right now it
  seems that all tests pass and it's working as expected. Could you take a look at
  this?

Hi jordan_rose,

http://llvm-reviews.chandlerc.com/D1259

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D1259?vs=3126&id=3148#toc

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
@@ -1295,14 +1295,14 @@
   return state->getSVal(Ex, LCtx);
 }
 
-static const Stmt *ResolveCondition(const Stmt *Condition,
-                                    const CFGBlock *B) {
+static std::pair<const Stmt *, const Stmt *>
+ResolveCondition(const Stmt *Condition, const Stmt *Term, const CFGBlock *B) {
   if (const Expr *Ex = dyn_cast<Expr>(Condition))
     Condition = Ex->IgnoreParens();
 
   const BinaryOperator *BO = dyn_cast<BinaryOperator>(Condition);
   if (!BO || !BO->isLogicalOp())
-    return Condition;
+    return std::make_pair(Condition, Term);
 
   // For logical operations, we still have the case where some branches
   // use the traditional "merge" approach and others sink the branch
@@ -1320,20 +1320,37 @@
       continue;
     if (CS->getStmt() != Condition)
       break;
-    return Condition;
+    return std::make_pair(Condition, Term);
   }
 
   assert(I != E);
 
   while (Condition) {
     BO = dyn_cast<BinaryOperator>(Condition);
     if (!BO || !BO->isLogicalOp())
-      return Condition;
+      return std::make_pair(Condition, Term);
+    Term = BO;
     Condition = BO->getRHS()->IgnoreParens();
   }
   llvm_unreachable("could not resolve condition");
 }
 
+static ProgramStateRef SaveConditionValue(ProgramStateRef State,
+                                          const LocationContext *LC,
+                                          const Stmt *Condition,
+                                          const Stmt *Term, bool value) {
+  if (const BinaryOperator *BinOp = dyn_cast<BinaryOperator>(Term)) {
+    if ((BinOp->getOpcode() == BO_LOr && value) ||
+        (BinOp->getOpcode() == BO_LAnd && !value) ||
+        BinOp->getRHS()->IgnoreParens() == Condition) {
+      return State->BindExpr(
+          Term, LC,
+          State->getStateManager().getSValBuilder().makeTruthVal(value));
+    }
+  }
+  return State;
+}
+
 void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
                                NodeBuilderContext& BldCtx,
                                ExplodedNode *Pred,
@@ -1351,12 +1368,15 @@
     return;
   }
 
-
-  // Resolve the condition in the precense of nested '||' and '&&'.
   if (const Expr *Ex = dyn_cast<Expr>(Condition))
     Condition = Ex->IgnoreParens();
 
-  Condition = ResolveCondition(Condition, BldCtx.getBlock());
+  // If the value is already available, we don't need to do anything.
+  if(Pred->getState()->getSVal(Condition, Pred->getLocationContext()).isUnknownOrUndef()) {
+    // Resolve the condition in the precense of nested '||' and '&&'.
+    llvm::tie(Condition, Term) = ResolveCondition(Condition, Term, BldCtx.getBlock());
+  }
+
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 Condition->getLocStart(),
                                 "Error evaluating branch");
@@ -1413,17 +1433,19 @@
 
     // Process the true branch.
     if (builder.isFeasible(true)) {
-      if (StTrue)
+      if (StTrue) {
+        StTrue = SaveConditionValue(StTrue, BldCtx.LC, Condition, Term, true);
         builder.generateNode(StTrue, true, PredI);
-      else
+      } else
         builder.markInfeasible(true);
     }
 
     // Process the false branch.
     if (builder.isFeasible(false)) {
-      if (StFalse)
+      if (StFalse) {
+        StFalse = SaveConditionValue(StFalse, BldCtx.LC, Condition, Term, false);
         builder.generateNode(StFalse, false, PredI);
-      else
+      } else
         builder.markInfeasible(false);
     }
   }
Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -135,9 +135,7 @@
     if (i != 5)
       return;
     if (i == 5 && (i == 4 || check(NoReturnDtor()) || i == 5)) {
-      // FIXME: Should be no-warning, because the noreturn destructor should
-      // fire on all paths.
-      clang_analyzer_eval(true); // expected-warning{{TRUE}}
+      clang_analyzer_eval(true); // no warning
     }
   }
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1259.2.patch
Type: text/x-patch
Size: 4239 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130802/fed5cf73/attachment.bin>