Index: clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h +++ clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h @@ -344,6 +344,9 @@ : Args(A), DefaultArgEffect(defaultEff), Receiver(ReceiverEff), This(ThisEff), Ret(R) {} + RetainSummary(ArgEffects::Factory &AF) : Args(AF.getEmptyMap()), + Ret(RetEffect::MakeNoRet()){}; + /// getArg - Return the argument effect on the argument specified by /// idx (starting from 0). ArgEffect getArg(unsigned idx) const { @@ -494,10 +497,14 @@ const bool ARCEnabled; /// Track Objective-C and CoreFoundation objects. - const bool TrackObjCAndCFObjects; + bool TrackObjCAndCFObjects = false; /// Track sublcasses of OSObject. - const bool TrackOSObjects; + bool TrackOSObjects = false; + + /// Track out parameters returning OSObjects even when they don't + /// have annotations. + bool TrackUnannotatedOSOutParameters = false; /// FuncSummaries - A map from FunctionDecls to summaries. FuncSummariesTy FuncSummaries; @@ -654,23 +661,29 @@ RetainSummaryTemplate &Template); public: - RetainSummaryManager(ASTContext &ctx, - bool usesARC, - bool trackObjCAndCFObjects, - bool trackOSObjects) - : Ctx(ctx), - ARCEnabled(usesARC), - TrackObjCAndCFObjects(trackObjCAndCFObjects), - TrackOSObjects(trackOSObjects), - AF(BPAlloc), - ObjCAllocRetE(usesARC ? RetEffect::MakeNotOwned(ObjKind::ObjC) - : RetEffect::MakeOwned(ObjKind::ObjC)), - ObjCInitRetE(usesARC ? RetEffect::MakeNotOwned(ObjKind::ObjC) - : RetEffect::MakeOwnedWhenTrackedReceiver()) { + RetainSummaryManager(ASTContext &ctx) + : Ctx(ctx), ARCEnabled((bool)ctx.getLangOpts().ObjCAutoRefCount), + AF(BPAlloc), + ObjCAllocRetE(ARCEnabled ? RetEffect::MakeNotOwned(ObjKind::ObjC) + : RetEffect::MakeOwned(ObjKind::ObjC)), + ObjCInitRetE(ARCEnabled ? RetEffect::MakeNotOwned(ObjKind::ObjC) + : RetEffect::MakeOwnedWhenTrackedReceiver()) { InitializeClassMethodSummaries(); InitializeMethodSummaries(); } + void setTrackObjCAndCFObjects() { + TrackObjCAndCFObjects = true; + } + + void setTrackOSObjects() { + TrackOSObjects = true; + } + + void setTrackUnannotatedOSOutParameters() { + TrackUnannotatedOSOutParameters = true; + } + enum class BehaviorSummary { // Function does not return. NoOp, Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -256,39 +256,22 @@ mutable std::unique_ptr overAutorelease, returnNotOwnedForOwned; mutable std::unique_ptr leakWithinFunction, leakAtReturn; - mutable std::unique_ptr Summaries; + std::unique_ptr Summaries; public: static constexpr const char *DeallocTagDescription = "DeallocSent"; - /// Track Objective-C and CoreFoundation objects. - bool TrackObjCAndCFObjects = false; - - /// Track sublcasses of OSObject. - bool TrackOSObjects = false; - - RetainCountChecker() {} + RetainCountChecker(ASTContext &Ctx) { + Summaries = llvm::make_unique(Ctx); + } RefCountBug *getLeakWithinFunctionBug(const LangOptions &LOpts) const; RefCountBug *getLeakAtReturnBug(const LangOptions &LOpts) const; - RetainSummaryManager &getSummaryManager(ASTContext &Ctx) const { - // FIXME: We don't support ARC being turned on and off during one analysis. - // (nor, for that matter, do we support changing ASTContexts) - bool ARCEnabled = (bool)Ctx.getLangOpts().ObjCAutoRefCount; - if (!Summaries) { - Summaries.reset(new RetainSummaryManager( - Ctx, ARCEnabled, TrackObjCAndCFObjects, TrackOSObjects)); - } else { - assert(Summaries->isARCEnabled() == ARCEnabled); - } + RetainSummaryManager &getSummaryManager() const { return *Summaries; } - RetainSummaryManager &getSummaryManager(CheckerContext &C) const { - return getSummaryManager(C.getASTContext()); - } - void printState(raw_ostream &Out, ProgramStateRef State, const char *NL, const char *Sep) const override; Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -392,7 +392,7 @@ void RetainCountChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { - RetainSummaryManager &Summaries = getSummaryManager(C); + RetainSummaryManager &Summaries = getSummaryManager(); // Leave null if no receiver. QualType ReceiverType; @@ -696,7 +696,7 @@ if (RE.getKind() == RetEffect::OwnedWhenTrackedReceiver) { if (ReceiverIsTracked) - RE = getSummaryManager(C).getObjAllocRetEffect(); + RE = getSummaryManager().getObjAllocRetEffect(); else RE = RetEffect::MakeNoRet(); } @@ -913,7 +913,7 @@ if (!FD) return false; - RetainSummaryManager &SmrMgr = getSummaryManager(C); + RetainSummaryManager &SmrMgr = getSummaryManager(); QualType ResultTy = CE->getCallReturnType(C.getASTContext()); // See if the function has 'rc_ownership_trusted_implementation' @@ -1051,7 +1051,7 @@ X = *T; // Consult the summary of the enclosing method. - RetainSummaryManager &Summaries = getSummaryManager(C); + RetainSummaryManager &Summaries = getSummaryManager(); const Decl *CD = &Pred->getCodeDecl(); RetEffect RE = RetEffect::MakeNoRet(); @@ -1382,7 +1382,7 @@ if (!Ctx.inTopFrame()) return; - RetainSummaryManager &SmrMgr = getSummaryManager(Ctx); + RetainSummaryManager &SmrMgr = getSummaryManager(); const LocationContext *LCtx = Ctx.getLocationContext(); const FunctionDecl *FD = dyn_cast(LCtx->getDecl()); @@ -1524,9 +1524,16 @@ // Checker registration. //===----------------------------------------------------------------------===// -void ento::registerRetainCountChecker(CheckerManager &Mgr) { - auto *Chk = Mgr.registerChecker(); - Chk->TrackObjCAndCFObjects = true; +// FIXME: hack for getting the value of an option with a known prefix, +// otherwise the current system does not work if the same +// checker is registered multiple times with different names. +static bool getOption(const AnalyzerOptions &Options, StringRef Prefix, + StringRef Option, StringRef Default) { + auto I = Options.Config.find((Prefix + ":" + Option).str()); + if (I != Options.Config.end()) { + return I->getValue() == Default; + } + return false; } // FIXME: remove this, hack for backwards compatibility: @@ -1534,14 +1541,22 @@ // osx.cocoa.RetainCount, and it should be possible to disable // osx.OSObjectRetainCount using osx.cocoa.RetainCount:CheckOSObject=false. static bool hasPrevCheckOSObjectOptionDisabled(AnalyzerOptions &Options) { - auto I = Options.Config.find("osx.cocoa.RetainCount:CheckOSObject"); - if (I != Options.Config.end()) - return I->getValue() == "false"; - return false; + return getOption(Options, "osx.cocoa.RetainCount", "CheckOSObject", "false"); +} + +void ento::registerRetainCountChecker(CheckerManager &Mgr) { + auto *Chk = Mgr.registerChecker(Mgr.getASTContext()); + Chk->getSummaryManager().setTrackObjCAndCFObjects(); } void ento::registerOSObjectRetainCountChecker(CheckerManager &Mgr) { - auto *Chk = Mgr.registerChecker(); + AnalyzerOptions &Opts = Mgr.getAnalyzerOptions(); + auto *Chk = Mgr.registerChecker(Mgr.getASTContext()); if (!hasPrevCheckOSObjectOptionDisabled(Mgr.getAnalyzerOptions())) - Chk->TrackOSObjects = true; + Chk->getSummaryManager().setTrackOSObjects(); + + if (getOption(Opts, "osx.OSObjectRetainCount", + "TrackUnannotatedOSOutParameters", "false")) { + Chk->getSummaryManager().setTrackUnannotatedOSOutParameters(); + } } Index: clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp +++ clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp @@ -182,6 +182,20 @@ return FName.contains_lower("MakeCollectable"); } +/// \return A Pointer to OSObject subclass record declaration, if {@code QT} +/// is a pointer to such object. NULL otherwise. +static const CXXRecordDecl* findPointerToOSObject(QualType QT) { + if (!QT.isNull()) { + if (QT->isPointerType()) { + QualType PT = QT->getPointeeType(); + if (const auto *RD = PT->getAsCXXRecordDecl()) + if (isOSObjectSubclass(RD)) + return RD; + } + } + return nullptr; +} + /// A function is OSObject related if it is declared on a subclass /// of OSObject, or any of the parameters is a subclass of an OSObject. static bool isOSObjectRelated(const CXXMethodDecl *MD) { @@ -189,42 +203,77 @@ return true; for (ParmVarDecl *Param : MD->parameters()) { - QualType PT = Param->getType()->getPointeeType(); - if (!PT.isNull()) - if (CXXRecordDecl *RD = PT->getAsCXXRecordDecl()) - if (isOSObjectSubclass(RD)) - return true; + QualType QT = Param->getType(); + if (findPointerToOSObject(QT) || + (QT->isPointerType() && findPointerToOSObject(QT->getPointeeType()))) + return true; } return false; } +static bool isOSObjectGetter(const NamedDecl *ND) { + if (const IdentifierInfo *II = ND->getIdentifier()) { + return II->getName().startswith("get") || + II->getName().startswith("Get"); + } + return false; +} + +/// \return Whether the chain of typedefs starting from {@code QT} +/// has a typedef with a given name {@code Name}. +static bool hasTypedefNamed(QualType QT, + StringRef Name) { + while (auto *T = dyn_cast(QT)) { + const auto &Context = T->getDecl()->getASTContext(); + if (T->getDecl()->getIdentifier() == &Context.Idents.get(Name)) + return true; + QT = T->getDecl()->getUnderlyingType(); + } + return false; +} + +static QualType getCallableReturnType(const NamedDecl *ND) { + if (const auto *FD = dyn_cast(ND)) { + return FD->getReturnType(); + } else if (const auto *MD = dyn_cast(ND)) { + return MD->getReturnType(); + } else { + llvm_unreachable("Unexpected decl"); + } +} + +static ArgEffectKind +getOSObjectRetainedOutParamEffectKind(const NamedDecl *ND, + bool HasRetainedOnZeroAnnotation, + bool HasRetainedOnNonZeroAnnotation) { + QualType QT = getCallableReturnType(ND); + + // The usual convention is to create an object on non-zero return, but + // it's reverted if the typedef chain has a typedef kern_return_t, + // because kReturnSuccess constant is defined as zero. + // The convention can be overwritten by custom attributes. + bool SuccessOnZero = + HasRetainedOnZeroAnnotation || + (hasTypedefNamed(QT, "kern_return_t") && !HasRetainedOnNonZeroAnnotation); + bool ShouldSplit = !QT.isNull() && !QT->isVoidType(); + ArgEffectKind AK = RetainedOutParameter; + + if (ShouldSplit && SuccessOnZero) { + AK = RetainedOutParameterOnZero; + } else if (ShouldSplit && + (!SuccessOnZero || HasRetainedOnNonZeroAnnotation)) { + AK = RetainedOutParameterOnNonZero; + } + return AK; +} + const RetainSummary * 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); - } - } - } - } - if (const auto *MD = dyn_cast(FD)) { const CXXRecordDecl *Parent = MD->getParent(); - if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) { + if (Parent && isOSObjectSubclass(Parent)) { if (FName == "release") return getOSSummaryReleaseRule(FD); @@ -239,6 +288,53 @@ } } + RetainSummary Out(AF); + bool SummaryFound = false; + + if (const auto *RD = findPointerToOSObject(RetTy)) { + 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 (!isOSObjectGetter(FD) || isOSIteratorSubclass(RD)) { + Out.setRetEffect(RetEffect::MakeOwned(ObjKind::OS)); + SummaryFound = true; + } else { + Out.setRetEffect(RetEffect::MakeNotOwned(ObjKind::OS)); + SummaryFound = true; + } + } + } + + if (TrackUnannotatedOSOutParameters) { + for (unsigned ParmIdx = 0; ParmIdx < FD->parameters().size(); ParmIdx++) { + const ParmVarDecl *PD = FD->parameters()[ParmIdx]; + QualType QT = PD->getType(); + + // Decayed type is used to differentiate between double pointers + // and arrays of pointers (although they have the same operational + // semantics, the intent differs). + if (QT.isNull() || !QT->isPointerType() || isa(QT)) + continue; + + const auto *RD = findPointerToOSObject(QT->getPointeeType()); + if (!RD) + continue; + + ArgEffectKind AE = + isOSObjectGetter(FD) + ? UnretainedOutParameter + : getOSObjectRetainedOutParamEffectKind(FD, false, false); + + Out.addArg(AF, ParmIdx, ArgEffect(AE, ObjKind::OS)); + SummaryFound = true; + } + } + + if (SummaryFound) + return getPersistentSummary(Out); return nullptr; } @@ -478,6 +574,8 @@ getSummaryForObjCOrCFObject(FD, FName, RetTy, FT, AllowAnnotations)) return S; + // Unless the OSObject tracking is enabled, AND the method is + // OSObject-related, all calls to CXX methods are treated as escaping. if (const auto *MD = dyn_cast(FD)) if (!(TrackOSObjects && isOSObjectRelated(MD))) return getPersistentSummary(RetEffect::MakeNoRet(), @@ -815,28 +913,6 @@ return None; } -/// \return Whether the chain of typedefs starting from {@code QT} -/// has a typedef with a given name {@code Name}. -static bool hasTypedefNamed(QualType QT, - StringRef Name) { - while (auto *T = dyn_cast(QT)) { - const auto &Context = T->getDecl()->getASTContext(); - if (T->getDecl()->getIdentifier() == &Context.Idents.get(Name)) - return true; - QT = T->getDecl()->getUnderlyingType(); - } - return false; -} - -static QualType getCallableReturnType(const NamedDecl *ND) { - if (const auto *FD = dyn_cast(ND)) { - return FD->getReturnType(); - } else if (const auto *MD = dyn_cast(ND)) { - return MD->getReturnType(); - } else { - llvm_unreachable("Unexpected decl"); - } -} bool RetainSummaryManager::applyParamAnnotationEffect( const ParmVarDecl *pd, unsigned parm_idx, const NamedDecl *FD, @@ -855,25 +931,10 @@ // For OSObjects, we try to guess whether the object is created based // on the return value. if (K == ObjKind::OS) { - QualType QT = getCallableReturnType(FD); - - bool HasRetainedOnZero = pd->hasAttr(); - bool HasRetainedOnNonZero = pd->hasAttr(); - - // The usual convention is to create an object on non-zero return, but - // it's reverted if the typedef chain has a typedef kern_return_t, - // because kReturnSuccess constant is defined as zero. - // The convention can be overwritten by custom attributes. - bool SuccessOnZero = - HasRetainedOnZero || - (hasTypedefNamed(QT, "kern_return_t") && !HasRetainedOnNonZero); - bool ShouldSplit = !QT.isNull() && !QT->isVoidType(); - ArgEffectKind AK = RetainedOutParameter; - if (ShouldSplit && SuccessOnZero) { - AK = RetainedOutParameterOnZero; - } else if (ShouldSplit && (!SuccessOnZero || HasRetainedOnNonZero)) { - AK = RetainedOutParameterOnNonZero; - } + ArgEffectKind AK = getOSObjectRetainedOutParamEffectKind( + FD, pd->hasAttr(), + pd->hasAttr()); + Template->addArg(AF, parm_idx, ArgEffect(AK, ObjKind::OS)); } @@ -1200,10 +1261,8 @@ CallEffects CallEffects::getEffect(const ObjCMethodDecl *MD) { ASTContext &Ctx = MD->getASTContext(); - LangOptions L = Ctx.getLangOpts(); - RetainSummaryManager M(Ctx, L.ObjCAutoRefCount, - /*TrackNSAndCFObjects=*/true, - /*TrackOSObjects=*/false); + RetainSummaryManager M(Ctx); + M.setTrackOSObjects(); const RetainSummary *S = M.getMethodSummary(MD); CallEffects CE(S->getRetEffect(), S->getReceiverEffect()); unsigned N = MD->param_size(); @@ -1215,10 +1274,8 @@ CallEffects CallEffects::getEffect(const FunctionDecl *FD) { ASTContext &Ctx = FD->getASTContext(); - LangOptions L = Ctx.getLangOpts(); - RetainSummaryManager M(Ctx, L.ObjCAutoRefCount, - /*TrackNSAndCFObjects=*/true, - /*TrackOSObjects=*/false); + RetainSummaryManager M(Ctx); + M.setTrackObjCAndCFObjects(); const RetainSummary *S = M.getFunctionSummary(FD); CallEffects CE(S->getRetEffect()); unsigned N = FD->param_size(); Index: clang/test/Analysis/osobject-retain-release.cpp =================================================================== --- clang/test/Analysis/osobject-retain-release.cpp +++ clang/test/Analysis/osobject-retain-release.cpp @@ -1,5 +1,7 @@ // RUN: %clang_analyze_cc1 -fblocks -analyze -analyzer-output=text\ // RUN: -analyzer-checker=core,osx -verify %s +// RUN: %clang_analyze_cc1 -fblocks -analyze -analyzer-output=text\ +// RUN: -analyzer-checker=core,osx -verify -DDEFAULT_OUTPARAMS=true -analyzer-config "osx.cocoa.RetainCount:TrackUnannotatedOSOutParameters=true" %s struct OSMetaClass; @@ -263,6 +265,20 @@ } // expected-warning{{Potential leak of an object stored into 'obj'}} // expected-note@-1{{Object leaked: object allocated and stored into 'obj' is not referenced later in this execution path and has a retain count of +1}} +void unannotated_out_param(OSObject **obj); + +void use_unannotated_out_param() { + OSObject *obj; + unannotated_out_param(&obj); +#ifdef DEFAULT_OUTPARAMS + // expected-note@-2{{Call to function 'unannotated_out_param' writes an OSObject of type 'OSObject' with a +1 retain count into an out parameter 'obj'}} +#endif +} +#ifdef DEFAULT_OUTPARAMS + // expected-warning@-2{{Potential leak of an object stored into 'obj'}} + // expected-note@-3{{Object leaked: object allocated and stored into 'obj' is not referenced later in this execution path and has a retain count of +1}} +#endif + struct StructWithField { OSObject *obj;