[analyzer] Do not run visitors until the fixpoint, run only once.

In the current implementation, we run visitors until the fixed point is
reached.
That is, if a visitor adds another visitor, the currently processed path
is destroyed, all diagnostics is discarded, and it is regenerated again,
until it's no longer modified.
This pattern has a few negative implications:

 - This loop does not even guarantee to terminate.
   E.g. just imagine two visitors bouncing a diagnostics around.
 - Performance-wise, e.g. for sqlite3 all visitors are being re-run at
   least 10 times for some bugs.
   We have already seen a few reports where it leads to timeouts.
 - If we want to add more computationally intense visitors, this will
   become worse.
 - From architectural standpoint, the current layout requires copying
   visitors, which is conceptually wrong, and can be annoying (e.g. no
   unique_ptr on visitors allowed).

The proposed change is a much simpler architecture: the outer loop
processes nodes upwards, and whenever the visitor is added it only
processes current nodes and above, thus guaranteeing termination.

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@335666 91177308-0d34-0410-b5e6-96231b3b80d8

20 files changed, 418 insertions, 427 deletions

diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
index 67d2dedc15..9f0a83d255 100644
--- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -23,6 +23,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FoldingSet.h"
@@ -65,6 +66,11 @@ class SValBuilder;
 // Interface for individual bug reports.
 //===----------------------------------------------------------------------===//
 
+/// A mapping from diagnostic consumers to the diagnostics they should
+/// consume.
+using DiagnosticForConsumerMapTy =
+    llvm::DenseMap<PathDiagnosticConsumer *, std::unique_ptr<PathDiagnostic>>;
+
 /// This class provides an interface through which checkers can create
 /// individual bug reports.
 class BugReport : public llvm::ilist_node<BugReport> {
@@ -130,10 +136,6 @@ protected:
   /// Used for ensuring the visitors are only added once.
   llvm::FoldingSet<BugReporterVisitor> CallbacksSet;
 
-  /// Used for clients to tell if the report's configuration has changed
-  /// since the last time they checked.
-  unsigned ConfigurationChangeToken = 0;
-  
   /// When set, this flag disables all callstack pruning from a diagnostic
   /// path.  This is useful for some reports that want maximum fidelty
   /// when reporting an issue.
@@ -229,10 +231,6 @@ public:
   bool isInteresting(SVal V);
   bool isInteresting(const LocationContext *LC);
 
-  unsigned getConfigurationChangeToken() const {
-    return ConfigurationChangeToken;
-  }
-
   /// Returns whether or not this report should be considered valid.
   ///
   /// Invalid reports are those that have been classified as likely false
@@ -345,6 +343,9 @@ public:
   /// registerVarDeclsLastStore().
   void addVisitor(std::unique_ptr<BugReporterVisitor> visitor);
 
+  /// Remove all visitors attached to this bug report.
+  void clearVisitors();
+
   /// Iterators through the custom diagnostic visitors.
   visitor_iterator visitor_begin() { return Callbacks.begin(); }
   visitor_iterator visitor_end() { return Callbacks.end(); }
@@ -406,6 +407,8 @@ public:
 /// BugReporter is a utility class for generating PathDiagnostics for analysis.
 /// It collects the BugReports and BugTypes and knows how to generate
 /// and flush the corresponding diagnostics.
+///
+/// The base class is used for generating path-insensitive
 class BugReporter {
 public:
   enum Kind { BaseBRKind, GRBugReporterKind };
@@ -422,11 +425,11 @@ private:
   /// Generate and flush the diagnostics for the given bug report.
   void FlushReport(BugReportEquivClass& EQ);
 
-  /// Generate and flush the diagnostics for the given bug report
-  /// and PathDiagnosticConsumer.
-  void FlushReport(BugReport *exampleReport,
-                   PathDiagnosticConsumer &PD,
-                   ArrayRef<BugReport*> BugReports);
+  /// Generate the diagnostics for the given bug report.
+  std::unique_ptr<DiagnosticForConsumerMapTy>
+  generateDiagnosticForConsumerMap(BugReport *exampleReport,
+                                   ArrayRef<PathDiagnosticConsumer *> consumers,
+                                   ArrayRef<BugReport *> bugReports);
 
   /// The set of bug reports tracked by the BugReporter.
   llvm::FoldingSet<BugReportEquivClass> EQClasses;
@@ -472,10 +475,10 @@ public:
 
   AnalyzerOptions &getAnalyzerOptions() { return D.getAnalyzerOptions(); }
 
-  virtual bool generatePathDiagnostic(PathDiagnostic& pathDiagnostic,
-                                      PathDiagnosticConsumer &PC,
-                                      ArrayRef<BugReport *> &bugReports) {
-    return true;
+  virtual std::unique_ptr<DiagnosticForConsumerMapTy>
+  generatePathDiagnostics(ArrayRef<PathDiagnosticConsumer *> consumers,
+                          ArrayRef<BugReport *> &bugReports) {
+    return {};
   }
 
   void Register(BugType *BT);
@@ -506,7 +509,7 @@ private:
                              StringRef category);
 };
 
-// FIXME: Get rid of GRBugReporter.  It's the wrong abstraction.
+/// GRBugReporter is used for generating path-sensitive reports.
 class GRBugReporter : public BugReporter {
   ExprEngine& Eng;
 
@@ -528,16 +531,14 @@ public:
   ///  engine.
   ProgramStateManager &getStateManager();
 
-  /// Generates a path corresponding to one of the given bug reports.
-  ///
-  /// Which report is used for path generation is not specified. The
-  /// bug reporter will try to pick the shortest path, but this is not
-  /// guaranteed.
+  /// \p bugReports A set of bug reports within a *single* equivalence class
   ///
-  /// \return True if the report was valid and a path was generated,
-  ///         false if the reports should be considered invalid.
-  bool generatePathDiagnostic(PathDiagnostic &PD, PathDiagnosticConsumer &PC,
-                              ArrayRef<BugReport*> &bugReports) override;
+  /// \return A mapping from consumers to the corresponding diagnostics.
+  /// Iterates through the bug reports within a single equivalence class,
+  /// stops at a first non-invalidated report.
+  std::unique_ptr<DiagnosticForConsumerMapTy>
+  generatePathDiagnostics(ArrayRef<PathDiagnosticConsumer *> consumers,
+                          ArrayRef<BugReport *> &bugReports) override;
 
   /// classof - Used by isa<>, cast<>, and dyn_cast<>.
   static bool classof(const BugReporter* R) {
@@ -545,13 +546,27 @@ public:
   }
 };
 
+
+class NodeMapClosure : public BugReport::NodeResolver {
+  InterExplodedGraphMap &M;
+
+public:
+  NodeMapClosure(InterExplodedGraphMap &m) : M(m) {}
+
+  const ExplodedNode *getOriginalNode(const ExplodedNode *N) override {
+    return M.lookup(N);
+  }
+};
+
 class BugReporterContext {
   GRBugReporter &BR;
+  NodeMapClosure NMC;
 
   virtual void anchor();
 
 public:
-  BugReporterContext(GRBugReporter& br) : BR(br) {}
+  BugReporterContext(GRBugReporter &br, InterExplodedGraphMap &Backmap)
+      : BR(br), NMC(Backmap) {}
 
   virtual ~BugReporterContext() = default;
 
@@ -575,7 +590,7 @@ public:
     return BR.getSourceManager();
   }
 
-  virtual BugReport::NodeResolver& getNodeResolver() = 0;
+  NodeMapClosure& getNodeResolver() { return NMC; }
 };
 
 } // namespace ento
diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
index 9662d91fea..031a76c7bd 100644
--- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -40,12 +40,6 @@ class MemRegion;
 class PathDiagnosticPiece;
 
 /// BugReporterVisitors are used to add custom diagnostics along a path.
