3 files changed, 70 insertions, 19 deletions

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();
 }