From 343a38646a56b7167a779489ac5f01875609ef02 Mon Sep 17 00:00:00 2001 From: George Karpenkov Date: Thu, 10 Jan 2019 18:14:51 +0000 Subject: [PATCH] [analyzer] [RetainCountChecker] [NFC] Refactor the way attributes are handled Make sure all checks for attributes go through a centralized function, which checks whether attribute handling is enabled, and performs validation. The type of the attribute is returned. Sadly, metaprogramming is required as attributes have no sensible static getters. Differential Revision: https://reviews.llvm.org/D56222 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@350862 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/RetainSummaryManager.h | 18 +- .../Core/RetainSummaryManager.cpp | 186 ++++++++++-------- 2 files changed, 113 insertions(+), 91 deletions(-) diff --git a/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h b/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h index 809789c231..65648e44d4 100644 --- a/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h +++ b/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h @@ -741,16 +741,18 @@ public: RetEffect getObjAllocRetEffect() const { return ObjCAllocRetE; } - /// \return True if the declaration has an attribute {@code T}, - /// AND we are tracking that attribute. False otherwise. + /// Determine whether a declaration {@code D} of correspondent type (return + /// type for functions/methods) {@code QT} has any of the given attributes, + /// provided they pass necessary validation checks AND tracking the given + /// attribute is enabled. + /// Returns the object kind corresponding to the present attribute, or None, + /// if none of the specified attributes are present. + /// Crashes if passed an attribute which is not explicitly handled. template - bool hasEnabledAttr(const Decl *D) { - return isAttrEnabled() && D->hasAttr(); - } + Optional hasAnyEnabledAttrOf(const Decl *D, QualType QT); - /// Check whether we are tracking properties specified by the attributes. - template - bool isAttrEnabled(); + template + Optional hasAnyEnabledAttrOf(const Decl *D, QualType QT); friend class RetainSummaryTemplate; }; diff --git a/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp b/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp index 51fa760ab5..a0fe2fefde 100644 --- a/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp +++ b/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp @@ -36,17 +36,79 @@ constexpr static bool isOneOf() { return std::is_same::value || isOneOf(); } -template bool RetainSummaryManager::isAttrEnabled() { +namespace { + +struct GeneralizedReturnsRetainedAttr { + static bool classof(const Attr *A) { + if (auto AA = dyn_cast(A)) + return AA->getAnnotation() == "rc_ownership_returns_retained"; + return false; + } +}; + +struct GeneralizedReturnsNotRetainedAttr { + static bool classof(const Attr *A) { + if (auto AA = dyn_cast(A)) + return AA->getAnnotation() == "rc_ownership_returns_not_retained"; + return false; + } +}; + +struct GeneralizedConsumedAttr { + static bool classof(const Attr *A) { + if (auto AA = dyn_cast(A)) + return AA->getAnnotation() == "rc_ownership_consumed"; + return false; + } +}; + +} + +template +Optional RetainSummaryManager::hasAnyEnabledAttrOf(const Decl *D, + QualType QT) { + ObjKind K; if (isOneOf()) { - return TrackObjCAndCFObjects; + CFReturnsNotRetainedAttr>()) { + if (!TrackObjCAndCFObjects) + return None; + + K = ObjKind::CF; + } else if (isOneOf()) { + + if (!TrackObjCAndCFObjects) + return None; + + if (isOneOf() && + !cocoa::isCocoaObjectRef(QT)) + return None; + K = ObjKind::ObjC; } else if (isOneOf()) { - return TrackOSObjects; + if (!TrackOSObjects) + return None; + K = ObjKind::OS; + } else if (isOneOf()) { + K = ObjKind::Generalized; + } else { + llvm_unreachable("Unexpected attribute"); } - llvm_unreachable("Unexpected attribute passed"); + if (D->hasAttr()) + return K; + return None; +} + +template +Optional RetainSummaryManager::hasAnyEnabledAttrOf(const Decl *D, + QualType QT) { + if (auto Out = hasAnyEnabledAttrOf(D, QT)) + return Out; + return hasAnyEnabledAttrOf(D, QT); } const RetainSummary * @@ -727,33 +789,18 @@ RetainSummaryManager::getCFSummaryGetRule(const FunctionDecl *FD) { Optional RetainSummaryManager::getRetEffectFromAnnotations(QualType RetTy, const Decl *D) { - if (TrackObjCAndCFObjects && cocoa::isCocoaObjectRef(RetTy)) { - if (D->hasAttr()) - return ObjCAllocRetE; - - if (D->hasAttr() || - D->hasAttr()) - return RetEffect::MakeNotOwned(ObjKind::ObjC); - - } else if (!RetTy->isPointerType()) { - return None; - } + if (hasAnyEnabledAttrOf(D, RetTy)) + return ObjCAllocRetE; - if (hasEnabledAttr(D)) { - return RetEffect::MakeOwned(ObjKind::CF); - } else if (hasEnabledAttr(D)) { - return RetEffect::MakeOwned(ObjKind::OS); - } else if (hasRCAnnotation(D, "rc_ownership_returns_retained")) { - return RetEffect::MakeOwned(ObjKind::Generalized); - } + if (auto K = hasAnyEnabledAttrOf(D, RetTy)) + return RetEffect::MakeOwned(*K); - if (hasEnabledAttr(D)) { - return RetEffect::MakeNotOwned(ObjKind::CF); - } else if (hasEnabledAttr(D)) { - return RetEffect::MakeNotOwned(ObjKind::OS); - } else if (hasRCAnnotation(D, "rc_ownership_returns_not_retained")) { - return RetEffect::MakeNotOwned(ObjKind::Generalized); - } + if (auto K = hasAnyEnabledAttrOf< + CFReturnsNotRetainedAttr, OSReturnsNotRetainedAttr, + GeneralizedReturnsNotRetainedAttr, NSReturnsNotRetainedAttr, + NSReturnsAutoreleasedAttr>(D, RetTy)) + return RetEffect::MakeNotOwned(*K); if (const auto *MD = dyn_cast(D)) for (const auto *PD : MD->overridden_methods()) @@ -766,37 +813,20 @@ RetainSummaryManager::getRetEffectFromAnnotations(QualType RetTy, bool RetainSummaryManager::applyFunctionParamAnnotationEffect( const ParmVarDecl *pd, unsigned parm_idx, const FunctionDecl *FD, RetainSummaryTemplate &Template) { - if (hasEnabledAttr(pd)) { - Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::ObjC)); + QualType QT = pd->getType(); + if (auto K = + hasAnyEnabledAttrOf(pd, QT)) { + Template->addArg(AF, parm_idx, ArgEffect(DecRef, *K)); return true; - } else if (hasEnabledAttr(pd)) { - Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::CF)); + } else if (auto K = + hasAnyEnabledAttrOf(pd, QT)) { + Template->addArg(AF, parm_idx, ArgEffect(RetainedOutParameter, *K)); return true; - } else if (hasEnabledAttr(pd)) { - Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::OS)); + } else if (auto K = hasAnyEnabledAttrOf(pd, QT)) { + Template->addArg(AF, parm_idx, ArgEffect(UnretainedOutParameter, *K)); return true; - } else if (hasRCAnnotation(pd, "rc_ownership_consumed")) { - Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::Generalized)); - return true; - } else if (hasEnabledAttr(pd) || - hasRCAnnotation(pd, "rc_ownership_returns_retained")) { - QualType PointeeTy = pd->getType()->getPointeeType(); - if (!PointeeTy.isNull()) { - if (coreFoundation::isCFObjectRef(PointeeTy)) { - Template->addArg(AF, parm_idx, ArgEffect(RetainedOutParameter, - ObjKind::CF)); - return true; - } - } - } else if (hasEnabledAttr(pd)) { - QualType PointeeTy = pd->getType()->getPointeeType(); - if (!PointeeTy.isNull()) { - if (coreFoundation::isCFObjectRef(PointeeTy)) { - Template->addArg(AF, parm_idx, ArgEffect(UnretainedOutParameter, - ObjKind::CF)); - return true; - } - } } else { if (const auto *MD = dyn_cast(FD)) { for (const auto *OD : MD->overridden_methods()) { @@ -831,7 +861,7 @@ RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ, if (Optional RetE = getRetEffectFromAnnotations(RetTy, FD)) Template->setRetEffect(*RetE); - if (hasEnabledAttr(FD)) + if (hasAnyEnabledAttrOf(FD, RetTy)) Template->setThisEffect(ArgEffect(DecRef, ObjKind::OS)); } @@ -845,35 +875,25 @@ RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ, RetainSummaryTemplate Template(Summ, *this); // Effects on the receiver. - if (MD->hasAttr()) + if (hasAnyEnabledAttrOf(MD, MD->getReturnType())) Template->setReceiverEffect(ArgEffect(DecRef, ObjKind::ObjC)); // Effects on the parameters. unsigned parm_idx = 0; - for (auto pi=MD->param_begin(), pe=MD->param_end(); - pi != pe; ++pi, ++parm_idx) { + for (auto pi = MD->param_begin(), pe = MD->param_end(); pi != pe; + ++pi, ++parm_idx) { const ParmVarDecl *pd = *pi; - if (pd->hasAttr()) { - Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::ObjC)); - } else if (pd->hasAttr()) { - Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::CF)); - } else if (pd->hasAttr()) { - Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::OS)); - } else if (pd->hasAttr()) { - QualType PointeeTy = pd->getType()->getPointeeType(); - if (!PointeeTy.isNull()) - if (coreFoundation::isCFObjectRef(PointeeTy)) - Template->addArg(AF, parm_idx, - ArgEffect(RetainedOutParameter, ObjKind::CF)); - } else if (pd->hasAttr()) { - QualType PointeeTy = pd->getType()->getPointeeType(); - if (!PointeeTy.isNull()) - if (coreFoundation::isCFObjectRef(PointeeTy)) - Template->addArg(AF, parm_idx, ArgEffect(UnretainedOutParameter, - ObjKind::CF)); + QualType QT = pd->getType(); + if (auto K = + hasAnyEnabledAttrOf( + pd, QT)) { + Template->addArg(AF, parm_idx, ArgEffect(DecRef, *K)); + } else if (auto K = hasAnyEnabledAttrOf(pd, QT)) { + Template->addArg(AF, parm_idx, ArgEffect(RetainedOutParameter, *K)); + } else if (auto K = hasAnyEnabledAttrOf(pd, QT)) { + Template->addArg(AF, parm_idx, ArgEffect(UnretainedOutParameter, *K)); } } - QualType RetTy = MD->getReturnType(); if (Optional RetE = getRetEffectFromAnnotations(RetTy, MD)) Template->setRetEffect(*RetE); -- 2.40.0