From: Ted Kremenek Date: Fri, 16 Oct 2009 00:30:49 +0000 (+0000) Subject: retain/release checker: Stop tracking reference counts for any symbols touched by... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=473e16745a6f3370ba3ab6fe70bff43b1c8b2ab9;p=clang retain/release checker: Stop tracking reference counts for any symbols touched by StoreManager::InvalidateRegion(). This fixes and . 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 3ff253d0ab..7a462c579f 100644 --- 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 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 a4f451f364..d81d83c7bf 100644 --- 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(R) || isa(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(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 49cd4151a6..2f6425c0c7 100644 --- 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(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(*I); + } } else { // Nuke all other arguments passed by reference. diff --git a/lib/Analysis/RegionStore.cpp b/lib/Analysis/RegionStore.cpp index 9456ab6454..18d58ab10f 100644 --- 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 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(R)) + IS->insert(SR->getSymbol()); } - // Handle region. + // Handle the region itself. if (isa(R) || isa(R) || isa(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 c6f9cffc5f..eacac49c81 100644 --- 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 -//===----------------------------------------------------------------------===// -// - False positive due to not invalidating the -// reference count of a tracked region that was itself invalidated. -//===----------------------------------------------------------------------===// +//===------------------------------------------------------------------------------------------===// +// (also ) - 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; } -//===----------------------------------------------------------------------===// -// -//===----------------------------------------------------------------------===// - 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]; +} +