This commit reflects changes to the retain/release checker motivated by my
recent discussions with Thomas Clement and Ken Ferry concerning the "fundamental rule" for Cocoa memory management (http://developer.apple.com/documentation/Cocoa/Conceptual/MemoryMgmt/Tasks/MemoryManagementRules.html). Here is the revised behavior of the checker concerning tracking retain/release counts for objects returned from message expressions involving instance methods: 1) Track the returned object if the return type of the message expression is id<..>, id, or a pointer to *any* object that subclasses NSObject. Such objects are assumed to have a retain count. Previously the checker only tracked objects when the receiver of the message expression was part of the standard Cocoa API (i.e., had class names prefixed with 'NS'). This should significantly expand the amount of checking performed. 2) Consider the object owned if the selector of the message expression contains "alloc", "new", or "copy". Previously we also considered "create", but this doesn't follow from the fundamental rule (discussions with the Cocoa folks confirms this). llvm-svn: 61837
This commit is contained in:
parent
7e47cc7cda
commit
1d92d2c813
|
@ -55,8 +55,9 @@ using llvm::CStrInCStrNoCase;
|
|||
//
|
||||
static bool followsFundamentalRule(const char* s) {
|
||||
while (*s == '_') ++s;
|
||||
return CStrInCStrNoCase(s, "create") || CStrInCStrNoCase(s, "copy") ||
|
||||
CStrInCStrNoCase(s, "new") == s || CStrInCStrNoCase(s, "alloc") == s;
|
||||
return CStrInCStrNoCase(s, "copy")
|
||||
|| CStrInCStrNoCase(s, "new") == s
|
||||
|| CStrInCStrNoCase(s, "alloc") == s;
|
||||
}
|
||||
|
||||
static bool followsReturnRule(const char* s) {
|
||||
|
@ -128,25 +129,6 @@ static bool isCGRefType(QualType T) {
|
|||
return true;
|
||||
}
|
||||
|
||||
static bool isNSType(QualType T) {
|
||||
|
||||
if (!T->isPointerType())
|
||||
return false;
|
||||
|
||||
ObjCInterfaceType* OT = dyn_cast<ObjCInterfaceType>(T.getTypePtr());
|
||||
|
||||
if (!OT)
|
||||
return false;
|
||||
|
||||
const char* ClsName = OT->getDecl()->getIdentifier()->getName();
|
||||
assert (ClsName);
|
||||
|
||||
if (ClsName[0] != 'N' || ClsName[1] != 'S')
|
||||
return false;
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
//===----------------------------------------------------------------------===//
|
||||
// Primitives used for constructing summaries for function/method calls.
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
@ -561,6 +543,8 @@ public:
|
|||
void InitializeClassMethodSummaries();
|
||||
void InitializeMethodSummaries();
|
||||
|
||||
bool isTrackedObjectType(QualType T);
|
||||
|
||||
private:
|
||||
|
||||
void addClsMethSummary(IdentifierInfo* ClsII, Selector S,
|
||||
|
@ -626,6 +610,8 @@ public:
|
|||
|
||||
} // end anonymous namespace
|
||||
|
||||
|
||||
|
||||
//===----------------------------------------------------------------------===//
|
||||
// Implementation of checker data structures.
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
@ -696,6 +682,33 @@ RetainSummaryManager::getPersistentSummary(ArgEffects* AE, RetEffect RetEff,
|
|||
return Summ;
|
||||
}
|
||||
|
||||
//===----------------------------------------------------------------------===//
|
||||
// Predicates.
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
bool RetainSummaryManager::isTrackedObjectType(QualType T) {
|
||||
if (!Ctx.isObjCObjectPointerType(T))
|
||||
return false;
|
||||
|
||||
// Does it subclass NSObject?
|
||||
ObjCInterfaceType* OT = dyn_cast<ObjCInterfaceType>(T.getTypePtr());
|
||||
|
||||
// We assume that id<..>, id, and "Class" all represent tracked objects.
|
||||
if (!OT)
|
||||
return true;
|
||||
|
||||
// Does the object type subclass NSObject?
|
||||
// FIXME: We can memoize here if this gets too expensive.
|
||||
IdentifierInfo* NSObjectII = &Ctx.Idents.get("NSObject");
|
||||
ObjCInterfaceDecl* ID = OT->getDecl();
|
||||
|
||||
for ( ; ID ; ID = ID->getSuperClass())
|
||||
if (ID->getIdentifier() == NSObjectII)
|
||||
return true;
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
//===----------------------------------------------------------------------===//
|
||||
// Summary creation for functions (largely uses of Core Foundation).
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
@ -953,28 +966,21 @@ RetainSummaryManager::getMethodSummary(ObjCMessageExpr* ME,
|
|||
|
||||
if (I != ObjCMethodSummaries.end())
|
||||
return I->second;
|
||||
|
||||
if (!ME->getType()->isPointerType())
|
||||
return 0;
|
||||
|
||||
// "initXXX": pass-through for receiver.
|
||||
|
||||
// "initXXX": pass-through for receiver.
|
||||
const char* s = S.getIdentifierInfoForSlot(0)->getName();
|
||||
assert (ScratchArgs.empty());
|
||||
|
||||
if (strncmp(s, "init", 4) == 0 || strncmp(s, "_init", 5) == 0)
|
||||
return getInitMethodSummary(ME);
|
||||
return getInitMethodSummary(ME);
|
||||
|
||||
// "copyXXX", "createXXX", "newXXX": allocators.
|
||||
|
||||
if (!isNSType(ME->getReceiver()->getType()))
|
||||
// Look for methods that return an owned object.
|
||||
if (!isTrackedObjectType(Ctx.getCanonicalType(ME->getType())))
|
||||
return 0;
|
||||
|
||||
if (followsFundamentalRule(s)) {
|
||||
|
||||
RetEffect E = isGCEnabled() ? RetEffect::MakeNoRet()
|
||||
: RetEffect::MakeOwned(true);
|
||||
|
||||
if (followsFundamentalRule(s)) {
|
||||
RetEffect E = isGCEnabled() ? RetEffect::MakeNoRet()
|
||||
: RetEffect::MakeOwned(true);
|
||||
RetainSummary* Summ = getPersistentSummary(E);
|
||||
ObjCMethodSummaries[ME] = Summ;
|
||||
return Summ;
|
||||
|
@ -2648,7 +2654,7 @@ PathDiagnosticPiece* CFRefReport::getEndPath(BugReporter& br,
|
|||
ObjCMethodDecl& MD = cast<ObjCMethodDecl>(BR.getGraph().getCodeDecl());
|
||||
os << " is returned from a method whose name ('"
|
||||
<< MD.getSelector().getAsString()
|
||||
<< "') does not contain 'create' or 'copy' or otherwise starts with"
|
||||
<< "') does not contain 'copy' or otherwise starts with"
|
||||
" 'new' or 'alloc'. This violates the naming convention rules given"
|
||||
" in the Memory Management Guide for Cocoa (object leaked).";
|
||||
}
|
||||
|
|
|
@ -180,7 +180,9 @@ void f12() {
|
|||
|
||||
@interface SharedClass : NSObject
|
||||
+ (id)sharedInstance;
|
||||
- (id)notShared;
|
||||
@end
|
||||
|
||||
@implementation SharedClass
|
||||
|
||||
- (id)_init {
|
||||
|
@ -190,6 +192,10 @@ void f12() {
|
|||
return self;
|
||||
}
|
||||
|
||||
- (id)notShared {
|
||||
return [[SharedClass alloc] _init]; // expected-warning{{This violates the naming convention rules}}
|
||||
}
|
||||
|
||||
+ (id)sharedInstance {
|
||||
static SharedClass *_sharedInstance = 0;
|
||||
if (!_sharedInstance) {
|
||||
|
@ -198,3 +204,8 @@ void f12() {
|
|||
return _sharedInstance; // no-warning
|
||||
}
|
||||
@end
|
||||
|
||||
id testSharedClassFromFunction() {
|
||||
return [[SharedClass alloc] _init]; // no-warning
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue