[clang] f1a7d5a - [-Wcalled-once-parameter] Harden analysis in terms of block use

Valeriy Savchenko via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 18 02:13:03 PDT 2021


Author: Valeriy Savchenko
Date: 2021-03-18T12:12:18+03:00
New Revision: f1a7d5a7b0ec810057ff6e88371ab86d1fce812c

URL: https://github.com/llvm/llvm-project/commit/f1a7d5a7b0ec810057ff6e88371ab86d1fce812c
DIFF: https://github.com/llvm/llvm-project/commit/f1a7d5a7b0ec810057ff6e88371ab86d1fce812c.diff

LOG: [-Wcalled-once-parameter] Harden analysis in terms of block use

This patch introduces a very simple inter-procedural analysis
between blocks and enclosing functions.

We always analyze blocks first (analysis is done as part of semantic
analysis that goes side-by-side with the parsing process), and at the
moment of reporting we don't know how that block will be actually
used.

This patch introduces new logic delaying reports of the "never called"
warnings on blocks.  If we are not sure that the block will be called
exactly once, we shouldn't warn our users about that.  Double calls,
however, don't require such delays.  While analyzing the enclosing
function, we can actually decide what we should do with those
warnings.

Additionally, as a side effect, we can be more confident about blocks
in such context and can treat them not as escapes, but as direct
calls.

rdar://74090107

Differential Revision: https://reviews.llvm.org/D98688

Added: 
    

Modified: 
    clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
    clang/include/clang/Sema/AnalysisBasedWarnings.h
    clang/lib/Analysis/CalledOnceCheck.cpp
    clang/lib/Sema/AnalysisBasedWarnings.cpp
    clang/test/SemaObjC/warn-called-once.m

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h b/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
index fc574c680a44..a0c767bf92d2 100644
--- a/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
+++ b/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
@@ -17,6 +17,7 @@
 namespace clang {
 
 class AnalysisDeclContext;
+class BlockDecl;
 class CFG;
 class Decl;
 class DeclContext;
@@ -79,6 +80,7 @@ class CalledOnceCheckHandler {
   /// 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 Function -- function declaration where the problem occured.
   /// \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
@@ -86,9 +88,22 @@ class CalledOnceCheckHandler {
   /// 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,
+                                 const Decl *Function, const Stmt *Where,
+                                 NeverCalledReason Reason,
                                  bool IsCalledDirectly,
                                  bool IsCompletionHandler) {}
+
+  /// Called when the block is guaranteed to be called exactly once.
+  /// It means that we can be stricter with what we report on that block.
+  /// \param Block -- block declaration that is known to be called exactly once.
+  virtual void
+  handleBlockThatIsGuaranteedToBeCalledOnce(const BlockDecl *Block) {}
+
+  /// Called when the block has no guarantees about how many times it can get
+  /// called.
+  /// It means that we should be more lenient with reporting warnings in it.
+  /// \param Block -- block declaration in question.
+  virtual void handleBlockWithNoGuarantees(const BlockDecl *Block) {}
 };
 
 /// Check given CFG for 'called once' parameter violations.

diff  --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h
index e13fe955eaf4..49b69c585ff7 100644
--- a/clang/include/clang/Sema/AnalysisBasedWarnings.h
+++ b/clang/include/clang/Sema/AnalysisBasedWarnings.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H
 
 #include "llvm/ADT/DenseMap.h"
+#include <memory>
 
 namespace clang {
 
@@ -47,6 +48,9 @@ class AnalysisBasedWarnings {
   Sema &S;
   Policy DefaultPolicy;
 
+  class InterProceduralData;
+  std::unique_ptr<InterProceduralData> IPData;
+
   enum VisitFlag { NotVisited = 0, Visited = 1, Pending = 2 };
   llvm::DenseMap<const FunctionDecl*, VisitFlag> VisitedFD;
 
@@ -88,6 +92,7 @@ class AnalysisBasedWarnings {
 
 public:
   AnalysisBasedWarnings(Sema &s);
+  ~AnalysisBasedWarnings();
 
   void IssueWarnings(Policy P, FunctionScopeInfo *fscope,
                      const Decl *D, QualType BlockType);
@@ -97,6 +102,7 @@ class AnalysisBasedWarnings {
   void PrintStats() const;
 };
 
-}} // end namespace clang::sema
+} // namespace sema
+} // namespace clang
 
 #endif

