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

Jordan Rose jordan_rose at apple.com
Thu May 2 22:47:31 PDT 2013


Author: jrose
Date: Fri May  3 00:47:31 2013
New Revision: 180997

URL: http://llvm.org/viewvc/llvm-project?rev=180997&view=rev
Log:
[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>

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/trunk/test/Analysis/inlining/false-positive-suppression.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=180997&r1=180996&r2=180997&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri May  3 00:47:31 2013
@@ -418,6 +418,35 @@ void FindLastStoreBRVisitor ::Profile(ll
   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 *FindLastStoreBRVisi
 
   // 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();
     }
   }
 

Modified: cfe/trunk/test/Analysis/inlining/false-positive-suppression.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/false-positive-suppression.c?rev=180997&r1=180996&r2=180997&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/false-positive-suppression.c (original)
+++ cfe/trunk/test/Analysis/inlining/false-positive-suppression.c Fri May  3 00:47:31 2013
@@ -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 at -2 {{Dereference of null pointer}}
+#endif
+}
+
+void testInlinedIsDifferent() {
+  // <rdar://problem/13787723>
+  inlinedIsDifferent(0);
+}
+
 
 // ---------------------------------------
 // FALSE NEGATIVES (over-suppression)





More information about the cfe-commits mailing list