[llvm-branch-commits] [cfe-branch] r196795 - Merging r196593:

Bill Wendling isanbard at gmail.com
Mon Dec 9 10:37:01 PST 2013


Author: void
Date: Mon Dec  9 12:37:00 2013
New Revision: 196795

URL: http://llvm.org/viewvc/llvm-project?rev=196795&view=rev
Log:
Merging r196593:
------------------------------------------------------------------------
r196593 | zaks | 2013-12-06 10:56:29 -0800 (Fri, 06 Dec 2013) | 7 lines

Revert "[analyzer] Refactor conditional expression evaluating code"

This reverts commit r189090.

The original patch introduced regressions (see the added live-variables.* tests). The patch depends on the correctness of live variable analyses, which are not computed correctly. I've opened PR18159 to track the proper resolution to this problem.

The patch was a stepping block to r189746. This is why part of the patch reverts temporary destructor tests that started crashing. The temporary destructors feature is disabled by default.
------------------------------------------------------------------------

Added:
    cfe/branches/release_34/test/Analysis/live-variables.cpp
      - copied unchanged from r196593, cfe/trunk/test/Analysis/live-variables.cpp
    cfe/branches/release_34/test/Analysis/live-variables.m
      - copied unchanged from r196593, cfe/trunk/test/Analysis/live-variables.m
Modified:
    cfe/branches/release_34/   (props changed)
    cfe/branches/release_34/lib/Analysis/LiveVariables.cpp
    cfe/branches/release_34/lib/StaticAnalyzer/Core/ExprEngine.cpp
    cfe/branches/release_34/lib/StaticAnalyzer/Core/ExprEngineC.cpp
    cfe/branches/release_34/test/Analysis/MismatchedDeallocator-checker-test.mm   (props changed)
    cfe/branches/release_34/test/Analysis/NewDelete-checker-test.cpp   (props changed)
    cfe/branches/release_34/test/Analysis/temporaries.cpp
    cfe/branches/release_34/test/SemaCXX/warn-unreachable.cpp   (props changed)

Propchange: cfe/branches/release_34/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Dec  9 12:37:00 2013
@@ -1,4 +1,4 @@
 /cfe/branches/type-system-rewrite:134693-134817
-/cfe/trunk:195126,195128,195135-195136,195146,195149,195154,195158,195163,195168,195174,195249,195268,195283,195303,195326,195329,195367,195384,195409,195420,195422,195501,195547,195556,195558,195587,195620,195635,195669,195687,195693,195710,195713,195716,195756,195760,195768,195777,195789,195792,195804,195827,195843-195844,195877,195887-195888,195897,195903,195905-195906,195932,195936-195943,195970,195983,196045,196048,196050,196058,196114-196115,196153,196189-196192,196198-196199,196206,196208-196209,196211,196215,196359-196362,196370,196387,196423,196454,196456,196459,196488,196532-196533,196535,196538,196588,196630,196658,196712,196720,196724
+/cfe/trunk:195126,195128,195135-195136,195146,195149,195154,195158,195163,195168,195174,195249,195268,195283,195303,195326,195329,195367,195384,195409,195420,195422,195501,195547,195556,195558,195587,195620,195635,195669,195687,195693,195710,195713,195716,195756,195760,195768,195777,195789,195792,195804,195827,195843-195844,195877,195887-195888,195897,195903,195905-195906,195932,195936-195943,195970,195983,196045,196048,196050,196058,196114-196115,196153,196189-196192,196198-196199,196206,196208-196209,196211,196215,196359-196362,196370,196387,196423,196454,196456,196459,196488,196532-196533,196535,196538,196588,196593,196630,196658,196712,196720,196724
 /cfe/trunk/test:170344
 /cfe/trunk/test/SemaTemplate:126920

