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

[vsavchenko]

Summary

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

[NoQ]

clang/lib/Sema/AnalysisBasedWarnings.cpp

1729

Do i understand correctly that you're relying on the order in which your analysis is invoked from Sema? I.e., Sema parses the inner block before the outer function, so it'll analyze the block first, so by the time you see a block expression in the surrounding function you'll already have all diagnostics for the block readily available and no other diagnostics will ever show up, right?

2206

Do we really need manual new-delete here? Ownership semantics don't seem to be that complicated, a unique_ptr would probably suffice. Or maybe even just store it by value.

[vsavchenko]

clang/lib/Sema/AnalysisBasedWarnings.cpp

1729

Exactly, function is analyzed the moment it's fully parsed.

2206

I definitely don't want to move InterProceduralData definition to the header, so value is not an option.
std::unique_ptr will definitely work, but we still need to move the destructor into the cpp file because of the forward declaration as far as I understand.

[vsavchenko]

Replace manual memory management of IPData with std::unique_ptr

[NoQ]

Great, thanks!!

clang/test/SemaObjC/warn-called-once.m

861

What would it take to move this note to the callback(); line? It would be great to do so because blocks are often huge and ^ is often hard to notice.

[vsavchenko]

clang/test/SemaObjC/warn-called-once.m

861

It will be tricky not to say more. We will need to store the call after we finish analyzing the block. The tricky part is that technically it might not have any "calls", but simply escapes.
If one day we track aliases of our parameters (and the capturing block is an alias), then such blocks might be called multiple times on different paths and we'll need to report the call site of the block for sure.

clang/include/clang/Analysis/Analyses/CalledOnceCheck.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_ANALYSIS_ANALYSES_CALLEDONCECHECK_H
 #define LLVM_CLANG_ANALYSIS_ANALYSES_CALLEDONCECHECK_H
 
 namespace clang {
 
 class AnalysisDeclContext;
+class BlockDecl;
 class CFG;
 class Decl;
 class DeclContext;
 class Expr;
 class ParmVarDecl;
 class Stmt;
 
 /// Classification of situations when parameter is not called on every path.
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 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
   /// 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,
+                                 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.
 ///
 /// It traverses the function and tracks how such parameters are used.
 /// It detects two main violations:
 ///   * parameter is called twice
 ///   * parameter is not called

clang/include/clang/Sema/AnalysisBasedWarnings.h

public:
     Policy();
     void disableCheckFallThrough() { enableCheckFallThrough = 0; }
   };
 
 private:
   Sema &S;
   Policy DefaultPolicy;
 
+  class InterProceduralData;
+  InterProceduralData *IPData;
+
   enum VisitFlag { NotVisited = 0, Visited = 1, Pending = 2 };
   llvm::DenseMap<const FunctionDecl*, VisitFlag> VisitedFD;
 
   /// \name Statistics
   /// @{
 
   /// Number of function CFGs built and analyzed.
   unsigned NumFunctionsAnalyzed;
private:
   /// Max number of block visits during uninitialized use analysis of
   /// a single function.
   unsigned MaxUninitAnalysisBlockVisitsPerFunction;
 
   /// @}
 
 public:
   AnalysisBasedWarnings(Sema &s);
+  ~AnalysisBasedWarnings();
 
   void IssueWarnings(Policy P, FunctionScopeInfo *fscope,
                      const Decl *D, QualType BlockType);
 
   Policy getDefaultPolicy() { return DefaultPolicy; }
 
   void PrintStats() const;
 };
 
-}} // end namespace clang::sema
+} // namespace sema
+} // namespace clang
 
 #endif

clang/lib/Analysis/CalledOnceCheck.cpp

 //===- 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/ASTContext.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"
constexpr llvm::StringLiteral CONVENTIONAL_NAMES[] = {
     "withCompletion",    "completionBlock", "withCompletionBlock",
     "replyTo",           "reply",           "withReplyTo"};
 constexpr llvm::StringLiteral CONVENTIONAL_SUFFIXES[] = {
     "WithCompletionHandler", "WithCompletion", "WithCompletionBlock",
     "WithReplyTo", "WithReply"};
 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.
   // 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
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));
+          Handler.handleNeverCalled(
+              Parameter, AC.getDecl(), Clarification->Location,
+              Clarification->Reason, !IsEscape, !isExplicitlyMarked(Parameter));
         }
       }
     }
   }
 
   //===----------------------------------------------------------------------===//
   //                   Predicate functions to check parameters
   //===----------------------------------------------------------------------===//
private:
 
   bool isCaptured(const ParmVarDecl *Parameter) const {
     if (const BlockDecl *Block = dyn_cast<BlockDecl>(AC.getDecl())) {
       return Block->capturesVariable(Parameter);
     }
     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.
   ///
   /// 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
void VisitObjCMessageExpr(const ObjCMessageExpr *Message) {
     if (const Expr *Receiver = Message->getInstanceReceiver()) {
       checkEscapee(Receiver);
     }
     // Message expressions unlike calls, could not be direct.
     checkIndirectCall(Message);
   }
 
   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 difficult 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);
