diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h --- 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 @@ -141,6 +142,9 @@ const MemRegion *Dest, *Origin; }; +class Tracker; +using TrackerRef = llvm::IntrusiveRefCntPtr; + class ExpressionHandler; class StoreHandler; @@ -149,7 +153,7 @@ /// 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 { private: using ExpressionHandlerPtr = std::unique_ptr; using StoreHandlerPtr = std::unique_ptr; @@ -158,11 +162,17 @@ std::deque ExpressionHandlers; std::deque 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 @@ -313,6 +323,18 @@ 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. /// @@ -330,13 +352,31 @@ 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; @@ -360,11 +400,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::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 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ b/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 + // 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( - AllVarBindings[0].second.castAs(), AllocBindingToReport, - false, bugreporter::TrackingKind::Thorough); + // region, thus guaranteeing that trackStoredValue will work. + bugreporter::trackStoredValue(AllVarBindings[0].second.castAs(), + AllocBindingToReport, *this); } else { AllocBindingToReport = AllocFirstBinding; } diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp @@ -86,9 +86,9 @@ auto R = std::make_unique(*BT, os.str(), N); if (const Expr *Ex = FindBlockDeclRefExpr(BE->getBody(), VD)) R->addRange(Ex->getSourceRange()); - R->addVisitor(std::make_unique( - *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 --- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/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(V.getAsRegion())) { if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) { if (auto KV = State->getSVal(OriginalR).getAs()) - BR.addVisitor( - *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(V, R, Opts.EnableNullFPSuppression, - Opts.Kind, Origin); + Report.addVisitor( + this, V, R, Opts.EnableNullFPSuppression, Opts.Kind, Origin); return {true, false}; } @@ -2262,11 +2262,18 @@ 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. //===----------------------------------------------------------------------===//