Modified: cfe/branches/release_34/lib/Analysis/LiveVariables.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_34/lib/Analysis/LiveVariables.cpp?rev=196795&r1=196794&r2=196795&view=diff
==============================================================================
--- cfe/branches/release_34/lib/Analysis/LiveVariables.cpp (original)
+++ cfe/branches/release_34/lib/Analysis/LiveVariables.cpp Mon Dec  9 12:37:00 2013
@@ -211,8 +211,6 @@ class TransferFunctions : public StmtVis
   LiveVariables::LivenessValues &val;
   LiveVariables::Observer *observer;
   const CFGBlock *currentBlock;
-
-  void markLogicalExprLeaves(const Expr *E);
 public:
   TransferFunctions(LiveVariablesImpl &im,
                     LiveVariables::LivenessValues &Val,
@@ -369,25 +367,9 @@ void TransferFunctions::VisitBinaryOpera
         if (observer)
           observer->observerKill(DR);
       }
-  } else if (B->isLogicalOp()) {
-    // Leaf expressions in the logical operator tree are live until we reach the
-    // outermost logical operator. Static analyzer relies on this behaviour.
-    markLogicalExprLeaves(B->getLHS()->IgnoreParens());
-    markLogicalExprLeaves(B->getRHS()->IgnoreParens());
   }
 }
 
-void TransferFunctions::markLogicalExprLeaves(const Expr *E) {
-  const BinaryOperator *B = dyn_cast<BinaryOperator>(E);
-  if (!B || !B->isLogicalOp()) {
-    val.liveStmts = LV.SSetFact.add(val.liveStmts, E);
-    return;
-  }
-
-  markLogicalExprLeaves(B->getLHS()->IgnoreParens());
-  markLogicalExprLeaves(B->getRHS()->IgnoreParens());
-}
-
 void TransferFunctions::VisitBlockExpr(BlockExpr *BE) {
   AnalysisDeclContext::referenced_decls_iterator I, E;
   llvm::tie(I, E) =

Modified: cfe/branches/release_34/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_34/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=196795&r1=196794&r2=196795&view=diff
==============================================================================
--- cfe/branches/release_34/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/branches/release_34/lib/StaticAnalyzer/Core/ExprEngine.cpp Mon Dec  9 12:37:00 2013
@@ -1386,27 +1386,11 @@ void ExprEngine::processBranch(const Stm
     return;
   }
 
-  SValBuilder &SVB = Pred->getState()->getStateManager().getSValBuilder();
-  SVal TrueVal = SVB.makeTruthVal(true);
-  SVal FalseVal = SVB.makeTruthVal(false);
 
   if (const Expr *Ex = dyn_cast<Expr>(Condition))
     Condition = Ex->IgnoreParens();
 
-  // If the value is already available, we don't need to do anything.
-  if (Pred->getState()->getSVal(Condition, LCtx).isUnknownOrUndef()) {
-    // Resolve the condition in the presence of nested '||' and '&&'.
-    Condition = ResolveCondition(Condition, BldCtx.getBlock());
-  }
-
-  // Cast truth values to the correct type.
-  if (const Expr *Ex = dyn_cast<Expr>(Condition)) {
-    TrueVal = SVB.evalCast(TrueVal, Ex->getType(),
-                           getContext().getLogicalOperationType());
-    FalseVal = SVB.evalCast(FalseVal, Ex->getType(),
-                            getContext().getLogicalOperationType());
-  }
-
+  Condition = ResolveCondition(Condition, BldCtx.getBlock());
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 Condition->getLocStart(),
                                 "Error evaluating branch");
@@ -1449,37 +1433,31 @@ void ExprEngine::processBranch(const Stm
       }
     }
     
-    ProgramStateRef StTrue, StFalse;
-
     // If the condition is still unknown, give up.
     if (X.isUnknownOrUndef()) {
-
-      StTrue = PrevState->BindExpr(Condition, BldCtx.LC, TrueVal);
-      StFalse = PrevState->BindExpr(Condition, BldCtx.LC, FalseVal);
-
-      builder.generateNode(StTrue, true, PredI);
-      builder.generateNode(StFalse, false, PredI);
+      builder.generateNode(PrevState, true, PredI);
+      builder.generateNode(PrevState, false, PredI);
       continue;
     }
 
     DefinedSVal V = X.castAs<DefinedSVal>();
