[cfe-commits] r58012 - in /cfe/trunk: lib/Analysis/CFRefCount.cpp test/Analysis/refcnt_naming.m

Ted Kremenek kremenek at apple.com
Wed Oct 22 16:56:22 PDT 2008


Author: kremenek
Date: Wed Oct 22 18:56:21 2008
New Revision: 58012

URL: http://llvm.org/viewvc/llvm-project?rev=58012&view=rev
Log:
Warn about potentially leaked objects that are returned from methods whose names do not follow the Cocoa Memory Management guidelines.

Added:
    cfe/trunk/test/Analysis/refcnt_naming.m
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=58012&r1=58011&r2=58012&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/CFRefCount.cpp (original)
+++ cfe/trunk/lib/Analysis/CFRefCount.cpp Wed Oct 22 18:56:21 2008
@@ -1067,7 +1067,9 @@
     ReturnedNotOwned, // Return object does not pass ownership to caller.
     ErrorUseAfterRelease, // Object used after released.    
     ErrorReleaseNotOwned, // Release of an object that was not owned.
-    ErrorLeak  // A memory leak due to excessive reference counts.
+    ErrorLeak,  // A memory leak due to excessive reference counts.
+    ErrorLeakReturned // A memory leak due to the returning method not having
+                      // the correct naming conventions.            
   };
   
 private:
@@ -1201,6 +1203,10 @@
       Out << "Leaked";
       break;            
       
+    case ErrorLeakReturned:
+      Out << "Leaked (Bad naming)";
+      break;
+      
     case ErrorUseAfterRelease:
       Out << "Use-After-Release [ERROR]";
       break;
@@ -1292,9 +1298,9 @@
                            const GRState* St,
                            RefVal::Kind hasErr, SymbolID Sym);
   
-  const GRState* HandleSymbolDeath(GRStateManager& VMgr,
-                                      const GRState* St,
-                                      SymbolID sid, RefVal V, bool& hasLeak);
+  const GRState* HandleSymbolDeath(GRStateManager& VMgr, const GRState* St,
+                                   const Decl* CD, SymbolID sid, RefVal V,
+                                   bool& hasLeak);
   
 public:
   
@@ -1802,14 +1808,45 @@
 
 // End-of-path.
 
