Fix bug in ConditionBRVisitor where for C++ (and not C) we were not ignoring

implicit pointer-to-boolean conversions in condition expressions.  This would
result in inconsistent diagnostic emission between C and C++.

A consequence of this is now ConditionBRVisitor and TrackConstraintBRVisitor may
emit redundant diagnostics, for example:

  "Assuming pointer value is null" (TrackConstraintBRVisitor)
  "Assuming 'p' is null" (ConditionBRVisitor)

We need to reconcile the two, and perhaps prefer one over the other in some
cases.

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

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index d5fe1e0a453cb3ac466f2eda619c365efe1fa781..6b7fc4add8b20b55920e5c2b6362084306035dbb 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -696,8 +696,7 @@ ConditionBRVisitor::VisitTerminator(const Stmt *Term,
   assert(Cond);
   assert(srcBlk->succ_size() == 2);
   const bool tookTrue = *(srcBlk->succ_begin()) == dstBlk;
-  return VisitTrueTest(Cond->IgnoreParenNoopCasts(BRC.getASTContext()),
-                       tookTrue, BRC, R, N);
+  return VisitTrueTest(Cond, tookTrue, BRC, R, N);
 }
 
 PathDiagnosticPiece *
@@ -710,7 +709,7 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond,
   const Expr *Ex = Cond;
   
   while (true) {
-    Ex = Ex->IgnoreParens();
+    Ex = Ex->IgnoreParenCasts();
     switch (Ex->getStmtClass()) {
       default:
         return 0;
@@ -724,7 +723,7 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond,
         const UnaryOperator *UO = cast<UnaryOperator>(Ex);
         if (UO->getOpcode() == UO_LNot) {
           tookTrue = !tookTrue;
-          Ex = UO->getSubExpr()->IgnoreParenNoopCasts(BRC.getASTContext());
+          Ex = UO->getSubExpr();
           continue;
         }
         return 0;

diff --git a/test/Analysis/diagnostics/deref-track-symbolic-region.cpp b/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
index 87313de13a8543d2381523468bcddd588c55a720..4a5f0cfa3f74382ccd631b6bf526286209d8cc85 100644 (file)
--- a/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
+++ b/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
@@ -11,6 +11,7 @@ void test(S *p) {
   if (p) return;
                //expected-note@-1{{Taking false branch}}
                //expected-note@-2{{Assuming pointer value is null}}
+               //expected-note@-3{{Assuming 'p' is null}}
   r.y = 5; // expected-warning {{Access to field 'y' results in a dereference of a null pointer (loaded from variable 'r')}}
            // expected-note@-1{{Access to field 'y' results in a dereference of a null pointer (loaded from variable 'r')}}
 }

diff --git a/test/Analysis/method-call-path-notes.cpp b/test/Analysis/method-call-path-notes.cpp
index 17034b9b0001b3331619b133f6d357dd0aad5be9..b95ec98ec8465c22aec559ee0156889d968a4e15 100644 (file)
--- a/test/Analysis/method-call-path-notes.cpp
+++ b/test/Analysis/method-call-path-notes.cpp
@@ -25,7 +25,7 @@ void test_ic_set_to_null() {
 }
 
 void test_ic_null(TestInstanceCall *p) {
-  if (!p) // expected-note {{Assuming pointer value is null}} expected-note {{Taking true branch}}
+  if (!p) // expected-note {{Assuming pointer value is null}} expected-note {{Assuming 'p' is null}} expected-note {{Taking true branch}}
     p->foo(); // expected-warning {{Called C++ object pointer is null}} expected-note{{Called C++ object pointer is null}}
 }
 
@@ -37,7 +37,7 @@ void test_ic_member_ptr() {
 }
 
 void test_cast(const TestInstanceCall *p) {
-  if (!p) // expected-note {{Assuming pointer value is null}} expected-note {{Taking true branch}}
+  if (!p) // expected-note {{Assuming pointer value is null}} expected-note {{Assuming 'p' is null}} expected-note {{Taking true branch}}
     const_cast<TestInstanceCall *>(p)->foo(); // expected-warning {{Called C++ object pointer is null}} expected-note {{Called C++ object pointer is null}}
 }