[analyzer] Fix a false positive of the 'self' initialization checker.

A common pattern in classes with multiple initializers is to put the
subclass's common initialization bits into a static function that receives
the value of 'self', e.g:

   if (!(self = [super init]))
     return nil;
   if (!(self = _commonInit(self)))
     return nil;

It was reported that 'self' was not set to the result of [super init].
Until we can use inter-procedural analysis, in such a call, transfer the
ObjCSelfInitChecker flags associated with 'self' to the result of the call.

Fixes rdar://8937441 & http://llvm.org/PR9094

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@124940 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
index ff1e5dde76a2398124d591c6345f19967bc606eb..786a588531c6b8fcc4328f5000397690bcd41546 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
@@ -135,9 +135,7 @@ static SelfFlagEnum getSelfFlags(SVal val, CheckerContext &C) {
 
 static void addSelfFlag(SVal val, SelfFlagEnum flag, CheckerContext &C) {
   const GRState *state = C.getState();
-  // FIXME: We tag the symbol that the SVal wraps but this is conceptually
-  // wrong, we should tag the SVal; the fact that there is a symbol behind the
-  // SVal is irrelevant.
+  // We tag the symbol that the SVal wraps.
   if (SymbolRef sym = val.getAsSymbol())
     C.addTransition(state->set<SelfFlag>(sym, getSelfFlags(val, C) | flag));
 }
@@ -227,7 +225,19 @@ void ObjCSelfInitChecker::PreVisitReturnStmt(CheckerContext &C,
 
 // When a call receives a reference to 'self', [Pre/Post]VisitGenericCall pass
 // the SelfFlags from the object 'self' point to before the call, to the new
-// object after the call.
+// object after the call. This is to avoid invalidation of 'self' by logging
+// functions.
+// Another common pattern in classes with multiple initializers is to put the
+// subclass's common initialization bits into a static function that receives
+// the value of 'self', e.g:
+// @code
+//   if (!(self = [super init]))
+//     return nil;
+//   if (!(self = _commonInit(self)))
+//     return nil;
+// @endcode
+// Until we can use inter-procedural analysis, in such a call, transfer the
+// SelfFlags to the result of the call.
 
 void ObjCSelfInitChecker::PreVisitGenericCall(CheckerContext &C,
                                               const CallExpr *CE) {
@@ -238,6 +248,9 @@ void ObjCSelfInitChecker::PreVisitGenericCall(CheckerContext &C,
     if (isSelfVar(argV, C)) {
       preCallSelfFlags = getSelfFlags(state->getSVal(cast<Loc>(argV)), C);
       return;
+    } else if (hasSelfFlag(argV, SelfFlag_Self, C)) {
+      preCallSelfFlags = getSelfFlags(argV, C);
+      return;
     }
   }
 }
@@ -251,6 +264,9 @@ void ObjCSelfInitChecker::PostVisitGenericCall(CheckerContext &C,
     if (isSelfVar(argV, C)) {
       addSelfFlag(state->getSVal(cast<Loc>(argV)), preCallSelfFlags, C);
       return;
+    } else if (hasSelfFlag(argV, SelfFlag_Self, C)) {
+      addSelfFlag(state->getSVal(CE), preCallSelfFlags, C);
+      return;
     }
   }
 }

diff --git a/test/Analysis/self-init.m b/test/Analysis/self-init.m
index 1dc5aa92f9c3953ab3df219c3fabc80557ae304d..b4c6f29e387df52975775a40305a5a4339c05dfe 100644 (file)
--- a/test/Analysis/self-init.m
+++ b/test/Analysis/self-init.m
@@ -42,6 +42,11 @@ extern NSString * const NSConnectionReplyMode;
 void log(void *obj);
 extern void *somePtr;
 
+@class MyObj;
+static id _commonInit(MyObj *self) {
+  return self;
+}
+
 @interface MyObj : NSObject {
        id myivar;
        int myint;
@@ -141,6 +146,14 @@ extern void *somePtr;
        return self; // expected-warning {{Returning 'self'}}
 }
 
+-(id)init14 {
+  if (!(self = [super init]))
+    return 0;
+  if (!(self = _commonInit(self)))
+    return 0;
+  return self;
+}
+
 -(void)doSomething {}
 
 @end