[analyzer] Don't reinitialize static globals more than once along a path

This patch makes sure that we do not reinitialize static globals when
the function is called more than once along a path. The motivation is
code with initialization patterns that rely on 2 static variables, where
one of them has an initializer while the other does not. Currently, we
reset the static variables with initializers on every visit to the
function along a path.

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

diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index ea2fc3c5b29a6cf6ae26daef239f2c51ba19bf60..a30d589d87f25fc22ff3bed15497afd2c68cf2e4 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -423,37 +423,53 @@ void ExprEngine::VisitCompoundLiteralExpr(const CompoundLiteralExpr *CL,
     B.generateNode(CL, Pred, state->BindExpr(CL, LC, ILV));
 }
 
+/// The GDM component containing the set of global variables which have been
+/// previously initialized with explicit initializers.
+REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedGlobalsSet,
+                                 llvm::ImmutableSet<const VarDecl *> )
+
 void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
                                ExplodedNodeSet &Dst) {
-  
-  // FIXME: static variables may have an initializer, but the second
-  //  time a function is called those values may not be current.
-  //  This may need to be reflected in the CFG.
-  
   // Assumption: The CFG has one DeclStmt per Decl.
   const Decl *D = *DS->decl_begin();
-  
-  if (!D || !isa<VarDecl>(D)) {
+  const VarDecl *VD = dyn_cast_or_null<VarDecl>(D);
+
+  if (!D || !VD) {
     //TODO:AZ: remove explicit insertion after refactoring is done.
     Dst.insert(Pred);
     return;
   }
+
+  // Check if a value has been previously initialized. There will be an entry in
+  // the set for variables with global storage which have been previously
+  // initialized.
+  if (VD->hasGlobalStorage())
+    if (Pred->getState()->contains<InitializedGlobalsSet>(VD)) {
+      Dst.insert(Pred);
+      return;
+    }
   
   // FIXME: all pre/post visits should eventually be handled by ::Visit().
   ExplodedNodeSet dstPreVisit;
   getCheckerManager().runCheckersForPreStmt(dstPreVisit, Pred, DS, *this);
   
   StmtNodeBuilder B(dstPreVisit, Dst, *currBldrCtx);
-  const VarDecl *VD = dyn_cast<VarDecl>(D);
   for (ExplodedNodeSet::iterator I = dstPreVisit.begin(), E = dstPreVisit.end();
        I!=E; ++I) {
     ExplodedNode *N = *I;
     ProgramStateRef state = N->getState();
-    
-    // Decls without InitExpr are not initialized explicitly.
     const LocationContext *LC = N->getLocationContext();
-    
+
+    // Decls without InitExpr are not initialized explicitly.
     if (const Expr *InitEx = VD->getInit()) {
+
+      // Note in the state that the initialization has occurred.
+      ExplodedNode *UpdatedN = N;
+      if (VD->hasGlobalStorage()) {
+        state = state->add<InitializedGlobalsSet>(VD);
+        UpdatedN = B.generateNode(DS, N, state);
+      }
+
       SVal InitVal = state->getSVal(InitEx, LC);
 
       if (InitVal == state->getLValue(VD, LC) ||
@@ -461,7 +477,7 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
            isa<CXXConstructExpr>(InitEx->IgnoreImplicit()))) {
         // We constructed the object directly in the variable.
         // No need to bind anything.
-        B.generateNode(DS, N, state);
+        B.generateNode(DS, UpdatedN, state);
       } else {
         // We bound the temp obj region to the CXXConstructExpr. Now recover
         // the lazy compound value when the variable is not a reference.
@@ -482,9 +498,11 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
           InitVal = svalBuilder.conjureSymbolVal(0, InitEx, LC, Ty,
                                                  currBldrCtx->blockCount());
         }
-        B.takeNodes(N);
+
+
+        B.takeNodes(UpdatedN);
         ExplodedNodeSet Dst2;
-        evalBind(Dst2, DS, N, state->getLValue(VD, LC), InitVal, true);
+        evalBind(Dst2, DS, UpdatedN, state->getLValue(VD, LC), InitVal, true);
         B.addNodes(Dst2);
       }
     }

diff --git a/test/Analysis/global-region-invalidation.c b/test/Analysis/global-region-invalidation.c
index 2d64b49a8baff68592acf6c68f2b050a11d1efea..113e6ae36346ad5c6ffc821ad02eb711c255b1ac 100644 (file)
--- a/test/Analysis/global-region-invalidation.c
+++ b/test/Analysis/global-region-invalidation.c
@@ -84,3 +84,27 @@ void testAnalyzerEvalIsPure() {
   }
 }
 
+// Test that static variables with initializers do not get reinitialized on
+// recursive calls.
+void Function2(void);
+int *getPtr();
+void Function1(void) {
+  static unsigned flag;
+  static int *p = 0;
+  if (!flag) {
+    flag = 1;
+    p = getPtr();
+  }
+  int m = *p; // no-warning: p is never null.
+  m++;
+  Function2();
+}
+void Function2(void) {
+    Function1();
+}
+
+void SetToNonZero(void) {
+  static int g = 5;
+  clang_analyzer_eval(g == 5); // expected-warning{{TRUE}}
+}
+

diff --git a/test/Analysis/global_region_invalidation.mm b/test/Analysis/global_region_invalidation.mm
new file mode 100644 (file)
index 0000000..0d7f172
--- /dev/null
+++ b/test/Analysis/global_region_invalidation.mm
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+void use(int);
+id foo(int x) {
+  if (x)
+    return 0;
+  static id p = foo(1); 
+    clang_analyzer_eval(p == 0); // expected-warning{{TRUE}}
+  return p;
+}
\ No newline at end of file