retain/release checker: Stop tracking reference counts for any symbols touched by...

This fixes <rdar://problem/7257223> and <rdar://problem/7283470>.

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

diff --git a/include/clang/Analysis/PathSensitive/Store.h b/include/clang/Analysis/PathSensitive/Store.h
index 3ff253d0abf3003fa8755f918a543d9eb6063b71..7a462c579f9fa0b2e2b5669bea84519211dc56f9 100644 (file)
--- a/include/clang/Analysis/PathSensitive/Store.h
+++ b/include/clang/Analysis/PathSensitive/Store.h
@@ -141,9 +141,12 @@ public:
                                             const VarDecl *VD,
                                             const LocationContext *LC) = 0;
 
+  typedef llvm::DenseSet<SymbolRef> InvalidatedSymbols;
+  
   virtual const GRState *InvalidateRegion(const GRState *state,
                                           const MemRegion *R,
-                                          const Expr *E, unsigned Count) = 0;
+                                          const Expr *E, unsigned Count,
+                                          InvalidatedSymbols *IS) = 0;
 
   // FIXME: Make out-of-line.
   virtual const GRState *setExtent(const GRState *state,

diff --git a/lib/Analysis/BasicStore.cpp b/lib/Analysis/BasicStore.cpp
index a4f451f364906da6e7bc3f24ba6cc78d9573ab26..d81d83c7bfa225b98f54f2d5770713779eae790a 100644 (file)
--- a/lib/Analysis/BasicStore.cpp
+++ b/lib/Analysis/BasicStore.cpp
@@ -49,7 +49,8 @@ public:
                                  QualType T = QualType());
 
   const GRState *InvalidateRegion(const GRState *state, const MemRegion *R,
-                                  const Expr *E, unsigned Count);
+                                  const Expr *E, unsigned Count,
+                                  InvalidatedSymbols *IS);
 
   const GRState *Bind(const GRState *state, Loc L, SVal V) {
     return state->makeWithStore(BindInternal(state->getStore(), L, V));
@@ -623,12 +624,21 @@ StoreManager::BindingsHandler::~BindingsHandler() {}
 const GRState *BasicStoreManager::InvalidateRegion(const GRState *state,
                                                    const MemRegion *R,
                                                    const Expr *E,
-                                                   unsigned Count) {
+                                                   unsigned Count,
+                                                   InvalidatedSymbols *IS) {
   R = R->getBaseRegion();
 
   if (!(isa<VarRegion>(R) || isa<ObjCIvarRegion>(R)))
       return state;
 
+  if (IS) {
+    BindingsTy B = GetBindings(state->getStore());
+    if (BindingsTy::data_type *Val = B.lookup(R)) {
+      if (SymbolRef Sym = Val->getAsSymbol())
+        IS->insert(Sym);
+    }
+  }
+
   QualType T = cast<TypedRegion>(R)->getValueType(R->getContext());
   SVal V = ValMgr.getConjuredSymbolVal(R, E, T, Count);
   return Bind(state, loc::MemRegionVal(R), V);

diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index 49cd4151a6a2b3070bab4b0010d153498f262121..2f6425c0c74f9dce44938fa94cc08436840a8b2d 100644 (file)
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -2859,14 +2859,13 @@ void CFRefCount::EvalSummary(ExplodedNodeSet& Dst,
           // FIXME: What about layers of ElementRegions?
         }
 
-        // Is the invalidated variable something that we were tracking?
-        SymbolRef Sym = state->getSValAsScalarOrLoc(R).getAsLocSymbol();
-
-        // Remove any existing reference-count binding.
-        if (Sym)
-          state = state->remove<RefBindings>(Sym);
-
-        state = StoreMgr.InvalidateRegion(state, R, *I, Count);
+        StoreManager::InvalidatedSymbols IS;
+        state = StoreMgr.InvalidateRegion(state, R, *I, Count, &IS);
+        for (StoreManager::InvalidatedSymbols::iterator I = IS.begin(),
+             E = IS.end(); I!=E; ++I) {
+          // Remove any existing reference-count binding.
+          state = state->remove<RefBindings>(*I);
+        }
       }
       else {
         // Nuke all other arguments passed by reference.

diff --git a/lib/Analysis/RegionStore.cpp b/lib/Analysis/RegionStore.cpp
index 9456ab64542cf6e2d1978f5f28bed8a0c9686e90..18d58ab10f699eed13544e08b8a2279454c7fec9 100644 (file)
--- a/lib/Analysis/RegionStore.cpp
+++ b/lib/Analysis/RegionStore.cpp
@@ -262,7 +262,8 @@ public:
   //===-------------------------------------------------------------------===//
 
   const GRState *InvalidateRegion(const GRState *state, const MemRegion *R,
-                                  const Expr *E, unsigned Count);
+                                  const Expr *E, unsigned Count,
+                                  InvalidatedSymbols *IS);
 
 private:
   void RemoveSubRegionBindings(RegionBindings &B, const MemRegion *R,
@@ -455,7 +456,8 @@ void RegionStoreManager::RemoveSubRegionBindings(RegionBindings &B,
 const GRState *RegionStoreManager::InvalidateRegion(const GRState *state,
                                                     const MemRegion *R,
                                                     const Expr *Ex,
-                                                    unsigned Count) {
+                                                    unsigned Count,
+                                                    InvalidatedSymbols *IS) {
   ASTContext& Ctx = StateMgr.getContext();
 
   // Strip away casts.
@@ -490,9 +492,21 @@ const GRState *RegionStoreManager::InvalidateRegion(const GRState *state,
     if (Optional<SVal> V = getDirectBinding(B, R)) {
       if (const MemRegion *RV = V->getAsRegion())
         WorkList.push_back(RV);
+      
+      // A symbol?  Mark it touched by the invalidation.
+      if (IS) {
+        if (SymbolRef Sym = V->getAsSymbol())
+          IS->insert(Sym);
+      }
+    }
+
+    // Symbolic region?  Mark that symbol touched by the invalidation.
+    if (IS) {
+      if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(R))
+        IS->insert(SR->getSymbol());
     }
 
-    // Handle region.
+    // Handle the region itself.
     if (isa<AllocaRegion>(R) || isa<SymbolicRegion>(R) ||
         isa<ObjCObjectRegion>(R)) {
       // Invalidate the region by setting its default value to
@@ -1376,7 +1390,7 @@ const GRState *RegionStoreManager::Bind(const GRState *state, Loc L, SVal V) {
         // For now, just invalidate the fields of the struct/union/class.
         // FIXME: Precisely handle the fields of the record.
         if (superTy->isRecordType())
-          return InvalidateRegion(state, superR, NULL, 0);
+          return InvalidateRegion(state, superR, NULL, 0, NULL);
       }
     }
   }

diff --git a/test/Analysis/retain-release-region-store.m b/test/Analysis/retain-release-region-store.m
index c6f9cffc5f7201d2149985e5dc58a9a9a42c8832..eacac49c81271ea0641a198c91a36f53492bdd8a 100644 (file)
--- a/test/Analysis/retain-release-region-store.m
+++ b/test/Analysis/retain-release-region-store.m
@@ -1,5 +1,4 @@
 // RUN: clang-cc -analyze -checker-cfref -analyzer-store=region -verify %s
-// XFAIL
 
 //===----------------------------------------------------------------------===//
 // The following code is reduced using delta-debugging from
@@ -80,8 +79,12 @@ typedef mach_error_t DAReturn;
 typedef const struct __DADissenter * DADissenterRef;
 extern DADissenterRef DADissenterCreate( CFAllocatorRef allocator, DAReturn status, CFStringRef string );
 @interface NSNumber : NSObject
- - (id)initWithInt:(int)value;
- @end
+- (id)initWithInt:(int)value;
+@end
+typedef unsigned long NSUInteger;
+@interface NSArray : NSObject
+-(id) initWithObjects:(const id *)objects count:(NSUInteger) cnt;
+@end
 
 //===----------------------------------------------------------------------===//
 // Test cases.
@@ -124,15 +127,14 @@ CFAbsoluteTime f4() {
 }
 @end
 
-//===----------------------------------------------------------------------===//
-// <rdar://problem/7257223> - False positive due to not invalidating the
-// reference count of a tracked region that was itself invalidated.
-//===----------------------------------------------------------------------===//
+//===------------------------------------------------------------------------------------------===//
+// <rdar://problem/7257223> (also <rdar://problem/7283470>) - False positive due to not invalidating
+//  the reference count of a tracked region that was itself invalidated.
+//===------------------------------------------------------------------------------------------===//
 
 typedef struct __rdar_7257223 { CFDateRef x; } RDar7257223;
 void rdar_7257223_aux(RDar7257223 *p);
 
-// THIS CASE CURRENTLY FAILS.
 CFDateRef rdar7257223_Create(void) {
   RDar7257223 s;
   CFAbsoluteTime t = CFAbsoluteTimeGetCurrent();
@@ -148,10 +150,6 @@ CFDateRef rdar7257223_Create_2(void) {
   return s.x;
 }
 
-//===----------------------------------------------------------------------===//
-// <rdar://problem/7283470>
-//===----------------------------------------------------------------------===//
-
 void rdar7283470(void) {
   NSNumber *numbers[] = {
     [[NSNumber alloc] initWithInt:1], // no-warning
@@ -175,3 +173,35 @@ void rdar7283470_positive(void) {
   };
 }
 
+void rdar7283470_2(void) {
+  NSNumber *numbers[] = {
+    [[NSNumber alloc] initWithInt:1], // no-warning
+    [[NSNumber alloc] initWithInt:2], // no-warning
+    [[NSNumber alloc] initWithInt:3], // no-warning
+    [[NSNumber alloc] initWithInt:4], // no-warning
+    [[NSNumber alloc] initWithInt:5]  // no-warning
+  };
+  
+  NSArray *s_numbers =[[NSArray alloc] initWithObjects:&numbers[0] count:sizeof(numbers) / sizeof(numbers[0])];
+  
+  for (unsigned i = 0 ; i < sizeof(numbers) / sizeof(numbers[0]) ; ++i)
+    [numbers[i] release];
+  
+  [s_numbers release];
+}
+
+void rdar7283470_2_positive(void) {
+  NSNumber *numbers[] = {
+    [[NSNumber alloc] initWithInt:1], // no-warning
+    [[NSNumber alloc] initWithInt:2], // no-warning
+    [[NSNumber alloc] initWithInt:3], // no-warning
+    [[NSNumber alloc] initWithInt:4], // no-warning
+    [[NSNumber alloc] initWithInt:5]  // no-warning
+  };
+  
+  NSArray *s_numbers =[[NSArray alloc] initWithObjects: &numbers[0] count:sizeof(numbers) / sizeof(numbers[0])]; // expected-warning{{leak}}
+  
+  for (unsigned i = 0 ; i < sizeof(numbers) / sizeof(numbers[0]) ; ++i)
+    [numbers[i] release];
+}
+