r286694 - [analyzer] Improve misleading RetainCountChcker diagnostic under ARC

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Fri Nov 11 19:03:06 2016
New Revision: 286694

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

Added:
    cfe/trunk/test/Analysis/retain-release-arc.m
Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=286694&r1=286693&r2=286694&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Fri Nov 11 19:03:06 2016
@@ -2367,10 +2367,15 @@ CFRefLeakReportVisitor::getEndPath(BugRe
       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);

Added: cfe/trunk/test/Analysis/retain-release-arc.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release-arc.m?rev=286694&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/retain-release-arc.m (added)
+++ cfe/trunk/test/Analysis/retain-release-arc.m Fri Nov 11 19:03:06 2016
@@ -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;
+ at class NSDictionary;
+ at class NSData;
+ at class NSString;
+
+ at protocol NSObject
+- (BOOL)isEqual:(id)object;
+- (id)retain;
+- (oneway void)release;
+- (id)autorelease;
+- (NSString *)description;
+- (id)init;
+ at end
+ at interface NSObject <NSObject> {}
++ (id)allocWithZone:(NSZone *)zone;
++ (id)alloc;
++ (id)new;
+- (void)dealloc;
+ at end
+
+ at interface NSDictionary : NSObject
+ at end
+
+ at interface SomeClass
+ at end
+
+ at implementation SomeClass
+- (NSDictionary *)copyTestWithBridgeReturningRetainable:(NSData *)plistData {
+  CFErrorRef error;
+  CFDictionaryRef testDict = CFPropertyListCreateWithData(kCFAllocatorDefault, (__bridge CFDataRef)plistData, 0, 0, &error);
+#if HAS_ARC
+      // expected-note at -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 at -2 {{Potential leak of an object stored into 'testDict'}}
+      // expected-note at -3 {{Object returned to caller as an owning reference (single retain count transferred to caller)}}
+      // expected-note at -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 at -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;
+}
+ at end
+