From: Anna Zaks Date: Tue, 28 Feb 2012 01:54:22 +0000 (+0000) Subject: [analyzer] Fix Malloc False Positive (PR 12100) X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=07d39a479cf8f20294407e749f9933da34ebecb7;p=clang [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 f7f199e26c..007eba19ab 100644 --- 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(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(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 a98d3b8c59..7b6e0d75d6 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -193,11 +193,14 @@ static void findPtrToConstParams(llvm::SmallSet &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 ce2ff206cf..21cc745b0f 100644 --- 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 efab640475..88739db0f9 100644 --- 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 9bfb7939e7..f5b6333a8f 100644 --- 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 - (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 - (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);