Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ cfe/trunk/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,42 +76,67 @@ return false; } +static bool isSynthesizedRetainableProperty(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->isObjCRetainableType()) + return false; + + (*PD) = I->getPropertyDecl(); + // Shouldn't be able to synthesize a property that doesn't exist. + assert(*PD); + + return true; +} + +static bool synthesizedPropertyRequiresRelease(const ObjCPropertyDecl *PD) { + // A synthesized property must be released if and only if the kind of setter + // was neither 'assign' or 'weak'. + ObjCPropertyDecl::SetterKind SK = PD->getSetterKind(); + return (SK != ObjCPropertyDecl::Assign && SK != ObjCPropertyDecl::Weak); +} + static void checkObjCDealloc(const CheckerBase *Checker, const ObjCImplementationDecl *D, const LangOptions &LOpts, BugReporter &BR) { - assert (LOpts.getGC() != LangOptions::GCOnly); + assert(LOpts.getGC() != LangOptions::GCOnly); + assert(!LOpts.ObjCAutoRefCount); ASTContext &Ctx = BR.getContext(); const ObjCInterfaceDecl *ID = D->getClassInterface(); - // Does the class contain any ivars that are pointers (or id<...>)? + // Does the class contain any synthesized properties that are retainable? // If not, skip the check entirely. - // NOTE: This is motivated by PR 2517: - // 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() || // Skip IBOutlets. - Ivar->hasAttr()) // Skip IBOutletCollections. + bool containsRetainedSynthesizedProperty = false; + for (const auto *I : D->property_impls()) { + const ObjCIvarDecl *ID = nullptr; + const ObjCPropertyDecl *PD = nullptr; + if (!isSynthesizedRetainableProperty(I, &ID, &PD)) continue; - containsPointerIvar = true; - break; + if (synthesizedPropertyRequiresRelease(PD)) { + containsRetainedSynthesizedProperty = true; + break; + } } - if (!containsPointerIvar) + if (!containsRetainedSynthesizedProperty) 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,28 +195,12 @@ // 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 (!isSynthesizedRetainableProperty(I, &ID, &PD)) continue; - // ivar must be released if and only if the kind of setter was not 'assign' - bool requiresRelease = PD->getSetterKind() != ObjCPropertyDecl::Assign; + bool requiresRelease = synthesizedPropertyRequiresRelease(PD); if (scan_ivar_release(MD->getBody(), ID, PD, RS, SelfII, Ctx) != requiresRelease) { const char *name = nullptr; @@ -203,24 +212,28 @@ ? "missing ivar release (leak)" : "missing ivar release (Hybrid MM, non-GC)"; - os << "The '" << *ID - << "' instance variable was retained by a synthesized property but " - "wasn't released in 'dealloc'"; + os << "The '" << *ID << "' instance variable in '" << *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 " + os << "The '" << *ID << "' instance variable in '" << *D + << "' was not retained by a synthesized property " "but was released in 'dealloc'"; } - PathDiagnosticLocation SDLoc = - PathDiagnosticLocation::createBegin(I, BR.getSourceManager()); + // If @synthesize statement is missing, fall back to @property statement. + const Decl *SPDecl = I->getLocation().isValid() + ? static_cast(I) + : static_cast(PD); + PathDiagnosticLocation SPLoc = + PathDiagnosticLocation::createBegin(SPDecl, BR.getSourceManager()); BR.EmitBasicReport(MD, Checker, name, - categories::CoreFoundationObjectiveC, os.str(), SDLoc); + categories::CoreFoundationObjectiveC, os.str(), SPLoc); } } } @@ -235,7 +248,8 @@ public: void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& mgr, BugReporter &BR) const { - if (mgr.getLangOpts().getGC() == LangOptions::GCOnly) + if (mgr.getLangOpts().getGC() == LangOptions::GCOnly || + mgr.getLangOpts().ObjCAutoRefCount) return; checkObjCDealloc(this, cast(D), mgr.getLangOpts(), BR); Index: cfe/trunk/test/Analysis/DeallocMissingRelease.m =================================================================== --- cfe/trunk/test/Analysis/DeallocMissingRelease.m +++ cfe/trunk/test/Analysis/DeallocMissingRelease.m @@ -0,0 +1,192 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks %s 2>&1 | FileCheck -check-prefix=CHECK %s +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -triple x86_64-apple-darwin10 -fobjc-arc -fobjc-runtime-has-weak %s 2>&1 | FileCheck -check-prefix=CHECK-ARC -allow-empty '--implicit-check-not=error:' '--implicit-check-not=warning:' %s + +#define nil ((id)0) + +#define NON_ARC !__has_feature(objc_arc) + +#if NON_ARC +#define WEAK_ON_ARC +#else +#define WEAK_ON_ARC __weak +#endif + +typedef signed char BOOL; +@protocol NSObject +- (BOOL)isEqual:(id)object; +- (Class)class; +@end + +@interface 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; +#if NON_ARC + _ivar = [ivar retain]; +#endif + return self; +} +- (void)dealloc +{ +#if NON_ARC + [super dealloc]; +#endif +} +@end + +@interface MyIvarClass2 : NSObject { + NSObject *_ivar; +} +- (NSObject *)ivar; +- (void)setIvar:(NSObject *)ivar; +@end + +@implementation MyIvarClass2 +- (instancetype)init +{ + self = [super init]; + return self; +} +- (void)dealloc +{ +#if NON_ARC + [super dealloc]; +#endif +} +- (NSObject *)ivar +{ + return _ivar; +} +- (void)setIvar:(NSObject *)ivar +{ +#if NON_ARC + [_ivar release]; + _ivar = [ivar retain]; +#else + _ivar = ivar; +#endif +} +@end + +//===------------------------------------------------------------------------=== +// Warn about missing release in -dealloc for properties. + +@interface MyPropertyClass1 : NSObject +// CHECK: DeallocMissingRelease.m:[[@LINE+1]]:1: warning: The '_ivar' instance variable in 'MyPropertyClass1' was retained by a synthesized property but was not released in 'dealloc' +@property (copy) NSObject *ivar; +@end + +@implementation MyPropertyClass1 +- (void)dealloc +{ +#if NON_ARC + [super dealloc]; +#endif +} +@end + +@interface MyPropertyClass2 : NSObject +// CHECK: DeallocMissingRelease.m:[[@LINE+1]]:1: warning: The '_ivar' instance variable in 'MyPropertyClass2' was retained by a synthesized property but was not released in 'dealloc' +@property (retain) NSObject *ivar; +@end + +@implementation MyPropertyClass2 +- (void)dealloc +{ +#if NON_ARC + [super dealloc]; +#endif +} +@end + +@interface MyPropertyClass3 : NSObject { + NSObject *_ivar; +} +@property (retain) NSObject *ivar; +@end + +@implementation MyPropertyClass3 +// CHECK: DeallocMissingRelease.m:[[@LINE+1]]:1: warning: The '_ivar' instance variable in 'MyPropertyClass3' was retained by a synthesized property but was not released in 'dealloc' +@synthesize ivar = _ivar; +- (void)dealloc +{ +#if NON_ARC + [super dealloc]; +#endif +} +@end + +@interface MyPropertyClass4 : NSObject { + void (^_blockPropertyIvar)(void); +} +@property (copy) void (^blockProperty)(void); +@end + +@implementation MyPropertyClass4 +// CHECK: DeallocMissingRelease.m:[[@LINE+1]]:1: warning: The '_blockPropertyIvar' instance variable in 'MyPropertyClass4' was retained by a synthesized property but was not released in 'dealloc' +@synthesize blockProperty = _blockPropertyIvar; +- (void)dealloc +{ +#if NON_ARC + [super dealloc]; +#endif +} +@end + +@interface MyPropertyClass5 : NSObject { + WEAK_ON_ARC NSObject *_ivar; +} +@property (weak) NSObject *ivar; +@end + +@implementation MyPropertyClass5 +@synthesize ivar = _ivar; // no-warning +- (void)dealloc +{ +#if NON_ARC + [super dealloc]; +#endif +} +@end + +//===------------------------------------------------------------------------=== +// : '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 +#if NON_ARC + [super dealloc]; +#endif +} +@end +// CHECK: 4 warnings generated. Index: cfe/trunk/test/Analysis/MissingDealloc.m =================================================================== --- cfe/trunk/test/Analysis/MissingDealloc.m +++ cfe/trunk/test/Analysis/MissingDealloc.m @@ -1,5 +1,6 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc %s -verify -// expected-no-diagnostics +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks %s 2>&1 | FileCheck -check-prefix=CHECK %s +// RUN: %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 -allow-empty '--implicit-check-not=error:' '--implicit-check-not=warning:' %s + typedef signed char BOOL; @protocol NSObject - (BOOL)isEqual:(id)object; @@ -13,23 +14,74 @@ typedef struct objc_selector *SEL; -// : '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 or synthesized. + +@interface MissingDeallocWithIntProperty : NSObject +@property (assign) int ivar; +@end + +@implementation MissingDeallocWithIntProperty +@end + +@interface MissingDeallocWithSELProperty : NSObject +@property (assign) SEL ivar; +@end + +@implementation MissingDeallocWithSELProperty +@end + +//===------------------------------------------------------------------------=== +// Warn about missing -dealloc method. + +@interface MissingDeallocWithCopyProperty : NSObject +@property (copy) NSObject *ivar; +@end + +// CHECK: MissingDealloc.m:[[@LINE+1]]:1: warning: Objective-C class 'MissingDeallocWithCopyProperty' lacks a 'dealloc' instance method +@implementation MissingDeallocWithCopyProperty +@end + +@interface MissingDeallocWithRetainProperty : NSObject +@property (retain) NSObject *ivar; +@end + +// CHECK: MissingDealloc.m:[[@LINE+1]]:1: warning: Objective-C class 'MissingDeallocWithRetainProperty' lacks a 'dealloc' instance method +@implementation MissingDeallocWithRetainProperty +@end + +@interface MissingDeallocWithIVarAndRetainProperty : NSObject { + NSObject *_ivar2; } +@property (retain) NSObject *ivar1; +@end + +// CHECK: MissingDealloc.m:[[@LINE+1]]:1: warning: Objective-C class 'MissingDeallocWithIVarAndRetainProperty' lacks a 'dealloc' instance method +@implementation MissingDeallocWithIVarAndRetainProperty @end +@interface MissingDeallocWithReadOnlyRetainedProperty : NSObject +@property (readonly,retain) NSObject *ivar; +@end + +// CHECK: MissingDealloc.m:[[@LINE+1]]:1: warning: Objective-C class 'MissingDeallocWithReadOnlyRetainedProperty' lacks a 'dealloc' instance method +@implementation MissingDeallocWithReadOnlyRetainedProperty +@end + + //===------------------------------------------------------------------------=== // Don't warn about iVars that are selectors. @@ -65,27 +117,6 @@ @end //===------------------------------------------------------------------------=== -// -// 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 +143,4 @@ // do something which uses resourcepath } @end +// CHECK: 4 warnings generated. Index: cfe/trunk/test/Analysis/PR2978.m =================================================================== --- cfe/trunk/test/Analysis/PR2978.m +++ cfe/trunk/test/Analysis/PR2978.m @@ -14,6 +14,7 @@ id _Y; id _Z; id _K; + id _L; id _N; id _M; id _V; @@ -23,6 +24,7 @@ @property(retain) id Y; @property(assign) id Z; @property(assign) id K; +@property(weak) id L; @property(readonly) id N; @property(retain) id M; @property(retain) id V; @@ -33,13 +35,14 @@ @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 'MyClass' was retained by a synthesized property but was not released in 'dealloc'}} +@synthesize Z = _Z; // expected-warning{{The '_Z' instance variable in 'MyClass' was not retained by a synthesized property but was released in 'dealloc'}} @synthesize K = _K; -@synthesize N = _N; +@synthesize L = _L; // no-warning +@synthesize N = _N; // expected-warning{{The '_N' instance variable in 'MyClass' was not retained by a synthesized property but was released in 'dealloc'}} @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 'MyClass' was retained by a synthesized property but was not released in 'dealloc'}} -(id) O{ return 0; } -(void) setO:(id)arg { }