+// The "fundamental rule" for naming conventions of methods:
+//  (url broken into two lines)
+//  http://developer.apple.com/documentation/Cocoa/Conceptual/
+//     MemoryMgmt/Tasks/MemoryManagementRules.html
+//
+// "You take ownership of an object if you create it using a method whose name
+//  begins with “alloc” or “new” or contains “copy” (for example, alloc, 
+//  newObject, or mutableCopy), or if you send it a retain message. You are
+//  responsible for relinquishing ownership of objects you own using release
+//  or autorelease. Any other time you receive an object, you must
+//  not release it."
+//
+static bool followsFundamentalRule(const char* s) {
+  return CStrInCStrNoCase(s, "create") || CStrInCStrNoCase(s, "copy")  || 
+         CStrInCStrNoCase(s, "new");
+}  
+
 const GRState* CFRefCount::HandleSymbolDeath(GRStateManager& VMgr,
-                                          const GRState* St, SymbolID sid,
-                                          RefVal V, bool& hasLeak) {
-    
-  hasLeak = V.isOwned() || 
-            ((V.isNotOwned() || V.isReturnedOwned()) && V.getCount() > 0);
+                                             const GRState* St, const Decl* CD,
+                                             SymbolID sid,
+                                             RefVal V, bool& hasLeak) {
 
   GRStateRef state(St, VMgr);
+  assert (!V.isReturnedOwned() || CD &&
+          "CodeDecl must be available for reporting ReturnOwned errors.");
+  
+  if (V.isReturnedOwned() && V.getCount() == 0)
+    if (const ObjCMethodDecl* MD = dyn_cast<ObjCMethodDecl>(CD)) {
+      std::string s = MD->getSelector().getName();
+      if (!followsFundamentalRule(s.c_str())) {
+        hasLeak = true;
+        return state.set<RefBindings>(sid, V ^ RefVal::ErrorLeakReturned);
+      }
+    }
+
+  // All other cases.
+  
+  hasLeak = V.isOwned() || 
+            ((V.isNotOwned() || V.isReturnedOwned()) && V.getCount() > 0);
 
   if (!hasLeak)
     return state.remove<RefBindings>(sid);
@@ -1824,11 +1861,12 @@
   RefBindings B = St->get<RefBindings>();
   
   llvm::SmallVector<SymbolID, 10> Leaked;
+  const Decl* CodeDecl = &Eng.getGraph().getCodeDecl();
   
   for (RefBindings::iterator I = B.begin(), E = B.end(); I != E; ++I) {
     bool hasLeak = false;
     
-    St = HandleSymbolDeath(Eng.getStateManager(), St,
+    St = HandleSymbolDeath(Eng.getStateManager(), St, CodeDecl,
                            (*I).first, (*I).second, hasLeak);
     
     if (hasLeak) Leaked.push_back((*I).first);
@@ -1876,7 +1914,7 @@
     
     bool hasLeak = false;
     
-    St = HandleSymbolDeath(Eng.getStateManager(), St, *I, *T, hasLeak);
+    St = HandleSymbolDeath(Eng.getStateManager(), St, 0, *I, *T, hasLeak);
     
     if (hasLeak)
       Leaked.push_back(*I);    
@@ -2453,9 +2491,6 @@
   
   if (!getBugType().isLeak())
     return RangedBugReport::getEndPath(BR, EndN);
-
-  // Get the retain count.
-  unsigned long RetCount = EndN->getState()->get<RefBindings>(Sym)->getCount();
   
   // We are a leak.  Walk up the graph to get to the first node where the
   // symbol appeared, and also get the first VarDecl that tracked object
@@ -2518,9 +2553,22 @@
   
   if (FirstBinding)
     os << " and stored into '" << FirstBinding->getString() << '\'';  
+
+  
+  // Get the retain count.
+  const RefVal* RV = EndN->getState()->get<RefBindings>(Sym);
   
-  os << " is no longer referenced after this point and has a retain count of +"
-     << RetCount << " (object leaked).";
+  if (RV->getKind() == RefVal::ErrorLeakReturned) {
+    ObjCMethodDecl& MD = cast<ObjCMethodDecl>(BR.getGraph().getCodeDecl());
+    os << " is returned from a method whose name ('"
+       << MD.getSelector().getName()
+       << "') does not contain 'create', "
+          "'copy', or 'new'.  This violates the naming convention rules given"
+          " in the Memory Management Guide for Cocoa (object leaked).";
+  }
+  else
+    os << " is no longer referenced after this point and has a retain count of +"
+       << RV->getCount() << " (object leaked).";
   
   return new PathDiagnosticPiece(L, os.str(), Hint);
 }

Added: cfe/trunk/test/Analysis/refcnt_naming.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/refcnt_naming.m?rev=58012&view=auto

==============================================================================
--- cfe/trunk/test/Analysis/refcnt_naming.m (added)
+++ cfe/trunk/test/Analysis/refcnt_naming.m Wed Oct 22 18:56:21 2008
@@ -0,0 +1,26 @@
+// RUN: clang -checker-cfref -verify %s
+
+typedef const struct __CFString * CFStringRef;
+typedef const struct __CFAllocator * CFAllocatorRef;
+typedef const struct __CFURL * CFURLRef;
+extern CFURLRef CFURLCreateWithString(CFAllocatorRef allocator, CFStringRef URLString, CFURLRef baseURL);
+typedef signed char BOOL;
+ at protocol NSObject  - (BOOL)isEqual:(id)object; @end
+ at interface NSObject <NSObject> {} @end
+ at class NSArray, NSString, NSURL;
+
+ at interface MyClass : NSObject
+{
+}
+- (NSURL *)myMethod:(NSString *)inString;
+ at end
+
+ at implementation MyClass
+
+- (NSURL *)myMethod:(NSString *)inString
+{
+	NSURL *url = (NSURL *)CFURLCreateWithString(0, (CFStringRef)inString, 0);
+	return url; // expected-warning{{leak}}
+}
+
+ at end





More information about the cfe-commits mailing list