[analyzer] ObjCDeallocChecker: Only operate on classes with retained properties.

Previously the ObjC Dealloc Checker only checked classes with ivars, not
retained properties, which caused three bugs:

- False positive warnings about a missing -dealloc method in classes with only
ivars.
- Missing warnings about a missing -dealloc method on classes with only
properties.
- Missing warnings about an over-released or under-released ivar associated with
a retained property in classes with only properties.

The fix is to check only classes with at least one retained synthesized
property.

This also exposed a bug when reporting an over-released or under-released
property that did not contain a synthesize statement. The checker tried to
associate the warning with an @synthesize statement that did not exist, which
caused an assertion failure in debug builds. The fix is to fall back to the
@property statement in this case.

A patch by David Kilzer!

Part of rdar://problem/6927496

Differential Revision: http://reviews.llvm.org/D5023

llvm-svn: 258896
This commit is contained in:
Devin Coughlin 2016-01-27 01:41:58 +00:00
parent ece881d518
commit 3075134739
4 changed files with 331 additions and 90 deletions

View File

@ -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 @@ static bool scan_ivar_release(Stmt *S, ObjCIvarDecl *ID,
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<IBOutletAttr>() || // Skip IBOutlets.
Ivar->hasAttr<IBOutletCollectionAttr>()) // 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 @@ static void checkObjCDealloc(const CheckerBase *Checker,
}
}
PathDiagnosticLocation DLoc =
PathDiagnosticLocation::createBegin(D, BR.getSourceManager());
if (!MD) { // No dealloc found.
const char* name = LOpts.getGC() == LangOptions::NonGC
@ -155,6 +177,9 @@ static void checkObjCDealloc(const CheckerBase *Checker,
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 @@ static void checkObjCDealloc(const CheckerBase *Checker,
// 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)
const ObjCIvarDecl *ID = nullptr;
const ObjCPropertyDecl *PD = nullptr;
if (!isSynthesizedRetainableProperty(I, &ID, &PD))
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())
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 @@ static void checkObjCDealloc(const CheckerBase *Checker,
? "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<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);
}
}
}
@ -235,7 +248,8 @@ class ObjCDeallocChecker : public Checker<
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<ObjCImplementationDecl>(D), mgr.getLangOpts(),
BR);

View File

@ -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 <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
//===------------------------------------------------------------------------===
// <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
#if NON_ARC
[super dealloc];
#endif
}
@end
// CHECK: 4 warnings generated.

View File

@ -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 signed char BOOL;
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;
}
@property(assign) id myproperty;
@end
@implementation MyObject
@synthesize myproperty=_myproperty; // no-warning
- (void)dealloc {
self.myproperty = 0;
[super dealloc];
@interface MissingDeallocWithIvar : NSObject {
NSObject *_ivar;
}
@end
@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.
@ -64,27 +116,6 @@ IBOutlet NSWindow *window;
@implementation HasOutlet // no-warning
@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 +143,4 @@ IBOutlet NSWindow *window;
// do something which uses resourcepath
}
@end
// CHECK: 4 warnings generated.

View File

@ -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 { }