Address Jordan's concern about the false-positive warning for the missing 
-dealloc method (<rdar://problem/6074390>) and fix bugs for missing warnings 
with retained properties.

http://reviews.llvm.org/D5023

Files:
  lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  test/Analysis/DeallocMissingRelease.m
  test/Analysis/MissingDealloc.m
  test/Analysis/PR2978.m
Index: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -28,7 +28,7 @@
 using namespace clang;
 using namespace ento;
 
-static bool scan_ivar_release(Stmt *S, ObjCIvarDecl *ID,
+static bool scan_ivar_release(Stmt *S, const ObjCIvarDecl *ID,
                               const ObjCPropertyDecl *PD,
                               Selector Release,
                               IdentifierInfo* SelfII,
@@ -76,6 +76,31 @@
   return false;
 }
 
+static bool isSynthesizedWritablePointerProperty(const ObjCPropertyImplDecl *I,
+                                                 const ObjCIvarDecl **ID,
+                                                 const ObjCPropertyDecl **PD) {
+
+  if (I->getPropertyImplementation() != ObjCPropertyImplDecl::Synthesize)
+    return false;
+
+  (*ID) = I->getPropertyIvarDecl();
+  if (!(*ID))
+    return false;
+
+  QualType T = (*ID)->getType();
+  if (!T->isObjCObjectPointerType())
+    return false;
+
+  (*PD) = I->getPropertyDecl();
+  if (!(*PD))
+    return false;
+
+  if ((*PD)->isReadOnly())
+    return false;
+
+  return true;
+}
+
 static void checkObjCDealloc(const CheckerBase *Checker,
                              const ObjCImplementationDecl *D,
                              const LangOptions &LOpts, BugReporter &BR) {
@@ -85,33 +110,33 @@
   ASTContext &Ctx = BR.getContext();
   const ObjCInterfaceDecl *ID = D->getClassInterface();
 
-  // Does the class contain any ivars that are pointers (or id<...>)?
-  // If not, skip the check entirely.
-  // NOTE: This is motivated by PR 2517:
+  // Does the class contain any properties that are retained, synthesized,
+  // writable pointers (or id<...>)?  If not, skip the check entirely.
+  // NOTE: This is motivated by PR 2517 and <rdar://problem/6074390>:
   //        http://llvm.org/bugs/show_bug.cgi?id=2517
 
-  bool containsPointerIvar = false;
-
-  for (const auto *Ivar : ID->ivars()) {
-    QualType T = Ivar->getType();
-
-    if (!T->isObjCObjectPointerType() ||
-        Ivar->hasAttr<IBOutletAttr>() || // Skip IBOutlets.
-        Ivar->hasAttr<IBOutletCollectionAttr>()) // Skip IBOutletCollections.
+  bool containsRetainedSynthesizedWritablePointerProperty = false;
+  for (const auto *I : D->property_impls()) {
+    const ObjCIvarDecl *ID = nullptr;
+    const ObjCPropertyDecl *PD = nullptr;
+    if (!isSynthesizedWritablePointerProperty(I, &ID, &PD))
       continue;
 
-    containsPointerIvar = true;
-    break;
+    // Property must be released if and only if the kind of setter
+    // was not 'assign'.
+    if (PD->getSetterKind() != ObjCPropertyDecl::Assign) {
+      containsRetainedSynthesizedWritablePointerProperty = true;
+      break;
+    }
   }
 
-  if (!containsPointerIvar)
+  if (!containsRetainedSynthesizedWritablePointerProperty)
     return;
 
   // Determine if the class subclasses NSObject.
   IdentifierInfo* NSObjectII = &Ctx.Idents.get("NSObject");
   IdentifierInfo* SenTestCaseII = &Ctx.Idents.get("SenTestCase");
 
-
   for ( ; ID ; ID = ID->getSuperClass()) {
     IdentifierInfo *II = ID->getIdentifier();
 
@@ -142,9 +167,6 @@
     }
   }
 
-  PathDiagnosticLocation DLoc =
-    PathDiagnosticLocation::createBegin(D, BR.getSourceManager());
-
   if (!MD) { // No dealloc found.
 
     const char* name = LOpts.getGC() == LangOptions::NonGC
@@ -155,6 +177,9 @@
     llvm::raw_string_ostream os(buf);
     os << "Objective-C class '" << *D << "' lacks a 'dealloc' instance method";
 
+    PathDiagnosticLocation DLoc =
+        PathDiagnosticLocation::createBegin(D, BR.getSourceManager());
+
     BR.EmitBasicReport(D, Checker, name, categories::CoreFoundationObjectiveC,
                        os.str(), DLoc);
     return;
@@ -170,27 +195,13 @@
   // Scan for missing and extra releases of ivars used by implementations
   // of synthesized properties
   for (const auto *I : D->property_impls()) {
-    // We can only check the synthesized properties
-    if (I->getPropertyImplementation() != ObjCPropertyImplDecl::Synthesize)
-      continue;
-
-    ObjCIvarDecl *ID = I->getPropertyIvarDecl();
-    if (!ID)
-      continue;
-
-    QualType T = ID->getType();
-    if (!T->isObjCObjectPointerType()) // Skip non-pointer ivars
-      continue;
-
-    const ObjCPropertyDecl *PD = I->getPropertyDecl();
-    if (!PD)
-      continue;
-
-    // ivars cannot be set via read-only properties, so we'll skip them
-    if (PD->isReadOnly())
+    const ObjCIvarDecl *ID = nullptr;
+    const ObjCPropertyDecl *PD = nullptr;
+    if (!isSynthesizedWritablePointerProperty(I, &ID, &PD))
       continue;
 
-    // ivar must be released if and only if the kind of setter was not 'assign'
+    // Property must be released if and only if the kind of setter
+    // was not 'assign'.
     bool requiresRelease = PD->getSetterKind() != ObjCPropertyDecl::Assign;
     if (scan_ivar_release(MD->getBody(), ID, PD, RS, SelfII, Ctx)
        != requiresRelease) {
@@ -204,23 +215,29 @@
                : "missing ivar release (Hybrid MM, non-GC)";
 
         os << "The '" << *ID
-           << "' instance variable was retained by a synthesized property but "
-              "wasn't released in 'dealloc'";
+           << "' instance variable in Objective-C class '" << *D
+           << "' was retained by a synthesized property "
+              "but was not released in 'dealloc'";
       } else {
         name = LOpts.getGC() == LangOptions::NonGC
                ? "extra ivar release (use-after-release)"
                : "extra ivar release (Hybrid MM, non-GC)";
 
         os << "The '" << *ID
-           << "' instance variable was not retained by a synthesized property "
+           << "' instance variable in Objective-C class '" << *D
+           << "' was not retained by a synthesized property "
               "but was released in 'dealloc'";
       }
 
-      PathDiagnosticLocation SDLoc =
-        PathDiagnosticLocation::createBegin(I, BR.getSourceManager());
+      // If @synthesize stmt is missing, fall back to @property stmt.
+      const Decl *SPDecl = I->getLocation().isValid()
+                               ? static_cast<const Decl *>(I)
+                               : static_cast<const Decl *>(PD);
+      PathDiagnosticLocation SPLoc =
+          PathDiagnosticLocation::createBegin(SPDecl, BR.getSourceManager());
 
       BR.EmitBasicReport(MD, Checker, name,
-                         categories::CoreFoundationObjectiveC, os.str(), SDLoc);
+                         categories::CoreFoundationObjectiveC, os.str(), SPLoc);
     }
   }
 }
Index: test/Analysis/DeallocMissingRelease.m
===================================================================
--- /dev/null
+++ test/Analysis/DeallocMissingRelease.m
@@ -0,0 +1,152 @@
+#define nil ((id)0)
+
+typedef signed char BOOL;
+@protocol NSObject
+- (BOOL)isEqual:(id)object;
+- (Class)class;
+@end
+
+@interface NSObject <NSObject> {}
+- (void)dealloc;
+- (id)init;
+- (id)retain;
+- (oneway void)release;
+@end
+
+typedef struct objc_selector *SEL;
+
+//===------------------------------------------------------------------------===
+// Do not warn about missing release in -dealloc for ivars.
+
+@interface MyIvarClass1 : NSObject {
+  NSObject *_ivar;
+}
+@end
+
+@implementation MyIvarClass1
+- (instancetype)initWithIvar:(NSObject *)ivar
+{
+  self = [super init];
+  if (!self)
+    return nil;
+  _ivar = [ivar retain];
+  return self;
+}
+- (void)dealloc
+{
+  [super dealloc];
+}
+@end
+
+@interface MyIvarClass2 : NSObject {
+  NSObject *_ivar;
+}
+- (NSObject *)ivar;
+- (void)setIvar:(NSObject *)ivar;
+@end
+
+@implementation MyIvarClass2
+- (instancetype)init
+{
+  self = [super init];
+  return self;
+}
+- (void)dealloc
+{
+  [super dealloc];
+}
+- (NSObject *)ivar
+{
+  return _ivar;
+}
+- (void)setIvar:(NSObject *)ivar
+{
+  [_ivar release];
+  _ivar = [ivar retain];
+}
+@end
+
+//===------------------------------------------------------------------------===
+// Warn about missing release in -dealloc for properties.
+
+@interface MyPropertyClass1 : NSObject
+@property (copy) NSObject *ivar;
+@end
+
+@implementation MyPropertyClass1
+- (void)dealloc
+{
+  [super dealloc];
+}
+@end
+
+@interface MyPropertyClass2 : NSObject
+@property (retain) NSObject *ivar;
+@end
+
+@implementation MyPropertyClass2
+- (void)dealloc
+{
+  [super dealloc];
+}
+@end
+
+@interface MyPropertyClass3 : NSObject {
+  NSObject *_ivar;
+}
+@property (retain) NSObject *ivar;
+@end
+
+@implementation MyPropertyClass3
+@synthesize ivar = _ivar;
+- (void)dealloc
+{
+  [super dealloc];
+}
+@end
+
+//===------------------------------------------------------------------------===
+// <rdar://problem/6380411>: 'myproperty' has kind 'assign' and thus the
+//  assignment through the setter does not perform a release.
+
+@interface MyObject : NSObject {
+  id __unsafe_unretained _myproperty;
+}
+@property(assign) id myproperty;
+@end
+
+@implementation MyObject
+@synthesize myproperty=_myproperty; // no-warning
+- (void)dealloc {
+  // Don't claim that myproperty is released since it the property
+  // has the 'assign' attribute.
+  self.myproperty = 0; // no-warning
+  [super dealloc];
+}
+@end
+
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks %s 2>&1 | FileCheck -check-prefix=CHECK %s
+// CHECK: DeallocMissingRelease.m:73:1: warning: The '_ivar' instance variable in Objective-C class 'MyPropertyClass1' was retained by a synthesized property but was not released in 'dealloc'
+// CHECK: DeallocMissingRelease.m:84:1: warning: The '_ivar' instance variable in Objective-C class 'MyPropertyClass2' was retained by a synthesized property but was not released in 'dealloc'
+// CHECK: DeallocMissingRelease.m:101:1: warning: The '_ivar' instance variable in Objective-C class 'MyPropertyClass3' was retained by a synthesized property but was not released in 'dealloc'
+// CHECK: 3 warnings generated.
+
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -fobjc-gc %s 2>&1 | FileCheck -check-prefix=CHECK-GC %s
+// CHECK-GC: DeallocMissingRelease.m:73:1: warning: The '_ivar' instance variable in Objective-C class 'MyPropertyClass1' was retained by a synthesized property but was not released in 'dealloc'
+// CHECK-GC: DeallocMissingRelease.m:84:1: warning: The '_ivar' instance variable in Objective-C class 'MyPropertyClass2' was retained by a synthesized property but was not released in 'dealloc'
+// CHECK-GC: DeallocMissingRelease.m:101:1: warning: The '_ivar' instance variable in Objective-C class 'MyPropertyClass3' was retained by a synthesized property but was not released in 'dealloc'
+// CHECK-GC: 3 warnings generated.
+
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -fobjc-gc-only %s 2>&1
+
+// RUN: not %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -triple x86_64-apple-darwin10 -fobjc-arc %s 2>&1 | FileCheck -check-prefix=CHECK-ARC %s
+// CHECK-ARC: DeallocMissingRelease.m:32:17: error: ARC forbids explicit message send of 'retain'
+// CHECK-ARC: DeallocMissingRelease.m:37:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: DeallocMissingRelease.m:56:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: DeallocMissingRelease.m:64:10: error: ARC forbids explicit message send of 'release'
+// CHECK-ARC: DeallocMissingRelease.m:65:17: error: ARC forbids explicit message send of 'retain'
+// CHECK-ARC: DeallocMissingRelease.m:79:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: DeallocMissingRelease.m:90:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: DeallocMissingRelease.m:104:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: DeallocMissingRelease.m:124:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: 9 errors generated.
Index: test/Analysis/MissingDealloc.m
===================================================================
--- test/Analysis/MissingDealloc.m
+++ test/Analysis/MissingDealloc.m
@@ -1,5 +1,3 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc %s -verify
-// expected-no-diagnostics
 typedef signed char BOOL;
 @protocol NSObject
 - (BOOL)isEqual:(id)object;
@@ -13,21 +11,67 @@
 
 typedef struct objc_selector *SEL;
 
-// <rdar://problem/6380411>: 'myproperty' has kind 'assign' and thus the
-//  assignment through the setter does not perform a release.
+//===------------------------------------------------------------------------===
+// Do not warn about missing -dealloc method.  Not enough context to know
+// whether the ivar is retained or not.
 
-@interface MyObject : NSObject {
-  id _myproperty;  
+@interface MissingDeallocWithIvar : NSObject {
+  NSObject *_ivar;
 }
-@property(assign) id myproperty;
 @end
 
-@implementation MyObject
-@synthesize myproperty=_myproperty; // no-warning
-- (void)dealloc {
-  self.myproperty = 0;
-  [super dealloc]; 
+@implementation MissingDeallocWithIvar
+@end
+
+//===------------------------------------------------------------------------===
+// Do not warn about missing -dealloc method.  These properties are not
+// retained, synthesized or writeable pointers.
+
+@interface MissingDeallocWithIntProperty : NSObject
+@property (assign) int ivar;
+@end
+
+@implementation MissingDeallocWithIntProperty
+@end
+
+@interface MissingDeallocWithSELProperty : NSObject
+@property (assign) SEL ivar;
+@end
+
+@implementation MissingDeallocWithSELProperty
+@end
+
+@interface MissingDeallocWithReadOnlyRetainedProperty : NSObject
+@property (readonly,retain) NSObject *ivar;
+@end
+
+@implementation MissingDeallocWithReadOnlyRetainedProperty
+@end
+
+//===------------------------------------------------------------------------===
+// Warn about missing -dealloc method.
+
+@interface MissingDeallocWithCopyProperty : NSObject
+@property (copy) NSObject *ivar;
+@end
+
+@implementation MissingDeallocWithCopyProperty
+@end
+
+@interface MissingDeallocWithRetainProperty : NSObject
+@property (retain) NSObject *ivar;
+@end
+
+@implementation MissingDeallocWithRetainProperty
+@end
+
+@interface MissingDeallocWithIVarAndRetainProperty : NSObject {
+  NSObject *_ivar2;
 }
+@property (retain) NSObject *ivar1;
+@end
+
+@implementation MissingDeallocWithIVarAndRetainProperty
 @end
 
 //===------------------------------------------------------------------------===
@@ -65,27 +109,6 @@
 @end
 
 //===------------------------------------------------------------------------===
-// <rdar://problem/6380411>
-// Was bogus warning: "The '_myproperty' instance variable was not retained by a
-//  synthesized property but was released in 'dealloc'"
-
-@interface MyObject_rdar6380411 : NSObject {
-    id _myproperty;
-}
-@property(assign) id myproperty;
-@end
-
-@implementation MyObject_rdar6380411
-@synthesize myproperty=_myproperty;
-- (void)dealloc {
-    // Don't claim that myproperty is released since it the property
-    // has the 'assign' attribute.
-    self.myproperty = 0; // no-warning
-    [super dealloc];
-}
-@end
-
-//===------------------------------------------------------------------------===
 // PR 3187: http://llvm.org/bugs/show_bug.cgi?id=3187
 // - Disable the missing -dealloc check for classes that subclass SenTestCase
 
@@ -112,3 +135,19 @@
   // do something which uses resourcepath
 }
 @end
+
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks %s 2>&1 | FileCheck -check-prefix=CHECK %s
+// CHECK: MissingDealloc.m:58:1: warning: Objective-C class 'MissingDeallocWithCopyProperty' lacks a 'dealloc' instance method
+// CHECK: MissingDealloc.m:65:1: warning: Objective-C class 'MissingDeallocWithRetainProperty' lacks a 'dealloc' instance method
+// CHECK: MissingDealloc.m:74:1: warning: Objective-C class 'MissingDeallocWithIVarAndRetainProperty' lacks a 'dealloc' instance method
+// CHECK: 3 warnings generated.
+
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -fobjc-gc %s 2>&1 | FileCheck -check-prefix=CHECK-GC %s
+// CHECK-GC: MissingDealloc.m:58:1: warning: Objective-C class 'MissingDeallocWithCopyProperty' lacks a 'dealloc' instance method
+// CHECK-GC: MissingDealloc.m:65:1: warning: Objective-C class 'MissingDeallocWithRetainProperty' lacks a 'dealloc' instance method
+// CHECK-GC: MissingDealloc.m:74:1: warning: Objective-C class 'MissingDeallocWithIVarAndRetainProperty' lacks a 'dealloc' instance method
+// CHECK-GC: 3 warnings generated.
+
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -fobjc-gc-only %s 2>&1
+
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -triple x86_64-apple-darwin10 -fobjc-arc %s 2>&1
Index: test/Analysis/PR2978.m
===================================================================
--- test/Analysis/PR2978.m
+++ test/Analysis/PR2978.m
@@ -33,13 +33,13 @@
 
 @implementation MyClass
 @synthesize X = _X;
-@synthesize Y = _Y; // expected-warning{{The '_Y' instance variable was retained by a synthesized property but wasn't released in 'dealloc'}}
-@synthesize Z = _Z; // expected-warning{{The '_Z' instance variable was not retained by a synthesized property but was released in 'dealloc'}}
+@synthesize Y = _Y; // expected-warning{{The '_Y' instance variable in Objective-C class 'MyClass' was retained by a synthesized property but was not released in 'dealloc'}}
+@synthesize Z = _Z; // expected-warning{{The '_Z' instance variable in Objective-C class 'MyClass' was not retained by a synthesized property but was released in 'dealloc'}}
 @synthesize K = _K;
 @synthesize N = _N;
 @synthesize M = _M;
 @synthesize V = _V;
-@synthesize W = _W; // expected-warning{{The '_W' instance variable was retained by a synthesized property but wasn't released in 'dealloc'}}
+@synthesize W = _W; // expected-warning{{The '_W' instance variable in Objective-C class 'MyClass' was retained by a synthesized property but was not released in 'dealloc'}}
 
 -(id) O{ return 0; }
 -(void) setO:(id)arg { }
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to