[analyzer] Change FindLastStoreBRVisitor to use Tracker

Additionally, this commit completely removes any uses of
FindLastStoreBRVisitor from the analyzer except for the
one in Tracker.

The next step is actually removing this class altogether
from the header file.

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

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

@@ -19,6 +19,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include <list>
@@ -147,6 +148,9 @@ struct StoreInfo {
   const MemRegion *Dest, *Origin;
 };
 
+class Tracker;
+using TrackerRef = llvm::IntrusiveRefCntPtr<Tracker>;
+
 class ExpressionHandler;
 class StoreHandler;
 
@@ -155,7 +159,7 @@ class StoreHandler;
 /// Tracker aimes at providing a sensible set of default behaviors that can be
 /// used by any checker, while providing mechanisms to hook into any part of the
 /// tracking process and insert checker-specific logic.
-class Tracker {
+class Tracker : public llvm::RefCountedBase<Tracker> {
 private:
   using ExpressionHandlerPtr = std::unique_ptr<ExpressionHandler>;
   using StoreHandlerPtr = std::unique_ptr<StoreHandler>;
@@ -164,11 +168,17 @@ private:
   std::list<ExpressionHandlerPtr> ExpressionHandlers;
   std::list<StoreHandlerPtr> StoreHandlers;
 
-public:
+protected:
   /// \param Report The bug report to which visitors should be attached.
   Tracker(PathSensitiveBugReport &Report);
+
+public:
   virtual ~Tracker() = default;
 
+  static TrackerRef create(PathSensitiveBugReport &Report) {
+    return new Tracker(Report);
+  }
+
   PathSensitiveBugReport &getReport() { return Report; }
 
   /// Describes a tracking result with the most basic information of what was
@@ -318,6 +328,18 @@ public:
   Tracker &getParentTracker() { return ParentTracker; }
 };
 
+/// Visitor that tracks expressions and values.
+class TrackingBugReporterVisitor : public BugReporterVisitor {
+private:
+  TrackerRef ParentTracker;
+
+public:
+  TrackingBugReporterVisitor(TrackerRef ParentTracker)
+      : ParentTracker(ParentTracker) {}
+
+  Tracker &getParentTracker() { return *ParentTracker; }
+};
+
 /// Attempts to add visitors to track expression value back to its point of
 /// origin.
 ///
@@ -335,13 +357,31 @@ bool trackExpressionValue(const ExplodedNode *N, const Expr *E,
                           TrackingKind TKind = TrackingKind::Thorough,
                           bool EnableNullFPSuppression = true);
 
+/// Track how the value got stored into the given region and where it came
+/// from.
+///
+/// \param V We're searching for the store where \c R received this value.
+/// \param R The region we're tracking.
+/// \param Opts Tracking options specifying how we want to track the value.
+/// \param Origin Only adds notes when the last store happened in a
+///        different stackframe to this one. Disregarded if the tracking kind
+///        is thorough.
+///        This is useful, because for non-tracked regions, notes about
+///        changes to its value in a nested stackframe could be pruned, and
+///        this visitor can prevent that without polluting the bugpath too
+///        much.
+void trackStoredValue(KnownSVal V, const MemRegion *R,
+                      PathSensitiveBugReport &Report, TrackingOptions Opts = {},
+                      const StackFrameContext *Origin = nullptr);
+
 const Expr *getDerefExpr(const Stmt *S);
 
 } // namespace bugreporter
 
 /// Finds last store into the given region,
 /// which is different from a given symbolic value.
