Added GRStateManager::scanReachableSymbols(), a method which scans the reachable

symbols from an SVal.

- Fixed a bug in EnvironmentManager::RemoveDeadBindings() where it did not mark
  live all the symbols reachable from a live block-level expression.

- Fixed a bug in the retain/release checker where it did not stop tracking
  symbols that 'escaped' via compound literals being assigned to something the
  BasicStoreManager didn't reason about.

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

diff --git a/include/clang/Analysis/PathSensitive/Environment.h b/include/clang/Analysis/PathSensitive/Environment.h
index 7613c9ea5f61a8c48db5b45ecab16d5fb079c717..5c702d659c360e70463e610d1f728b8d1f421587 100644 (file)
--- a/include/clang/Analysis/PathSensitive/Environment.h
+++ b/include/clang/Analysis/PathSensitive/Environment.h
@@ -141,6 +141,7 @@ public:
 
   Environment
   RemoveDeadBindings(Environment Env, Stmt* Loc, SymbolReaper& SymReaper,
+                     GRStateManager& StateMgr,
                      llvm::SmallVectorImpl<const MemRegion*>& DRoots);
 
 };

diff --git a/include/clang/Analysis/PathSensitive/GRState.h b/include/clang/Analysis/PathSensitive/GRState.h
index b227f91a78f03552f354decc2046038bac7b5954..ac0908cf1f4e62d37548d8d520ea443073a5cdcc 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRState.h
+++ b/include/clang/Analysis/PathSensitive/GRState.h
@@ -610,6 +610,9 @@ public:
   void EndPath(const GRState* St) {
     ConstraintMgr->EndPath(St);
   }
+
+  bool scanReachableSymbols(nonloc::CompoundVal val, SymbolVisitor& vistor);
+  bool scanReachableSymbols(SVal val, SymbolVisitor& visitor);
 };
   
 //===----------------------------------------------------------------------===//
@@ -732,6 +735,13 @@ public:
     return GRStateRef(Mgr->Assume(St, Cond, Assumption, isFeasible), *Mgr);  
   }
   
+  template <typename CB>
+  CB scanReachableSymbols(SVal val) {
+    CB cb(*this);
+    Mgr->scanReachableSymbols(val, cb);
+    return cb;
+  }
+  
   // Pretty-printing.
   void print(std::ostream& Out, const char* nl = "\n",
              const char *sep = "") const;

diff --git a/include/clang/Analysis/PathSensitive/SymbolManager.h b/include/clang/Analysis/PathSensitive/SymbolManager.h
index 6b777be0c507d8aeec0487b1ce100170a66205dc..18c177bdd6ecc6c03a8e29bf1e345b681a209813 100644 (file)
--- a/include/clang/Analysis/PathSensitive/SymbolManager.h
+++ b/include/clang/Analysis/PathSensitive/SymbolManager.h
@@ -263,6 +263,15 @@ public:
   }
 };
   
+class SymbolVisitor {
+public:
+  // VisitSymbol - A visitor method invoked by
+  //  GRStateManager::scanReachableSymbols.  The method returns \c true if
+  //  symbols should continue be scanned and \c false otherwise.
+  virtual bool VisitSymbol(SymbolRef sym) = 0;
+  virtual ~SymbolVisitor();
+};
+  
 } // end clang namespace
 
 #endif

diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index 875c4e39b8833d56fbd67b14211d0a643559e52f..81faf21653f619522201a8be10ed772b17ae6c81 100644 (file)
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -21,6 +21,7 @@
 #include "clang/Analysis/LocalCheckers.h"
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Analysis/PathSensitive/BugReporter.h"
+#include "clang/Analysis/PathSensitive/SymbolManager.h"
 #include "clang/AST/DeclObjC.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FoldingSet.h"
@@ -1736,13 +1737,25 @@ void CFRefCount::EvalObjCMessageExpr(ExplodedNodeSet<GRState>& Dst,
   EvalSummary(Dst, Eng, Builder, ME, ME->getReceiver(), Summ,
               ME->arg_begin(), ME->arg_end(), Pred);
 }
