Skip to content

Commit f0088ee

Browse files
authored
[analyzer] Delay the checker constructions after parsing (#127409)
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. **FYI: This may be a breaking change** to some downstream users that may had some means to attach different listeners and what not to e.g. the Preprocessor inside their checker register functions. Since we delay the calls to these register fns after parsing is already done, they would of course miss the parsing Preprocessor events.
1 parent f4e8f6d commit f0088ee

File tree

2 files changed

+40
-39
lines changed

2 files changed

+40
-39
lines changed

clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,28 @@ enum class OpenVariant {
4040
OpenAt
4141
};
4242

43+
static std::optional<int> getCreateFlagValue(const ASTContext &Ctx,
44+
const Preprocessor &PP) {
45+
std::optional<int> MacroVal = tryExpandAsInteger("O_CREAT", PP);
46+
if (MacroVal.has_value())
47+
return MacroVal;
48+
49+
// If we failed, fall-back to known values.
50+
if (Ctx.getTargetInfo().getTriple().getVendor() == llvm::Triple::Apple)
51+
return {0x0200};
52+
return MacroVal;
53+
}
54+
4355
namespace {
4456

45-
class UnixAPIMisuseChecker
46-
: public Checker<check::PreCall, check::ASTDecl<TranslationUnitDecl>> {
57+
class UnixAPIMisuseChecker : public Checker<check::PreCall> {
4758
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
4859
const BugType BT_getline{this, "Improper use of getdelim",
4960
categories::UnixAPI};
5061
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
5162
categories::UnixAPI};
5263
const BugType BT_ArgumentNull{this, "NULL pointer", categories::UnixAPI};
53-
mutable std::optional<uint64_t> Val_O_CREAT;
64+
const std::optional<int> Val_O_CREAT;
5465

5566
ProgramStateRef
5667
EnsurePtrNotNull(SVal PtrVal, const Expr *PtrExpr, CheckerContext &C,
@@ -63,6 +74,9 @@ class UnixAPIMisuseChecker
6374
const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const;
6475

6576
public:
77+
UnixAPIMisuseChecker(const ASTContext &Ctx, const Preprocessor &PP)
78+
: Val_O_CREAT(getCreateFlagValue(Ctx, PP)) {}
79+
6680
void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
6781
BugReporter &BR) const;
6882

@@ -134,20 +148,6 @@ ProgramStateRef UnixAPIMisuseChecker::EnsurePtrNotNull(
134148
return PtrNotNull;
135149
}
136150

137-
void UnixAPIMisuseChecker::checkASTDecl(const TranslationUnitDecl *TU,
138-
AnalysisManager &Mgr,
139-
BugReporter &) const {
140-
// The definition of O_CREAT is platform specific.
141-
// Try to get the macro value from the preprocessor.
142-
Val_O_CREAT = tryExpandAsInteger("O_CREAT", Mgr.getPreprocessor());
143-
// If we failed, fall-back to known values.
144-
if (!Val_O_CREAT) {
145-
if (TU->getASTContext().getTargetInfo().getTriple().getVendor() ==
146-
llvm::Triple::Apple)
147-
Val_O_CREAT = 0x0200;
148-
}
149-
}
150-
151151
//===----------------------------------------------------------------------===//
152152
// "open" (man 2 open)
153153
//===----------------------------------------------------------------------===/
@@ -262,7 +262,7 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
262262
return;
263263
}
264264

265-
if (!Val_O_CREAT) {
265+
if (!Val_O_CREAT.has_value()) {
266266
return;
267267
}
268268

@@ -276,7 +276,7 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
276276
}
277277
NonLoc oflags = V.castAs<NonLoc>();
278278
NonLoc ocreateFlag = C.getSValBuilder()
279-
.makeIntVal(*Val_O_CREAT, oflagsEx->getType())
279+
.makeIntVal(Val_O_CREAT.value(), oflagsEx->getType())
280280
.castAs<NonLoc>();
281281
SVal maskedFlagsUC = C.getSValBuilder().evalBinOpNN(state, BO_And,
282282
oflags, ocreateFlag,
@@ -621,14 +621,17 @@ void UnixAPIPortabilityChecker::checkPreStmt(const CallExpr *CE,
621621
// Registration.
622622
//===----------------------------------------------------------------------===//
623623

624-
#define REGISTER_CHECKER(CHECKERNAME) \
625-
void ento::register##CHECKERNAME(CheckerManager &mgr) { \
626-
mgr.registerChecker<CHECKERNAME>(); \
627-
} \
628-
\
629-
bool ento::shouldRegister##CHECKERNAME(const CheckerManager &mgr) { \
630-
return true; \
631-
}
624+
void ento::registerUnixAPIMisuseChecker(CheckerManager &Mgr) {
625+
Mgr.registerChecker<UnixAPIMisuseChecker>(Mgr.getASTContext(),
626+
Mgr.getPreprocessor());
627+
}
628+
bool ento::shouldRegisterUnixAPIMisuseChecker(const CheckerManager &Mgr) {
629+
return true;
630+
}
632631

633-
REGISTER_CHECKER(UnixAPIMisuseChecker)
634-
REGISTER_CHECKER(UnixAPIPortabilityChecker)
632+
void ento::registerUnixAPIPortabilityChecker(CheckerManager &Mgr) {
633+
Mgr.registerChecker<UnixAPIPortabilityChecker>();
634+
}
635+
bool ento::shouldRegisterUnixAPIPortabilityChecker(const CheckerManager &Mgr) {
636+
return true;
637+
}

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -224,16 +224,6 @@ class AnalysisConsumer : public AnalysisASTConsumer,
224224
}
225225
}
226226

227-
void Initialize(ASTContext &Context) override {
228-
Ctx = &Context;
229-
checkerMgr = std::make_unique<CheckerManager>(*Ctx, Opts, PP, Plugins,
230-
CheckerRegistrationFns);
231-
232-
Mgr = std::make_unique<AnalysisManager>(*Ctx, PP, PathConsumers,
233-
CreateStoreMgr, CreateConstraintMgr,
234-
checkerMgr.get(), Opts, Injector);
235-
}
236-
237227
/// Store the top level decls in the set to be processed later on.
238228
/// (Doing this pre-processing avoids deserialization of data from PCH.)
239229
bool HandleTopLevelDecl(DeclGroupRef D) override;
@@ -615,6 +605,14 @@ void AnalysisConsumer::HandleTranslationUnit(ASTContext &C) {
615605
if (Diags.hasErrorOccurred() || Diags.hasFatalErrorOccurred())
616606
return;
617607

608+
Ctx = &C;
609+
checkerMgr = std::make_unique<CheckerManager>(*Ctx, Opts, PP, Plugins,
610+
CheckerRegistrationFns);
611+
612+
Mgr = std::make_unique<AnalysisManager>(*Ctx, PP, PathConsumers,
613+
CreateStoreMgr, CreateConstraintMgr,
614+
checkerMgr.get(), Opts, Injector);
615+
618616
// Explicitly destroy the PathDiagnosticConsumer. This will flush its output.
619617
// FIXME: This should be replaced with something that doesn't rely on
620618
// side-effects in PathDiagnosticConsumer's destructor. This is required when

0 commit comments

Comments
 (0)