[llvm-branch-commits] [clang] fec1a44 - [-Wcalled-once-parameter] Introduce 'called_once' attribute
Valeriy Savchenko via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Jan 5 07:33:10 PST 2021
Author: Valeriy Savchenko
Date: 2021-01-05T18:26:44+03:00
New Revision: fec1a442e3b12fc01929b2b405d1abf7df9ab68e
URL: https://github.com/llvm/llvm-project/commit/fec1a442e3b12fc01929b2b405d1abf7df9ab68e
DIFF: https://github.com/llvm/llvm-project/commit/fec1a442e3b12fc01929b2b405d1abf7df9ab68e.diff
LOG: [-Wcalled-once-parameter] Introduce 'called_once' attribute
This commit introduces a new attribute `called_once`.
It can be applied to function-like parameters to signify that
this parameter should be called exactly once. This concept
is particularly widespread in asynchronous programs.
Additionally, this commit introduce a new group of dataflow
analysis-based warnings to check this property. It identifies
and reports the following situations:
* parameter is called twice
* parameter is never called
* parameter is not called on one of the paths
Current implementation can also automatically infer `called_once`
attribute for completion handler paramaters that should follow the
same principle by convention. This behavior is OFF by default and
can be turned on by using `-Wcompletion-handler`.
Differential Revision: https://reviews.llvm.org/D92039
rdar://72812043
Added:
clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
clang/lib/Analysis/CalledOnceCheck.cpp
clang/test/SemaObjC/attr-called-once.m
clang/test/SemaObjC/warn-called-once.m
Modified:
clang/include/clang/AST/ParentMap.h
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/CMakeLists.txt
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test
Removed:
################################################################################
diff --git a/clang/include/clang/AST/ParentMap.h b/clang/include/clang/AST/ParentMap.h
index 1e65d7efd272..86e2f048a344 100644
--- a/clang/include/clang/AST/ParentMap.h
+++ b/clang/include/clang/AST/ParentMap.h
@@ -51,9 +51,7 @@ class ParentMap {
return getParentIgnoreParenCasts(const_cast<Stmt*>(S));
}
- bool hasParent(Stmt* S) const {
- return getParent(S) != nullptr;
- }
+ bool hasParent(const Stmt *S) const { return getParent(S) != nullptr; }
bool isConsumedExpr(Expr *E) const;
diff --git a/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h b/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
new file mode 100644
index 000000000000..fc574c680a44
--- /dev/null
+++ b/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
@@ -0,0 +1,112 @@
+//===- 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.
+ /// \param Poised -- true, if the second call is guaranteed to happen after
+ /// the first call.
+ virtual void handleDoubleCall(const ParmVarDecl *Parameter, const Expr *Call,
+ const Expr *PrevCall, bool IsCompletionHandler,
+ bool Poised) {}
+
+ /// 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 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(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
index ba6c459f4a43..b84e6a14f371 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1798,6 +1798,13 @@ def ReturnsNonNull : InheritableAttr {
let Documentation = [ReturnsNonNullDocs];
}
+def CalledOnce : Attr {
+ let Spellings = [Clang<"called_once">];
+ let Subjects = SubjectList<[ParmVar]>;
+ let LangOpts = [ObjC];
+ let Documentation = [CalledOnceDocs];
+}
+
// 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/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 6f47ca505b5e..9cf0c59e07bb 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -5162,6 +5162,59 @@ in the future.
}];
}
+def CalledOnceDocs : Documentation {
+ let Category = DocCatVariable;
+ let Content = [{
+The ``called_once`` attribute specifies that the annotated function or method
+parameter is invoked exactly once on all execution paths. It only applies
+to parameters with function-like types, i.e. function pointers or blocks. This
+concept is particularly useful for asynchronous programs.
+
+Clang implements a check for ``called_once`` parameters,
+``-Wcalled-once-parameter``. It is on by default and finds the following
+violations:
+
+* Parameter is not called at all.
+
+* Parameter is called more than once.
+
+* Parameter is not called on one of the execution paths.
+
+In the latter case, Clang pinpoints the path where parameter is not invoked
+by showing the control-flow statement where the path diverges.
+
+.. code-block:: objc
+
+ void fooWithCallback(void (^callback)(void) __attribute__((called_once))) {
+ if (somePredicate()) {
+ ...
+ callback();
+ } esle {
+ callback(); // OK: callback is called on every path
+ }
+ }
+
+ void barWithCallback(void (^callback)(void) __attribute__((called_once))) {
+ if (somePredicate()) {
+ ...
+ callback(); // note: previous call is here
+ }
+ callback(); // warning: callback is called twice
+ }
+
+ void foobarWithCallback(void (^callback)(void) __attribute__((called_once))) {
+ if (somePredicate()) { // warning: callback is not called when condition is false
+ ...
+ callback();
+ }
+ }
+
+This attribute is useful for API developers who want to double-check if they
+implemented their method correctly.
+
+ }];
+}
+
def GnuInlineDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 82fa97ec5fb5..d500ab321058 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -487,6 +487,8 @@ def ObjCPointerIntrospect : DiagGroup<"deprecated-objc-pointer-introspection", [
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
index 24c2bb57b6f9..0405195912b2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3953,6 +3953,40 @@ def err_attribute_preferred_name_arg_invalid : Error<
"argument %0 to 'preferred_name' attribute is not a typedef for "
"a specialization of %1">;
+// 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<CompletionHandler>, DefaultIgnore;
+def warn_called_once_never_called : Warning<
+ "%select{|captured }1%0 parameter marked 'called_once' is never called">,
+ InGroup<CalledOnceParameter>;
+
+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<CompletionHandler>, 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<CalledOnceParameter>;
+
+def warn_completion_handler_called_twice : Warning<
+ "completion handler is called twice">,
+ InGroup<CompletionHandler>, DefaultIgnore;
+def warn_called_once_gets_called_twice : Warning<
+ "%0 parameter marked 'called_once' is called twice">,
+ InGroup<CalledOnceParameter>;
+def note_called_once_gets_called_twice : Note<
+ "previous call is here%select{; set to nil to indicate "
+ "it cannot be called afterwards|}0">;
+
// 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
index 6cad671a8a4d..ed626a6e130c 100644
--- a/clang/lib/Analysis/CMakeLists.txt
+++ b/clang/lib/Analysis/CMakeLists.txt
@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
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
index 000000000000..2eff97640dfa
--- /dev/null
+++ b/clang/lib/Analysis/CalledOnceCheck.cpp
@@ -0,0 +1,1524 @@
+//===- 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/DeclBase.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/AST/Type.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 <memory>
+
+using namespace clang;
+
+namespace {
+static constexpr unsigned EXPECTED_MAX_NUMBER_OF_PARAMS = 2;
+template <class T>
+using ParamSizedVector = llvm::SmallVector<T, EXPECTED_MAX_NUMBER_OF_PARAMS>;
+static constexpr unsigned EXPECTED_NUMBER_OF_BASIC_BLOCKS = 8;
+template <class T>
+using CFGSizedVector = llvm::SmallVector<T, EXPECTED_NUMBER_OF_BASIC_BLOCKS>;
+constexpr llvm::StringLiteral CONVENTIONAL_NAMES[] = {
+ "completionHandler", "completion", "withCompletionHandler"};
+constexpr llvm::StringLiteral CONVENTIONAL_SUFFIXES[] = {
+ "WithCompletionHandler", "WithCompletion"};
+constexpr llvm::StringLiteral CONVENTIONAL_CONDITIONS[] = {
+ "error", "cancel", "shouldCall", "done", "OK", "success"};
+
+class ParameterStatus {
+public:
+ // Status kind is basically the main part of parameter's status.
+ // The kind represents our knowledge (so far) about a tracked parameter
+ // in the context of this analysis.
+ //
+ // Since we want to report on missing and extraneous calls, we need to
+ // track the fact whether paramater was called or not. This automatically
+ // decides two kinds: `NotCalled` and `Called`.
+ //
+ // One of the erroneous situations is the case when parameter is called only
+ // on some of the paths. We could've considered it `NotCalled`, but we want
+ // to report double call warnings even if these two calls are not guaranteed
+ // to happen in every execution. We also don't want to have it as `Called`
+ // because not calling tracked parameter on all of the paths is an error
+ // on its own. For these reasons, we need to have a separate kind,
+ // `MaybeCalled`, and change `Called` to `DefinitelyCalled` to avoid
+ // confusion.
+ //
+ // Two violations of calling parameter more than once and not calling it on
+ // every path are not, however, mutually exclusive. In situations where both
+ // violations take place, we prefer to report ONLY double call. It's always
+ // harder to pinpoint a bug that has arisen when a user neglects to take the
+ // right action (and therefore, no action is taken), than when a user takes
+ // the wrong action. And, in order to remember that we already reported
+ // a double call, we need another kind: `Reported`.
+ //
+ // Our analysis is intra-procedural and, while in the perfect world,
+ // developers only use tracked parameters to call them, in the real world,
+ // the picture might be
diff erent. Parameters can be stored in global
+ // variables or leaked into other functions that we know nothing about.
+ // We try to be lenient and trust users. Another kind `Escaped` reflects
+ // such situations. We don't know if it gets called there or not, but we
+ // should always think of `Escaped` as the best possible option.
+ //
+ // Some of the paths in the analyzed functions might end with a call
+ // to noreturn functions. Such paths are not required to have parameter
+ // calls and we want to track that. For the purposes of better diagnostics,
+ // we don't want to reuse `Escaped` and, thus, have another kind `NoReturn`.
+ //
+ // Additionally, we have `NotVisited` kind that tells us nothing about
+ // a tracked parameter, but is used for tracking analyzed (aka visited)
+ // basic blocks.
+ //
+ // If we consider `|` to be a JOIN operation of two kinds coming from
+ // two
diff erent paths, the following properties must hold:
+ //
+ // 1. for any Kind K: K | K == K
+ // Joining two identical kinds should result in the same kind.
+ //
+ // 2. for any Kind K: Reported | K == Reported
+ // Doesn't matter on which path it was reported, it still is.
+ //
+ // 3. for any Kind K: NoReturn | K == K
+ // We can totally ignore noreturn paths during merges.
+ //
+ // 4. DefinitelyCalled | NotCalled == MaybeCalled
+ // Called on one path, not called on another - that's simply
+ // a definition for MaybeCalled.
+ //
+ // 5. for any Kind K in [DefinitelyCalled, NotCalled, MaybeCalled]:
+ // Escaped | K == K
+ // Escaped mirrors other statuses after joins.
+ // Every situation, when we join any of the listed kinds K,
+ // is a violation. For this reason, in order to assume the
+ // best outcome for this escape, we consider it to be the
+ // same as the other path.
+ //
+ // 6. for any Kind K in [DefinitelyCalled, NotCalled]:
+ // MaybeCalled | K == MaybeCalled
+ // MaybeCalled should basically stay after almost every join.
+ 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)
+ };
+
+ constexpr ParameterStatus() = default;
+ /* implicit */ ParameterStatus(Kind K) : StatusKind(K) {
+ assert(!seenAnyCalls(K) && "Can't initialize status without a call");
+ }
+ ParameterStatus(Kind K, const Expr *Call) : StatusKind(K), Call(Call) {
+ assert(seenAnyCalls(K) && "This kind is not supposed to have a call");
+ }
+
+ const Expr &getCall() const {
+ assert(seenAnyCalls(getKind()) && "ParameterStatus doesn't have a call");
+ return *Call;
+ }
+ static bool seenAnyCalls(Kind K) {
+ return (K & DefinitelyCalled) == DefinitelyCalled && K != Reported;
+ }
+ bool seenAnyCalls() const { return seenAnyCalls(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;
+};
+
+/// State aggregates statuses of all tracked parameters.
+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 seenAnyCalls(unsigned Index) const {
+ return getStatusFor(Index).seenAnyCalls();
+ }
+ /// 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
diff erent sizes");
+ for (auto Pair : llvm::zip(ParamData, Other.ParamData)) {
+ std::get<0>(Pair).join(std::get<1>(Pair));
+ }
+ }
+
+ using iterator = ParamSizedVector<ParameterStatus>::iterator;
+ using const_iterator = ParamSizedVector<ParameterStatus>::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<ParameterStatus> 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<DeclRefFinder, const DeclRefExpr *> {
+public:
+ /// Find a DeclRefExpr in the given expression.
+ ///
+ /// In its most basic form (ShouldRetrieveFromComparisons == false),
+ /// this function can be simply reduced to the following question:
+ ///
+ /// - If expression E is used as a function argument, could we say
+ /// that DeclRefExpr nested in E is used as an argument?
+ ///
+ /// According to this rule, we can say that parens, casts and dereferencing
+ /// (dereferencing only applied to function pointers, but this is our case)
+ /// can be skipped.
+ ///
+ /// When we should look into comparisons the question changes to:
+ ///
+ /// - If expression E is used as a condition, could we say that
+ /// DeclRefExpr is being checked?
+ ///
+ /// And even though, these are two
diff erent questions, they have quite a lot
+ /// in common. Actually, we can say that whatever expression answers
+ /// positively the first question also fits the second question as well.
+ ///
+ /// In addition, we skip binary operators == and !=, and unary opeartor !.
+ 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
diff erent 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<ParmVarDecl>(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<IfStmt>(S)) {
+ return If->getCond();
+ }
+ if (const auto *Ternary = dyn_cast<AbstractConditionalOperator>(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<NamesCollector> {
+public:
+ static constexpr unsigned EXPECTED_NUMBER_OF_NAMES = 5;
+ using NameCollection =
+ llvm::SmallVector<llvm::StringRef, EXPECTED_NUMBER_OF_NAMES>;
+
+ static NameCollection collect(const Expr *From) {
+ NamesCollector Impl;
+ Impl.TraverseStmt(const_cast<Expr *>(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);
+ });
+ });
+}
+
+/// Clarification is a simple pair of a reason why parameter is not called
+/// on every path and a statement to blame.
+struct Clarification {
+ NeverCalledReason Reason;
+ const Stmt *Location;
+};
+
+/// A helper class that can produce a clarification based on the given pair
+/// of basic blocks.
+class NotCalledClarifier
+ : public ConstStmtVisitor<NotCalledClarifier,
+ llvm::Optional<Clarification>> {
+public:
+ /// The main entrypoint for the class, the function that tries to find the
+ /// clarification of how to explain which sub-path starts with a CFG edge
+ /// from Conditional to SuccWithoutCall.
+ ///
+ /// This means that this function has one precondition:
+ /// SuccWithoutCall should be a successor block for Conditional.
+ ///
+ /// Because clarification is not needed for non-trivial pairs of blocks
+ /// (i.e. SuccWithoutCall is not the only successor), it returns meaningful
+ /// results only for such cases. For this very reason, the parent basic
+ /// block, Conditional, is named that way, so it is clear what kind of
+ /// block is expected.
+ static llvm::Optional<Clarification>
+ clarify(const CFGBlock *Conditional, const CFGBlock *SuccWithoutCall) {
+ if (const Stmt *Terminator = Conditional->getTerminatorStmt()) {
+ return NotCalledClarifier{Conditional, SuccWithoutCall}.Visit(Terminator);
+ }
+ return llvm::None;
+ }
+
+ llvm::Optional<Clarification> VisitIfStmt(const IfStmt *If) {
+ return VisitBranchingBlock(If, NeverCalledReason::IfThen);
+ }
+
+ llvm::Optional<Clarification>
+ VisitAbstractConditionalOperator(const AbstractConditionalOperator *Ternary) {
+ return VisitBranchingBlock(Ternary, NeverCalledReason::IfThen);
+ }
+
+ llvm::Optional<Clarification> 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<Clarification> VisitForStmt(const ForStmt *For) {
+ return VisitBranchingBlock(For, NeverCalledReason::LoopEntered);
+ }
+
+ llvm::Optional<Clarification> VisitWhileStmt(const WhileStmt *While) {
+ return VisitBranchingBlock(While, NeverCalledReason::LoopEntered);
+ }
+
+ llvm::Optional<Clarification>
+ 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<Clarification> VisitBinaryOperator(const BinaryOperator *) {
+ // We don't want to report on short-curcuit logical operations.
+ return llvm::None;
+ }
+
+ llvm::Optional<Clarification> VisitStmt(const Stmt *Terminator) {
+ // If we got here, we didn't have a visit function for more derived
+ // 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<unsigned>(ReasonForTrueBranch) + SuccessorIndex;
+ assert(RawReason <=
+ static_cast<unsigned>(NeverCalledReason::LARGEST_VALUE));
+ return static_cast<NeverCalledReason>(RawReason);
+ }
+
+private:
+ NotCalledClarifier(const CFGBlock *Parent, const CFGBlock *SuccInQuestion)
+ : Parent(Parent), SuccInQuestion(SuccInQuestion) {}
+
+ const CFGBlock *Parent, *SuccInQuestion;
+};
+
+class CalledOnceChecker : public ConstStmtVisitor<CalledOnceChecker> {
+public:
+ static void check(AnalysisDeclContext &AC, CalledOnceCheckHandler &Handler,
+ bool CheckConventionalParameters) {
+ CalledOnceChecker(AC, Handler, CheckConventionalParameters).check();
+ }
+
+private:
+ CalledOnceChecker(AnalysisDeclContext &AC, CalledOnceCheckHandler &Handler,
+ bool CheckConventionalParameters)
+ : FunctionCFG(*AC.getCFG()), 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<FunctionDecl>(AnalyzedDecl)) {
+ findParamsToTrack(Function);
+ } else if (const auto *Method = dyn_cast<ObjCMethodDecl>(AnalyzedDecl)) {
+ findParamsToTrack(Method);
+ } else if (const auto *Block = dyn_cast<BlockDecl>(AnalyzedDecl)) {
+ findCapturesToTrack(Block);
+ findParamsToTrack(Block);
+ }
+
+ // Have something to track, let's init states for every block from the CFG.
+ if (size() != 0) {
+ States =
+ CFGSizedVector<State>(FunctionCFG.getNumBlockIDs(), State(size()));
+ }
+ }
+
+ void findCapturesToTrack(const BlockDecl *Block) {
+ for (const auto &Capture : Block->captures()) {
+ if (const auto *P = dyn_cast<ParmVarDecl>(Capture.getVariable())) {
+ // Parameter DeclContext is its owning function or method.
+ const DeclContext *ParamContext = P->getDeclContext();
+ if (shouldBeCalledOnce(ParamContext, P)) {
+ TrackedParams.push_back(P);
+ }
+ }
+ }
+ }
+
+ template <class FunctionLikeDecl>
+ void findParamsToTrack(const FunctionLikeDecl *Function) {
+ for (unsigned Index : llvm::seq<unsigned>(0u, Function->param_size())) {
+ if (shouldBeCalledOnce(Function, Index)) {
+ TrackedParams.push_back(Function->getParamDecl(Index));
+ }
+ }
+ }
+
+ //===----------------------------------------------------------------------===//
+ // 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 it 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 traverse basic blocks bottom-up.
+ for (const CFGElement &Element : llvm::reverse(*BB)) {
+ if (Optional<CFGStmt> S = Element.getAs<CFGStmt>()) {
+ 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 (!hasEverEscaped(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(V + E),
+ // where V is the number of basic blocks in FunctionCFG,
+ // E is the number of edges between 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 <class CallLikeExpr>
+ void checkIndirectCall(const CallLikeExpr *CallOrMessage) {
+ // CallExpr::arguments does not interact nicely with llvm::enumerate.
+ llvm::ArrayRef<const Expr *> 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.seenAnyCalls()) {
+
+ // 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),
+ // We are sure that the second call is definitely
+ // going to happen if the status is 'DefinitelyCalled'.
+ CurrentParamStatus.getKind() == ParameterStatus::DefinitelyCalled);
+
+ // 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
+ //===----------------------------------------------------------------------===//
+
+ /// Return true if parameter is explicitly marked as 'called_once'.
+ static bool isExplicitlyMarked(const ParmVarDecl *Parameter) {
+ return Parameter->hasAttr<CalledOnceAttr>();
+ }
+
+ /// Return true if the given name matches conventional pattens.
+ static bool isConventional(llvm::StringRef Name) {
+ return llvm::count(CONVENTIONAL_NAMES, Name) != 0;
+ }
+
+ /// Return true if the given name has conventional suffixes.
+ static bool hasConventionalSuffix(llvm::StringRef Name) {
+ return llvm::any_of(CONVENTIONAL_SUFFIXES, [Name](llvm::StringRef Suffix) {
+ return Name.endswith(Suffix);
+ });
+ }
+
+ /// Return true if the given type can be used for conventional parameters.
+ static bool isConventional(QualType Ty) {
+ if (!Ty->isBlockPointerType()) {
+ return false;
+ }
+
+ QualType BlockType = Ty->getAs<BlockPointerType>()->getPointeeType();
+ // Completion handlers should have a block type with void return type.
+ return BlockType->getAs<FunctionType>()->getReturnType()->isVoidType();
+ }
+
+ /// Return true if the only parameter of the function is conventional.
+ static bool isOnlyParameterConventional(const FunctionDecl *Function) {
+ return Function->getNumParams() == 1 &&
+ hasConventionalSuffix(Function->getName());
+ }
+
+ /// Return true/false if 'swift_async' attribute states that the given
+ /// parameter is conventionally called once.
+ /// Return llvm::None if the given declaration doesn't have 'swift_async'
+ /// attribute.
+ static llvm::Optional<bool> isConventionalSwiftAsync(const Decl *D,
+ unsigned ParamIndex) {
+ if (const SwiftAsyncAttr *A = D->getAttr<SwiftAsyncAttr>()) {
+ if (A->getKind() == SwiftAsyncAttr::None) {
+ return false;
+ }
+
+ return A->getCompletionHandlerIndex().getASTIndex() == ParamIndex;
+ }
+ return llvm::None;
+ }
+
+ /// Return true if the specified selector piece matches conventions.
+ static bool isConventionalSelectorPiece(Selector MethodSelector,
+ unsigned PieceIndex,
+ QualType PieceType) {
+ if (!isConventional(PieceType)) {
+ return false;
+ }
+
+ if (MethodSelector.getNumArgs() == 1) {
+ assert(PieceIndex == 0);
+ return hasConventionalSuffix(MethodSelector.getNameForSlot(0));
+ }
+
+ return isConventional(MethodSelector.getNameForSlot(PieceIndex));
+ }
+
+ bool shouldBeCalledOnce(const ParmVarDecl *Parameter) const {
+ return isExplicitlyMarked(Parameter) ||
+ (CheckConventionalParameters &&
+ isConventional(Parameter->getName()) &&
+ isConventional(Parameter->getType()));
+ }
+
+ bool shouldBeCalledOnce(const DeclContext *ParamContext,
+ const ParmVarDecl *Param) {
+ unsigned ParamIndex = Param->getFunctionScopeIndex();
+ if (const auto *Function = dyn_cast<FunctionDecl>(ParamContext)) {
+ return shouldBeCalledOnce(Function, ParamIndex);
+ }
+ if (const auto *Method = dyn_cast<ObjCMethodDecl>(ParamContext)) {
+ return shouldBeCalledOnce(Method, ParamIndex);
+ }
+ return shouldBeCalledOnce(Param);
+ }
+
+ bool shouldBeCalledOnce(const BlockDecl *Block, unsigned ParamIndex) const {
+ return shouldBeCalledOnce(Block->getParamDecl(ParamIndex));
+ }
+
+ bool shouldBeCalledOnce(const FunctionDecl *Function,
+ unsigned ParamIndex) const {
+ if (ParamIndex >= Function->getNumParams()) {
+ return false;
+ }
+ // 'swift_async' goes first and overrides anything else.
+ if (auto ConventionalAsync =
+ isConventionalSwiftAsync(Function, ParamIndex)) {
+ return ConventionalAsync.getValue();
+ }
+
+ return shouldBeCalledOnce(Function->getParamDecl(ParamIndex)) ||
+ (CheckConventionalParameters &&
+ isOnlyParameterConventional(Function));
+ }
+
+ bool shouldBeCalledOnce(const ObjCMethodDecl *Method,
+ unsigned ParamIndex) const {
+ Selector MethodSelector = Method->getSelector();
+ if (ParamIndex >= MethodSelector.getNumArgs()) {
+ return false;
+ }
+
+ // 'swift_async' goes first and overrides anything else.
+ if (auto ConventionalAsync = isConventionalSwiftAsync(Method, ParamIndex)) {
+ return ConventionalAsync.getValue();
+ }
+
+ const ParmVarDecl *Parameter = Method->getParamDecl(ParamIndex);
+ return shouldBeCalledOnce(Parameter) ||
+ (CheckConventionalParameters &&
+ isConventionalSelectorPiece(MethodSelector, ParamIndex,
+ Parameter->getType()));
+ }
+
+ bool shouldBeCalledOnce(const CallExpr *Call, unsigned ParamIndex) const {
+ const FunctionDecl *Function = Call->getDirectCallee();
+ return Function && shouldBeCalledOnce(Function, ParamIndex);
+ }
+
+ bool shouldBeCalledOnce(const ObjCMessageExpr *Message,
+ unsigned ParamIndex) const {
+ const ObjCMethodDecl *Method = Message->getMethodDecl();
+ return Method && ParamIndex < Method->param_size() &&
+ shouldBeCalledOnce(Method, ParamIndex);
+ }
+
+ //===----------------------------------------------------------------------===//
+ // Utility methods
+ //===----------------------------------------------------------------------===//
+
+ bool isCaptured(const ParmVarDecl *Parameter) const {
+ if (const BlockDecl *Block = dyn_cast<BlockDecl>(AC.getDecl())) {
+ return Block->capturesVariable(Parameter);
+ }
+ return false;
+ }
+
+ /// Return true if the 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<ObjCMethodDecl>(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 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.
+ if (OnlyBlock && OnlyBlock->hasNoReturnElement()) {
+ return true;
+ }
+ // Fallthrough, CFGs with only one block can fall into #1 and #3 as well.
+ }
+
+ // 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<ParentMap> 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<CFGStmt> S = Element.getAs<CFGStmt>()) {
+ const Stmt *SuspiciousStmt = S->getStmt();
+
+ if (isa<ReturnStmt>(SuspiciousStmt)) {
+ // Let's initialize this structure to test whether
+ // some further statement is a part of this return.
+ ReturnChildren = std::make_unique<ParentMap>(
+ const_cast<Stmt *>(SuspiciousStmt));
+ // Return statements are allowed as part of #1.
+ return true;
+ }
+
+ return SuspiciousStmt->getBeginLoc().isMacroID() ||
+ (ReturnChildren &&
+ ReturnChildren->hasParent(SuspiciousStmt));
+ }
+ return true;
+ });
+ });
+ }
+
+ /// Check if parameter with the given index has ever escaped.
+ bool hasEverEscaped(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);
+ if (SuppressOnConventionalErrorPaths) {
+ 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 analysis 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 &&
+ anySuccessorHasStatus(JoinBlock, ParameterIndex, BeforeJoin);
+ }
+
+ /// Return true if any of the successors of the given basic block has
+ /// a specified status for the given parameter.
+ bool anySuccessorHasStatus(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;
+ }
+ }
+
+ /// Check if the given assignment represents suppression and act on it.
+ void checkSuppression(const BinaryOperator *Assignment) {
+ // Suppression has the following form:
+ // parameter = 0;
+ // 0 can be of any form (NULL, nil, etc.)
+ if (auto Index = getIndexOfExpression(Assignment->getLHS())) {
+
+ // We don't care what is written in the RHS, it could be whatever
+ // we can interpret as 0.
+ if (auto Constant =
+ Assignment->getRHS()->IgnoreParenCasts()->getIntegerConstantExpr(
+ AC.getASTContext())) {
+
+ ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(*Index);
+
+ if (0 == *Constant && CurrentParamStatus.seenAnyCalls()) {
+ // Even though this suppression mechanism is introduced to tackle
+ // false positives for multiple calls, the fact that the user has
+ // to use suppression can also tell us that we couldn't figure out
+ // how
diff erent paths cancel each other out. And if that is true,
+ // we will most certainly have false positives about parameters not
+ // being called on certain paths.
+ //
+ // For this reason, we abandon tracking this parameter altogether.
+ CurrentParamStatus = ParameterStatus::Reported;
+ }
+ }
+ }
+ }
+
+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<ParmVarDecl>(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());
+
+ // Let's check whether this assignment is a suppression.
+ checkSuppression(Op);
+ }
+ }
+
+ 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<VarDecl>(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 parameter
+ // is intentionally not called on this path.
+ if (Cast->getType().getCanonicalType()->isVoidType()) {
+ 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<unsigned> getIndexOfCallee(const CallExpr *Call) const {
+ return getIndexOfExpression(Call->getCallee());
+ }
+
+ llvm::Optional<unsigned> getIndexOfExpression(const Expr *E) const {
+ if (const ParmVarDecl *Parameter = findReferencedParmVarDecl(E)) {
+ return getIndex(*Parameter);
+ }
+
+ return llvm::None;
+ }
+
+ llvm::Optional<unsigned> 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 ParmVarDecl *>::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;
+ // As of now, we turn this behavior off. So, we still are going to report
+ // missing calls on paths that look like it was intentional.
+ // Technically such reports are true positives, but they can make some users
+ // grumpy because of the sheer number of warnings.
+ // It can be turned back on if we decide that we want to have the other way
+ // around.
+ bool SuppressOnConventionalErrorPaths = false;
+
+ State CurrentState;
+ ParamSizedVector<const ParmVarDecl *> TrackedParams;
+ CFGSizedVector<State> States;
+};
+
+} // end anonymous namespace
+
+namespace clang {
+void checkCalledOnceParameters(AnalysisDeclContext &AC,
+ CalledOnceCheckHandler &Handler,
+ bool CheckConventionalParameters) {
+ CalledOnceChecker::check(AC, Handler, CheckConventionalParameters);
+}
+} // end namespace clang
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 2850162141c9..edd9742ed207 100644
--- 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,82 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
});
}
};
+
+class CalledOnceCheckReporter : public CalledOnceCheckHandler {
+public:
+ CalledOnceCheckReporter(Sema &S) : S(S) {}
+ void handleDoubleCall(const ParmVarDecl *Parameter, const Expr *Call,
+ const Expr *PrevCall, bool IsCompletionHandler,
+ bool Poised) 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)
+ << Poised;
+ }
+
+ 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<unsigned> 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 +2342,17 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
}
}
+ // Check for violations of "called once" parameter properties.
+ if (S.getLangOpts().ObjC &&
+ shouldAnalyzeCalledOnceParameters(Diags, D->getBeginLoc())) {
+ if (AC.getCFG()) {
+ CalledOnceCheckReporter Reporter(S);
+ checkCalledOnceParameters(
+ 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
index 7750d713f927..c836031afcfb 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3693,6 +3693,26 @@ static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
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) {
+ // 'called_once' only applies to parameters representing functions.
+ QualType T = cast<ParmVarDecl>(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;
@@ -7692,6 +7712,9 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
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
index d8bfd3f8b46a..da555c95eaf0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -15404,10 +15404,6 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
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<BlockDecl::Capture, 4> Captures;
for (Capture &Cap : BSI->Captures) {
@@ -15475,6 +15471,10 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
}
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/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index 729546891e8a..b96cb4bc745d 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -40,6 +40,7 @@
// CHECK-NEXT: CXX11NoReturn (SubjectMatchRule_function)
// CHECK-NEXT: CallableWhen (SubjectMatchRule_function_is_member)
// CHECK-NEXT: Callback (SubjectMatchRule_function)
+// CHECK-NEXT: CalledOnce (SubjectMatchRule_variable_is_parameter)
// CHECK-NEXT: Capability (SubjectMatchRule_record, SubjectMatchRule_type_alias)
// CHECK-NEXT: CarriesDependency (SubjectMatchRule_variable_is_parameter, SubjectMatchRule_objc_method, SubjectMatchRule_function)
// CHECK-NEXT: Cleanup (SubjectMatchRule_variable_is_local)
diff --git a/clang/test/SemaObjC/attr-called-once.m b/clang/test/SemaObjC/attr-called-once.m
new file mode 100644
index 000000000000..5112d0d3a251
--- /dev/null
+++ b/clang/test/SemaObjC/attr-called-once.m
@@ -0,0 +1,20 @@
+// 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
+
+void test5(void (*foo)() __attribute__((called_once(1))));
+// expected-error at -1{{'called_once' attribute takes no arguments}}
+void test6(void (*foo)() __attribute__((called_once("str1", "str2"))));
+// expected-error at -1{{'called_once' attribute takes no arguments}}
+
+CALLED_ONCE void test7(); // expected-warning{{'called_once' attribute only applies to parameters}}
+void test8() {
+ void (*foo)() CALLED_ONCE; // expected-warning{{'called_once' attribute only applies to parameters}}
+ foo();
+}
diff --git a/clang/test/SemaObjC/warn-called-once.m b/clang/test/SemaObjC/warn-called-once.m
new file mode 100644
index 000000000000..094f92a49935
--- /dev/null
+++ b/clang/test/SemaObjC/warn-called-once.m
@@ -0,0 +1,1050 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fblocks -fobjc-exceptions -Wcompletion-handler %s
+
+#define NULL (void *)0
+#define nil (id)0
+#define CALLED_ONCE __attribute__((called_once))
+#define NORETURN __attribute__((noreturn))
+
+ at protocol NSObject
+ at end
+ at interface NSObject <NSObject>
+- (id)copy;
+- (id)class;
+- autorelease;
+ at end
+
+typedef unsigned int NSUInteger;
+typedef struct {
+} NSFastEnumerationState;
+
+ at interface NSArray <__covariant NSFastEnumeration>
+- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state objects:(id *)stackbuf count:(NSUInteger)len;
+ at end
+ at interface NSMutableArray<ObjectType> : NSArray <ObjectType>
+- addObject:anObject;
+ at end
+ at 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 at -1{{previous call is here; set to nil to indicate it cannot be called afterwards}}
+
+ 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 at -1{{previous call is here; set to nil to indicate it cannot be called afterwards}}
+ }
+
+ 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 at -1{{'callback' parameter marked 'called_once' is called twice}}
+ }
+}
+
+void never_called_trivial(void (^callback)(void) CALLED_ONCE) {
+ // expected-warning at -1{{'callback' parameter marked 'called_once' is never called}}
+}
+
+int never_called_branching(int x, void (^callback)(void) CALLED_ONCE) {
+ // expected-warning at -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 at -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 at -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 at -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
diff erent 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 at -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 at -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 at -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_1(void (^completionHandler)(void)) {
+ // expected-warning at -1{{completion handler is never called}}
+}
+
+void never_called_conv_2(void (^completion)(void)) {
+ // expected-warning at -1{{completion handler is never called}}
+}
+
+void never_called_conv_WithCompletion(void (^callback)(void)) {
+ // expected-warning at -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 at -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) {
+ // expected-warning at -1{{completion handler is never called when taking true branch}}
+ // This behavior might be tweaked in the future
+ 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 at -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 at -1{{'callback' parameter marked 'called_once' is never used when taking false branch}}
+ (void)callback;
+ }
+}
+
+ at 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)indirect_call_conv_3WithCompletion:(void (^)(void))handler;
+- (void)indirect_call_conv_4:(void (^)(void))handler
+ __attribute__((swift_async(swift_private, 1)));
+- (void)exit:(int)code NORETURN;
+- (int)condition;
+ at end
+
+ at interface TestClass : TestBase
+ at property(strong) NSMutableArray *handlers;
+ at property(strong) id storedHandler;
+ at property int wasCanceled;
+ at property(getter=hasErrors) int error;
+ at end
+
+ at 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)double_indirect_call_4:(void (^)(void))completion {
+ [self indirect_call_conv_2:0 // expected-note{{previous call is here}}
+ completionHandler:completion];
+ [self indirect_call_conv_2:1 // expected-warning{{completion handler is called twice}}
+ completionHandler:completion];
+}
+
+- (void)double_indirect_call_5:(void (^)(void))withCompletionHandler {
+ [self indirect_call_conv_2:0 // expected-note{{previous call is here}}
+ completionHandler:withCompletionHandler];
+ [self indirect_call_conv_2:1 // expected-warning{{completion handler is called twice}}
+ completionHandler:withCompletionHandler];
+}
+
+- (void)double_indirect_call_6:(void (^)(void))completionHandler {
+ [self indirect_call_conv_3WithCompletion: // expected-note{{previous call is here}}
+ completionHandler];
+ [self indirect_call_conv_3WithCompletion: // expected-warning{{completion handler is called twice}}
+ completionHandler];
+}
+
+- (void)double_indirect_call_7:(void (^)(void))completionHandler {
+ [self indirect_call_conv_4: // expected-note{{previous call is here}}
+ completionHandler];
+ [self indirect_call_conv_4: // expected-warning{{completion handler is called twice}}
+ completionHandler];
+}
+
+- (void)never_called_trivial:(void (^)(void))CALLED_ONCE callback {
+ // expected-warning at -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 at -1{{'callback' parameter marked 'called_once' is never used when taking false branch}}
+ [self escape:callback];
+ }
+ }];
+}
+
+- (void)block_call_3:(int)cond
+ completionHandler:(void (^)(void))callback {
+ [self indirect_call:
+ ^{
+ if (cond) {
+ // expected-warning at -1{{completion handler is never used when taking false branch}}
+ [self escape:callback];
+ }
+ }];
+}
+
+- (void)block_call_4WithCompletion:(void (^)(void))callback {
+ [self indirect_call:
+ ^{
+ if ([self condition]) {
+ // expected-warning at -1{{completion handler is never used when taking false branch}}
+ [self escape:callback];
+ }
+ }];
+}
+
+- (void)never_called_conv:(void (^)(void))completionHandler {
+ // expected-warning at -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 at -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)
+ // expected-warning at -1{{completion handler is never called when taking true branch}}
+ // This behavior might be tweaked in the future
+ return;
+
+ completionHandler();
+}
+
+- (void)conventional_error_path_2:(void (^)(void))completionHandler {
+ if (self.wasCanceled)
+ // expected-warning at -1{{completion handler is never used when taking true branch}}
+ // This behavior might be tweaked in the future
+ return;
+
+ [_handlers addObject:completionHandler];
+}
+
+- (void)conventional_error_path_3:(void (^)(void))completionHandler {
+ if (self.hasErrors)
+ // expected-warning at -1{{completion handler is never called when taking true branch}}
+ // This behavior might be tweaked in the future
+ return;
+
+ completionHandler();
+}
+
+- (void)conventional_error_path_3:(int)cond completionHandler:(void (^)(void))handler {
+ if (self.wasCanceled)
+ // expected-warning at -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 at -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 at -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 at -1{{completion handler is never called when taking true branch}}
+ return;
+ }
+
+ completionHandler();
+}
+
+- (void)testWithCompletionHandler:(void (^)(void))callback {
+ if ([self condition]) {
+ // expected-warning at -1{{completion handler is never called when taking false branch}}
+ callback();
+ }
+}
+
+- (void)testWithCompletion:(void (^)(void))callback {
+ if ([self condition]) {
+ // expected-warning at -1{{completion handler is never called when taking false branch}}
+ callback();
+ }
+}
+
+- (void)completion_handler_wrong_type:(int (^)(void))completionHandler {
+ // We don't want to consider completion handlers with non-void return types.
+ if ([self condition]) {
+ // no-warning
+ completionHandler();
+ }
+}
+
+- (void)test_swift_async_none:(int)cond
+ completionHandler:(void (^)(void))handler __attribute__((swift_async(none))) {
+ if (cond) {
+ // no-warning
+ handler();
+ }
+}
+
+- (void)test_swift_async_param:(int)cond
+ callback:(void (^)(void))callback
+ __attribute__((swift_async(swift_private, 2))) {
+ if (cond) {
+ // expected-warning at -1{{completion handler is never called when taking false branch}}
+ callback();
+ }
+}
+
+- (void)test_nil_suggestion:(int)cond1
+ second:(int)cond2
+ completion:(void (^)(void))handler {
+ if (cond1) {
+ handler();
+ // expected-note at -1{{previous call is here; set to nil to indicate it cannot be called afterwards}}
+ }
+
+ if (cond2) {
+ handler(); // expected-warning{{completion handler is called twice}}
+ }
+}
+
+- (void)test_nil_suppression_1:(int)cond1
+ second:(int)cond2
+ completion:(void (^)(void))handler {
+ if (cond1) {
+ handler();
+ handler = nil;
+ // no-warning
+ }
+
+ if (cond2) {
+ handler();
+ }
+}
+
+- (void)test_nil_suppression_2:(int)cond1
+ second:(int)cond2
+ completion:(void (^)(void))handler {
+ if (cond1) {
+ handler();
+ handler = NULL;
+ // no-warning
+ }
+
+ if (cond2) {
+ handler();
+ }
+}
+
+- (void)test_nil_suppression_3:(int)cond1
+ second:(int)cond2
+ completion:(void (^)(void))handler {
+ if (cond1) {
+ handler();
+ handler = 0;
+ // no-warning
+ }
+
+ if (cond2) {
+ handler();
+ }
+}
+
+ at end
More information about the llvm-branch-commits
mailing list