[analyzer] Fix Malloc False Positive (PR 12100)

When allocated buffer is passed to CF/NS..NoCopy functions, the
ownership is transfered unless the deallocator argument is set to
'kCFAllocatorNull'.

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

diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index f7f199e26c347105250df5e13de8d43ec7300294..007eba19ab09eee245d742e4f4d618e4c6baa370 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1094,14 +1094,32 @@ bool MallocChecker::doesNotFreeMemory(const CallOrObjCMessage *Call,
   if (!SM.isInSystemHeader(D->getLocation()))
     return false;
 
-  // Process C functions.
+  // Process C/ObjC functions.
   if (const FunctionDecl *FD  = dyn_cast_or_null<FunctionDecl>(D)) {
     // White list the system functions whose arguments escape.
     const IdentifierInfo *II = FD->getIdentifier();
-    if (II) {
-      StringRef FName = II->getName();
-      if (FName.equals("pthread_setspecific"))
-        return false;
+    if (!II)
+      return true;
+    StringRef FName = II->getName();
+
+    // White list thread local storage.
+    if (FName.equals("pthread_setspecific"))
+      return false;
+
+    // White list the 'XXXNoCopy' ObjC Methods.
+    if (FName.endswith("NoCopy")) {
+      // Look for the deallocator argument. We know that the memory ownership
+      // is not transfered only if the deallocator argument is
+      // 'kCFAllocatorNull'.
+      for (unsigned i = 1; i < Call->getNumArgs(); ++i) {
+        const Expr *ArgE = Call->getArg(i)->IgnoreParenCasts();
+        if (const DeclRefExpr *DE = dyn_cast<DeclRefExpr>(ArgE)) {
+          StringRef DeallocatorName = DE->getFoundDecl()->getName();
+          if (DeallocatorName == "kCFAllocatorNull")
+            return true;
+        }
+      }
+      return false;
     }
 
     // Otherwise, assume that the function does not free memory.

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index a98d3b8c59302f4cc0ccf31c3e9cee35f145283c..7b6e0d75d63a9458bd3ffc31b590c800f2030f71 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -193,11 +193,14 @@ static void findPtrToConstParams(llvm::SmallSet<unsigned, 1> &PreserveArgs,
     // argument is const.
     if (II) {
       StringRef FName = II->getName();
-      // 'int pthread_setspecific(ptheread_key k, const void *)' stores a value
-      // into thread local storage. The value can later be retrieved with
+      //  - 'int pthread_setspecific(ptheread_key k, const void *)' stores a
+      // value into thread local storage. The value can later be retrieved with
       // 'void *ptheread_getspecific(pthread_key)'. So even thought the
       // parameter is 'const void *', the region escapes through the call.
-      if (FName.equals("pthread_setspecific"))
+      //  - ObjC functions that end with "NoCopy" can free memory, of the passed
+      // in buffer.
+      if (FName == "pthread_setspecific" ||
+          FName.endswith("NoCopy"))
         return;
     }
 

diff --git a/test/Analysis/keychainAPI.m b/test/Analysis/keychainAPI.m
index ce2ff206cfdbe25ee3c71173f45582b3863c9e2d..21cc745b0f57be98d0c8deec00a6c83dba48e719 100644 (file)
--- a/test/Analysis/keychainAPI.m
+++ b/test/Analysis/keychainAPI.m
@@ -274,8 +274,8 @@ void DellocWithCFStringCreate2(CFAllocatorRef alloc) {
   char * x;
   st = SecKeychainItemCopyContent(2, ptr, ptr, &length, &bytes);
   if (st == noErr) {
-    CFStringRef userStr = CFStringCreateWithBytesNoCopy(alloc, bytes, length, 5, 0, kCFAllocatorNull); 
-    CFRelease(userStr); // expected-warning{{Allocated data is not released}}
+    CFStringRef userStr = CFStringCreateWithBytesNoCopy(alloc, bytes, length, 5, 0, kCFAllocatorNull); // expected-warning{{Allocated data is not released}}
+    CFRelease(userStr); 
   }
 }
 

diff --git a/test/Analysis/malloc.mm b/test/Analysis/malloc.mm
index efab640475f9d373d77b8f209d513c87a044a4e0..88739db0f9faf79345d4faed2ede3489809888ab 100644 (file)
--- a/test/Analysis/malloc.mm
+++ b/test/Analysis/malloc.mm
@@ -61,3 +61,25 @@ void testNSDatafFreeWhenDoneFN(NSUInteger dataLength) {
   NSData *nsdata = [NSData dataWithBytesNoCopy:data length:dataLength freeWhenDone:1];
   free(data); // false negative
 }
+
+// Test CF/NS...NoCopy. PR12100.
+
+void testNSDatafFreeWhenDone(NSUInteger dataLength) {
+  CFStringRef str;
+  char *bytes = (char*)malloc(12);
+  str = CFStringCreateWithCStringNoCopy(0, bytes, NSNEXTSTEPStringEncoding, 0); // no warning
+  CFRelease(str); // default allocator also frees bytes
+}
+
+void stringWithExternalContentsExample(void) {
+#define BufferSize 1000
+    CFMutableStringRef mutStr;
+    UniChar *myBuffer;
+ 
+    myBuffer = (UniChar *)malloc(BufferSize * sizeof(UniChar));
+ 
+    mutStr = CFStringCreateMutableWithExternalCharactersNoCopy(0, myBuffer, 0, BufferSize, kCFAllocatorNull); // expected-warning{{leak}}
+ 
+    CFRelease(mutStr);
+    //free(myBuffer);
+}

diff --git a/test/Analysis/system-header-simulator-objc.h b/test/Analysis/system-header-simulator-objc.h
index 9bfb7939e773292c88879d5080ff055c3da49c25..f5b6333a8f1f7018e99ceb100037b2d7d9df5459 100644 (file)
--- a/test/Analysis/system-header-simulator-objc.h
+++ b/test/Analysis/system-header-simulator-objc.h
@@ -1,9 +1,37 @@
 #pragma clang system_header
 
 typedef unsigned int UInt32;
+typedef unsigned short UInt16;
+
 typedef signed long CFIndex;
 typedef signed char BOOL;
 typedef unsigned long NSUInteger;
+typedef unsigned short unichar;
+typedef UInt16 UniChar;
+
+enum {
+    NSASCIIStringEncoding = 1,
+    NSNEXTSTEPStringEncoding = 2,
+    NSJapaneseEUCStringEncoding = 3,
+    NSUTF8StringEncoding = 4,
+    NSISOLatin1StringEncoding = 5,
+    NSSymbolStringEncoding = 6,
+    NSNonLossyASCIIStringEncoding = 7,
+};
+typedef const struct __CFString * CFStringRef;
+typedef struct __CFString * CFMutableStringRef;
+typedef NSUInteger NSStringEncoding;
+typedef UInt32 CFStringEncoding;
+
+typedef const void * CFTypeRef;
+
+typedef const struct __CFAllocator * CFAllocatorRef;
+extern const CFAllocatorRef kCFAllocatorDefault;
+extern const CFAllocatorRef kCFAllocatorSystemDefault;
+extern const CFAllocatorRef kCFAllocatorMalloc;
+extern const CFAllocatorRef kCFAllocatorMallocZone;
+extern const CFAllocatorRef kCFAllocatorNull;
+
 @class NSString, Protocol;
 extern void NSLog(NSString *format, ...) __attribute__((format(__NSString__, 1, 2)));
 typedef struct _NSZone NSZone;
@@ -44,17 +72,7 @@ NSFastEnumerationState;
 @end extern NSString * const NSBundleDidLoadNotification;
 typedef double NSTimeInterval;
 @interface NSDate : NSObject <NSCopying, NSCoding>  - (NSTimeInterval)timeIntervalSinceReferenceDate;
-@end            typedef unsigned short unichar;
-enum {
-    NSASCIIStringEncoding = 1,
-    NSNEXTSTEPStringEncoding = 2,
-    NSJapaneseEUCStringEncoding = 3,
-    NSUTF8StringEncoding = 4,
-    NSISOLatin1StringEncoding = 5,
-    NSSymbolStringEncoding = 6,
-    NSNonLossyASCIIStringEncoding = 7,
-};
-typedef NSUInteger NSStringEncoding;
+@end
 
 @interface NSString : NSObject <NSCopying, NSMutableCopying, NSCoding>
 - (NSUInteger)length;
@@ -73,3 +91,10 @@ typedef NSUInteger NSStringEncoding;
 - (id)initWithBytesNoCopy:(void *)bytes length:(NSUInteger)length;
 - (id)initWithBytesNoCopy:(void *)bytes length:(NSUInteger)length freeWhenDone:(BOOL)b;
 @end
+
+
+extern void CFRelease(CFTypeRef cf);
+
+extern CFMutableStringRef CFStringCreateMutableWithExternalCharactersNoCopy(CFAllocatorRef alloc, UniChar *chars, CFIndex numChars, CFIndex capacity, CFAllocatorRef externalCharactersAllocator);
+extern CFStringRef CFStringCreateWithCStringNoCopy(CFAllocatorRef alloc, const char *cStr, CFStringEncoding encoding, CFAllocatorRef contentsDeallocator);
+extern void CFStringAppend(CFMutableStringRef theString, CFStringRef appendedString);