[clang] [NFC][analyzer] Multipart checker refactor 2: NullabilityChecker (PR #132250)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 20 09:29:39 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Donát Nagy (NagyDonat)
<details>
<summary>Changes</summary>
Simplify `NullabilityChecker.cpp` with new multipart checker framework introduced in 27099982da2f5a6c2d282d6b385e79d080669546. This is part of a commit series that will perform analogous changes in all checker classes that implement multiple user-facing checker parts (with separate names).
---
Full diff: https://github.com/llvm/llvm-project/pull/132250.diff
1 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp (+81-76)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 04472bb3895a7..8fe7d21ca7984 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -112,25 +112,38 @@ class NullabilityChecker
void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
const char *Sep) const override;
- enum CheckKind {
- CK_NullPassedToNonnull,
- CK_NullReturnedFromNonnull,
- CK_NullableDereferenced,
- CK_NullablePassedToNonnull,
- CK_NullableReturnedFromNonnull,
- CK_NumCheckKinds
+ // FIXME: This enumeration of checker parts is extremely similar to the
+ // ErrorKind enum. It would be nice to unify them to simplify the code.
+ // FIXME: The modeling checker NullabilityBase is a dummy "empty checker
+ // part" that registers this checker class without enabling any of the real
+ // checker parts. As far as I understand no other checker references it, so
+ // it should be removed.
+ enum : CheckerPartIdx {
+ NullabilityBase,
+ NullPassedToNonnullChecker,
+ NullReturnedFromNonnullChecker,
+ NullableDereferencedChecker,
+ NullablePassedToNonnullChecker,
+ NullableReturnedFromNonnullChecker,
+ NumCheckerParts
};
- bool ChecksEnabled[CK_NumCheckKinds] = {false};
- CheckerNameRef CheckNames[CK_NumCheckKinds];
- mutable std::unique_ptr<BugType> BTs[CK_NumCheckKinds];
-
- const std::unique_ptr<BugType> &getBugType(CheckKind Kind) const {
- if (!BTs[Kind])
- BTs[Kind].reset(new BugType(CheckNames[Kind], "Nullability",
- categories::MemoryError));
- return BTs[Kind];
- }
+ // FIXME: Currently the `Description` fields of these `BugType`s are all
+ // identical ("Nullability") -- they should be more descriptive than this.
+ // NOTE: NullabilityBase is a dummy checker part that does nothing, so its
+ // bug type is left empty.
+ BugType BugTypes[NumCheckerParts] = {
+ {this, NullabilityBase, "", ""},
+ {this, NullPassedToNonnullChecker, "Nullability",
+ categories::MemoryError},
+ {this, NullReturnedFromNonnullChecker, "Nullability",
+ categories::MemoryError},
+ {this, NullableDereferencedChecker, "Nullability",
+ categories::MemoryError},
+ {this, NullablePassedToNonnullChecker, "Nullability",
+ categories::MemoryError},
+ {this, NullableReturnedFromNonnullChecker, "Nullability",
+ categories::MemoryError}};
// When set to false no nullability information will be tracked in
// NullabilityMap. It is possible to catch errors like passing a null pointer
@@ -163,17 +176,16 @@ class NullabilityChecker
///
/// When \p SuppressPath is set to true, no more bugs will be reported on this
/// path by this checker.
- void reportBugIfInvariantHolds(StringRef Msg, ErrorKind Error, CheckKind CK,
- ExplodedNode *N, const MemRegion *Region,
- CheckerContext &C,
+ void reportBugIfInvariantHolds(StringRef Msg, ErrorKind Error,
+ CheckerPartIdx Idx, ExplodedNode *N,
+ const MemRegion *Region, CheckerContext &C,
const Stmt *ValueExpr = nullptr,
bool SuppressPath = false) const;
- void reportBug(StringRef Msg, ErrorKind Error, CheckKind CK, ExplodedNode *N,
- const MemRegion *Region, BugReporter &BR,
+ void reportBug(StringRef Msg, ErrorKind Error, CheckerPartIdx Idx,
+ ExplodedNode *N, const MemRegion *Region, BugReporter &BR,
const Stmt *ValueExpr = nullptr) const {
- const std::unique_ptr<BugType> &BT = getBugType(CK);
- auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
+ auto R = std::make_unique<PathSensitiveBugReport>(BugTypes[Idx], Msg, N);
if (Region) {
R->markInteresting(Region);
R->addVisitor<NullabilityBugVisitor>(Region);
@@ -479,7 +491,7 @@ static bool checkInvariantViolation(ProgramStateRef State, ExplodedNode *N,
}
void NullabilityChecker::reportBugIfInvariantHolds(
- StringRef Msg, ErrorKind Error, CheckKind CK, ExplodedNode *N,
+ StringRef Msg, ErrorKind Error, CheckerPartIdx Idx, ExplodedNode *N,
const MemRegion *Region, CheckerContext &C, const Stmt *ValueExpr,
bool SuppressPath) const {
ProgramStateRef OriginalState = N->getState();
@@ -491,7 +503,7 @@ void NullabilityChecker::reportBugIfInvariantHolds(
N = C.addTransition(OriginalState, N);
}
- reportBug(Msg, Error, CK, N, Region, C.getBugReporter(), ValueExpr);
+ reportBug(Msg, Error, Idx, N, Region, C.getBugReporter(), ValueExpr);
}
/// Cleaning up the program state.
@@ -545,19 +557,19 @@ void NullabilityChecker::checkEvent(ImplicitNullDerefEvent Event) const {
if (!TrackedNullability)
return;
- if (ChecksEnabled[CK_NullableDereferenced] &&
+ if (isPartEnabled(NullableDereferencedChecker) &&
TrackedNullability->getValue() == Nullability::Nullable) {
BugReporter &BR = *Event.BR;
// Do not suppress errors on defensive code paths, because dereferencing
// a nullable pointer is always an error.
if (Event.IsDirectDereference)
reportBug("Nullable pointer is dereferenced",
- ErrorKind::NullableDereferenced, CK_NullableDereferenced,
+ ErrorKind::NullableDereferenced, NullableDereferencedChecker,
Event.SinkNode, Region, BR);
else {
reportBug("Nullable pointer is passed to a callee that requires a "
"non-null",
- ErrorKind::NullablePassedToNonnull, CK_NullableDereferenced,
+ ErrorKind::NullablePassedToNonnull, NullableDereferencedChecker,
Event.SinkNode, Region, BR);
}
}
@@ -711,7 +723,8 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
bool NullReturnedFromNonNull = (RequiredNullability == Nullability::Nonnull &&
Nullness == NullConstraint::IsNull);
- if (ChecksEnabled[CK_NullReturnedFromNonnull] && NullReturnedFromNonNull &&
+ if (isPartEnabled(NullReturnedFromNonnullChecker) &&
+ NullReturnedFromNonNull &&
RetExprTypeLevelNullability != Nullability::Nonnull &&
!InSuppressedMethodFamily) {
static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull");
@@ -725,7 +738,7 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
OS << " returned from a " << C.getDeclDescription(D) <<
" that is expected to return a non-null value";
reportBugIfInvariantHolds(OS.str(), ErrorKind::NilReturnedToNonnull,
- CK_NullReturnedFromNonnull, N, nullptr, C,
+ NullReturnedFromNonnullChecker, N, nullptr, C,
RetExpr);
return;
}
@@ -746,7 +759,7 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
State->get<NullabilityMap>(Region);
if (TrackedNullability) {
Nullability TrackedNullabValue = TrackedNullability->getValue();
- if (ChecksEnabled[CK_NullableReturnedFromNonnull] &&
+ if (isPartEnabled(NullableReturnedFromNonnullChecker) &&
Nullness != NullConstraint::IsNotNull &&
TrackedNullabValue == Nullability::Nullable &&
RequiredNullability == Nullability::Nonnull) {
@@ -759,7 +772,8 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
" that is expected to return a non-null value";
reportBugIfInvariantHolds(OS.str(), ErrorKind::NullableReturnedToNonnull,
- CK_NullableReturnedFromNonnull, N, Region, C);
+ NullableReturnedFromNonnullChecker, N, Region,
+ C);
}
return;
}
@@ -810,7 +824,7 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
unsigned ParamIdx = Param->getFunctionScopeIndex() + 1;
- if (ChecksEnabled[CK_NullPassedToNonnull] &&
+ if (isPartEnabled(NullPassedToNonnullChecker) &&
Nullness == NullConstraint::IsNull &&
ArgExprTypeLevelNullability != Nullability::Nonnull &&
RequiredNullability == Nullability::Nonnull &&
@@ -825,7 +839,8 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
OS << " passed to a callee that requires a non-null " << ParamIdx
<< llvm::getOrdinalSuffix(ParamIdx) << " parameter";
reportBugIfInvariantHolds(OS.str(), ErrorKind::NilPassedToNonnull,
- CK_NullPassedToNonnull, N, nullptr, C, ArgExpr,
+ NullPassedToNonnullChecker, N, nullptr, C,
+ ArgExpr,
/*SuppressPath=*/false);
return;
}
@@ -842,7 +857,7 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
TrackedNullability->getValue() != Nullability::Nullable)
continue;
- if (ChecksEnabled[CK_NullablePassedToNonnull] &&
+ if (isPartEnabled(NullablePassedToNonnullChecker) &&
RequiredNullability == Nullability::Nonnull &&
isDiagnosableCall(Call)) {
ExplodedNode *N = C.addTransition(State);
@@ -851,16 +866,16 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
OS << "Nullable pointer is passed to a callee that requires a non-null "
<< ParamIdx << llvm::getOrdinalSuffix(ParamIdx) << " parameter";
reportBugIfInvariantHolds(OS.str(), ErrorKind::NullablePassedToNonnull,
- CK_NullablePassedToNonnull, N, Region, C,
+ NullablePassedToNonnullChecker, N, Region, C,
ArgExpr, /*SuppressPath=*/true);
return;
}
- if (ChecksEnabled[CK_NullableDereferenced] &&
+ if (isPartEnabled(NullableDereferencedChecker) &&
Param->getType()->isReferenceType()) {
ExplodedNode *N = C.addTransition(State);
reportBugIfInvariantHolds("Nullable pointer is dereferenced",
ErrorKind::NullableDereferenced,
- CK_NullableDereferenced, N, Region, C,
+ NullableDereferencedChecker, N, Region, C,
ArgExpr, /*SuppressPath=*/true);
return;
}
@@ -1295,7 +1310,7 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S,
bool NullAssignedToNonNull = (LocNullability == Nullability::Nonnull &&
RhsNullness == NullConstraint::IsNull);
- if (ChecksEnabled[CK_NullPassedToNonnull] && NullAssignedToNonNull &&
+ if (isPartEnabled(NullPassedToNonnullChecker) && NullAssignedToNonNull &&
ValNullability != Nullability::Nonnull &&
ValueExprTypeLevelNullability != Nullability::Nonnull &&
!isARCNilInitializedLocal(C, S)) {
@@ -1314,7 +1329,8 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S,
OS << (LocType->isObjCObjectPointerType() ? "nil" : "Null");
OS << " assigned to a pointer which is expected to have non-null value";
reportBugIfInvariantHolds(OS.str(), ErrorKind::NilAssignedToNonnull,
- CK_NullPassedToNonnull, N, nullptr, C, ValueStmt);
+ NullPassedToNonnullChecker, N, nullptr, C,
+ ValueStmt);
return;
}
@@ -1340,14 +1356,15 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S,
if (RhsNullness == NullConstraint::IsNotNull ||
TrackedNullability->getValue() != Nullability::Nullable)
return;
- if (ChecksEnabled[CK_NullablePassedToNonnull] &&
+ if (isPartEnabled(NullablePassedToNonnullChecker) &&
LocNullability == Nullability::Nonnull) {
static CheckerProgramPointTag Tag(this, "NullablePassedToNonnull");
ExplodedNode *N = C.addTransition(State, C.getPredecessor(), &Tag);
reportBugIfInvariantHolds("Nullable pointer is assigned to a pointer "
"which is expected to have non-null value",
ErrorKind::NullableAssignedToNonnull,
- CK_NullablePassedToNonnull, N, ValueRegion, C);
+ NullablePassedToNonnullChecker, N, ValueRegion,
+ C);
}
return;
}
@@ -1394,38 +1411,26 @@ void NullabilityChecker::printState(raw_ostream &Out, ProgramStateRef State,
}
}
-void ento::registerNullabilityBase(CheckerManager &mgr) {
- mgr.registerChecker<NullabilityChecker>();
-}
-
-bool ento::shouldRegisterNullabilityBase(const CheckerManager &mgr) {
- return true;
-}
-
-#define REGISTER_CHECKER(name, trackingRequired) \
- void ento::register##name##Checker(CheckerManager &mgr) { \
- NullabilityChecker *checker = mgr.getChecker<NullabilityChecker>(); \
- checker->ChecksEnabled[NullabilityChecker::CK_##name] = true; \
- checker->CheckNames[NullabilityChecker::CK_##name] = \
- mgr.getCurrentCheckerName(); \
- checker->NeedTracking = checker->NeedTracking || trackingRequired; \
- checker->NoDiagnoseCallsToSystemHeaders = \
- checker->NoDiagnoseCallsToSystemHeaders || \
- mgr.getAnalyzerOptions().getCheckerBooleanOption( \
- checker, "NoDiagnoseCallsToSystemHeaders", true); \
+#define REGISTER_CHECKER(Part, TrackingRequired) \
+ void ento::register##Part(CheckerManager &Mgr) { \
+ auto *Checker = \
+ Mgr.registerChecker<NullabilityChecker, NullabilityChecker::Part>(); \
+ Checker->NeedTracking = Checker->NeedTracking || TrackingRequired; \
+ Checker->NoDiagnoseCallsToSystemHeaders = \
+ Checker->NoDiagnoseCallsToSystemHeaders || \
+ Mgr.getAnalyzerOptions().getCheckerBooleanOption( \
+ Checker, "NoDiagnoseCallsToSystemHeaders", true); \
} \
\
- bool ento::shouldRegister##name##Checker(const CheckerManager &mgr) { \
- return true; \
- }
-
-// The checks are likely to be turned on by default and it is possible to do
-// them without tracking any nullability related information. As an optimization
-// no nullability information will be tracked when only these two checks are
-// enables.
-REGISTER_CHECKER(NullPassedToNonnull, false)
-REGISTER_CHECKER(NullReturnedFromNonnull, false)
-
-REGISTER_CHECKER(NullableDereferenced, true)
-REGISTER_CHECKER(NullablePassedToNonnull, true)
-REGISTER_CHECKER(NullableReturnedFromNonnull, true)
+ bool ento::shouldRegister##Part(const CheckerManager &) { return true; }
+
+// These checker parts are likely to be turned on by default and they don't
+// need the tracking of nullability related information. As an optimization
+// nullability information won't be tracked when the rest are disabled.
+REGISTER_CHECKER(NullabilityBase, false)
+REGISTER_CHECKER(NullPassedToNonnullChecker, false)
+REGISTER_CHECKER(NullReturnedFromNonnullChecker, false)
+
+REGISTER_CHECKER(NullableDereferencedChecker, true)
+REGISTER_CHECKER(NullablePassedToNonnullChecker, true)
+REGISTER_CHECKER(NullableReturnedFromNonnullChecker, true)
``````````
</details>
https://github.com/llvm/llvm-project/pull/132250
More information about the cfe-commits
mailing list