+          }
+        }
       }
     }
   }
 
   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

clang/lib/Sema/AnalysisBasedWarnings.cpp

for (const auto &Use : Uses) {
         continue;
       S.Diag(Use.getUseExpr()->getBeginLoc(),
              diag::note_arc_weak_also_accessed_here)
           << Use.getUseExpr()->getSourceRange();
     }
   }
 }
 
+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;
   typedef SmallVector<UninitUse, 2> UsesVec;
   typedef llvm::PointerIntPair<UsesVec *, 1, bool> MappedType;
   // Prefer using MapVector to DenseMap, so that iteration order will be
   // the same as insertion order. This is needed to obtain a deterministic
   // order of diagnostics when calling flushDiagnostics().
static bool hasAlwaysUninitializedUse(const UsesVec* vec) {
     return std::any_of(vec->begin(), vec->end(), [](const UninitUse &U) {
       return U.getKind() == UninitUse::Always ||
              U.getKind() == UninitUse::AfterCall ||
              U.getKind() == UninitUse::AfterDecl;
     });
   }
 };
 
+/// 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 {
     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,
+  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,
                                  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;
   }
 
+  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[] = {
     diag::warn_called_once_never_called,
     diag::warn_called_once_never_called_when,
     diag::warn_called_once_gets_called_twice};
 
 constexpr unsigned CompletionHandlerWarnings[]{

 
 bool shouldAnalyzeCalledOnceParameters(const DiagnosticsEngine &Diags,
                                        SourceLocation At) {
   return shouldAnalyzeCalledOnceImpl(CalledOnceWarnings, Diags, At) ||
          shouldAnalyzeCalledOnceConventions(Diags, At);
 }
 } // 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
 //===----------------------------------------------------------------------===//
 namespace clang {
 namespace threadSafety {
 namespace {
 class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler {
   Sema &S;

 } // namespace consumed
 } // namespace clang
 
 //===----------------------------------------------------------------------===//
 // AnalysisBasedWarnings - Worker object used by Sema to execute analysis-based
 //  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 different 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),
+sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
+    : S(s), IPData(new 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) || 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);
 }
 
+sema::AnalysisBasedWarnings::~AnalysisBasedWarnings() { delete IPData; }
+
 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,
 
   // We avoid doing analysis-based warnings when there are errors for
   // two reasons:
   // (1) The CFGs often can't be constructed (if the body is invalid), so
   //     don't bother trying.
   // (2) The code already has problems; running the analysis just takes more
   //     time.
   DiagnosticsEngine &Diags = S.getDiagnostics();
if (CFG *cfg = AC.getCFG()) {
       }
     }
   }
 
   // Check for violations of "called once" parameter properties.
   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()));
     }
   }
 
   bool FallThroughDiagFull =
       !Diags.isIgnored(diag::warn_unannotated_fallthrough, D->getBeginLoc());

clang/test/SemaObjC/warn-called-once.m

 - (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state objects:(id *)stackbuf count:(NSUInteger)len;
 @end
 @interface NSMutableArray<ObjectType> : NSArray <ObjectType>
 - addObject:anObject;
 @end
 @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);
 void indirect_conv(void (^completionHandler)(void));
 void filler(void);
 void exit(int) NORETURN;
 
 void double_call_one_block(void (^callback)(void) CALLED_ONCE) {
void indirect_call_within_direct_call(void (^callback)(void) CALLED_ONCE,
   //       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(^{
+  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(void (^callback)(void) CALLED_ONCE) {
   escape(^{
     callback();
   });
   callback();
   // no-warning

 }
 
 void block_call_4(int cond, void (^callback)(void) CALLED_ONCE) {
   ^{
     if (cond) {
       // expected-warning@-1{{'callback' parameter marked 'called_once' is never used when taking false branch}}
       escape(callback);
     }
-  }();
+  }(); // no-warning
 }
 
 void block_call_5(void (^outer)(void) CALLED_ONCE) {
   ^(void (^inner)(void) CALLED_ONCE) {
     // expected-warning@-1{{'inner' parameter marked 'called_once' is never called}}
   }(outer);
 }
 
 void block_with_called_once(void (^outer)(void) CALLED_ONCE) {
   escape_void((__bridge void *)^(void (^inner)(void) CALLED_ONCE) {
     inner(); // expected-note{{previous call is here}}
     inner(); // expected-warning{{'inner' parameter marked 'called_once' is called twice}}
   });
   outer(); // expected-note{{previous call is here}}
   outer(); // expected-warning{{'outer' parameter marked 'called_once' is called twice}}
 }
 
+void 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;
 
   callback();
 }
 
 void never_called_if_then_1(int cond, void (^callback)(void) CALLED_ONCE) {
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:(^{
+  [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 {
   [self indirect_call:
             ^{
               if (cond) {
                 // expected-warning@-1{{'callback' parameter marked 'called_once' is never used when taking false branch}}
                 [self escape:callback];