[analyzer] Check the stack frame when looking for a var's initialization.

FindLastStoreBRVisitor is responsible for finding where a particular region
gets its value; if the region is a VarRegion, it's possible that value was
assigned at initialization, i.e. at its DeclStmt. However, if a function is
called recursively, the same DeclStmt may be evaluated multiple times in
multiple stack frames. FindLastStoreBRVisitor was not taking this into
account and just picking the first one it saw.

<rdar://problem/13787723>

llvm-svn: 180997

2 files changed, 53 insertions, 7 deletions

diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 7b970a09dcd..e078745737f 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -418,6 +418,35 @@ void FindLastStoreBRVisitor ::Profile(llvm::FoldingSetNodeID &ID) const {
   ID.AddBoolean(EnableNullFPSuppression);
 }
 
+/// Returns true if \p N represents the DeclStmt declaring and initializing
+/// \p VR.
+static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
+  Optional<PostStmt> P = N->getLocationAs<PostStmt>();
+  if (!P)
+    return false;
+
+  const DeclStmt *DS = P->getStmtAs<DeclStmt>();
+  if (!DS)
+    return false;
+
+  if (DS->getSingleDecl() != VR->getDecl())
+    return false;
+
+  const MemSpaceRegion *VarSpace = VR->getMemorySpace();
+  const StackSpaceRegion *FrameSpace = dyn_cast<StackSpaceRegion>(VarSpace);
+  if (!FrameSpace) {
+    // If we ever directly evaluate global DeclStmts, this assertion will be
+    // invalid, but this still seems preferable to silently accepting an
+    // initialization that may be for a path-sensitive variable.
+    assert(VR->getDecl()->isStaticLocal() && "non-static stackless VarRegion");
+    return true;
+  }
+
+  assert(VR->getDecl()->hasLocalStorage());
+  const LocationContext *LCtx = N->getLocationContext();
+  return FrameSpace->getStackFrame() == LCtx->getCurrentStackFrame();
+}
+
 PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
                                                        const ExplodedNode *Pred,
                                                        BugReporterContext &BRC,
@@ -432,13 +461,9 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
 
   // First see if we reached the declaration of the region.
   if (const VarRegion *VR = dyn_cast<VarRegion>(R)) {
-    if (Optional<PostStmt> P = Pred->getLocationAs<PostStmt>()) {
-      if (const DeclStmt *DS = P->getStmtAs<DeclStmt>()) {
-        if (DS->getSingleDecl() == VR->getDecl()) {
-          StoreSite = Pred;
-          InitE = VR->getDecl()->getInit();
-        }
-      }
+    if (isInitializationOfVar(Pred, VR)) {
+      StoreSite = Pred;
+      InitE = VR->getDecl()->getInit();
     }
   }
 
diff --git a/clang/test/Analysis/inlining/false-positive-suppression.c b/clang/test/Analysis/inlining/false-positive-suppression.c
index a836d9c6243..c5c6bb875ea 100644
--- a/clang/test/Analysis/inlining/false-positive-suppression.c
+++ b/clang/test/Analysis/inlining/false-positive-suppression.c
@@ -143,6 +143,27 @@ void testTrackNullVariable() {
 #endif
 }
 
+void inlinedIsDifferent(int inlined) {
+  int i;
+
+  // We were erroneously picking up the inner stack frame's initialization,
+  // even though the error occurs in the outer stack frame!
+  int *p = inlined ? &i : getNull();
+
+  if (!inlined)
+    inlinedIsDifferent(1);
+
+  *p = 1;
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Dereference of null pointer}}
+#endif
+}
+
+void testInlinedIsDifferent() {
+  // <rdar://problem/13787723>
+  inlinedIsDifferent(0);
+}
+
 
 // ---------------------------------------
 // FALSE NEGATIVES (over-suppression)