[cfe-commits] r51433 - /cfe/trunk/lib/Analysis/CFRefCount.cpp

[Ted Kremenek]

Author: kremenek
Date: Thu May 22 12:31:13 2008
New Revision: 51433

URL: http://llvm.org/viewvc/llvm-project?rev=51433&view=rev
Log:
Expand retain/release checker to consider methods/function calls that cause a
tracked object to "escape": it's reference count might be incremented by the
called function, thus causing an object's lifetime to extend beyond when the
local reference count is decremented to 0.

This addresses: <rdar://problem/5933215>

Modified:
    cfe/trunk/lib/Analysis/CFRefCount.cpp

Modified: cfe/trunk/lib/Analysis/CFRefCount.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFRefCount.cpp?rev=51433&r1=51432&r2=51433&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/CFRefCount.cpp (original)
+++ cfe/trunk/lib/Analysis/CFRefCount.cpp Thu May 22 12:31:13 2008
@@ -94,7 +94,7 @@
 //===----------------------------------------------------------------------===//
 
 namespace {  
-  enum ArgEffect { IncRef, DecRef, DoNothing, StopTracking };
+  enum ArgEffect { IncRef, DecRef, DoNothing, StopTracking, MayEscape };
   typedef std::vector<std::pair<unsigned,ArgEffect> > ArgEffects;
 }
 
@@ -286,12 +286,12 @@
   
   RetainSummary* getPersistentSummary(ArgEffects* AE, RetEffect RetEff,
                                       ArgEffect ReceiverEff = DoNothing,
-                                      ArgEffect DefaultEff = DoNothing);
+                                      ArgEffect DefaultEff = MayEscape);
                  
 
   RetainSummary* getPersistentSummary(RetEffect RE,
                                       ArgEffect ReceiverEff = DoNothing,
-                                      ArgEffect DefaultEff = DoNothing) {
+                                      ArgEffect DefaultEff = MayEscape) {
     return getPersistentSummary(getArgEffects(), RE, ReceiverEff, DefaultEff);
   }
   
@@ -499,19 +499,22 @@
   switch (func) {
     case cfretain: {
       ScratchArgs.push_back(std::make_pair(0, IncRef));
-      return getPersistentSummary(RetEffect::MakeAlias(0));
+      return getPersistentSummary(RetEffect::MakeAlias(0),
+                                  DoNothing, DoNothing);
     }
       
     case cfrelease: {
       ScratchArgs.push_back(std::make_pair(0, DecRef));
-      return getPersistentSummary(RetEffect::MakeNoRet());
+      return getPersistentSummary(RetEffect::MakeNoRet(),
+                                  DoNothing, DoNothing);
     }
       
     case cfmakecollectable: {
       if (GCEnabled)
         ScratchArgs.push_back(std::make_pair(0, DecRef));
       
-      return getPersistentSummary(RetEffect::MakeAlias(0));    
+      return getPersistentSummary(RetEffect::MakeAlias(0),
+                                  DoNothing, DoNothing);    
     }
       
     default:
@@ -525,7 +528,7 @@
     dyn_cast<FunctionType>(FD->getType().getTypePtr());
   
   if (FT && !isCFRefType(FT->getResultType()))
-    return 0;
+    return getPersistentSummary(RetEffect::MakeNoRet());
 
   // FIXME: Add special-cases for functions that retain/release.  For now
   //  just handle the default case.
@@ -549,14 +552,14 @@
     // in the future.
   
     if (!isCFRefType(RetTy) && !RetTy->isPointerType())
-      return 0;
+      return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing);
   }
   
   // FIXME: Add special-cases for functions that retain/release.  For now
   //  just handle the default case.
   
   assert (ScratchArgs.empty());  
-  return getPersistentSummary(RetEffect::MakeNotOwned());
+  return getPersistentSummary(RetEffect::MakeNotOwned(), DoNothing, DoNothing);
 }
 
 //===----------------------------------------------------------------------===//
@@ -801,7 +804,7 @@
   
   // State creation: normal state.
   
-  static RefVal makeOwned(unsigned Count = 0) {
+  static RefVal makeOwned(unsigned Count = 1) {
     return RefVal(Owned, Count);
   }
   
@@ -887,16 +890,6 @@
   }
 }
   
-static inline unsigned GetCount(RefVal V) {
-  switch (V.getKind()) {
-    default:
-      return V.getCount();
-
-    case RefVal::Owned:
-      return V.getCount()+1;
-  }
-}
-  
 //===----------------------------------------------------------------------===//
 // Transfer functions.
 //===----------------------------------------------------------------------===//
@@ -1096,7 +1089,7 @@
 }
 
 static inline ArgEffect GetArgE(RetainSummary* Summ, unsigned idx) {
-  return Summ ? Summ->getArg(idx) : DoNothing;
+  return Summ ? Summ->getArg(idx) : MayEscape;
 }
 
 static inline RetEffect GetRetEffect(RetainSummary* Summ) {
@@ -1396,7 +1389,7 @@
   ValueState StImpl = *St;  
 
   StImpl.CheckerState =
-    RefBFactory.Add(B, sid, RefVal::makeLeak(GetCount(V))).getRoot();
+    RefBFactory.Add(B, sid, RefVal::makeLeak(V.getCount())).getRoot();
   
   return VMgr.getPersistentState(StImpl);
 }
@@ -1517,7 +1510,8 @@
       
     case RefVal::Owned: { 
       unsigned cnt = X.getCount();
-      X = RefVal::makeReturnedOwned(cnt);
+      assert (cnt > 0);
+      X = RefVal::makeReturnedOwned(cnt - 1);
       break;
     }
       
@@ -1588,6 +1582,14 @@
   switch (E) {
     default:
       assert (false && "Unhandled CFRef transition.");
+
+    case MayEscape:
+      if (V.getKind() == RefVal::Owned) {
+        V = RefVal::makeNotOwned(V.getCount());
+        break;
+      }
+
+      // Fall-through.
       
     case DoNothing:
       if (!isGCEnabled() && V.getKind() == RefVal::Released) {
@@ -1634,7 +1636,7 @@
           
         case RefVal::Owned: {
           unsigned Count = V.getCount();
-          V = Count > 0 ? RefVal::makeOwned(Count - 1) : RefVal::makeReleased();
+          V = Count > 1 ? RefVal::makeOwned(Count - 1) : RefVal::makeReleased();
           break;
         }
           
@@ -1907,14 +1909,16 @@
   switch (CurrV.getKind()) {
     case RefVal::Owned:
     case RefVal::NotOwned:
-      assert (PrevV.getKind() == CurrV.getKind());
+
+      if (PrevV.getCount() == CurrV.getCount())
+        return 0;
       
       if (PrevV.getCount() > CurrV.getCount())
         os << "Reference count decremented.";
       else
         os << "Reference count incremented.";
       
-      if (unsigned Count = GetCount(CurrV)) {
+      if (unsigned Count = CurrV.getCount()) {
 
         os << " Object has +" << Count;
         
@@ -2027,7 +2031,7 @@
     RefBindings B = RefBindings((RefBindings::TreeTy*) St->CheckerState);
     RefBindings::TreeTy* T = B.SlimFind(Sym);
     assert (T);
-    RetCount = GetCount(T->getValue().second);
+    RetCount = T->getValue().second.getCount();
   }
 
   // We are a leak.  Walk up the graph to get to the first node where the