[cfe-commits] r85484 - in /cfe/trunk: lib/Analysis/RegionStore.cpp test/Analysis/misc-ps-region-store.m

[Ted Kremenek]

Author: kremenek
Date: Thu Oct 29 00:14:17 2009
New Revision: 85484

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

Modified:
    cfe/trunk/lib/Analysis/RegionStore.cpp
    cfe/trunk/test/Analysis/misc-ps-region-store.m

Modified: cfe/trunk/lib/Analysis/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RegionStore.cpp?rev=85484&r1=85483&r2=85484&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/RegionStore.cpp (original)
+++ cfe/trunk/lib/Analysis/RegionStore.cpp Thu Oct 29 00:14:17 2009
@@ -1630,6 +1630,8 @@
   // 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 @@
     }
     
     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 @@
   
   llvm::DenseSet<RBDNode> Visited;
   
+tryAgain:
   while (!WorkList.empty()) {
     RBDNode N = WorkList.back();
     WorkList.pop_back();
@@ -1740,6 +1746,21 @@
     }
   }
   
+  // 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.

Modified: cfe/trunk/test/Analysis/misc-ps-region-store.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps-region-store.m?rev=85484&r1=85483&r2=85484&view=diff

==============================================================================
--- cfe/trunk/test/Analysis/misc-ps-region-store.m (original)
+++ cfe/trunk/test/Analysis/misc-ps-region-store.m Thu Oct 29 00:14:17 2009
@@ -386,3 +386,32 @@
     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;
+}