From: Csaba Dabis <dabis.csaba98@gmail.com>
Date: Sat, 16 Mar 2019 10:06:06 +0000 (+0000)
Subject: Revert "[analyzer] ConditionBRVisitor: Unknown condition evaluation support"
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=61c69cbbca443c04ea5740678444e9eb80ce68a1;p=clang

Revert "[analyzer] ConditionBRVisitor: Unknown condition evaluation support"

This reverts commit 0fe67a61cd4aec13c7969a179517f1cc06ab05cd.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@356320 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 2c4277c8b7..bf7d330e4d 100644
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1815,6 +1815,12 @@ std::shared_ptr<PathDiagnosticPiece>
 ConditionBRVisitor::VisitNodeImpl(const ExplodedNode *N,
                                   BugReporterContext &BRC, BugReport &BR) {
   ProgramPoint progPoint = N->getLocation();
+  ProgramStateRef CurrentState = N->getState();
+  ProgramStateRef PreviousState = N->getFirstPred()->getState();
+
+  // If the constraint information does not changed there is no assumption.
+  if (BRC.getStateManager().haveEqualConstraints(CurrentState, PreviousState))
+    return nullptr;
 
   // If an assumption was made on a branch, it should be caught
   // here by looking at the state transition.
@@ -1883,8 +1889,6 @@ std::shared_ptr<PathDiagnosticPiece> ConditionBRVisitor::VisitTerminator(
     break;
   }
 
-  Cond = Cond->IgnoreParens();
-
   // However, when we encounter a logical operator as a branch condition,
   // then the condition is actually its RHS, because LHS would be
   // the condition for the logical operator terminator.
@@ -1904,18 +1908,6 @@ std::shared_ptr<PathDiagnosticPiece>
 ConditionBRVisitor::VisitTrueTest(const Expr *Cond, bool tookTrue,
                                   BugReporterContext &BRC, BugReport &R,
                                   const ExplodedNode *N) {
-  ProgramStateRef CurrentState = N->getState();
-  ProgramStateRef PreviousState = N->getFirstPred()->getState();
-  const LocationContext *LCtx = N->getLocationContext();
-
-  // If the constraint information is changed between the current and the
-  // previous program state we assuming the newly seen constraint information.
-  // If we cannot evaluate the condition (and the constraints are the same)
-  // the analyzer has no information about the value and just assuming it.
-  if (BRC.getStateManager().haveEqualConstraints(CurrentState, PreviousState) &&
-      CurrentState->getSVal(Cond, LCtx).isValid())
-    return nullptr;
-
   // These will be modified in code below, but we need to preserve the original
   //  values in case we want to throw the generic message.
   const Expr *CondTmp = Cond;
@@ -1951,6 +1943,7 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond, bool tookTrue,
 
   // Condition too complex to explain? Just say something so that the user
   // knew we've made some path decision at this point.
+  const LocationContext *LCtx = N->getLocationContext();
   PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
   if (!Loc.isValid() || !Loc.asLocation().isValid())
     return nullptr;
diff --git a/test/Analysis/diagnostics/macros.cpp b/test/Analysis/diagnostics/macros.cpp
index 67cdef7f38..5aa2c03ab0 100644
--- a/test/Analysis/diagnostics/macros.cpp
+++ b/test/Analysis/diagnostics/macros.cpp
@@ -30,8 +30,7 @@ void testnullptrMacro(int *p) {
 
 // There are no path notes on the comparison to float types.
 void testDoubleMacro(double d) {
-  if (d == DBL_MAX) { // expected-note {{Assuming 'd' is equal to DBL_MAX}}
-                      // expected-note@-1 {{Taking true branch}}
+  if (d == DBL_MAX) { // expected-note {{Taking true branch}}
 
     char *p = NULL; // expected-note {{'p' initialized to a null pointer value}}
     *p = 7;         // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
diff --git a/test/Analysis/uninit-vals.m b/test/Analysis/uninit-vals.m
index 5b959c7bfe..33352122ca 100644
--- a/test/Analysis/uninit-vals.m
+++ b/test/Analysis/uninit-vals.m
@@ -164,8 +164,7 @@ void PR14765_test() {
                                            // expected-note@-1{{TRUE}}
 
   testObj->origin = makePoint(0.0, 0.0);
-  if (testObj->size > 0) { ; } // expected-note{{Assuming the condition is false}}
-                               // expected-note@-1{{Taking false branch}}
+  if (testObj->size > 0) { ; } // expected-note{{Taking false branch}}
 
   // FIXME: Assigning to 'testObj->origin' kills the default binding for the
   // whole region, meaning that we've forgotten that testObj->size should also
@@ -219,14 +218,10 @@ void PR14765_test_int() {
                                                // expected-note@-1{{TRUE}}
 
   testObj->origin = makeIntPoint(1, 2);
-  if (testObj->size > 0) { ; } // expected-note{{Assuming the condition is false}}
+  if (testObj->size > 0) { ; } // expected-note{{Taking false branch}}
                                // expected-note@-1{{Taking false branch}}
-                               // expected-note@-2{{Assuming the condition is false}}
+                               // expected-note@-2{{Taking false branch}}
                                // expected-note@-3{{Taking false branch}}
-                               // expected-note@-4{{Assuming the condition is false}}
-                               // expected-note@-5{{Taking false branch}}
-                               // expected-note@-6{{Assuming the condition is false}}
-                               // expected-note@-7{{Taking false branch}}
 
   // FIXME: Assigning to 'testObj->origin' kills the default binding for the
   // whole region, meaning that we've forgotten that testObj->size should also