[analyzer] Add a simple check for initializing reference variables with null.

There's still more work to be done here; this doesn't catch reference
parameters or return values. But it's a step in the right direction.

Part of <rdar://problem/11212286>.

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

diff --git a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index 9b56c9fe137b2c8adf85009f19d52abc13ee3439..a0022541f98ee366e0bb9687eb99f285b3b0a488 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -26,13 +26,18 @@ using namespace ento;
 namespace {
 class DereferenceChecker
     : public Checker< check::Location,
-                        EventDispatcher<ImplicitNullDerefEvent> > {
+                      check::Bind,
+                      EventDispatcher<ImplicitNullDerefEvent> > {
   mutable OwningPtr<BuiltinBug> BT_null;
   mutable OwningPtr<BuiltinBug> BT_undef;
 
+  void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext &C,
+                 bool IsBind = false) const;
+
 public:
   void checkLocation(SVal location, bool isLoad, const Stmt* S,
                      CheckerContext &C) const;
+  void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const;
 
   static const MemRegion *AddDerefSource(raw_ostream &os,
                              SmallVectorImpl<SourceRange> &Ranges,
@@ -76,6 +81,108 @@ DereferenceChecker::AddDerefSource(raw_ostream &os,
   return sourceR;
 }
 
+void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
+                                   CheckerContext &C, bool IsBind) const {
+  // Generate an error node.
+  ExplodedNode *N = C.generateSink(State);
+  if (!N)
+    return;
+
+  // We know that 'location' cannot be non-null.  This is what
+  // we call an "explicit" null dereference.
+  if (!BT_null)
+    BT_null.reset(new BuiltinBug("Dereference of null pointer"));
+
+  SmallString<100> buf;
+  SmallVector<SourceRange, 2> Ranges;
+
+  // Walk through lvalue casts to get the original expression
+  // that syntactically caused the load.
+  if (const Expr *expr = dyn_cast<Expr>(S))
+    S = expr->IgnoreParenLValueCasts();
+
+  const MemRegion *sourceR = 0;
+
+  if (IsBind) {
+    if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(S)) {
+      if (BO->isAssignmentOp())
+        S = BO->getRHS();
+    } else if (const DeclStmt *DS = dyn_cast<DeclStmt>(S)) {
+      assert(DS->isSingleDecl() && "We process decls one by one");
+      if (const VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl()))
+        if (const Expr *Init = VD->getAnyInitializer())
+          S = Init;
+    }
+  }
+
+  switch (S->getStmtClass()) {
+  case Stmt::ArraySubscriptExprClass: {
+    llvm::raw_svector_ostream os(buf);
+    os << "Array access";
+    const ArraySubscriptExpr *AE = cast<ArraySubscriptExpr>(S);
+    sourceR = AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
+                             State.getPtr(), N->getLocationContext());
+    os << " results in a null pointer dereference";
+    break;
+  }
+  case Stmt::UnaryOperatorClass: {
+    llvm::raw_svector_ostream os(buf);
+    os << "Dereference of null pointer";
+    const UnaryOperator *U = cast<UnaryOperator>(S);
+    sourceR = AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(),
+                             State.getPtr(), N->getLocationContext(), true);
+    break;
+  }
+  case Stmt::MemberExprClass: {
+    const MemberExpr *M = cast<MemberExpr>(S);
+    if (M->isArrow()) {
+      llvm::raw_svector_ostream os(buf);
+      os << "Access to field '" << M->getMemberNameInfo()
+         << "' results in a dereference of a null pointer";
+      sourceR = AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(),
+                               State.getPtr(), N->getLocationContext(), true);
+    }
+    break;
+  }
+  case Stmt::ObjCIvarRefExprClass: {
+    const ObjCIvarRefExpr *IV = cast<ObjCIvarRefExpr>(S);
+    if (const DeclRefExpr *DR =
+        dyn_cast<DeclRefExpr>(IV->getBase()->IgnoreParenCasts())) {
+      if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
+        llvm::raw_svector_ostream os(buf);
+        os << "Instance variable access (via '" << VD->getName()
+           << "') results in a null pointer dereference";
+      }
+    }
+    Ranges.push_back(IV->getSourceRange());
+    break;
+  }
+  default:
+    break;
+  }
+
+  BugReport *report =
+    new BugReport(*BT_null,
+                  buf.empty() ? BT_null->getDescription() : buf.str(),
+                  N);
+
+  report->addVisitor(
+    bugreporter::getTrackNullOrUndefValueVisitor(N,
+                                                 bugreporter::GetDerefExpr(N),
+                                                 report));
+
+  for (SmallVectorImpl<SourceRange>::iterator
+       I = Ranges.begin(), E = Ranges.end(); I!=E; ++I)
+    report->addRange(*I);
+
+  if (sourceR) {
+    report->markInteresting(sourceR);
+    report->markInteresting(State->getRawSVal(loc::MemRegionVal(sourceR)));
+  }
+
+  C.EmitReport(report);
+}
+
 void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
                                        CheckerContext &C) const {
   // Check for dereference of an undefined value.
@@ -101,115 +208,66 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
     return;
 
   ProgramStateRef state = C.getState();
-  const LocationContext *LCtx = C.getLocationContext();
+
   ProgramStateRef notNullState, nullState;
   llvm::tie(notNullState, nullState) = state->assume(location);
 
   // The explicit NULL case.
   if (nullState) {
     if (!notNullState) {
-      // Generate an error node.
-      ExplodedNode *N = C.generateSink(nullState);
-      if (!N)
-        return;
-
-      // We know that 'location' cannot be non-null.  This is what
-      // we call an "explicit" null dereference.
-      if (!BT_null)
-        BT_null.reset(new BuiltinBug("Dereference of null pointer"));
-
-      SmallString<100> buf;
-      SmallVector<SourceRange, 2> Ranges;
-      
-      // Walk through lvalue casts to get the original expression
-      // that syntactically caused the load.
-      if (const Expr *expr = dyn_cast<Expr>(S))
-        S = expr->IgnoreParenLValueCasts();
-      
-      const MemRegion *sourceR = 0;
-
-      switch (S->getStmtClass()) {
-        case Stmt::ArraySubscriptExprClass: {
-          llvm::raw_svector_ostream os(buf);
-          os << "Array access";
-          const ArraySubscriptExpr *AE = cast<ArraySubscriptExpr>(S);
-          sourceR =
-            AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
-                           state.getPtr(), LCtx);
-          os << " results in a null pointer dereference";
-          break;
-        }
-        case Stmt::UnaryOperatorClass: {
-          llvm::raw_svector_ostream os(buf);
-          os << "Dereference of null pointer";
-          const UnaryOperator *U = cast<UnaryOperator>(S);
-          sourceR =
-            AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(),
-                           state.getPtr(), LCtx, true);
-          break;
-        }
-        case Stmt::MemberExprClass: {
-          const MemberExpr *M = cast<MemberExpr>(S);
-          if (M->isArrow()) {
-            llvm::raw_svector_ostream os(buf);
-            os << "Access to field '" << M->getMemberNameInfo()
-               << "' results in a dereference of a null pointer";
-            sourceR =
-              AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(),
-                             state.getPtr(), LCtx, true);
-          }
-          break;
-        }
-        case Stmt::ObjCIvarRefExprClass: {
-          const ObjCIvarRefExpr *IV = cast<ObjCIvarRefExpr>(S);
-          if (const DeclRefExpr *DR =
-              dyn_cast<DeclRefExpr>(IV->getBase()->IgnoreParenCasts())) {
-            if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
-              llvm::raw_svector_ostream os(buf);
-              os << "Instance variable access (via '" << VD->getName()
-                 << "') results in a null pointer dereference";
-            }
-          }
-          Ranges.push_back(IV->getSourceRange());
-          break;
-        }
-        default:
-          break;
-      }
+      reportBug(nullState, S, C);
+      return;
+    }
 
