From: Ted Kremenek Date: Wed, 22 Oct 2008 23:56:21 +0000 (+0000) Subject: Warn about potentially leaked objects that are returned from methods whose names... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3ad2cc89ab6302ef5bda1a1550d405a15df2b013;p=clang Warn about potentially leaked objects that are returned from methods whose names do not follow the Cocoa Memory Management guidelines. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@58012 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp index db8a4598ba..7f89ea6f95 100644 --- a/lib/Analysis/CFRefCount.cpp +++ b/lib/Analysis/CFRefCount.cpp @@ -1067,7 +1067,9 @@ public: 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 @@ void RefVal::print(std::ostream& Out) const { Out << "Leaked"; break; + case ErrorLeakReturned: + Out << "Leaked (Bad naming)"; + break; + case ErrorUseAfterRelease: Out << "Use-After-Release [ERROR]"; break; @@ -1292,9 +1298,9 @@ private: 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 @@ void CFRefCount::EvalStore(ExplodedNodeSet& Dst, // 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(CD)) { + std::string s = MD->getSelector().getName(); + if (!followsFundamentalRule(s.c_str())) { + hasLeak = true; + return state.set(sid, V ^ RefVal::ErrorLeakReturned); + } + } + + // All other cases. + + hasLeak = V.isOwned() || + ((V.isNotOwned() || V.isReturnedOwned()) && V.getCount() > 0); if (!hasLeak) return state.remove(sid); @@ -1824,11 +1861,12 @@ void CFRefCount::EvalEndPath(GRExprEngine& Eng, RefBindings B = St->get(); llvm::SmallVector 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 @@ void CFRefCount::EvalDeadSymbols(ExplodedNodeSet& Dst, 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 @@ PathDiagnosticPiece* CFRefReport::getEndPath(BugReporter& br, if (!getBugType().isLeak()) return RangedBugReport::getEndPath(BR, EndN); - - // Get the retain count. - unsigned long RetCount = EndN->getState()->get(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 @@ PathDiagnosticPiece* CFRefReport::getEndPath(BugReporter& br, if (FirstBinding) os << " and stored into '" << FirstBinding->getString() << '\''; + + + // Get the retain count. + const RefVal* RV = EndN->getState()->get(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(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); } diff --git a/test/Analysis/refcnt_naming.m b/test/Analysis/refcnt_naming.m new file mode 100644 index 0000000000..a923ebaad6 --- /dev/null +++ b/test/Analysis/refcnt_naming.m @@ -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; +@protocol NSObject - (BOOL)isEqual:(id)object; @end +@interface NSObject {} @end +@class NSArray, NSString, NSURL; + +@interface MyClass : NSObject +{ +} +- (NSURL *)myMethod:(NSString *)inString; +@end + +@implementation MyClass + +- (NSURL *)myMethod:(NSString *)inString +{ + NSURL *url = (NSURL *)CFURLCreateWithString(0, (CFStringRef)inString, 0); + return url; // expected-warning{{leak}} +} + +@end