[analyzer] Check that an ObjCIvarRefExpr's base is non-null even as an lvalue.

Like with struct fields, we want to catch cases like this early,
so that we can produce better diagnostics and path notes:

  PointObj *p = nil;
  int *px = &p->_x; // should warn here
  *px = 1;

llvm-svn: 164442

3 files changed, 22 insertions, 14 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index 45d854b0087..b2922f2c5fc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -91,6 +91,8 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
     BT_null.reset(new BuiltinBug("Dereference of null pointer"));
 
   SmallString<100> buf;
+  llvm::raw_svector_ostream os(buf);
+
   SmallVector<SourceRange, 2> Ranges;
 
   // Walk through lvalue casts to get the original expression
@@ -112,7 +114,6 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
 
   switch (S->getStmtClass()) {
   case Stmt::ArraySubscriptExprClass: {
-    llvm::raw_svector_ostream os(buf);
     os << "Array access";
     const ArraySubscriptExpr *AE = cast<ArraySubscriptExpr>(S);
     AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
@@ -121,7 +122,6 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
     break;
   }
   case Stmt::UnaryOperatorClass: {
-    llvm::raw_svector_ostream os(buf);
     os << "Dereference of null pointer";
     const UnaryOperator *U = cast<UnaryOperator>(S);
     AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(),
@@ -131,7 +131,6 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
   case Stmt::MemberExprClass: {
     const MemberExpr *M = cast<MemberExpr>(S);
     if (M->isArrow() || bugreporter::isDeclRefExprToReference(M->getBase())) {
-      llvm::raw_svector_ostream os(buf);
       os << "Access to field '" << M->getMemberNameInfo()
          << "' results in a dereference of a null pointer";
       AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(),
@@ -141,21 +140,17 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
   }
   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());
+    os << "Access to instance variable '" << *IV->getDecl()
+       << "' results in a dereference of a null pointer";
+    AddDerefSource(os, Ranges, IV->getBase()->IgnoreParenCasts(),
+                   State.getPtr(), N->getLocationContext(), true);
     break;
   }
   default:
     break;
   }
 
+  os.flush();
   BugReport *report =
     new BugReport(*BT_null,
                   buf.empty() ? BT_null->getDescription() : buf.str(),
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index be946842fc3..422fde71590 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -65,6 +65,9 @@ const Stmt *bugreporter::GetDerefExpr(const ExplodedNode *N) {
         return ME->getBase()->IgnoreParenCasts();
       }
     }
+    else if (const ObjCIvarRefExpr *IvarRef = dyn_cast<ObjCIvarRefExpr>(S)) {
+      return IvarRef->getBase()->IgnoreParenCasts();
+    }
     else if (const ArraySubscriptExpr *AE = dyn_cast<ArraySubscriptExpr>(S)) {
       return AE->getBase();
     }
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
index 51dda19b531..ee315a4604a 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -25,11 +25,21 @@ void ExprEngine::VisitLvalObjCIvarRefExpr(const ObjCIvarRefExpr *Ex,
   ProgramStateRef state = Pred->getState();
   const LocationContext *LCtx = Pred->getLocationContext();
   SVal baseVal = state->getSVal(Ex->getBase(), LCtx);
+
+  // First check that the base object is valid.
+  ExplodedNodeSet DstLoc;
+  evalLocation(DstLoc, Ex, Ex, Pred, state, baseVal,
+               /*Tag=*/0, /*isLoad=*/true);
+
+  // Bind the lvalue to the expression.
   SVal location = state->getLValue(Ex->getDecl(), baseVal);
   
   ExplodedNodeSet dstIvar;
-  StmtNodeBuilder Bldr(Pred, dstIvar, *currBldrCtx);
-  Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, location));
+  StmtNodeBuilder Bldr(DstLoc, dstIvar, *currBldrCtx);
+  for (ExplodedNodeSet::iterator I = DstLoc.begin(), E = DstLoc.end();
+       I != E; ++I) {
+    Bldr.generateNode(Ex, (*I), (*I)->getState()->BindExpr(Ex, LCtx, location));
+  }
   
   // Perform the post-condition check of the ObjCIvarRefExpr and store
   // the created nodes in 'Dst'.