-///
-/// Custom visitors should subclass the BugReporterVisitorImpl class for a
-/// default implementation of the clone() method.
-/// (Warning: if you have a deep subclass of BugReporterVisitorImpl, the
-/// default implementation of clone() will NOT do the right thing, and you
-/// will have to provide your own implementation.)
 class BugReporterVisitor : public llvm::FoldingSetNode {
 public:
   BugReporterVisitor() = default;
@@ -53,19 +47,13 @@ public:
   BugReporterVisitor(BugReporterVisitor &&) {}
   virtual ~BugReporterVisitor();
 
-  /// Returns a copy of this BugReporter.
-  ///
-  /// Custom BugReporterVisitors should not override this method directly.
-  /// Instead, they should inherit from BugReporterVisitorImpl and provide
-  /// a protected or public copy constructor.
-  ///
-  /// (Warning: if you have a deep subclass of BugReporterVisitorImpl, the
-  /// default implementation of clone() will NOT do the right thing, and you
-  /// will have to provide your own implementation.)
-  virtual std::unique_ptr<BugReporterVisitor> clone() const = 0;
-
   /// Return a diagnostic piece which should be associated with the
   /// given node.
+  /// Note that this function does *not* get run on the very last node
+  /// of the report, as the PathDiagnosticPiece associated with the
+  /// last node should be unique.
+  /// Use {@code getEndPath} to customize the note associated with the report
+  /// end instead.
   ///
   /// The last parameter can be used to register a new visitor with the given
   /// BugReport while processing a node.
@@ -84,34 +72,18 @@ public:
   ///
   /// NOTE that this function can be implemented on at most one used visitor,
   /// and otherwise it crahes at runtime.
-  virtual std::unique_ptr<PathDiagnosticPiece>
+  virtual std::shared_ptr<PathDiagnosticPiece>
   getEndPath(BugReporterContext &BRC, const ExplodedNode *N, BugReport &BR);
 
   virtual void Profile(llvm::FoldingSetNodeID &ID) const = 0;
 
   /// Generates the default final diagnostic piece.
-  static std::unique_ptr<PathDiagnosticPiece>
+  static std::shared_ptr<PathDiagnosticPiece>
   getDefaultEndPath(BugReporterContext &BRC, const ExplodedNode *N,
                     BugReport &BR);
 };
 
-/// This class provides a convenience implementation for clone() using the
-/// Curiously-Recurring Template Pattern. If you are implementing a custom
-/// BugReporterVisitor, subclass BugReporterVisitorImpl and provide a public
-/// or protected copy constructor.
-///
-/// (Warning: if you have a deep subclass of BugReporterVisitorImpl, the
-/// default implementation of clone() will NOT do the right thing, and you
-/// will have to provide your own implementation.)
-template <class DERIVED>
-class BugReporterVisitorImpl : public BugReporterVisitor {
-  std::unique_ptr<BugReporterVisitor> clone() const override {
-    return llvm::make_unique<DERIVED>(*static_cast<const DERIVED *>(this));
-  }
-};
-
-class FindLastStoreBRVisitor final
-    : public BugReporterVisitorImpl<FindLastStoreBRVisitor> {
+class FindLastStoreBRVisitor final : public BugReporterVisitor {
   const MemRegion *R;
   SVal V;
   bool Satisfied = false;
@@ -138,8 +110,7 @@ public:
                                                  BugReport &BR) override;
 };
 
