Index: include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- include/clang/StaticAnalyzer/Checkers/Checkers.td +++ include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -520,6 +520,9 @@ def ObjCPropertyChecker : Checker<"ObjCProperty">, HelpText<"Check for proper uses of Objective-C properties">; +def OSObjectRetainCountChecker : Checker<"OSObjectRetainCount">, + HelpText<"Check for leaks and improper reference count management for OSObject">; + } // end "osx" let ParentPackage = Cocoa in { Index: include/clang/StaticAnalyzer/Core/RetainSummaryManager.h =================================================================== --- include/clang/StaticAnalyzer/Core/RetainSummaryManager.h +++ include/clang/StaticAnalyzer/Core/RetainSummaryManager.h @@ -469,6 +469,8 @@ } }; +class RetainSummaryTemplate; + class RetainSummaryManager { typedef llvm::DenseMap FuncSummariesTy; @@ -483,7 +485,10 @@ /// Records whether or not the analyzed code runs in ARC mode. const bool ARCEnabled; - /// Track sublcasses of OSObject + /// Track Objective-C and CoreFoundation objects. + const bool TrackObjCAndCFObjects; + + /// Track sublcasses of OSObject. const bool TrackOSObjects; /// FuncSummaries - A map from FunctionDecls to summaries. @@ -626,13 +631,36 @@ const RetainSummary * generateSummary(const FunctionDecl *FD, bool &AllowAnnotations); + /// Return a summary for OSObject, or nullptr if not found. + const RetainSummary *getSummaryForOSObject(const FunctionDecl *FD, + StringRef FName, QualType RetTy); + + /// Return a summary for Objective-C or CF object, or nullptr if not found. + const RetainSummary *getSummaryForObjCOrCFObject( + const FunctionDecl *FD, + StringRef FName, + QualType RetTy, + const FunctionType *FT, + bool &AllowAnnotations); + + /// Apply the annotation of {@code pd} in function {@code FD} + /// to the resulting summary stored in out-parameter {@code Template}. + /// \return whether an annotation was applied. + bool applyFunctionParamAnnotationEffect(const ParmVarDecl *pd, + unsigned parm_idx, + const FunctionDecl *FD, + ArgEffects::Factory &AF, + RetainSummaryTemplate &Template); + public: RetainSummaryManager(ASTContext &ctx, bool usesARC, - bool trackOSObject) + bool trackObjCAndCFObjects, + bool trackOSObjects) : Ctx(ctx), ARCEnabled(usesARC), - TrackOSObjects(trackOSObject), + TrackObjCAndCFObjects(trackObjCAndCFObjects), + TrackOSObjects(trackOSObjects), AF(BPAlloc), ScratchArgs(AF.getEmptyMap()), ObjCAllocRetE(usesARC ? RetEffect::MakeNotOwned(RetEffect::ObjC) : RetEffect::MakeOwned(RetEffect::ObjC)), @@ -709,6 +737,7 @@ void updateSummaryFromAnnotations(const RetainSummary *&Summ, const FunctionDecl *FD); + void updateSummaryForCall(const RetainSummary *&Summ, const CallEvent &Call); @@ -716,9 +745,21 @@ RetEffect getObjAllocRetEffect() const { return ObjCAllocRetE; } + /// \return True if the declaration has an attribute {@code T}, + /// AND we are tracking that attribute. False otherwise. + template + bool hasEnabledAttr(const Decl *D) { + return isAttrEnabled() && D->hasAttr(); + } + + /// Check whether we are tracking properties specified by the attributes. + template + bool isAttrEnabled(); + friend class RetainSummaryTemplate; }; + // Used to avoid allocating long-term (BPAlloc'd) memory for default retain // summaries. If a function or method looks like it has a default summary, but // it has annotations, the annotations are added to the stack-based template Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h =================================================================== --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -92,7 +92,7 @@ /// See the RefVal::Kind enum for possible values. unsigned RawKind : 5; - /// The kind of object being tracked (CF or ObjC), if known. + /// The kind of object being tracked (CF or ObjC or OSObject), if known. /// /// See the RetEffect::ObjKind enum for possible values. unsigned RawObjectKind : 3; @@ -268,10 +268,12 @@ mutable bool ShouldResetSummaryLog; public: - /// Optional setting to indicate if leak reports should include - /// the allocation line. - bool IncludeAllocationLine; - bool ShouldCheckOSObjectRetainCount; + + /// Track Objective-C and CoreFoundation objects. + bool TrackObjCAndCFObjects = false; + + /// Track sublcasses of OSObject. + bool TrackOSObjects = false; RetainCountChecker() : ShouldResetSummaryLog(false) {} @@ -290,7 +292,7 @@ bool ARCEnabled = (bool)Ctx.getLangOpts().ObjCAutoRefCount; if (!Summaries) { Summaries.reset(new RetainSummaryManager( - Ctx, ARCEnabled, ShouldCheckOSObjectRetainCount)); + Ctx, ARCEnabled, TrackObjCAndCFObjects, TrackOSObjects)); } else { assert(Summaries->isARCEnabled() == ARCEnabled); } Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -1504,9 +1504,10 @@ void ento::registerRetainCountChecker(CheckerManager &Mgr) { auto *Chk = Mgr.registerChecker(); + Chk->TrackObjCAndCFObjects = true; +} - AnalyzerOptions &Options = Mgr.getAnalyzerOptions(); - - Chk->ShouldCheckOSObjectRetainCount = Options.getCheckerBooleanOption( - "CheckOSObject", true, Chk); +void ento::registerOSObjectRetainCountChecker(CheckerManager &Mgr) { + auto *Chk = Mgr.registerChecker(); + Chk->TrackOSObjects = true; } Index: lib/StaticAnalyzer/Core/RetainSummaryManager.cpp =================================================================== --- lib/StaticAnalyzer/Core/RetainSummaryManager.cpp +++ lib/StaticAnalyzer/Core/RetainSummaryManager.cpp @@ -24,6 +24,31 @@ using namespace clang; using namespace ento; +template +constexpr static bool isOneOf() { + return false; +} + +/// Helper function to check whether the class is one of the +/// rest of varargs. +template +constexpr static bool isOneOf() { + return std::is_same::value || isOneOf(); +} + +template bool RetainSummaryManager::isAttrEnabled() { + if (isOneOf()) { + return TrackObjCAndCFObjects; + } else if (isOneOf()) { + return TrackOSObjects; + } + llvm_unreachable("Unexpected attribute passed"); +} + ArgEffects RetainSummaryManager::getArgEffects() { ArgEffects AE = ScratchArgs; ScratchArgs = AF.getEmptyMap(); @@ -116,30 +141,60 @@ } const RetainSummary * -RetainSummaryManager::generateSummary(const FunctionDecl *FD, - bool &AllowAnnotations) { - // We generate "stop" summaries for implicitly defined functions. - if (FD->isImplicit()) { - return getPersistentStopSummary(); +RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD, + StringRef FName, QualType RetTy) { + if (RetTy->isPointerType()) { + const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl(); + if (PD && isOSObjectSubclass(PD)) { + if (const IdentifierInfo *II = FD->getIdentifier()) { + if (isOSObjectDynamicCast(II->getName())) + return getDefaultSummary(); + + // All objects returned with functions *not* starting with + // get, or iterators, are returned at +1. + if ((!II->getName().startswith("get") && + !II->getName().startswith("Get")) || + isOSIteratorSubclass(PD)) { + return getOSSummaryCreateRule(FD); + } else { + return getOSSummaryGetRule(FD); + } + } + } } - const IdentifierInfo *II = FD->getIdentifier(); + if (const auto *MD = dyn_cast(FD)) { + const CXXRecordDecl *Parent = MD->getParent(); + if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) { + if (FName == "release") + return getOSSummaryReleaseRule(FD); - StringRef FName = II ? II->getName() : ""; + if (FName == "retain") + return getOSSummaryRetainRule(FD); - // Strip away preceding '_'. Doing this here will effect all the checks - // down below. - FName = FName.substr(FName.find_first_not_of('_')); + if (FName == "free") + return getOSSummaryFreeRule(FD); + + if (MD->getOverloadedOperator() == OO_New) + return getOSSummaryCreateRule(MD); + } + } + + return nullptr; +} + +const RetainSummary *RetainSummaryManager::getSummaryForObjCOrCFObject( + const FunctionDecl *FD, + StringRef FName, + QualType RetTy, + const FunctionType *FT, + bool &AllowAnnotations) { - // Inspect the result type. Strip away any typedefs. - const auto *FT = FD->getType()->getAs(); - QualType RetTy = FT->getReturnType(); std::string RetTyName = RetTy.getAsString(); // FIXME: This should all be refactored into a chain of "summary lookup" // filters. assert(ScratchArgs.isEmpty()); - if (FName == "pthread_create" || FName == "pthread_setspecific") { // Part of: and . // This will be addressed better with IPA. @@ -230,30 +285,11 @@ if (RetTy->isPointerType()) { - const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl(); - if (TrackOSObjects && PD && isOSObjectSubclass(PD)) { - if (const IdentifierInfo *II = FD->getIdentifier()) { - - if (isOSObjectDynamicCast(II->getName())) - return getDefaultSummary(); - - // All objects returned with functions *not* starting with - // get, or iterators, are returned at +1. - if ((!II->getName().startswith("get") && - !II->getName().startswith("Get")) || - isOSIteratorSubclass(PD)) { - return getOSSummaryCreateRule(FD); - } else { - return getOSSummaryGetRule(FD); - } - } - } - // For CoreFoundation ('CF') types. if (cocoa::isRefType(RetTy, "CF", FName)) { if (isRetain(FD, FName)) { - // CFRetain isn't supposed to be annotated. However, this may as well - // be a user-made "safe" CFRetain function that is incorrectly + // CFRetain isn't supposed to be annotated. However, this may as + // well be a user-made "safe" CFRetain function that is incorrectly // annotated as cf_returns_retained due to lack of better options. // We want to ignore such annotation. AllowAnnotations = false; @@ -294,27 +330,9 @@ } } - if (const auto *MD = dyn_cast(FD)) { - const CXXRecordDecl *Parent = MD->getParent(); - if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) { - if (FName == "release") - return getOSSummaryReleaseRule(FD); - - if (FName == "retain") - return getOSSummaryRetainRule(FD); - - if (FName == "free") - return getOSSummaryFreeRule(FD); - - if (MD->getOverloadedOperator() == OO_New) - return getOSSummaryCreateRule(MD); - } - } - // Check for release functions, the only kind of functions that we care // about that don't return a pointer type. - if (FName.size() >= 2 && FName[0] == 'C' && - (FName[1] == 'F' || FName[1] == 'G')) { + if (FName.startswith("CG") || FName.startswith("CF")) { // Test for 'CGCF'. FName = FName.substr(FName.startswith("CGCF") ? 4 : 2); @@ -349,11 +367,41 @@ } } - if (const auto *MD = dyn_cast(FD)) { + return nullptr; +} + +const RetainSummary * +RetainSummaryManager::generateSummary(const FunctionDecl *FD, + bool &AllowAnnotations) { + // We generate "stop" summaries for implicitly defined functions. + if (FD->isImplicit()) + return getPersistentStopSummary(); + + const IdentifierInfo *II = FD->getIdentifier(); + + StringRef FName = II ? II->getName() : ""; + + // Strip away preceding '_'. Doing this here will effect all the checks + // down below. + FName = FName.substr(FName.find_first_not_of('_')); + + // Inspect the result type. Strip away any typedefs. + const auto *FT = FD->getType()->getAs(); + QualType RetTy = FT->getReturnType(); + + if (TrackOSObjects) + if (const RetainSummary *S = getSummaryForOSObject(FD, FName, RetTy)) + return S; + + if (TrackObjCAndCFObjects) + if (const RetainSummary *S = + getSummaryForObjCOrCFObject(FD, FName, RetTy, FT, AllowAnnotations)) + return S; + + if (const auto *MD = dyn_cast(FD)) if (!(TrackOSObjects && isOSObjectRelated(MD))) return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, StopTracking, DoNothing); - } return getDefaultSummary(); } @@ -658,7 +706,7 @@ Optional RetainSummaryManager::getRetEffectFromAnnotations(QualType RetTy, const Decl *D) { - if (cocoa::isCocoaObjectRef(RetTy)) { + if (TrackObjCAndCFObjects && cocoa::isCocoaObjectRef(RetTy)) { if (D->hasAttr()) return ObjCAllocRetE; @@ -670,17 +718,17 @@ return None; } - if (D->hasAttr()) { + if (hasEnabledAttr(D)) { return RetEffect::MakeOwned(RetEffect::CF); - } else if (D->hasAttr()) { + } else if (hasEnabledAttr(D)) { return RetEffect::MakeOwned(RetEffect::OS); } else if (hasRCAnnotation(D, "rc_ownership_returns_retained")) { return RetEffect::MakeOwned(RetEffect::Generalized); } - if (D->hasAttr()) { + if (hasEnabledAttr(D)) { return RetEffect::MakeNotOwned(RetEffect::CF); - } else if (D->hasAttr()) { + } else if (hasEnabledAttr(D)) { return RetEffect::MakeNotOwned(RetEffect::OS); } else if (hasRCAnnotation(D, "rc_ownership_returns_not_retained")) { return RetEffect::MakeNotOwned(RetEffect::Generalized); @@ -694,22 +742,20 @@ return None; } -/// Apply the annotation of {@code pd} in function {@code FD} -/// to the resulting summary stored in out-parameter {@code Template}. -/// Return whether an annotation was applied. -bool applyFunctionParamAnnotationEffect(const ParmVarDecl *pd, +bool RetainSummaryManager::applyFunctionParamAnnotationEffect(const ParmVarDecl *pd, unsigned parm_idx, const FunctionDecl *FD, ArgEffects::Factory &AF, RetainSummaryTemplate &Template) { - if (pd->hasAttr()) { + if (hasEnabledAttr(pd)) { Template->addArg(AF, parm_idx, DecRefMsg); return true; - } else if (pd->hasAttr() || pd->hasAttr() || + } else if (hasEnabledAttr(pd) || + hasEnabledAttr(pd) || hasRCAnnotation(pd, "rc_ownership_consumed")) { Template->addArg(AF, parm_idx, DecRef); return true; - } else if (pd->hasAttr() || + } else if (hasEnabledAttr(pd) || hasRCAnnotation(pd, "rc_ownership_returns_retained")) { QualType PointeeTy = pd->getType()->getPointeeType(); if (!PointeeTy.isNull()) { @@ -718,7 +764,7 @@ return true; } } - } else if (pd->hasAttr()) { + } else if (hasEnabledAttr(pd)) { QualType PointeeTy = pd->getType()->getPointeeType(); if (!PointeeTy.isNull()) { if (coreFoundation::isCFObjectRef(PointeeTy)) { @@ -760,7 +806,7 @@ if (Optional RetE = getRetEffectFromAnnotations(RetTy, FD)) Template->setRetEffect(*RetE); - if (FD->hasAttr()) + if (hasEnabledAttr(FD)) Template->setThisEffect(DecRef); } @@ -779,8 +825,7 @@ // Effects on the parameters. unsigned parm_idx = 0; - for (ObjCMethodDecl::param_const_iterator - pi=MD->param_begin(), pe=MD->param_end(); + for (auto pi=MD->param_begin(), pe=MD->param_end(); pi != pe; ++pi, ++parm_idx) { const ParmVarDecl *pd = *pi; if (pd->hasAttr()) { @@ -933,6 +978,10 @@ const ObjCMethodDecl *MD, QualType RetTy, ObjCMethodSummariesTy &CachedSummaries) { + // Objective-C method summaries are only applicable to ObjC and CF objects. + if (!TrackObjCAndCFObjects) + return getDefaultSummary(); + // Look up a summary in our summary cache. const RetainSummary *Summ = CachedSummaries.find(ID, S); @@ -1043,7 +1092,9 @@ CallEffects CallEffects::getEffect(const ObjCMethodDecl *MD) { ASTContext &Ctx = MD->getASTContext(); LangOptions L = Ctx.getLangOpts(); - RetainSummaryManager M(Ctx, L.ObjCAutoRefCount, /*TrackOSObjects=*/false); + RetainSummaryManager M(Ctx, L.ObjCAutoRefCount, + /*TrackNSAndCFObjects=*/true, + /*TrackOSObjects=*/false); const RetainSummary *S = M.getMethodSummary(MD); CallEffects CE(S->getRetEffect()); CE.Receiver = S->getReceiverEffect(); @@ -1057,7 +1108,9 @@ CallEffects CallEffects::getEffect(const FunctionDecl *FD) { ASTContext &Ctx = FD->getASTContext(); LangOptions L = Ctx.getLangOpts(); - RetainSummaryManager M(Ctx, L.ObjCAutoRefCount, /*TrackOSObjects=*/false); + RetainSummaryManager M(Ctx, L.ObjCAutoRefCount, + /*TrackNSAndCFObjects=*/true, + /*TrackOSObjects=*/false); const RetainSummary *S = M.getFunctionSummary(FD); CallEffects CE(S->getRetEffect()); unsigned N = FD->param_size(); Index: test/Analysis/osobject-retain-release.cpp =================================================================== --- test/Analysis/osobject-retain-release.cpp +++ test/Analysis/osobject-retain-release.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-output=text -verify %s +// RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core,osx -analyzer-output=text -verify %s struct OSMetaClass; Index: test/Analysis/test-separate-retaincount.cpp =================================================================== --- test/Analysis/test-separate-retaincount.cpp +++ test/Analysis/test-separate-retaincount.cpp @@ -0,0 +1,37 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -analyzer-disable-checker osx.cocoa.RetainCount -DNO_CF_OBJECT -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -analyzer-disable-checker osx.OSObjectRetainCount -DNO_OS_OBJECT -verify %s + +typedef const void * CFTypeRef; +extern CFTypeRef CFRetain(CFTypeRef cf); +extern void CFRelease(CFTypeRef cf); + +#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained)) +extern CFTypeRef CFCreate() CF_RETURNS_RETAINED; + +using size_t = decltype(sizeof(int)); + +struct OSObject { + virtual void retain(); + virtual void release(); + + static void * operator new(size_t size); + virtual ~OSObject(){} +}; + +void cf_overrelease() { + CFTypeRef cf = CFCreate(); + CFRelease(cf); + CFRelease(cf); +#ifndef NO_CF_OBJECT + // expected-warning@-2{{Reference-counted object is used after it is released}} +#endif +} + +void osobject_overrelease() { + OSObject *o = new OSObject; + o->release(); + o->release(); +#ifndef NO_OS_OBJECT + // expected-warning@-2{{Reference-counted object is used after it is released}} +#endif +}