-      BugReport *report =
-        new BugReport(*BT_null,
-                              buf.empty() ? BT_null->getDescription():buf.str(),
-                              N);
+    // Otherwise, we have the case where the location could either be
+    // null or not-null.  Record the error node as an "implicit" null
+    // dereference.
+    if (ExplodedNode *N = C.generateSink(nullState)) {
+      ImplicitNullDerefEvent event = { l, isLoad, N, &C.getBugReporter() };
+      dispatchEvent(event);
+    }
+  }
 
-      report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
-                                        bugreporter::GetDerefExpr(N), report));
+  // From this point forward, we know that the location is not null.
+  C.addTransition(notNullState);
+}
 
-      for (SmallVectorImpl<SourceRange>::iterator
-            I = Ranges.begin(), E = Ranges.end(); I!=E; ++I)
-        report->addRange(*I);
+void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S,
+                                   CheckerContext &C) const {
+  // If we're binding to a reference, check if the value is potentially null.
+  if (V.isUndef())
+    return;
 
-      if (sourceR) {
-        report->markInteresting(sourceR);
-        report->markInteresting(state->getRawSVal(loc::MemRegionVal(sourceR)));
-      }
+  const MemRegion *MR = L.getAsRegion();
+  const TypedValueRegion *TVR = dyn_cast_or_null<TypedValueRegion>(MR);
+  if (!TVR)
+    return;
 
-      C.EmitReport(report);
+  if (!TVR->getValueType()->isReferenceType())
+    return;
+
+  ProgramStateRef State = C.getState();
+
+  ProgramStateRef StNonNull, StNull;
+  llvm::tie(StNonNull, StNull) = State->assume(cast<DefinedOrUnknownSVal>(V));
+
+  if (StNull) {
+    if (!StNonNull) {
+      reportBug(StNull, S, C, /*isBind=*/true);
       return;
     }
-    else {
-      // Otherwise, we have the case where the location could either be
-      // null or not-null.  Record the error node as an "implicit" null
-      // dereference.
-      if (ExplodedNode *N = C.generateSink(nullState)) {
-        ImplicitNullDerefEvent event = { l, isLoad, N, &C.getBugReporter() };
-        dispatchEvent(event);
-      }
+
+    // At this point the value could be either null or non-null.
+    // Record this as an "implicit" null dereference.
+    if (ExplodedNode *N = C.generateSink(StNull)) {
+      ImplicitNullDerefEvent event = { V, /*isLoad=*/true, N,
+                                       &C.getBugReporter() };
+      dispatchEvent(event);
     }
   }
 
-  // From this point forward, we know that the location is not null.
-  C.addTransition(notNullState);
+  // From here on out, assume the value is non-null.
+  C.addTransition(StNonNull);
 }
 
 void ento::registerDereferenceChecker(CheckerManager &mgr) {

diff --git a/test/Analysis/initializer.cpp b/test/Analysis/initializer.cpp
index 3f008ffbbf35092940a183e907f93928e9e906f3..f7a6fd7bea97d0995db5b49d5834df847033cabc 100644 (file)
--- a/test/Analysis/initializer.cpp
+++ b/test/Analysis/initializer.cpp
@@ -43,3 +43,24 @@ void testIndirectMember() {
   IndirectMember obj(3);
   clang_analyzer_eval(obj.getX() == 3); // expected-warning{{TRUE}}
 }
+
+
+// ------------------------------------
+// False negatives
+// ------------------------------------
+
+struct RefWrapper {
+  RefWrapper(int *p) : x(*p) {}
+  RefWrapper(int &r) : x(r) {}
+  int &x;
+};
+
+void testReferenceMember() {
+  int *p = 0;
+  RefWrapper X(p); // should warn in the constructor
+}
+
+void testReferenceMember2() {
+  int *p = 0;
+  RefWrapper X(*p); // should warn here
+}

diff --git a/test/Analysis/misc-ps-region-store.cpp b/test/Analysis/misc-ps-region-store.cpp
index 381aa0331638f16e4cbd0d347885fd82f867d8a2..fcffe075360fabcf4da22c9c3e02eac45df66091 100644 (file)
--- a/test/Analysis/misc-ps-region-store.cpp
+++ b/test/Analysis/misc-ps-region-store.cpp
@@ -271,13 +271,27 @@ class Rdar9212495_A : public Rdar9212495_B {};
 const Rdar9212495_A& rdar9212495(const Rdar9212495_C* ptr) {
   const Rdar9212495_A& val = dynamic_cast<const Rdar9212495_A&>(*ptr);
   
+  // This is not valid C++; dynamic_cast with a reference type will throw an
+  // exception if the pointer does not match the expected type.
   if (&val == 0) {
-    val.bar(); // FIXME: This should eventually be a null dereference.
+    val.bar(); // no warning (unreachable)
+    int *p = 0;
+    *p = 0xDEAD; // no warning (unreachable)
   }
   
   return val;
 }
 
+const Rdar9212495_A* rdar9212495_ptr(const Rdar9212495_C* ptr) {
+  const Rdar9212495_A* val = dynamic_cast<const Rdar9212495_A*>(ptr);
+
+  if (val == 0) {
+    val->bar(); // expected-warning{{Called C++ object pointer is null}}
+  }
+
+  return val;
+}
+
 // Test constructors invalidating arguments.  Previously this raised
 // an uninitialized value warning.
 extern "C" void __attribute__((noreturn)) PR9645_exit(int i);

diff --git a/test/Analysis/reference.cpp b/test/Analysis/reference.cpp
index a12e0bd41cdce44786eb4e7ddba12aa5b703aa72..c9bfadced7992a0cca0af6395218570202f7c434 100644 (file)
--- a/test/Analysis/reference.cpp
+++ b/test/Analysis/reference.cpp
@@ -90,3 +90,29 @@ namespace PR13440 {
     clang_analyzer_eval(s2.x[0] == 42); // expected-warning{{TRUE}}
   }
 }
+
+void testRef() {
+  int *x = 0;
+  int &y = *x; // expected-warning{{Dereference of null pointer}}
+  y = 5;
+}
+
+
+// ------------------------------------
+// False negatives
+// ------------------------------------
+
+namespace rdar11212286 {
+  class B{};
+
+  B test() {
+    B *x = 0;
+    return *x; // should warn here!
+  }
+
+  B &testRef() {
+    B *x = 0;
+    return *x; // should warn here!
+  }
+
+}