From: Anna Zaks Date: Mon, 16 Apr 2012 21:51:09 +0000 (+0000) Subject: [analyzer] Fix a false alarm in SelfInitChecker (radar://11235991). X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9a70cddef6850f302615b4f5d27f16ec45926ca6;p=clang [analyzer] Fix a false alarm in SelfInitChecker (radar://11235991). Along with it, fix a couple of other corner cases and add more tests. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@154866 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp index d15c8ba408..97b58cf79b 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp @@ -183,9 +183,6 @@ static void checkForInvalidSelf(const Expr *E, CheckerContext &C, void ObjCSelfInitChecker::checkPostObjCMessage(ObjCMessage msg, CheckerContext &C) const { - CallOrObjCMessage MsgWrapper(msg, C.getState(), C.getLocationContext()); - checkPostStmt(MsgWrapper, C); - // When encountering a message that does initialization (init rule), // tag the return value so that we know later on that if self has this value // then it is properly initialized. @@ -209,6 +206,9 @@ void ObjCSelfInitChecker::checkPostObjCMessage(ObjCMessage msg, return; } + CallOrObjCMessage MsgWrapper(msg, C.getState(), C.getLocationContext()); + checkPostStmt(MsgWrapper, C); + // We don't check for an invalid 'self' in an obj-c message expression to cut // down false positives where logging functions get information from self // (like its class) or doing "invalidation" on self when the initialization @@ -277,6 +277,11 @@ void ObjCSelfInitChecker::checkPreStmt(const CallOrObjCMessage &CE, CheckerContext &C) const { ProgramStateRef state = C.getState(); unsigned NumArgs = CE.getNumArgs(); + // If we passed 'self' as and argument to the call, record it in the state + // to be propagated after the call. + // Note, we could have just given up, but try to be more optimistic here and + // assume that the functions are going to continue initialization or will not + // modify self. for (unsigned i = 0; i < NumArgs; ++i) { SVal argV = CE.getArgSVal(i); if (isSelfVar(argV, C)) { @@ -298,14 +303,24 @@ void ObjCSelfInitChecker::checkPostStmt(const CallOrObjCMessage &CE, for (unsigned i = 0; i < NumArgs; ++i) { SVal argV = CE.getArgSVal(i); if (isSelfVar(argV, C)) { + // If the address of 'self' is being passed to the call, assume that the + // 'self' after the call will have the same flags. + // EX: log(&self) SelfFlagEnum prevFlags = (SelfFlagEnum)state->get(); state = state->remove(); addSelfFlag(state, state->getSVal(cast(argV)), prevFlags, C); return; } else if (hasSelfFlag(argV, SelfFlag_Self, C)) { + // If 'self' is passed to the call by value, assume that the function + // returns 'self'. So assign the flags, which were set on 'self' to the + // return value. + // EX: self = performMoreInitialization(self) SelfFlagEnum prevFlags = (SelfFlagEnum)state->get(); state = state->remove(); - addSelfFlag(state, state->getSVal(cast(argV)), prevFlags, C); + const Expr *CallExpr = CE.getOriginExpr(); + if (CallExpr) + addSelfFlag(state, state->getSVal(CallExpr, C.getLocationContext()), + prevFlags, C); return; } } @@ -358,7 +373,7 @@ static bool isSelfVar(SVal location, CheckerContext &C) { return false; loc::MemRegionVal MRV = cast(location); - if (const DeclRegion *DR = dyn_cast(MRV.getRegion())) + if (const DeclRegion *DR = dyn_cast(MRV.stripCasts())) return (DR->getDecl() == analCtx->getSelfDecl()); return false; diff --git a/test/Analysis/self-init.m b/test/Analysis/self-init.m index 3db42e9cf5..d515173343 100644 --- a/test/Analysis/self-init.m +++ b/test/Analysis/self-init.m @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=osx.cocoa.SelfInit %s -verify +// RUN: %clang_cc1 -analyze -analyzer-checker=osx.cocoa.SelfInit -fobjc-default-synthesize-properties %s -verify @class NSZone, NSCoder; @protocol NSObject- (id)self; @@ -48,9 +48,7 @@ void log(void *obj); extern void *somePtr; @class MyObj; -static id _commonInit(MyObj *self) { - return self; -} +extern id _commonInit(MyObj *self); @interface MyObj : NSObject { id myivar; @@ -59,6 +57,7 @@ static id _commonInit(MyObj *self) { -(id)_init; -(id)initWithSomething:(int)x; -(void)doSomething; ++(id)commonInitMember:(id)s; @end @interface MyProxyObj : NSProxy {} @@ -90,6 +89,14 @@ static id _commonInit(MyObj *self) { return self; } +-(id)init4_w { + [super init]; + if (self) { + log(&self); + } + return self; // expected-warning {{Returning 'self' while it is not set to the result of '[(super or self) init...]'}} +} + - (id)initWithSomething:(int)x { if ((self = [super init])) myint = x; @@ -157,6 +164,12 @@ static id _commonInit(MyObj *self) { return self; } +-(id)init14_w { + [super init]; + self = _commonInit(self); + return self; // expected-warning {{Returning 'self' while it is not set to the result of '[(super or self) init...]'}} +} + -(id)init15 { if (!(self = [super init])) return 0; @@ -176,6 +189,28 @@ static id _commonInit(MyObj *self) { return 0; } +-(id)init18 { + self = [super init]; + self = _commonInit(self); + return self; +} + ++(id)commonInitMember:(id)s { + return s; +} + +-(id)init19 { + self = [super init]; + self = [MyObj commonInitMember:self]; + return self; +} + +-(id)init19_w { + [super init]; + self = [MyObj commonInitMember:self]; + return self; // expected-warning {{Returning 'self'}} +} + -(void)doSomething {} @end @@ -201,3 +236,21 @@ static id _commonInit(MyObj *self) { } @end +// Test radar:11235991 - passing self to a call to super. +@protocol MyDelegate +@end +@interface Object : NSObject +- (id) initWithObject: (id)i; +@end +@interface Derived: Object +- (id) initWithInt: (int)t; +@property (nonatomic, retain, readwrite) Object *size; +@end +@implementation Derived +- (id) initWithInt: (int)t { + if ((self = [super initWithObject:self])) { + _size = [[Object alloc] init]; + } + return self; +} +@end