[analyzer] tracking stores/constraints now works for ObjC ivars or struct fields.

This required more changes than I originally expected:

- ObjCIvarRegion implements "canPrintPretty" et al
- DereferenceChecker indicates the null pointer source is an ivar
- bugreporter::trackNullOrUndefValue() uses an alternate algorithm
  to compute the location region to track by scouring the ExplodedGraph.
  This allows us to get the actual MemRegion for variables, ivars,
  fields, etc.  We only hand construct a VarRegion for C++ references.
- ExplodedGraph no longer drops nodes for expressions that are marked
  'lvalue'.  This is to facilitate the logic in the previous bullet.
  This may lead to a slight increase in size in the ExplodedGraph,
  which I have not measured, but it is likely not to be a big deal.

I have validated each of the changed plist output.

Fixes <rdar://problem/12114812>

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

4 files changed, 78 insertions, 19 deletions

diff --git a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index 3aa8aaac0d..72d46c50e1 100644
--- a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -76,6 +76,14 @@ DereferenceChecker::AddDerefSource(raw_ostream &os,
       Ranges.push_back(SourceRange(L, L));
       break;
     }
+    case Stmt::ObjCIvarRefExprClass: {
+      const ObjCIvarRefExpr *IV = cast<ObjCIvarRefExpr>(Ex);
+      os << " (" << (loadedFrom ? "loaded from" : "via")
+         << " ivar '" << IV->getDecl()->getName() << "')";
+      SourceLocation L = IV->getLocation();
+      Ranges.push_back(SourceRange(L, L));
+      break;
+    }    
   }
 }
 
diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index f59458ca6e..cb716a3154 100644
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -662,14 +662,47 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S,
     // C++ user-defined implicit conversions, because those have a constructor
     // or function call inside.
     Ex = Ex->IgnoreParenCasts();
-    if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(Ex)) {
-      // FIXME: Right now we only track VarDecls because it's non-trivial to
-      // get a MemRegion for any other DeclRefExprs. <rdar://problem/12114812>
-      if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
-        ProgramStateManager &StateMgr = state->getStateManager();
-        MemRegionManager &MRMgr = StateMgr.getRegionManager();
-        const VarRegion *R = MRMgr.getVarRegion(VD, N->getLocationContext());
 
+    if (Ex->isLValue()) {
+      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>(Ex)) {
+        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());
+          }
+        }
+      }
+
+      // 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;
+        const Expr *SearchEx = Ex;
+        if (const OpaqueValueExpr *OPE = dyn_cast<OpaqueValueExpr>(Ex)) {
+          SearchEx = OPE->getSourceExpr();
+        }
+        while (LVNode) {
+          if (Optional<PostStmt> P = LVNode->getLocation().getAs<PostStmt>()) {
+            if (P->getStmt() == SearchEx)
+              break;
+          }
+          LVNode = LVNode->getFirstPred();
+        }
+        assert(LVNode && "Unable to find the lvalue node.");
+        ProgramStateRef LVState = LVNode->getState();
+        if (Optional<Loc> L =
+              LVState->getSVal(Ex, LVNode->getLocationContext()).getAs<Loc>()) {
+          R = L->getAsRegion();
+        }
+      }
+
+      if (R) {
         // Mark both the variable region and its contents as interesting.
         SVal V = state->getRawSVal(loc::MemRegionVal(R));
 
@@ -692,6 +725,12 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S,
         report.markInteresting(V);
         report.addVisitor(new UndefOrNullArgVisitor(R));
 
+        if (isa<SymbolicRegion>(R)) {
+          TrackConstraintBRVisitor *VI =
+            new TrackConstraintBRVisitor(loc::MemRegionVal(R), false);
+          report.addVisitor(VI);
+        }
+
         // If the contents are symbolic, find out when they became null.
         if (V.getAsLocSymbol()) {
           BugReporterVisitor *ConstraintTracker =
@@ -706,8 +745,8 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S,
     }
   }
 
-  // If the expression does NOT refer to a variable, we can still track
-  // constraints on its contents.
+  // If the expression is not an "lvalue expression", we can still
+  // track the constraints on its contents.
   SVal V = state->getSValAsScalarOrLoc(S, N->getLocationContext());
 
   // Uncomment this to find cases where we aren't properly getting the
diff --git a/lib/StaticAnalyzer/Core/ExplodedGraph.cpp b/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
index a72f49d805..443d87076a 100644
--- a/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ b/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -66,9 +66,10 @@ bool ExplodedGraph::shouldCollect(const ExplodedNode *node) {
   // (5) The 'store' is the same as the predecessor.
   // (6) The 'GDM' is the same as the predecessor.
   // (7) The LocationContext is the same as the predecessor.
-  // (8) The PostStmt isn't for a non-consumed Stmt or Expr.
-  // (9) The successor is not a CallExpr StmtPoint (so that we would be able to
-  //     find it when retrying a call with no inlining).
+  // (8) Expressions that are *not* lvalue expressions.
+  // (9) The PostStmt isn't for a non-consumed Stmt or Expr.
+  // (10) The successor is not a CallExpr StmtPoint (so that we would
+  //      be able to find it when retrying a call with no inlining).
   // FIXME: It may be safe to reclaim PreCall and PostCall nodes as well.
 
   // Conditions 1 and 2.
@@ -99,20 +100,23 @@ bool ExplodedGraph::shouldCollect(const ExplodedNode *node) {
   if (state->store != pred_state->store || state->GDM != pred_state->GDM ||
       progPoint.getLocationContext() != pred->getLocationContext())
     return false;
-  
+
   // Condition 8.
-  // Do not collect nodes for non-consumed Stmt or Expr to ensure precise
-  // diagnostic generation; specifically, so that we could anchor arrows
-  // pointing to the beginning of statements (as written in code).
+  // Do not collect nodes for lvalue expressions since they are
+  // used extensively for generating path diagnostics.
   const Expr *Ex = dyn_cast<Expr>(ps.getStmt());
-  if (!Ex)
+  if (!Ex || Ex->isLValue())
     return false;
 
+  // Condition 9.
+  // Do not collect nodes for non-consumed Stmt or Expr to ensure precise
+  // diagnostic generation; specifically, so that we could anchor arrows
+  // pointing to the beginning of statements (as written in code).
   ParentMap &PM = progPoint.getLocationContext()->getParentMap();
   if (!PM.isConsumedExpr(Ex))
     return false;
-  
-  // Condition 9.
+
+  // Condition 10.
   const ProgramPoint SuccLoc = succ->getLocation();
   if (Optional<StmtPoint> SP = SuccLoc.getAs<StmtPoint>())
     if (CallEvent::isCallStmt(SP->getStmt()))
diff --git a/lib/StaticAnalyzer/Core/MemRegion.cpp b/lib/StaticAnalyzer/Core/MemRegion.cpp
index a3e42eaa46..e1b8b186c7 100644
--- a/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -566,6 +566,14 @@ void VarRegion::printPretty(raw_ostream &os) const {
   os << getDecl()->getName();
 }
 
+bool ObjCIvarRegion::canPrintPretty() const {
+  return true;
+}
+
+void ObjCIvarRegion::printPretty(raw_ostream &os) const {
+  os << getDecl()->getName();
+}
+
 bool FieldRegion::canPrintPretty() const {
   return superRegion->canPrintPretty();
 }