r356323 - [analyzer] ConditionBRVisitor: Unknown condition evaluation support

Csaba Dabis via cfe-commits cfe-commits at lists.llvm.org
Sat Mar 16 06:47:55 PDT 2019


Author: charusso
Date: Sat Mar 16 06:47:55 2019
New Revision: 356323

URL: http://llvm.org/viewvc/llvm-project?rev=356323&view=rev
Log:
[analyzer] ConditionBRVisitor: Unknown condition evaluation support

Summary:
If the constraint information is not changed between two program states the
analyzer has not learnt new information and made no report. But it is
possible to happen because we have no information at all. The new approach
evaluates the condition to determine if that is the case and let the user
know we just `Assuming...` some value.

Reviewers: NoQ, george.karpenkov

Reviewed By: NoQ

Subscribers: llvm-commits, xazax.hun, baloghadamsoftware, szepet, a.sidorin,
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gsd, gerazo

Tags: #clang, #llvm

Differential Revision: https://reviews.llvm.org/D57410

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/trunk/test/Analysis/diagnostics/macros.cpp
    cfe/trunk/test/Analysis/uninit-vals.m

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=356323&r1=356322&r2=356323&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Sat Mar 16 06:47:55 2019
@@ -1815,12 +1815,6 @@ 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.
@@ -1910,6 +1904,18 @@ 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;
@@ -1945,7 +1951,6 @@ ConditionBRVisitor::VisitTrueTest(const
 
   // 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;

Modified: cfe/trunk/test/Analysis/diagnostics/macros.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/macros.cpp?rev=356323&r1=356322&r2=356323&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/diagnostics/macros.cpp (original)
+++ cfe/trunk/test/Analysis/diagnostics/macros.cpp Sat Mar 16 06:47:55 2019
@@ -30,7 +30,8 @@ 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 {{Taking true branch}}
+  if (d == DBL_MAX) { // expected-note {{Assuming 'd' is equal to DBL_MAX}}
+                      // expected-note at -1 {{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')}}

Modified: cfe/trunk/test/Analysis/uninit-vals.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-vals.m?rev=356323&r1=356322&r2=356323&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/uninit-vals.m (original)
+++ cfe/trunk/test/Analysis/uninit-vals.m Sat Mar 16 06:47:55 2019
@@ -164,7 +164,8 @@ void PR14765_test() {
                                            // expected-note at -1{{TRUE}}
 
   testObj->origin = makePoint(0.0, 0.0);
-  if (testObj->size > 0) { ; } // expected-note{{Taking false branch}}
+  if (testObj->size > 0) { ; } // expected-note{{Assuming the condition is false}}
+                               // expected-note at -1{{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
@@ -218,10 +219,14 @@ void PR14765_test_int() {
                                                // expected-note at -1{{TRUE}}
 
   testObj->origin = makeIntPoint(1, 2);
-  if (testObj->size > 0) { ; } // expected-note{{Taking false branch}}
+  if (testObj->size > 0) { ; } // expected-note{{Assuming the condition is false}}
                                // expected-note at -1{{Taking false branch}}
-                               // expected-note at -2{{Taking false branch}}
+                               // expected-note at -2{{Assuming the condition is false}}
                                // expected-note at -3{{Taking false branch}}
+                               // expected-note at -4{{Assuming the condition is false}}
+                               // expected-note at -5{{Taking false branch}}
+                               // expected-note at -6{{Assuming the condition is false}}
+                               // expected-note at -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




More information about the cfe-commits mailing list