[analyzer] BugReporter - more precise tracking of C++ references

When BugReporter tracks C++ references involved in a null pointer violation, we
want to differentiate between a null reference and a reference to a null pointer. In the
first case, we want to track the region for the reference location; in the second, we want
to track the null pointer.

In addition, the core creates CXXTempObjectRegion to represent the location of the
C++ reference, so teach FindLastStoreBRVisitor about it.

This helps null pointer suppression to kick in.

(Patch by Anna and Jordan.)

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

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 517c1a10f8ee16bfd19f764cc4238d4345621203..56d6d26133dadc61aaef49d28fe59d0cba5af1dd 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -470,6 +470,11 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
         IsParam = true;
       }
     }
+
+    // If this is a CXXTempObjectRegion, the Expr responsible for its creation
+    // is wrapped inside of it.
+    if (const CXXTempObjectRegion *TmpR = dyn_cast<CXXTempObjectRegion>(R))
+      InitE = TmpR->getExpr();
   }
 
   if (!StoreSite)
@@ -756,6 +761,27 @@ SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ,
   return 0;
 }
 
+static const MemRegion *getLocationRegionIfReference(const Expr *E,
+                                                     const ExplodedNode *N) {
+  if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E)) {
+    if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
+      if (!VD->getType()->isReferenceType())
+        return 0;
+      ProgramStateManager &StateMgr = N->getState()->getStateManager();
+      MemRegionManager &MRMgr = StateMgr.getRegionManager();
+      return MRMgr.getVarRegion(VD, N->getLocationContext());
+    }
+  }
+
+  // FIXME: This does not handle other kinds of null references,
+  // for example, references from FieldRegions:
+  //   struct Wrapper { int &ref; };
+  //   Wrapper w = { *(int *)0 };
+  //   w.ref = 1;
+
+  return 0;
+}
+
 bool bugreporter::trackNullOrUndefValue(const ExplodedNode *ErrorNode,
                                         const Stmt *S,
                                         BugReport &report, bool IsArg) {
@@ -804,33 +830,37 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *ErrorNode,
   if (Inner && ExplodedGraph::isInterestingLValueExpr(Inner)) {
     const MemRegion *R = 0;
 
-    // First check if this is a DeclRefExpr for a C++ reference type.
-    // For those, we want the location of the reference.
-    if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(Inner)) {
-      if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
-        if (VD->getType()->isReferenceType()) {
-          ProgramStateManager &StateMgr = state->getStateManager();
-          MemRegionManager &MRMgr = StateMgr.getRegionManager();
-          R = MRMgr.getVarRegion(VD, N->getLocationContext());
-        }
+    // Find the ExplodedNode where the lvalue (the value of 'Ex')
+    // was computed.  We need this for getting the location value.
+    const ExplodedNode *LVNode = N;
+    while (LVNode) {
+      if (Optional<PostStmt> P = LVNode->getLocation().getAs<PostStmt>()) {
+        if (P->getStmt() == Inner)
+          break;
       }
+      LVNode = LVNode->getFirstPred();
     }
+    assert(LVNode && "Unable to find the lvalue node.");
+    ProgramStateRef LVState = LVNode->getState();
+    SVal LVal = LVState->getSVal(Inner, LVNode->getLocationContext());
+    
+    if (LVState->isNull(LVal).isConstrainedTrue()) {
+      // In case of C++ references, we want to differentiate between a null
+      // reference and reference to null pointer.
+      // If the LVal is null, check if we are dealing with null reference.
+      // For those, we want to track the location of the reference.
+      if (const MemRegion *RR = getLocationRegionIfReference(Inner, N))
+        R = RR;
+    } else {
+      R = LVState->getSVal(Inner, LVNode->getLocationContext()).getAsRegion();
 
-    // For all other cases, find the location by scouring the ExplodedGraph.
-    if (!R) {
-      // Find the ExplodedNode where the lvalue (the value of 'Ex')
-      // was computed.  We need this for getting the location value.
-      const ExplodedNode *LVNode = N;
-      while (LVNode) {
-        if (Optional<PostStmt> P = LVNode->getLocation().getAs<PostStmt>()) {
-          if (P->getStmt() == Inner)
-            break;
-        }
-        LVNode = LVNode->getFirstPred();
+      // If this is a C++ reference to a null pointer, we are tracking the
+      // pointer. In additon, we should find the store at which the reference
+      // got initialized.
+      if (const MemRegion *RR = getLocationRegionIfReference(Inner, N)) {
+        if (Optional<KnownSVal> KV = LVal.getAs<KnownSVal>())
+          report.addVisitor(new FindLastStoreBRVisitor(*KV, RR));
       }
-      assert(LVNode && "Unable to find the lvalue node.");
-      ProgramStateRef LVState = LVNode->getState();
-      R = LVState->getSVal(Inner, LVNode->getLocationContext()).getAsRegion();
     }
 
     if (R) {

diff --git a/test/Analysis/diagnostics/deref-track-symbolic-region.cpp b/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
index e166109ef8168043d665329cb4b410ee4b013b9f..6d348415aaf3de4ab4378d579f7e095c3bf1a4b4 100644 (file)
--- a/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
+++ b/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
@@ -25,4 +25,19 @@ void testRefParam(int *ptr) {
        extern void use(int &ref);
        use(ref); // expected-warning{{Forming reference to null pointer}}
             // expected-note@-1{{Forming reference to null pointer}}
+}
+
+int testRefToNullPtr() {
+  int *p = 0; // expected-note {{'p' initialized to a null pointer value}}
+  int *const &p2 = p; // expected-note{{'p2' initialized here}}
+  int *p3 = p2; // expected-note {{'p3' initialized to a null pointer value}}
+  return *p3; // expected-warning {{Dereference of null pointer}}
+              // expected-note@-1{{Dereference of null pointer}}
+}
+
+int testRefToNullPtr2() {
+  int *p = 0; // expected-note {{'p' initialized to a null pointer value}}
+  int *const &p2 = p;// expected-note{{'p2' initialized here}}
+  return *p2; //expected-warning {{Dereference of null pointer}}
+              // expected-note@-1{{Dereference of null pointer}}
 }
\ No newline at end of file

diff --git a/test/Analysis/inlining/false-positive-suppression.cpp b/test/Analysis/inlining/false-positive-suppression.cpp
index f27c7cf364a9f4e572ea7ebfd4bf4795242b2b10..613f42111078060039ef21e384e17c109cf6f5f6 100644 (file)
--- a/test/Analysis/inlining/false-positive-suppression.cpp
+++ b/test/Analysis/inlining/false-positive-suppression.cpp
@@ -125,3 +125,52 @@ namespace References {
 #endif
   }
 }
+
+class X{
+public:
+       void get();
+};
+
+X *getNull() {
+       return 0;
+}
+
+void deref1(X *const &p) {
+       return p->get();
+       #ifndef SUPPRESSED
+         // expected-warning@-2 {{Called C++ object pointer is null}}
+       #endif
+}
+
+void test1() {
+       return deref1(getNull());
+}
+
+void deref2(X *p3) {
+       p3->get();
+       #ifndef SUPPRESSED
+         // expected-warning@-2 {{Called C++ object pointer is null}}
+       #endif
+}
+
+void pass2(X *const &p2) {
+       deref2(p2);
+}
+
+void test2() {
+       pass2(getNull());
+}
+
+void deref3(X *const &p2) {
+       X *p3 = p2;
+       p3->get();
+       #ifndef SUPPRESSED
+         // expected-warning@-2 {{Called C++ object pointer is null}}
+       #endif
+}
+
+void test3() {
+       deref3(getNull());
+}
+
+