[analyzer] Improve misleading RetainCountChcker diagnostic under ARC
authorDevin Coughlin <dcoughlin@apple.com>
Sat, 12 Nov 2016 01:03:06 +0000 (01:03 +0000)
committerDevin Coughlin <dcoughlin@apple.com>
Sat, 12 Nov 2016 01:03:06 +0000 (01:03 +0000)
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

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
clang/test/Analysis/retain-release-arc.m [new file with mode: 0644]

index 26026dd..7c31cba 100644 (file)
@@ -2367,10 +2367,15 @@ CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC,
       os << "that is annotated as NS_RETURNS_NOT_RETAINED";
     else {
       if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(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<FunctionDecl>(D);
diff --git a/clang/test/Analysis/retain-release-arc.m b/clang/test/Analysis/retain-release-arc.m
new file mode 100644 (file)
index 0000000..2a6a40e
--- /dev/null
@@ -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 <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
+