+
+    ProgramStateRef StTrue, StFalse;
     tie(StTrue, StFalse) = PrevState->assume(V);
 
     // Process the true branch.
     if (builder.isFeasible(true)) {
-      if (StTrue) {
-        StTrue = StTrue->BindExpr(Condition, BldCtx.LC, TrueVal);
+      if (StTrue)
         builder.generateNode(StTrue, true, PredI);
-      } else
+      else
         builder.markInfeasible(true);
     }
 
     // Process the false branch.
     if (builder.isFeasible(false)) {
-      if (StFalse) {
-        StFalse = StFalse->BindExpr(Condition, BldCtx.LC, FalseVal);
+      if (StFalse)
         builder.generateNode(StFalse, false, PredI);
-      } else
+      else
         builder.markInfeasible(false);
     }
   }

Modified: cfe/branches/release_34/lib/StaticAnalyzer/Core/ExprEngineC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_34/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=196795&r1=196794&r2=196795&view=diff
==============================================================================
--- cfe/branches/release_34/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original)
+++ cfe/branches/release_34/lib/StaticAnalyzer/Core/ExprEngineC.cpp Mon Dec  9 12:37:00 2013
@@ -505,33 +505,6 @@ void ExprEngine::VisitDeclStmt(const Dec
   getCheckerManager().runCheckersForPostStmt(Dst, B.getResults(), DS, *this);
 }
 
