[PATCH] WIP Fix for temporary destructors in conditionals

[Pavel Labath]

labath added you to the CC list for the revision "WIP Fix for temporary destructors in conditionals".

Hi jordan_rose,

Not really intended for submission (yet), merely a proof of concept.

The change in Environment.cpp is just a hack to make every expression "live". It
should go away once we teach SymbolReaper to leave our conditionals alone.

The important part is in ExprEngine. Here, I store computed value of the
conditional expression so that I don't have to recompute it when I visit it for
the second time.

This change, together with the reverted temporary destructors
patch makes the analyzer work correctly on the code from bug #16664.

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

Files:
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: lib/StaticAnalyzer/Core/Environment.cpp
===================================================================
--- lib/StaticAnalyzer/Core/Environment.cpp
+++ lib/StaticAnalyzer/Core/Environment.cpp
@@ -165,7 +165,8 @@
     const EnvironmentEntry &BlkExpr = I.getKey();
     const SVal &X = I.getData();
 
-    if (SymReaper.isLive(BlkExpr.getStmt(), BlkExpr.getLocationContext())) {
+    if (SymReaper.isLive(BlkExpr.getStmt(), BlkExpr.getLocationContext()) ||
+        true) {
       // Copy the binding to the new map.
       EBMapRef = EBMapRef.add(BlkExpr, X);
 
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1351,12 +1351,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 '&&'.
+    Condition = ResolveCondition(Condition, BldCtx.getBlock());
+  }
+
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 Condition->getLocStart(),
                                 "Error evaluating branch");
@@ -1413,17 +1416,23 @@
 
     // Process the true branch.
     if (builder.isFeasible(true)) {
-      if (StTrue)
+      if (StTrue) {
+        StTrue = StTrue->BindExpr(
+            Term, BldCtx.LC,
+            StTrue->getStateManager().getSValBuilder().makeTruthVal(true));
         builder.generateNode(StTrue, true, PredI);
-      else
+      } else
         builder.markInfeasible(true);
     }
 
     // Process the false branch.
     if (builder.isFeasible(false)) {
-      if (StFalse)
+      if (StFalse) {
+        StFalse = StFalse->BindExpr(
+            Term, BldCtx.LC,
+            StFalse->getStateManager().getSValBuilder().makeTruthVal(false));
         builder.generateNode(StFalse, false, PredI);
-      else
+      } else
         builder.markInfeasible(false);
     }
   }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1259.1.patch
Type: text/x-patch
Size: 2331 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130801/5f5c6625/attachment.bin>