-class TrackConstraintBRVisitor final
-    : public BugReporterVisitorImpl<TrackConstraintBRVisitor> {
+class TrackConstraintBRVisitor final : public BugReporterVisitor {
   DefinedSVal Constraint;
   bool Assumption;
   bool IsSatisfied = false;
@@ -172,8 +143,7 @@ private:
 
 /// \class NilReceiverBRVisitor
 /// Prints path notes when a message is sent to a nil receiver.
-class NilReceiverBRVisitor final
-    : public BugReporterVisitorImpl<NilReceiverBRVisitor> {
+class NilReceiverBRVisitor final : public BugReporterVisitor {
 public:
   void Profile(llvm::FoldingSetNodeID &ID) const override {
     static int x = 0;
@@ -191,8 +161,7 @@ public:
 };
 
 /// Visitor that tries to report interesting diagnostics from conditions.
-class ConditionBRVisitor final
-    : public BugReporterVisitorImpl<ConditionBRVisitor> {
+class ConditionBRVisitor final : public BugReporterVisitor {
   // FIXME: constexpr initialization isn't supported by MSVC2013.
   static const char *const GenericTrueMessage;
   static const char *const GenericFalseMessage;
@@ -255,7 +224,7 @@ public:
 ///
 /// Currently this suppresses reports based on locations of bugs.
 class LikelyFalsePositiveSuppressionBRVisitor final
-    : public BugReporterVisitorImpl<LikelyFalsePositiveSuppressionBRVisitor> {
+    : public BugReporterVisitor {
 public:
   static void *getTag() {
     static int Tag = 0;
@@ -282,8 +251,7 @@ public:
 ///
 /// As a result, BugReporter will not prune the path through the function even
 /// if the region's contents are not modified/accessed by the call.
-class UndefOrNullArgVisitor final
-    : public BugReporterVisitorImpl<UndefOrNullArgVisitor> {
+class UndefOrNullArgVisitor final : public BugReporterVisitor {
   /// The interesting memory region this visitor is tracking.
   const MemRegion *R;
 
@@ -302,8 +270,7 @@ public:
                                                  BugReport &BR) override;
 };
 
-class SuppressInlineDefensiveChecksVisitor final
-    : public BugReporterVisitorImpl<SuppressInlineDefensiveChecksVisitor> {
+class SuppressInlineDefensiveChecksVisitor final : public BugReporterVisitor {
   /// The symbolic value for which we are tracking constraints.
   /// This value is constrained to null in the end of path.
   DefinedSVal V;
@@ -333,8 +300,7 @@ public:
                                                  BugReport &BR) override;
 };
 
-class CXXSelfAssignmentBRVisitor final
-    : public BugReporterVisitorImpl<CXXSelfAssignmentBRVisitor> {
+class CXXSelfAssignmentBRVisitor final : public BugReporterVisitor {
   bool Satisfied = false;
 
 public:
@@ -350,7 +316,7 @@ public:
 
 /// The bug visitor prints a diagnostic message at the location where a given
 /// variable was tainted.
-class TaintBugVisitor final : public BugReporterVisitorImpl<TaintBugVisitor> {
+class TaintBugVisitor final : public BugReporterVisitor {
 private:
   const SVal V;
 
@@ -367,8 +333,7 @@ public:
 /// The bug visitor will walk all the nodes in a path and collect all the
 /// constraints. When it reaches the root node, will create a refutation
 /// manager and check if the constraints are satisfiable
-class FalsePositiveRefutationBRVisitor final
-    : public BugReporterVisitorImpl<FalsePositiveRefutationBRVisitor> {
+class FalsePositiveRefutationBRVisitor final : public BugReporterVisitor {
 private:
   /// Holds the constraints in a given path
   // TODO: should we use a set?
diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
index 244cbfea9e..cca977de88 100644
--- a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -811,7 +811,7 @@ public:
 
   bool isWithinCall() const { return !pathStack.empty(); }
 
-  void setEndOfPath(std::unique_ptr<PathDiagnosticPiece> EndPiece) {
+  void setEndOfPath(std::shared_ptr<PathDiagnosticPiece> EndPiece) {
     assert(!Loc.isValid() && "End location already set!");
     Loc = EndPiece->getLocation();
     assert(Loc.isValid() && "Invalid location for end-of-path piece");
@@ -824,12 +824,6 @@ public:
     VerboseDesc += S;
   }
 
-  void resetPath() {
-    pathStack.clear();
-    pathImpl.clear();
-    Loc = PathDiagnosticLocation();
-  }
-
   /// If the last piece of the report point to the header file, resets
   /// the location of the report to be the last location in the main source
   /// file.
@@ -865,7 +859,6 @@ public:
   filesmap_iterator executedLines_end() const { return ExecutedLines->end(); }
 
   PathDiagnosticLocation getLocation() const {
-    assert(Loc.isValid() && "No report location set yet!");
     return Loc;
   }
 
diff --git a/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp b/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
index 53632b41df..d3489282ab 100644
--- a/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
@@ -39,7 +39,7 @@ class DeleteWithNonVirtualDtorChecker
     : public Checker<check::PreStmt<CXXDeleteExpr>> {
   mutable std::unique_ptr<BugType> BT;
 
-  class DeleteBugVisitor : public BugReporterVisitorImpl<DeleteBugVisitor> {
+  class DeleteBugVisitor : public BugReporterVisitor {
   public:
     DeleteBugVisitor() : Satisfied(false) {}
     void Profile(llvm::FoldingSetNodeID &ID) const override {
diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
index 109897be29..4e4d81cd67 100644
--- a/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
@@ -38,8 +38,7 @@ class DynamicTypeChecker : public Checker<check::PostStmt<ImplicitCastExpr>> {
           new BugType(this, "Dynamic and static type mismatch", "Type Error"));
   }
 
-  class DynamicTypeBugVisitor
-      : public BugReporterVisitorImpl<DynamicTypeBugVisitor> {
+  class DynamicTypeBugVisitor : public BugReporterVisitor {
   public:
     DynamicTypeBugVisitor(const MemRegion *Reg) : Reg(Reg) {}
 
diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
index 71dbe2f4b7..126e57645a 100644
--- a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
+++ b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
@@ -74,7 +74,7 @@ class DynamicTypePropagation:
           new BugType(this, "Generics", categories::CoreFoundationObjectiveC));
   }
 
-  class GenericsBugVisitor : public BugReporterVisitorImpl<GenericsBugVisitor> {
+  class GenericsBugVisitor : public BugReporterVisitor {
   public:
     GenericsBugVisitor(SymbolRef S) : Sym(S) {}
 
diff --git a/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp b/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
index bfdb511e74..849b1193c0 100644
--- a/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
@@ -113,8 +113,7 @@ NonLocalizedStringChecker::NonLocalizedStringChecker() {
 }
 
 namespace {
-class NonLocalizedStringBRVisitor final
-    : public BugReporterVisitorImpl<NonLocalizedStringBRVisitor> {
+class NonLocalizedStringBRVisitor final : public BugReporterVisitor {
 
   const MemRegion *NonLocalizedString;
   bool Satisfied;
diff --git a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h
index 0ee91cca47..40eb0631d7 100644
--- a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h
+++ b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h
@@ -78,7 +78,7 @@ private:
 
   /// Bug visitor class to find the node where the request region was previously
   /// used in order to include it into the BugReport path.
-  class RequestNodeVisitor : public BugReporterVisitorImpl<RequestNodeVisitor> {
+  class RequestNodeVisitor : public BugReporterVisitor {
   public:
     RequestNodeVisitor(const MemRegion *const MemoryRegion,
                        const std::string &ErrText)
diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
index 8ba0bcbcee..b8ef6701c0 100644
--- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
@@ -120,8 +120,7 @@ private:
   /// The bug visitor which allows us to print extra diagnostics along the
   /// BugReport path. For example, showing the allocation site of the leaked
   /// region.
-  class SecKeychainBugVisitor
-    : public BugReporterVisitorImpl<SecKeychainBugVisitor> {
+  class SecKeychainBugVisitor : public BugReporterVisitor {
   protected:
     // The allocated region symbol tracked by the main analysis.
     SymbolRef Sym;
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 2ab817a1cc..b008bcdf4e 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -431,8 +431,7 @@ private:
   /// The bug visitor which allows us to print extra diagnostics along the
   /// BugReport path. For example, showing the allocation site of the leaked
   /// region.
-  class MallocBugVisitor final
-      : public BugReporterVisitorImpl<MallocBugVisitor> {
+  class MallocBugVisitor final : public BugReporterVisitor {
   protected:
     enum NotificationMode {
       Normal,
@@ -511,7 +510,7 @@ private:
                                                    BugReporterContext &BRC,
                                                    BugReport &BR) override;
 
-    std::unique_ptr<PathDiagnosticPiece>
+    std::shared_ptr<PathDiagnosticPiece>
     getEndPath(BugReporterContext &BRC, const ExplodedNode *EndPathNode,
                BugReport &BR) override {
       if (!IsLeak)
@@ -521,7 +520,7 @@ private:
         PathDiagnosticLocation::createEndOfPath(EndPathNode,
                                                 BRC.getSourceManager());
       // Do not add the statement itself as a range in case of leak.
-      return llvm::make_unique<PathDiagnosticEventPiece>(L, BR.getDescription(),
+      return std::make_shared<PathDiagnosticEventPiece>(L, BR.getDescription(),
                                                          false);
     }
 
diff --git a/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp b/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
index c6493fed8a..258eb05ca7 100644
--- a/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
@@ -61,7 +61,7 @@ public:
 
 private:
   enum MisuseKind {MK_FunCall, MK_Copy, MK_Move};
-  class MovedBugVisitor : public BugReporterVisitorImpl<MovedBugVisitor> {
+  class MovedBugVisitor : public BugReporterVisitor {
   public:
     MovedBugVisitor(const MemRegion *R) : Region(R), Found(false) {}
 
diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 94be72b048..7d1ca61c97 100644
--- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -128,8 +128,7 @@ public:
   DefaultBool NeedTracking;
 
 private:
-  class NullabilityBugVisitor
-      : public BugReporterVisitorImpl<NullabilityBugVisitor> {
+  class NullabilityBugVisitor : public BugReporterVisitor {
   public:
     NullabilityBugVisitor(const MemRegion *M) : Region(M) {}
 
diff --git a/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
index 69b19a7859..fcba3b33f3 100644
--- a/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
@@ -62,9 +62,7 @@ private:
 REGISTER_SET_WITH_PROGRAMSTATE(CalledSuperDealloc, SymbolRef)
 
 namespace {
-class SuperDeallocBRVisitor final
-    : public BugReporterVisitorImpl<SuperDeallocBRVisitor> {
-
+class SuperDeallocBRVisitor final : public BugReporterVisitor {
   SymbolRef ReceiverSymbol;
   bool Satisfied;
 
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
index 589054bdaf..78e4b8f250 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -1788,8 +1788,7 @@ namespace {
   //===---------===//
   // Bug Reports.  //
   //===---------===//
-
-  class CFRefReportVisitor : public BugReporterVisitorImpl<CFRefReportVisitor> {
+  class CFRefReportVisitor : public BugReporterVisitor {
   protected:
     SymbolRef Sym;
     const SummaryLogTy &SummaryLog;
@@ -1810,7 +1809,7 @@ namespace {
                                                    BugReporterContext &BRC,
                                                    BugReport &BR) override;
 
-    std::unique_ptr<PathDiagnosticPiece> getEndPath(BugReporterContext &BRC,
+    std::shared_ptr<PathDiagnosticPiece> getEndPath(BugReporterContext &BRC,
                                                     const ExplodedNode *N,
                                                     BugReport &BR) override;
   };
@@ -1821,18 +1820,9 @@ namespace {
                            const SummaryLogTy &log)
        : CFRefReportVisitor(sym, GCEnabled, log) {}
 
-    std::unique_ptr<PathDiagnosticPiece> getEndPath(BugReporterContext &BRC,
+    std::shared_ptr<PathDiagnosticPiece> getEndPath(BugReporterContext &BRC,
                                                     const ExplodedNode *N,
                                                     BugReport &BR) override;
-
-    std::unique_ptr<BugReporterVisitor> clone() const override {
-      // The curiously-recurring template pattern only works for one level of
-      // subclassing. Rather than make a new template base for
-      // CFRefReportVisitor, we simply override clone() to do the right thing.
-      // This could be trouble someday if BugReporterVisitorImpl is ever
-      // used for something else besides a convenient implementation of clone().
-      return llvm::make_unique<CFRefLeakReportVisitor>(*this);
-    }
   };
 
   class CFRefReport : public BugReport {
@@ -2365,14 +2355,14 @@ GetAllocationSite(ProgramStateManager& StateMgr, const ExplodedNode *N,
                         InterestingMethodContext);
 }
 
-std::unique_ptr<PathDiagnosticPiece>
+std::shared_ptr<PathDiagnosticPiece>
 CFRefReportVisitor::getEndPath(BugReporterContext &BRC,
                                const ExplodedNode *EndN, BugReport &BR) {
   BR.markInteresting(Sym);
   return BugReporterVisitor::getDefaultEndPath(BRC, EndN, BR);
 }
 
-std::unique_ptr<PathDiagnosticPiece>
+std::shared_ptr<PathDiagnosticPiece>
 CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC,
                                    const ExplodedNode *EndN, BugReport &BR) {
 
@@ -2459,7 +2449,7 @@ CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC,
     os << " is not referenced later in this execution path and has a retain "
           "count of +" << RV->getCount();
 
-  return llvm::make_unique<PathDiagnosticEventPiece>(L, os.str());
+  return std::make_shared<PathDiagnosticEventPiece>(L, os.str());
 }
 
 void CFRefLeakReport::deriveParamLocation(CheckerContext &Ctx, SymbolRef sym) {
diff --git a/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp b/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
index b83dea772b..eac743349d 100644
--- a/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
@@ -55,7 +55,7 @@ public:
   }
 };
 
-class DivisionBRVisitor : public BugReporterVisitorImpl<DivisionBRVisitor> {
+class DivisionBRVisitor : public BugReporterVisitor {
 private:
   SymbolRef ZeroSymbol;
   const StackFrameContext *SFC;
diff --git a/lib/StaticAnalyzer/Checkers/ValistChecker.cpp b/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
index 039bc08c5c..bd657340fc 100644
--- a/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -69,7 +69,7 @@ private:
                             bool IsCopy) const;
   void checkVAListEndCall(const CallEvent &Call, CheckerContext &C) const;
 
-  class ValistBugVisitor : public BugReporterVisitorImpl<ValistBugVisitor> {
+  class ValistBugVisitor : public BugReporterVisitor {
   public:
     ValistBugVisitor(const MemRegion *Reg, bool IsLeak = false)
         : Reg(Reg), IsLeak(IsLeak) {}
@@ -78,7 +78,7 @@ private:
       ID.AddPointer(&X);
       ID.AddPointer(Reg);
     }
-    std::unique_ptr<PathDiagnosticPiece>
+    std::shared_ptr<PathDiagnosticPiece>
     getEndPath(BugReporterContext &BRC, const ExplodedNode *EndPathNode,
                BugReport &BR) override {
       if (!IsLeak)
@@ -87,8 +87,7 @@ private:
       PathDiagnosticLocation L = PathDiagnosticLocation::createEndOfPath(
           EndPathNode, BRC.getSourceManager());
       // Do not add the statement itself as a range in case of leak.
-      return llvm::make_unique<PathDiagnosticEventPiece>(L, BR.getDescription(),
-                                                         false);
+      return std::make_shared<PathDiagnosticEventPiece>(L, BR.getDescription(), false);
     }
     std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
                                                    const ExplodedNode *PrevN,
diff --git a/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp b/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
index c5010f5378..71703d6cd1 100644
--- a/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -57,7 +57,7 @@ private:
   void reportBug(StringRef Msg, bool PureError, const MemRegion *Reg,
                  CheckerContext &C) const;
 
-  class VirtualBugVisitor : public BugReporterVisitorImpl<VirtualBugVisitor> {
+  class VirtualBugVisitor : public BugReporterVisitor {
   private:
     const MemRegion *ObjectRegion;
     bool Found;
diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp
index 94d08b4935..2a53d57d6c 100644
--- a/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -343,21 +343,9 @@ static void removePiecesWithInvalidLocations(PathPieces &Pieces) {
 
 namespace {
 
-class NodeMapClosure : public BugReport::NodeResolver {
-  InterExplodedGraphMap &M;
-
-public:
-  NodeMapClosure(InterExplodedGraphMap &m) : M(m) {}
-
-  const ExplodedNode *getOriginalNode(const ExplodedNode *N) override {
-    return M.lookup(N);
-  }
-};
-
 class PathDiagnosticBuilder : public BugReporterContext {
   BugReport *R;
   PathDiagnosticConsumer *PDC;
-  NodeMapClosure NMC;
 
 public:
   const LocationContext *LC;
@@ -365,7 +353,7 @@ public:
   PathDiagnosticBuilder(GRBugReporter &br,
                         BugReport *r, InterExplodedGraphMap &Backmap,
                         PathDiagnosticConsumer *pdc)
-      : BugReporterContext(br), R(r), PDC(pdc), NMC(Backmap),
+      : BugReporterContext(br, Backmap), R(r), PDC(pdc),
         LC(r->getErrorNode()->getLocationContext()) {}
 
   PathDiagnosticLocation ExecutionContinues(const ExplodedNode *N);
@@ -383,8 +371,6 @@ public:
     return getParentMap().getParent(S);
   }
 
-  NodeMapClosure& getNodeResolver() override { return NMC; }
-
   PathDiagnosticLocation getEnclosingStmtLocation(const Stmt *S);
 
   PathDiagnosticConsumer::PathGenerationScheme getGenerationScheme() const {
@@ -1021,6 +1007,9 @@ static const char StrLoopRangeEmpty[] =
 static const char StrLoopCollectionEmpty[] =
   "Loop body skipped when collection is empty";
 
+static std::unique_ptr<FilesToLineNumsMap>
+findExecutedLines(SourceManager &SM, const ExplodedNode *N);
+
 /// Generate diagnostics for the node \p N,
 /// and write it into \p PD.
 /// \p AddPathEdges Whether diagnostic consumer can generate path arrows
@@ -1259,83 +1248,15 @@ static void generatePathDiagnosticsForNode(const ExplodedNode *N,
   }
 }
 
-/// There are two path diagnostics generation modes: with adding edges (used
-/// for plists) and without  (used for HTML and text).
-/// When edges are added (\p ActiveScheme is Extensive),
-/// the path is modified to insert artificially generated
-/// edges.
-/// Otherwise, more detailed diagnostics is emitted for block edges, explaining
-/// the transitions in words.
-static bool generatePathDiagnostics(
-    PathDiagnostic &PD, PathDiagnosticBuilder &PDB, const ExplodedNode *N,
-    LocationContextMap &LCM,
-    ArrayRef<std::unique_ptr<BugReporterVisitor>> visitors,
-    BugReport *R,
-    PathDiagnosticConsumer::PathGenerationScheme ActiveScheme) {
-  const ExplodedNode *LastNode = N;
-  BugReport *report = PDB.getBugReport();
-  StackDiagVector CallStack;
-  InterestingExprs IE;
-  bool AddPathEdges = (ActiveScheme == PathDiagnosticConsumer::Extensive);
-  bool GenerateDiagnostics = (ActiveScheme != PathDiagnosticConsumer::None);
-
-  PathDiagnosticLocation PrevLoc = GenerateDiagnostics ?
-    PD.getLocation() : PathDiagnosticLocation();
-
-  const ExplodedNode *NextNode = N->getFirstPred();
-  while (NextNode) {
-    N = NextNode;
-    NextNode = N->getFirstPred();
-
-    if (GenerateDiagnostics)
-      generatePathDiagnosticsForNode(
-          N, PD, PrevLoc, PDB, LCM, CallStack, IE, AddPathEdges);
-
-    if (!NextNode) {
-      for (auto &V : visitors) {
-        V->finalizeVisitor(PDB, LastNode, *R);
-      }
-      continue;
-    }
-
-    // Add pieces from custom visitors.
-    llvm::FoldingSet<PathDiagnosticPiece> DeduplicationSet;
-    for (auto &V : visitors) {
-      if (auto p = V->VisitNode(N, NextNode, PDB, *report)) {
-
-        if (!GenerateDiagnostics)
-          continue;
-
-        if (DeduplicationSet.GetOrInsertNode(p.get()) != p.get())
-          continue;
-
-        if (AddPathEdges)
-          addEdgeToPath(PD.getActivePath(), PrevLoc, p->getLocation(), PDB.LC);
-        updateStackPiecesWithMessage(*p, CallStack);
-        PD.getActivePath().push_front(std::move(p));
-      }
-    }
-  }
-
-  if (AddPathEdges) {
-    // Add an edge to the start of the function.
-    // We'll prune it out later, but it helps make diagnostics more uniform.
-    const StackFrameContext *CalleeLC = PDB.LC->getCurrentStackFrame();
-    const Decl *D = CalleeLC->getDecl();
-    addEdgeToPath(PD.getActivePath(), PrevLoc,
-                  PathDiagnosticLocation::createBegin(D, PDB.getSourceManager()),
-                  CalleeLC);
-  }
-
-  if (!report->isValid())
-    return false;
-
-  // After constructing the full PathDiagnostic, do a pass over it to compact
-  // PathDiagnosticPieces that occur within a macro.
-  if (!AddPathEdges && GenerateDiagnostics)
-    CompactPathDiagnostic(PD.getMutablePieces(), PDB.getSourceManager());
-
-  return true;
+static std::unique_ptr<PathDiagnostic>
+generateEmptyDiagnosticForReport(BugReport *R, SourceManager &SM) {
+  BugType &BT = R->getBugType();
+  return llvm::make_unique<PathDiagnostic>(
+      R->getBugType().getCheckName(), R->getDeclWithIssue(),
+      R->getBugType().getName(), R->getDescription(),
+      R->getShortDescription(/*Fallback=*/false), BT.getCategory(),
+      R->getUniqueingLocation(), R->getUniqueingDecl(),
+      findExecutedLines(SM, R->getErrorNode()));
 }
 
 static const Stmt *getStmtParent(const Stmt *S, const ParentMap &PM) {
@@ -1957,6 +1878,129 @@ static void dropFunctionEntryEdge(PathPieces &Path, LocationContextMap &LCM,
   Path.pop_front();
 }
 
+using VisitorsDiagnosticsTy = llvm::DenseMap<const ExplodedNode *,
+                   std::vector<std::shared_ptr<PathDiagnosticPiece>>>;
+
+/// This function is responsible for generating diagnostic pieces that are
+/// *not* provided by bug report visitors.
+/// These diagnostics may differ depending on the consumer's settings,
+/// and are therefore constructed separately for each consumer.
+///
+/// There are two path diagnostics generation modes: with adding edges (used
+/// for plists) and without  (used for HTML and text).
+/// When edges are added (\p ActiveScheme is Extensive),
+/// the path is modified to insert artificially generated
+/// edges.
+/// Otherwise, more detailed diagnostics is emitted for block edges, explaining
+/// the transitions in words.
+static std::unique_ptr<PathDiagnostic> generatePathDiagnosticForConsumer(
+    PathDiagnosticConsumer::PathGenerationScheme ActiveScheme,
+    PathDiagnosticBuilder &PDB,
+    const ExplodedNode *ErrorNode,
+    const VisitorsDiagnosticsTy &VisitorsDiagnostics) {
+
+  bool GenerateDiagnostics = (ActiveScheme != PathDiagnosticConsumer::None);
+  bool AddPathEdges = (ActiveScheme == PathDiagnosticConsumer::Extensive);
+  SourceManager &SM = PDB.getSourceManager();
+  BugReport *R = PDB.getBugReport();
+  AnalyzerOptions &Opts = PDB.getBugReporter().getAnalyzerOptions();
+  StackDiagVector CallStack;
+  InterestingExprs IE;
+  LocationContextMap LCM;
+  std::unique_ptr<PathDiagnostic> PD = generateEmptyDiagnosticForReport(R, SM);
+
+  if (GenerateDiagnostics) {
+    auto EndNotes = VisitorsDiagnostics.find(ErrorNode);
+    std::shared_ptr<PathDiagnosticPiece> LastPiece;
+    if (EndNotes != VisitorsDiagnostics.end()) {
+      assert(!EndNotes->second.empty());
+      LastPiece = EndNotes->second[0];
+    } else {
+      LastPiece = BugReporterVisitor::getDefaultEndPath(PDB, ErrorNode, *R);
+    }
+    PD->setEndOfPath(LastPiece);
+  }
+
+  PathDiagnosticLocation PrevLoc = PD->getLocation();
+  const ExplodedNode *NextNode = ErrorNode->getFirstPred();
+  while (NextNode) {
+    if (GenerateDiagnostics)
+      generatePathDiagnosticsForNode(
+          NextNode, *PD, PrevLoc, PDB, LCM, CallStack, IE, AddPathEdges);
+
+    auto VisitorNotes = VisitorsDiagnostics.find(NextNode);
+    NextNode = NextNode->getFirstPred();
+    if (!GenerateDiagnostics || VisitorNotes == VisitorsDiagnostics.end())
+      continue;
+
+    // This is a workaround due to inability to put shared PathDiagnosticPiece
+    // into a FoldingSet.
+    std::set<llvm::FoldingSetNodeID> DeduplicationSet;
+
+    // Add pieces from custom visitors.
+    for (const auto &Note : VisitorNotes->second) {
+      llvm::FoldingSetNodeID ID;
+      Note->Profile(ID);
+      auto P = DeduplicationSet.insert(ID);
+      if (!P.second)
+        continue;
+
+      if (AddPathEdges)
+        addEdgeToPath(PD->getActivePath(), PrevLoc, Note->getLocation(),
+                      PDB.LC);
+      updateStackPiecesWithMessage(*Note, CallStack);
+      PD->getActivePath().push_front(Note);
+    }
+  }
+
+  if (AddPathEdges) {
+    // Add an edge to the start of the function.
+    // We'll prune it out later, but it helps make diagnostics more uniform.
+    const StackFrameContext *CalleeLC = PDB.LC->getCurrentStackFrame();
+    const Decl *D = CalleeLC->getDecl();
+    addEdgeToPath(PD->getActivePath(), PrevLoc,
+                  PathDiagnosticLocation::createBegin(D, SM), CalleeLC);
+  }
+
+  if (!AddPathEdges && GenerateDiagnostics)
+    CompactPathDiagnostic(PD->getMutablePieces(), SM);
+
+  // Finally, prune the diagnostic path of uninteresting stuff.
+  if (!PD->path.empty()) {
+    if (R->shouldPrunePath() && Opts.shouldPrunePaths()) {
+      bool stillHasNotes =
+          removeUnneededCalls(PD->getMutablePieces(), R, LCM);
+      assert(stillHasNotes);
+      (void)stillHasNotes;
+    }
+
+    // Redirect all call pieces to have valid locations.
+    adjustCallLocations(PD->getMutablePieces());
+    removePiecesWithInvalidLocations(PD->getMutablePieces());
+
+    if (AddPathEdges) {
+
+      // Reduce the number of edges from a very conservative set
+      // to an aesthetically pleasing subset that conveys the
+      // necessary information.
+      OptimizedCallsSet OCS;
+      while (optimizeEdges(PD->getMutablePieces(), SM, OCS, LCM)) {}
+
+      // Drop the very first function-entry edge. It's not really necessary
+      // for top-level functions.
+      dropFunctionEntryEdge(PD->getMutablePieces(), LCM, SM);
+    }
+
+    // Remove messages that are basically the same, and edges that may not
+    // make sense.
+    // We have to do this after edge optimization in the Extensive mode.
+    removeRedundantMsgs(PD->getMutablePieces());
+    removeEdgesToDefaultInitializers(PD->getMutablePieces());
+  }
+  return PD;
+}
+
+
 //===----------------------------------------------------------------------===//
 // Methods for BugType and subclasses.
 //===----------------------------------------------------------------------===//
@@ -1979,14 +2023,17 @@ void BugReport::addVisitor(std::unique_ptr<BugReporterVisitor> visitor) {
 
   llvm::FoldingSetNodeID ID;
   visitor->Profile(ID);
-  void *InsertPos;
 
-  if (CallbacksSet.FindNodeOrInsertPos(ID, InsertPos))
+  void *InsertPos = nullptr;
+  if (CallbacksSet.FindNodeOrInsertPos(ID, InsertPos)) {
     return;
+  }
 
-  CallbacksSet.InsertNode(visitor.get(), InsertPos);
   Callbacks.push_back(std::move(visitor));
-  ++ConfigurationChangeToken;
+}
+
+void BugReport::clearVisitors() {
+  Callbacks.clear();
 }
 
 BugReport::~BugReport() {
@@ -2032,9 +2079,7 @@ void BugReport::markInteresting(SymbolRef sym) {
   if (!sym)
     return;
 
-  // If the symbol wasn't already in our set, note a configuration change.
-  if (getInterestingSymbols().insert(sym).second)
-    ++ConfigurationChangeToken;
+  getInterestingSymbols().insert(sym);
 
   if (const auto *meta = dyn_cast<SymbolMetadata>(sym))
     getInterestingRegions().insert(meta->getRegion());
@@ -2044,10 +2089,8 @@ void BugReport::markInteresting(const MemRegion *R) {
   if (!R)
     return;
 
-  // If the base region wasn't already in our set, note a configuration change.
   R = R->getBaseRegion();
-  if (getInterestingRegions().insert(R).second)
-    ++ConfigurationChangeToken;
+  getInterestingRegions().insert(R);
 
   if (const auto *SR = dyn_cast<SymbolicRegion>(R))
     getInterestingSymbols().insert(SR->getSymbol());
@@ -2487,36 +2530,70 @@ static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM) {
   path.insert(path.end(), Pieces.begin(), Pieces.end());
 }
 
-bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD,
-                                           PathDiagnosticConsumer &PC,
-                                           ArrayRef<BugReport *> &bugReports) {
-  assert(!bugReports.empty());
+/// Generate notes from all visitors.
+/// Notes associated with {@code ErrorNode} are generated using
+/// {@code getEndPath}, and the rest are generated with {@code VisitNode}.
+static std::unique_ptr<VisitorsDiagnosticsTy>
+generateVisitorsDiagnostics(BugReport *R, const ExplodedNode *ErrorNode,
+                            BugReporterContext &BRC) {
+  auto Notes = llvm::make_unique<VisitorsDiagnosticsTy>();
+  BugReport::VisitorList visitors;
 
-  bool HasValid = false;
-  bool HasInvalid = false;
-  SmallVector<const ExplodedNode *, 32> errorNodes;
-  for (const auto I : bugReports) {
-    if (I->isValid()) {
-      HasValid = true;
-      errorNodes.push_back(I->getErrorNode());
-    } else {
-      // Keep the errorNodes list in sync with the bugReports list.
-      HasInvalid = true;
-      errorNodes.push_back(nullptr);
+  // Run visitors on all nodes starting from the node *before* the last one.
+  // The last node is reserved for notes generated with {@code getEndPath}.
+  const ExplodedNode *NextNode = ErrorNode->getFirstPred();
+  while (NextNode) {
+
+    // At each iteration, move all visitors from report to visitor list.
+    for (BugReport::visitor_iterator I = R->visitor_begin(),
+                                     E = R->visitor_end();
+         I != E; ++I) {
+      visitors.push_back(std::move(*I));
     }
-  }
+    R->clearVisitors();
 
-  // If all the reports have been marked invalid by a previous path generation,
-  // we're done.
-  if (!HasValid)
-    return false;
+    const ExplodedNode *Pred = NextNode->getFirstPred();
+    if (!Pred) {
+      std::shared_ptr<PathDiagnosticPiece> LastPiece;
+      for (auto &V : visitors) {
+        V->finalizeVisitor(BRC, ErrorNode, *R);
 
-  using PathGenerationScheme = PathDiagnosticConsumer::PathGenerationScheme;
+        if (auto Piece = V->getEndPath(BRC, ErrorNode, *R)) {
+          assert(!LastPiece &&
+                 "There can only be one final piece in a diagnostic.");
+          LastPiece = std::move(Piece);
+          llvm::errs() << "Writing to last piece" << "\n";
+          (*Notes)[ErrorNode].push_back(LastPiece);
+        }
+      }
+      break;
+    }
 
-  PathGenerationScheme ActiveScheme = PC.getGenerationScheme();
+    for (auto &V : visitors) {
+      auto P = V->VisitNode(NextNode, Pred, BRC, *R);
+      if (P)
+        (*Notes)[NextNode].push_back(std::move(P));
+    }
 
-  TrimmedGraph TrimG(&getGraph(), errorNodes);
-  ReportGraph ErrorGraph;
+    if (!R->isValid())
+      break;
+
+    NextNode = Pred;
+  }
+
+  return Notes;
+}
+
+/// Find a non-invalidated report for a given equivalence class,
+/// and return together with a cache of visitors notes.
+/// If none found, return a nullptr paired with an empty cache.
+static
+std::pair<BugReport*, std::unique_ptr<VisitorsDiagnosticsTy>> findValidReport(
+  TrimmedGraph &TrimG,
+  ReportGraph &ErrorGraph,
+  ArrayRef<BugReport *> &bugReports,
+  AnalyzerOptions &Opts,
+  GRBugReporter &Reporter) {
 
   while (TrimG.popNextReportGraph(ErrorGraph)) {
     // Find the BugReport with the original location.
@@ -2524,15 +2601,12 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD,
     BugReport *R = bugReports[ErrorGraph.Index];
     assert(R && "No original report found for sliced graph.");
     assert(R->isValid() && "Report selected by trimmed graph marked invalid.");
-
-    // Start building the path diagnostic...
-    PathDiagnosticBuilder PDB(*this, R, ErrorGraph.BackMap, &PC);
-    const ExplodedNode *N = ErrorGraph.ErrorNode;
+    const ExplodedNode *ErrorNode = ErrorGraph.ErrorNode;
 
     // Register refutation visitors first, if they mark the bug invalid no
     // further analysis is required
     R->addVisitor(llvm::make_unique<LikelyFalsePositiveSuppressionBRVisitor>());
-    if (getAnalyzerOptions().shouldCrosscheckWithZ3())
+    if (Opts.shouldCrosscheckWithZ3())
       R->addVisitor(llvm::make_unique<FalsePositiveRefutationBRVisitor>());
 
     // Register additional node visitors.
@@ -2540,101 +2614,59 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD,
     R->addVisitor(llvm::make_unique<ConditionBRVisitor>());
     R->addVisitor(llvm::make_unique<CXXSelfAssignmentBRVisitor>());
 
-    BugReport::VisitorList visitors;
-    unsigned origReportConfigToken, finalReportConfigToken;
-    LocationContextMap LCM;
-
-    // While generating diagnostics, it's possible the visitors will decide
-    // new symbols and regions are interesting, or add other visitors based on
-    // the information they find. If they do, we need to regenerate the path
-    // based on our new report configuration.
-    do {
-      // Get a clean copy of all the visitors.
-      for (BugReport::visitor_iterator I = R->visitor_begin(),
-                                       E = R->visitor_end(); I != E; ++I)
-        visitors.push_back((*I)->clone());
-
-      // Clear out the active path from any previous work.
-      PD.resetPath();
-      origReportConfigToken = R->getConfigurationChangeToken();
-
-      // Generate the very last diagnostic piece - the piece is visible before
-      // the trace is expanded.
-      std::unique_ptr<PathDiagnosticPiece> LastPiece;
-      for (BugReport::visitor_iterator I = visitors.begin(), E = visitors.end();
-          I != E; ++I) {
-        if (std::unique_ptr<PathDiagnosticPiece> Piece =
-                (*I)->getEndPath(PDB, N, *R)) {
-          assert(!LastPiece &&
-                 "There can only be one final piece in a diagnostic.");
-          LastPiece = std::move(Piece);
-        }
-      }
+    BugReporterContext BRC(Reporter, ErrorGraph.BackMap);
 
-      if (ActiveScheme != PathDiagnosticConsumer::None) {
-        if (!LastPiece)
-          LastPiece = BugReporterVisitor::getDefaultEndPath(PDB, N, *R);
-        assert(LastPiece);
-        PD.setEndOfPath(std::move(LastPiece));
-      }
-
-      // Make sure we get a clean location context map so we don't
-      // hold onto old mappings.
-      LCM.clear();
-
-      generatePathDiagnostics(PD, PDB, N, LCM, visitors, R, ActiveScheme);
-
-      // Clean up the visitors we used.
-      visitors.clear();
+    // Run all visitors on a given graph, once.
+    std::unique_ptr<VisitorsDiagnosticsTy> visitorNotes =
+        generateVisitorsDiagnostics(R, ErrorNode, BRC);
 
-      // Did anything change while generating this path?
-      finalReportConfigToken = R->getConfigurationChangeToken();
-    } while (finalReportConfigToken != origReportConfigToken);
-
-    if (!R->isValid())
-      continue;
-
-    // Finally, prune the diagnostic path of uninteresting stuff.
-    if (!PD.path.empty()) {
-      if (R->shouldPrunePath() && getAnalyzerOptions().shouldPrunePaths()) {
-        bool stillHasNotes = removeUnneededCalls(PD.getMutablePieces(), R, LCM);
-        assert(stillHasNotes);
-        (void)stillHasNotes;
-      }
+    if (R->isValid())
+      return std::make_pair(R, std::move(visitorNotes));
+  }
+  return std::make_pair(nullptr, llvm::make_unique<VisitorsDiagnosticsTy>());
+}
 
-      // Redirect all call pieces to have valid locations.
-      adjustCallLocations(PD.getMutablePieces());
-      removePiecesWithInvalidLocations(PD.getMutablePieces());
+std::unique_ptr<DiagnosticForConsumerMapTy>
+GRBugReporter::generatePathDiagnostics(
+    ArrayRef<PathDiagnosticConsumer *> consumers,
+    ArrayRef<BugReport *> &bugReports) {
+  assert(!bugReports.empty());
 
-      if (ActiveScheme == PathDiagnosticConsumer::Extensive) {
-        SourceManager &SM = getSourceManager();
+  auto Out = llvm::make_unique<DiagnosticForConsumerMapTy>();
+  bool HasValid = false;
+  SmallVector<const ExplodedNode *, 32> errorNodes;
+  for (const auto I : bugReports) {
+    if (I->isValid()) {
+      HasValid = true;
+      errorNodes.push_back(I->getErrorNode());
+    } else {
+      // Keep the errorNodes list in sync with the bugReports list.
+      errorNodes.push_back(nullptr);
+    }
+  }
 
-        // Reduce the number of edges from a very conservative set
-        // to an aesthetically pleasing subset that conveys the
-        // necessary information.
-        OptimizedCallsSet OCS;
-        while (optimizeEdges(PD.getMutablePieces(), SM, OCS, LCM)) {}
+  // If all the reports have been marked invalid by a previous path generation,
+  // we're done.
+  if (!HasValid)
+    return Out;
 
-        // Drop the very first function-entry edge. It's not really necessary
-        // for top-level functions.
-        dropFunctionEntryEdge(PD.getMutablePieces(), LCM, SM);
-      }
+  TrimmedGraph TrimG(&getGraph(), errorNodes);
+  ReportGraph ErrorGraph;
+  auto ReportInfo = findValidReport(TrimG, ErrorGraph, bugReports,
+                  getAnalyzerOptions(), *this);
+  BugReport *R = ReportInfo.first;
 
-      // Remove messages that are basically the same, and edges that may not
-      // make sense.
-      // We have to do this after edge optimization in the Extensive mode.
-      removeRedundantMsgs(PD.getMutablePieces());
-      removeEdgesToDefaultInitializers(PD.getMutablePieces());
+  if (R && R->isValid()) {
+    const ExplodedNode *ErrorNode = ErrorGraph.ErrorNode;
+    for (PathDiagnosticConsumer *PC : consumers) {
+      PathDiagnosticBuilder PDB(*this, R, ErrorGraph.BackMap, PC);
+      std::unique_ptr<PathDiagnostic> PD = generatePathDiagnosticForConsumer(
+          PC->getGenerationScheme(), PDB, ErrorNode, *ReportInfo.second);
+      (*Out)[PC] = std::move(PD);
     }
-
-    // We found a report and didn't suppress it.
-    return true;
   }
 
-  // We suppressed all the reports in this equivalence class.
-  assert(!HasInvalid && "Inconsistent suppression");
-  (void)HasInvalid;
-  return false;
+  return Out;
 }
 
 void BugReporter::Register(BugType *BT) {
@@ -2893,11 +2925,55 @@ FindReportInEquivalenceClass(BugReportEquivClass& EQ,
 
 void BugReporter::FlushReport(BugReportEquivClass& EQ) {
   SmallVector<BugReport*, 10> bugReports;
-  BugReport *exampleReport = FindReportInEquivalenceClass(EQ, bugReports);
-  if (exampleReport) {
-    for (PathDiagnosticConsumer *PDC : getPathDiagnosticConsumers()) {
-      FlushReport(exampleReport, *PDC, bugReports);
+  BugReport *report = FindReportInEquivalenceClass(EQ, bugReports);
+  if (!report)
+    return;
+
+  ArrayRef<PathDiagnosticConsumer*> Consumers = getPathDiagnosticConsumers();
+  std::unique_ptr<DiagnosticForConsumerMapTy> Diagnostics =
+      generateDiagnosticForConsumerMap(report, Consumers, bugReports);
+
+  for (auto &P : *Diagnostics) {
+    PathDiagnosticConsumer *Consumer = P.first;
+    std::unique_ptr<PathDiagnostic> &PD = P.second;
+
+    // If the path is empty, generate a single step path with the location
+    // of the issue.
+    if (PD->path.empty()) {
+      PathDiagnosticLocation L = report->getLocation(getSourceManager());
+      auto piece = llvm::make_unique<PathDiagnosticEventPiece>(
+        L, report->getDescription());
+      for (SourceRange Range : report->getRanges())
+        piece->addRange(Range);
+      PD->setEndOfPath(std::move(piece));
+    }
+
+    PathPieces &Pieces = PD->getMutablePieces();
+    if (getAnalyzerOptions().shouldDisplayNotesAsEvents()) {
+      // For path diagnostic consumers that don't support extra notes,
+      // we may optionally convert those to path notes.
+      for (auto I = report->getNotes().rbegin(),
+           E = report->getNotes().rend(); I != E; ++I) {
+        PathDiagnosticNotePiece *Piece = I->get();
+        auto ConvertedPiece = std::make_shared<PathDiagnosticEventPiece>(
+          Piece->getLocation(), Piece->getString());
+        for (const auto &R: Piece->getRanges())
+          ConvertedPiece->addRange(R);
+
+        Pieces.push_front(std::move(ConvertedPiece));
+      }
+    } else {
+      for (auto I = report->getNotes().rbegin(),
+           E = report->getNotes().rend(); I != E; ++I)
+        Pieces.push_front(*I);
     }
+
+    // Get the meta data.
+    const BugReport::ExtraTextList &Meta = report->getExtraText();
+    for (const auto &i : Meta)
+      PD->addMeta(i);
+
+    Consumer->HandlePathDiagnostic(std::move(PD));
   }
 }
 
@@ -2974,79 +3050,41 @@ findExecutedLines(SourceManager &SM, const ExplodedNode *N) {
   return ExecutedLines;
 }
 
-void BugReporter::FlushReport(BugReport *exampleReport,
-                              PathDiagnosticConsumer &PD,
-                              ArrayRef<BugReport*> bugReports) {
-  // FIXME: Make sure we use the 'R' for the path that was actually used.
-  // Probably doesn't make a difference in practice.
-  BugType& BT = exampleReport->getBugType();
-
-  auto D = llvm::make_unique<PathDiagnostic>(
-      exampleReport->getBugType().getCheckName(),
-      exampleReport->getDeclWithIssue(), exampleReport->getBugType().getName(),
-      exampleReport->getDescription(),
-      exampleReport->getShortDescription(/*Fallback=*/false), BT.getCategory(),
-      exampleReport->getUniqueingLocation(), exampleReport->getUniqueingDecl(),
-      findExecutedLines(getSourceManager(), exampleReport->getErrorNode()));
+std::unique_ptr<DiagnosticForConsumerMapTy>
+BugReporter::generateDiagnosticForConsumerMap(
+    BugReport *report, ArrayRef<PathDiagnosticConsumer *> consumers,
+    ArrayRef<BugReport *> bugReports) {
 
-  if (exampleReport->isPathSensitive()) {
-    // Generate the full path diagnostic, using the generation scheme
-    // specified by the PathDiagnosticConsumer. Note that we have to generate
-    // path diagnostics even for consumers which do not support paths, because
-    // the BugReporterVisitors may mark this bug as a false positive.
-    assert(!bugReports.empty());
+  if (!report->isPathSensitive()) {
+    auto Out = llvm::make_unique<DiagnosticForConsumerMapTy>();
+    for (auto *Consumer : consumers)
+      (*Out)[Consumer] = generateEmptyDiagnosticForReport(report,
+                                                          getSourceManager());
+    return Out;
+  }
 
-    MaxBugClassSize.updateMax(bugReports.size());
+  // Generate the full path sensitive diagnostic, using the generation scheme
+  // specified by the PathDiagnosticConsumer. Note that we have to generate
+  // path diagnostics even for consumers which do not support paths, because
+  // the BugReporterVisitors may mark this bug as a false positive.
+  assert(!bugReports.empty());
+  MaxBugClassSize.updateMax(bugReports.size());
+  std::unique_ptr<DiagnosticForConsumerMapTy> Out =
+    generatePathDiagnostics(consumers, bugReports);
 
-    if (!generatePathDiagnostic(*D.get(), PD, bugReports))
-      return;
+  if (Out->empty())
+    return Out;
 
-    MaxValidBugClassSize.updateMax(bugReports.size());
+  MaxValidBugClassSize.updateMax(bugReports.size());
 
-    // Examine the report and see if the last piece is in a header. Reset the
-    // report location to the last piece in the main source file.
-    AnalyzerOptions &Opts = getAnalyzerOptions();
+  // Examine the report and see if the last piece is in a header. Reset the
+  // report location to the last piece in the main source file.
+  AnalyzerOptions &Opts = getAnalyzerOptions();
+  for (auto const &P : *Out)
     if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll)
-      D->resetDiagnosticLocationToMainFile();
-  }
-
-  // If the path is empty, generate a single step path with the location
-  // of the issue.
-  if (D->path.empty()) {
-    PathDiagnosticLocation L = exampleReport->getLocation(getSourceManager());
-    auto piece = llvm::make_unique<PathDiagnosticEventPiece>(
-        L, exampleReport->getDescription());
-    for (SourceRange Range : exampleReport->getRanges())
-      piece->addRange(Range);
-    D->setEndOfPath(std::move(piece));
-  }
-
-  PathPieces &Pieces = D->getMutablePieces();
-  if (getAnalyzerOptions().shouldDisplayNotesAsEvents()) {
-    // For path diagnostic consumers that don't support extra notes,
-    // we may optionally convert those to path notes.
-    for (auto I = exampleReport->getNotes().rbegin(),
-              E = exampleReport->getNotes().rend(); I != E; ++I) {
-      PathDiagnosticNotePiece *Piece = I->get();
-      auto ConvertedPiece = std::make_shared<PathDiagnosticEventPiece>(
-          Piece->getLocation(), Piece->getString());
-      for (const auto &R: Piece->getRanges())
-        ConvertedPiece->addRange(R);
-
-      Pieces.push_front(std::move(ConvertedPiece));
-    }
-  } else {
-    for (auto I = exampleReport->getNotes().rbegin(),
-              E = exampleReport->getNotes().rend(); I != E; ++I)
-      Pieces.push_front(*I);
-  }
-
-  // Get the meta data.
-  const BugReport::ExtraTextList &Meta = exampleReport->getExtraText();
-  for (const auto &i : Meta)
-    D->addMeta(i);
+      P.second->resetDiagnosticLocationToMainFile();
 
-  PD.HandlePathDiagnostic(std::move(D));
+  return Out;
 }
 
 void BugReporter::EmitBasicReport(const Decl *DeclWithIssue,
diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index fc31d3c1fb..cd13412301 100644
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -177,7 +177,7 @@ const Stmt *bugreporter::GetRetValExpr(const ExplodedNode *N) {
 // Definitions for bug reporter visitors.
 //===----------------------------------------------------------------------===//
 
-std::unique_ptr<PathDiagnosticPiece>
+std::shared_ptr<PathDiagnosticPiece>
 BugReporterVisitor::getEndPath(BugReporterContext &BRC,
                                const ExplodedNode *EndPathNode, BugReport &BR) {
   return nullptr;
@@ -188,7 +188,7 @@ BugReporterVisitor::finalizeVisitor(BugReporterContext &BRC,
                                     const ExplodedNode *EndPathNode,
                                     BugReport &BR) {}
 
-std::unique_ptr<PathDiagnosticPiece> BugReporterVisitor::getDefaultEndPath(
+std::shared_ptr<PathDiagnosticPiece> BugReporterVisitor::getDefaultEndPath(
     BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) {
   PathDiagnosticLocation L =
     PathDiagnosticLocation::createEndOfPath(EndPathNode,BRC.getSourceManager());
@@ -197,12 +197,12 @@ std::unique_ptr<PathDiagnosticPiece> BugReporterVisitor::getDefaultEndPath(
 
   // Only add the statement itself as a range if we didn't specify any
   // special ranges for this report.
-  auto P = llvm::make_unique<PathDiagnosticEventPiece>(
+  auto P = std::make_shared<PathDiagnosticEventPiece>(
       L, BR.getDescription(), Ranges.begin() == Ranges.end());
   for (SourceRange Range : Ranges)
     P->addRange(Range);
 
-  return std::move(P);
+  return P;
 }
 
 /// \return name of the macro inside the location \p Loc.
@@ -234,8 +234,7 @@ namespace {
 /// for which  the region of interest \p RegionOfInterest was passed into,
 /// but not written inside, and it has caused an undefined read or a null
 /// pointer dereference outside.
-class NoStoreFuncVisitor final
-    : public BugReporterVisitorImpl<NoStoreFuncVisitor> {
+class NoStoreFuncVisitor final : public BugReporterVisitor {
   const SubRegion *RegionOfInterest;
   static constexpr const char *DiagnosticsMsg =
       "Returning without writing to '";
@@ -526,8 +525,7 @@ private:
   }
 };
 
-class MacroNullReturnSuppressionVisitor final
-    : public BugReporterVisitorImpl<MacroNullReturnSuppressionVisitor> {
+class MacroNullReturnSuppressionVisitor final : public BugReporterVisitor {
   const SubRegion *RegionOfInterest;
 
 public:
@@ -609,7 +607,7 @@ private:
 ///
 /// This visitor is intended to be used when another visitor discovers that an
 /// interesting value comes from an inlined function call.
-class ReturnVisitor : public BugReporterVisitorImpl<ReturnVisitor> {
+class ReturnVisitor : public BugReporterVisitor {
   const StackFrameContext *StackFrame;
   enum {
     Initial,
diff --git a/test/Analysis/null-deref-path-notes.c b/test/Analysis/null-deref-path-notes.c
index 3fd559df29..c73f64066b 100644
--- a/test/Analysis/null-deref-path-notes.c
+++ b/test/Analysis/null-deref-path-notes.c
@@ -24,14 +24,14 @@ void f2(char *source) {
 }
 
 void f3(char *source) {
-  char *destination = 0; // FIXME: There should be a note here as well.
+  char *destination = 0; // expected-note{{'destination' initialized to a null pointer value}}
   destination = destination + 0; // expected-note{{Null pointer value stored to 'destination'}}
   memcpy(destination, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
                                    // expected-note@-1{{Null pointer argument in call to memory copy function}}
 }
 
 void f4(char *source) {
-  char *destination = 0; // FIXME: There should be a note here as well.
+  char *destination = 0; // expected-note{{'destination' initialized to a null pointer value}}
   destination = destination - 0; // expected-note{{Null pointer value stored to 'destination'}}
   memcpy(destination, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
                                    // expected-note@-1{{Null pointer argument in call to memory copy function}}