https://github.com/NagyDonat created
https://github.com/llvm/llvm-project/pull/139256
The checker classes (i.e. classes derived from `CheckerBase` via the utility
template `Checker<...>`) act as intermediates between the user and the analyzer
engine, so they have two interfaces:
- On the frontend side, they have a public name, can be enabled or disabled,
can accept checker options and can be reported as the source of bug reports.
- On the backend side, they can handle various checker callbacks and they
"leave a mark" on the `ExplodedNode`s that are created by them. (These
`ProgramPointTag` marks are internal: they appear in debug logs and can be
queried by checker logic; but the user doesn't see them.)
In a significant majority of the checkers there is 1:1 correspondence between
these sides, but there are also many checker classes where several related
user-facing checkers share the same backend class. Historically each of these
"multi-part checker" classes had its own hacks to juggle its multiple names,
which led to lots of ugliness like lazy initialization of `mutable
std::unique_ptr<BugType>` members and redundant data members (when a checker
used its custom `CheckNames` array and ignored the inherited single `Name`).
My recent commit 27099982da2f5a6c2d282d6b385e79d080669546 tried to unify and
standardize these existing solutions to get rid of some of the technical debt,
but it still used enum values to identify the checker parts within a
"multi-part" checker class, which led to some ugliness.
This commit introduces a new framework which takes a more direct,
object-oriented approach: instead of identifying checker parts with `{parent
checker object, index of part}` pairs, the parts of a multi-part checker become
stand-alone objects that store their own name (and enabled/disabled status) as
a data member.
This is implemented by separating the functionality of `CheckerBase` into two
new classes: `CheckerFrontend` and `CheckerBackend`. The name `CheckerBase` is
kept (as a class derived from both `CheckerFrontend` and `CheckerBackend`), so
"simple" checkers that use `CheckerBase` and `Checker<...>` continues to work
without changes. However we also get first-class support for the "many
frontends - one backend" situation:
- The class `CheckerFamily<...>` works exactly like `Checker<...>` but inherits
from `CheckerBackend` instead of `CheckerBase`, so it won't have a superfluous
single `Name` member.
- Classes deriving from `CheckerFamily` can freely own multiple
`CheckerFrontend` data members, which are enabled within the registration
methods corresponding to their name and can be used to initialize the
`BugType`s that they can emit.
In this scheme each `CheckerFamily` needs to override the pure virtual method
`ProgramPointTag::getTagDescription()` which returns a string which represents
that class for debugging purposes. (Previously this used the name of one
arbitrary sub-checker, which was passable for debugging purposes, but not too
elegant.)
I'm planning to implement follow-up commits that convert all the "multi-part"
checkers to this `CheckerFamily` framework.
From 3bead14691a29482705c76951eaed176bfbfc996 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Thu, 8 May 2025 18:46:41 +0200
Subject: [PATCH] [analyzer][NFC] Introduce framework for checker families
The checker classes (i.e. classes derived from `CheckerBase` via the
utility template `Checker<...>`) act as intermediates between the user
and the analyzer engine, so they have two interfaces:
- On the frontend side, they have a public name, can be enabled or
disabled, can accept checker options and can be reported as the source
of bug reports.
- On the backend side, they can handle various checker callbacks and
they "leave a mark" on the `ExplodedNode`s that are created by them.
(These `ProgramPointTag` marks are internal: they appear in debug logs
and can be queried by checker logic; but the user doesn't see them.)
In a significant majority of the checkers there is 1:1 correspondence
between these sides, but there are also many checker classes where
several related user-facing checkers share the same backend class.
Historically each of these "multi-part checker" classes had its own
hacks to juggle its multiple names, which led to lots of ugliness like
lazy initialization of `mutable std::unique_ptr<BugType>` members and
redundant data members (when a checker used its custom `CheckNames`
array and ignored the inherited single `Name`).
My recent commit 27099982da2f5a6c2d282d6b385e79d080669546 tried to
unify and standardize these existing solutions to get rid of some of the
technical debt, but it still used enum values to identify the checker
parts within a "multi-part" checker class, which led to some ugliness.
This commit introduces a new framework which takes a more direct,
object-oriented approach: instead of identifying checker parts with
`{parent checker object, index of part}` pairs, the parts of a
multi-part checker become stand-alone objects that store their own name
(and enabled/disabled status) as a data member.
This is implemented by separating the functionality of `CheckerBase`
into two new classes: `CheckerFrontend` and `CheckerBackend`. The name
`CheckerBase` is kept (as a class derived from both `CheckerFrontend`
and `CheckerBackend`), so "simple" checkers that use `CheckerBase` and
`Checker<...>` continues to work without changes. However we also get
first-class support for the "many frontends - one backend" situation:
- The class `CheckerFamily<...>` works exactly like `Checker<...>` but
inherits from `CheckerBackend` instead of `CheckerBase`, so it won't
have a superfluous single `Name` member.
- Classes deriving from `CheckerFamily` can freely own multiple
`CheckerFrontend` data members, which are enabled within the
registration methods corresponding to their name and can be used to
initialize the `BugType`s that they can emit.
In this scheme each `CheckerFamily` needs to override the pure virtual
method `ProgramPointTag::getTagDescription()` which returns a string
which represents that class for debugging purposes. (Previously this
used the name of one arbitrary sub-checker, which was passable for
debugging purposes, but not too elegant.)
I'm planning to implement follow-up commits that convert all the
"multi-part" checkers to this `CheckerFamily` framework.
---
clang/include/clang/Analysis/ProgramPoint.h | 4 +-
.../Core/BugReporter/BugReporter.h | 7 +-
.../StaticAnalyzer/Core/BugReporter/BugType.h | 44 +++---
.../clang/StaticAnalyzer/Core/Checker.h | 132 ++++++++++--------
.../StaticAnalyzer/Core/CheckerManager.h | 80 ++++-------
.../Checkers/DivZeroChecker.cpp | 34 +++--
.../Checkers/ObjCSelfInitChecker.cpp | 2 +-
.../Checkers/VirtualCallChecker.cpp | 29 ++--
clang/lib/StaticAnalyzer/Core/BugReporter.cpp | 4 +-
clang/lib/StaticAnalyzer/Core/Checker.cpp | 14 +-
.../StaticAnalyzer/Core/CheckerManager.cpp | 22 +--
11 files changed, 180 insertions(+), 192 deletions(-)
diff --git a/clang/include/clang/Analysis/ProgramPoint.h
b/clang/include/clang/Analysis/ProgramPoint.h
index c40aa3d8ffb72..d81b8e845cb48 100644
--- a/clang/include/clang/Analysis/ProgramPoint.h
+++ b/clang/include/clang/Analysis/ProgramPoint.h
@@ -40,8 +40,8 @@ class ProgramPointTag {
ProgramPointTag(void *tagKind = nullptr) : TagKind(tagKind) {}
virtual ~ProgramPointTag();
- /// The description of this program point which will be displayed when the
- /// ExplodedGraph is dumped in DOT format for debugging.
+ /// The description of this program point which will be dumped for debugging
+ /// purposes. Do not use in user-facing output!
virtual StringRef getTagDescription() const = 0;
/// Used to implement 'isKind' in subclasses.
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
index 8e1d25b3eefa1..33d37febc7327 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -643,9 +643,10 @@ class BugReporter {
/// reports.
virtual void emitReport(std::unique_ptr<BugReport> R);
- void EmitBasicReport(const Decl *DeclWithIssue, const CheckerBase *Checker,
- StringRef BugName, StringRef BugCategory,
- StringRef BugStr, PathDiagnosticLocation Loc,
+ void EmitBasicReport(const Decl *DeclWithIssue,
+ const CheckerFrontend *Checker, StringRef BugName,
+ StringRef BugCategory, StringRef BugStr,
+ PathDiagnosticLocation Loc,
ArrayRef<SourceRange> Ranges = {},
ArrayRef<FixItHint> Fixits = {});
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
index 3a635e0d0125a..b05360904f86d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
@@ -27,11 +27,7 @@ class BugReporter;
class BugType {
private:
- struct CheckerPartRef {
- const CheckerBase *Checker;
- CheckerPartIdx Idx;
- };
- using CheckerNameInfo = std::variant<CheckerNameRef, CheckerPartRef>;
+ using CheckerNameInfo = std::variant<CheckerNameRef, const CheckerFrontend
*>;
const CheckerNameInfo CheckerName;
const std::string Description;
@@ -43,7 +39,7 @@ class BugType {
public:
// Straightforward constructor where the checker name is specified directly.
// TODO: As far as I know all applications of this constructor involve ugly
- // hacks that could be avoided by switching to a different constructor.
+ // hacks that could be avoided by switching to the other constructor.
// When those are all eliminated, this constructor should be removed to
// eliminate the `variant` and simplify this class.
BugType(CheckerNameRef CheckerName, StringRef Desc,
@@ -52,18 +48,11 @@ class BugType {
SuppressOnSink(SuppressOnSink) {}
// Constructor that can be called from the constructor of a checker object.
// At that point the checker name is not yet available, but we can save a
- // pointer to the checker and later use that to query the name.
- BugType(const CheckerBase *Checker, StringRef Desc,
+ // pointer to the checker and use that to query the name.
+ BugType(const CheckerFrontend *CF, StringRef Desc,
StringRef Cat = categories::LogicError, bool SuppressOnSink = false)
- : CheckerName(CheckerPartRef{Checker, DefaultPart}), Description(Desc),
- Category(Cat), SuppressOnSink(SuppressOnSink) {}
- // Constructor that can be called from the constructor of a checker object
- // when it has multiple parts with separate names. We save the name and the
- // part index to be able to query the name of that part later.
- BugType(const CheckerBase *Checker, CheckerPartIdx Idx, StringRef Desc,
- StringRef Cat = categories::LogicError, bool SuppressOnSink = false)
- : CheckerName(CheckerPartRef{Checker, Idx}), Description(Desc),
- Category(Cat), SuppressOnSink(SuppressOnSink) {}
+ : CheckerName(CF), Description(Desc), Category(Cat),
+ SuppressOnSink(SuppressOnSink) {}
virtual ~BugType() = default;
StringRef getDescription() const { return Description; }
@@ -72,8 +61,7 @@ class BugType {
if (const auto *CNR = std::get_if<CheckerNameRef>(&CheckerName))
return *CNR;
- auto [Checker, Idx] = std::get<CheckerPartRef>(CheckerName);
- return Checker->getName(Idx);
+ return std::get<const CheckerFrontend *>(CheckerName)->getName();
}
/// isSuppressOnSink - Returns true if bug reports associated with this bug
@@ -82,6 +70,24 @@ class BugType {
bool isSuppressOnSink() const { return SuppressOnSink; }
};
+/// Trivial convenience class for the common case when a certain checker
+/// frontend always uses the same bug type. This way instead of writing
+/// ```
+/// CheckerFrontend LongCheckerFrontendName;
+/// BugType LongCheckerFrontendNameBug{LongCheckerFrontendName, "..."};
+/// ```
+/// we can use `CheckerFrontendWithBugType LongCheckerFrontendName{"..."}`.
+class CheckerFrontendWithBugType : public CheckerFrontend {
+ BugType BT;
+
+public:
+ CheckerFrontendWithBugType(StringRef Desc,
+ StringRef Cat = categories::LogicError,
+ bool SuppressOnSink = false)
+ : BT(this, Desc, Cat, SuppressOnSink) {}
+ const BugType &getBT() const { return BT; }
+};
+
} // namespace ento
} // end clang namespace
diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h
b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index a54c5bee612f6..45b0398f3aca5 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -484,83 +484,87 @@ class Call {
} // end eval namespace
-class CheckerBase : public ProgramPointTag {
- /// A single checker class (i.e. a subclass of `CheckerBase`) can implement
- /// multiple user-facing checkers that have separate names and can be enabled
- /// separately, but are backed by the same singleton checker object.
- SmallVector<std::optional<CheckerNameRef>, 1> RegisteredNames;
-
- friend class ::clang::ento::CheckerManager;
+/// A `CheckerFrontend` instance is what the user recognizes as "one checker":
+/// it has a public canonical name (injected from the `CheckerManager`), can be
+/// enabled or disabled, can have associated checker options and can be printed
+/// as the "source" of bug reports.
+/// The singleton instance of a simple `Checker<...>` is-a `CheckerFrontend`
+/// (for historical reasons, to preserve old straightforward code), while the
+/// singleton instance of a `CheckerFamily<...>` class owns multiple
+/// `CheckerFrontend` instances as data members.
+/// Modeling checkers that are hidden from the user but can be enabled or
+/// disabled separately (as dependencies of other checkers) are also considered
+/// to be `CheckerFrontend`s.
+class CheckerFrontend {
+ /// The `Name` is nullopt if and only if the checker is disabled.
+ std::optional<CheckerNameRef> Name;
public:
- CheckerNameRef getName(CheckerPartIdx Idx = DefaultPart) const {
- assert(Idx < RegisteredNames.size() && "Checker part index is too large!");
- std::optional<CheckerNameRef> Name = RegisteredNames[Idx];
- assert(Name && "Requested checker part is not registered!");
- return *Name;
- }
-
- bool isPartEnabled(CheckerPartIdx Idx) const {
- return Idx < RegisteredNames.size() && RegisteredNames[Idx].has_value();
- }
-
- void registerCheckerPart(CheckerPartIdx Idx, CheckerNameRef Name) {
- // Paranoia: notice if e.g. UINT_MAX is passed as a checker part index.
- assert(Idx < 256 && "Checker part identifiers should be small integers.");
-
- if (Idx >= RegisteredNames.size())
- RegisteredNames.resize(Idx + 1, std::nullopt);
-
- assert(!RegisteredNames[Idx] && "Repeated registration of checker a
part!");
- RegisteredNames[Idx] = Name;
- }
-
- StringRef getTagDescription() const override {
- // When the ExplodedGraph is dumped for debugging (in DOT format), this
- // method is called to attach a description to nodes created by this
- // checker _class_. Ideally this should be recognizable identifier of the
- // whole class, but for this debugging purpose it's sufficient to use the
- // name of the first registered checker part.
- for (const auto &OptName : RegisteredNames)
- if (OptName)
- return *OptName;
-
- return "Unregistered checker";
+ void enable(CheckerManager &Mgr) {
+ assert(!Name && "Checker part registered twice!");
+ Name = Mgr.getCurrentCheckerName();
}
+ bool isEnabled() const { return static_cast<bool>(Name); }
+ CheckerNameRef getName() const { return *Name; }
+};
+/// `CheckerBackend` is an abstract base class that serves as the common
+/// ancestor of all the `Checker<...>` and `CheckerFamily<...>` classes which
+/// can create `ExplodedNode`s (by acting as a `ProgramPointTag`) and can be
+/// registered to handle various checker callbacks. (Moreover the debug
+/// callback `printState` is also introduced here.)
+class CheckerBackend : public ProgramPointTag {
+public:
/// Debug state dump callback, see CheckerManager::runCheckersForPrintState.
/// Default implementation does nothing.
virtual void printState(raw_ostream &Out, ProgramStateRef State,
const char *NL, const char *Sep) const;
};
-/// Dump checker name to stream.
-raw_ostream& operator<<(raw_ostream &Out, const CheckerBase &Checker);
-
-/// Tag that can use a checker name as a message provider
-/// (see SimpleProgramPointTag).
-class CheckerProgramPointTag : public SimpleProgramPointTag {
+/// The non-templated common ancestor of all the simple `Checker<...>` classes.
+class CheckerBase : public CheckerFrontend, public CheckerBackend {
public:
- CheckerProgramPointTag(StringRef CheckerName, StringRef Msg);
- CheckerProgramPointTag(const CheckerBase *Checker, StringRef Msg);
+ /// Attached to nodes created by this checker class when the ExplodedGraph is
+ /// dumped for debugging.
+ StringRef getTagDescription() const override;
};
-template <typename CHECK1, typename... CHECKs>
-class Checker : public CHECK1, public CHECKs..., public CheckerBase {
+// Template magic to implement the static method `_register()` which registers
+// the `Checker` or `CheckerFamily` for all the implemented callbacks.
+template <typename CHECKER, typename CHECK1, typename... CHECKs>
+static void registerImpl(CHECKER *Chk, CheckerManager &Mgr) {
+ CHECK1::_register(Chk, Mgr);
+ registerImpl<CHECKER, CHECKs...>(Chk, Mgr);
+}
+
+template <typename CHECKER>
+static void registerImpl(CHECKER *Chk, CheckerManager &Mgr) {}
+
+/// Simple checker classes that implement one frontend (i.e. checker name)
+/// should derive from this template and specify all the implemented callbacks
+/// (i.e. classes like `check::PreStmt` or `eval::Call`) as template arguments
+/// of `Checker`.
+template <typename... CHECKs>
+class Checker : public CheckerBase, public CHECKs... {
public:
template <typename CHECKER>
- static void _register(CHECKER *checker, CheckerManager &mgr) {
- CHECK1::_register(checker, mgr);
- Checker<CHECKs...>::_register(checker, mgr);
+ static void _register(CHECKER *Chk, CheckerManager &Mgr) {
+ registerImpl<CHECKER, CHECKs...>(Chk, Mgr);
}
};
-template <typename CHECK1>
-class Checker<CHECK1> : public CHECK1, public CheckerBase {
+/// Checker families (where a single backend class implements multiple related
+/// frontends) should derive from this template, specify all the implemented
+/// callbacks (i.e. classes like `check::PreStmt` or `eval::Call`) as template
+/// arguments of `FamilyChecker` and implement the pure virtual method
+/// `StringRef getTagDescription()` which is inherited from `ProgramPointTag`
+/// and should return a string identifying the class for debugging purposes.
+template <typename... CHECKs>
+class CheckerFamily : public CheckerBackend, public CHECKs... {
public:
template <typename CHECKER>
- static void _register(CHECKER *checker, CheckerManager &mgr) {
- CHECK1::_register(checker, mgr);
+ static void _register(CHECKER *Chk, CheckerManager &Mgr) {
+ registerImpl<CHECKER, CHECKs...>(Chk, Mgr);
}
};
@@ -581,6 +585,20 @@ class EventDispatcher {
}
};
+/// Tag that can use a checker name as a message provider
+/// (see SimpleProgramPointTag).
+/// FIXME: This is a cargo cult class which is copied into several checkers but
+/// does not provide anything useful.
+/// The only added functionality provided by this class (compared to
+/// SimpleProgramPointTag) is that it composes the tag description string from
+/// two arguments -- but tag descriptions only appear in debug output so there
+/// is no reason to bother with this.
+class CheckerProgramPointTag : public SimpleProgramPointTag {
+public:
+ CheckerProgramPointTag(StringRef CheckerName, StringRef Msg);
+ CheckerProgramPointTag(const CheckerBase *Checker, StringRef Msg);
+};
+
/// We dereferenced a location that may be null.
struct ImplicitNullDerefEvent {
SVal Location;
diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 03ffadd346d0b..8fa122f004bfe 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -39,7 +39,8 @@ class AnalysisManager;
class CXXAllocatorCall;
class BugReporter;
class CallEvent;
-class CheckerBase;
+class CheckerFrontend;
+class CheckerBackend;
class CheckerContext;
class CheckerRegistry;
struct CheckerRegistryData;
@@ -64,9 +65,9 @@ class CheckerFn<RET(Ps...)> {
Func Fn;
public:
- CheckerBase *Checker;
+ CheckerBackend *Checker;
- CheckerFn(CheckerBase *checker, Func fn) : Fn(fn), Checker(checker) {}
+ CheckerFn(CheckerBackend *checker, Func fn) : Fn(fn), Checker(checker) {}
RET operator()(Ps... ps) const {
return Fn(Checker, ps...);
@@ -116,19 +117,6 @@ class CheckerNameRef {
operator StringRef() const { return Name; }
};
-/// A single checker class (and its singleton instance) can act as the
-/// implementation of several (user-facing or modeling) checker parts that
-/// have shared state and logic, but have their own names and can be enabled or
-/// disabled separately.
-/// Each checker class that implement multiple parts introduces its own enum
-/// type to assign small numerical indices (0, 1, 2 ...) to their parts. The
-/// type alias 'CheckerPartIdx' is conceptually the union of these enum types.
-using CheckerPartIdx = unsigned;
-
-/// If a checker doesn't have multiple parts, then its single part is
-/// represented by this index.
-constexpr inline CheckerPartIdx DefaultPart = 0;
-
enum class ObjCMessageVisitKind {
Pre,
Post,
@@ -193,14 +181,7 @@ class CheckerManager {
/// Emits an error through a DiagnosticsEngine about an invalid user supplied
/// checker option value.
- void reportInvalidCheckerOptionValue(const CheckerBase *C,
- StringRef OptionName,
- StringRef ExpectedValueDesc) const {
- reportInvalidCheckerOptionValue(C, DefaultPart, OptionName,
- ExpectedValueDesc);
- }
-
- void reportInvalidCheckerOptionValue(const CheckerBase *C, CheckerPartIdx
Idx,
+ void reportInvalidCheckerOptionValue(const CheckerFrontend *CP,
StringRef OptionName,
StringRef ExpectedValueDesc) const;
@@ -210,28 +191,15 @@ class CheckerManager {
// Checker registration.
//===--------------------------------------------------------------------===//
- /// Construct the singleton instance of a checker, register it for the
- /// supported callbacks and record its name with `registerCheckerPart()`.
- /// Arguments passed to this function are forwarded to the constructor of the
- /// checker.
- ///
- /// If `CHECKER` has multiple parts, then the constructor call and the
- /// callback registration only happen within the first `registerChecker()`
- /// call; while the subsequent calls only enable additional parts of the
- /// existing checker object (while registering their names).
- ///
- /// \returns a pointer to the checker object.
- template <typename CHECKER, CheckerPartIdx Idx = DefaultPart, typename... AT>
- CHECKER *registerChecker(AT &&...Args) {
- // This assert could be removed but then we need to make sure that calls
- // registering different parts of the same checker pass the same arguments.
- static_assert(
- Idx == DefaultPart || !sizeof...(AT),
- "Argument forwarding isn't supported with multi-part checkers!");
-
+ /// If the the singleton instance of a checker class is not yet constructed,
+ /// then construct it (with the supplied arguments), register it for the
+ /// callbacks that are supported by it, and return it. Otherwise, just return
+ /// a pointer to the existing instance.
+ template <typename CHECKER, typename... AT>
+ CHECKER *getChecker(AT &&...Args) {
CheckerTag Tag = getTag<CHECKER>();
- std::unique_ptr<CheckerBase> &Ref = CheckerTags[Tag];
+ std::unique_ptr<CheckerBackend> &Ref = CheckerTags[Tag];
if (!Ref) {
std::unique_ptr<CHECKER> Checker =
std::make_unique<CHECKER>(std::forward<AT>(Args)...);
@@ -239,18 +207,18 @@ class CheckerManager {
Ref = std::move(Checker);
}
- CHECKER *Result = static_cast<CHECKER *>(Ref.get());
- Result->registerCheckerPart(Idx, CurrentCheckerName);
- return Result;
+ return static_cast<CHECKER *>(Ref.get());
}
- template <typename CHECKER>
- CHECKER *getChecker() {
- CheckerTag Tag = getTag<CHECKER>();
- std::unique_ptr<CheckerBase> &Ref = CheckerTags[Tag];
- assert(Ref && "Requested checker is not registered! Maybe you should add
it"
- " as a dependency in Checkers.td?");
- return static_cast<CHECKER *>(Ref.get());
+ /// Register a single-part checker (derived from `Checker`): construct its
+ /// singleton instance, register it for the supported callbacks and record
+ /// its name (with `CheckerFrontend::enable`). Calling this multiple times
+ /// triggers an assertion failure.
+ template <typename CHECKER, typename... AT>
+ CHECKER *registerChecker(AT &&...Args) {
+ CHECKER *Chk = getChecker<CHECKER>(std::forward<AT>(Args)...);
+ Chk->enable(*this);
+ return Chk;
}
template <typename CHECKER> bool isRegisteredChecker() {
@@ -482,7 +450,7 @@ class CheckerManager {
/// Run checkers for debug-printing a ProgramState.
///
/// Unlike most other callbacks, any checker can simply implement the virtual
- /// method CheckerBase::printState if it has custom data to print.
+ /// method CheckerBackend::printState if it has custom data to print.
///
/// \param Out The output stream
/// \param State The state being printed
@@ -651,7 +619,7 @@ class CheckerManager {
template <typename T>
static void *getTag() { static int tag; return &tag; }
- llvm::DenseMap<CheckerTag, std::unique_ptr<CheckerBase>> CheckerTags;
+ llvm::DenseMap<CheckerTag, std::unique_ptr<CheckerBackend>> CheckerTags;
struct DeclCheckerInfo {
CheckDeclFunc CheckFn;
diff --git a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
index 3dd57732305b2..7672c63a646e4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
@@ -25,7 +25,7 @@ using namespace ento;
using namespace taint;
namespace {
-class DivZeroChecker : public Checker<check::PreStmt<BinaryOperator>> {
+class DivZeroChecker : public CheckerFamily<check::PreStmt<BinaryOperator>> {
void reportBug(StringRef Msg, ProgramStateRef StateZero,
CheckerContext &C) const;
void reportTaintBug(StringRef Msg, ProgramStateRef StateZero,
@@ -33,17 +33,15 @@ class DivZeroChecker : public
Checker<check::PreStmt<BinaryOperator>> {
llvm::ArrayRef<SymbolRef> TaintedSyms) const;
public:
- /// This checker class implements several user facing checkers
- enum : CheckerPartIdx {
- DivideZeroChecker,
- TaintedDivChecker,
- NumCheckerParts
- };
- BugType BugTypes[NumCheckerParts] = {
- {this, DivideZeroChecker, "Division by zero"},
- {this, TaintedDivChecker, "Division by zero", categories::TaintedData}};
+ /// This checker family implements two user-facing checker parts.
+ CheckerFrontendWithBugType DivideZeroChecker{"Division by zero"},
+ TaintedDivChecker{"Division by zero", categories::TaintedData};
void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const;
+
+ /// Identifies this checker family for debugging purposes. For backwards
+ /// compatibility, this is the name of the older sub-checker.
+ StringRef getTagDescription() const override { return "core.DivideZero"; }
};
} // end anonymous namespace
@@ -56,11 +54,11 @@ static const Expr *getDenomExpr(const ExplodedNode *N) {
void DivZeroChecker::reportBug(StringRef Msg, ProgramStateRef StateZero,
CheckerContext &C) const {
- if (!isPartEnabled(DivideZeroChecker))
+ if (!DivideZeroChecker.isEnabled())
return;
if (ExplodedNode *N = C.generateErrorNode(StateZero)) {
- auto R = std::make_unique<PathSensitiveBugReport>(
- BugTypes[DivideZeroChecker], Msg, N);
+ auto R =
std::make_unique<PathSensitiveBugReport>(DivideZeroChecker.getBT(),
+ Msg, N);
bugreporter::trackExpressionValue(N, getDenomExpr(N), *R);
C.emitReport(std::move(R));
}
@@ -69,11 +67,11 @@ void DivZeroChecker::reportBug(StringRef Msg,
ProgramStateRef StateZero,
void DivZeroChecker::reportTaintBug(
StringRef Msg, ProgramStateRef StateZero, CheckerContext &C,
llvm::ArrayRef<SymbolRef> TaintedSyms) const {
- if (!isPartEnabled(TaintedDivChecker))
+ if (!TaintedDivChecker.isEnabled())
return;
if (ExplodedNode *N = C.generateNonFatalErrorNode(StateZero)) {
- auto R = std::make_unique<PathSensitiveBugReport>(
- BugTypes[TaintedDivChecker], Msg, N);
+ auto R =
std::make_unique<PathSensitiveBugReport>(TaintedDivChecker.getBT(),
+ Msg, N);
bugreporter::trackExpressionValue(N, getDenomExpr(N), *R);
for (auto Sym : TaintedSyms)
R->markInteresting(Sym);
@@ -127,13 +125,13 @@ void DivZeroChecker::checkPreStmt(const BinaryOperator *B,
}
void ento::registerDivZeroChecker(CheckerManager &Mgr) {
- Mgr.registerChecker<DivZeroChecker, DivZeroChecker::DivideZeroChecker>();
+ Mgr.getChecker<DivZeroChecker>()->DivideZeroChecker.enable(Mgr);
}
bool ento::shouldRegisterDivZeroChecker(const CheckerManager &) { return true;
}
void ento::registerTaintedDivChecker(CheckerManager &Mgr) {
- Mgr.registerChecker<DivZeroChecker, DivZeroChecker::TaintedDivChecker>();
+ Mgr.getChecker<DivZeroChecker>()->TaintedDivChecker.enable(Mgr);
}
bool ento::shouldRegisterTaintedDivChecker(const CheckerManager &) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
index 217c46451f80f..ace3426387568 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
@@ -342,7 +342,7 @@ void ObjCSelfInitChecker::printState(raw_ostream &Out,
ProgramStateRef State,
if (FlagMap.isEmpty() && !DidCallInit && !PreCallFlags)
return;
- Out << Sep << NL << *this << " :" << NL;
+ Out << Sep << NL << "ObjCSelfInitChecker:" << NL;
if (DidCallInit)
Out << " An init method has been called." << NL;
diff --git a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
index 5b0d303ee5bbc..b73c25b95245a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -40,15 +40,13 @@ template <> struct FoldingSetTrait<ObjectState> {
namespace {
class VirtualCallChecker
- : public Checker<check::BeginFunction, check::EndFunction, check::PreCall>
{
+ : public CheckerFamily<check::BeginFunction, check::EndFunction,
+ check::PreCall> {
public:
- enum : CheckerPartIdx { PureChecker, ImpureChecker, NumCheckerParts };
-
- BugType BugTypes[NumCheckerParts] = {
- {this, PureChecker, "Pure virtual method call",
- categories::CXXObjectLifecycle},
- {this, ImpureChecker, "Unexpected loss of virtual dispatch",
- categories::CXXObjectLifecycle}};
+ CheckerFrontendWithBugType PureChecker{"Pure virtual method call",
+ categories::CXXObjectLifecycle},
+ ImpureChecker{"Unexpected loss of virtual dispatch",
+ categories::CXXObjectLifecycle};
bool ShowFixIts = false;
@@ -56,6 +54,9 @@ class VirtualCallChecker
void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+ /// Identifies this checker family for debugging purposes.
+ StringRef getTagDescription() const override { return "VirtualCallChecker"; }
+
private:
void registerCtorDtorCallInState(bool IsBeginFunction,
CheckerContext &C) const;
@@ -147,15 +148,15 @@ void VirtualCallChecker::checkPreCall(const CallEvent
&Call,
if (!N)
return;
- const CheckerPartIdx Part = IsPure ? PureChecker : ImpureChecker;
+ const CheckerFrontendWithBugType &Part = IsPure ? PureChecker :
ImpureChecker;
- if (!isPartEnabled(Part)) {
+ if (!Part.isEnabled()) {
// The respective check is disabled.
return;
}
auto Report =
- std::make_unique<PathSensitiveBugReport>(BugTypes[Part], OS.str(), N);
+ std::make_unique<PathSensitiveBugReport>(Part.getBT(), OS.str(), N);
if (ShowFixIts && !IsPure) {
// FIXME: These hints are valid only when the virtual call is made
@@ -210,7 +211,7 @@ void VirtualCallChecker::registerCtorDtorCallInState(bool
IsBeginFunction,
}
void ento::registerPureVirtualCallChecker(CheckerManager &Mgr) {
- Mgr.registerChecker<VirtualCallChecker, VirtualCallChecker::PureChecker>();
+ Mgr.getChecker<VirtualCallChecker>()->PureChecker.enable(Mgr);
}
bool ento::shouldRegisterPureVirtualCallChecker(const CheckerManager &Mgr) {
@@ -218,8 +219,8 @@ bool ento::shouldRegisterPureVirtualCallChecker(const
CheckerManager &Mgr) {
}
void ento::registerVirtualCallChecker(CheckerManager &Mgr) {
- auto *Chk = Mgr.registerChecker<VirtualCallChecker,
- VirtualCallChecker::ImpureChecker>();
+ auto *Chk = Mgr.getChecker<VirtualCallChecker>();
+ Chk->ImpureChecker.enable(Mgr);
Chk->ShowFixIts = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
Mgr.getCurrentCheckerName(), "ShowFixIts");
}
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index 28b96f2717210..39d9ad43fde29 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -3411,12 +3411,12 @@
PathSensitiveBugReporter::generateDiagnosticForConsumerMap(
}
void BugReporter::EmitBasicReport(const Decl *DeclWithIssue,
- const CheckerBase *Checker, StringRef Name,
+ const CheckerFrontend *CP, StringRef Name,
StringRef Category, StringRef Str,
PathDiagnosticLocation Loc,
ArrayRef<SourceRange> Ranges,
ArrayRef<FixItHint> Fixits) {
- EmitBasicReport(DeclWithIssue, Checker->getName(), Name, Category, Str, Loc,
+ EmitBasicReport(DeclWithIssue, CP->getName(), Name, Category, Str, Loc,
Ranges, Fixits);
}
diff --git a/clang/lib/StaticAnalyzer/Core/Checker.cpp
b/clang/lib/StaticAnalyzer/Core/Checker.cpp
index 2bbb7a541457b..f5a07f5d305c5 100644
--- a/clang/lib/StaticAnalyzer/Core/Checker.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Checker.cpp
@@ -18,8 +18,10 @@ using namespace ento;
int ImplicitNullDerefEvent::Tag;
-void CheckerBase::printState(raw_ostream &Out, ProgramStateRef State,
- const char *NL, const char *Sep) const {}
+StringRef CheckerBase::getTagDescription() const { return getName(); }
+
+void CheckerBackend::printState(raw_ostream &Out, ProgramStateRef State,
+ const char *NL, const char *Sep) const {}
CheckerProgramPointTag::CheckerProgramPointTag(StringRef CheckerName,
StringRef Msg)
@@ -27,10 +29,4 @@ CheckerProgramPointTag::CheckerProgramPointTag(StringRef
CheckerName,
CheckerProgramPointTag::CheckerProgramPointTag(const CheckerBase *Checker,
StringRef Msg)
- : SimpleProgramPointTag(Checker->getName(), Msg) {}
-
-raw_ostream& clang::ento::operator<<(raw_ostream &Out,
- const CheckerBase &Checker) {
- Out << Checker.getName();
- return Out;
-}
+ : SimpleProgramPointTag(Checker->getTagDescription(), Msg) {}
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
index 7ae86f133904b..705eedc106b4a 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -50,11 +50,11 @@ bool CheckerManager::hasPathSensitiveCheckers() const {
}
void CheckerManager::reportInvalidCheckerOptionValue(
- const CheckerBase *C, CheckerPartIdx Idx, StringRef OptionName,
+ const CheckerFrontend *CP, StringRef OptionName,
StringRef ExpectedValueDesc) const {
getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input)
- << (llvm::Twine(C->getName(Idx)) + ":" + OptionName).str()
+ << (llvm::Twine(CP->getName()) + ":" + OptionName).str()
<< ExpectedValueDesc;
}
@@ -135,12 +135,11 @@ static void expandGraphWithCheckers(CHECK_CTX checkCtx,
namespace {
-std::string checkerScopeName(StringRef Name, const CheckerBase *Checker) {
+std::string checkerScopeName(StringRef Name, const CheckerBackend *Checker) {
if (!llvm::timeTraceProfilerEnabled())
return "";
- StringRef CheckerName =
- Checker ? static_cast<StringRef>(Checker->getName()) : "<unknown>";
- return (Name + ":" + CheckerName).str();
+ StringRef CheckerTag = Checker ? Checker->getTagDescription() : "<unknown>";
+ return (Name + ":" + CheckerTag).str();
}
struct CheckStmtContext {
@@ -690,7 +689,7 @@ void CheckerManager::runCheckersForEvalCall(ExplodedNodeSet
&Dst,
ExprEngine &Eng,
const EvalCallOptions &CallOpts) {
for (auto *const Pred : Src) {
- std::optional<CheckerNameRef> evaluatorChecker;
+ std::optional<StringRef> evaluatorChecker;
ExplodedNodeSet checkDst;
NodeBuilder B(Pred, checkDst, Eng.getBuilderContext());
@@ -722,12 +721,12 @@ void
CheckerManager::runCheckersForEvalCall(ExplodedNodeSet &Dst,
"while the {2} checker also tried to evaluate the same call. At "
"most one checker supposed to evaluate a call.",
toString(Call), evaluatorChecker,
- EvalCallChecker.Checker->getName());
+ EvalCallChecker.Checker->getTagDescription());
llvm_unreachable(AssertionMessage.c_str());
}
#endif
if (evaluated) {
- evaluatorChecker = EvalCallChecker.Checker->getName();
+ evaluatorChecker = EvalCallChecker.Checker->getTagDescription();
Dst.insert(checkDst);
#ifdef NDEBUG
break; // on release don't check that no other checker also evals.
@@ -798,8 +797,9 @@ void
CheckerManager::runCheckersForPrintStateJson(raw_ostream &Out,
if (TempBuf.empty())
continue;
- Indent(Out, Space, IsDot) << "{ \"checker\": \"" << CT.second->getName()
- << "\", \"messages\": [" << NL;
+ Indent(Out, Space, IsDot)
+ << "{ \"checker\": \"" << CT.second->getTagDescription()
+ << "\", \"messages\": [" << NL;
Indent(Out, InnerSpace, IsDot)
<< '\"' << TempBuf.str().trim() << '\"' << NL;
Indent(Out, Space, IsDot) << "]}";
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
