vsavchenko created this revision.
vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, 
manas, RedDocMD.
Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103618

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1488,8 +1488,8 @@
     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 @@
               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);
           }
         }
       }
@@ -2240,8 +2240,8 @@
 Tracker::Result Tracker::track(KnownSVal V, const MemRegion *R,
                                TrackingOptions Opts,
                                const StackFrameContext *Origin) {
-  Report.addVisitor<FindLastStoreBRVisitor>(V, R, Opts.EnableNullFPSuppression,
-                                            Opts.Kind, Origin);
+  Report.addVisitor<FindLastStoreBRVisitor>(
+      *this, V, R, Opts.EnableNullFPSuppression, Opts.Kind, Origin);
   return {true, false};
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
@@ -86,9 +86,9 @@
         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::Tracker(*R).track(*V, VR,
+                                       {bugreporter::TrackingKind::Thorough,
+                                        /*EnableNullFPSuppression*/ false});
         R->disablePathPruning();
         // need location of block
         C.emitReport(std::move(R));
Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -980,13 +980,12 @@
     // 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
-    //       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);
+    //       Tracker 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 Tracker will work.
+    bugreporter::Tracker(*this).track(
+        AllVarBindings[0].second.castAs<KnownSVal>(), AllocBindingToReport);
   } else {
     AllocBindingToReport = AllocFirstBinding;
   }
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -313,6 +313,18 @@
   Tracker &getParentTracker() { return ParentTracker; }
 };
 
+/// Visitor that tracks expressions and values.
+class TrackingBugReporterVisitor : public BugReporterVisitor {
+private:
+  Tracker &ParentTracker;
+
+public:
+  TrackingBugReporterVisitor(Tracker &ParentTracker)
+      : ParentTracker(ParentTracker) {}
+
+  Tracker &getParentTracker() { return ParentTracker; }
+};
+
 /// Attempts to add visitors to track expression value back to its point of
 /// origin.
 ///
@@ -336,7 +348,8 @@
 
 /// 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;
@@ -360,11 +373,13 @@
   ///        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::Tracker &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);
   }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to