From: Devin Coughlin Date: Thu, 11 Feb 2016 22:13:20 +0000 (+0000) Subject: [analyzer] Improve pattern matching in ObjCDealloc checker. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6df7834ea1564dc1602d8d98a7c9390d17e8bf25;p=clang [analyzer] Improve pattern matching in ObjCDealloc checker. Look through PseudoObjectExpr and OpaqueValueExprs when scanning for release-like operations. This commit also adds additional tests in anticipation of re-writing this as a path-sensitive checker. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@260608 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp index 1a3519307a..902babfe50 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(E)) + E = POE->getSyntacticForm()->IgnoreParenCasts(); + if (const OpaqueValueExpr *OVE = dyn_cast(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(S)) if (ME->getSelector() == Release) if (ME->getInstanceReceiver()) - if (Expr *Receiver = ME->getInstanceReceiver()->IgnoreParenCasts()) - if (ObjCIvarRefExpr *E = dyn_cast(Receiver)) + if (const Expr *Receiver = peel(ME->getInstanceReceiver())) + if (auto *E = dyn_cast(Receiver)) if (E->getDecl() == ID) return true; // [self setMyIvar:nil]; if (ObjCMessageExpr *ME = dyn_cast(S)) if (ME->getInstanceReceiver()) - if (Expr *Receiver = ME->getInstanceReceiver()->IgnoreParenCasts()) - if (DeclRefExpr *E = dyn_cast(Receiver)) + if (const Expr *Receiver = peel(ME->getInstanceReceiver())) + if (auto *E = dyn_cast(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(S)) if (BO->isAssignmentOp()) - if (ObjCPropertyRefExpr *PRE = - dyn_cast(BO->getLHS()->IgnoreParenCasts())) + if (auto *PRE = dyn_cast(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 3a2b556c11..e782f9968f 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 1fbd9723a2..2067b3e85a 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; }