+
+namespace {
+class VISIBILITY_HIDDEN StopTrackingCallback : public SymbolVisitor {
+  GRStateRef state;
+public:
+  StopTrackingCallback(GRStateRef st) : state(st) {}
+  GRStateRef getState() { return state; }
+
+  bool VisitSymbol(SymbolRef sym) {
+    state = state.remove<RefBindings>(sym);
+    return true;
+  }
   
-void CFRefCount::EvalBind(GRStmtNodeBuilderRef& B, SVal location, SVal val) {  
-  // Check if we have a binding for "Val" and if we are storing it to something
-  // we don't understand or otherwise the value "escapes" the function.  
-  if (!isa<loc::SymbolVal>(val))
-    return;
+  const GRState* getState() const { return state.getState(); }
+};
+} // end anonymous namespace
   
+
+void CFRefCount::EvalBind(GRStmtNodeBuilderRef& B, SVal location, SVal val) {  
   // Are we storing to something that causes the value to "escape"?  
   bool escapes = false;
   
@@ -1752,7 +1765,6 @@ void CFRefCount::EvalBind(GRStmtNodeBuilderRef& B, SVal location, SVal val) {
   // (2) we are binding to a memregion that does not have stack storage
   // (3) we are binding to a memregion with stack storage that the store
   //     does not understand.  
-  SymbolRef Sym = cast<loc::SymbolVal>(val).getSymbol();
   GRStateRef state = B.getState();
 
   if (!isa<loc::MemRegionVal>(location))
@@ -1769,15 +1781,15 @@ void CFRefCount::EvalBind(GRStmtNodeBuilderRef& B, SVal location, SVal val) {
     }
   }
 
-  // Our store can represent the binding and we aren't storing to something
-  // that doesn't have local storage.  Just return and have the simulation
-  // state continue as is.  We should also just return if the tracked symbol
-  // is not associated with a reference count.
-  if (!escapes || !state.get<RefBindings>(Sym))
-    return;
+  // If our store can represent the binding and we aren't storing to something
+  // that doesn't have local storage then just return and have the simulation
+  // state continue as is.
+  if (!escapes)
+      return;
 
-  // The tracked object excapes. Stop tracking the object.
-  B.MakeNode(state.remove<RefBindings>(Sym));
+  // Otherwise, find all symbols referenced by 'val' that we are tracking
+  // and stop tracking them.
+  B.MakeNode(state.scanReachableSymbols<StopTrackingCallback>(val).getState());
 }
 
 std::pair<GRStateRef,bool>

diff --git a/lib/Analysis/Environment.cpp b/lib/Analysis/Environment.cpp
index c4ed349906d6b020a2daee8d626777f370eb39c8..e3ed678d5e95926de41a2231d5e6b9ffdba1981d 100644 (file)
--- a/lib/Analysis/Environment.cpp
+++ b/lib/Analysis/Environment.cpp
@@ -10,11 +10,11 @@
 //  This file defined the Environment and EnvironmentManager classes.
 //
 //===----------------------------------------------------------------------===//
-
-#include "clang/Analysis/PathSensitive/Environment.h"
+#include "clang/Analysis/PathSensitive/GRState.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "llvm/ADT/ImmutableMap.h"
 #include "llvm/Support/Streams.h"
+#include "llvm/Support/Compiler.h"
 
 using namespace clang;
 
@@ -106,9 +106,19 @@ Environment EnvironmentManager::BindExpr(const Environment& Env, Stmt* E,SVal V,
   return isBlkExpr ? AddBlkExpr(Env, E, V) : AddSubExpr(Env, E, V);
 }
 
+namespace {
+class VISIBILITY_HIDDEN MarkLiveCallback : public SymbolVisitor {
+  SymbolReaper &SymReaper;
+public:
+  MarkLiveCallback(SymbolReaper &symreaper) : SymReaper(symreaper) {}  
+  bool VisitSymbol(SymbolRef sym) { SymReaper.markLive(sym); return true; }
+};
+} // end anonymous namespace
+
 Environment 
 EnvironmentManager::RemoveDeadBindings(Environment Env, Stmt* Loc,
                                        SymbolReaper& SymReaper,
+                                       GRStateManager& StateMgr,
                               llvm::SmallVectorImpl<const MemRegion*>& DRoots) {
   
   // Drop bindings for subexpressions.
@@ -126,11 +136,9 @@ EnvironmentManager::RemoveDeadBindings(Environment Env, Stmt* Loc,
       if (isa<loc::MemRegionVal>(X))
         DRoots.push_back(cast<loc::MemRegionVal>(X).getRegion());
 
-      // Mark all symbols in the block expr's value.
-      for (SVal::symbol_iterator SI = X.symbol_begin(), SE = X.symbol_end();
-           SI != SE; ++SI)
-        SymReaper.markLive(*SI);
-
+      // Mark all symbols in the block expr's value live.
+      MarkLiveCallback cb(SymReaper);
+      StateMgr.scanReachableSymbols(X, cb);
     } else {
       // The block expr is dead.
       SVal X = I.getData();

diff --git a/lib/Analysis/GRState.cpp b/lib/Analysis/GRState.cpp
index ea6f7a03d6915849f501f19cad1ef1ad3db9c9ba..0788b432ba5e91b00bf9cac1c4eb344e52504070 100644 (file)
--- a/lib/Analysis/GRState.cpp
+++ b/lib/Analysis/GRState.cpp
@@ -45,7 +45,7 @@ GRStateManager::RemoveDeadBindings(const GRState* state, Stmt* Loc,
   llvm::SmallVector<const MemRegion*, 10> RegionRoots;
   GRState NewState = *state;
 
-  NewState.Env = EnvMgr.RemoveDeadBindings(NewState.Env, Loc, SymReaper,
+  NewState.Env = EnvMgr.RemoveDeadBindings(NewState.Env, Loc, SymReaper, *this,
                                            RegionRoots);
 
   // Clean up the store.
@@ -204,6 +204,35 @@ const GRState* GRStateManager::addGDM(const GRState* St, void* Key, void* Data){
   return getPersistentState(NewSt);
 }
 
+//===----------------------------------------------------------------------===//
+// Utility.
+//===----------------------------------------------------------------------===//
+
+bool GRStateManager::scanReachableSymbols(nonloc::CompoundVal val,
+                                          SymbolVisitor& visitor) {
+  for (nonloc::CompoundVal::iterator I=val.begin(), E=val.end(); I!=E; ++I)
+    if (!scanReachableSymbols(*I, visitor)) return false;
+
+  return true;
+}
+
+bool GRStateManager::scanReachableSymbols(SVal val, SymbolVisitor& visitor) {
+  
+  // FIXME: Scan through through the reachable regions.
+  // if (isa<Loc>(val)) { ... }
+  
+  if (loc::SymbolVal *X = dyn_cast<loc::SymbolVal>(&val))
+    return visitor.VisitSymbol(X->getSymbol());
+  
+  if (nonloc::SymbolVal *X = dyn_cast<nonloc::SymbolVal>(&val))
+    return visitor.VisitSymbol(X->getSymbol());
+  
+  if (nonloc::CompoundVal *X = dyn_cast<nonloc::CompoundVal>(&val))
+    return scanReachableSymbols(*X, visitor);
+  
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 // Queries.
 //===----------------------------------------------------------------------===//

diff --git a/lib/Analysis/SymbolManager.cpp b/lib/Analysis/SymbolManager.cpp
index 1672af8c611c776c0797a3dd12a2c85817f3715c..f1c1cc0a466e6d2679435b8f85ef3551299925cb 100644 (file)
--- a/lib/Analysis/SymbolManager.cpp
+++ b/lib/Analysis/SymbolManager.cpp
@@ -99,4 +99,5 @@ bool SymbolReaper::isLive(SymbolRef sym) {
   // the analyzed function/method.
   return isa<SymbolRegionRValue>(SymMgr.getSymbolData(sym));
 }
-  
+
+SymbolVisitor::~SymbolVisitor() {}

diff --git a/test/Analysis/rdar-6582778-basic-store.c b/test/Analysis/rdar-6582778-basic-store.c
new file mode 100644 (file)
index 0000000..c0a5993
--- /dev/null
+++ b/test/Analysis/rdar-6582778-basic-store.c
@@ -0,0 +1,22 @@
+// RUN: clang -analyze -checker-cfref -analyzer-store-basic -verify %s
+
+typedef const void * CFTypeRef;
+typedef double CFTimeInterval;
+typedef CFTimeInterval CFAbsoluteTime;
+typedef const struct __CFAllocator * CFAllocatorRef;
+typedef const struct __CFDate * CFDateRef;
+
+extern CFDateRef CFDateCreate(CFAllocatorRef allocator, CFAbsoluteTime at);
+CFAbsoluteTime CFAbsoluteTimeGetCurrent(void);
+
+void f(void) {
+  CFAbsoluteTime t = CFAbsoluteTimeGetCurrent();
+  CFTypeRef vals[] = { CFDateCreate(0, t) }; // no-warning
+}
+
+CFTypeRef global;
+
+void g(void) {
+  CFAbsoluteTime t = CFAbsoluteTimeGetCurrent();
+  global = CFDateCreate(0, t); // no-warning
+}