From 62a5c34ddc54696725683f6c5af1c8e1592c5c38 Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Fri, 30 Mar 2012 05:48:16 +0000 Subject: [PATCH] [analyzer]Malloc,RetainRelease: Allow pointer to escape via NSMapInsert. Fixes a false positive (radar://11152419). The current solution of adding the info into 3 places is quite ugly. Pending a generic pointer escapes callback. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@153731 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 5 +++++ .../Checkers/RetainCountChecker.cpp | 7 +++++++ .../Core/ExprEngineCallAndReturn.cpp | 4 ++++ test/Analysis/malloc.mm | 19 +++++++++++++++++++ test/Analysis/retain-release.mm | 19 +++++++++++++++++++ test/Analysis/system-header-simulator-objc.h | 1 + 6 files changed, 55 insertions(+) diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 7b9adb7c15..7456af2344 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -1262,6 +1262,11 @@ bool MallocChecker::doesNotFreeMemory(const CallOrObjCMessage *Call, return false; } + // Whitelist NSXXInsertXX, for example NSMapInsertIfAbsent, since they can + // be deallocated by NSMapRemove. + if (FName.startswith("NS") && (FName.find("Insert") != StringRef::npos)) + return false; + // Otherwise, assume that the function does not free memory. // Most system calls, do not free the memory. return true; diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index a6d33ad2a0..7fa6975478 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -993,6 +993,13 @@ const RetainSummary * RetainSummaryManager::getSummary(const FunctionDecl *FD) { // libdispatch finalizers. ScratchArgs = AF.add(ScratchArgs, 1, StopTracking); S = getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + } else if (FName.startswith("NS") && + (FName.find("Insert") != StringRef::npos)) { + // Whitelist NSXXInsertXX, for example NSMapInsertIfAbsent, since they can + // be deallocated by NSMapRemove. (radar://11152419) + ScratchArgs = AF.add(ScratchArgs, 1, StopTracking); + ScratchArgs = AF.add(ScratchArgs, 2, StopTracking); + S = getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); } // Did we get a summary? diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index fead0862fa..16f5d0bb1a 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -245,9 +245,13 @@ static void findPtrToConstParams(llvm::SmallSet &PreserveArgs, // in buffer. // - Many CF containers allow objects to escape through custom // allocators/deallocators upon container construction. + // - NSXXInsertXX, for example NSMapInsertIfAbsent, since they can + // be deallocated by NSMapRemove. if (FName == "pthread_setspecific" || FName == "funopen" || FName.endswith("NoCopy") || + (FName.startswith("NS") && + (FName.find("Insert") != StringRef::npos)) || Call.isCFCGAllowingEscape(FName)) return; } diff --git a/test/Analysis/malloc.mm b/test/Analysis/malloc.mm index fe14edeedd..d2409ac160 100644 --- a/test/Analysis/malloc.mm +++ b/test/Analysis/malloc.mm @@ -106,6 +106,25 @@ void testBlocks() { myBlock(3); } +// Test NSMapInsert. +@interface NSMapTable : NSObject +@end +extern void *NSMapGet(NSMapTable *table, const void *key); +extern void NSMapInsert(NSMapTable *table, const void *key, const void *value); +extern void NSMapInsertKnownAbsent(NSMapTable *table, const void *key, const void *value); +char *strdup(const char *s); + +NSString * radar11152419(NSString *string1, NSMapTable *map) { + const char *strkey = "key"; + NSString *string = ( NSString *)NSMapGet(map, strkey); + if (!string) { + string = [string1 copy]; + NSMapInsert(map, strdup(strkey), (void*)string); // no warning + NSMapInsertKnownAbsent(map, strdup(strkey), (void*)string); // no warning + } + return string; +} + // Test that we handle pointer escaping through OSAtomicEnqueue. typedef volatile struct { void *opaque1; diff --git a/test/Analysis/retain-release.mm b/test/Analysis/retain-release.mm index c463f8ada9..01727ea644 100644 --- a/test/Analysis/retain-release.mm +++ b/test/Analysis/retain-release.mm @@ -111,6 +111,7 @@ typedef struct _NSZone NSZone; @protocol NSObject - (BOOL)isEqual:(id)object; - (id)retain; +- (id)copy; - (oneway void)release; - (id)autorelease; @end @protocol NSCopying - (id)copyWithZone:(NSZone *)zone; @@ -347,3 +348,21 @@ int rdar10553686_positive(void) return 0; } +@interface NSMapTable : NSObject +@end +extern void *NSMapGet(NSMapTable *table, const void *key); +extern void NSMapInsert(NSMapTable *table, const void *key, const void *value); +extern void NSMapInsertKnownAbsent(NSMapTable *table, const void *key, const void *value); +char *strdup(const char *s); + +NSString * radar11152419(NSString *string1, NSString *key1, NSMapTable *map) { + NSString *string = ( NSString *)NSMapGet(map, key1); + if (!string) { + string = [string1 copy]; + NSString *key = [key1 copy]; + NSMapInsert(map, (void*) key, (void*)string); // no warning + NSMapInsertKnownAbsent(map, (void*)key, (void*)string); // no warning + } + return string; +} + diff --git a/test/Analysis/system-header-simulator-objc.h b/test/Analysis/system-header-simulator-objc.h index 3fe21920ae..92d5899abf 100644 --- a/test/Analysis/system-header-simulator-objc.h +++ b/test/Analysis/system-header-simulator-objc.h @@ -39,6 +39,7 @@ typedef struct _NSZone NSZone; @protocol NSObject - (BOOL)isEqual:(id)object; - (id)retain; +- (id)copy; - (oneway void)release; - (id)autorelease; - (id)init; -- 2.40.0