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 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 Ranges = {}, ArrayRef Fixits = {}); diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h index 3a635e0d0125a..73bece803cf15 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; + using CheckerNameInfo = std::variant; 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 *Checker, 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(Checker), 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(&CheckerName)) return *CNR; - auto [Checker, Idx] = std::get(CheckerName); - return Checker->getName(Idx); + return std::get(CheckerName)->getName(); } /// isSuppressOnSink - Returns true if bug reports associated with this bug @@ -82,6 +70,21 @@ 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, public BugType { +public: + CheckerFrontendWithBugType(StringRef Desc, + StringRef Cat = categories::LogicError, + bool SuppressOnSink = false) + : BugType(this, Desc, Cat, SuppressOnSink) {} +}; + } // 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..db3806b425dda 100644 --- a/clang/include/clang/StaticAnalyzer/Core/Checker.h +++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h @@ -484,83 +484,90 @@ 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, 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 Name; public: - CheckerNameRef getName(CheckerPartIdx Idx = DefaultPart) const { - assert(Idx < RegisteredNames.size() && "Checker part index is too large!"); - std::optional 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 Name.has_value(); } + 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 -class Checker : public CHECK1, public CHECKs..., public CheckerBase { +/// 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 +class Checker : public CheckerBase, public CHECKs... { public: template - static void _register(CHECKER *checker, CheckerManager &mgr) { - CHECK1::_register(checker, mgr); - Checker::_register(checker, mgr); + static void _register(CHECKER *Chk, CheckerManager &Mgr) { + (CHECKs::_register(Chk, Mgr), ...); } }; -template -class Checker : public CHECK1, public CheckerBase { +/// Checker families (where a single backend class implements multiple related +/// frontends) should derive from this template and specify all the implemented +/// callbacks (i.e. classes like `check::PreStmt` or `eval::Call`) as template +/// arguments of `FamilyChecker.` +/// +/// NOTE: Classes deriving from `CheckerFamily` must implement the pure +/// virtual method `StringRef getTagDescription()` which is inherited from +/// `ProgramPointTag` and should return the name of the class as a string. +/// +/// Obviously, this boilerplate is not a good thing, but unfortunately there is +/// no portable way to stringify the name of a type (e.g. class), so any +/// portable implementation of `getTagDescription` would need to take the +/// name of the class from *somewhere* where it's present as a string -- and +/// then directly placing it in a method override is much simpler than +/// loading it from `Checkers.td`. +/// +/// Note that the existing `CLASS` field in `Checkers.td` is not suitable for +/// our goals, because instead of storing the same class name for each +/// frontend, in fact each frontendchecker needs to have its own unique value +/// there (to ensure that the names of the register methods are all unique). +template +class CheckerFamily : public CheckerBackend, public CHECKs... { public: template - static void _register(CHECKER *checker, CheckerManager &mgr) { - CHECK1::_register(checker, mgr); + static void _register(CHECKER *Chk, CheckerManager &Mgr) { + (CHECKs::_register(Chk, Mgr), ...); } }; @@ -581,6 +588,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..58f59f7ab049f 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 { 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 *Checker, 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 - 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 + CHECKER *getChecker(AT &&...Args) { CheckerTag Tag = getTag(); - std::unique_ptr &Ref = CheckerTags[Tag]; + std::unique_ptr &Ref = CheckerTags[Tag]; if (!Ref) { std::unique_ptr Checker = std::make_unique(std::forward(Args)...); @@ -239,18 +207,18 @@ class CheckerManager { Ref = std::move(Checker); } - CHECKER *Result = static_cast(Ref.get()); - Result->registerCheckerPart(Idx, CurrentCheckerName); - return Result; + return static_cast(Ref.get()); } - template - CHECKER *getChecker() { - CheckerTag Tag = getTag(); - std::unique_ptr &Ref = CheckerTags[Tag]; - assert(Ref && "Requested checker is not registered! Maybe you should add it" - " as a dependency in Checkers.td?"); - return static_cast(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 + CHECKER *registerChecker(AT &&...Args) { + CHECKER *Chk = getChecker(std::forward(Args)...); + Chk->enable(*this); + return Chk; } template 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 static void *getTag() { static int tag; return &tag; } - llvm::DenseMap> CheckerTags; + llvm::DenseMap> CheckerTags; struct DeclCheckerInfo { CheckDeclFunc CheckFn; diff --git a/clang/lib/Analysis/ProgramPoint.cpp b/clang/lib/Analysis/ProgramPoint.cpp index e508681410b0b..d7cd38a7325f5 100644 --- a/clang/lib/Analysis/ProgramPoint.cpp +++ b/clang/lib/Analysis/ProgramPoint.cpp @@ -357,6 +357,4 @@ SimpleProgramPointTag::SimpleProgramPointTag(StringRef MsgProvider, StringRef Msg) : Desc((MsgProvider + " : " + Msg).str()) {} -StringRef SimpleProgramPointTag::getTagDescription() const { - return Desc; -} +StringRef SimpleProgramPointTag::getTagDescription() const { return Desc; } diff --git a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp index 3dd57732305b2..95a9582ecdcb1 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> { +class DivZeroChecker : public CheckerFamily> { void reportBug(StringRef Msg, ProgramStateRef StateZero, CheckerContext &C) const; void reportTaintBug(StringRef Msg, ProgramStateRef StateZero, @@ -33,17 +33,15 @@ class DivZeroChecker : public Checker> { llvm::ArrayRef 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"}; + CheckerFrontendWithBugType TaintedDivChecker{"Division by zero", + categories::TaintedData}; void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const; + + /// Identifies this checker family for debugging purposes. + StringRef getTagDescription() const override { return "DivZeroChecker"; } }; } // 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( - BugTypes[DivideZeroChecker], Msg, N); + auto R = + std::make_unique(DivideZeroChecker, 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 TaintedSyms) const { - if (!isPartEnabled(TaintedDivChecker)) + if (!TaintedDivChecker.isEnabled()) return; if (ExplodedNode *N = C.generateNonFatalErrorNode(StateZero)) { - auto R = std::make_unique( - BugTypes[TaintedDivChecker], Msg, N); + auto R = + std::make_unique(TaintedDivChecker, 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(); + Mgr.getChecker()->DivideZeroChecker.enable(Mgr); } bool ento::shouldRegisterDivZeroChecker(const CheckerManager &) { return true; } void ento::registerTaintedDivChecker(CheckerManager &Mgr) { - Mgr.registerChecker(); + Mgr.getChecker()->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..67429ee2c25f9 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp @@ -40,15 +40,13 @@ template <> struct FoldingSetTrait { namespace { class VirtualCallChecker - : public Checker { + : public CheckerFamily { 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}; + CheckerFrontendWithBugType 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,14 @@ 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(BugTypes[Part], OS.str(), N); + auto Report = std::make_unique(Part, OS.str(), N); if (ShowFixIts && !IsPure) { // FIXME: These hints are valid only when the virtual call is made @@ -210,7 +210,7 @@ void VirtualCallChecker::registerCtorDtorCallInState(bool IsBeginFunction, } void ento::registerPureVirtualCallChecker(CheckerManager &Mgr) { - Mgr.registerChecker(); + Mgr.getChecker()->PureChecker.enable(Mgr); } bool ento::shouldRegisterPureVirtualCallChecker(const CheckerManager &Mgr) { @@ -218,8 +218,8 @@ bool ento::shouldRegisterPureVirtualCallChecker(const CheckerManager &Mgr) { } void ento::registerVirtualCallChecker(CheckerManager &Mgr) { - auto *Chk = Mgr.registerChecker(); + auto *Chk = Mgr.getChecker(); + 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..10901a2db0a69 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -3411,9 +3411,9 @@ PathSensitiveBugReporter::generateDiagnosticForConsumerMap( } void BugReporter::EmitBasicReport(const Decl *DeclWithIssue, - const CheckerBase *Checker, StringRef Name, - StringRef Category, StringRef Str, - PathDiagnosticLocation Loc, + const CheckerFrontend *Checker, + StringRef Name, StringRef Category, + StringRef Str, PathDiagnosticLocation Loc, ArrayRef Ranges, ArrayRef Fixits) { EmitBasicReport(DeclWithIssue, Checker->getName(), Name, Category, Str, Loc, 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..4c37b65ae5c68 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 *Checker, StringRef OptionName, StringRef ExpectedValueDesc) const { getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input) - << (llvm::Twine(C->getName(Idx)) + ":" + OptionName).str() + << (llvm::Twine(Checker->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(Checker->getName()) : ""; - return (Name + ":" + CheckerName).str(); + StringRef CheckerTag = Checker ? Checker->getTagDescription() : ""; + 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 evaluatorChecker; + std::optional 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) << "]}"; diff --git a/clang/test/Analysis/ftime-trace.cpp b/clang/test/Analysis/ftime-trace.cpp index 2940ff2e02891..e349eab8b62ad 100644 --- a/clang/test/Analysis/ftime-trace.cpp +++ b/clang/test/Analysis/ftime-trace.cpp @@ -39,7 +39,7 @@ // Finally, each checker call back is also present: // -// CHECK: "name": "Total Stmt:core.DivideZero", +// CHECK: "name": "Total Stmt:DivZeroChecker", // CHECK-NEXT: "args": { // CHECK-NEXT: "count": {{[0-9]+}}, // CHECK-NEXT: "avg ms": {{[0-9]+}}