From: Ted Kremenek <kremenek@apple.com>
Date: Tue, 12 May 2009 20:06:54 +0000 (+0000)
Subject: Fix: <rdar://problem/6320065> false positive - init method returns an object owned... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=78a35a3900b39702ffb9835702a1329f8d3e04b3;p=clang

Fix: <rdar://problem/6320065> false positive - init method returns an object owned by caller

Now 'init' methods are treated by the retain/release checker as
claiming their receiver and allocating a new object.


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

diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index dc9602a7b2..fc6de60032 100644
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -279,7 +279,8 @@ namespace {
 class VISIBILITY_HIDDEN RetEffect {
 public:
   enum Kind { NoRet, Alias, OwnedSymbol, OwnedAllocatedSymbol,
-              NotOwnedSymbol, GCNotOwnedSymbol, ReceiverAlias };
+              NotOwnedSymbol, GCNotOwnedSymbol, ReceiverAlias,
+              OwnedWhenTrackedReceiver };
     
   enum ObjKind { CF, ObjC, AnyObj };  
 
@@ -302,9 +303,14 @@ public:
   }
   
   bool isOwned() const {
-    return K == OwnedSymbol || K == OwnedAllocatedSymbol;
+    return K == OwnedSymbol || K == OwnedAllocatedSymbol ||
+           K == OwnedWhenTrackedReceiver;
   }
     
+  static RetEffect MakeOwnedWhenTrackedReceiver() {
+    return RetEffect(OwnedWhenTrackedReceiver, ObjC);
+  }
+  
   static RetEffect MakeAlias(unsigned Idx) {
     return RetEffect(Alias, Idx);
   }
