[clang] b6bcf95 - [analyzer] Change FindLastStoreBRVisitor to use Tracker
Valeriy Savchenko via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 11 02:51:54 PDT 2021
Author: Valeriy Savchenko
Date: 2021-06-11T12:49:03+03:00
New Revision: b6bcf953220db7880f2bb508f6f5c02b41078b2c
URL: https://github.com/llvm/llvm-project/commit/b6bcf953220db7880f2bb508f6f5c02b41078b2c
DIFF: https://github.com/llvm/llvm-project/commit/b6bcf953220db7880f2bb508f6f5c02b41078b2c.diff
LOG: [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
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
index 5c4345a32c8d3..4b0d5378e0c13 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ b/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 @@ class Tracker {
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 @@ class StoreHandler {
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
+///
diff erent 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
diff erent 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 @@ class FindLastStoreBRVisitor final : public BugReporterVisitor {
/// 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);
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
index 4408cbe069420..64ac6bc4c06b7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ b/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;
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
index e457513d8de44..816a547cadc3d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
+++ b/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));
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 2df3687f2bb45..ec16a4deae029 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/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.
//===----------------------------------------------------------------------===//
More information about the cfe-commits
mailing list