diff --git a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp index 1a3519307a21151a683f33ca3086d83218893101..902babfe502caed67569165d1516b3335ff4775c 100644 --- a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp @@ -28,6 +28,16 @@ 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; +} + static bool scan_ivar_release(Stmt *S, const ObjCIvarDecl *ID, const ObjCPropertyDecl *PD, Selector Release, @@ -38,30 +48,29 @@ static bool scan_ivar_release(Stmt *S, const ObjCIvarDecl *ID, if (ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S)) if (ME->getSelector() == Release) if (ME->getInstanceReceiver()) - if (Expr *Receiver = ME->getInstanceReceiver()->IgnoreParenCasts()) - if (ObjCIvarRefExpr *E = dyn_cast<ObjCIvarRefExpr>(Receiver)) + 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 (Expr *Receiver = ME->getInstanceReceiver()->IgnoreParenCasts()) - if (DeclRefExpr *E = dyn_cast<DeclRefExpr>(Receiver)) + 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 && - ME->getArg(0)->isNullPointerConstant(Ctx, + 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 (ObjCPropertyRefExpr *PRE = - dyn_cast<ObjCPropertyRefExpr>(BO->getLHS()->IgnoreParenCasts())) + if (auto *PRE = dyn_cast<ObjCPropertyRefExpr>(peel(BO->getLHS()))) if (PRE->isExplicitProperty() && PRE->getExplicitProperty() == PD) - if (BO->getRHS()->isNullPointerConstant(Ctx, + if (peel(BO->getRHS())->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNull)) { // This is only a 'release' if the property kind is not // 'assign'. diff --git a/test/Analysis/DeallocMissingRelease.m b/test/Analysis/DeallocMissingRelease.m index 3a2b556c11da810a061b5d8f00915c7025dfe3d7..e782f9968f9885cc9d1c2b4f0843b3250ad0d5c0 100644 --- a/test/Analysis/DeallocMissingRelease.m +++ b/test/Analysis/DeallocMissingRelease.m @@ -189,4 +189,64 @@ typedef struct objc_selector *SEL; #endif } @end + +@interface ClassWithControlFlowInRelease : NSObject { + BOOL _ivar1; +} +@property (retain) NSObject *ivar2; +@end + +@implementation ClassWithControlFlowInRelease +- (void)dealloc; { + if (_ivar1) { + // We really should warn because there is a path through -dealloc on which + // _ivar2 is not released. +#if NON_ARC + [_ivar2 release]; // no-warning +#endif + } + +#if NON_ARC + [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 +@end + +@implementation ClassWithNildOutProperty +- (void)dealloc; { + self.ivar = nil; + +#if NON_ARC + [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 +@end + +@implementation ClassWithNildOutPropertyViaSetter +- (void)dealloc; { + [self setIvar:nil]; + +#if NON_ARC + [super dealloc]; +#endif +} + +@end + // CHECK: 4 warnings generated. diff --git a/test/Analysis/PR2978.m b/test/Analysis/PR2978.m index 1fbd9723a2b1ce2d08025b655f4272bfa76ff3eb..2067b3e85af37120baafea014b524465a37ee1d1 100644 --- a/test/Analysis/PR2978.m +++ b/test/Analysis/PR2978.m @@ -17,6 +17,8 @@ id _L; id _N; id _M; + id _P; + id _Q; id _V; id _W; } @@ -27,6 +29,9 @@ @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 Q; + @property(retain) id V; @property(retain) id W; -(id) O; @@ -41,6 +46,7 @@ @synthesize L = _L; // no-warning @synthesize N = _N; // no-warning @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 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'}} @@ -57,6 +63,9 @@ [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]; + self.P = 0; [super dealloc]; return 0; }