diff  --git a/clang/lib/Analysis/CalledOnceCheck.cpp b/clang/lib/Analysis/CalledOnceCheck.cpp
index d24e0b500564..29021b0a9016 100644
--- a/clang/lib/Analysis/CalledOnceCheck.cpp
+++ b/clang/lib/Analysis/CalledOnceCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/Analyses/CalledOnceCheck.h"
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
@@ -57,6 +58,20 @@ constexpr llvm::StringLiteral CONVENTIONAL_SUFFIXES[] = {
 constexpr llvm::StringLiteral CONVENTIONAL_CONDITIONS[] = {
     "error", "cancel", "shouldCall", "done", "OK", "success"};
 
+struct KnownCalledOnceParameter {
+  llvm::StringLiteral FunctionName;
+  unsigned ParamIndex;
+};
+constexpr KnownCalledOnceParameter KNOWN_CALLED_ONCE_PARAMETERS[] = {
+    {"dispatch_async", 1},
+    {"dispatch_async_and_wait", 1},
+    {"dispatch_after", 2},
+    {"dispatch_sync", 1},
+    {"dispatch_once", 1},
+    {"dispatch_barrier_async", 1},
+    {"dispatch_barrier_async_and_wait", 1},
+    {"dispatch_barrier_sync", 1}};
+
 class ParameterStatus {
 public:
   // Status kind is basically the main part of parameter's status.
@@ -929,9 +944,9 @@ class CalledOnceChecker : public ConstStmtVisitor<CalledOnceChecker> {
                "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));
+          Handler.handleNeverCalled(
+              Parameter, AC.getDecl(), Clarification->Location,
+              Clarification->Reason, !IsEscape, !isExplicitlyMarked(Parameter));
         }
       }
     }
@@ -1091,6 +1106,91 @@ class CalledOnceChecker : public ConstStmtVisitor<CalledOnceChecker> {
     return false;
   }
 
+  // Return a call site where the block is called exactly once or null otherwise
+  const Expr *getBlockGuaraneedCallSite(const BlockExpr *Block) const {
+    ParentMap &PM = AC.getParentMap();
+
+    // We don't want to track the block through assignments and so on, instead
+    // we simply see how the block used and if it's used directly in a call,
+    // we decide based on call to what it is.
+    //
+    // In order to do this, we go up the parents of the block looking for
+    // a call or a message expressions.  These might not be immediate parents
+    // of the actual block expression due to casts and parens, so we skip them.
+    for (const Stmt *Prev = Block, *Current = PM.getParent(Block);
+         Current != nullptr; Prev = Current, Current = PM.getParent(Current)) {
+      // Skip no-op (for our case) operations.
+      if (isa<CastExpr>(Current) || isa<ParenExpr>(Current))
+        continue;
+
+      // At this point, Prev represents our block as an immediate child of the
+      // call.
+      if (const auto *Call = dyn_cast<CallExpr>(Current)) {
+        // It might be the call of the Block itself...
+        if (Call->getCallee() == Prev)
+          return Call;
+
+        // ...or it can be an indirect call of the block.
+        return shouldBlockArgumentBeCalledOnce(Call, Prev) ? Call : nullptr;
+      }
+      if (const auto *Message = dyn_cast<ObjCMessageExpr>(Current)) {
+        return shouldBlockArgumentBeCalledOnce(Message, Prev) ? Message
+                                                              : nullptr;
+      }
+
+      break;
+    }
+
+    return nullptr;
+  }
+
+  template <class CallLikeExpr>
+  bool shouldBlockArgumentBeCalledOnce(const CallLikeExpr *CallOrMessage,
+                                       const Stmt *BlockArgument) const {
+    // CallExpr::arguments does not interact nicely with llvm::enumerate.
+    llvm::ArrayRef<const Expr *> Arguments = llvm::makeArrayRef(
+        CallOrMessage->getArgs(), CallOrMessage->getNumArgs());
+
+    for (const auto &Argument : llvm::enumerate(Arguments)) {
+      if (Argument.value() == BlockArgument) {
+        return shouldBlockArgumentBeCalledOnce(CallOrMessage, Argument.index());
+      }
+    }
+
+    return false;
+  }
+
+  bool shouldBlockArgumentBeCalledOnce(const CallExpr *Call,
+                                       unsigned ParamIndex) const {
+    const FunctionDecl *Function = Call->getDirectCallee();
+    return shouldBlockArgumentBeCalledOnce(Function, ParamIndex) ||
+           shouldBeCalledOnce(Call, ParamIndex);
+  }
+
+  bool shouldBlockArgumentBeCalledOnce(const ObjCMessageExpr *Message,
+                                       unsigned ParamIndex) const {
+    // At the moment, we don't have any Obj-C methods we want to specifically
+    // check in here.
+    return shouldBeCalledOnce(Message, ParamIndex);
+  }
+
+  static bool shouldBlockArgumentBeCalledOnce(const FunctionDecl *Function,
+                                              unsigned ParamIndex) {
+    // There is a list of important API functions that while not following
+    // conventions nor being directly annotated, still guarantee that the
+    // callback parameter will be called exactly once.
+    //
+    // Here we check if this is the case.
+    return Function &&
+           llvm::any_of(KNOWN_CALLED_ONCE_PARAMETERS,
+                        [Function, ParamIndex](
+                            const KnownCalledOnceParameter &Reference) {
+                          return Reference.FunctionName ==
+                                     Function->getName() &&
+                                 Reference.ParamIndex == ParamIndex;
+                        });
+  }
+
   /// Return true if the analyzed function is actually a default implementation
   /// of the method that has to be overriden.
   ///
