[cfe-commits] r100067 - in /cfe/trunk: lib/Checker/RegionStore.cpp test/Analysis/retain-release-region-store.m

Ted Kremenek kremenek at apple.com
Wed Mar 31 17:15:55 PDT 2010


Author: kremenek
Date: Wed Mar 31 19:15:55 2010
New Revision: 100067

URL: http://llvm.org/viewvc/llvm-project?rev=100067&view=rev
Log:
Fix a bug (PR 6699) in RegionStore::RemoveDeadBindings() where
array values with a non-zero offset would get prematurely pruned from the store.

Modified:
    cfe/trunk/lib/Checker/RegionStore.cpp
    cfe/trunk/test/Analysis/retain-release-region-store.m

Modified: cfe/trunk/lib/Checker/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/RegionStore.cpp?rev=100067&r1=100066&r2=100067&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/RegionStore.cpp (original)
+++ cfe/trunk/lib/Checker/RegionStore.cpp Wed Mar 31 19:15:55 2010
@@ -536,15 +536,15 @@
         // First visit the cluster.
       static_cast<DERIVED*>(this)->VisitCluster(baseR, C->begin(), C->end());
 
-        // Next, visit the region.
-      static_cast<DERIVED*>(this)->VisitRegion(baseR);
+        // Next, visit the base region.
+      static_cast<DERIVED*>(this)->VisitBaseRegion(baseR);
     }
   }
 
 public:
   void VisitAddedToCluster(const MemRegion *baseR, RegionCluster &C) {}
   void VisitCluster(const MemRegion *baseR, BindingKey *I, BindingKey *E) {}
-  void VisitRegion(const MemRegion *baseR) {}
+  void VisitBaseRegion(const MemRegion *baseR) {}
 };
 }
 
@@ -580,7 +580,7 @@
       Ex(ex), Count(count), IS(is) {}
 
   void VisitCluster(const MemRegion *baseR, BindingKey *I, BindingKey *E);
-  void VisitRegion(const MemRegion *baseR);
+  void VisitBaseRegion(const MemRegion *baseR);
 
 private:
   void VisitBinding(SVal V);
@@ -627,7 +627,7 @@
   }
 }
 
-void InvalidateRegionsWorker::VisitRegion(const MemRegion *baseR) {
+void InvalidateRegionsWorker::VisitBaseRegion(const MemRegion *baseR) {
   if (IS) {
     // Symbolic region?  Mark that symbol touched by the invalidation.
     if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(baseR))
@@ -1706,8 +1706,8 @@
   // Called by ClusterAnalysis.
   void VisitAddedToCluster(const MemRegion *baseR, RegionCluster &C);
   void VisitCluster(const MemRegion *baseR, BindingKey *I, BindingKey *E);
-  void VisitRegion(const MemRegion *baseR);
 
+  void VisitBindingKey(BindingKey K);
   bool UpdatePostponed();
   void VisitBinding(SVal V);
 };
@@ -1744,11 +1744,8 @@
 
 void RemoveDeadBindingsWorker::VisitCluster(const MemRegion *baseR,
                                             BindingKey *I, BindingKey *E) {
-  for ( ; I != E; ++I) {
-    const MemRegion *R = I->getRegion();
-    if (R != baseR)
-      VisitRegion(R);
-  }
+  for ( ; I != E; ++I)
+    VisitBindingKey(*I);
 }
 
 void RemoveDeadBindingsWorker::VisitBinding(SVal V) {
@@ -1776,34 +1773,36 @@
     SymReaper.markLive(*SI);
 }
 
-void RemoveDeadBindingsWorker::VisitRegion(const MemRegion *R) {
+void RemoveDeadBindingsWorker::VisitBindingKey(BindingKey K) {
+  const MemRegion *R = K.getRegion();
+
   // Mark this region "live" by adding it to the worklist.  This will cause
   // use to visit all regions in the cluster (if we haven't visited them
   // already).
-  AddToWorkList(R);
+  if (AddToWorkList(R)) {
+    // Mark the symbol for any live SymbolicRegion as "live".  This means we
+    // should continue to track that symbol.
+    if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(R))
+      SymReaper.markLive(SymR->getSymbol());
+
+    // For BlockDataRegions, enqueue the VarRegions for variables marked
+    // with __block (passed-by-reference).
+    // via BlockDeclRefExprs.
+    if (const BlockDataRegion *BD = dyn_cast<BlockDataRegion>(R)) {
+      for (BlockDataRegion::referenced_vars_iterator
+           RI = BD->referenced_vars_begin(), RE = BD->referenced_vars_end();
+           RI != RE; ++RI) {
+        if ((*RI)->getDecl()->getAttr<BlocksAttr>())
+          AddToWorkList(*RI);
+      }
 
-  // Mark the symbol for any live SymbolicRegion as "live".  This means we
-  // should continue to track that symbol.
-  if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(R))
-    SymReaper.markLive(SymR->getSymbol());
-
-  // For BlockDataRegions, enqueue the VarRegions for variables marked
-  // with __block (passed-by-reference).
-  // via BlockDeclRefExprs.
-  if (const BlockDataRegion *BD = dyn_cast<BlockDataRegion>(R)) {
-    for (BlockDataRegion::referenced_vars_iterator
-         RI = BD->referenced_vars_begin(), RE = BD->referenced_vars_end();
-         RI != RE; ++RI) {
-      if ((*RI)->getDecl()->getAttr<BlocksAttr>())
-        AddToWorkList(*RI);
+      // No possible data bindings on a BlockDataRegion.
+      return;
     }
-
-    // No possible data bindings on a BlockDataRegion.
-    return;
   }
 
-  // Get the data binding for R (if any).
-  if (const Optional<SVal> &V = RM.getBinding(B, R))
+  // Visit the data binding for K.
+  if (const SVal *V = RM.Lookup(B, K))
     VisitBinding(*V);
 }
 

Modified: cfe/trunk/test/Analysis/retain-release-region-store.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release-region-store.m?rev=100067&r1=100066&r2=100067&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/retain-release-region-store.m (original)
+++ cfe/trunk/test/Analysis/retain-release-region-store.m Wed Mar 31 19:15:55 2010
@@ -207,3 +207,19 @@
     [numbers[i] release];
 }
 
+void pr6699(int x) {
+  CFDateRef values[2];
+  values[0] = values[1] = 0;
+
+  if (x) {
+    CFAbsoluteTime t = CFAbsoluteTimeGetCurrent();
+    values[1] = CFDateCreate(0, t);
+  }
+
+  if (values[1]) {
+    // A bug in RegionStore::RemoveDeadBindings caused 'values[1]' to get prematurely
+    // pruned from the store.
+    CFRelease(values[1]); // no-warning
+  }
+}
+





More information about the cfe-commits mailing list