Fix a bug (PR 6699) in RegionStore::RemoveDeadBindings() where

array values with a non-zero offset would get prematurely pruned from the store.

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

diff --git a/lib/Checker/RegionStore.cpp b/lib/Checker/RegionStore.cpp
index d590a86a81cbaf4af602ce8fdd1d3d40f217cf4d..b333a5d2830e3203e7c59fa7fa9270d1b4ad59d0 100644 (file)
--- a/lib/Checker/RegionStore.cpp
+++ b/lib/Checker/RegionStore.cpp
@@ -536,15 +536,15 @@ public:
         // 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 @@ public:
       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::VisitCluster(const MemRegion *baseR,
   }
 }
 
-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 @@ public:
   // 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::VisitAddedToCluster(const MemRegion *baseR,
 
 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 @@ void RemoveDeadBindingsWorker::VisitBinding(SVal V) {
     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);
-
-  // 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());
+  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);
+      }
 
-  // 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);
 }
 

diff --git a/test/Analysis/retain-release-region-store.m b/test/Analysis/retain-release-region-store.m
index 111d4b92d126dbb4e3362a16322b4189f2783e14..db49b91c27f56342d59324a10c9f6e6996814324 100644 (file)
--- a/test/Analysis/retain-release-region-store.m
+++ b/test/Analysis/retain-release-region-store.m
@@ -207,3 +207,19 @@ void rdar7283470_2_positive(void) {
     [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
+  }
+}
+