dcoughlin updated this revision to Diff 48995. dcoughlin marked 8 inline comments as done. dcoughlin added a comment.
This update addresses Anna's review comments. The big change is that the program state now maps instance symbols to sets of initial ivar symbols that must be released. (Rather than just storing the set, as before). With this approach we don't need to play the trick of explicitly binding the initial field symbols in the store to get notified that they escape when the instance escapes. Instead, we can look up the instance in the map when it escapes and remove the entire set. I've also fixed a leak where we weren't removing values properly from the set that must be released and fixed a false positive so we no longer warn about a missing release when the field is known to be nil. http://reviews.llvm.org/D17511 Files: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp test/Analysis/DeallocMissingRelease.m test/Analysis/MissingDealloc.m test/Analysis/PR2978.m test/Analysis/properties.m
Index: test/Analysis/properties.m =================================================================== --- test/Analysis/properties.m +++ test/Analysis/properties.m @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class %s -// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class -fobjc-arc %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,alpha.osx.cocoa.Dealloc,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,alpha.osx.cocoa.Dealloc,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class -fobjc-arc %s void clang_analyzer_eval(int); @@ -22,6 +22,7 @@ -(id)copy; -(id)retain; -(oneway void)release; +-(void)dealloc; @end @interface NSString : NSObject <NSCopying, NSMutableCopying, NSCoding> - (NSUInteger)length; @@ -138,6 +139,14 @@ @implementation Person @synthesize name = _name; + +-(void)dealloc { +#if !__has_feature(objc_arc) + self.name = [[NSString alloc] init]; // expected-warning {{leak}} + + [super dealloc]; // expected-warning {{The '_name' ivar in 'Person' was retained by a synthesized property but not released before '[super dealloc]}} +#endif +} @end #if !__has_feature(objc_arc) Index: test/Analysis/PR2978.m =================================================================== --- test/Analysis/PR2978.m +++ test/Analysis/PR2978.m @@ -5,7 +5,7 @@ @interface NSObject - (void)release; -- dealloc; +- (void)dealloc; @end @interface MyClass : NSObject { @@ -19,55 +19,97 @@ id _M; id _P; id _Q; + id _R; + id _S; id _V; id _W; + + MyClass *_other; + + id _nonPropertyIvar; } @property(retain) id X; @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(weak) id P; // expected-warning {{'_P' instance variable in 'MyClass' was not retained by a synthesized property but was released in 'dealloc'}} +@property(weak) id P; @property(weak) id Q; +@property(retain) id R; +@property(weak, readonly) id S; + +@property(assign, readonly) id T; // Shadowed in class extension +@property(assign) id U; @property(retain) id V; @property(retain) id W; -(id) O; -(void) setO: (id) arg; @end +@interface MyClass () +// Shadows T to make it readwrite internally but readonly externally. +@property(assign, readwrite) id T; +@end + @implementation MyClass @synthesize X = _X; -@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 Y = _Y; +@synthesize Z = _Z; @synthesize K = _K; -@synthesize L = _L; // no-warning -@synthesize N = _N; // no-warning +@synthesize L = _L; +@synthesize N = _N; @synthesize M = _M; -@synthesize Q = _Q; // expected-warning {{'_Q' instance variable in 'MyClass' was not retained by a synthesized property but was released in 'dealloc'}} +@synthesize Q = _Q; +@synthesize R = _R; @synthesize V = _V; -@synthesize W = _W; // expected-warning{{The '_W' instance variable in 'MyClass' was retained by a synthesized property but was not released in 'dealloc'}} +@synthesize W = _W; -(id) O{ return 0; } -(void) setO:(id)arg { } -- (id)dealloc + +-(void) releaseInHelper { + [_R release]; // no-warning + _R = @"Hi"; +} + +- (void)dealloc { + [_X release]; - [_Z release]; + [_Z release]; // expected-warning{{The '_Z' ivar in 'MyClass' was synthesized for an assign, readwrite property but was released in 'dealloc'}} + [_T release]; // no-warning + + [_other->_Z release]; // no-warning [_N release]; - + self.M = 0; // This will release '_M' [self setV:0]; // This will release '_V' [self setW:@"newW"]; // This will release '_W', but retain the new value - self.O = 0; // no-warning - [_Q release]; + [_S release]; // expected-warning {{The '_S' ivar in 'MyClass' was synthesized for a weak property but was released in 'dealloc'}} + + self.O = 0; // no-warning + + [_Q release]; // expected-warning {{The '_Q' ivar in 'MyClass' was synthesized for a weak property but was released in 'dealloc'}} + self.P = 0; + + [self releaseInHelper]; + + [_nonPropertyIvar release]; // no-warning + + // Silly, but not an error. + if (!_U) + [_U release]; + [super dealloc]; - return 0; + // expected-warning@-1{{The '_Y' ivar in 'MyClass' was retained by a synthesized property but not released before '[super dealloc]'}} + // expected-warning@-2{{The '_W' ivar in 'MyClass' was retained by a synthesized property but not released before '[super dealloc]'}} + } @end Index: test/Analysis/MissingDealloc.m =================================================================== --- test/Analysis/MissingDealloc.m +++ test/Analysis/MissingDealloc.m @@ -128,6 +128,9 @@ @interface MyClassTest : SenTestCase { NSString *resourcePath; } + +@property (retain) NSObject *ivar; + @end @interface NSBundle : NSObject {} @@ -143,4 +146,15 @@ // do something which uses resourcepath } @end + +//===------------------------------------------------------------------------=== +// Don't warn for clases that aren't subclasses of NSObject + +__attribute__((objc_root_class)) +@interface NonNSObjectMissingDealloc +@property (retain) NSObject *ivar; +@end +@implementation NonNSObjectMissingDealloc +@end + // CHECK: 4 warnings generated. Index: test/Analysis/DeallocMissingRelease.m =================================================================== --- test/Analysis/DeallocMissingRelease.m +++ test/Analysis/DeallocMissingRelease.m @@ -1,5 +1,5 @@ -// 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 +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -triple x86_64-apple-darwin10 -fobjc-arc -fobjc-runtime-has-weak -verify %s #define nil ((id)0) @@ -11,22 +11,27 @@ #define WEAK_ON_ARC __weak #endif +// No diagnostics expected under ARC. +#if !NON_ARC + // expected-no-diagnostics +#endif + typedef signed char BOOL; @protocol NSObject - (BOOL)isEqual:(id)object; - (Class)class; @end @interface NSObject <NSObject> {} ++ (instancetype)alloc; - (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 { @@ -87,33 +92,30 @@ } @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]; + [super dealloc]; // expected-warning {{The '_ivar' ivar in 'MyPropertyClass1' was copied by a synthesized property but not released before '[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]; + [super dealloc]; // expected-warning {{The '_ivar' ivar in 'MyPropertyClass2' was retained by a synthesized property but not released before '[super dealloc]'}} #endif } @end @@ -125,14 +127,14 @@ @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]; + [super dealloc]; // expected-warning {{The '_ivar' ivar in 'MyPropertyClass3' was retained by a synthesized property but not released before '[super dealloc]'}} #endif } + @end @interface MyPropertyClass4 : NSObject { @@ -142,12 +144,11 @@ @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]; + [super dealloc]; // expected-warning {{The '_blockPropertyIvar' ivar in 'MyPropertyClass4' was copied by a synthesized property but not released before '[super dealloc]'}} #endif } @end @@ -159,16 +160,77 @@ @end @implementation MyPropertyClass5 -@synthesize ivar = _ivar; // no-warning +@synthesize ivar = _ivar; - (void)dealloc { #if NON_ARC + [super dealloc]; // no-warning because it is a weak property +#endif +} +@end + +@interface MyPropertyClassWithReturnInDealloc : NSObject { + NSObject *_ivar; +} +@property (retain) NSObject *ivar; +@end + +@implementation MyPropertyClassWithReturnInDealloc +@synthesize ivar = _ivar; +- (void)dealloc +{ + return; +#if NON_ARC + // expected-warning@-2{{The '_ivar' ivar in 'MyPropertyClassWithReturnInDealloc' was retained by a synthesized property but not released before '[super dealloc]'}} [super dealloc]; #endif } @end -//===------------------------------------------------------------------------=== +@interface MyPropertyClassWithReleaseInOtherInstance : NSObject { + NSObject *_ivar; + MyPropertyClassWithReleaseInOtherInstance *_other; +} +@property (retain) NSObject *ivar; + +-(void)releaseIvars; +@end + +@implementation MyPropertyClassWithReleaseInOtherInstance +@synthesize ivar = _ivar; + +-(void)releaseIvars; { +#if NON_ARC + [_ivar release]; +#endif +} + +- (void)dealloc +{ + [_other releaseIvars]; +#if NON_ARC + [super dealloc]; // expected-warning {{The '_ivar' ivar in 'MyPropertyClassWithReleaseInOtherInstance' was retained by a synthesized property but not released before '[super dealloc]'}} +#endif +} +@end + +@interface MyPropertyClassWithNeitherReturnNorSuperDealloc : NSObject { + NSObject *_ivar; +} +@property (retain) NSObject *ivar; +@end + +@implementation MyPropertyClassWithNeitherReturnNorSuperDealloc +@synthesize ivar = _ivar; +- (void)dealloc +{ +} +#if NON_ARC + // expected-warning@-2 {{method possibly missing a [super dealloc] call}} (From Sema) + // expected-warning@-3{{The '_ivar' ivar in 'MyPropertyClassWithNeitherReturnNorSuperDealloc' was retained by a synthesized property but not released before '[super dealloc]'}} +#endif +@end + // <rdar://problem/6380411>: 'myproperty' has kind 'assign' and thus the // assignment through the setter does not perform a release. @@ -202,51 +264,419 @@ // We really should warn because there is a path through -dealloc on which // _ivar2 is not released. #if NON_ARC - [_ivar2 release]; // no-warning + [_ivar2 release]; #endif } #if NON_ARC - [super dealloc]; + [super dealloc]; // expected-warning {{The '_ivar2' ivar in 'ClassWithControlFlowInRelease' was retained by a synthesized property but not released before '[super dealloc]'}} #endif } - @end -//===------------------------------------------------------------------------=== // Don't warn when the property is nil'd out in -dealloc @interface ClassWithNildOutProperty : NSObject -@property (retain) NSObject *ivar; // no-warning +@property (retain) NSObject *ivar; +@property (assign) int *intPtrProp; @end @implementation ClassWithNildOutProperty - (void)dealloc; { self.ivar = nil; + // Make sure to handle setting a non-retainable property to 0. + self.intPtrProp = 0; #if NON_ARC - [super dealloc]; + [super dealloc]; // no-warning +#endif +} +@end + +// Do warn when the ivar but not the property is nil'd out in -dealloc + +@interface ClassWithNildOutIvar : NSObject +@property (retain) NSObject *ivar; +@end + +@implementation ClassWithNildOutIvar +- (void)dealloc; { + // Oops. Meant self.ivar = nil + _ivar = nil; + +#if NON_ARC + [super dealloc]; // expected-warning {{The '_ivar' ivar in 'ClassWithNildOutIvar' was retained by a synthesized property but not released before '[super dealloc]'}} #endif } +@end +// Do warn when the ivar is updated to a different value that is then +// released. + +@interface ClassWithUpdatedIvar : NSObject +@property (retain) NSObject *ivar; @end -//===------------------------------------------------------------------------=== +@implementation ClassWithUpdatedIvar +- (void)dealloc; { + _ivar = [[NSObject alloc] init]; + +#if NON_ARC + [_ivar release]; +#endif + +#if NON_ARC + [super dealloc]; // expected-warning {{The '_ivar' ivar in 'ClassWithUpdatedIvar' was retained by a synthesized property but not released before '[super dealloc]'}} +#endif +} +@end + + // Don't warn when the property is nil'd out with a setter in -dealloc @interface ClassWithNildOutPropertyViaSetter : NSObject -@property (retain) NSObject *ivar; // no-warning +@property (retain) NSObject *ivar; @end @implementation ClassWithNildOutPropertyViaSetter - (void)dealloc; { [self setIvar:nil]; #if NON_ARC + [super dealloc]; // no-warning +#endif +} +@end + + +// Don't warn about missing releases when -dealloc helpers are called. + +@interface ClassWithDeallocHelpers : NSObject +@property (retain) NSObject *ivarReleasedInMethod; +@property (retain) NSObject *propNilledOutInMethod; + +@property (retain) NSObject *ivarReleasedInFunction; +@property (retain) NSObject *propNilledOutInFunction; + +@property (retain) NSObject *ivarNeverReleased; +- (void)invalidateInMethod; +@end + +void ReleaseValueHelper(NSObject *iv) { +#if NON_ARC + [iv release]; +#endif +} + +void NilOutPropertyHelper(ClassWithDeallocHelpers *o) { + o.propNilledOutInFunction = nil; +} + +@implementation ClassWithDeallocHelpers +- (void)invalidateInMethod { +#if NON_ARC + [_ivarReleasedInMethod release]; +#endif + self.propNilledOutInMethod = nil; +} + +- (void)dealloc; { + ReleaseValueHelper(_ivarReleasedInFunction); + NilOutPropertyHelper(self); + + [self invalidateInMethod]; +#if NON_ARC + [super dealloc]; // expected-warning {{The '_ivarNeverReleased' ivar in 'ClassWithDeallocHelpers' was retained by a synthesized property but not released before '[super dealloc]'}} +#endif +} +@end + + +// Don't warn when self in -dealloc escapes. + +@interface ClassWhereSelfEscapesViaMethodCall : NSObject +@property (retain) NSObject *ivar; // no-warning +@end + +@interface ClassWhereSelfEscapesViaMethodCall (Other) +- (void)invalidate; // In other translation unit. +@end + +@implementation ClassWhereSelfEscapesViaMethodCall +- (void)dealloc; { + [self invalidate]; +#if NON_ARC + [super dealloc]; +#endif +} // no-warning +@end + +@interface ClassWhereSelfEscapesViaPropertyAccess : NSObject +@property (retain) NSObject *ivar; +@end + +@interface ClassWhereSelfEscapesViaPropertyAccess (Other) +// The implementation of this property is unknown and therefore could +// release ivar. +@property (retain) NSObject *otherIvar; +@end + +@implementation ClassWhereSelfEscapesViaPropertyAccess +- (void)dealloc; { + self.otherIvar = nil; +#if NON_ARC [super dealloc]; #endif +} // no-warning +@end + +// Don't treat self as escaping when setter called on *synthesized* +// property. + +@interface ClassWhereSelfEscapesViaSynthesizedPropertyAccess : NSObject +@property (retain) NSObject *ivar; +@property (retain) NSObject *otherIvar; +@end + +@implementation ClassWhereSelfEscapesViaSynthesizedPropertyAccess +- (void)dealloc; { + self.otherIvar = nil; +#if NON_ARC + [super dealloc]; // expected-warning {{The '_ivar' ivar in 'ClassWhereSelfEscapesViaSynthesizedPropertyAccess' was retained by a synthesized property but not released before '[super dealloc]'}} +#endif } +@end + +// Don't warn when value escapes. +@interface ClassWhereIvarValueEscapes : NSObject +@property (retain) NSObject *ivar; +@end + +void ReleaseMe(id arg); + +@implementation ClassWhereIvarValueEscapes +- (void)dealloc; { + + ReleaseMe(_ivar); + +#if NON_ARC + [super dealloc]; +#endif +} // no-warning +@end + +// Don't warn when value is known to be nil. + +@interface ClassWhereIvarIsNil : NSObject +@property (retain) NSObject *ivarIsNil; +@end + +@implementation ClassWhereIvarIsNil +- (void)dealloc; { + +#if NON_ARC + if (_ivarIsNil) + [_ivarIsNil release]; + + [super dealloc]; +#endif +} // no-warning +@end + + +// Don't warn for non-retainable properties. + +@interface ClassWithNonRetainableProperty : NSObject +@property (assign) int *ivar; // no-warning +@end + +@implementation ClassWithNonRetainableProperty +- (void)dealloc; { +#if NON_ARC + [super dealloc]; +#endif +} // no-warning +@end + + +@interface SuperClassOfClassWithInlinedSuperDealloc : NSObject +@property (retain) NSObject *propInSuper; +@end + +@implementation SuperClassOfClassWithInlinedSuperDealloc +- (void)dealloc { +#if NON_ARC + [super dealloc]; // expected-warning {{The '_propInSuper' ivar in 'SuperClassOfClassWithInlinedSuperDealloc' was retained by a synthesized property but not released before '[super dealloc]'}} +#endif +} +@end + +@interface ClassWithInlinedSuperDealloc : SuperClassOfClassWithInlinedSuperDealloc +@property (retain) NSObject *propInSub; +@end + +@implementation ClassWithInlinedSuperDealloc +- (void)dealloc { +#if NON_ARC + [super dealloc]; // expected-warning {{The '_propInSub' ivar in 'ClassWithInlinedSuperDealloc' was retained by a synthesized property but not released before '[super dealloc]'}} +#endif +} +@end + + +@interface SuperClassOfClassWithInlinedSuperDeallocAndInvalidation : NSObject +@property (retain) NSObject *propInSuper; + +- (void)invalidate; +@end + +@implementation SuperClassOfClassWithInlinedSuperDeallocAndInvalidation + +- (void)invalidate { +#if NON_ARC + [_propInSuper release]; +#endif + _propInSuper = nil; +} + +- (void)dealloc { + [self invalidate]; +#if NON_ARC + [super dealloc]; // no-warning +#endif +} +@end + +@interface ClassWithInlinedSuperDeallocAndInvalidation : SuperClassOfClassWithInlinedSuperDeallocAndInvalidation +@property (retain) NSObject *propInSub; +@end + +@implementation ClassWithInlinedSuperDeallocAndInvalidation + +- (void)invalidate { +#if NON_ARC + [_propInSub release]; +#endif + [super invalidate]; +} + +- (void)dealloc { +#if NON_ARC + [super dealloc]; // no-warning +#endif +} +@end + + +@interface SuperClassOfClassThatEscapesBeforeInliningSuper : NSObject +@property (retain) NSObject *propInSuper; +@end + +@implementation SuperClassOfClassThatEscapesBeforeInliningSuper + +- (void)dealloc { + +#if NON_ARC + [super dealloc]; // expected-warning {{The '_propInSuper' ivar in 'SuperClassOfClassThatEscapesBeforeInliningSuper' was retained by a synthesized property but not released before '[super dealloc]'}} +#endif +} +@end + +@interface ClassThatEscapesBeforeInliningSuper : SuperClassOfClassThatEscapesBeforeInliningSuper +@property (retain) NSObject *propInSub; +@end + +@interface ClassThatEscapesBeforeInliningSuper (Other) +- (void)invalidate; // No implementation in translation unit. +@end + +@implementation ClassThatEscapesBeforeInliningSuper +- (void)dealloc { + [self invalidate]; + +#if NON_ARC + [super dealloc]; // no-warning +#endif +} +@end + + +#if NON_ARC +@interface ReleaseIvarInField : NSObject { + int _tag; + union { + NSObject *field1; + NSObject *field2; + } _someUnion; + + struct { + NSObject *field1; + } _someStruct; +} +@end + +@implementation ReleaseIvarInField +- (void)dealloc { + if (_tag) { + [_someUnion.field1 release]; + } else { + [_someUnion.field2 release]; + } + + [_someStruct.field1 release]; + [super dealloc]; +} +@end +#endif + +#if NON_ARC +@interface ReleaseIvarInArray : NSObject { + NSObject *_array[3]; +} +@end + +@implementation ReleaseIvarInArray +- (void)dealloc { + for (int i = 0; i < 3; i++) { + [_array[i] release]; + } + [super dealloc]; +} +@end +#endif + +// Don't warn about missing releases for subclasses of SenTestCase or +// for classes that are not subclasses of NSObject. + +@interface SenTestCase : NSObject {} +@end + +@interface MyClassTest : SenTestCase +@property (retain) NSObject *ivar; +@end + +@implementation MyClassTest +-(void)tearDown { +#if NON_ARC + [_ivar release]; +#endif +} + +-(void)dealloc; { +#if NON_ARC + [super dealloc]; // no-warning +#endif +} +@end + +__attribute__((objc_root_class)) +@interface NonNSObjectMissingDealloc +@property (retain) NSObject *ivar; +@end +@implementation NonNSObjectMissingDealloc +-(void)dealloc; { + +} @end -// CHECK: 4 warnings generated. Index: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp @@ -7,9 +7,24 @@ // //===----------------------------------------------------------------------===// // -// This file defines a CheckObjCDealloc, a checker that -// analyzes an Objective-C class's implementation to determine if it -// correctly implements -dealloc. +// This checker analyzes Objective-C -dealloc methods and their callees +// to warn about improper releasing of instance variables that back synthesized +// properties. It warns about missing releases in the following cases: +// - When a class has a synthesized instance variable for a 'retain' or 'copy' +// property and lacks a -dealloc method in its implementation. +// - When a class has a synthesized instance variable for a 'retain'/'copy' +// property but the ivar is not released in -dealloc by either -release +// or by nilling out the property. +// +// It warns about extra releases in -dealloc (but not in callees) when a +// synthesized instance variable is released in the following cases: +// - When the property is 'assign' and is not 'readonly'. +// - When the property is 'weak'. +// +// This checker only warns for instance variables synthesized to back +// properties. Handling the more general case would require inferring whether +// an instance variable is stored retained or not. For synthesized properties, +// this is specified in the property declaration itself. // //===----------------------------------------------------------------------===// @@ -20,71 +35,36 @@ #include "clang/AST/ExprObjC.h" #include "clang/Basic/LangOptions.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" #include "llvm/Support/raw_ostream.h" using namespace clang; using namespace ento; -// FIXME: This was taken from IvarInvalidationChecker.cpp -static const Expr *peel(const Expr *E) { - E = E->IgnoreParenCasts(); - if (const PseudoObjectExpr *POE = dyn_cast<PseudoObjectExpr>(E)) - E = POE->getSyntacticForm()->IgnoreParenCasts(); - if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(E)) - E = OVE->getSourceExpr()->IgnoreParenCasts(); - return E; -} +/// Indicates whether an instance variable is required to be released in +/// -dealloc. +enum class ReleaseRequirement { + /// The instance variable must be released, either by calling + /// -release on it directly or by nilling it out with a property setter. + MustRelease, -static bool scan_ivar_release(Stmt *S, const ObjCIvarDecl *ID, - const ObjCPropertyDecl *PD, - Selector Release, - IdentifierInfo* SelfII, - ASTContext &Ctx) { - - // [mMyIvar release] - if (ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S)) - if (ME->getSelector() == Release) - if (ME->getInstanceReceiver()) - if (const Expr *Receiver = peel(ME->getInstanceReceiver())) - if (auto *E = dyn_cast<ObjCIvarRefExpr>(Receiver)) - if (E->getDecl() == ID) - return true; - - // [self setMyIvar:nil]; - if (ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S)) - if (ME->getInstanceReceiver()) - if (const Expr *Receiver = peel(ME->getInstanceReceiver())) - if (auto *E = dyn_cast<DeclRefExpr>(Receiver)) - if (E->getDecl()->getIdentifier() == SelfII) - if (ME->getMethodDecl() == PD->getSetterMethodDecl() && - ME->getNumArgs() == 1 && - peel(ME->getArg(0))->isNullPointerConstant(Ctx, - Expr::NPC_ValueDependentIsNull)) - return true; - - // self.myIvar = nil; - if (BinaryOperator* BO = dyn_cast<BinaryOperator>(S)) - if (BO->isAssignmentOp()) - if (auto *PRE = dyn_cast<ObjCPropertyRefExpr>(peel(BO->getLHS()))) - if (PRE->isExplicitProperty() && PRE->getExplicitProperty() == PD) - if (peel(BO->getRHS())->isNullPointerConstant(Ctx, - Expr::NPC_ValueDependentIsNull)) { - // This is only a 'release' if the property kind is not - // 'assign'. - return PD->getSetterKind() != ObjCPropertyDecl::Assign; - } - - // Recurse to children. - for (Stmt *SubStmt : S->children()) - if (SubStmt && scan_ivar_release(SubStmt, ID, PD, Release, SelfII, Ctx)) - return true; + /// The instance variable must not be directly released with -release. + MustNotReleaseDirectly, - return false; -} + /// The requirement for the instance variable could not be determined. + Unknown +}; +/// Returns true if the property implementation is synthesized and the +/// type of the property is retainable. static bool isSynthesizedRetainableProperty(const ObjCPropertyImplDecl *I, const ObjCIvarDecl **ID, const ObjCPropertyDecl **PD) { @@ -107,171 +87,764 @@ 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); -} +namespace { + +class ObjCDeallocChecker + : public Checker<check::ASTDecl<ObjCImplementationDecl>, + check::PreObjCMessage, check::PostObjCMessage, + check::BeginFunction, check::EndFunction, + check::PointerEscape, + check::PreStmt<ReturnStmt>> { + + mutable IdentifierInfo *NSObjectII, *SenTestCaseII; + mutable Selector DeallocSel, ReleaseSel; + + std::unique_ptr<BugType> MissingReleaseBugType; + std::unique_ptr<BugType> ExtraReleaseBugType; + +public: + ObjCDeallocChecker(); + + void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& Mgr, + BugReporter &BR) const; + void checkBeginFunction(CheckerContext &Ctx) const; + void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; + void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; + + ProgramStateRef checkPointerEscape(ProgramStateRef State, + const InvalidatedSymbols &Escaped, + const CallEvent *Call, + PointerEscapeKind Kind) const; + void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const; + void checkEndFunction(CheckerContext &Ctx) const; + +private: + void diagnoseMissingReleases(CheckerContext &C) const; + + bool diagnoseExtraRelease(SymbolRef ReleasedValue, const ObjCMethodCall &M, + CheckerContext &C) const; + + SymbolRef getValueExplicitlyReleased(const ObjCMethodCall &M, + CheckerContext &C) const; + SymbolRef getValueReleasedByNillingOut(const ObjCMethodCall &M, + CheckerContext &C) const; + + SymbolRef getInstanceSymbolFromIvarSymbol(SymbolRef IvarSym) const; + + ReleaseRequirement + getDeallocReleaseRequirement(const ObjCPropertyImplDecl *PropImpl) const; + + bool isInInstanceDealloc(const CheckerContext &C, SVal &SelfValOut) const; + bool isInInstanceDealloc(const CheckerContext &C, const LocationContext *LCtx, + SVal &SelfValOut) const; + bool instanceDeallocIsOnStack(const CheckerContext &C, + SVal &InstanceValOut) const; + + bool isSuperDeallocMessage(const ObjCMethodCall &M) const; + + const ObjCImplDecl *getContainingObjCImpl(const LocationContext *LCtx) const; + + const ObjCPropertyDecl * + findShadowedPropertyDecl(const ObjCPropertyImplDecl *PropImpl) const; + + ProgramStateRef removeValueRequiringRelease(ProgramStateRef State, + SymbolRef InstanceSym, + SymbolRef ValueSym) const; + + void initIdentifierInfoAndSelectors(ASTContext &Ctx) const; + + bool classHasSeparateTeardown(const ObjCInterfaceDecl *ID) const; +}; +} // End anonymous namespace. + +typedef llvm::ImmutableSet<SymbolRef> SymbolSet; + +/// Maps from the symbol for a class instance to the set of +/// symbols remaining that must be released in -dealloc. +REGISTER_MAP_WITH_PROGRAMSTATE(UnreleasedIvarMap, SymbolRef, SymbolSet); + +template<> struct ProgramStateTrait<SymbolSet> +: public ProgramStatePartialTrait<SymbolSet> { + static void *GDMIndex() { static int index = 0; return &index; } +}; -static void checkObjCDealloc(const CheckerBase *Checker, - const ObjCImplementationDecl *D, - const LangOptions &LOpts, BugReporter &BR) { - assert(LOpts.getGC() != LangOptions::GCOnly); - assert(!LOpts.ObjCAutoRefCount); +/// An AST check that diagnose when the class requires a -dealloc method and +/// is missing one. +void ObjCDeallocChecker::checkASTDecl(const ObjCImplementationDecl *D, + AnalysisManager &Mgr, + BugReporter &BR) const { + assert(Mgr.getLangOpts().getGC() != LangOptions::GCOnly); + assert(!Mgr.getLangOpts().ObjCAutoRefCount); + initIdentifierInfoAndSelectors(Mgr.getASTContext()); - ASTContext &Ctx = BR.getContext(); const ObjCInterfaceDecl *ID = D->getClassInterface(); // Does the class contain any synthesized properties that are retainable? // If not, skip the check entirely. bool containsRetainedSynthesizedProperty = false; for (const auto *I : D->property_impls()) { - const ObjCIvarDecl *ID = nullptr; - const ObjCPropertyDecl *PD = nullptr; - if (!isSynthesizedRetainableProperty(I, &ID, &PD)) - continue; - - if (synthesizedPropertyRequiresRelease(PD)) { + if (getDeallocReleaseRequirement(I) == ReleaseRequirement::MustRelease) { containsRetainedSynthesizedProperty = true; break; } } 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(); - - if (II == NSObjectII) - break; - - // FIXME: For now, ignore classes that subclass SenTestCase, as these don't - // need to implement -dealloc. They implement tear down in another way, - // which we should try and catch later. - // http://llvm.org/bugs/show_bug.cgi?id=3187 - if (II == SenTestCaseII) - return; - } - - if (!ID) + // If the class is known to have a lifecycle with a separate teardown method + // then it may not require a -dealloc method. + if (classHasSeparateTeardown(ID)) return; - // Get the "dealloc" selector. - IdentifierInfo* II = &Ctx.Idents.get("dealloc"); - Selector S = Ctx.Selectors.getSelector(0, &II); const ObjCMethodDecl *MD = nullptr; // Scan the instance methods for "dealloc". for (const auto *I : D->instance_methods()) { - if (I->getSelector() == S) { + if (I->getSelector() == DeallocSel) { MD = I; break; } } if (!MD) { // No dealloc found. + const char* Name = "Missing -dealloc"; - const char* name = LOpts.getGC() == LangOptions::NonGC - ? "missing -dealloc" - : "missing -dealloc (Hybrid MM, non-GC)"; - - std::string buf; - llvm::raw_string_ostream os(buf); - os << "Objective-C class '" << *D << "' lacks a 'dealloc' instance method"; + std::string Buf; + 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); + BR.EmitBasicReport(D, this, Name, categories::CoreFoundationObjectiveC, + OS.str(), DLoc); return; } +} + +/// If this is the beginning of -dealloc, mark the values initially stored in +/// instance variables that must be released by the end of -dealloc +/// as unreleased in the state. +void ObjCDeallocChecker::checkBeginFunction( + CheckerContext &C) const { + initIdentifierInfoAndSelectors(C.getASTContext()); - // Get the "release" selector. - IdentifierInfo* RII = &Ctx.Idents.get("release"); - Selector RS = Ctx.Selectors.getSelector(0, &RII); + // Only do this if the current method is -dealloc. + SVal SelfVal; + if (!isInInstanceDealloc(C, SelfVal)) + return; - // Get the "self" identifier - IdentifierInfo* SelfII = &Ctx.Idents.get("self"); + SymbolRef SelfSymbol = SelfVal.getAsSymbol(); - // Scan for missing and extra releases of ivars used by implementations - // of synthesized properties - for (const auto *I : D->property_impls()) { - const ObjCIvarDecl *ID = nullptr; - const ObjCPropertyDecl *PD = nullptr; - if (!isSynthesizedRetainableProperty(I, &ID, &PD)) + const LocationContext *LCtx = C.getLocationContext(); + ProgramStateRef InitialState = C.getState(); + + ProgramStateRef State = InitialState; + + SymbolSet::Factory &F = State->getStateManager().get_context<SymbolSet>(); + + // Symbols that must be released by the end of the -dealloc; + SymbolSet RequiredReleases = F.getEmptySet(); + + // If we're an inlined -dealloc, we should add our symbols to the existing + // set from our subclass. + if (const SymbolSet *CurrSet = State->get<UnreleasedIvarMap>(SelfSymbol)) + RequiredReleases = *CurrSet; + + for (auto *PropImpl : getContainingObjCImpl(LCtx)->property_impls()) { + ReleaseRequirement Requirement = getDeallocReleaseRequirement(PropImpl); + if (Requirement != ReleaseRequirement::MustRelease) continue; - bool requiresRelease = synthesizedPropertyRequiresRelease(PD); - if (scan_ivar_release(MD->getBody(), ID, PD, RS, SelfII, Ctx) - != requiresRelease) { - const char *name = nullptr; - std::string buf; - llvm::raw_string_ostream os(buf); - - if (requiresRelease) { - name = LOpts.getGC() == LangOptions::NonGC - ? "missing ivar release (leak)" - : "missing ivar release (Hybrid MM, non-GC)"; - - os << "The '" << *ID << "' instance variable in '" << *D - << "' was retained by a synthesized property " - "but was not released in 'dealloc'"; - } else { - // It is common for the ivars for read-only assign properties to - // always be stored retained, so don't warn for a release in - // dealloc for the ivar backing these properties. - if (PD->isReadOnly()) - continue; - - name = LOpts.getGC() == LangOptions::NonGC - ? "extra ivar release (use-after-release)" - : "extra ivar release (Hybrid MM, non-GC)"; - - os << "The '" << *ID << "' instance variable in '" << *D - << "' was not retained by a synthesized property " - "but was released in 'dealloc'"; - } - - // 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(), SPLoc); - } + SVal LVal = State->getLValue(PropImpl->getPropertyIvarDecl(), SelfVal); + Optional<Loc> LValLoc = LVal.getAs<Loc>(); + if (!LValLoc) + continue; + + SVal InitialVal = State->getSVal(LValLoc.getValue()); + SymbolRef Symbol = InitialVal.getAsSymbol(); + if (!Symbol || !isa<SymbolRegionValue>(Symbol)) + continue; + + // Mark the value as requiring a release. + RequiredReleases = F.add(RequiredReleases, Symbol); + } + + if (!RequiredReleases.isEmpty()) { + State = State->set<UnreleasedIvarMap>(SelfSymbol, RequiredReleases); + } + + if (State != InitialState) { + C.addTransition(State); } } -//===----------------------------------------------------------------------===// -// ObjCDeallocChecker -//===----------------------------------------------------------------------===// +/// Given a symbol for an ivar, return a symbol for the instance containing +/// the ivar. Returns nullptr if the instance symbol cannot be found. +SymbolRef +ObjCDeallocChecker::getInstanceSymbolFromIvarSymbol(SymbolRef IvarSym) const { + if (auto *SRV = dyn_cast<SymbolRegionValue>(IvarSym)) { + const TypedValueRegion *TVR = SRV->getRegion(); + const ObjCIvarRegion *IvarRegion = dyn_cast_or_null<ObjCIvarRegion>(TVR); + if (!IvarRegion) + return nullptr; + + return IvarRegion->getSymbolicBase()->getSymbol(); + } -namespace { -class ObjCDeallocChecker : public Checker< - check::ASTDecl<ObjCImplementationDecl> > { -public: - void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& mgr, - BugReporter &BR) const { - if (mgr.getLangOpts().getGC() == LangOptions::GCOnly || - mgr.getLangOpts().ObjCAutoRefCount) + return nullptr; +} + +/// If we are in -dealloc or -dealloc is on the stack, handle the call if it is +/// a release or a nilling-out property setter. +void ObjCDeallocChecker::checkPreObjCMessage( + const ObjCMethodCall &M, CheckerContext &C) const { + // Only run if -dealloc is on the stack. + SVal DeallocedInstance; + if (!instanceDeallocIsOnStack(C, DeallocedInstance)) + return; + + SymbolRef ReleasedValue = getValueExplicitlyReleased(M, C); + + if (ReleasedValue) { + // An instance variable symbol was released with -release: + // [_property release]; + if (diagnoseExtraRelease(ReleasedValue,M, C)) return; - checkObjCDealloc(this, cast<ObjCImplementationDecl>(D), mgr.getLangOpts(), - BR); + } else { + // An instance variable symbol was released nilling out its property: + // self.property = nil; + ReleasedValue = getValueReleasedByNillingOut(M, C); } -}; + + if (!ReleasedValue) + return; + + SymbolRef InstanceSym = getInstanceSymbolFromIvarSymbol(ReleasedValue); + if (!InstanceSym) + return; + ProgramStateRef InitialState = C.getState(); + + ProgramStateRef ReleasedState = + removeValueRequiringRelease(InitialState, InstanceSym, ReleasedValue); + + if (ReleasedState != InitialState) { + C.addTransition(ReleasedState); + } +} + +/// If the message was a call to '[super dealloc]', diagnose any missing +/// releases. +void ObjCDeallocChecker::checkPostObjCMessage( + const ObjCMethodCall &M, CheckerContext &C) const { + // We perform this check post-message so that if the super -dealloc + // calls a helper method and that this class overrides, any ivars released in + // the helper method will be recorded before checking. + if (isSuperDeallocMessage(M)) + diagnoseMissingReleases(C); +} + +/// Check for missing releases even when -dealloc does not call +/// '[super dealloc]'. +void ObjCDeallocChecker::checkEndFunction( + CheckerContext &C) const { + diagnoseMissingReleases(C); +} + +/// Check for missing releases on early return. +void ObjCDeallocChecker::checkPreStmt( + const ReturnStmt *RS, CheckerContext &C) const { + diagnoseMissingReleases(C); } -void ento::registerObjCDeallocChecker(CheckerManager &mgr) { - mgr.registerChecker<ObjCDeallocChecker>(); +/// If a symbol escapes conservatively assume unseen code released it. +ProgramStateRef ObjCDeallocChecker::checkPointerEscape( + ProgramStateRef State, const InvalidatedSymbols &Escaped, + const CallEvent *Call, PointerEscapeKind Kind) const { + + // Don't treat calls to '[super dealloc]' as escaping for the purposes + // of this checker. Because the checker diagnoses missing releases in the + // post-message handler for '[super dealloc], escaping here would cause + // the checker to never warn. + auto *OMC = dyn_cast_or_null<ObjCMethodCall>(Call); + if (OMC && isSuperDeallocMessage(*OMC)) + return State; + + for (const auto &Sym : Escaped) { + State = State->remove<UnreleasedIvarMap>(Sym); + + SymbolRef InstanceSymbol = getInstanceSymbolFromIvarSymbol(Sym); + if (!InstanceSymbol) + continue; + + State = removeValueRequiringRelease(State, InstanceSymbol, Sym); + } + + return State; +} + +/// Report any unreleased instance variables for the current instance being +/// dealloced. +void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const { + ProgramStateRef State = C.getState(); + + SVal SelfVal; + if (!isInInstanceDealloc(C, SelfVal)) + return; + + const MemRegion *SelfRegion = SelfVal.castAs<loc::MemRegionVal>().getRegion(); + const LocationContext *LCtx = C.getLocationContext(); + + ExplodedNode *ErrNode = nullptr; + + SymbolRef SelfSym = SelfVal.getAsSymbol(); + if (!SelfSym) + return; + + const SymbolSet *OldUnreleased = State->get<UnreleasedIvarMap>(SelfSym); + if (!OldUnreleased) + return; + + SymbolSet NewUnreleased = *OldUnreleased; + SymbolSet::Factory &F = State->getStateManager().get_context<SymbolSet>(); + + ProgramStateRef InitialState = State; + + for (auto *IvarSymbol : *OldUnreleased) { + const TypedValueRegion *TVR = + cast<SymbolRegionValue>(IvarSymbol)->getRegion(); + const ObjCIvarRegion *IvarRegion = cast<ObjCIvarRegion>(TVR); + + // Don't warn if the ivar is not for this instance. + if (SelfRegion != IvarRegion->getSuperRegion()) + continue; + + // Prevent an inlined call to -dealloc in a super class from warning + // about the values the subclass's -dealloc should release. + if (IvarRegion->getDecl()->getContainingInterface() != + cast<ObjCMethodDecl>(LCtx->getDecl())->getClassInterface()) + continue; + + // Prevents diagnosing multiple times for the same instance variable + // at, for example, both a return and at the end of of the function. + NewUnreleased = F.remove(NewUnreleased, IvarSymbol); + + if (State->getStateManager() + .getConstraintManager() + .isNull(State, IvarSymbol) + .isConstrainedTrue()) { + continue; + } + + // A missing release manifests as a leak, so treat as a non-fatal error. + if (!ErrNode) + ErrNode = C.generateNonFatalErrorNode(); + // If we've already reached this node on another path, return without + // diagnosing. + if (!ErrNode) + return; + + std::string Buf; + llvm::raw_string_ostream OS(Buf); + + const ObjCIvarDecl *IvarDecl = IvarRegion->getDecl(); + const ObjCInterfaceDecl *Interface = IvarDecl->getContainingInterface(); + // If the class is known to have a lifecycle with teardown that is + // separate from -dealloc, do not warn about missing releases. We + // suppress here (rather than not tracking for instance variables in + // such classes) because these classes are rare. + if (classHasSeparateTeardown(Interface)) + return; + + ObjCImplDecl *ImplDecl = Interface->getImplementation(); + + const ObjCPropertyImplDecl *PropImpl = + ImplDecl->FindPropertyImplIvarDecl(IvarDecl->getIdentifier()); + + const ObjCPropertyDecl *PropDecl = PropImpl->getPropertyDecl(); + + assert(PropDecl->getSetterKind() == ObjCPropertyDecl::Copy || + PropDecl->getSetterKind() == ObjCPropertyDecl::Retain); + + OS << "The '" << *IvarDecl << "' ivar in '" << *ImplDecl + << "' was "; + + if (PropDecl->getSetterKind() == ObjCPropertyDecl::Retain) + OS << "retained"; + else + OS << "copied"; + + OS << " by a synthesized property but not released" + " before '[super dealloc]'"; + + std::unique_ptr<BugReport> BR( + new BugReport(*MissingReleaseBugType, OS.str(), ErrNode)); + + C.emitReport(std::move(BR)); + } + + if (NewUnreleased.isEmpty()) { + State = State->remove<UnreleasedIvarMap>(SelfSym); + } else { + State = State->set<UnreleasedIvarMap>(SelfSym, NewUnreleased); + } + + if (ErrNode) { + C.addTransition(State, ErrNode); + } else if (State != InitialState) { + C.addTransition(State); + } + + // Make sure that after checking in the top-most frame the list of + // tracked ivars is empty. This is intended to detect accidental leaks in + // the UnreleasedIvarMap program state. + assert(!LCtx->inTopFrame() || State->get<UnreleasedIvarMap>().isEmpty()); +} + +/// Emits a warning if the current context is -dealloc and \param ReleasedValue +/// must not be directly released in a -dealloc. Returns true if a diagnostic +/// was emitted. +bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue, + const ObjCMethodCall &M, + CheckerContext &C) const { + SVal DeallocedInstance; + if (!isInInstanceDealloc(C, DeallocedInstance)) + return false; + + // Try to get the region from which the the released value was loaded. + // Note that, unlike diagnosing for missing releases, here we don't track + // values that must not be released in the state. This is because even if + // these values escape, it is still an error under the rules of MRR to + // release them in -dealloc. + const MemRegion *RegionLoadedFrom = nullptr; + if (auto *DerivedSym = dyn_cast<SymbolDerived>(ReleasedValue)) + RegionLoadedFrom = DerivedSym->getRegion(); + else if (auto *RegionSym = dyn_cast<SymbolRegionValue>(ReleasedValue)) + RegionLoadedFrom = RegionSym->getRegion(); + else + return false; + + auto *ReleasedIvar = dyn_cast<ObjCIvarRegion>(RegionLoadedFrom); + if (!ReleasedIvar) + return false; + + if (DeallocedInstance.castAs<loc::MemRegionVal>().getRegion() != + ReleasedIvar->getSuperRegion()) + return false; + + const LocationContext *LCtx = C.getLocationContext(); + const ObjCIvarDecl *ReleasedIvarDecl = ReleasedIvar->getDecl(); + + // If the ivar belongs to a property that must not be released directly + // in dealloc, emit a warning. + const ObjCImplDecl *Container = getContainingObjCImpl(LCtx); + const ObjCPropertyImplDecl *PropImpl = + Container->FindPropertyImplIvarDecl(ReleasedIvarDecl->getIdentifier()); + if (!PropImpl) + return false; + + if (getDeallocReleaseRequirement(PropImpl) != + ReleaseRequirement::MustNotReleaseDirectly) { + return false; + } + + // If the property is readwrite but it shadows a read-only property in its + // external interface, treat the property a read-only. If the outside + // world cannot write to a property then the internal implementation is free + // to make its own convention about whether the value is stored retained + // or not. We look up the shadow here rather than in + // getDeallocReleaseRequirement() because doing so can be expensive. + const ObjCPropertyDecl *PropDecl = findShadowedPropertyDecl(PropImpl); + if (PropDecl) { + if (PropDecl->isReadOnly()) + return false; + } else { + PropDecl = PropImpl->getPropertyDecl(); + } + + ExplodedNode *ErrNode = C.generateNonFatalErrorNode(); + if (!ErrNode) + return false; + + std::string Buf; + llvm::raw_string_ostream OS(Buf); + + assert(PropDecl->getSetterKind() == ObjCPropertyDecl::Weak || + (PropDecl->getSetterKind() == ObjCPropertyDecl::Assign && + !PropDecl->isReadOnly())); + + OS << "The '" << *PropImpl->getPropertyIvarDecl() + << "' ivar in '" << *Container + << "' was synthesized for "; + + if (PropDecl->getSetterKind() == ObjCPropertyDecl::Weak) + OS << "a weak"; + else + OS << "an assign, readwrite"; + + OS << " property but was released in 'dealloc'"; + + std::unique_ptr<BugReport> BR( + new BugReport(*ExtraReleaseBugType, OS.str(), ErrNode)); + BR->addRange(M.getOriginExpr()->getSourceRange()); + + C.emitReport(std::move(BR)); + + return true; +} + + +ObjCDeallocChecker:: + ObjCDeallocChecker() + : NSObjectII(nullptr), SenTestCaseII(nullptr) { + + MissingReleaseBugType.reset( + new BugType(this, "Missing ivar release (leak)", + categories::MemoryCoreFoundationObjectiveC)); + + ExtraReleaseBugType.reset( + new BugType(this, "Extra ivar release", + categories::MemoryCoreFoundationObjectiveC)); +} + +void ObjCDeallocChecker::initIdentifierInfoAndSelectors( + ASTContext &Ctx) const { + if (NSObjectII) + return; + + NSObjectII = &Ctx.Idents.get("NSObject"); + SenTestCaseII = &Ctx.Idents.get("SenTestCase"); + + IdentifierInfo *DeallocII = &Ctx.Idents.get("dealloc"); + IdentifierInfo *ReleaseII = &Ctx.Idents.get("release"); + DeallocSel = Ctx.Selectors.getSelector(0, &DeallocII); + ReleaseSel = Ctx.Selectors.getSelector(0, &ReleaseII); +} + +/// Returns true if \param M is a call to '[super dealloc]'. +bool ObjCDeallocChecker::isSuperDeallocMessage( + const ObjCMethodCall &M) const { + if (M.getOriginExpr()->getReceiverKind() != ObjCMessageExpr::SuperInstance) + return false; + + return M.getSelector() == DeallocSel; +} + +/// Returns the ObjCImplDecl containing the method declaration in \param LCtx. +const ObjCImplDecl * +ObjCDeallocChecker::getContainingObjCImpl(const LocationContext *LCtx) const { + auto *MD = cast<ObjCMethodDecl>(LCtx->getDecl()); + return cast<ObjCImplDecl>(MD->getDeclContext()); +} + +/// Returns the property that shadowed by \param PropImpl if one exists and +/// nullptr otherwise. +const ObjCPropertyDecl *ObjCDeallocChecker::findShadowedPropertyDecl( + const ObjCPropertyImplDecl *PropImpl) const { + const ObjCPropertyDecl *PropDecl = PropImpl->getPropertyDecl(); + + // Only readwrite properties can shadow. + if (PropDecl->isReadOnly()) + return nullptr; + + auto *CatDecl = dyn_cast<ObjCCategoryDecl>(PropDecl->getDeclContext()); + + // Only class extensions can contain shadowing properties. + if (!CatDecl || !CatDecl->IsClassExtension()) + return nullptr; + + IdentifierInfo *ID = PropDecl->getIdentifier(); + DeclContext::lookup_result R = CatDecl->getClassInterface()->lookup(ID); + for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) { + auto *ShadowedPropDecl = dyn_cast<ObjCPropertyDecl>(*I); + if (!ShadowedPropDecl) + continue; + + if (ShadowedPropDecl->isInstanceProperty()) { + assert(ShadowedPropDecl->isReadOnly()); + return ShadowedPropDecl; + } + } + + return nullptr; +} + +/// Remove the \param Value requiring a release from the tracked set for +/// \param Instance and return the resultant state. +ProgramStateRef ObjCDeallocChecker::removeValueRequiringRelease( + ProgramStateRef State, SymbolRef Instance, SymbolRef Value) const { + assert(Instance); + assert(Value); + + const SymbolSet *Unreleased = State->get<UnreleasedIvarMap>(Instance); + if (!Unreleased) + return State; + + // Mark the value as no longer requiring a release. + SymbolSet::Factory &F = State->getStateManager().get_context<SymbolSet>(); + SymbolSet NewUnreleased = F.remove(*Unreleased, Value); + + if (NewUnreleased.isEmpty()) { + return State->remove<UnreleasedIvarMap>(Instance); + } + + return State->set<UnreleasedIvarMap>(Instance, NewUnreleased); +} + +/// Determines whether the instance variable for \p PropImpl must or must not be +/// released in -dealloc or whether it cannot be determined. +ReleaseRequirement ObjCDeallocChecker::getDeallocReleaseRequirement( + const ObjCPropertyImplDecl *PropImpl) const { + const ObjCIvarDecl *IvarDecl; + const ObjCPropertyDecl *PropDecl; + if (!isSynthesizedRetainableProperty(PropImpl, &IvarDecl, &PropDecl)) + return ReleaseRequirement::Unknown; + + ObjCPropertyDecl::SetterKind SK = PropDecl->getSetterKind(); + + switch (SK) { + // Retain and copy setters retain/copy their values before storing and so + // the value in their instance variables must be released in -dealloc. + case ObjCPropertyDecl::Retain: + case ObjCPropertyDecl::Copy: + return ReleaseRequirement::MustRelease; + + case ObjCPropertyDecl::Weak: + return ReleaseRequirement::MustNotReleaseDirectly; + + case ObjCPropertyDecl::Assign: + // It is common for the ivars for read-only assign properties to + // always be stored retained, so their release requirement cannot be + // be determined. + if (PropDecl->isReadOnly()) + return ReleaseRequirement::Unknown; + + return ReleaseRequirement::MustNotReleaseDirectly; + } +} + +/// Returns the released value if \param M is a call to -release. Returns +/// nullptr otherwise. +SymbolRef +ObjCDeallocChecker::getValueExplicitlyReleased(const ObjCMethodCall &M, + CheckerContext &C) const { + if (M.getSelector() != ReleaseSel) + return nullptr; + + return M.getReceiverSVal().getAsSymbol(); +} + +/// Returns the released value if \param M is a call a setter that releases +/// and nils out its underlying instance variable. +SymbolRef +ObjCDeallocChecker::getValueReleasedByNillingOut(const ObjCMethodCall &M, + CheckerContext &C) const { + SVal ReceiverVal = M.getReceiverSVal(); + if (!ReceiverVal.isValid()) + return nullptr; + + // Is the first argument nil? + if (M.getNumArgs() == 0) + return nullptr; + SVal Arg = M.getArgSVal(0); + ProgramStateRef notNilState, nilState; + std::tie(notNilState, nilState) = + M.getState()->assume(Arg.castAs<DefinedOrUnknownSVal>()); + if (!(nilState && !notNilState)) + return nullptr; + + const ObjCPropertyDecl *Prop = M.getAccessedProperty(); + if (!Prop) + return nullptr; + + ObjCIvarDecl *PropIvarDecl = Prop->getPropertyIvarDecl(); + if (!PropIvarDecl) + return nullptr; + + ProgramStateRef State = C.getState(); + + SVal LVal = State->getLValue(PropIvarDecl, ReceiverVal); + Optional<Loc> LValLoc = LVal.getAs<Loc>(); + if (!LValLoc) + return nullptr; + + SVal CurrentValInIvar = State->getSVal(LValLoc.getValue()); + return CurrentValInIvar.getAsSymbol(); +} + +/// Returns true if the current context is a call to -dealloc and false +/// otherwise. If true, it also sets \param SelfValOut to the value of +/// 'self'. +bool ObjCDeallocChecker::isInInstanceDealloc(const CheckerContext &C, + SVal &SelfValOut) const { + return isInInstanceDealloc(C, C.getLocationContext(), SelfValOut); +} + +/// Returns true if \param LCtx is a call to -dealloc and false +/// otherwise. If true, it also sets \param SelfValOut to the value of +/// 'self'. +bool ObjCDeallocChecker::isInInstanceDealloc(const CheckerContext &C, + const LocationContext *LCtx, + SVal &SelfValOut) const { + auto *MD = dyn_cast<ObjCMethodDecl>(LCtx->getDecl()); + if (!MD || !MD->isInstanceMethod() || MD->getSelector() != DeallocSel) + return false; + + const ImplicitParamDecl *SelfDecl = LCtx->getSelfDecl(); + assert(SelfDecl && "No self in -dealloc?"); + + ProgramStateRef State = C.getState(); + SelfValOut = State->getSVal(State->getRegion(SelfDecl, LCtx)); + return true; +} + +/// Returns true if there is a call to -dealloc anywhere on the stack and false +/// otherwise. If true, it also sets \param SelfValOut to the value of +/// 'self' in the frame for -dealloc. +bool ObjCDeallocChecker::instanceDeallocIsOnStack(const CheckerContext &C, + SVal &InstanceValOut) const { + const LocationContext *LCtx = C.getLocationContext(); + + while (LCtx) { + if (isInInstanceDealloc(C, LCtx, InstanceValOut)) + return true; + + LCtx = LCtx->getParent(); + } + + return false; +} + +/// Returns true if the \param ID is a class in which which is known to have +/// a separate teardown lifecycle. In this case, -dealloc warnings +/// about missing releases should be suppressed. +bool ObjCDeallocChecker::classHasSeparateTeardown( + const ObjCInterfaceDecl *ID) const { + // Suppress if the class is not a subclass of NSObject. + for ( ; ID ; ID = ID->getSuperClass()) { + IdentifierInfo *II = ID->getIdentifier(); + + if (II == NSObjectII) + return false; + + // FIXME: For now, ignore classes that subclass SenTestCase, as these don't + // need to implement -dealloc. They implement tear down in another way, + // which we should try and catch later. + // http://llvm.org/bugs/show_bug.cgi?id=3187 + if (II == SenTestCaseII) + return true; + } + + return true; +} + +void ento::registerObjCDeallocChecker(CheckerManager &Mgr) { + const LangOptions &LangOpts = Mgr.getLangOpts(); + // These checker only makes sense under MRR. + if (LangOpts.getGC() == LangOptions::GCOnly || LangOpts.ObjCAutoRefCount) + return; + + Mgr.registerChecker<ObjCDeallocChecker>(); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits