diff --git a/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h b/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h new file mode 100644 --- /dev/null +++ b/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h @@ -0,0 +1,111 @@ +//===- CalledOnceCheck.h - Check 'called once' parameters -------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines a check for function-like parameters that should be +// called exactly one time. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_CALLEDONCECHECK_H +#define LLVM_CLANG_ANALYSIS_ANALYSES_CALLEDONCECHECK_H + +namespace clang { + +class AnalysisDeclContext; +class CFG; +class Decl; +class DeclContext; +class Expr; +class ParmVarDecl; +class Stmt; + +/// Classification of situations when parameter is not called on every path. +/// \enum IfThen -- then branch of the if statement has no call. +/// \enum IfElse -- else branch of the if statement has no call. +/// \enum Switch -- one of the switch cases doesn't have a call. +/// \enum SwitchSkipped -- there is no call if none of the cases appies. +/// \enum LoopEntered -- no call when the loop is entered. +/// \enum LoopSkipped -- no call when the loop is not entered. +/// \enum FallbackReason -- fallback case when we were not able to figure out +/// the reason. +enum class NeverCalledReason { + IfThen, + IfElse, + Switch, + SwitchSkipped, + LoopEntered, + LoopSkipped, + FallbackReason, + LARGEST_VALUE = FallbackReason +}; + +class CalledOnceCheckHandler { +public: + CalledOnceCheckHandler() = default; + virtual ~CalledOnceCheckHandler() = default; + + /// Called when parameter is called twice. + /// \param Parameter -- parameter that should be called once. + /// \param Call -- call to report the warning. + /// \param PrevCall -- previous call. + /// \param IsCompletionHandler -- true, if parameter is a completion handler. + virtual void handleDoubleCall(const ParmVarDecl *Parameter, const Expr *Call, + const Expr *PrevCall, + bool IsCompletionHandler) {} + + /// Called when parameter is not called at all. + /// \param Parameter -- parameter that should be called once. + /// \param IsCompletionHandler -- true, if parameter is a completion handler. + virtual void handleNeverCalled(const ParmVarDecl *Parameter, + bool IsCompletionHandler) {} + + /// Called when captured parameter is not called at all. + /// \param Parameter -- parameter that should be called once. + /// \param Where -- declaration that captures \p Parameter + /// \param IsCompletionHandler -- true, if parameter is a completion handler. + virtual void handleCapturedNeverCalled(const ParmVarDecl *Parameter, + const Decl *Where, + bool IsCompletionHandler) {} + + /// Called when parameter is not called on one of the paths. + /// Usually we try to find a statement that is the least common ancestor of + /// the path containing the call and not containing the call. This helps us + /// to pinpoint a bad path for the user. + /// \param Parameter -- parameter that should be called once. + /// \param Where -- the least common ancestor statement. + /// \param Reason -- a reason describing the path without a call. + /// \param IsCalledDirectly -- true, if parameter actually gets called on + /// the other path. It is opposed to be used in some other way (added to some + /// collection, passed as a parameter, etc.). + /// \param IsCompletionHandler -- true, if parameter is a completion handler. + virtual void handleNeverCalled(const ParmVarDecl *Parameter, + const Stmt *Where, NeverCalledReason Reason, + bool IsCalledDirectly, + bool IsCompletionHandler) {} +}; + +/// Check given CFG for 'called once' parameter violations. +/// +/// It traverses the function and tracks how such parameters are used. +/// It detects two main violations: +/// * parameter is called twice +/// * parameter is not called +/// +/// \param FunctionCFG -- CFG of the function to analyze. +/// \param AC -- context. +/// \param Handler -- a handler for found violations. +/// \param CheckConventionalParameters -- true, if we want to check parameters +/// not explicitly marked as 'called once', but having the same requirements +/// according to conventions. +void checkCalledOnceParameters(const CFG &FunctionCFG, AnalysisDeclContext &AC, + CalledOnceCheckHandler &Handler, + bool CheckConventionalParameters); + +} // end namespace clang + +#endif /* LLVM_CLANG_ANALYSIS_ANALYSES_CALLEDONCECHECK_H */ diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1767,6 +1767,13 @@ let Documentation = [ReturnsNonNullDocs]; } +def CalledOnce : Attr { + let Spellings = [Clang<"called_once">]; + let Subjects = SubjectList<[ParmVar]>; + let LangOpts = [ObjC]; + let Documentation = [Undocumented]; +} + // pass_object_size(N) indicates that the parameter should have // __builtin_object_size with Type=N evaluated on the parameter at the callsite. def PassObjectSize : InheritableParamAttr { diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -485,6 +485,8 @@ def ObjCMultipleMethodNames : DiagGroup<"objc-multiple-method-names">; def ObjCFlexibleArray : DiagGroup<"objc-flexible-array">; def ObjCBoxing : DiagGroup<"objc-boxing">; +def CompletionHandler : DiagGroup<"completion-handler">; +def CalledOnceParameter : DiagGroup<"called-once-parameter", [CompletionHandler]>; def OpenCLUnsupportedRGBA: DiagGroup<"opencl-unsupported-rgba">; def UnderalignedExceptionObject : DiagGroup<"underaligned-exception-object">; def DeprecatedObjCIsaUsage : DiagGroup<"deprecated-objc-isa-usage">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3924,6 +3924,38 @@ def note_protocol_decl_undefined : Note< "protocol %0 has no definition">; +// called-once attribute diagnostics. +def err_called_once_attribute_wrong_type : Error< + "'called_once' attribute only applies to function-like parameters">; + +def warn_completion_handler_never_called : Warning< + "%select{|captured }1completion handler is never called">, + InGroup, DefaultIgnore; +def warn_called_once_never_called : Warning< + "%select{|captured }1%0 parameter marked 'called_once' is never called">, + InGroup; + +def warn_completion_handler_never_called_when : Warning< + "completion handler is never %select{used|called}1 when " + "%select{taking true branch|taking false branch|" + "handling this case|none of the cases applies|" + "entering the loop|skipping the loop|taking one of the branches}2">, + InGroup, DefaultIgnore; +def warn_called_once_never_called_when : Warning< + "%0 parameter marked 'called_once' is never %select{used|called}1 when " + "%select{taking true branch|taking false branch|" + "handling this case|none of the cases applies|" + "entering the loop|skipping the loop|taking one of the branches}2">, + InGroup; + +def warn_completion_handler_called_twice : Warning< + "completion handler is called twice">, + InGroup, DefaultIgnore; +def warn_called_once_gets_called_twice : Warning< + "%0 parameter marked 'called_once' is called twice">, + InGroup; +def note_called_once_gets_called_twice : Note<"previous call is here">; + // objc_designated_initializer attribute diagnostics. def warn_objc_designated_init_missing_super_call : Warning< "designated initializer missing a 'super' call to a designated initializer of the super class">, diff --git a/clang/lib/Analysis/CMakeLists.txt b/clang/lib/Analysis/CMakeLists.txt --- a/clang/lib/Analysis/CMakeLists.txt +++ b/clang/lib/Analysis/CMakeLists.txt @@ -6,6 +6,7 @@ add_clang_library(clangAnalysis AnalysisDeclContext.cpp BodyFarm.cpp + CalledOnceCheck.cpp CFG.cpp CFGReachabilityAnalysis.cpp CFGStmtMap.cpp diff --git a/clang/lib/Analysis/CalledOnceCheck.cpp b/clang/lib/Analysis/CalledOnceCheck.cpp new file mode 100644 --- /dev/null +++ b/clang/lib/Analysis/CalledOnceCheck.cpp @@ -0,0 +1,1292 @@ +//===- CalledOnceCheck.cpp - Check 'called once' parameters ---------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Analysis/Analyses/CalledOnceCheck.h" +#include "clang/AST/Attr.h" +#include "clang/AST/Decl.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprObjC.h" +#include "clang/AST/OperationKinds.h" +#include "clang/AST/ParentMap.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/Stmt.h" +#include "clang/AST/StmtObjC.h" +#include "clang/AST/StmtVisitor.h" +#include "clang/Analysis/AnalysisDeclContext.h" +#include "clang/Analysis/CFG.h" +#include "clang/Analysis/FlowSensitive/DataflowWorklist.h" +#include "clang/Basic/IdentifierTable.h" +#include "clang/Basic/LLVM.h" +#include "llvm/ADT/BitVector.h" +#include "llvm/ADT/BitmaskEnum.h" +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/PointerIntPair.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/Sequence.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/Compiler.h" +#include "llvm/Support/ErrorHandling.h" +#include + +using namespace clang; + +namespace { +static constexpr unsigned EXPECTED_MAX_NUMBER_OF_PARAMS = 2; +template +using ParamSizedVector = llvm::SmallVector; +static constexpr unsigned EXPECTED_NUMBER_OF_BASIC_BLOCKS = 8; +template +using CFGSizedVector = llvm::SmallVector; +constexpr llvm::StringLiteral CONVENTIONAL_NAME = "completionHandler"; +constexpr llvm::StringLiteral CONVENTIONAL_CONDITIONS[] = { + "error", "cancel", "shouldCall", "done", "OK", "success"}; + +class ParameterStatus { +public: + enum Kind { + // No-return paths should be absolutely transparent for the analysis. + // 0x0 is the identity element for selected join operation (binary or). + NoReturn = 0x0, /* 0000 */ + // Escaped marks situations when marked parameter escaped into + // another function (so we can assume that it was possibly called there). + Escaped = 0x1, /* 0001 */ + // Parameter was definitely called once at this point. + DefinitelyCalled = 0x3, /* 0011 */ + // Kinds less or equal to NON_ERROR_STATUS are not considered errors. + NON_ERROR_STATUS = DefinitelyCalled, + // Parameter was not yet called. + NotCalled = 0x5, /* 0101 */ + // Parameter was not called at least on one path leading to this point, + // while there is also at least one path that it gets called. + MaybeCalled = 0x7, /* 0111 */ + // Parameter was not yet analyzed. + NotVisited = 0x8, /* 1000 */ + // We already reported a violation and stopped tracking calls for this + // parameter. + Reported = 0x15, /* 1111 */ + LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ Reported) + + // Defined elements should maintain the following properties: + // 1. for any Kind K: NoReturn | K == K + // 2. for any Kind K: Reported | K == K + // 3. for any Kind K: K | K == K + // 4. DefinitelyCalled | NotCalled == MaybeCalled + // 5. for any Kind K in [DefinitelyCalled, NotCalled, MaybeCalled]: + // Escaped | K == K + // 6. for any Kind K in [DefinitelyCalled, NotCalled]: + // MaybeCalled | K == MaybeCalled + }; + + constexpr ParameterStatus() = default; + /* implicit */ ParameterStatus(Kind K) : StatusKind(K) { + assert(!canBeCalled(K) && "Can't initialize status without a call"); + } + ParameterStatus(Kind K, const Expr *Call) : StatusKind(K), Call(Call) { + assert(canBeCalled(K) && "This kind is not supposed to have a call"); + } + + const Expr &getCall() const { + assert(canBeCalled(getKind()) && "ParameterStatus doesn't have a call"); + return *Call; + } + static bool canBeCalled(Kind K) { + return (K & DefinitelyCalled) == DefinitelyCalled && K != Reported; + } + bool canBeCalled() const { return canBeCalled(getKind()); } + + static bool isErrorStatus(Kind K) { return K > NON_ERROR_STATUS; } + bool isErrorStatus() const { return isErrorStatus(getKind()); } + + Kind getKind() const { return StatusKind; } + + void join(const ParameterStatus &Other) { + // If we have a pointer already, let's keep it. + // For the purposes of the analysis, it doesn't really matter + // which call we report. + // + // If we don't have a pointer, let's take whatever gets joined. + if (!Call) { + Call = Other.Call; + } + // Join kinds. + StatusKind |= Other.getKind(); + } + + bool operator==(const ParameterStatus &Other) const { + // We compare only kinds, pointers on their own is only additional + // information. + return getKind() == Other.getKind(); + } + +private: + // It would've been a perfect place to use llvm::PointerIntPair, but + // unfortunately NumLowBitsAvailable for clang::Expr had been reduced to 2. + Kind StatusKind = NotVisited; + const Expr *Call = nullptr; +}; + +class State { +public: + State(unsigned Size, ParameterStatus::Kind K = ParameterStatus::NotVisited) + : ParamData(Size, K) {} + + /// Return status of a parameter with the given index. + /// \{ + ParameterStatus &getStatusFor(unsigned Index) { return ParamData[Index]; } + const ParameterStatus &getStatusFor(unsigned Index) const { + return ParamData[Index]; + } + /// \} + + /// Return true if parameter with the given index can be called. + bool canBeCalled(unsigned Index) const { + return getStatusFor(Index).canBeCalled(); + } + /// Return a reference that we consider a call. + /// + /// Should only be used for parameters that can be called. + const Expr &getCallFor(unsigned Index) const { + return getStatusFor(Index).getCall(); + } + /// Return status kind of parameter with the given index. + ParameterStatus::Kind getKindFor(unsigned Index) const { + return getStatusFor(Index).getKind(); + } + + bool isVisited() const { + return llvm::all_of(ParamData, [](const ParameterStatus &S) { + return S.getKind() != ParameterStatus::NotVisited; + }); + } + + // Join other state into the current state. + void join(const State &Other) { + assert(ParamData.size() == Other.ParamData.size() && + "Couldn't join statuses with different sizes"); + for (auto Pair : llvm::zip(ParamData, Other.ParamData)) { + std::get<0>(Pair).join(std::get<1>(Pair)); + } + } + + using iterator = ParamSizedVector::iterator; + using const_iterator = ParamSizedVector::const_iterator; + + iterator begin() { return ParamData.begin(); } + iterator end() { return ParamData.end(); } + + const_iterator begin() const { return ParamData.begin(); } + const_iterator end() const { return ParamData.end(); } + + bool operator==(const State &Other) const { + return ParamData == Other.ParamData; + } + +private: + ParamSizedVector ParamData; +}; + +/// A simple class that finds DeclRefExpr in the given expression. +/// +/// However, we don't want to find ANY nested DeclRefExpr skipping whatever +/// expressions on our way. Only certain expressions considered "no-op" +/// for our task are indeed skipped. +class DeclRefFinder + : public ConstStmtVisitor { +public: + static const DeclRefExpr *find(const Expr *E, + bool ShouldRetrieveFromComparisons = false) { + return DeclRefFinder(ShouldRetrieveFromComparisons).Visit(E); + } + + const DeclRefExpr *VisitDeclRefExpr(const DeclRefExpr *DR) { return DR; } + + const DeclRefExpr *VisitUnaryOperator(const UnaryOperator *UO) { + switch (UO->getOpcode()) { + case UO_LNot: + // We care about logical not only if we care about comparisons. + if (!ShouldRetrieveFromComparisons) + return nullptr; + LLVM_FALLTHROUGH; + // Function pointer/references can be dereferenced before a call. + // That doesn't make it, however, any different from a regular call. + // For this reason, dereference operation is a "no-op". + case UO_Deref: + return Visit(UO->getSubExpr()); + default: + return nullptr; + } + } + + const DeclRefExpr *VisitBinaryOperator(const BinaryOperator *BO) { + if (!ShouldRetrieveFromComparisons) + return nullptr; + + switch (BO->getOpcode()) { + case BO_EQ: + case BO_NE: { + const DeclRefExpr *LHS = Visit(BO->getLHS()); + return LHS ? LHS : Visit(BO->getRHS()); + } + default: + return nullptr; + } + } + + const DeclRefExpr *VisitOpaqueValueExpr(const OpaqueValueExpr *OVE) { + return Visit(OVE->getSourceExpr()); + } + + const DeclRefExpr *VisitExpr(const Expr *E) { + // It is a fallback method that gets called whenever the actual type + // of the given expression is not covered. + // + // We first check if we have anything to skip. And then repeat the whole + // procedure for a nested expression instead. + const Expr *DeclutteredExpr = E->IgnoreParenCasts(); + return E != DeclutteredExpr ? Visit(DeclutteredExpr) : nullptr; + } + +private: + DeclRefFinder(bool ShouldRetrieveFromComparisons) + : ShouldRetrieveFromComparisons(ShouldRetrieveFromComparisons) {} + + bool ShouldRetrieveFromComparisons; +}; + +const DeclRefExpr *findDeclRefExpr(const Expr *In, + bool ShouldRetrieveFromComparisons = false) { + return DeclRefFinder::find(In, ShouldRetrieveFromComparisons); +} + +const ParmVarDecl * +findReferencedParmVarDecl(const Expr *In, + bool ShouldRetrieveFromComparisons = false) { + if (const DeclRefExpr *DR = + findDeclRefExpr(In, ShouldRetrieveFromComparisons)) { + return dyn_cast(DR->getDecl()); + } + + return nullptr; +} + +/// Return conditions expression of a statement if it has one. +const Expr *getCondition(const Stmt *S) { + if (!S) { + return nullptr; + } + + if (const auto *If = dyn_cast(S)) { + return If->getCond(); + } + if (const auto *Ternary = dyn_cast(S)) { + return Ternary->getCond(); + } + + return nullptr; +} + +/// A small helper class that collects all named identifiers in the given +/// expression. It traverses it recursively, so names from deeper levels +/// of the AST will end up in the results. +/// Results might have duplicate names, if this is a problem, convert to +/// string sets afterwards. +class NamesCollector : public RecursiveASTVisitor { +public: + static constexpr unsigned EXPECTED_NUMBER_OF_NAMES = 5; + using NameCollection = + llvm::SmallVector; + + static NameCollection collect(const Expr *From) { + NamesCollector Impl; + Impl.TraverseStmt(const_cast(From)); + return Impl.Result; + } + + bool VisitDeclRefExpr(const DeclRefExpr *E) { + Result.push_back(E->getDecl()->getName()); + return true; + } + + bool VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *E) { + llvm::StringRef Name; + + if (E->isImplicitProperty()) { + ObjCMethodDecl *PropertyMethodDecl = nullptr; + if (E->isMessagingGetter()) { + PropertyMethodDecl = E->getImplicitPropertyGetter(); + } else { + PropertyMethodDecl = E->getImplicitPropertySetter(); + } + assert(PropertyMethodDecl && + "Implicit property must have associated declaration"); + Name = PropertyMethodDecl->getSelector().getNameForSlot(0); + } else { + assert(E->isExplicitProperty()); + Name = E->getExplicitProperty()->getName(); + } + + Result.push_back(Name); + return true; + } + +private: + NamesCollector() = default; + NameCollection Result; +}; + +/// Check whether the given expression mentions any of conventional names. +bool mentionsAnyOfConventionalNames(const Expr *E) { + NamesCollector::NameCollection MentionedNames = NamesCollector::collect(E); + + return llvm::any_of(MentionedNames, [](llvm::StringRef ConditionName) { + return llvm::any_of( + CONVENTIONAL_CONDITIONS, + [ConditionName](const llvm::StringLiteral &Conventional) { + return ConditionName.contains_lower(Conventional); + }); + }); +} + +struct Clarification { + NeverCalledReason Reason; + const Stmt *Location; +}; + +class NotCalledClarifier + : public ConstStmtVisitor> { +public: + static llvm::Optional + clarify(const CFGBlock *Conditional, const CFGBlock *SuccWithoutCall) { + + if (const Stmt *Terminator = Conditional->getTerminatorStmt()) { + return NotCalledClarifier{Conditional, SuccWithoutCall}.Visit(Terminator); + } + return llvm::None; + } + + llvm::Optional VisitIfStmt(const IfStmt *If) { + return VisitBranchingBlock(If, NeverCalledReason::IfThen); + } + + llvm::Optional + VisitAbstractConditionalOperator(const AbstractConditionalOperator *Ternary) { + return VisitBranchingBlock(Ternary, NeverCalledReason::IfThen); + } + + llvm::Optional VisitSwitchStmt(const SwitchStmt *Switch) { + const Stmt *CaseToBlame = SuccInQuestion->getLabel(); + if (!CaseToBlame) { + // If interesting basic block is not labeled, it means that this + // basic block does not represent any of the cases. + return Clarification{NeverCalledReason::SwitchSkipped, Switch}; + } + + for (const SwitchCase *Case = Switch->getSwitchCaseList(); Case; + Case = Case->getNextSwitchCase()) { + if (Case == CaseToBlame) { + return Clarification{NeverCalledReason::Switch, Case}; + } + } + + llvm_unreachable("Found unexpected switch structure"); + } + + llvm::Optional VisitForStmt(const ForStmt *For) { + return VisitBranchingBlock(For, NeverCalledReason::LoopEntered); + } + + llvm::Optional VisitWhileStmt(const WhileStmt *While) { + return VisitBranchingBlock(While, NeverCalledReason::LoopEntered); + } + + llvm::Optional + VisitBranchingBlock(const Stmt *Terminator, NeverCalledReason DefaultReason) { + assert(Parent->succ_size() == 2 && + "Branching block should have exactly two successors"); + unsigned SuccessorIndex = getSuccessorIndex(Parent, SuccInQuestion); + NeverCalledReason ActualReason = + updateForSuccessor(DefaultReason, SuccessorIndex); + return Clarification{ActualReason, Terminator}; + } + + llvm::Optional VisitBinaryOperator(const BinaryOperator *) { + // We don't want to report on short-curcuit logical operations. + return llvm::None; + } + + llvm::Optional VisitStmt(const Stmt *Terminator) { + // If we got here, we didn't have a visit function for more dirived + // classes of statement that this terminator actually belongs to. + // + // This is not a good scenario and should not happen in practice, but + // at least we'll warn the user. + return Clarification{NeverCalledReason::FallbackReason, Terminator}; + } + + static unsigned getSuccessorIndex(const CFGBlock *Parent, + const CFGBlock *Child) { + CFGBlock::const_succ_iterator It = llvm::find(Parent->succs(), Child); + assert(It != Parent->succ_end() && + "Given blocks should be in parent-child relationship"); + return It - Parent->succ_begin(); + } + + static NeverCalledReason + updateForSuccessor(NeverCalledReason ReasonForTrueBranch, + unsigned SuccessorIndex) { + assert(SuccessorIndex <= 1); + unsigned RawReason = + static_cast(ReasonForTrueBranch) + SuccessorIndex; + assert(RawReason <= + static_cast(NeverCalledReason::LARGEST_VALUE)); + return static_cast(RawReason); + } + +private: + NotCalledClarifier(const CFGBlock *Parent, const CFGBlock *SuccInQuestion) + : Parent(Parent), SuccInQuestion(SuccInQuestion) {} + + const CFGBlock *Parent, *SuccInQuestion; +}; + +class CalledOnceChecker : public ConstStmtVisitor { +public: + static void check(const CFG &FunctionCFG, AnalysisDeclContext &AC, + CalledOnceCheckHandler &Handler, + bool CheckConventionalParameters) { + CalledOnceChecker(FunctionCFG, AC, Handler, CheckConventionalParameters) + .check(); + } + +private: + CalledOnceChecker(const CFG &FunctionCFG, AnalysisDeclContext &AC, + CalledOnceCheckHandler &Handler, + bool CheckConventionalParameters) + : FunctionCFG(FunctionCFG), AC(AC), Handler(Handler), + CheckConventionalParameters(CheckConventionalParameters), + CurrentState(0) { + initDataStructures(); + assert(size() == 0 || + !States.empty() && "Data structures are inconsistent"); + } + + //===----------------------------------------------------------------------===// + // Initializing functions + //===----------------------------------------------------------------------===// + + void initDataStructures() { + const Decl *AnalyzedDecl = AC.getDecl(); + + if (const auto *Function = dyn_cast(AnalyzedDecl)) { + findParamsToTrack(Function); + } else if (const auto *Method = dyn_cast(AnalyzedDecl)) { + findParamsToTrack(Method); + if (CheckConventionalParameters) { + findConventionalParamsToTrack(Method); + } + } else if (const auto *Block = dyn_cast(AnalyzedDecl)) { + findCapturesToTrack(Block); + findParamsToTrack(Block); + } + + // Have something to track, let's init states for every block from the CFG. + if (size() != 0) { + States = + CFGSizedVector(FunctionCFG.getNumBlockIDs(), State(size())); + } + } + + void findCapturesToTrack(const BlockDecl *Block) { + for (const auto &Capture : Block->captures()) { + if (const auto *P = dyn_cast(Capture.getVariable())) { + if (shouldBeCalledOnce(P)) { + TrackedParams.push_back(P); + } + } + } + } + + template + void findParamsToTrack(const FunctionLikeDecl *Function) { + for (const ParmVarDecl *P : Function->parameters()) { + if (shouldBeCalledOnce(P)) { + TrackedParams.push_back(P); + } + } + } + + void findConventionalParamsToTrack(const ObjCMethodDecl *Method) { + Selector MethodSelector = Method->getSelector(); + + // We're interested only in methods with extra selector slots. + if (MethodSelector.getNumArgs() <= 1) { + return; + } + + for (unsigned Index : llvm::seq(1u, MethodSelector.getNumArgs())) { + if (shouldBeCalledOnce(MethodSelector, Index)) { + const ParmVarDecl *Parameter = Method->getParamDecl(Index); + // Let's track it only if we wouldn've decided about this based + // on parameter itself. + if (!shouldBeCalledOnce(Parameter)) { + TrackedParams.push_back(Parameter); + } + } + } + } + + //===----------------------------------------------------------------------===// + // Main logic 'check' functions + //===----------------------------------------------------------------------===// + + void check() { + // Nothing to check here: we don't have marked parameters. + if (size() == 0 || isPossiblyEmptyImpl()) + return; + + assert( + llvm::none_of(States, [](const State &S) { return S.isVisited(); }) && + "None of the blocks should be 'visited' before the analysis"); + + // For our task, both backward and forward approaches suite well. + // However, in order to report better diagnostics, we decided to go with + // backward analysis. + // + // Let's consider the following CFG and how forward and backward analyses + // will work for it. + // + // FORWARD: | BACKWARD: + // #1 | #1 + // +---------+ | +-----------+ + // | if | | |MaybeCalled| + // +---------+ | +-----------+ + // |NotCalled| | | if | + // +---------+ | +-----------+ + // / \ | / \ + // #2 / \ #3 | #2 / \ #3 + // +----------------+ +---------+ | +----------------+ +---------+ + // | foo() | | ... | | |DefinitelyCalled| |NotCalled| + // +----------------+ +---------+ | +----------------+ +---------+ + // |DefinitelyCalled| |NotCalled| | | foo() | | ... | + // +----------------+ +---------+ | +----------------+ +---------+ + // \ / | \ / + // \ #4 / | \ #4 / + // +-----------+ | +---------+ + // | ... | | |NotCalled| + // +-----------+ | +---------+ + // |MaybeCalled| | | ... | + // +-----------+ | +---------+ + // + // The most natural way to report lacking call in the block #3 would be to + // message that the false branch of the if statement in the block #1 doesn't + // have a call. And while with the forward approach we'll need to find a + // least common ancestor or something like that to find the 'if' to blame, + // backward analysis gives is to us out of the box. + BackwardDataflowWorklist Worklist(FunctionCFG, AC); + + // Let's visit EXIT. + const CFGBlock *Exit = &FunctionCFG.getExit(); + assignState(Exit, State(size(), ParameterStatus::NotCalled)); + Worklist.enqueuePredecessors(Exit); + + while (const CFGBlock *BB = Worklist.dequeue()) { + assert(BB && "Worklist should filter out null blocks"); + check(BB); + assert(CurrentState.isVisited() && + "After the check, basic block should be visited"); + + // Traverse successor basic blocks if the status of this block + // has changed. + if (assignState(BB, CurrentState)) { + Worklist.enqueuePredecessors(BB); + } + } + + // Check that we have all tracked parameters at the last block. + // As we are performing a backward version of the analysis, + // it should be the ENTRY block. + checkEntry(&FunctionCFG.getEntry()); + } + + void check(const CFGBlock *BB) { + // We start with a state 'inherited' from all the successors. + CurrentState = joinSuccessors(BB); + assert(CurrentState.isVisited() && + "Shouldn't start with a 'not visited' state"); + + // This is the 'exit' situation, broken promises are probably OK + // in such scenarios. + if (BB->hasNoReturnElement()) { + markNoReturn(); + // This block still can have calls (even multiple calls) and + // for this reason there is no early return here. + } + + // We use a backward dataflow propagation and for this reason we + // should travers basic blocks bottom-up. + for (const CFGElement &Element : llvm::reverse(*BB)) { + if (Optional S = Element.getAs()) { + check(S->getStmt()); + } + } + } + void check(const Stmt *S) { Visit(S); } + + void checkEntry(const CFGBlock *Entry) { + // We finalize this algorithm with the ENTRY block because + // we use a backward version of the analysis. This is where + // we can judge that some of the tracked parameters are not called on + // every path from ENTRY to EXIT. + + const State &EntryStatus = getState(Entry); + llvm::BitVector NotCalledOnEveryPath(size(), false); + llvm::BitVector NotUsedOnEveryPath(size(), false); + + // Check if there are no calls of the marked parameter at all + for (const auto &IndexedStatus : llvm::enumerate(EntryStatus)) { + const ParmVarDecl *Parameter = getParameter(IndexedStatus.index()); + + switch (IndexedStatus.value().getKind()) { + case ParameterStatus::NotCalled: + // If there were places where this parameter escapes (aka being used), + // we can provide a more useful diagnostic by pointing at the exact + // branches where it is not even mentioned. + if (!wasEverUsed(IndexedStatus.index())) { + // This parameter is was not used at all, so we should report the + // most generic version of the warning. + if (isCaptured(Parameter)) { + // We want to specify that it was captured by the block. + Handler.handleCapturedNeverCalled(Parameter, AC.getDecl(), + !isExplicitlyMarked(Parameter)); + } else { + Handler.handleNeverCalled(Parameter, + !isExplicitlyMarked(Parameter)); + } + } else { + // Mark it as 'interesting' to figure out which paths don't even + // have escapes. + NotUsedOnEveryPath[IndexedStatus.index()] = true; + } + + break; + case ParameterStatus::MaybeCalled: + // If we have 'maybe called' at this point, we have an error + // that there is at least one path where this parameter + // is not called. + // + // However, reporting the warning with only that information can be + // too vague for the users. For this reason, we mark such parameters + // as "interesting" for further analysis. + NotCalledOnEveryPath[IndexedStatus.index()] = true; + break; + default: + break; + } + } + + // Early exit if we don't have parameters for extra analysis. + if (NotCalledOnEveryPath.none() && NotUsedOnEveryPath.none()) + return; + + // We are looking for a pair of blocks A, B so that the following is true: + // * A is a predecessor of B + // * B is marked as NotCalled + // * A has at least one successor marked as either + // Escaped or DefinitelyCalled + // + // In that situation, it is guaranteed that B is the first block of the path + // where the user doesn't call or use parameter in question. + // + // For this reason, branch A -> B can be used for reporting. + // + // This part of the algorithm is guarded by a condition that the function + // does indeed have a violation of contract. For this reason, we can + // spend more time to find a good spot to place the warning. + // + // The following algorithm has the worst case complexity of O(N^2), + // where N is the number of basic blocks in FunctionCFG. + for (const CFGBlock *BB : FunctionCFG) { + if (!BB) + continue; + + const State &BlockState = getState(BB); + + for (unsigned Index : llvm::seq(0u, size())) { + // We don't want to use 'isLosingCall' here because we want to report + // the following situation as well: + // + // MaybeCalled + // | ... | + // MaybeCalled NotCalled + // + // Even though successor is not 'DefinitelyCalled', it is still useful + // to report it, it is still a path without a call. + if (NotCalledOnEveryPath[Index] && + BlockState.getKindFor(Index) == ParameterStatus::MaybeCalled) { + + findAndReportNotCalledBranches(BB, Index); + } else if (NotUsedOnEveryPath[Index] && + isLosingEscape(BlockState, BB, Index)) { + + findAndReportNotCalledBranches(BB, Index, /* IsEscape = */ true); + } + } + } + } + + /// Check potential call of a tracked parameter. + void checkDirectCall(const CallExpr *Call) { + if (auto Index = getIndexOfCallee(Call)) { + processCallFor(*Index, Call); + } + } + + /// Check the call expression for being an indirect call of one of the tracked + /// parameters. It is indirect in the sense that this particular call is not + /// calling the parameter itself, but rather uses it as the argument. + template + void checkIndirectCall(const CallLikeExpr *CallOrMessage) { + // CallExpr::arguments does not interact nicely with llvm::enumerate. + llvm::ArrayRef Arguments = llvm::makeArrayRef( + CallOrMessage->getArgs(), CallOrMessage->getNumArgs()); + + // Let's check if any of the call arguments is a point of interest. + for (const auto &Argument : llvm::enumerate(Arguments)) { + if (auto Index = getIndexOfExpression(Argument.value())) { + ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(*Index); + + if (shouldBeCalledOnce(CallOrMessage, Argument.index())) { + // If the corresponding parameter is marked as 'called_once' we should + // consider it as a call. + processCallFor(*Index, CallOrMessage); + } else if (CurrentParamStatus.getKind() == ParameterStatus::NotCalled) { + // Otherwise, we mark this parameter as escaped, which can be + // interpreted both as called or not called depending on the context. + CurrentParamStatus = ParameterStatus::Escaped; + } + // Otherwise, let's keep the state as it is. + } + } + } + + /// Process call of the parameter with the given index + void processCallFor(unsigned Index, const Expr *Call) { + ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(Index); + + if (CurrentParamStatus.canBeCalled()) { + + // At this point, this parameter was called, so this is a second call. + const ParmVarDecl *Parameter = getParameter(Index); + Handler.handleDoubleCall(Parameter, &CurrentState.getCallFor(Index), Call, + !isExplicitlyMarked(Parameter)); + // Mark this parameter as already reported on, so we don't repeat + // warnings. + CurrentParamStatus = ParameterStatus::Reported; + + } else if (CurrentParamStatus.getKind() != ParameterStatus::Reported) { + // If we didn't report anything yet, let's mark this parameter + // as called. + ParameterStatus Called(ParameterStatus::DefinitelyCalled, Call); + CurrentParamStatus = Called; + } + } + + void findAndReportNotCalledBranches(const CFGBlock *Parent, unsigned Index, + bool IsEscape = false) { + for (const CFGBlock *Succ : Parent->succs()) { + if (!Succ) + continue; + + if (getState(Succ).getKindFor(Index) == ParameterStatus::NotCalled) { + assert(Parent->succ_size() >= 2 && + "Block should have at least two successors at this point"); + if (auto Clarification = NotCalledClarifier::clarify(Parent, Succ)) { + const ParmVarDecl *Parameter = getParameter(Index); + Handler.handleNeverCalled(Parameter, Clarification->Location, + Clarification->Reason, !IsEscape, + !isExplicitlyMarked(Parameter)); + } + } + } + } + + //===----------------------------------------------------------------------===// + // Predicate functions to check parameters + //===----------------------------------------------------------------------===// + + static bool isExplicitlyMarked(const ParmVarDecl *Parameter) { + return Parameter->hasAttr(); + } + + bool shouldBeCalledOnce(const ParmVarDecl *Parameter) const { + return (CheckConventionalParameters && + Parameter->getName() == CONVENTIONAL_NAME) || + isExplicitlyMarked(Parameter); + } + + bool shouldBeCalledOnce(Selector MethodSelector, unsigned ParamIndex) const { + return CheckConventionalParameters && MethodSelector.getNumArgs() > 1 && + MethodSelector.getNameForSlot(ParamIndex) == CONVENTIONAL_NAME; + } + + bool shouldBeCalledOnce(const CallExpr *Call, unsigned ParamIndex) const { + const FunctionDecl *Function = Call->getDirectCallee(); + return Function && ParamIndex < Function->getNumParams() && + shouldBeCalledOnce(Function->getParamDecl(ParamIndex)); + } + + bool shouldBeCalledOnce(const ObjCMessageExpr *Message, + unsigned ParamIndex) const { + const ObjCMethodDecl *Method = Message->getMethodDecl(); + Selector UsedSelector = Message->getSelector(); + return Method && ParamIndex < Method->param_size() && + (shouldBeCalledOnce(Method->getParamDecl(ParamIndex)) || + shouldBeCalledOnce(UsedSelector, ParamIndex)); + } + + //===----------------------------------------------------------------------===// + // Utility methods + //===----------------------------------------------------------------------===// + + bool isCaptured(const ParmVarDecl *Parameter) const { + if (const BlockDecl *Block = dyn_cast(AC.getDecl())) { + return Block->capturesVariable(Parameter); + } + return false; + } + + /// Return true if th analyzed function is actually a default implementation + /// of the method that has to be overriden. + /// + /// These functions can have tracked parameters, but wouldn't call them + /// because they are not designed to perform any meaningful actions. + /// + /// There are a couple of flavors of such default implementations: + /// 1. Empty methods or methods with a single return statement + /// 2. Methods that have one block with a call to no return function + /// 3. Methods with only assertion-like operations + bool isPossiblyEmptyImpl() const { + if (!isa(AC.getDecl())) { + // We care only about functions that are not supposed to be called. + // Only methods can be overriden. + return false; + } + + // Case #1 (without return statements) + if (FunctionCFG.size() == 2) { + // Method has only two blocks: ENTRY and EXIT. + // This is a equivalent to empty function. + return true; + } + + // Case #2 + if (FunctionCFG.size() == 3) { + const CFGBlock &Entry = FunctionCFG.getEntry(); + if (Entry.succ_empty()) { + return false; + } + + const CFGBlock *OnlyBlock = *Entry.succ_begin(); + // Method has only one block, let's see if it has a no-return + // element. + return OnlyBlock && OnlyBlock->hasNoReturnElement(); + } + + // Cases #1 (return statements) and #3. + // + // It is hard to detect that something is an assertion or came + // from assertion. Here we use a simple heuristic: + // + // - If it came from a macro, it can be an assertion. + // + // Additionally, we can't assume a number of basic blocks or the CFG's + // structure because assertions might include loops and conditions. + return llvm::all_of(FunctionCFG, [](const CFGBlock *BB) { + if (!BB) { + // Unreachable blocks are totally fine. + return true; + } + + // Return statements can have sub-expressions that are represented as + // separate statements of a basic block. We should allow this. + // This parent map will be initialized with a parent tree for all + // subexpressions of the block's return statement (if it has one). + std::unique_ptr ReturnChildren; + + return llvm::all_of( + llvm::reverse(*BB), // we should start with return statements, if we + // have any, i.e. from the bottom of the block + [&ReturnChildren](const CFGElement &Element) { + if (Optional S = Element.getAs()) { + const Stmt *SuspiciousStmt = S->getStmt(); + + if (isa(SuspiciousStmt)) { + // Let's initialize this structure to test whether + // some further statement is a part of this return. + ReturnChildren = std::make_unique( + const_cast(SuspiciousStmt)); + // Return statements are allowed as part of #1. + return true; + } + + return SuspiciousStmt->getBeginLoc().isMacroID() || + (ReturnChildren && + ReturnChildren->getParent(SuspiciousStmt)); + } + return true; + }); + }); + } + + /// Check if parameter with the given index was ever used. + /// NOTE: This function checks only for escapes. + bool wasEverUsed(unsigned Index) const { + return llvm::any_of(States, [Index](const State &StateForOneBB) { + return StateForOneBB.getKindFor(Index) == ParameterStatus::Escaped; + }); + } + + /// Return status stored for the given basic block. + /// \{ + State &getState(const CFGBlock *BB) { + assert(BB); + return States[BB->getBlockID()]; + } + const State &getState(const CFGBlock *BB) const { + assert(BB); + return States[BB->getBlockID()]; + } + /// \} + + /// Assign status to the given basic block. + /// + /// Returns true when the stored status changed. + bool assignState(const CFGBlock *BB, const State &ToAssign) { + State &Current = getState(BB); + if (Current == ToAssign) { + return false; + } + + Current = ToAssign; + return true; + } + + /// Join all incoming statuses for the given basic block. + State joinSuccessors(const CFGBlock *BB) const { + auto Succs = + llvm::make_filter_range(BB->succs(), [this](const CFGBlock *Succ) { + return Succ && this->getState(Succ).isVisited(); + }); + // We came to this block from somewhere after all. + assert(!Succs.empty() && + "Basic block should have at least one visited successor"); + + State Result = getState(*Succs.begin()); + + for (const CFGBlock *Succ : llvm::drop_begin(Succs, 1)) { + Result.join(getState(Succ)); + } + + if (const Expr *Condition = getCondition(BB->getTerminatorStmt())) { + handleConditional(BB, Condition, Result); + } + + return Result; + } + + void handleConditional(const CFGBlock *BB, const Expr *Condition, + State &ToAlter) const { + handleParameterCheck(BB, Condition, ToAlter); + handleConventionalCheck(BB, Condition, ToAlter); + } + + void handleParameterCheck(const CFGBlock *BB, const Expr *Condition, + State &ToAlter) const { + // In this function, we try to deal with the following pattern: + // + // if (parameter) + // parameter(...); + // + // It's not good to show a warning here because clearly 'parameter' + // couldn't and shouldn't be called on the 'else' path. + // + // Let's check if this if statement has a check involving one of + // the tracked parameters. + if (const ParmVarDecl *Parameter = findReferencedParmVarDecl( + Condition, + /* ShouldRetrieveFromComparisons = */ true)) { + if (const auto Index = getIndex(*Parameter)) { + ParameterStatus &CurrentStatus = ToAlter.getStatusFor(*Index); + + // We don't want to deep dive into semantics of the check and + // figure out if that check was for null or something else. + // We simply trust the user that they know what they are doing. + // + // For this reason, in the following loop we look for the + // best-looking option. + for (const CFGBlock *Succ : BB->succs()) { + if (!Succ) + continue; + + const ParameterStatus &StatusInSucc = + getState(Succ).getStatusFor(*Index); + + if (StatusInSucc.isErrorStatus()) { + continue; + } + + // Let's use this status instead. + CurrentStatus = StatusInSucc; + + if (StatusInSucc.getKind() == ParameterStatus::DefinitelyCalled) { + // This is the best option to have and we already found it. + break; + } + + // If we found 'Escaped' first, we still might find 'DefinitelyCalled' + // on the other branch. And we prefer the latter. + } + } + } + } + + void handleConventionalCheck(const CFGBlock *BB, const Expr *Condition, + State &ToAlter) const { + // Even when the analyzer is technically correct, it is a widespread pattern + // not to call completion handlers in some scenarios. These usually have + // typical conditional names, such as 'error' or 'cancel'. + if (!mentionsAnyOfConventionalNames(Condition)) { + return; + } + + for (const auto &IndexedStatus : llvm::enumerate(ToAlter)) { + const ParmVarDecl *Parameter = getParameter(IndexedStatus.index()); + // Conventions do not apply to explicitly marked parameters. + if (isExplicitlyMarked(Parameter)) { + continue; + } + + ParameterStatus &CurrentStatus = IndexedStatus.value(); + // If we did find that on one of the branches the user uses the callback + // and doesn't on the other path, we believe that they know what they are + // doing and trust them. + // + // There are two possible scenarios for that: + // 1. Current status is 'MaybeCalled' and one of the branches is + // 'DefinitelyCalled' + // 2. Current status is 'NotCalled' and one of the branches is 'Escaped' + if (isLosingCall(ToAlter, BB, IndexedStatus.index()) || + isLosingEscape(ToAlter, BB, IndexedStatus.index())) { + CurrentStatus = ParameterStatus::Escaped; + } + } + } + + bool isLosingCall(const State &StateAfterJoin, const CFGBlock *JoinBlock, + unsigned ParameterIndex) const { + // Let's check if the block represents DefinitelyCalled -> MaybeCalled + // transition. + return isLosingJoin(StateAfterJoin, JoinBlock, ParameterIndex, + ParameterStatus::MaybeCalled, + ParameterStatus::DefinitelyCalled); + } + + bool isLosingEscape(const State &StateAfterJoin, const CFGBlock *JoinBlock, + unsigned ParameterIndex) const { + // Let's check if the block represents Escaped -> NotCalled transition. + return isLosingJoin(StateAfterJoin, JoinBlock, ParameterIndex, + ParameterStatus::NotCalled, ParameterStatus::Escaped); + } + + bool isLosingJoin(const State &StateAfterJoin, const CFGBlock *JoinBlock, + unsigned ParameterIndex, ParameterStatus::Kind AfterJoin, + ParameterStatus::Kind BeforeJoin) const { + assert(!ParameterStatus::isErrorStatus(BeforeJoin) && + ParameterStatus::isErrorStatus(AfterJoin) && + "It's not a losing join if statuses do not represent " + "correct-to-error transition"); + + const ParameterStatus &CurrentStatus = + StateAfterJoin.getStatusFor(ParameterIndex); + + return CurrentStatus.getKind() == AfterJoin && + isAnyOfSuccessorsHasStatus(JoinBlock, ParameterIndex, BeforeJoin); + } + + /// Return true if any of the successors of the given basic block has + /// a specified status for the given parameter. + bool isAnyOfSuccessorsHasStatus(const CFGBlock *Parent, + unsigned ParameterIndex, + ParameterStatus::Kind ToFind) const { + return llvm::any_of( + Parent->succs(), [this, ParameterIndex, ToFind](const CFGBlock *Succ) { + return Succ && getState(Succ).getKindFor(ParameterIndex) == ToFind; + }); + } + + /// Check given expression that was discovered to escape. + void checkEscapee(const Expr *E) { + if (const ParmVarDecl *Parameter = findReferencedParmVarDecl(E)) { + checkEscapee(*Parameter); + } + } + + /// Check given parameter that was discovered to escape. + void checkEscapee(const ParmVarDecl &Parameter) { + if (auto Index = getIndex(Parameter)) { + ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(*Index); + + if (CurrentParamStatus.getKind() == ParameterStatus::NotCalled) { + CurrentParamStatus = ParameterStatus::Escaped; + } + } + } + + /// Mark all parameters in the current state as 'no-return'. + void markNoReturn() { + for (ParameterStatus &PS : CurrentState) { + PS = ParameterStatus::NoReturn; + } + } + +public: + //===----------------------------------------------------------------------===// + // Tree traversal methods + //===----------------------------------------------------------------------===// + + void VisitCallExpr(const CallExpr *Call) { + // This call might be a direct call, i.e. a parameter call... + checkDirectCall(Call); + // ... or an indirect call, i.e. when parameter is an argument. + checkIndirectCall(Call); + } + + void VisitObjCMessageExpr(const ObjCMessageExpr *Message) { + // The most common situation that we are defending against here is + // copying a tracked parameter. + if (const Expr *Receiver = Message->getInstanceReceiver()) { + checkEscapee(Receiver); + } + // Message expressions unlike calls, could not be direct. + checkIndirectCall(Message); + } + + void VisitBlockExpr(const BlockExpr *Block) { + for (const auto &Capture : Block->getBlockDecl()->captures()) { + // If a block captures a tracked parameter, it should be + // considered escaped. + // On one hand, blocks that do that should definitely call it on + // every path. However, it is not guaranteed that the block + // itself gets called whenever it gets created. + // + // Because we don't want to track blocks and whether they get called, + // we consider such parameters simply escaped. + if (const auto *Param = dyn_cast(Capture.getVariable())) { + checkEscapee(*Param); + } + } + } + + void VisitBinaryOperator(const BinaryOperator *Op) { + if (Op->getOpcode() == clang::BO_Assign) { + // Let's check if one of the tracked parameters is assigned into + // something, and if it is we don't want to track extra variables, so we + // consider it as an escapee. + checkEscapee(Op->getRHS()); + } + } + + void VisitDeclStmt(const DeclStmt *DS) { + // Variable initialization is not assignment and should be handled + // separately. + // + // Multiple declarations can be a part of declaration statement. + for (const auto *Declaration : DS->getDeclGroup()) { + if (const auto *Var = dyn_cast(Declaration)) { + if (Var->getInit()) { + checkEscapee(Var->getInit()); + } + } + } + } + + void VisitCStyleCastExpr(const CStyleCastExpr *Cast) { + // We consider '(void)parameter' as a manual no-op escape. + // It should be used to explicitly tell the analysis that this paramater + // is intentionally not called on this path. + if (Cast->getType() == AC.getASTContext().VoidTy) { + checkEscapee(Cast->getSubExpr()); + } + } + + void VisitObjCAtThrowStmt(const ObjCAtThrowStmt *) { + // It is OK not to call marked parameters on exceptional paths. + markNoReturn(); + } + +private: + unsigned size() const { return TrackedParams.size(); } + + llvm::Optional getIndexOfCallee(const CallExpr *Call) const { + return getIndexOfExpression(Call->getCallee()); + } + + llvm::Optional getIndexOfExpression(const Expr *E) const { + if (const ParmVarDecl *Parameter = findReferencedParmVarDecl(E)) { + return getIndex(*Parameter); + } + + return llvm::None; + } + + llvm::Optional getIndex(const ParmVarDecl &Parameter) const { + // Expected number of parameters that we actually track is 1. + // + // Also, the maximum number of declared parameters could not be on a scale + // of hundreds of thousands. + // + // In this setting, linear search seems reasonable and even performs better + // than bisection. + ParamSizedVector::const_iterator It = + llvm::find(TrackedParams, &Parameter); + + if (It != TrackedParams.end()) { + return It - TrackedParams.begin(); + } + + return llvm::None; + } + + const ParmVarDecl *getParameter(unsigned Index) const { + assert(Index < TrackedParams.size()); + return TrackedParams[Index]; + } + + const CFG &FunctionCFG; + AnalysisDeclContext &AC; + CalledOnceCheckHandler &Handler; + bool CheckConventionalParameters; + + State CurrentState; + ParamSizedVector TrackedParams; + CFGSizedVector States; +}; + +} // end anonymous namespace + +namespace clang { +void checkCalledOnceParameters(const CFG &FunctionCFG, AnalysisDeclContext &AC, + CalledOnceCheckHandler &Handler, + bool CheckConventionalParameters) { + CalledOnceChecker::check(FunctionCFG, AC, Handler, + CheckConventionalParameters); +} +} // end namespace clang diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -24,6 +24,7 @@ #include "clang/AST/StmtObjC.h" #include "clang/AST/StmtVisitor.h" #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h" +#include "clang/Analysis/Analyses/CalledOnceCheck.h" #include "clang/Analysis/Analyses/Consumed.h" #include "clang/Analysis/Analyses/ReachableCode.h" #include "clang/Analysis/Analyses/ThreadSafety.h" @@ -36,6 +37,7 @@ #include "clang/Lex/Preprocessor.h" #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaInternal.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/BitVector.h" #include "llvm/ADT/MapVector.h" #include "llvm/ADT/SmallString.h" @@ -1623,6 +1625,81 @@ }); } }; + +class CalledOnceCheckReporter : public CalledOnceCheckHandler { +public: + CalledOnceCheckReporter(Sema &S) : S(S) {} + void handleDoubleCall(const ParmVarDecl *Parameter, const Expr *Call, + const Expr *PrevCall, + bool IsCompletionHandler) override { + auto DiagToReport = IsCompletionHandler + ? diag::warn_completion_handler_called_twice + : diag::warn_called_once_gets_called_twice; + S.Diag(Call->getBeginLoc(), DiagToReport) << Parameter; + S.Diag(PrevCall->getBeginLoc(), diag::note_called_once_gets_called_twice); + } + + void handleNeverCalled(const ParmVarDecl *Parameter, + bool IsCompletionHandler) override { + auto DiagToReport = IsCompletionHandler + ? diag::warn_completion_handler_never_called + : diag::warn_called_once_never_called; + S.Diag(Parameter->getBeginLoc(), DiagToReport) + << Parameter << /* Captured */ false; + } + + void handleNeverCalled(const ParmVarDecl *Parameter, const Stmt *Where, + NeverCalledReason Reason, bool IsCalledDirectly, + bool IsCompletionHandler) override { + auto DiagToReport = IsCompletionHandler + ? diag::warn_completion_handler_never_called_when + : diag::warn_called_once_never_called_when; + S.Diag(Where->getBeginLoc(), DiagToReport) + << Parameter << IsCalledDirectly << (unsigned)Reason; + } + + void handleCapturedNeverCalled(const ParmVarDecl *Parameter, + const Decl *Where, + bool IsCompletionHandler) override { + auto DiagToReport = IsCompletionHandler + ? diag::warn_completion_handler_never_called + : diag::warn_called_once_never_called; + S.Diag(Where->getBeginLoc(), DiagToReport) + << Parameter << /* Captured */ true; + } + +private: + Sema &S; +}; + +constexpr unsigned CalledOnceWarnings[] = { + diag::warn_called_once_never_called, + diag::warn_called_once_never_called_when, + diag::warn_called_once_gets_called_twice}; + +constexpr unsigned CompletionHandlerWarnings[]{ + diag::warn_completion_handler_never_called, + diag::warn_completion_handler_never_called_when, + diag::warn_completion_handler_called_twice}; + +bool shouldAnalyzeCalledOnceImpl(llvm::ArrayRef DiagIDs, + const DiagnosticsEngine &Diags, + SourceLocation At) { + return llvm::any_of(DiagIDs, [&Diags, At](unsigned DiagID) { + return !Diags.isIgnored(DiagID, At); + }); +} + +bool shouldAnalyzeCalledOnceConventions(const DiagnosticsEngine &Diags, + SourceLocation At) { + return shouldAnalyzeCalledOnceImpl(CompletionHandlerWarnings, Diags, At); +} + +bool shouldAnalyzeCalledOnceParameters(const DiagnosticsEngine &Diags, + SourceLocation At) { + return shouldAnalyzeCalledOnceImpl(CalledOnceWarnings, Diags, At) || + shouldAnalyzeCalledOnceConventions(Diags, At); +} } // anonymous namespace namespace clang { @@ -2264,6 +2341,17 @@ } } + // Check for violations of "called once" parameter properties. + if (S.getLangOpts().ObjC && + shouldAnalyzeCalledOnceParameters(Diags, D->getBeginLoc())) { + if (CFG *FunctionCFG = AC.getCFG()) { + CalledOnceCheckReporter Reporter(S); + checkCalledOnceParameters( + *FunctionCFG, AC, Reporter, + shouldAnalyzeCalledOnceConventions(Diags, D->getBeginLoc())); + } + } + bool FallThroughDiagFull = !Diags.isIgnored(diag::warn_unannotated_fallthrough, D->getBeginLoc()); bool FallThroughDiagPerFunction = !Diags.isIgnored( diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3611,6 +3611,29 @@ S.Context, AL, EncodingIndices.data(), EncodingIndices.size())); } +static bool isFunctionLike(const Type &T) { + // Check for explicit function types. + // 'called_once' is only supported in Objective-C and it has + // function pointers and block pointers. + return T.isFunctionPointerType() || T.isBlockPointerType(); +} + +/// Handle 'called_once' attribute. +static void handleCalledOnceAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + if (D->isInvalidDecl()) + return; + + // 'called_once' only applies to parameters representing functions. + QualType T = cast(D)->getType(); + + if (!isFunctionLike(*T)) { + S.Diag(AL.getLoc(), diag::err_called_once_attribute_wrong_type); + return; + } + + D->addAttr(::new (S.Context) CalledOnceAttr(S.Context, AL)); +} + static void handleTransparentUnionAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // Try to find the underlying union declaration. RecordDecl *RD = nullptr; @@ -7474,6 +7497,9 @@ case ParsedAttr::AT_Callback: handleCallbackAttr(S, D, AL); break; + case ParsedAttr::AT_CalledOnce: + handleCalledOnceAttr(S, D, AL); + break; case ParsedAttr::AT_CUDAGlobal: handleGlobalAttr(S, D, AL); break; diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -15260,10 +15260,6 @@ PopDeclContext(); - // Pop the block scope now but keep it alive to the end of this function. - AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy(); - PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(&WP, BD, BlockTy); - // Set the captured variables on the block. SmallVector Captures; for (Capture &Cap : BSI->Captures) { @@ -15331,6 +15327,10 @@ } BD->setCaptures(Context, Captures, BSI->CXXThisCaptureIndex != 0); + // Pop the block scope now but keep it alive to the end of this function. + AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy(); + PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(&WP, BD, BlockTy); + BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy); // If the block isn't obviously global, i.e. it captures anything at diff --git a/clang/test/SemaObjC/attr-called-once.m b/clang/test/SemaObjC/attr-called-once.m new file mode 100644 --- /dev/null +++ b/clang/test/SemaObjC/attr-called-once.m @@ -0,0 +1,9 @@ +// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc -fblocks %s + +#define CALLED_ONCE __attribute__((called_once)) + +void test1(int x CALLED_ONCE); // expected-error{{'called_once' attribute only applies to function-like parameters}} +void test2(double x CALLED_ONCE); // expected-error{{'called_once' attribute only applies to function-like parameters}} + +void test3(void (*foo)() CALLED_ONCE); // no-error +void test4(int (^foo)(int) CALLED_ONCE); // no-error diff --git a/clang/test/SemaObjC/warn-called-once.m b/clang/test/SemaObjC/warn-called-once.m new file mode 100644 --- /dev/null +++ b/clang/test/SemaObjC/warn-called-once.m @@ -0,0 +1,887 @@ +// RUN: %clang_cc1 -verify -fsyntax-only -fblocks -fobjc-exceptions -Wcompletion-handler %s + +#define NULL (void *)0 +#define CALLED_ONCE __attribute__((called_once)) +#define NORETURN __attribute__((noreturn)) + +@protocol NSObject +@end +@interface NSObject +- (id)copy; +- (id)class; +- autorelease; +@end + +typedef unsigned int NSUInteger; +typedef struct { +} NSFastEnumerationState; + +@interface NSArray <__covariant NSFastEnumeration> +- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state objects:(id *)stackbuf count:(NSUInteger)len; +@end +@interface NSMutableArray : NSArray +- addObject:anObject; +@end +@class NSString, Protocol; +extern void NSLog(NSString *format, ...); + +void escape(void (^callback)(void)); +void escape_void(void *); +void indirect_call(void (^callback)(void) CALLED_ONCE); +void indirect_conv(void (^completionHandler)(void)); +void filler(void); +void exit(int) NORETURN; + +void double_call_one_block(void (^callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void double_call_one_block_parens(void (^callback)(void) CALLED_ONCE) { + (callback)(); // expected-note{{previous call is here}} + (callback)(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void double_call_one_block_ptr(void (*callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void double_call_one_block_ptr_deref(void (*callback)(void) CALLED_ONCE) { + (*callback)(); // expected-note{{previous call is here}} + (*callback)(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void multiple_call_one_block(void (^callback)(void) CALLED_ONCE) { + // We don't really need to repeat the same warning for the same parameter. + callback(); // no-warning + callback(); // no-warning + callback(); // no-warning + callback(); // expected-note{{previous call is here}} + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void double_call_branching_1(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + callback(); // expected-note{{previous call is here}} + } else { + cond += 42; + } + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void double_call_branching_2(int cond, void (^callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + + if (cond) { + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} + } else { + cond += 42; + } +} + +void double_call_branching_3(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + callback(); + } else { + callback(); + } + // no-warning +} + +void double_call_branching_4(int cond1, int cond2, void (^callback)(void) CALLED_ONCE) { + if (cond1) { + cond2 = !cond2; + } else { + callback(); // expected-note{{previous call is here}} + } + + if (cond2) { + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} + } +} + +void double_call_loop(int counter, void (^callback)(void) CALLED_ONCE) { + while (counter > 0) { + counter--; + // Both note and warning are on the same line, which is a common situation + // in loops. + callback(); // expected-note{{previous call is here}} + // expected-warning@-1{{'callback' parameter marked 'called_once' is called twice}} + } +} + +void never_called_trivial(void (^callback)(void) CALLED_ONCE) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called}} +} + +int never_called_branching(int x, void (^callback)(void) CALLED_ONCE) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called}} + x -= 42; + + if (x == 10) { + return 0; + } + + return x + 15; +} + +void escaped_one_block_1(void (^callback)(void) CALLED_ONCE) { + escape(callback); // no-warning +} + +void escaped_one_block_2(void (^callback)(void) CALLED_ONCE) { + escape(callback); // no-warning + callback(); +} + +void escaped_one_path_1(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + escape(callback); // no-warning + } else { + callback(); + } +} + +void escaped_one_path_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + escape(callback); // no-warning + } + + callback(); +} + +void escaped_one_path_3(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never used when taking false branch}} + escape(callback); + } +} + +void escape_in_between_1(void (^callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + escape(callback); + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void escape_in_between_2(int cond, void (^callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + if (cond) { + escape(callback); + } + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void escape_in_between_3(int cond, void (^callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + + if (cond) { + escape(callback); + } else { + escape_void((__bridge void *)callback); + } + + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void escaped_as_void_ptr(void (^callback)(void) CALLED_ONCE) { + escape_void((__bridge void *)callback); // no-warning +} + +void indirect_call_no_warning_1(void (^callback)(void) CALLED_ONCE) { + indirect_call(callback); // no-warning +} + +void indirect_call_no_warning_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + indirect_call(callback); + } else { + callback(); + } + // no-warning +} + +void indirect_call_double_call(void (^callback)(void) CALLED_ONCE) { + indirect_call(callback); // expected-note{{previous call is here}} + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void indirect_call_within_direct_call(void (^callback)(void) CALLED_ONCE, + void (^meta)(void (^param)(void) CALLED_ONCE) CALLED_ONCE) { + // TODO: Report warning for 'callback'. + // At the moment, it is not possible to access 'called_once' attribute from the type + // alone when there is no actual declaration of the marked parameter. + meta(callback); + callback(); + // no-warning +} + +void block_call_1(void (^callback)(void) CALLED_ONCE) { + indirect_call(^{ + callback(); + }); + callback(); + // no-warning +} + +void block_call_2(void (^callback)(void) CALLED_ONCE) { + escape(^{ + callback(); + }); + callback(); + // no-warning +} + +void block_call_3(int cond, void (^callback)(void) CALLED_ONCE) { + ^{ + if (cond) { + callback(); // expected-note{{previous call is here}} + } + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} + }(); // no-warning +} + +void block_call_4(int cond, void (^callback)(void) CALLED_ONCE) { + ^{ + if (cond) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never used when taking false branch}} + escape(callback); + } + }(); +} + +void block_call_5(void (^outer)(void) CALLED_ONCE) { + ^(void (^inner)(void) CALLED_ONCE) { + // expected-warning@-1{{'inner' parameter marked 'called_once' is never called}} + }(outer); +} + +void block_with_called_once(void (^outer)(void) CALLED_ONCE) { + escape_void((__bridge void *)^(void (^inner)(void) CALLED_ONCE) { + inner(); // expected-note{{previous call is here}} + inner(); // expected-warning{{'inner' parameter marked 'called_once' is called twice}} + }); + outer(); // expected-note{{previous call is here}} + outer(); // expected-warning{{'outer' parameter marked 'called_once' is called twice}} +} + +void never_called_one_exit(int cond, void (^callback)(void) CALLED_ONCE) { + if (!cond) // expected-warning{{'callback' parameter marked 'called_once' is never called when taking true branch}} + return; + + callback(); +} + +void never_called_if_then_1(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking true branch}} + } else { + callback(); + } +} + +void never_called_if_then_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking true branch}} + // This way the first statement in the basic block is different from + // the first statement in the compound statement + (void)cond; + } else { + callback(); + } +} + +void never_called_if_else_1(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking false branch}} + callback(); + } else { + } +} + +void never_called_if_else_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking false branch}} + callback(); + } +} + +void never_called_two_ifs(int cond1, int cond2, void (^callback)(void) CALLED_ONCE) { + if (cond1) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking false branch}} + if (cond2) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking true branch}} + return; + } + callback(); + } +} + +void never_called_ternary_then(int cond, void (^other)(void), void (^callback)(void) CALLED_ONCE) { + return cond ? // expected-warning{{'callback' parameter marked 'called_once' is never called when taking true branch}} + other() + : callback(); +} + +void never_called_for_false(int size, void (^callback)(void) CALLED_ONCE) { + for (int i = 0; i < size; ++i) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called when skipping the loop}} + callback(); + break; + } +} + +void never_called_for_true(int size, void (^callback)(void) CALLED_ONCE) { + for (int i = 0; i < size; ++i) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called when entering the loop}} + return; + } + callback(); +} + +void never_called_while_false(int cond, void (^callback)(void) CALLED_ONCE) { + while (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when skipping the loop}} + callback(); + break; + } +} + +void never_called_while_true(int cond, void (^callback)(void) CALLED_ONCE) { + while (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when entering the loop}} + return; + } + callback(); +} + +void never_called_switch_case(int cond, void (^callback)(void) CALLED_ONCE) { + switch (cond) { + case 1: + callback(); + break; + case 2: + callback(); + break; + case 3: // expected-warning{{'callback' parameter marked 'called_once' is never called when handling this case}} + break; + default: + callback(); + break; + } +} + +void never_called_switch_default(int cond, void (^callback)(void) CALLED_ONCE) { + switch (cond) { + case 1: + callback(); + break; + case 2: + callback(); + break; + default: // expected-warning{{'callback' parameter marked 'called_once' is never called when handling this case}} + break; + } +} + +void never_called_switch_two_cases(int cond, void (^callback)(void) CALLED_ONCE) { + switch (cond) { + case 1: // expected-warning{{'callback' parameter marked 'called_once' is never called when handling this case}} + break; + case 2: // expected-warning{{'callback' parameter marked 'called_once' is never called when handling this case}} + break; + default: + callback(); + break; + } +} + +void never_called_switch_none(int cond, void (^callback)(void) CALLED_ONCE) { + switch (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when none of the cases applies}} + case 1: + callback(); + break; + case 2: + callback(); + break; + } +} + +enum YesNoOrMaybe { + YES, + NO, + MAYBE +}; + +void exhaustive_switch(enum YesNoOrMaybe cond, void (^callback)(void) CALLED_ONCE) { + switch (cond) { + case YES: + callback(); + break; + case NO: + callback(); + break; + case MAYBE: + callback(); + break; + } + // no-warning +} + +void called_twice_exceptions(void (^callback)(void) CALLED_ONCE) { + // TODO: Obj-C exceptions are not supported in CFG, + // we should report warnings in these as well. + @try { + callback(); + callback(); + } + @finally { + callback(); + } +} + +void noreturn_1(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + exit(1); + } else { + callback(); + } + // no-warning +} + +void noreturn_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + callback(); + exit(1); + } else { + callback(); + } + // no-warning +} + +void noreturn_3(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + exit(1); + } + + callback(); + // no-warning +} + +void noreturn_4(void (^callback)(void) CALLED_ONCE) { + exit(1); + // no-warning +} + +void noreturn_5(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + // NOTE: This is an ambiguous case caused by the fact that we do a backward + // analysis. We can probably report it here, but for the sake of + // the simplicity of our analysis, we don't. + if (cond == 42) { + callback(); + } + exit(1); + } + callback(); + // no-warning +} + +void never_called_noreturn_1(int cond, void (^callback)(void) CALLED_ONCE) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called}} + if (cond) { + exit(1); + } +} + +void double_call_noreturn(int cond, void (^callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + + if (cond) { + if (cond == 42) { + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} + } + exit(1); + } +} + +void call_with_check_1(void (^callback)(void) CALLED_ONCE) { + if (callback) + callback(); + // no-warning +} + +void call_with_check_2(void (^callback)(void) CALLED_ONCE) { + if (!callback) { + } else { + callback(); + } + // no-warning +} + +void call_with_check_3(void (^callback)(void) CALLED_ONCE) { + if (callback != NULL) + callback(); + // no-warning +} + +void call_with_check_4(void (^callback)(void) CALLED_ONCE) { + if (NULL != callback) + callback(); + // no-warning +} + +void call_with_check_5(void (^callback)(void) CALLED_ONCE) { + if (callback == NULL) { + } else { + callback(); + } + // no-warning +} + +void call_with_check_6(void (^callback)(void) CALLED_ONCE) { + if (NULL == callback) { + } else { + callback(); + } + // no-warning +} + +int call_with_check_7(int (^callback)(void) CALLED_ONCE) { + return callback ? callback() : 0; + // no-warning +} + +void unreachable_true_branch(void (^callback)(void) CALLED_ONCE) { + if (0) { + + } else { + callback(); + } + // no-warning +} + +void unreachable_false_branch(void (^callback)(void) CALLED_ONCE) { + if (1) { + callback(); + } + // no-warning +} + +void never_called_conv(void (^completionHandler)(void)) { + // expected-warning@-1{{completion handler is never called}} +} + +void indirectly_called_conv(void (^completionHandler)(void)) { + indirect_conv(completionHandler); + // no-warning +} + +void escape_through_assignment_1(void (^callback)(void) CALLED_ONCE) { + id escapee; + escapee = callback; + escape(escapee); + // no-warning +} + +void escape_through_assignment_2(void (^callback)(void) CALLED_ONCE) { + id escapee = callback; + escape(escapee); + // no-warning +} + +void escape_through_assignment_3(void (^callback1)(void) CALLED_ONCE, + void (^callback2)(void) CALLED_ONCE) { + id escapee1 = callback1, escapee2 = callback2; + escape(escapee1); + escape(escapee2); + // no-warning +} + +void not_called_in_throw_branch_1(id exception, void (^callback)(void) CALLED_ONCE) { + if (exception) { + @throw exception; + } + + callback(); +} + +void not_called_in_throw_branch_2(id exception, void (^callback)(void) CALLED_ONCE) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called}} + if (exception) { + @throw exception; + } +} + +void conventional_error_path_1(int error, void (^completionHandler)(void)) { + if (error) { + // no-warning + return; + } + + completionHandler(); +} + +void conventional_error_path_2(int error, void (^callback)(void) CALLED_ONCE) { + // Conventions do not apply to explicitly marked parameters. + if (error) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called when taking true branch}} + return; + } + + callback(); +} + +void suppression_1(void (^callback)(void) CALLED_ONCE) { + // This is a way to tell the analysis that we know about this path, + // and we do not want to call the callback here. + (void)callback; // no-warning +} + +void suppression_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + (void)callback; // no-warning + } else { + callback(); + } +} + +void suppression_3(int cond, void (^callback)(void) CALLED_ONCE) { + // Even if we do this on one of the paths, it doesn't mean we should + // forget about other paths. + if (cond) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never used when taking false branch}} + (void)callback; + } +} + +@interface TestBase : NSObject +- (void)escape:(void (^)(void))callback; +- (void)indirect_call:(void (^)(void))CALLED_ONCE callback; +- (void)indirect_call_conv_1:(int)cond + completionHandler:(void (^)(void))completionHandler; +- (void)indirect_call_conv_2:(int)cond + completionHandler:(void (^)(void))handler; +- (void)exit:(int)code NORETURN; +@end + +@interface TestClass : TestBase +@property(strong) NSMutableArray *handlers; +@property(strong) id storedHandler; +@property int wasCanceled; +@property(getter=hasErrors) int error; +@end + +@implementation TestClass + +- (void)double_indirect_call_1:(void (^)(void))CALLED_ONCE callback { + [self indirect_call:callback]; // expected-note{{previous call is here}} + [self indirect_call:callback]; // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +- (void)double_indirect_call_2:(void (^)(void))CALLED_ONCE callback { + [self indirect_call_conv_1:0 // expected-note{{previous call is here}} + completionHandler:callback]; + [self indirect_call_conv_1:1 // expected-warning{{'callback' parameter marked 'called_once' is called twice}} + completionHandler:callback]; +} + +- (void)double_indirect_call_3:(void (^)(void))completionHandler { + [self indirect_call_conv_2:0 // expected-note{{previous call is here}} + completionHandler:completionHandler]; + [self indirect_call_conv_2:1 // expected-warning{{completion handler is called twice}} + completionHandler:completionHandler]; +} + +- (void)never_called_trivial:(void (^)(void))CALLED_ONCE callback { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called}} + filler(); +} + +- (void)noreturn:(int)cond callback:(void (^)(void))CALLED_ONCE callback { + if (cond) { + [self exit:1]; + } + + callback(); + // no-warning +} + +- (void)escaped_one_path:(int)cond callback:(void (^)(void))CALLED_ONCE callback { + if (cond) { + [self escape:callback]; // no-warning + } else { + callback(); + } +} + +- (void)block_call_1:(void (^)(void))CALLED_ONCE callback { + // We consider captures by blocks as escapes + [self indirect_call:(^{ + callback(); + })]; + callback(); + // no-warning +} + +- (void)block_call_2:(int)cond callback:(void (^)(void))CALLED_ONCE callback { + [self indirect_call: + ^{ + if (cond) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never used when taking false branch}} + [self escape:callback]; + } + }]; +} + +- (void)never_called_conv:(void (^)(void))completionHandler { + // expected-warning@-1{{completion handler is never called}} + filler(); +} + +- (void)indirectly_called_conv:(void (^)(void))completionHandler { + indirect_conv(completionHandler); + // no-warning +} + +- (void)never_called_one_exit_conv:(int)cond completionHandler:(void (^)(void))handler { + if (!cond) // expected-warning{{completion handler is never called when taking true branch}} + return; + + handler(); +} + +- (void)escape_through_assignment:(void (^)(void))completionHandler { + _storedHandler = completionHandler; + // no-warning +} + +- (void)escape_through_copy:(void (^)(void))completionHandler { + _storedHandler = [completionHandler copy]; + // no-warning +} + +- (void)escape_through_copy_and_autorelease:(void (^)(void))completionHandler { + _storedHandler = [[completionHandler copy] autorelease]; + // no-warning +} + +- (void)complex_escape:(void (^)(void))completionHandler { + if (completionHandler) { + [_handlers addObject:[[completionHandler copy] autorelease]]; + } + // no-warning +} + +- (void)test_crash:(void (^)(void))completionHandler cond:(int)cond { + if (cond) { + // expected-warning@-1{{completion handler is never used when taking false branch}} + for (id _ in _handlers) { + } + + [_handlers addObject:completionHandler]; + } +} + +- (void)conventional_error_path_1:(void (^)(void))completionHandler { + if (self.wasCanceled) + // no-warning + return; + + completionHandler(); +} + +- (void)conventional_error_path_2:(void (^)(void))completionHandler { + if (self.wasCanceled) + // no-warning + return; + + [_handlers addObject:completionHandler]; +} + +- (void)conventional_error_path_3:(void (^)(void))completionHandler { + if (self.hasErrors) + // no-warning + return; + + completionHandler(); +} + +- (void)conventional_error_path_3:(int)cond completionHandler:(void (^)(void))handler { + if (self.wasCanceled) + // expected-warning@-1{{completion handler is never called when taking true branch}} + // TODO: When we have an error on some other path, in order not to prevent it from + // being reported, we report this one as well. + // Probably, we should address this at some point. + return; + + if (cond) { + // expected-warning@-1{{completion handler is never called when taking false branch}} + handler(); + } +} + +#define NSAssert(condition, desc, ...) NSLog(desc, ##__VA_ARGS__); + +- (void)empty_base_1:(void (^)(void))completionHandler { + NSAssert(0, @"Subclass must implement"); + // no-warning +} + +- (void)empty_base_2:(void (^)(void))completionHandler { + // no-warning +} + +- (int)empty_base_3:(void (^)(void))completionHandler { + return 1; + // no-warning +} + +- (int)empty_base_4:(void (^)(void))completionHandler { + NSAssert(0, @"Subclass must implement"); + return 1; + // no-warning +} + +- (int)empty_base_5:(void (^)(void))completionHandler { + NSAssert(0, @"%@ doesn't support", [self class]); + return 1; + // no-warning +} + +#undef NSAssert +#define NSAssert(condition, desc, ...) \ + if (!(condition)) { \ + NSLog(desc, ##__VA_ARGS__); \ + } + +- (int)empty_base_6:(void (^)(void))completionHandler { + NSAssert(0, @"%@ doesn't support", [self class]); + return 1; + // no-warning +} + +#undef NSAssert +#define NSAssert(condition, desc, ...) \ + do { \ + NSLog(desc, ##__VA_ARGS__); \ + } while (0) + +- (int)empty_base_7:(void (^)(void))completionHandler { + NSAssert(0, @"%@ doesn't support", [self class]); + return 1; + // no-warning +} + +- (void)two_conditions_1:(int)first + second:(int)second + completionHandler:(void (^)(void))completionHandler { + if (first && second) { + // expected-warning@-1{{completion handler is never called when taking false branch}} + completionHandler(); + } +} + +- (void)two_conditions_2:(int)first + second:(int)second + completionHandler:(void (^)(void))completionHandler { + if (first || second) { + // expected-warning@-1{{completion handler is never called when taking true branch}} + return; + } + + completionHandler(); +} +@end