[analyzer] Fix a false alarm in SelfInitChecker (radar://11235991).

Along with it, fix a couple of other corner cases and add more tests.

llvm-svn: 154866

clang/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<PreCallSelfFlags>();
       state = state->remove<PreCallSelfFlags>();
       addSelfFlag(state, state->getSVal(cast<Loc>(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<PreCallSelfFlags>();
       state = state->remove<PreCallSelfFlags>();
-      addSelfFlag(state, state->getSVal(cast<Loc>(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<loc::MemRegionVal>(location);
-  if (const DeclRegion *DR = dyn_cast<DeclRegion>(MRV.getRegion()))
+  if (const DeclRegion *DR = dyn_cast<DeclRegion>(MRV.stripCasts()))
     return (DR->getDecl() == analCtx->getSelfDecl());
 
   return false;

clang/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 <MyDelegate>
+- (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