-static ProgramStateRef evaluateLogicalExpression(const Expr *E,
-                                                 const LocationContext *LC,
-                                                 ProgramStateRef State) {
-  SVal X = State->getSVal(E, LC);
-  if (! X.isUnknown())
-    return State;
-
-  const BinaryOperator *B = dyn_cast<BinaryOperator>(E->IgnoreParens());
-  if (!B || (B->getOpcode() != BO_LAnd && B->getOpcode() != BO_LOr))
-    return State;
-
-  State = evaluateLogicalExpression(B->getLHS(), LC, State);
-  X = State->getSVal(B->getLHS(), LC);
-  QualType XType = B->getLHS()->getType();
-
-  assert(X.isConstant());
-  if (!X.isZeroConstant() == (B->getOpcode() == BO_LAnd)) {
-    // LHS not sufficient, we need to check RHS as well
-    State = evaluateLogicalExpression(B->getRHS(), LC, State);
-    X = State->getSVal(B->getRHS(), LC);
-    XType = B->getRHS()->getType();
-  }
-
-  SValBuilder &SVB = State->getStateManager().getSValBuilder();
-  return State->BindExpr(E, LC, SVB.evalCast(X, B->getType(), XType));
-}
-
 void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred,
                                   ExplodedNodeSet &Dst) {
   assert(B->getOpcode() == BO_LAnd ||
@@ -540,25 +513,64 @@ void ExprEngine::VisitLogicalExpr(const
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
 
-  state = evaluateLogicalExpression(B, Pred->getLocationContext(), state);
-  SVal X = state->getSVal(B, Pred->getLocationContext());
+  ExplodedNode *N = Pred;
+  while (!N->getLocation().getAs<BlockEntrance>()) {
+    ProgramPoint P = N->getLocation();
+    assert(P.getAs<PreStmt>()|| P.getAs<PreStmtPurgeDeadSymbols>());
+    (void) P;
+    assert(N->pred_size() == 1);
+    N = *N->pred_begin();
+  }
+  assert(N->pred_size() == 1);
+  N = *N->pred_begin();
+  BlockEdge BE = N->getLocation().castAs<BlockEdge>();
+  SVal X;
+
+  // Determine the value of the expression by introspecting how we
+  // got this location in the CFG.  This requires looking at the previous
+  // block we were in and what kind of control-flow transfer was involved.
+  const CFGBlock *SrcBlock = BE.getSrc();
+  // The only terminator (if there is one) that makes sense is a logical op.
+  CFGTerminator T = SrcBlock->getTerminator();
+  if (const BinaryOperator *Term = cast_or_null<BinaryOperator>(T.getStmt())) {
+    (void) Term;
+    assert(Term->isLogicalOp());
+    assert(SrcBlock->succ_size() == 2);
+    // Did we take the true or false branch?
+    unsigned constant = (*SrcBlock->succ_begin() == BE.getDst()) ? 1 : 0;
+    X = svalBuilder.makeIntVal(constant, B->getType());
+  }
+  else {
+    // If there is no terminator, by construction the last statement
+    // in SrcBlock is the value of the enclosing expression.
+    // However, we still need to constrain that value to be 0 or 1.
+    assert(!SrcBlock->empty());
+    CFGStmt Elem = SrcBlock->rbegin()->castAs<CFGStmt>();
+    const Expr *RHS = cast<Expr>(Elem.getStmt());
+    SVal RHSVal = N->getState()->getSVal(RHS, Pred->getLocationContext());
 
-  if (!X.isUndef()) {
-    DefinedOrUnknownSVal DefinedRHS = X.castAs<DefinedOrUnknownSVal>();
-    ProgramStateRef StTrue, StFalse;
-    llvm::tie(StTrue, StFalse) = state->assume(DefinedRHS);
-    if (StTrue) {
-      if (!StFalse) {
-        // The value is known to be true.
-        X = getSValBuilder().makeIntVal(1, B->getType());
-      } // else The truth value of X is unknown, just leave it as it is.
+    if (RHSVal.isUndef()) {
+      X = RHSVal;
     } else {
-      // The value is known to be false.
-      assert(StFalse && "Infeasible path!");
-      X = getSValBuilder().makeIntVal(0, B->getType());
+      DefinedOrUnknownSVal DefinedRHS = RHSVal.castAs<DefinedOrUnknownSVal>();
+      ProgramStateRef StTrue, StFalse;
+      llvm::tie(StTrue, StFalse) = N->getState()->assume(DefinedRHS);
+      if (StTrue) {
+        if (StFalse) {
+          // We can't constrain the value to 0 or 1.
+          // The best we can do is a cast.
+          X = getSValBuilder().evalCast(RHSVal, B->getType(), RHS->getType());
+        } else {
+          // The value is known to be true.
+          X = getSValBuilder().makeIntVal(1, B->getType());
+        }
+      } else {
+        // The value is known to be false.
+        assert(StFalse && "Infeasible path!");
+        X = getSValBuilder().makeIntVal(0, B->getType());
+      }
     }
   }
-
   Bldr.generateNode(B, Pred, state->BindExpr(B, Pred->getLocationContext(), X));
 }
 

Propchange: cfe/branches/release_34/test/Analysis/MismatchedDeallocator-checker-test.mm
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Dec  9 12:37:00 2013
@@ -1,4 +1,4 @@
 /cfe/branches/type-system-rewrite/test/Analysis/alloc-match-dealloc.mm:134693-134817
-/cfe/trunk/test/Analysis/MismatchedDeallocator-checker-test.mm:195983,196114,196387,196532,196538
+/cfe/trunk/test/Analysis/MismatchedDeallocator-checker-test.mm:195983,196114,196387,196532,196538,196593
 /cfe/trunk/test/SemaTemplate/test/Analysis/alloc-match-dealloc.mm:126920
 /cfe/trunk/test/test/Analysis/alloc-match-dealloc.mm:170344

Propchange: cfe/branches/release_34/test/Analysis/NewDelete-checker-test.cpp
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Dec  9 12:37:00 2013
@@ -1,4 +1,4 @@
 /cfe/branches/type-system-rewrite/test/Analysis/NewDelete-checker-test.mm:134693-134817
-/cfe/trunk/test/Analysis/NewDelete-checker-test.cpp:195983,196114,196387,196532,196538
+/cfe/trunk/test/Analysis/NewDelete-checker-test.cpp:195983,196114,196387,196532,196538,196593
 /cfe/trunk/test/SemaTemplate/test/Analysis/NewDelete-checker-test.mm:126920
 /cfe/trunk/test/test/Analysis/NewDelete-checker-test.mm:170344

Modified: cfe/branches/release_34/test/Analysis/temporaries.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_34/test/Analysis/temporaries.cpp?rev=196795&r1=196794&r2=196795&view=diff
==============================================================================
--- cfe/branches/release_34/test/Analysis/temporaries.cpp (original)
+++ cfe/branches/release_34/test/Analysis/temporaries.cpp Mon Dec  9 12:37:00 2013
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -w -std=c++03 %s
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -w -std=c++11 %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -w -analyzer-config cfg-temporary-dtors=true %s -DTEMPORARY_DTORS
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true %s 
 
 extern bool clang_analyzer_eval(bool);
 
@@ -111,17 +111,20 @@ namespace compound_literals {
 }
 
 namespace destructors {
-  void testPR16664Crash() {
+  void testPR16664andPR18159Crash() {
     struct Dtor {
       ~Dtor();
     };
     extern bool coin();
     extern bool check(const Dtor &);
 
-    // Don't crash here.
+#ifndef TEMPORARY_DTORS
+    // FIXME: Don't crash here when tmp dtros are enabled.
+    // PR16664 and PR18159 
     if (coin() && (coin() || coin() || check(Dtor()))) {
       Dtor();
     }
+#endif
   }
 
 #ifdef TEMPORARY_DTORS
@@ -147,9 +150,6 @@ namespace destructors {
   extern bool check(const NoReturnDtor &);
 
   void testConsistencyIf(int i) {
-    if (i == 5 && (i == 4 || i == 5 || check(NoReturnDtor())))
-      clang_analyzer_eval(true); // expected-warning{{TRUE}}
-
     if (i != 5)
       return;
     if (i == 5 && (i == 4 || check(NoReturnDtor()) || i == 5)) {
@@ -170,11 +170,18 @@ namespace destructors {
     clang_analyzer_eval(true); // no warning, unreachable code
   }
 
+
+/*
+  // PR16664 and PR18159 
+  FIXME: Don't crash here.
   void testConsistencyNested(int i) {
     extern bool compute(bool);
-
+  
+    if (i == 5 && (i == 4 || i == 5 || check(NoReturnDtor())))
+      clang_analyzer_eval(true); // expected TRUE
+  
     if (i == 5 && (i == 4 || i == 5 || check(NoReturnDtor())))
-      clang_analyzer_eval(true);  // expected-warning{{TRUE}}
+      clang_analyzer_eval(true);  // expected TRUE
 
     if (i != 5)
       return;
@@ -183,7 +190,7 @@ namespace destructors {
                 (i == 4 || compute(true) ||
                  compute(i == 5 && (i == 4 || check(NoReturnDtor()))))) ||
         i != 4) {
-      clang_analyzer_eval(true); // expected-warning{{TRUE}}
+      clang_analyzer_eval(true); // expected TRUE
     }
 
     if (compute(i == 5 &&
@@ -192,7 +199,8 @@ namespace destructors {
         i != 4) {
       clang_analyzer_eval(true); // no warning, unreachable code
     }
-  }
+  }*/
+  
 #endif // TEMPORARY_DTORS
 }
 

Propchange: cfe/branches/release_34/test/SemaCXX/warn-unreachable.cpp
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Dec  9 12:37:00 2013
@@ -1,2 +1,2 @@
 /cfe/branches/type-system-rewrite/test/SemaCXX/warn-unreachable.cpp:134693-134817
-/cfe/trunk/test/SemaCXX/warn-unreachable.cpp:121961,195983,196114,196387,196532,196538
+/cfe/trunk/test/SemaCXX/warn-unreachable.cpp:121961,195983,196114,196387,196532,196538,196593





More information about the llvm-branch-commits mailing list