@@ -1437,17 +1537,44 @@ class CalledOnceChecker : public ConstStmtVisitor<CalledOnceChecker> {
   }
 
   void VisitBlockExpr(const BlockExpr *Block) {
+    // Block expressions are tricky.  It is a very common practice to capture
+    // completion handlers by blocks and use them there.
+    // For this reason, it is important to analyze blocks and report warnings
+    // for completion handler misuse in blocks.
+    //
+    // However, it can be quite 
diff icult to track how the block itself is being
+    // used.  The full precise anlysis of that will be similar to alias analysis
+    // for completion handlers and can be too heavyweight for a compile-time
+    // diagnostic.  Instead, we judge about the immediate use of the block.
+    //
+    // Here, we try to find a call expression where we know due to conventions,
+    // annotations, or other reasons that the block is called once and only
+    // once.
+    const Expr *CalledOnceCallSite = getBlockGuaraneedCallSite(Block);
+
+    // We need to report this information to the handler because in the
+    // situation when we know that the block is called exactly once, we can be
+    // stricter in terms of reported diagnostics.
+    if (CalledOnceCallSite) {
+      Handler.handleBlockThatIsGuaranteedToBeCalledOnce(Block->getBlockDecl());
+    } else {
+      Handler.handleBlockWithNoGuarantees(Block->getBlockDecl());
+    }
+
     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);
+        if (auto Index = getIndex(*Param)) {
+          if (CalledOnceCallSite) {
+            // The call site of a block can be considered a call site of the
+            // captured parameter we track.
+            processCallFor(*Index, CalledOnceCallSite);
+          } else {
+            // We still should consider this block as an escape for parameter,
+            // if we don't know about its call site or the number of time it
+            // can be invoked.
+            processEscapeFor(*Index);
+          }
+        }
       }
     }
   }

diff  --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index edd9742ed207..bcd6a00d7ba5 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1506,6 +1506,25 @@ static void diagnoseRepeatedUseOfWeak(Sema &S,
   }
 }
 
+namespace clang {
+namespace {
+typedef SmallVector<PartialDiagnosticAt, 1> OptionalNotes;
+typedef std::pair<PartialDiagnosticAt, OptionalNotes> DelayedDiag;
+typedef std::list<DelayedDiag> DiagList;
+
+struct SortDiagBySourceLocation {
+  SourceManager &SM;
+  SortDiagBySourceLocation(SourceManager &SM) : SM(SM) {}
+
+  bool operator()(const DelayedDiag &left, const DelayedDiag &right) {
+    // Although this call will be slow, this is only called when outputting
+    // multiple warnings.
+    return SM.isBeforeInTranslationUnit(left.first.first, right.first.first);
+  }
+};
+} // anonymous namespace
+} // namespace clang
+
 namespace {
 class UninitValsDiagReporter : public UninitVariablesHandler {
   Sema &S;
@@ -1626,9 +1645,35 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
   }
 };
 
