From d1fe08b8a9abe19d5ba5d96465d20601542b3040 Mon Sep 17 00:00:00 2001 From: Devin Coughlin Date: Sat, 12 Nov 2016 01:03:06 +0000 Subject: [PATCH] [analyzer] Improve misleading RetainCountChcker diagnostic under ARC Under automated reference counting the analyzer treats a methods -- even those starting with 'copy' and friends -- as returning an unowned value. This is because ownership of CoreFoundation objects must be transferred to ARC with __bridge_transfer or CFBridgingRelease() before being returned as ARC-managed bridged objects. Unfortunately this could lead to a poor diagnostic inside copy methods under ARC where the analyzer would complain about a leak of a returned CF value inside a method "whose name does not start with 'copy'" -- even though the name did start with 'copy'. This commit improves the diagnostic under ARC to say inside a method "returned from a method managed by Automated Reference Counting". rdar://problem/28849667 llvm-svn: 286694 --- .../StaticAnalyzer/Checkers/RetainCountChecker.cpp | 13 +++- clang/test/Analysis/retain-release-arc.m | 86 ++++++++++++++++++++++ 2 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 clang/test/Analysis/retain-release-arc.m diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 26026dd..7c31cba 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -2367,10 +2367,15 @@ CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC, os << "that is annotated as NS_RETURNS_NOT_RETAINED"; else { if (const ObjCMethodDecl *MD = dyn_cast(D)) { - os << "whose name ('" << MD->getSelector().getAsString() - << "') does not start with 'copy', 'mutableCopy', 'alloc' or 'new'." - " This violates the naming convention rules" - " given in the Memory Management Guide for Cocoa"; + if (BRC.getASTContext().getLangOpts().ObjCAutoRefCount) { + os << "managed by Automated Reference Counting"; + } else { + os << "whose name ('" << MD->getSelector().getAsString() + << "') does not start with " + "'copy', 'mutableCopy', 'alloc' or 'new'." + " This violates the naming convention rules" + " given in the Memory Management Guide for Cocoa"; + } } else { const FunctionDecl *FD = cast(D); diff --git a/clang/test/Analysis/retain-release-arc.m b/clang/test/Analysis/retain-release-arc.m new file mode 100644 index 0000000..2a6a40e --- /dev/null +++ b/clang/test/Analysis/retain-release-arc.m @@ -0,0 +1,86 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -fobjc-arc -fblocks -verify -Wno-objc-root-class %s -analyzer-output=text +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -fblocks -verify -Wno-objc-root-class %s -analyzer-output=text + +#define HAS_ARC __has_feature(objc_arc) + +typedef unsigned long long CFOptionFlags; +typedef signed long long CFIndex; + +typedef CFIndex CFPropertyListFormat; enum { + kCFPropertyListOpenStepFormat = 1, + kCFPropertyListXMLFormat_v1_0 = 100, + kCFPropertyListBinaryFormat_v1_0 = 200 +}; + +typedef const struct __CFAllocator * CFAllocatorRef; +extern const CFAllocatorRef kCFAllocatorDefault; +typedef struct __CFDictionary * CFDictionaryRef; +typedef struct __CFError * CFErrorRef; +typedef struct __CFDataRef * CFDataRef; +typedef void * CFPropertyListRef; + +CFPropertyListRef CFPropertyListCreateWithData(CFAllocatorRef allocator, CFDataRef data, CFOptionFlags options, CFPropertyListFormat *format, CFErrorRef *error); + +typedef signed char BOOL; +typedef struct _NSZone NSZone; +@class NSDictionary; +@class NSData; +@class NSString; + +@protocol NSObject +- (BOOL)isEqual:(id)object; +- (id)retain; +- (oneway void)release; +- (id)autorelease; +- (NSString *)description; +- (id)init; +@end +@interface NSObject {} ++ (id)allocWithZone:(NSZone *)zone; ++ (id)alloc; ++ (id)new; +- (void)dealloc; +@end + +@interface NSDictionary : NSObject +@end + +@interface SomeClass +@end + +@implementation SomeClass +- (NSDictionary *)copyTestWithBridgeReturningRetainable:(NSData *)plistData { + CFErrorRef error; + CFDictionaryRef testDict = CFPropertyListCreateWithData(kCFAllocatorDefault, (__bridge CFDataRef)plistData, 0, 0, &error); +#if HAS_ARC + // expected-note@-2 {{Call to function 'CFPropertyListCreateWithData' returns a Core Foundation object with a +1 retain count}} +#endif + return (__bridge NSDictionary *)testDict; +#if HAS_ARC + // expected-warning@-2 {{Potential leak of an object stored into 'testDict'}} + // expected-note@-3 {{Object returned to caller as an owning reference (single retain count transferred to caller)}} + // expected-note@-4 {{Object leaked: object allocated and stored into 'testDict' is returned from a method managed by Automated Reference Counting}} +#endif +} + +- (NSDictionary *)copyTestWithoutBridgeReturningRetainable:(NSData *)plistData { + NSDictionary *testDict = [[NSDictionary alloc] init]; + return testDict; // no-warning +} + +- (NSDictionary *)copyTestWithBridgeTransferReturningRetainable:(NSData *)plistData { + CFErrorRef error; + CFDictionaryRef testDict = CFPropertyListCreateWithData(kCFAllocatorDefault, (__bridge CFDataRef)plistData, 0, 0, &error); + return (__bridge_transfer NSDictionary *)testDict; // no-warning under ARC +#if !HAS_ARC + // expected-warning@-2 {{'__bridge_transfer' casts have no effect when not using ARC}} // Warning from Sema +#endif +} + +- (CFDictionaryRef)copyTestReturningCoreFoundation:(NSData *)plistData { + CFErrorRef error; + CFDictionaryRef testDict = CFPropertyListCreateWithData(kCFAllocatorDefault, (__bridge CFDataRef)plistData, 0, 0, &error); + return testDict; +} +@end + -- 2.7.4