Index: clang/include/clang/Analysis/ObjCRetainCount.h =================================================================== --- clang/include/clang/Analysis/ObjCRetainCount.h +++ clang/include/clang/Analysis/ObjCRetainCount.h @@ -141,6 +141,10 @@ /// Indicates that the tracked object is a CF object. This is /// important between GC and non-GC code. CF, + + // Indicates that the tracked object is an OS Object. + OS, + /// Indicates that the tracked object is an Objective-C object. ObjC, /// Indicates that the tracked object could be a CF or Objective-C object. Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -717,6 +717,10 @@ return 0; } + SVal getThisSVal() const { + return getSVal(getCXXThisExpr()); + } + const Expr *getArgExpr(unsigned Index) const override { return getOriginExpr()->getArg(Index); } Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -100,7 +100,7 @@ /// The kind of object being tracked (CF or ObjC), if known. /// /// See the RetEffect::ObjKind enum for possible values. - unsigned RawObjectKind : 2; + unsigned RawObjectKind : 3; /// True if the current state and/or retain count may turn out to not be the /// best possible approximation of the reference counting state. Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -469,11 +469,10 @@ } } - // Evaluate the effect on the message receiver. + // Evaluate the effect on the message receiver / `this` argument. bool ReceiverIsTracked = false; if (!hasErr) { - const ObjCMethodCall *MsgInvocation = dyn_cast(&CallOrMsg); - if (MsgInvocation) { + if (const auto *MsgInvocation = dyn_cast(&CallOrMsg)) { if (SymbolRef Sym = MsgInvocation->getReceiverSVal().getAsLocSymbol()) { if (const RefVal *T = getRefBinding(state, Sym)) { ReceiverIsTracked = true; @@ -485,6 +484,15 @@ } } } + } else if (const auto *MCall = dyn_cast(&CallOrMsg)) { + // TODO: code duplication with branch above? + if (SymbolRef Sym = MCall->getThisSVal().getAsLocSymbol()) { + if (const RefVal *T = getRefBinding(state, Sym)) { + state = updateSymbol(state, Sym, *T, Summ.getThisEffect(), + hasErr, C); + + } + } } } @@ -993,7 +1001,7 @@ // does not understand. ProgramStateRef state = C.getState(); - if (Optional regionLoc = loc.getAs()) { + if (auto regionLoc = loc.getAs()) { escapes = !regionLoc->getRegion()->hasStackStorage(); if (!escapes) { @@ -1017,7 +1025,7 @@ // If we are storing the value into an auto function scope variable annotated // with (__attribute__((cleanup))), stop tracking the value to avoid leak // false positives. - if (const VarRegion *LVR = dyn_cast_or_null(loc.getAsRegion())) { + if (const auto *LVR = dyn_cast_or_null(loc.getAsRegion())) { const VarDecl *VD = LVR->getDecl(); if (VD->hasAttr()) { escapes = true; @@ -1036,9 +1044,11 @@ C.addTransition(state); } +// TODO: this should not be necesssary, as symbols turning into 0 +// should die anyway. ProgramStateRef RetainCountChecker::evalAssume(ProgramStateRef state, - SVal Cond, - bool Assumption) const { + SVal Cond, + bool Assumption) const { // FIXME: We may add to the interface of evalAssume the list of symbols // whose assumptions have changed. For now we just iterate through the // bindings and check if any of the tracked symbols are NULL. This isn't Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp @@ -120,6 +120,9 @@ if (CurrV.getObjKind() == RetEffect::CF) { os << " returns a Core Foundation object of type " << Sym->getType().getAsString() << " with a "; + } else if (CurrV.getObjKind() == RetEffect::OS) { + os << "returns an OS object of type " + << Sym->getType().getAsString() << " with a "; } else if (CurrV.getObjKind() == RetEffect::Generalized) { os << " returns an object of type " << Sym->getType().getAsString() << " with a "; Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h @@ -137,14 +137,21 @@ /// this is the effect applied to the state of the receiver. ArgEffect Receiver; + /// Effect on "this" pointer - applicable only to C++ method calls. + ArgEffect This; + /// Ret - The effect on the return value. Used to indicate if the /// function/method call returns a new tracked symbol. RetEffect Ret; public: - RetainSummary(ArgEffects A, RetEffect R, ArgEffect defaultEff, - ArgEffect ReceiverEff) - : Args(A), DefaultArgEffect(defaultEff), Receiver(ReceiverEff), Ret(R) {} + RetainSummary(ArgEffects A, + RetEffect R, + ArgEffect defaultEff, + ArgEffect ReceiverEff, + ArgEffect ThisEff) + : Args(A), DefaultArgEffect(defaultEff), Receiver(ReceiverEff), + This(ThisEff), Ret(R) {} /// getArg - Return the argument effect on the argument specified by /// idx (starting from 0). @@ -178,12 +185,16 @@ /// This is only meaningful if the summary applies to an ObjCMessageExpr*. ArgEffect getReceiverEffect() const { return Receiver; } + ArgEffect getThisEffect() const { return This; } + /// Test if two retain summaries are identical. Note that merely equivalent /// summaries are not necessarily identical (for example, if an explicit /// argument effect matches the default effect). bool operator==(const RetainSummary &Other) const { return Args == Other.Args && DefaultArgEffect == Other.DefaultArgEffect && - Receiver == Other.Receiver && Ret == Other.Ret; + Receiver == Other.Receiver && + This == Other.This && + Ret == Other.Ret; } /// Profile this summary for inclusion in a FoldingSet. @@ -191,6 +202,7 @@ ID.Add(Args); ID.Add(DefaultArgEffect); ID.Add(Receiver); + ID.Add(This); ID.Add(Ret); } @@ -315,21 +327,27 @@ /// data in ScratchArgs. ArgEffects getArgEffects(); + const RetainSummary *getOSSummaryCreateRule(const FunctionDecl *FD); + const RetainSummary *getOSSummaryRetainRule(const FunctionDecl *FD); + const RetainSummary *getOSSummaryReleaseRule(const FunctionDecl *FD); + const RetainSummary *getCFSummaryGetRule(const FunctionDecl *FD); + enum UnaryFuncKind { cfretain, cfrelease, cfautorelease, cfmakecollectable }; const RetainSummary *getUnarySummary(const FunctionType* FT, UnaryFuncKind func); const RetainSummary *getCFSummaryCreateRule(const FunctionDecl *FD); - const RetainSummary *getCFSummaryGetRule(const FunctionDecl *FD); const RetainSummary *getCFCreateGetRuleSummary(const FunctionDecl *FD); const RetainSummary *getPersistentSummary(const RetainSummary &OldSumm); const RetainSummary *getPersistentSummary(RetEffect RetEff, ArgEffect ReceiverEff = DoNothing, - ArgEffect DefaultEff = MayEscape) { - RetainSummary Summ(getArgEffects(), RetEff, DefaultEff, ReceiverEff); + ArgEffect DefaultEff = MayEscape, + ArgEffect ThisEff = DoNothing) { + RetainSummary Summ(getArgEffects(), RetEff, DefaultEff, ReceiverEff, + ThisEff); return getPersistentSummary(Summ); } Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp @@ -57,6 +57,31 @@ return Summ; } +static bool isOSObjectSubclass(QualType T); + +static bool isOSObjectSubclass(const CXXRecordDecl *RD) { + if (RD->getDeclName().getAsString() == "OSObject") + return true; + + const CXXRecordDecl *RDD = RD->getDefinition(); + if (!RDD) + return false; + + for (const CXXBaseSpecifier Spec : RDD->bases()) { + if (isOSObjectSubclass(Spec.getType())) + return true; + } + return false; +} + +/// \return Whether type represents an OSObject successor. +static bool isOSObjectSubclass(QualType T) { + if (const auto *RD = T->getAsCXXRecordDecl()) { + return isOSObjectSubclass(RD); + } + return false; +} + static bool hasRCAnnotation(const Decl *D, StringRef rcAnnotation) { for (const auto *Ann : D->specific_attrs()) { if (Ann->getAnnotation() == rcAnnotation) @@ -111,6 +136,7 @@ // filters. assert(ScratchArgs.isEmpty()); + // TODO: do those cases do anything at all since we have IPA already? if (FName == "pthread_create" || FName == "pthread_setspecific") { // Part of: and . // This will be addressed better with IPA. @@ -200,6 +226,9 @@ } if (RetTy->isPointerType()) { + if (isOSObjectSubclass(RetTy->getPointeeType())) + return getOSSummaryCreateRule(FD); + // For CoreFoundation ('CF') types. if (cocoa::isRefType(RetTy, "CF", FName)) { if (isRetain(FD, FName)) { @@ -245,6 +274,17 @@ } } + if (const auto *MD = dyn_cast(FD)) { + const CXXRecordDecl *Parent = MD->getParent(); + if (isOSObjectSubclass(Parent)) { + if (isRelease(FD, FName)) + return getOSSummaryReleaseRule(FD); + + if (isRetain(FD, FName)) + return getOSSummaryRetainRule(FD); + } + } + // 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' && @@ -415,6 +455,8 @@ Summ = getFunctionSummary(cast(Call).getDecl()); break; case CE_CXXMember: + Summ = getFunctionSummary(cast(Call).getDecl()); + break; case CE_CXXMemberOperator: case CE_Block: case CE_CXXConstructor: @@ -517,6 +559,28 @@ return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); } +const RetainSummary * +RetainSummaryManager::getOSSummaryRetainRule(const FunctionDecl *FD) { + return getPersistentSummary(RetEffect::MakeNoRet(), + /*ReceiverEff=*/DoNothing, + /*DefaultEff=*/DoNothing, + /*ThisEff=*/IncRef); +} + +const RetainSummary * +RetainSummaryManager::getOSSummaryReleaseRule(const FunctionDecl *FD) { + return getPersistentSummary(RetEffect::MakeNoRet(), + /*ReceiverEff=*/DoNothing, + /*DefaultEff=*/DoNothing, + /*ThisEff=*/DecRef); +} + +const RetainSummary * +RetainSummaryManager::getOSSummaryCreateRule(const FunctionDecl *FD) { + // TODO: is it ever not owned? need a check similar to CF objects. + return getPersistentSummary(RetEffect::MakeOwned(RetEffect::OS)); +} + const RetainSummary * RetainSummaryManager::getCFSummaryCreateRule(const FunctionDecl *FD) { assert (ScratchArgs.isEmpty());