[cfe-commits] r151608 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/MallocChecker.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/keychainAPI.m test/Analysis/malloc.mm test/Analysis/system-header-simulator-objc.h

Anna Zaks ganna at apple.com
Mon Feb 27 17:54:22 PST 2012 

Author: zaks
Date: Mon Feb 27 19:54:22 2012
New Revision: 151608

URL: http://llvm.org/viewvc/llvm-project?rev=151608&view=rev
Log:
[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'.

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    cfe/trunk/test/Analysis/keychainAPI.m
    cfe/trunk/test/Analysis/malloc.mm
    cfe/trunk/test/Analysis/system-header-simulator-objc.h

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=151608&r1=151607&r2=151608&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Mon Feb 27 19:54:22 2012
@@ -1094,14 +1094,32 @@
   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.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=151608&r1=151607&r2=151608&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Mon Feb 27 19:54:22 2012
@@ -193,11 +193,14 @@
     // 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;
     }
 

Modified: cfe/trunk/test/Analysis/keychainAPI.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/keychainAPI.m?rev=151608&r1=151607&r2=151608&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/keychainAPI.m (original)
+++ cfe/trunk/test/Analysis/keychainAPI.m Mon Feb 27 19:54:22 2012
@@ -274,8 +274,8 @@
   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); 
   }
 }
 

Modified: cfe/trunk/test/Analysis/malloc.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.mm?rev=151608&r1=151607&r2=151608&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.mm (original)
+++ cfe/trunk/test/Analysis/malloc.mm Mon Feb 27 19:54:22 2012
@@ -61,3 +61,25 @@
   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);
+}

Modified: cfe/trunk/test/Analysis/system-header-simulator-objc.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/system-header-simulator-objc.h?rev=151608&r1=151607&r2=151608&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/system-header-simulator-objc.h (original)
+++ cfe/trunk/test/Analysis/system-header-simulator-objc.h Mon Feb 27 19:54:22 2012
@@ -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 @@
 @end extern NSString * const NSBundleDidLoadNotification;
 typedef double NSTimeInterval;
 @interface NSDate : NSObject <NSCopying, NSCoding>  - (NSTimeInterval)timeIntervalSinceReferenceDate;
- at end            typedef unsigned short unichar;
-enum {
-    NSASCIIStringEncoding = 1,
-    NSNEXTSTEPStringEncoding = 2,
-    NSJapaneseEUCStringEncoding = 3,
-    NSUTF8StringEncoding = 4,
-    NSISOLatin1StringEncoding = 5,
-    NSSymbolStringEncoding = 6,
-    NSNonLossyASCIIStringEncoding = 7,
-};
-typedef NSUInteger NSStringEncoding;
+ at end
 
 @interface NSString : NSObject <NSCopying, NSMutableCopying, NSCoding>
 - (NSUInteger)length;
@@ -73,3 +91,10 @@
 - (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);

More information about the cfe-commits mailing list