[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

diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 7b9adb7c157d5e6164552917537e34bf0197c773..7456af23441772dd6912b74f8c18098a8ba96769 100644 (file)
--- 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 a6d33ad2a0f74d3e161ad5fa1e59305f8d2ff6fe..7fa6975478bc430f6a6ab1e886f3b938673d9532 100644 (file)
--- 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 fead0862fa89259e825773cb91848ed0b8242403..16f5d0bb1a471c03b46138d8ed0c96c745f72723 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -245,9 +245,13 @@ static void findPtrToConstParams(llvm::SmallSet<unsigned, 1> &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 fe14edeedd1ccd43ad5a45724a7fb03df073105e..d2409ac1609ce868d0d890a1a0f12fde440ba363 100644 (file)
--- a/test/Analysis/malloc.mm
+++ b/test/Analysis/malloc.mm
@@ -106,6 +106,25 @@ void testBlocks() {
   myBlock(3);
 }
 
+// Test NSMapInsert. 
+@interface NSMapTable : NSObject <NSCopying, NSCoding, NSFastEnumeration>
+@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 c463f8ada9e2d4a7b75af492932159a3a0e25970..01727ea64434b2b66ed33f3d056830cd3f08e589 100644 (file)
--- 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 <NSCopying, NSCoding, NSFastEnumeration>
+@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 3fe21920aef062b0753e945d6f8a8310a4355699..92d5899abf8d910bdec47daf99d07aed52721b23 100644 (file)
--- 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;