diff options
| -rw-r--r-- | clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp | 21 | ||||
| -rw-r--r-- | clang/test/Analysis/diagnostics/macros.cpp | 3 | ||||
| -rw-r--r-- | clang/test/Analysis/uninit-vals.m | 11 |
3 files changed, 24 insertions, 11 deletions
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index bf7d330e4d5..2c4277c8b70 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -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. @@ -1889,6 +1883,8 @@ 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. @@ -1908,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; @@ -1943,7 +1951,6 @@ 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/clang/test/Analysis/diagnostics/macros.cpp b/clang/test/Analysis/diagnostics/macros.cpp index 5aa2c03ab0e..67cdef7f389 100644 --- a/clang/test/Analysis/diagnostics/macros.cpp +++ b/clang/test/Analysis/diagnostics/macros.cpp @@ -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@-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')}} diff --git a/clang/test/Analysis/uninit-vals.m b/clang/test/Analysis/uninit-vals.m index 33352122ca5..5b959c7bfe1 100644 --- a/clang/test/Analysis/uninit-vals.m +++ b/clang/test/Analysis/uninit-vals.m @@ -164,7 +164,8 @@ void PR14765_test() { // expected-note@-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@-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@-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@-1{{Taking false branch}} - // expected-note@-2{{Taking false branch}} + // expected-note@-2{{Assuming the condition is false}} // 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 |