+/// Inter-procedural data for the called-once checker.
+class CalledOnceInterProceduralData {
+public:
+  // Add the delayed warning for the given block.
+  void addDelayedWarning(const BlockDecl *Block,
+                         PartialDiagnosticAt &&Warning) {
+    DelayedBlockWarnings[Block].emplace_back(std::move(Warning));
+  }
+  // Report all of the warnings we've gathered for the given block.
+  void flushWarnings(const BlockDecl *Block, Sema &S) {
+    for (const PartialDiagnosticAt &Delayed : DelayedBlockWarnings[Block])
+      S.Diag(Delayed.first, Delayed.second);
+
+    discardWarnings(Block);
+  }
+  // Discard all of the warnings we've gathered for the given block.
+  void discardWarnings(const BlockDecl *Block) {
+    DelayedBlockWarnings.erase(Block);
+  }
+
+private:
+  using DelayedDiagnostics = SmallVector<PartialDiagnosticAt, 2>;
+  llvm::DenseMap<const BlockDecl *, DelayedDiagnostics> DelayedBlockWarnings;
+};
+
 class CalledOnceCheckReporter : public CalledOnceCheckHandler {
 public:
-  CalledOnceCheckReporter(Sema &S) : S(S) {}
+  CalledOnceCheckReporter(Sema &S, CalledOnceInterProceduralData &Data)
+      : S(S), Data(Data) {}
   void handleDoubleCall(const ParmVarDecl *Parameter, const Expr *Call,
                         const Expr *PrevCall, bool IsCompletionHandler,
                         bool Poised) override {
@@ -1649,14 +1694,24 @@ class CalledOnceCheckReporter : public CalledOnceCheckHandler {
         << Parameter << /* Captured */ false;
   }
 
-  void handleNeverCalled(const ParmVarDecl *Parameter, const Stmt *Where,
-                         NeverCalledReason Reason, bool IsCalledDirectly,
+  void handleNeverCalled(const ParmVarDecl *Parameter, const Decl *Function,
+                         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;
+    PartialDiagnosticAt Warning(Where->getBeginLoc(), S.PDiag(DiagToReport)
+                                                          << Parameter
+                                                          << IsCalledDirectly
+                                                          << (unsigned)Reason);
+
+    if (const auto *Block = dyn_cast<BlockDecl>(Function)) {
+      // We shouldn't report these warnings on blocks immediately
+      Data.addDelayedWarning(Block, std::move(Warning));
+    } else {
+      S.Diag(Warning.first, Warning.second);
+    }
   }
 
   void handleCapturedNeverCalled(const ParmVarDecl *Parameter,
@@ -1669,8 +1724,18 @@ class CalledOnceCheckReporter : public CalledOnceCheckHandler {
         << Parameter << /* Captured */ true;
   }
 
+  void
+  handleBlockThatIsGuaranteedToBeCalledOnce(const BlockDecl *Block) override {
+    Data.flushWarnings(Block, S);
+  }
+
+  void handleBlockWithNoGuarantees(const BlockDecl *Block) override {
+    Data.discardWarnings(Block);
+  }
+
 private:
   Sema &S;
+  CalledOnceInterProceduralData &Data;
 };
 
 constexpr unsigned CalledOnceWarnings[] = {
@@ -1703,25 +1768,6 @@ bool shouldAnalyzeCalledOnceParameters(const DiagnosticsEngine &Diags,
 }
 } // anonymous namespace
 
-namespace clang {
-namespace {
-typedef SmallVector<PartialDiagnosticAt, 1> OptionalNotes;
-typedef std::pair<PartialDiagnosticAt, OptionalNotes> DelayedDiag;
-typedef std::list<DelayedDiag> DiagList;
-
-struct SortDiagBySourceLocation {
-  SourceManager &SM;
-  SortDiagBySourceLocation(SourceManager &SM) : SM(SM) {}
-
-  bool operator()(const DelayedDiag &left, const DelayedDiag &right) {
-    // Although this call will be slow, this is only called when outputting
-    // multiple warnings.
-    return SM.isBeforeInTranslationUnit(left.first.first, right.first.first);
-  }
-};
-} // anonymous namespace
-} // namespace clang
-
 //===----------------------------------------------------------------------===//
 // -Wthread-safety
 //===----------------------------------------------------------------------===//
@@ -2107,54 +2153,68 @@ class ConsumedWarningsHandler : public ConsumedWarningsHandlerBase {
 //  warnings on a function, method, or block.
 //===----------------------------------------------------------------------===//
 
-clang::sema::AnalysisBasedWarnings::Policy::Policy() {
+sema::AnalysisBasedWarnings::Policy::Policy() {
   enableCheckFallThrough = 1;
   enableCheckUnreachable = 0;
   enableThreadSafetyAnalysis = 0;
   enableConsumedAnalysis = 0;
 }
 
+/// InterProceduralData aims to be a storage of whatever data should be passed
+/// between analyses of 
diff erent functions.
+///
+/// At the moment, its primary goal is to make the information gathered during
+/// the analysis of the blocks available during the analysis of the enclosing
+/// function.  This is important due to the fact that blocks are analyzed before
+/// the enclosed function is even parsed fully, so it is not viable to access
+/// anything in the outer scope while analyzing the block.  On the other hand,
+/// re-building CFG for blocks and re-analyzing them when we do have all the
+/// information (i.e. during the analysis of the enclosing function) seems to be
+/// ill-designed.
+class sema::AnalysisBasedWarnings::InterProceduralData {
+public:
+  // It is important to analyze blocks within functions because it's a very
+  // common pattern to capture completion handler parameters by blocks.
+  CalledOnceInterProceduralData CalledOnceData;
+};
+
 static unsigned isEnabled(DiagnosticsEngine &D, unsigned diag) {
   return (unsigned)!D.isIgnored(diag, SourceLocation());
 }
 
-clang::sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
-  : S(s),
-    NumFunctionsAnalyzed(0),
-    NumFunctionsWithBadCFGs(0),
-    NumCFGBlocks(0),
-    MaxCFGBlocksPerFunction(0),
-    NumUninitAnalysisFunctions(0),
-    NumUninitAnalysisVariables(0),
-    MaxUninitAnalysisVariablesPerFunction(0),
-    NumUninitAnalysisBlockVisits(0),
-    MaxUninitAnalysisBlockVisitsPerFunction(0) {
+sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
+    : S(s), IPData(std::make_unique<InterProceduralData>()),
+      NumFunctionsAnalyzed(0), NumFunctionsWithBadCFGs(0), NumCFGBlocks(0),
+      MaxCFGBlocksPerFunction(0), NumUninitAnalysisFunctions(0),
+      NumUninitAnalysisVariables(0), MaxUninitAnalysisVariablesPerFunction(0),
+      NumUninitAnalysisBlockVisits(0),
+      MaxUninitAnalysisBlockVisitsPerFunction(0) {
 
   using namespace diag;
   DiagnosticsEngine &D = S.getDiagnostics();
 
   DefaultPolicy.enableCheckUnreachable =
-    isEnabled(D, warn_unreachable) ||
-    isEnabled(D, warn_unreachable_break) ||
-    isEnabled(D, warn_unreachable_return) ||
-    isEnabled(D, warn_unreachable_loop_increment);
+      isEnabled(D, warn_unreachable) || isEnabled(D, warn_unreachable_break) ||
+      isEnabled(D, warn_unreachable_return) ||
+      isEnabled(D, warn_unreachable_loop_increment);
 
-  DefaultPolicy.enableThreadSafetyAnalysis =
-    isEnabled(D, warn_double_lock);
+  DefaultPolicy.enableThreadSafetyAnalysis = isEnabled(D, warn_double_lock);
 
   DefaultPolicy.enableConsumedAnalysis =
-    isEnabled(D, warn_use_in_invalid_state);
+      isEnabled(D, warn_use_in_invalid_state);
 }
 
+// We need this here for unique_ptr with forward declared class.
+sema::AnalysisBasedWarnings::~AnalysisBasedWarnings() = default;
+
 static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) {
   for (const auto &D : fscope->PossiblyUnreachableDiags)
     S.Diag(D.Loc, D.PD);
 }
 
-void clang::sema::
-AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
-                                     sema::FunctionScopeInfo *fscope,
-                                     const Decl *D, QualType BlockType) {
+void clang::sema::AnalysisBasedWarnings::IssueWarnings(
+    sema::AnalysisBasedWarnings::Policy P, sema::FunctionScopeInfo *fscope,
+    const Decl *D, QualType BlockType) {
 
   // We avoid doing analysis-based warnings when there are errors for
   // two reasons:
@@ -2346,7 +2406,7 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
   if (S.getLangOpts().ObjC &&
       shouldAnalyzeCalledOnceParameters(Diags, D->getBeginLoc())) {
     if (AC.getCFG()) {
-      CalledOnceCheckReporter Reporter(S);
+      CalledOnceCheckReporter Reporter(S, IPData->CalledOnceData);
       checkCalledOnceParameters(
           AC, Reporter,
           shouldAnalyzeCalledOnceConventions(Diags, D->getBeginLoc()));

diff  --git a/clang/test/SemaObjC/warn-called-once.m b/clang/test/SemaObjC/warn-called-once.m
index 7d0679035238..825d491f53bb 100644
--- a/clang/test/SemaObjC/warn-called-once.m
+++ b/clang/test/SemaObjC/warn-called-once.m
@@ -31,6 +31,16 @@ @interface NSMutableArray<ObjectType> : NSArray <ObjectType>
 @class NSString, Protocol;
 extern void NSLog(NSString *format, ...);
 
+typedef int group_t;
+typedef struct dispatch_queue_s *dispatch_queue_t;
+typedef void (^dispatch_block_t)(void);
+extern dispatch_queue_t queue;
+
+void dispatch_group_async(dispatch_queue_t queue,
+                          group_t group,
+                          dispatch_block_t block);
+void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
+
 void escape(void (^callback)(void));
 void escape_void(void *);
 void indirect_call(void (^callback)(void) CALLED_ONCE);
@@ -225,11 +235,11 @@ void indirect_call_within_direct_call(void (^callback)(void) CALLED_ONCE,
 }
 
 void block_call_1(void (^callback)(void) CALLED_ONCE) {
-  indirect_call(^{
-    callback();
-  });
-  callback();
-  // no-warning
+  indirect_call( // expected-note{{previous call is here}}
+      ^{
+        callback();
+      });
+  callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}}
 }
 
 void block_call_2(void (^callback)(void) CALLED_ONCE) {
@@ -255,7 +265,7 @@ void block_call_4(int cond, void (^callback)(void) CALLED_ONCE) {
       // expected-warning at -1{{'callback' parameter marked 'called_once' is never used when taking false branch}}
       escape(callback);
     }
-  }();
+  }(); // no-warning
 }
 
 void block_call_5(void (^outer)(void) CALLED_ONCE) {
@@ -273,6 +283,32 @@ void block_with_called_once(void (^outer)(void) CALLED_ONCE) {
   outer(); // expected-warning{{'outer' parameter marked 'called_once' is called twice}}
 }
 
+void block_dispatch_call(int cond, void (^callback)(void) CALLED_ONCE) {
+  dispatch_async(queue, ^{
+    if (cond) // expected-warning{{'callback' parameter marked 'called_once' is never called when taking false branch}}
+      callback();
+  });
+}
+
+void block_escape_call_1(int cond, void (^callback)(void) CALLED_ONCE) {
+  escape_void((__bridge void *)^{
+    if (cond) {
+      // no-warning
+      callback();
+    }
+  });
+}
+
+void block_escape_call_2(int cond, void (^callback)(void) CALLED_ONCE) {
+  escape_void((__bridge void *)^{
+    if (cond) {
+      callback(); // expected-note{{previous call is here}}
+    }
+    // Double call can still be reported.
+    callback(); // expected-warning{{'callback' 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;
@@ -822,11 +858,10 @@ - (void)escaped_one_path:(int)cond callback:(void (^)(void))CALLED_ONCE callback
 
 - (void)block_call_1:(void (^)(void))CALLED_ONCE callback {
   // We consider captures by blocks as escapes
-  [self indirect_call:(^{
+  [self indirect_call:(^{ // expected-note{{previous call is here}}
           callback();
         })];
-  callback();
-  // no-warning
+  callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}}
 }
 
 - (void)block_call_2:(int)cond callback:(void (^)(void))CALLED_ONCE callback {


        


More information about the cfe-commits mailing list