@@ -635,6 +641,8 @@ class VISIBILITY_HIDDEN RetainSummaryManager {
   enum UnaryFuncKind { cfretain, cfrelease, cfmakecollectable };  
   
 public:
+  RetEffect getObjAllocRetEffect() const { return ObjCAllocRetE; }
+
   RetainSummary *getDefaultSummary() {
     RetainSummary *Summ = (RetainSummary*) BPAlloc.Allocate<RetainSummary>();
     return new (Summ) RetainSummary(DefaultSummary);
@@ -1127,12 +1135,14 @@ RetainSummary* RetainSummaryManager::getCFSummaryGetRule(FunctionDecl* FD) {
 
 RetainSummary*
 RetainSummaryManager::getInitMethodSummary(QualType RetTy) {
-  assert(ScratchArgs.isEmpty());
-    
-  // 'init' methods only return an alias if the return type is a location type.
-  return getPersistentSummary(Loc::IsLocType(RetTy)
-                              ? RetEffect::MakeReceiverAlias()
-                              : RetEffect::MakeNoRet());
+  assert(ScratchArgs.isEmpty());    
+  // 'init' methods conceptually return a newly allocated object and claim
+  // the receiver.  
+  if (isTrackedObjCObjectType(RetTy) || isTrackedCFObjectType(RetTy))
+    return getPersistentSummary(RetEffect::MakeOwnedWhenTrackedReceiver(),
+                                DecRefMsg);
+  
+  return getDefaultSummary();
 }
 
 void
@@ -1333,9 +1343,9 @@ void RetainSummaryManager::InitializeMethodSummaries() {
   
   // Create the "init" selector.  It just acts as a pass-through for the
   // receiver.
-  RetainSummary* InitSumm =
-    getPersistentSummary(RetEffect::MakeReceiverAlias());
-  addNSObjectMethSummary(GetNullarySelector("init", Ctx), InitSumm);
+  addNSObjectMethSummary(GetNullarySelector("init", Ctx),
+                 getPersistentSummary(RetEffect::MakeOwnedWhenTrackedReceiver(),
+                 DecRefMsg));
   
   // The next methods are allocators.
   RetainSummary* Summ = getPersistentSummary(ObjCAllocRetE);  
@@ -1378,33 +1388,33 @@ void RetainSummaryManager::InitializeMethodSummaries() {
   //  Thus, we need to track an NSWindow's display status.
   //  This is tracked in <rdar://problem/6062711>.
   //  See also http://llvm.org/bugs/show_bug.cgi?id=3714.
-  RetainSummary *NoTrackYet = getPersistentSummary(RetEffect::MakeNoRet());
+  RetainSummary *NoTrackYet = getPersistentSummary(RetEffect::MakeNoRet(),
+                                                   StopTracking,
+                                                   StopTracking);
   
   addClassMethSummary("NSWindow", "alloc", NoTrackYet);
 
-
 #if 0
-  RetainSummary *NSWindowSumm =
-    getPersistentSummary(RetEffect::MakeReceiverAlias(), StopTracking);
-  
-  addInstMethSummary("NSWindow", NSWindowSumm, "initWithContentRect",
+  addInstMethSummary("NSWindow", NoTrackYet, "initWithContentRect",
                      "styleMask", "backing", "defer", NULL);
   
-  addInstMethSummary("NSWindow", NSWindowSumm, "initWithContentRect",
+  addInstMethSummary("NSWindow", NoTrackYet, "initWithContentRect",
                      "styleMask", "backing", "defer", "screen", NULL);
 #endif
-    
+  
   // For NSPanel (which subclasses NSWindow), allocated objects are not
   //  self-owned.
   // FIXME: For now we don't track NSPanels. object for the same reason
   //   as for NSWindow objects.
   addClassMethSummary("NSPanel", "alloc", NoTrackYet);
   
-  addInstMethSummary("NSPanel", InitSumm, "initWithContentRect",
+#if 0
+  addInstMethSummary("NSPanel", NoTrackYet, "initWithContentRect",
                      "styleMask", "backing", "defer", NULL);
   
-  addInstMethSummary("NSPanel", InitSumm, "initWithContentRect",
+  addInstMethSummary("NSPanel", NoTrackYet, "initWithContentRect",
                      "styleMask", "backing", "defer", "screen", NULL);
+#endif
 
   // Create NSAssertionHandler summaries.
   addPanicSummary("NSAssertionHandler", "handleFailureInFunction", "file",
@@ -2788,6 +2798,20 @@ void CFRefCount::EvalSummary(ExplodedNodeSet<GRState>& Dst,
   // Consult the summary for the return value.  
   RetEffect RE = Summ.getRetEffect();
   
+  if (RE.getKind() == RetEffect::OwnedWhenTrackedReceiver) {
+    assert(Receiver);
+    SVal V = state.GetSValAsScalarOrLoc(Receiver);
+    bool found = false;
+    if (SymbolRef Sym = V.getAsLocSymbol())
+      if (state.get<RefBindings>(Sym)) {
+        found = true;
+        RE = Summaries.getObjAllocRetEffect();
+      }
+
+    if (!found)
+      RE = RetEffect::MakeNoRet();
+  } 
+  
   switch (RE.getKind()) {
     default:
       assert (false && "Unhandled RetEffect."); break;
@@ -3033,7 +3057,7 @@ void CFRefCount::EvalReturn(ExplodedNodeSet<GRState>& Dst,
   // Change the reference count.  
   RefVal X = *T;  
   
-  switch (X.getKind()) {      
+  switch (X.getKind()) { 
     case RefVal::Owned: { 
       unsigned cnt = X.getCount();
       assert (cnt > 0);
diff --git a/test/Analysis/retain-release.m b/test/Analysis/retain-release.m
index 5f2a1c8f85..7c039fa007 100644
--- a/test/Analysis/retain-release.m
+++ b/test/Analysis/retain-release.m
@@ -549,6 +549,49 @@ typedef CFTypeRef OtherRef;
 }
 @end
 
+//===----------------------------------------------------------------------===//
+//<rdar://problem/6320065> false positive - init method returns an object owned by caller
+//===----------------------------------------------------------------------===//
+
+@interface RDar6320065 : NSObject {
+  NSString *_foo;
+}
+- (id)initReturningNewClass;
+- (id)initReturningNewClassBad;
+- (id)initReturningNewClassBad2;
+@end
+
+@interface RDar6320065Subclass : RDar6320065
+@end
+
+@implementation RDar6320065
+- (id)initReturningNewClass {
+  [self release];
+  self = [[RDar6320065Subclass alloc] init]; // no-warning
+  return self;
+}
+- (id)initReturningNewClassBad {
+  [self release];
+  [[RDar6320065Subclass alloc] init]; // expected-warning {{leak}}
+  return self;
+}
+- (id)initReturningNewClassBad2 {
+  [self release];
+  self = [[RDar6320065Subclass alloc] init];
+  return [self autorelease]; // expected-warning{{Object with +0 retain counts returned to caller where a +1 (owning) retain count is expected}}
+}
+
+@end
+
+@implementation RDar6320065Subclass
+@end
+
+int RDar6320065_test() {
+  RDar6320065 *test = [[RDar6320065 alloc] init]; // no-warning
+  [test release];
+  return 0;
+}
+
 //===----------------------------------------------------------------------===//
 // Tests of ownership attributes.
 //===----------------------------------------------------------------------===//