[cfe-commits] r124940 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp test/Analysis/self-init.m

[Argyrios Kyrtzidis]

Author: akirtzidis
Date: Fri Feb  4 23:54:53 2011
New Revision: 124940

URL: http://llvm.org/viewvc/llvm-project?rev=124940&view=rev
Log:
[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

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
    cfe/trunk/test/Analysis/self-init.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp?rev=124940&r1=124939&r2=124940&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp Fri Feb  4 23:54:53 2011
@@ -135,9 +135,7 @@
 
 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 @@
 
 // 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 @@
     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 @@
     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;
     }
   }
 }

Modified: cfe/trunk/test/Analysis/self-init.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/self-init.m?rev=124940&r1=124939&r2=124940&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/self-init.m (original)
+++ cfe/trunk/test/Analysis/self-init.m Fri Feb  4 23:54:53 2011
@@ -42,6 +42,11 @@
 void log(void *obj);
 extern void *somePtr;
 
+ at class MyObj;
+static id _commonInit(MyObj *self) {
+  return self;
+}
+
 @interface MyObj : NSObject {
 	id myivar;
 	int myint;
@@ -141,6 +146,14 @@
 	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