Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, dcoughlin, Charusso,
baloghadamsoftware, dkrupp.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, donat.nagy, mikhail.ramalho,
a.sidorin, szepet, whisperity.
Please don't take a shot for each time I write checker, it'd end bad.
The term "super checker" or "modeling checker", and the term "subchecker"
always existed conceptually, but they weren't ever formalized. In the last half
a year or so, we referred to them as
- Super/modeling checker: A checker that models some C++ code, but doesn't (or,
as of now, just //shouldn't//) emit any diagnostics, at least not under its own
name.
- Subcheckers: Checkers that are a part of super/modeling checkers,
enabling/disabling them (ideally) only toggles whether a diagnostic is emitted
from the checker it is a part of. They don't possess a checker object on their
own, and are basically glorified checker options.
While checker dependencies were in similar shoes (existed conceptually but not
formalized), this change isn't //as// critical, it just removes boilerplate
code. When you look at `IteratorChecker`, `SecuritySyntaxChecker` or
`RetainCountBase` they all use a similar, some cases faulty implementation to
keep track of which subcheckers are enabled and what their name is, so its
about time we combined them.
I envision this interface to be used to enforce our currently non-existent
specification on the checker system.
In detail:
- Introduce `SuperChecker`:
- It is essentially the same as `Checker`, but requires an enum template
argument to keep track of which subcheckers are enabled.
- While previously we defined subcheckers as checkers that don't have a
checker object on their own, `SuperChecker` does create a `CheckerBase` object
for them, but they still can't have checker callbacks.
- Add `CheckerManager::registerSubChecker` adds a new checker to a
`SuperChecker`. It is ensured runtime that the `SuperChecker` was previously
registered, and that a subchecker isn't registered multiple times.
- Add thorough test cases for the new interface.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D67336
Files:
clang/include/clang/StaticAnalyzer/Core/Checker.h
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
Index: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===================================================================
--- clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -81,6 +81,127 @@
runCheckerOnCode<addLocIncDecChecker>("void f() { int *p; (*p)++; }"));
}
+//===----------------------------------------------------------------------===//
+// Subchecker system.
+//===----------------------------------------------------------------------===//
+
+enum CXX23ModelingDiagKind { IntPointer, NonLoad };
+
+class CXX23Modeling
+ : public SuperChecker<CXX23ModelingDiagKind, check::ASTCodeBody> {
+public:
+ void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
+ BugReporter &BR) const {
+ BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
+ "Sketchy C++23 code modeled",
+ PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+
+ if (const CheckerBase *IntPointerChecker = getSubChecker<IntPointer>())
+ BR.EmitBasicReport(D, IntPointerChecker, "Custom diagnostic",
+ categories::LogicError, "Sketchy C++23 int pointer",
+ PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+
+ if (const CheckerBase *NonLoadChecker = getSubChecker<NonLoad>())
+ BR.EmitBasicReport(D, NonLoadChecker, "Custom diagnostic",
+ categories::LogicError,
+ "Sketchy C++23 pointer non-loaded",
+ PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+ }
+};
+
+void registerCXX23IntPointer(CheckerManager &Mgr) {
+ Mgr.registerSubChecker<CXX23Modeling, CXX23ModelingDiagKind::IntPointer>();
+}
+
+void registerCXX23NonLoad(CheckerManager &Mgr) {
+ Mgr.registerSubChecker<CXX23Modeling, CXX23ModelingDiagKind::NonLoad>();
+}
+
+void addButDontSpecifyCXX23Modeling(AnalysisASTConsumer &AnalysisConsumer,
+ AnalyzerOptions &AnOpts) {
+ AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+ Registry.addChecker<CXX23Modeling>("test.CXX23Modeling", "Description", "");
+ });
+}
+
+void addAndEnableCXX23Modeling(AnalysisASTConsumer &AnalysisConsumer,
+ AnalyzerOptions &AnOpts) {
+ AnOpts.CheckersAndPackages = {{"test.CXX23Modeling", true}};
+ AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+ Registry.addChecker<CXX23Modeling>("test.CXX23Modeling", "Description", "");
+ });
+}
+
+void addButDisableCXX23Modeling(AnalysisASTConsumer &AnalysisConsumer,
+ AnalyzerOptions &AnOpts) {
+ AnOpts.CheckersAndPackages = {{"test.CXX23Modeling", false}};
+ AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+ Registry.addChecker<CXX23Modeling>("test.CXX23Modeling", "Description", "");
+ });
+}
+
+void addCXX23IntPointer(AnalysisASTConsumer &AnalysisConsumer,
+ AnalyzerOptions &AnOpts) {
+ AnOpts.CheckersAndPackages.emplace_back("test.CXX23IntPointer", true);
+ AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+ Registry.addChecker(registerCXX23IntPointer, CheckerRegistry::returnTrue,
+ "test.CXX23IntPointer", "Description", "",
+ /*IsHidden*/ false);
+ Registry.addDependency("test.CXX23IntPointer", "test.CXX23Modeling");
+ });
+}
+
+void addCXX23NonLoad(AnalysisASTConsumer &AnalysisConsumer,
+ AnalyzerOptions &AnOpts) {
+ AnOpts.CheckersAndPackages.emplace_back("test.CXX23NonLoad", true);
+ AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+ Registry.addChecker(registerCXX23NonLoad, CheckerRegistry::returnTrue,
+ "test.CXX23NonLoad", "Description", "",
+ /*IsHidden*/ false);
+ Registry.addDependency("test.CXX23NonLoad", "test.CXX23Modeling");
+ });
+}
+
+TEST(RegisterCustomCheckers, SuperChecker) {
+ std::string Output;
+ EXPECT_TRUE(runCheckerOnCode<addAndEnableCXX23Modeling>(
+ "void foo(int *a) { *a; }", Output));
+ EXPECT_EQ(Output, "test.CXX23Modeling:Sketchy C++23 code modeled\n");
+
+ Output.clear();
+ bool ReturnValue =
+ runCheckerOnCode<addAndEnableCXX23Modeling, addCXX23IntPointer>(
+ "void foo(int *a) { *a; }", Output);
+ EXPECT_TRUE(ReturnValue);
+ EXPECT_EQ(Output, "test.CXX23Modeling:Sketchy C++23 code modeled\n"
+ "test.CXX23IntPointer:Sketchy C++23 int pointer\n");
+
+ Output.clear();
+ ReturnValue =
+ runCheckerOnCode<addAndEnableCXX23Modeling, addCXX23IntPointer,
+ addCXX23NonLoad>("void foo(int *a) { *a; }", Output);
+ EXPECT_TRUE(ReturnValue);
+ EXPECT_EQ(Output, "test.CXX23Modeling:Sketchy C++23 code modeled\n"
+ "test.CXX23IntPointer:Sketchy C++23 int pointer\n"
+ "test.CXX23NonLoad:Sketchy C++23 pointer non-loaded\n");
+
+ Output.clear();
+ ReturnValue =
+ runCheckerOnCode<addButDontSpecifyCXX23Modeling, addCXX23IntPointer,
+ addCXX23NonLoad>("void foo(int *a) { *a; }", Output);
+ EXPECT_TRUE(ReturnValue);
+ EXPECT_EQ(Output, "test.CXX23Modeling:Sketchy C++23 code modeled\n"
+ "test.CXX23IntPointer:Sketchy C++23 int pointer\n"
+ "test.CXX23NonLoad:Sketchy C++23 pointer non-loaded\n");
+
+ Output.clear();
+ ReturnValue =
+ runCheckerOnCode<addButDisableCXX23Modeling, addCXX23IntPointer,
+ addCXX23NonLoad>("void foo(int *a) { *a; }", Output);
+ EXPECT_TRUE(ReturnValue);
+ EXPECT_EQ(Output, "");
+}
+
} // namespace
} // namespace ento
} // namespace clang
Index: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -180,6 +180,20 @@
return static_cast<CHECKER *>(CheckerTags[tag]);
}
+ /// Used to register subcheckers. Subcheckers aren't traditional checkers in
+ /// the sense that they don't have checker callbacks, but there is checker
+ /// object associated with them, which is retrievable though the checker they
+ /// are possessed by.
+ ///
+ /// \returns a pointer to the super checker object.
+ template <typename SuperChecker,
+ typename SuperChecker::SubCheckerTy SubCheckerKind>
+ SuperChecker *registerSubChecker() {
+ SuperChecker *Super = getChecker<SuperChecker>();
+ Super->template addSubChecker<SubCheckerKind>(CurrentCheckerName);
+ return Super;
+ }
+
//===----------------------------------------------------------------------===//
// Functions for running checkers for AST traversing.
//===----------------------------------------------------------------------===//
Index: clang/include/clang/StaticAnalyzer/Core/Checker.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -494,6 +494,8 @@
friend class ::clang::ento::CheckerManager;
public:
+ CheckerBase() = default;
+ CheckerBase(CheckerNameRef Name) : Name(Name) {}
StringRef getTagDescription() const override;
CheckerNameRef getCheckerName() const;
@@ -502,6 +504,47 @@
const char *NL, const char *Sep) const { }
};
+
+template <typename SubCheckerEnumTy>
+class SuperCheckerBase : public CheckerBase {
+ static_assert(std::is_enum<SubCheckerEnumTy>::value,
+ "SuperCheckers are required to provide an enum to keep track "
+ "of their subcheckers!");
+
+ using SubCheckerPair = std::pair<SubCheckerEnumTy, const CheckerBase>;
+ using SubCheckerVector = typename llvm::SmallVector<SubCheckerPair, 4>;
+
+public:
+ using SubCheckerTy = SubCheckerEnumTy;
+
+private:
+ SubCheckerVector Subcheckers;
+
+ typename SubCheckerVector::const_iterator
+ getSubCheckerPos(SubCheckerEnumTy SubCheckerKind) const {
+ return llvm::find_if(Subcheckers, [SubCheckerKind](const auto &E) {
+ return E.first == SubCheckerKind;
+ });
+ }
+
+public:
+ template <SubCheckerEnumTy SubCheckerKind>
+ void addSubChecker(CheckerNameRef Name) {
+ assert(getSubCheckerPos(SubCheckerKind) == Subcheckers.end() &&
+ "This subchecker was already added to the superchecker!");
+ Subcheckers.emplace_back(SubCheckerKind, CheckerBase(Name));
+ }
+
+ template <SubCheckerEnumTy SubCheckerKind>
+ const CheckerBase *getSubChecker() const {
+ typename SubCheckerVector::const_iterator Pos =
+ getSubCheckerPos(SubCheckerKind);
+ if (Pos == Subcheckers.end())
+ return nullptr;
+ return &Pos->second;
+ }
+};
+
/// Dump checker name to stream.
raw_ostream& operator<<(raw_ostream &Out, const CheckerBase &Checker);
@@ -513,24 +556,35 @@
CheckerProgramPointTag(const CheckerBase *Checker, StringRef Msg);
};
-template <typename CHECK1, typename... CHECKs>
-class Checker : public CHECK1, public CHECKs..., public CheckerBase {
+namespace checker_detail {
+template <typename BaseTy, typename CHECK1, typename... CHECKs>
+class CheckerImpl : public CHECK1, public CHECKs..., public BaseTy {
public:
template <typename CHECKER>
static void _register(CHECKER *checker, CheckerManager &mgr) {
CHECK1::_register(checker, mgr);
- Checker<CHECKs...>::_register(checker, mgr);
+ CheckerImpl<BaseTy, CHECKs...>::_register(checker, mgr);
}
};
-template <typename CHECK1>
-class Checker<CHECK1> : public CHECK1, public CheckerBase {
+template <typename BaseTy, typename CHECK1>
+class CheckerImpl<BaseTy, CHECK1> : public CHECK1, public BaseTy {
public:
template <typename CHECKER>
static void _register(CHECKER *checker, CheckerManager &mgr) {
CHECK1::_register(checker, mgr);
}
};
+} // end of namespace checker_detail
+
+template <typename... CHECKs>
+using Checker =
+ typename checker_detail::CheckerImpl<CheckerBase, CHECKs...>;
+
+template <typename SubCheckerEnumTy, typename... CHECKs>
+using SuperChecker =
+ typename checker_detail::CheckerImpl<SuperCheckerBase<SubCheckerEnumTy>,
+ CHECKs...>;
template <typename EVENT>
class EventDispatcher {
@@ -575,8 +629,7 @@
DefaultBool &operator=(bool b) { val = b; return *this; }
};
-} // end ento namespace
-
-} // end clang namespace
+} // namespace ento
+} // namespace clang
#endif
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
