Fix an insidious bug in RegionStore::RemoveDeadBindings() pointed out

by Zhongxing Xu.  RemoveDeadBindings() would falsely prune
SymbolicRegions from the store that wrapped derived symbols whose
liveness could only be determined after scanning the store.


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

2 files changed, 52 insertions, 2 deletions

diff --git a/lib/Analysis/RegionStore.cpp b/lib/Analysis/RegionStore.cpp
index 780772a6f1..5cfe4c0410 100644
--- a/lib/Analysis/RegionStore.cpp
+++ b/lib/Analysis/RegionStore.cpp
@@ -1630,6 +1630,8 @@ void RegionStoreManager::RemoveDeadBindings(GRState &state, Stmt* Loc,
   // Process the "intermediate" roots to find if they are referenced by
   // real roots.
   llvm::SmallVector<RBDNode, 10> WorkList;
+  llvm::SmallVector<RBDNode, 10> Postponed;
+
   llvm::DenseSet<const MemRegion*> IntermediateVisited;
   
   while (!IntermediateRoots.empty()) {
@@ -1647,8 +1649,11 @@ void RegionStoreManager::RemoveDeadBindings(GRState &state, Stmt* Loc,
     }
     
     if (const SymbolicRegion* SR = dyn_cast<SymbolicRegion>(R)) {
-      if (SymReaper.isLive(SR->getSymbol()))
-        WorkList.push_back(std::make_pair(&state, SR));
+      llvm::SmallVectorImpl<RBDNode> &Q =      
+        SymReaper.isLive(SR->getSymbol()) ? WorkList : Postponed;
+      
+        Q.push_back(std::make_pair(&state, SR));
+
       continue;
     }
     
@@ -1667,6 +1672,7 @@ void RegionStoreManager::RemoveDeadBindings(GRState &state, Stmt* Loc,
   
   llvm::DenseSet<RBDNode> Visited;
   
+tryAgain:
   while (!WorkList.empty()) {
     RBDNode N = WorkList.back();
     WorkList.pop_back();
@@ -1740,6 +1746,21 @@ void RegionStoreManager::RemoveDeadBindings(GRState &state, Stmt* Loc,
     }
   }
   
+  // See if any postponed SymbolicRegions are actually live now, after
+  // having done a scan.
+  for (llvm::SmallVectorImpl<RBDNode>::iterator I = Postponed.begin(),
+       E = Postponed.end() ; I != E ; ++I) {    
+    if (const SymbolicRegion *SR = cast_or_null<SymbolicRegion>(I->second)) {
+      if (SymReaper.isLive(SR->getSymbol())) {
+        WorkList.push_back(*I);
+        I->second = NULL;
+      }
+    }
+  }
+  
+  if (!WorkList.empty())
+    goto tryAgain;
+  
   // We have now scanned the store, marking reachable regions and symbols
   // as live.  We now remove all the regions that are dead from the store
   // as well as update DSymbols with the set symbols that are now dead.
diff --git a/test/Analysis/misc-ps-region-store.m b/test/Analysis/misc-ps-region-store.m
index a430a44db6..5bba63a3a2 100644
--- a/test/Analysis/misc-ps-region-store.m
+++ b/test/Analysis/misc-ps-region-store.m
@@ -386,3 +386,32 @@ void rdar_7332673_test2() {
     if ( rdar_7332673_test2_aux(value) != 1 ) {} // expected-warning{{Pass-by-value argument in function call is undefined}}
 }
 
+//===----------------------------------------------------------------------===//
+// <rdar://problem/7347252>: Because of a bug in
+//   RegionStoreManager::RemoveDeadBindings(), the symbol for s->session->p
+//   would incorrectly be pruned from the state after the call to
+//   rdar7347252_malloc1(), and would incorrectly result in a warning about
+//   passing a null pointer to rdar7347252_memcpy().
+//===----------------------------------------------------------------------===//
+
+struct rdar7347252_AA { char *p;};
+typedef struct {
+ struct rdar7347252_AA *session;
+ int t;
+ char *q;
+} rdar7347252_SSL1;
+
+int rdar7347252_f(rdar7347252_SSL1 *s);
+char *rdar7347252_malloc1(int);
+char *rdar7347252_memcpy1(char *d, char *s, int n) __attribute__((nonnull (1,2)));
+
+int rdar7347252(rdar7347252_SSL1 *s) {
+ rdar7347252_f(s);  // the SymbolicRegion of 's' is set a default binding of conjured symbol
+ if (s->session->p == ((void*)0)) {
+   if ((s->session->p = rdar7347252_malloc1(10)) == ((void*)0)) {
+     return 0;
+   }
+   rdar7347252_memcpy1(s->session->p, "aa", 2); // no-warning
+ }
+ return 0;
+}