Index: clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h +++ clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h @@ -508,9 +508,6 @@ /// AF - A factory for ArgEffects objects. ArgEffects::Factory AF; - /// ScratchArgs - A holding buffer for construct ArgEffects. - ArgEffects ScratchArgs; - /// ObjCAllocRetE - Default return effect for methods returning Objective-C /// objects. RetEffect ObjCAllocRetE; @@ -523,10 +520,6 @@ /// effects. llvm::FoldingSet SimpleSummaries; - /// getArgEffects - Returns a persistent ArgEffects object based on the - /// data in ScratchArgs. - ArgEffects getArgEffects(); - /// Create an OS object at +1. const RetainSummary *getOSSummaryCreateRule(const FunctionDecl *FD); @@ -554,25 +547,30 @@ const RetainSummary *getPersistentSummary(const RetainSummary &OldSumm); const RetainSummary *getPersistentSummary(RetEffect RetEff, + ArgEffects ScratchArgs, ArgEffect ReceiverEff = DoNothing, ArgEffect DefaultEff = MayEscape, ArgEffect ThisEff = DoNothing) { - RetainSummary Summ(getArgEffects(), RetEff, DefaultEff, ReceiverEff, + RetainSummary Summ(ScratchArgs, RetEff, DefaultEff, ReceiverEff, ThisEff); return getPersistentSummary(Summ); } const RetainSummary *getDoNothingSummary() { - return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + return getPersistentSummary(RetEffect::MakeNoRet(), + ArgEffects(AF.getEmptyMap()), + DoNothing, DoNothing); } const RetainSummary *getDefaultSummary() { return getPersistentSummary(RetEffect::MakeNoRet(), + ArgEffects(AF.getEmptyMap()), DoNothing, MayEscape); } const RetainSummary *getPersistentStopSummary() { return getPersistentSummary(RetEffect::MakeNoRet(), + ArgEffects(AF.getEmptyMap()), StopTracking, StopTracking); } @@ -649,7 +647,6 @@ bool applyFunctionParamAnnotationEffect(const ParmVarDecl *pd, unsigned parm_idx, const FunctionDecl *FD, - ArgEffects::Factory &AF, RetainSummaryTemplate &Template); public: @@ -661,7 +658,7 @@ ARCEnabled(usesARC), TrackObjCAndCFObjects(trackObjCAndCFObjects), TrackOSObjects(trackOSObjects), - AF(BPAlloc), ScratchArgs(AF.getEmptyMap()), + AF(BPAlloc), ObjCAllocRetE(usesARC ? RetEffect::MakeNotOwned(RetEffect::ObjC) : RetEffect::MakeOwned(RetEffect::ObjC)), ObjCInitRetE(usesARC ? RetEffect::MakeNotOwned(RetEffect::ObjC) Index: clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp +++ clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp @@ -49,12 +49,6 @@ llvm_unreachable("Unexpected attribute passed"); } -ArgEffects RetainSummaryManager::getArgEffects() { - ArgEffects AE = ScratchArgs; - ScratchArgs = AF.getEmptyMap(); - return AE; -} - const RetainSummary * RetainSummaryManager::getPersistentSummary(const RetainSummary &OldSumm) { // Unique "simple" summaries -- those without ArgEffects. @@ -190,11 +184,9 @@ const FunctionType *FT, bool &AllowAnnotations) { - std::string RetTyName = RetTy.getAsString(); + ArgEffects ScratchArgs(AF.getEmptyMap()); - // FIXME: This should all be refactored into a chain of "summary lookup" - // filters. - assert(ScratchArgs.isEmpty()); + std::string RetTyName = RetTy.getAsString(); if (FName == "pthread_create" || FName == "pthread_setspecific") { // Part of: and . // This will be addressed better with IPA. @@ -207,10 +199,12 @@ } else if (FName == "CMBufferQueueDequeueAndRetain" || FName == "CMBufferQueueDequeueIfDataReadyAndRetain") { // Part of: . - return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF), DoNothing, + return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF), + ScratchArgs, + DoNothing, DoNothing); } else if (FName == "CFPlugInInstanceCreate") { - return getPersistentSummary(RetEffect::MakeNoRet()); + return getPersistentSummary(RetEffect::MakeNoRet(), ScratchArgs); } else if (FName == "IORegistryEntrySearchCFProperty" || (RetTyName == "CFMutableDictionaryRef" && (FName == "IOBSDNameMatching" || FName == "IOServiceMatching" || @@ -219,21 +213,25 @@ FName == "IOOpenFirmwarePathMatching"))) { // Part of . (IOKit) // This should be addressed using a API table. - return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF), DoNothing, - DoNothing); + return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF), + ScratchArgs, DoNothing, DoNothing); } else if (FName == "IOServiceGetMatchingService" || FName == "IOServiceGetMatchingServices") { // FIXES: // This should be addressed using a API table. This strcmp is also // a little gross, but there is no need to super optimize here. ScratchArgs = AF.add(ScratchArgs, 1, DecRef); - return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + return getPersistentSummary(RetEffect::MakeNoRet(), + ScratchArgs, + DoNothing, DoNothing); } else if (FName == "IOServiceAddNotification" || FName == "IOServiceAddMatchingNotification") { // Part of . (IOKit) // This should be addressed using a API table. ScratchArgs = AF.add(ScratchArgs, 2, DecRef); - return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + return getPersistentSummary(RetEffect::MakeNoRet(), + ScratchArgs, + DoNothing, DoNothing); } else if (FName == "CVPixelBufferCreateWithBytes") { // FIXES: // Eventually this can be improved by recognizing that the pixel @@ -242,15 +240,17 @@ // FIXME: This function has an out parameter that returns an // allocated object. ScratchArgs = AF.add(ScratchArgs, 7, StopTracking); - return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + return getPersistentSummary(RetEffect::MakeNoRet(), + ScratchArgs, + DoNothing, DoNothing); } else if (FName == "CGBitmapContextCreateWithData") { // FIXES: // Eventually this can be improved by recognizing that 'releaseInfo' // passed to CGBitmapContextCreateWithData is released via // a callback and doing full IPA to make sure this is done correctly. ScratchArgs = AF.add(ScratchArgs, 8, StopTracking); - return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF), DoNothing, - DoNothing); + return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF), + ScratchArgs, DoNothing, DoNothing); } else if (FName == "CVPixelBufferCreateWithPlanarBytes") { // FIXES: // Eventually this can be improved by recognizing that the pixel @@ -258,7 +258,9 @@ // via a callback and doing full IPA to make sure this is done // correctly. ScratchArgs = AF.add(ScratchArgs, 12, StopTracking); - return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + return getPersistentSummary(RetEffect::MakeNoRet(), + ScratchArgs, + DoNothing, DoNothing); } else if (FName == "VTCompressionSessionEncodeFrame") { // The context argument passed to VTCompressionSessionEncodeFrame() // is passed to the callback specified when creating the session @@ -266,7 +268,9 @@ // To account for this possibility, conservatively stop tracking // the context. ScratchArgs = AF.add(ScratchArgs, 5, StopTracking); - return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + return getPersistentSummary(RetEffect::MakeNoRet(), + ScratchArgs, + DoNothing, DoNothing); } else if (FName == "dispatch_set_context" || FName == "xpc_connection_set_context") { // - The analyzer currently doesn't have @@ -276,7 +280,9 @@ // FIXME: this hack should possibly go away once we can handle // libdispatch and XPC finalizers. ScratchArgs = AF.add(ScratchArgs, 1, StopTracking); - return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + return getPersistentSummary(RetEffect::MakeNoRet(), + ScratchArgs, + DoNothing, DoNothing); } else if (FName.startswith("NSLog")) { return getDoNothingSummary(); } else if (FName.startswith("NS") && @@ -285,7 +291,8 @@ // be deallocated by NSMapRemove. (radar://11152419) ScratchArgs = AF.add(ScratchArgs, 1, StopTracking); ScratchArgs = AF.add(ScratchArgs, 2, StopTracking); - return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + return getPersistentSummary(RetEffect::MakeNoRet(), + ScratchArgs, DoNothing, DoNothing); } if (RetTy->isPointerType()) { @@ -368,7 +375,8 @@ ? MayEscape : DoNothing; - return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, E); + return getPersistentSummary(RetEffect::MakeNoRet(), ScratchArgs, + DoNothing, E); } } @@ -405,7 +413,9 @@ if (const auto *MD = dyn_cast(FD)) if (!(TrackOSObjects && isOSObjectRelated(MD))) - return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, StopTracking, + return getPersistentSummary(RetEffect::MakeNoRet(), + ArgEffects(AF.getEmptyMap()), + DoNothing, StopTracking, DoNothing); return getDefaultSummary(); @@ -473,6 +483,7 @@ ArgEffect DefEffect = getStopTrackingHardEquivalent(S->getDefaultArgEffect()); + ArgEffects ScratchArgs(AF.getEmptyMap()); ArgEffects CustomArgEffects = S->getArgEffects(); for (ArgEffects::iterator I = CustomArgEffects.begin(), E = CustomArgEffects.end(); @@ -499,7 +510,7 @@ } } - S = getPersistentSummary(RE, RecEffect, DefEffect); + S = getPersistentSummary(RE, ScratchArgs, RecEffect, DefEffect); } // Special case '[super init];' and '[self init];' @@ -639,14 +650,15 @@ RetainSummaryManager::getUnarySummary(const FunctionType* FT, UnaryFuncKind func) { + // Unary functions have no arg effects by definition. + ArgEffects ScratchArgs(AF.getEmptyMap()); + // Sanity check that this is *really* a unary function. This can // happen if people do weird things. const FunctionProtoType* FTP = dyn_cast(FT); if (!FTP || FTP->getNumParams() != 1) return getPersistentStopSummary(); - assert (ScratchArgs.isEmpty()); - ArgEffect Effect; switch (func) { case cfretain: Effect = IncRef; break; @@ -656,12 +668,15 @@ } ScratchArgs = AF.add(ScratchArgs, 0, Effect); - return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + return getPersistentSummary(RetEffect::MakeNoRet(), + ScratchArgs, + DoNothing, DoNothing); } const RetainSummary * RetainSummaryManager::getOSSummaryRetainRule(const FunctionDecl *FD) { return getPersistentSummary(RetEffect::MakeNoRet(), + AF.getEmptyMap(), /*ReceiverEff=*/DoNothing, /*DefaultEff=*/DoNothing, /*ThisEff=*/IncRef); @@ -670,6 +685,7 @@ const RetainSummary * RetainSummaryManager::getOSSummaryReleaseRule(const FunctionDecl *FD) { return getPersistentSummary(RetEffect::MakeNoRet(), + AF.getEmptyMap(), /*ReceiverEff=*/DoNothing, /*DefaultEff=*/DoNothing, /*ThisEff=*/DecRef); @@ -678,6 +694,7 @@ const RetainSummary * RetainSummaryManager::getOSSummaryFreeRule(const FunctionDecl *FD) { return getPersistentSummary(RetEffect::MakeNoRet(), + AF.getEmptyMap(), /*ReceiverEff=*/DoNothing, /*DefaultEff=*/DoNothing, /*ThisEff=*/Dealloc); @@ -685,25 +702,26 @@ const RetainSummary * RetainSummaryManager::getOSSummaryCreateRule(const FunctionDecl *FD) { - return getPersistentSummary(RetEffect::MakeOwned(RetEffect::OS)); + return getPersistentSummary(RetEffect::MakeOwned(RetEffect::OS), + AF.getEmptyMap()); } const RetainSummary * RetainSummaryManager::getOSSummaryGetRule(const FunctionDecl *FD) { - return getPersistentSummary(RetEffect::MakeNotOwned(RetEffect::OS)); + return getPersistentSummary(RetEffect::MakeNotOwned(RetEffect::OS), + AF.getEmptyMap()); } const RetainSummary * RetainSummaryManager::getCFSummaryCreateRule(const FunctionDecl *FD) { - assert (ScratchArgs.isEmpty()); - - return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF)); + return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF), + ArgEffects(AF.getEmptyMap())); } const RetainSummary * RetainSummaryManager::getCFSummaryGetRule(const FunctionDecl *FD) { - assert (ScratchArgs.isEmpty()); return getPersistentSummary(RetEffect::MakeNotOwned(RetEffect::CF), + ArgEffects(AF.getEmptyMap()), DoNothing, DoNothing); } @@ -753,11 +771,9 @@ return None; } -bool RetainSummaryManager::applyFunctionParamAnnotationEffect(const ParmVarDecl *pd, - unsigned parm_idx, - const FunctionDecl *FD, - ArgEffects::Factory &AF, - RetainSummaryTemplate &Template) { +bool RetainSummaryManager::applyFunctionParamAnnotationEffect( + const ParmVarDecl *pd, unsigned parm_idx, const FunctionDecl *FD, + RetainSummaryTemplate &Template) { if (hasEnabledAttr(pd)) { Template->addArg(AF, parm_idx, DecRefMsg); return true; @@ -787,7 +803,7 @@ if (const auto *MD = dyn_cast(FD)) { for (const auto *OD : MD->overridden_methods()) { const ParmVarDecl *OP = OD->parameters()[parm_idx]; - if (applyFunctionParamAnnotationEffect(OP, parm_idx, OD, AF, Template)) + if (applyFunctionParamAnnotationEffect(OP, parm_idx, OD, Template)) return true; } } @@ -810,7 +826,7 @@ for (auto pi = FD->param_begin(), pe = FD->param_end(); pi != pe; ++pi, ++parm_idx) { const ParmVarDecl *pd = *pi; - applyFunctionParamAnnotationEffect(pd, parm_idx, FD, AF, Template); + applyFunctionParamAnnotationEffect(pd, parm_idx, FD, Template); } QualType RetTy = FD->getReturnType(); @@ -950,15 +966,17 @@ } } - if (ScratchArgs.isEmpty() && ReceiverEff == DoNothing && + if (ReceiverEff == DoNothing && ResultEff.getKind() == RetEffect::NoRet) return getDefaultSummary(); - return getPersistentSummary(ResultEff, ReceiverEff, MayEscape); + return getPersistentSummary(ResultEff, ArgEffects(AF.getEmptyMap()), + ReceiverEff, MayEscape); } const RetainSummary *RetainSummaryManager::getInstanceMethodSummary( - const ObjCMethodCall &Msg, QualType ReceiverType) { + const ObjCMethodCall &Msg, + QualType ReceiverType) { const ObjCInterfaceDecl *ReceiverClass = nullptr; // We do better tracking of the type of the object than the core ExprEngine. @@ -985,7 +1003,8 @@ } const RetainSummary * -RetainSummaryManager::getMethodSummary(Selector S, const ObjCInterfaceDecl *ID, +RetainSummaryManager::getMethodSummary(Selector S, + const ObjCInterfaceDecl *ID, const ObjCMethodDecl *MD, QualType RetTy, ObjCMethodSummariesTy &CachedSummaries) { @@ -1010,25 +1029,29 @@ } void RetainSummaryManager::InitializeClassMethodSummaries() { - assert(ScratchArgs.isEmpty()); + ArgEffects ScratchArgs = AF.getEmptyMap(); + // Create the [NSAssertionHandler currentHander] summary. addClassMethSummary("NSAssertionHandler", "currentHandler", - getPersistentSummary(RetEffect::MakeNotOwned(RetEffect::ObjC))); + getPersistentSummary(RetEffect::MakeNotOwned(RetEffect::ObjC), + ScratchArgs)); // Create the [NSAutoreleasePool addObject:] summary. ScratchArgs = AF.add(ScratchArgs, 0, Autorelease); addClassMethSummary("NSAutoreleasePool", "addObject", getPersistentSummary(RetEffect::MakeNoRet(), + ScratchArgs, DoNothing, Autorelease)); } void RetainSummaryManager::InitializeMethodSummaries() { - assert (ScratchArgs.isEmpty()); - + ArgEffects ScratchArgs = AF.getEmptyMap(); // Create the "init" selector. It just acts as a pass-through for the // receiver. - const RetainSummary *InitSumm = getPersistentSummary(ObjCInitRetE, DecRefMsg); + const RetainSummary *InitSumm = getPersistentSummary(ObjCInitRetE, + ScratchArgs, + DecRefMsg); addNSObjectMethSummary(GetNullarySelector("init", Ctx), InitSumm); // awakeAfterUsingCoder: behaves basically like an 'init' method. It @@ -1037,25 +1060,26 @@ InitSumm); // The next methods are allocators. - const RetainSummary *AllocSumm = getPersistentSummary(ObjCAllocRetE); + const RetainSummary *AllocSumm = getPersistentSummary(ObjCAllocRetE, + ScratchArgs); const RetainSummary *CFAllocSumm = - getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF)); + getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF), ScratchArgs); // Create the "retain" selector. RetEffect NoRet = RetEffect::MakeNoRet(); - const RetainSummary *Summ = getPersistentSummary(NoRet, IncRefMsg); + const RetainSummary *Summ = getPersistentSummary(NoRet, ScratchArgs, IncRefMsg); addNSObjectMethSummary(GetNullarySelector("retain", Ctx), Summ); // Create the "release" selector. - Summ = getPersistentSummary(NoRet, DecRefMsg); + Summ = getPersistentSummary(NoRet, ScratchArgs, DecRefMsg); addNSObjectMethSummary(GetNullarySelector("release", Ctx), Summ); // Create the -dealloc summary. - Summ = getPersistentSummary(NoRet, Dealloc); + Summ = getPersistentSummary(NoRet, ScratchArgs, Dealloc); addNSObjectMethSummary(GetNullarySelector("dealloc", Ctx), Summ); // Create the "autorelease" selector. - Summ = getPersistentSummary(NoRet, Autorelease); + Summ = getPersistentSummary(NoRet, ScratchArgs, Autorelease); addNSObjectMethSummary(GetNullarySelector("autorelease", Ctx), Summ); // For NSWindow, allocated objects are (initially) self-owned. @@ -1064,9 +1088,8 @@ // Thus, we need to track an NSWindow's display status. // This is tracked in . // See also http://llvm.org/bugs/show_bug.cgi?id=3714. - const RetainSummary *NoTrackYet = getPersistentSummary(RetEffect::MakeNoRet(), - StopTracking, - StopTracking); + const RetainSummary *NoTrackYet = getPersistentSummary( + RetEffect::MakeNoRet(), ScratchArgs, StopTracking, StopTracking); addClassMethSummary("NSWindow", "alloc", NoTrackYet);