https://github.com/steakhal updated
https://github.com/llvm/llvm-project/pull/127409
>From 5eb1d222478c6966780f22aa664321807da87114 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbal...@gmail.com>
Date: Sun, 16 Feb 2025 20:37:34 +0100
Subject: [PATCH 1/2] [analyzer] Delay the checker constructions after parsing
If we were to delay checker constructions after we have a filled
ASTContext, then we could get rid of a bunch of "lazy initializers" in
checkers.
Turns out in the register functions of the checkers we could transfer
the ASTContext and all other things to checkers, so those could benefit
from in-class initializers and const fields.
For example, if a checker would take the ASTContext as the first field,
then the rest of the fields could use it in their in-class initializers,
so the ctor of the checker would only need to set a single field!
This would open uup countless opportunities for cleaning up the
asthetics of our checkers.
I attached a single use-case for the AST and the PP as demonstrating
purposes. You can imagine the rest.
---
.../Core/PathSensitive/CheckerHelpers.h | 17 ++++++
.../Checkers/UnixAPIChecker.cpp | 61 ++++++++++---------
.../Frontend/AnalysisConsumer.cpp | 18 +++---
3 files changed, 57 insertions(+), 39 deletions(-)
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index b4afaaeec9a4b..f7635aa0342d1 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -73,6 +73,23 @@ Nullability getNullabilityAnnotation(QualType Type);
/// returned.
std::optional<int> tryExpandAsInteger(StringRef Macro, const Preprocessor &PP);
+class CachedMacroValue {
+public:
+ CachedMacroValue(StringRef MacroSpelling, const Preprocessor &PP)
+ : MacroValueOrFailure(tryExpandAsInteger(MacroSpelling, PP)) {}
+ explicit CachedMacroValue(int ConcreteValue)
+ : MacroValueOrFailure(ConcreteValue) {}
+
+ int valueOr(int Default) const {
+ return MacroValueOrFailure.value_or(Default);
+ }
+ bool hasValue() const { return MacroValueOrFailure.has_value(); }
+ int value() const { return MacroValueOrFailure.value(); }
+
+private:
+ std::optional<int> MacroValueOrFailure;
+};
+
class OperatorKind {
union {
BinaryOperatorKind Bin;
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index da2d16ca9b5dd..0452e541e8d5c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -40,17 +40,28 @@ enum class OpenVariant {
OpenAt
};
+static CachedMacroValue getCreateFlagValue(const ASTContext &Ctx,
+ const Preprocessor &PP) {
+ CachedMacroValue MacroVal("O_CREAT", PP);
+ if (MacroVal.hasValue())
+ return MacroVal;
+
+ // If we failed, fall-back to known values.
+ if (Ctx.getTargetInfo().getTriple().getVendor() == llvm::Triple::Apple)
+ return CachedMacroValue{0x0200};
+ return MacroVal;
+}
+
namespace {
-class UnixAPIMisuseChecker
- : public Checker<check::PreCall, check::ASTDecl<TranslationUnitDecl>> {
+class UnixAPIMisuseChecker : public Checker<check::PreCall> {
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
const BugType BT_getline{this, "Improper use of getdelim",
categories::UnixAPI};
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
const BugType BT_ArgumentNull{this, "NULL pointer", categories::UnixAPI};
- mutable std::optional<uint64_t> Val_O_CREAT;
+ const CachedMacroValue Val_O_CREAT;
ProgramStateRef
EnsurePtrNotNull(SVal PtrVal, const Expr *PtrExpr, CheckerContext &C,
@@ -63,6 +74,9 @@ class UnixAPIMisuseChecker
const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const;
public:
+ UnixAPIMisuseChecker(const ASTContext &Ctx, const Preprocessor &PP)
+ : Val_O_CREAT(getCreateFlagValue(Ctx, PP)) {}
+
void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
BugReporter &BR) const;
@@ -134,20 +148,6 @@ ProgramStateRef UnixAPIMisuseChecker::EnsurePtrNotNull(
return PtrNotNull;
}
-void UnixAPIMisuseChecker::checkASTDecl(const TranslationUnitDecl *TU,
- AnalysisManager &Mgr,
- BugReporter &) const {
- // The definition of O_CREAT is platform specific.
- // Try to get the macro value from the preprocessor.
- Val_O_CREAT = tryExpandAsInteger("O_CREAT", Mgr.getPreprocessor());
- // If we failed, fall-back to known values.
- if (!Val_O_CREAT) {
- if (TU->getASTContext().getTargetInfo().getTriple().getVendor() ==
- llvm::Triple::Apple)
- Val_O_CREAT = 0x0200;
- }
-}
-
//===----------------------------------------------------------------------===//
// "open" (man 2 open)
//===----------------------------------------------------------------------===/
@@ -262,7 +262,7 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext
&C,
return;
}
- if (!Val_O_CREAT) {
+ if (!Val_O_CREAT.hasValue()) {
return;
}
@@ -276,7 +276,7 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext
&C,
}
NonLoc oflags = V.castAs<NonLoc>();
NonLoc ocreateFlag = C.getSValBuilder()
- .makeIntVal(*Val_O_CREAT, oflagsEx->getType())
+ .makeIntVal(Val_O_CREAT.value(),
oflagsEx->getType())
.castAs<NonLoc>();
SVal maskedFlagsUC = C.getSValBuilder().evalBinOpNN(state, BO_And,
oflags, ocreateFlag,
@@ -621,14 +621,17 @@ void UnixAPIPortabilityChecker::checkPreStmt(const
CallExpr *CE,
// Registration.
//===----------------------------------------------------------------------===//
-#define REGISTER_CHECKER(CHECKERNAME)
\
- void ento::register##CHECKERNAME(CheckerManager &mgr) {
\
- mgr.registerChecker<CHECKERNAME>();
\
- }
\
-
\
- bool ento::shouldRegister##CHECKERNAME(const CheckerManager &mgr) {
\
- return true;
\
- }
+void ento::registerUnixAPIMisuseChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<UnixAPIMisuseChecker>(Mgr.getASTContext(),
+ Mgr.getPreprocessor());
+}
+bool ento::shouldRegisterUnixAPIMisuseChecker(const CheckerManager &Mgr) {
+ return true;
+}
-REGISTER_CHECKER(UnixAPIMisuseChecker)
-REGISTER_CHECKER(UnixAPIPortabilityChecker)
+void ento::registerUnixAPIPortabilityChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<UnixAPIPortabilityChecker>();
+}
+bool ento::shouldRegisterUnixAPIPortabilityChecker(const CheckerManager &Mgr) {
+ return true;
+}
diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 189d7d6bede8e..db177b584b4f5 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -224,16 +224,6 @@ class AnalysisConsumer : public AnalysisASTConsumer,
}
}
- void Initialize(ASTContext &Context) override {
- Ctx = &Context;
- checkerMgr = std::make_unique<CheckerManager>(*Ctx, Opts, PP, Plugins,
- CheckerRegistrationFns);
-
- Mgr = std::make_unique<AnalysisManager>(*Ctx, PP, PathConsumers,
- CreateStoreMgr,
CreateConstraintMgr,
- checkerMgr.get(), Opts, Injector);
- }
-
/// Store the top level decls in the set to be processed later on.
/// (Doing this pre-processing avoids deserialization of data from PCH.)
bool HandleTopLevelDecl(DeclGroupRef D) override;
@@ -615,6 +605,14 @@ void AnalysisConsumer::HandleTranslationUnit(ASTContext
&C) {
if (Diags.hasErrorOccurred() || Diags.hasFatalErrorOccurred())
return;
+ Ctx = &C;
+ checkerMgr = std::make_unique<CheckerManager>(*Ctx, Opts, PP, Plugins,
+ CheckerRegistrationFns);
+
+ Mgr = std::make_unique<AnalysisManager>(*Ctx, PP, PathConsumers,
+ CreateStoreMgr, CreateConstraintMgr,
+ checkerMgr.get(), Opts, Injector);
+
// Explicitly destroy the PathDiagnosticConsumer. This will flush its
output.
// FIXME: This should be replaced with something that doesn't rely on
// side-effects in PathDiagnosticConsumer's destructor. This is required when
>From 10b4bd0403aa82a97f22a7fd55ee512625139091 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbal...@gmail.com>
Date: Mon, 17 Feb 2025 15:14:04 +0100
Subject: [PATCH 2/2] Drop the CachedMacroValue class
---
.../Core/PathSensitive/CheckerHelpers.h | 17 -----------------
.../StaticAnalyzer/Checkers/UnixAPIChecker.cpp | 14 +++++++-------
2 files changed, 7 insertions(+), 24 deletions(-)
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index f7635aa0342d1..b4afaaeec9a4b 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -73,23 +73,6 @@ Nullability getNullabilityAnnotation(QualType Type);
/// returned.
std::optional<int> tryExpandAsInteger(StringRef Macro, const Preprocessor &PP);
-class CachedMacroValue {
-public:
- CachedMacroValue(StringRef MacroSpelling, const Preprocessor &PP)
- : MacroValueOrFailure(tryExpandAsInteger(MacroSpelling, PP)) {}
- explicit CachedMacroValue(int ConcreteValue)
- : MacroValueOrFailure(ConcreteValue) {}
-
- int valueOr(int Default) const {
- return MacroValueOrFailure.value_or(Default);
- }
- bool hasValue() const { return MacroValueOrFailure.has_value(); }
- int value() const { return MacroValueOrFailure.value(); }
-
-private:
- std::optional<int> MacroValueOrFailure;
-};
-
class OperatorKind {
union {
BinaryOperatorKind Bin;
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 0452e541e8d5c..10dfa73cc522d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -40,15 +40,15 @@ enum class OpenVariant {
OpenAt
};
-static CachedMacroValue getCreateFlagValue(const ASTContext &Ctx,
- const Preprocessor &PP) {
- CachedMacroValue MacroVal("O_CREAT", PP);
- if (MacroVal.hasValue())
+static std::optional<int> getCreateFlagValue(const ASTContext &Ctx,
+ const Preprocessor &PP) {
+ std::optional<int> MacroVal = tryExpandAsInteger("O_CREAT", PP);
+ if (MacroVal.has_value())
return MacroVal;
// If we failed, fall-back to known values.
if (Ctx.getTargetInfo().getTriple().getVendor() == llvm::Triple::Apple)
- return CachedMacroValue{0x0200};
+ return {0x0200};
return MacroVal;
}
@@ -61,7 +61,7 @@ class UnixAPIMisuseChecker : public Checker<check::PreCall> {
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
const BugType BT_ArgumentNull{this, "NULL pointer", categories::UnixAPI};
- const CachedMacroValue Val_O_CREAT;
+ const std::optional<int> Val_O_CREAT;
ProgramStateRef
EnsurePtrNotNull(SVal PtrVal, const Expr *PtrExpr, CheckerContext &C,
@@ -262,7 +262,7 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext
&C,
return;
}
- if (!Val_O_CREAT.hasValue()) {
+ if (!Val_O_CREAT.has_value()) {
return;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