-class FindLastStoreBRVisitor final : public BugReporterVisitor {
+class FindLastStoreBRVisitor final
+    : public bugreporter::TrackingBugReporterVisitor {
   const MemRegion *R;
   SVal V;
   bool Satisfied = false;
@@ -365,11 +405,13 @@ public:
   ///        changes to its value in a nested stackframe could be pruned, and
   ///        this visitor can prevent that without polluting the bugpath too
   ///        much.
-  FindLastStoreBRVisitor(KnownSVal V, const MemRegion *R,
-                         bool InEnableNullFPSuppression, TrackingKind TKind,
+  FindLastStoreBRVisitor(bugreporter::TrackerRef ParentTracker, KnownSVal V,
+                         const MemRegion *R, bool InEnableNullFPSuppression,
+                         TrackingKind TKind,
                          const StackFrameContext *OriginSFC = nullptr)
-      : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression),
-        TKind(TKind), OriginSFC(OriginSFC) {
+      : TrackingBugReporterVisitor(ParentTracker), R(R), V(V),
+        EnableNullFPSuppression(InEnableNullFPSuppression), TKind(TKind),
+        OriginSFC(OriginSFC) {
     assert(R);
   }
 

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

@@ -980,13 +980,12 @@ void RefLeakReport::findBindingToReport(CheckerContext &Ctx,
     // got from one to another.
     //
     // NOTE: We use the actual SVal stored in AllocBindingToReport here because
-    //       FindLastStoreBRVisitor compares SVal's and it can get trickier for
+    //       trackStoredValue compares SVal's and it can get trickier for
     //       something like derived regions if we want to construct SVal from
     //       Sym. Instead, we take the value that is definitely stored in that
-    //       region, thus guaranteeing that FindLastStoreBRVisitor will work.
-    addVisitor<FindLastStoreBRVisitor>(
-        AllVarBindings[0].second.castAs<KnownSVal>(), AllocBindingToReport,
-        false, bugreporter::TrackingKind::Thorough);
+    //       region, thus guaranteeing that trackStoredValue will work.
+    bugreporter::trackStoredValue(AllVarBindings[0].second.castAs<KnownSVal>(),
+                                  AllocBindingToReport, *this);
   } else {
     AllocBindingToReport = AllocFirstBinding;
   }

clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp

@@ -86,9 +86,9 @@ UndefCapturedBlockVarChecker::checkPostStmt(const BlockExpr *BE,
         auto R = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N);
         if (const Expr *Ex = FindBlockDeclRefExpr(BE->getBody(), VD))
           R->addRange(Ex->getSourceRange());
-        R->addVisitor(std::make_unique<FindLastStoreBRVisitor>(
-            *V, VR, /*EnableNullFPSuppression*/ false,
-            bugreporter::TrackingKind::Thorough));
+        bugreporter::trackStoredValue(*V, VR, *R,
+                                      {bugreporter::TrackingKind::Thorough,
+                                       /*EnableNullFPSuppression*/ false});
         R->disablePathPruning();
         // need location of block
         C.emitReport(std::move(R));

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

@@ -1488,8 +1488,8 @@ FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
     if (!IsParam)
       InitE = InitE->IgnoreParenCasts();
 
-    bugreporter::trackExpressionValue(StoreSite, InitE, BR, TKind,
-                                      EnableNullFPSuppression);
+    getParentTracker().track(InitE, StoreSite,
+                             {TKind, EnableNullFPSuppression});
   }
 
   // Let's try to find the region where the value came from.
@@ -1588,8 +1588,8 @@ FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
               dyn_cast_or_null<BlockDataRegion>(V.getAsRegion())) {
           if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) {
             if (auto KV = State->getSVal(OriginalR).getAs<KnownSVal>())
-              BR.addVisitor<FindLastStoreBRVisitor>(
-                  *KV, OriginalR, EnableNullFPSuppression, TKind, OriginSFC);
+              getParentTracker().track(
+                  *KV, OriginalR, {TKind, EnableNullFPSuppression}, OriginSFC);
           }
         }
       }
@@ -2239,7 +2239,7 @@ Tracker::Result Tracker::track(SVal V, const MemRegion *R, TrackingOptions Opts,
                                const StackFrameContext *Origin) {
   if (auto KV = V.getAs<KnownSVal>()) {
     Report.addVisitor<FindLastStoreBRVisitor>(
-        *KV, R, Opts.EnableNullFPSuppression, Opts.Kind, Origin);
+        this, *KV, R, Opts.EnableNullFPSuppression, Opts.Kind, Origin);
     return {true};
   }
   return {};
@@ -2261,11 +2261,18 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
                                        PathSensitiveBugReport &report,
                                        bugreporter::TrackingKind TKind,
                                        bool EnableNullFPSuppression) {
-  return Tracker(report)
-      .track(E, InputNode, {TKind, EnableNullFPSuppression})
+  return Tracker::create(report)
+      ->track(E, InputNode, {TKind, EnableNullFPSuppression})
       .FoundSomethingToTrack;
 }
 
+void bugreporter::trackStoredValue(KnownSVal V, const MemRegion *R,
+                                   PathSensitiveBugReport &Report,
+                                   TrackingOptions Opts,
+                                   const StackFrameContext *Origin) {
+  Tracker::create(Report)->track(V, R, Opts, Origin);
+}
+
 //===----------------------------------------------------------------------===//
 // Implementation of NulReceiverBRVisitor.
 //===----------------------------------------------------------------------===//