retain/release checker: When generating summaries for CF/CG functions, allow arguments to "escape" if they are passed to a function containing the terms "InsertValue", "SetValue", or "AddValue". This fixes <rdar://problem/6539791>.

llvm-svn: 63341

2 files changed, 52 insertions, 6 deletions

diff --git a/clang/lib/Analysis/CFRefCount.cpp b/clang/lib/Analysis/CFRefCount.cpp
index 35b7ee7b805..ab9d409dd21 100644
--- a/clang/lib/Analysis/CFRefCount.cpp
+++ b/clang/lib/Analysis/CFRefCount.cpp
@@ -809,12 +809,27 @@ RetainSummary* RetainSummaryManager::getSummary(FunctionDecl* FD) {
       if (isRelease(FD, FName+2))
         S = getUnarySummary(FT, cfrelease);
       else {
-        // For CoreFoundation and CoreGraphics functions we assume they
-        // follow the ownership idiom strictly and thus do not cause
-        // ownership to "escape".
-        assert (ScratchArgs.empty());  
-        S = getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, 
-                                 DoNothing);
+        assert (ScratchArgs.empty());
+        // Remaining CoreFoundation and CoreGraphics functions.
+        // We use to assume that they all strictly followed the ownership idiom
+        // and that ownership cannot be transferred.  While this is technically
+        // correct, many methods allow a tracked object to escape.  For example:
+        //
+        //   CFMutableDictionaryRef x = CFDictionaryCreateMutable(...);        
+        //   CFDictionaryAddValue(y, key, x);
+        //   CFRelease(x); 
+        //   ... it is okay to use 'x' since 'y' has a reference to it
+        //
+        // We handle this and similar cases with the follow heuristic.  If the
+        // function name contains "InsertValue", "SetValue" or "AddValue" then
+        // we assume that arguments may "escape."
+        //
+        ArgEffect E = (CStrInCStrNoCase(FName, "InsertValue") ||
+                       CStrInCStrNoCase(FName, "AddValue") ||
+                       CStrInCStrNoCase(FName, "SetValue"))
+                      ? MayEscape : DoNothing;
+        
+        S = getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, E);
       }
     }
   }
diff --git a/clang/test/Analysis/rdar-6539791.c b/clang/test/Analysis/rdar-6539791.c
new file mode 100644
index 00000000000..0d0b675d945
--- /dev/null
+++ b/clang/test/Analysis/rdar-6539791.c
@@ -0,0 +1,31 @@
+// RUN: clang -analyze -checker-cfref -analyzer-store-basic -verify %s &&
+// RUN: clang -analyze -checker-cfref -analyzer-store-region -verify %s
+
+typedef const struct __CFAllocator * CFAllocatorRef;
+typedef struct __CFDictionary * CFMutableDictionaryRef;
+typedef signed long CFIndex;
+typedef CFIndex CFNumberType;
+typedef const void * CFTypeRef;
+typedef struct {} CFDictionaryKeyCallBacks, CFDictionaryValueCallBacks;
+typedef const struct __CFNumber * CFNumberRef;
+extern const CFAllocatorRef kCFAllocatorDefault;
+extern const CFDictionaryKeyCallBacks kCFTypeDictionaryKeyCallBacks;
+extern const CFDictionaryValueCallBacks kCFTypeDictionaryValueCallBacks;
+enum { kCFNumberSInt32Type = 3 };
+CFMutableDictionaryRef CFDictionaryCreateMutable(CFAllocatorRef allocator, CFIndex capacity, const CFDictionaryKeyCallBacks *keyCallBacks, const CFDictionaryValueCallBacks *valueCallBacks);
+void CFDictionaryAddValue(CFMutableDictionaryRef theDict, const void *key, const void *value);
+void CFRelease(CFTypeRef cf);
+extern CFNumberRef CFNumberCreate(CFAllocatorRef allocator, CFNumberType theType, const void *valuePtr);
+
+void f(CFMutableDictionaryRef y, void* key, void* val_key) {
+  CFMutableDictionaryRef x = CFDictionaryCreateMutable(kCFAllocatorDefault, 1, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
+  CFDictionaryAddValue(y, key, x);
+  CFRelease(x); // the dictionary keeps a reference, so the object isn't deallocated yet
+  signed z = 1;
+  CFNumberRef value = CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt32Type, &z);
+  if (value) {
+    CFDictionaryAddValue(x, val_key, value); // no-warning
+    CFRelease(value);
+    CFDictionaryAddValue(y, val_key, value); // no-warning
+  }
+}