https://github.com/NagyDonat updated
https://github.com/llvm/llvm-project/pull/132250
From d4878a62a69304dc2ae32902803f8c8efb1c69ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Thu, 20 Mar 2025 17:09:46 +0100
Subject: [PATCH 1/4] [NFC][analyzer] Multipart checker refactor 2:
NullabilityChecker
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).
---
.../Checkers/NullabilityChecker.cpp | 157 +++++++++---------
1 file changed, 81 insertions(+), 76 deletions(-)
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)
From 2708bed01435d5953bf0446626f7c1e6727fef31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Thu, 20 Mar 2025 17:52:24 +0100
Subject: [PATCH 2/4] Remove hidden dummy checker part NullabilityBase
As far as I see it was just a hacky implementation detail within the
old solution for registering the "real" checker parts. I'm placing this
into a separate commit to make the review easier.
---
.../clang/StaticAnalyzer/Checkers/Checkers.td | 56 +++++++++----------
.../Checkers/NullabilityChecker.cpp | 32 +++++------
.../test/Analysis/analyzer-enabled-checkers.c | 1 -
clang/test/Analysis/bugfix-124477.m | 2 +-
...c-library-functions-arg-enabled-checkers.c | 1 -
5 files changed, 41 insertions(+), 51 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 35df4e7003ac9..0df5304ccf939 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -326,39 +326,37 @@ def StdVariantChecker : Checker<"StdVariant">,
let ParentPackage = Nullability in {
-def NullabilityBase : Checker<"NullabilityBase">,
- HelpText<"Stores information during the analysis about nullability.">,
- Documentation<NotDocumented>,
- Hidden;
-
-def NullPassedToNonnullChecker : Checker<"NullPassedToNonnull">,
- HelpText<"Warns when a null pointer is passed to a pointer which has a "
- "_Nonnull type.">,
- Dependencies<[NullabilityBase]>,
- Documentation<HasDocumentation>;
+ def NullPassedToNonnullChecker
+ : Checker<"NullPassedToNonnull">,
+ HelpText<"Warns when a null pointer is passed to a pointer which has a
"
+ "_Nonnull type.">,
+ Documentation<HasDocumentation>;
-def NullReturnedFromNonnullChecker : Checker<"NullReturnedFromNonnull">,
- HelpText<"Warns when a null pointer is returned from a function that has "
- "_Nonnull return type.">,
- Dependencies<[NullabilityBase]>,
- Documentation<HasDocumentation>;
+ def NullReturnedFromNonnullChecker
+ : Checker<"NullReturnedFromNonnull">,
+ HelpText<
+ "Warns when a null pointer is returned from a function that has "
+ "_Nonnull return type.">,
+ Documentation<HasDocumentation>;
-def NullableDereferencedChecker : Checker<"NullableDereferenced">,
- HelpText<"Warns when a nullable pointer is dereferenced.">,
- Dependencies<[NullabilityBase]>,
- Documentation<HasDocumentation>;
+ def NullableDereferencedChecker
+ : Checker<"NullableDereferenced">,
+ HelpText<"Warns when a nullable pointer is dereferenced.">,
+ Documentation<HasDocumentation>;
-def NullablePassedToNonnullChecker : Checker<"NullablePassedToNonnull">,
- HelpText<"Warns when a nullable pointer is passed to a pointer which has a "
- "_Nonnull type.">,
- Dependencies<[NullabilityBase]>,
- Documentation<HasDocumentation>;
+ def NullablePassedToNonnullChecker
+ : Checker<"NullablePassedToNonnull">,
+ HelpText<
+ "Warns when a nullable pointer is passed to a pointer which has a "
+ "_Nonnull type.">,
+ Documentation<HasDocumentation>;
-def NullableReturnedFromNonnullChecker :
Checker<"NullableReturnedFromNonnull">,
- HelpText<"Warns when a nullable pointer is returned from a function that has
"
- "_Nonnull return type.">,
- Dependencies<[NullabilityBase]>,
- Documentation<NotDocumented>;
+ def NullableReturnedFromNonnullChecker
+ : Checker<"NullableReturnedFromNonnull">,
+ HelpText<"Warns when a nullable pointer is returned from a function "
+ "that has "
+ "_Nonnull return type.">,
+ Documentation<NotDocumented>;
} // end "nullability"
diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 8fe7d21ca7984..0129f6132e208 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -114,12 +114,7 @@ class NullabilityChecker
// 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,
@@ -130,10 +125,7 @@ class NullabilityChecker
// 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",
@@ -1412,25 +1404,27 @@ void NullabilityChecker::printState(raw_ostream &Out,
ProgramStateRef State,
}
#define REGISTER_CHECKER(Part, TrackingRequired)
\
- void ento::register##Part(CheckerManager &Mgr) {
\
- auto *Checker =
\
- Mgr.registerChecker<NullabilityChecker, NullabilityChecker::Part>();
\
+ void ento::register##Part##Checker(CheckerManager &Mgr) {
\
+ auto *Checker = Mgr.registerChecker<NullabilityChecker,
\
+ NullabilityChecker::Part##Checker>();
\
Checker->NeedTracking = Checker->NeedTracking || TrackingRequired;
\
Checker->NoDiagnoseCallsToSystemHeaders =
\
Checker->NoDiagnoseCallsToSystemHeaders ||
\
Mgr.getAnalyzerOptions().getCheckerBooleanOption(
\
- Checker, "NoDiagnoseCallsToSystemHeaders", true);
\
+ Mgr.getCurrentCheckerName(), "NoDiagnoseCallsToSystemHeaders",
\
+ true);
\
}
\
\
- bool ento::shouldRegister##Part(const CheckerManager &) { return true; }
+ bool ento::shouldRegister##Part##Checker(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(NullPassedToNonnull, false)
+REGISTER_CHECKER(NullReturnedFromNonnull, false)
-REGISTER_CHECKER(NullableDereferencedChecker, true)
-REGISTER_CHECKER(NullablePassedToNonnullChecker, true)
-REGISTER_CHECKER(NullableReturnedFromNonnullChecker, true)
+REGISTER_CHECKER(NullableDereferenced, true)
+REGISTER_CHECKER(NullablePassedToNonnull, true)
+REGISTER_CHECKER(NullableReturnedFromNonnull, true)
diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c
b/clang/test/Analysis/analyzer-enabled-checkers.c
index e5d0acb84a039..6d1e5311241d2 100644
--- a/clang/test/Analysis/analyzer-enabled-checkers.c
+++ b/clang/test/Analysis/analyzer-enabled-checkers.c
@@ -33,7 +33,6 @@
// CHECK-NEXT: core.uninitialized.CapturedBlockVariable
// CHECK-NEXT: core.uninitialized.UndefReturn
// CHECK-NEXT: deadcode.DeadStores
-// CHECK-NEXT: nullability.NullabilityBase
// CHECK-NEXT: nullability.NullPassedToNonnull
// CHECK-NEXT: nullability.NullReturnedFromNonnull
// CHECK-NEXT: security.insecureAPI.SecuritySyntaxChecker
diff --git a/clang/test/Analysis/bugfix-124477.m
b/clang/test/Analysis/bugfix-124477.m
index 80820f4c93444..8bb0196b2f9b8 100644
--- a/clang/test/Analysis/bugfix-124477.m
+++ b/clang/test/Analysis/bugfix-124477.m
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1
-analyzer-checker=core,apiModeling,nullability.NullableDereferenced,nullability.NullabilityBase
-x objective-c %s
+// RUN: %clang_analyze_cc1
-analyzer-checker=core,apiModeling,nullability.NullableDereferenced -x
objective-c %s
/*
This test is reduced from a static analyzer crash. The bug causing
the crash is explained in #124477. It can only be triggered in some
diff --git a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
index d2900c6a42fff..be60870fdb4dc 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -41,7 +41,6 @@
// CHECK-NEXT: core.uninitialized.CapturedBlockVariable
// CHECK-NEXT: core.uninitialized.UndefReturn
// CHECK-NEXT: deadcode.DeadStores
-// CHECK-NEXT: nullability.NullabilityBase
// CHECK-NEXT: nullability.NullPassedToNonnull
// CHECK-NEXT: nullability.NullReturnedFromNonnull
// CHECK-NEXT: security.insecureAPI.SecuritySyntaxChecker
From 3ca23ce42044b84507efd96874bea30cec437ff0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Thu, 20 Mar 2025 17:59:50 +0100
Subject: [PATCH 3/4] Use `BugType` parameters for
`reportBug{,IfInvariantHolds}`
---
.../Checkers/NullabilityChecker.cpp | 50 +++++++++----------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 0129f6132e208..1529c3a3b6319 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -169,15 +169,15 @@ 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,
- CheckerPartIdx Idx, ExplodedNode *N,
+ const BugType &BT, ExplodedNode *N,
const MemRegion *Region, CheckerContext &C,
const Stmt *ValueExpr = nullptr,
bool SuppressPath = false) const;
- void reportBug(StringRef Msg, ErrorKind Error, CheckerPartIdx Idx,
+ void reportBug(StringRef Msg, ErrorKind Error, const BugType &BT,
ExplodedNode *N, const MemRegion *Region, BugReporter &BR,
const Stmt *ValueExpr = nullptr) const {
- auto R = std::make_unique<PathSensitiveBugReport>(BugTypes[Idx], Msg, N);
+ auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
if (Region) {
R->markInteresting(Region);
R->addVisitor<NullabilityBugVisitor>(Region);
@@ -483,7 +483,7 @@ static bool checkInvariantViolation(ProgramStateRef State,
ExplodedNode *N,
}
void NullabilityChecker::reportBugIfInvariantHolds(
- StringRef Msg, ErrorKind Error, CheckerPartIdx Idx, ExplodedNode *N,
+ StringRef Msg, ErrorKind Error, const BugType &BT, ExplodedNode *N,
const MemRegion *Region, CheckerContext &C, const Stmt *ValueExpr,
bool SuppressPath) const {
ProgramStateRef OriginalState = N->getState();
@@ -495,7 +495,7 @@ void NullabilityChecker::reportBugIfInvariantHolds(
N = C.addTransition(OriginalState, N);
}
- reportBug(Msg, Error, Idx, N, Region, C.getBugReporter(), ValueExpr);
+ reportBug(Msg, Error, BT, N, Region, C.getBugReporter(), ValueExpr);
}
/// Cleaning up the program state.
@@ -555,14 +555,15 @@ void
NullabilityChecker::checkEvent(ImplicitNullDerefEvent Event) const {
// 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, NullableDereferencedChecker,
- Event.SinkNode, Region, BR);
+ reportBug(
+ "Nullable pointer is dereferenced", ErrorKind::NullableDereferenced,
+ BugTypes[NullableDereferencedChecker], Event.SinkNode, Region, BR);
else {
reportBug("Nullable pointer is passed to a callee that requires a "
"non-null",
- ErrorKind::NullablePassedToNonnull,
NullableDereferencedChecker,
- Event.SinkNode, Region, BR);
+ ErrorKind::NullablePassedToNonnull,
+ BugTypes[NullableDereferencedChecker], Event.SinkNode, Region,
+ BR);
}
}
}
@@ -730,8 +731,8 @@ 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,
- NullReturnedFromNonnullChecker, N, nullptr, C,
- RetExpr);
+ BugTypes[NullReturnedFromNonnullChecker], N,
+ nullptr, C, RetExpr);
return;
}
@@ -764,8 +765,8 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
" that is expected to return a non-null value";
reportBugIfInvariantHolds(OS.str(), ErrorKind::NullableReturnedToNonnull,
- NullableReturnedFromNonnullChecker, N, Region,
- C);
+ BugTypes[NullableReturnedFromNonnullChecker],
N,
+ Region, C);
}
return;
}
@@ -831,9 +832,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,
- NullPassedToNonnullChecker, N, nullptr, C,
- ArgExpr,
- /*SuppressPath=*/false);
+ BugTypes[NullPassedToNonnullChecker], N,
+ nullptr, C, ArgExpr, /*SuppressPath=*/false);
return;
}
@@ -858,8 +858,8 @@ 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,
- NullablePassedToNonnullChecker, N, Region, C,
- ArgExpr, /*SuppressPath=*/true);
+ BugTypes[NullablePassedToNonnullChecker], N,
+ Region, C, ArgExpr, /*SuppressPath=*/true);
return;
}
if (isPartEnabled(NullableDereferencedChecker) &&
@@ -867,8 +867,8 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
ExplodedNode *N = C.addTransition(State);
reportBugIfInvariantHolds("Nullable pointer is dereferenced",
ErrorKind::NullableDereferenced,
- NullableDereferencedChecker, N, Region, C,
- ArgExpr, /*SuppressPath=*/true);
+ BugTypes[NullableDereferencedChecker], N,
+ Region, C, ArgExpr, /*SuppressPath=*/true);
return;
}
continue;
@@ -1321,8 +1321,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,
- NullPassedToNonnullChecker, N, nullptr, C,
- ValueStmt);
+ BugTypes[NullPassedToNonnullChecker], N, nullptr,
+ C, ValueStmt);
return;
}
@@ -1355,8 +1355,8 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const
Stmt *S,
reportBugIfInvariantHolds("Nullable pointer is assigned to a pointer "
"which is expected to have non-null value",
ErrorKind::NullableAssignedToNonnull,
- NullablePassedToNonnullChecker, N, ValueRegion,
- C);
+ BugTypes[NullablePassedToNonnullChecker], N,
+ ValueRegion, C);
}
return;
}
From d490c74ea866107be4131bd6e694166e702ed8aa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Thu, 20 Mar 2025 18:23:31 +0100
Subject: [PATCH 4/4] Special case NoDiagnoseCallsToSystemHeaders as
group-level option
---
.../StaticAnalyzer/Checkers/NullabilityChecker.cpp | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 1529c3a3b6319..54f0d6e4d3486 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -1403,6 +1403,16 @@ void NullabilityChecker::printState(raw_ostream &Out,
ProgramStateRef State,
}
}
+// The checker group "nullability" consists of the checkers that are
+// implemented as the parts of the checker class `NullabilityChecker`. These
+// checkers share a checker option "nullability:NoDiagnoseCallsToSystemHeaders"
+// which semantically belongs to the whole group and not just one checker from
+// it. As this is a unique situation (I don't know about any other similar
+// group-level option) there is no mechanism to inject this group name from
+// e.g. Checkers.td, so I'm just hardcoding it here. (These are old stable
+// checkers, I don't think that their name will change.)
+#define CHECKER_GROUP_NAME "nullability"
+
#define REGISTER_CHECKER(Part, TrackingRequired)
\
void ento::register##Part##Checker(CheckerManager &Mgr) {
\
auto *Checker = Mgr.registerChecker<NullabilityChecker,
\
@@ -1411,8 +1421,7 @@ void NullabilityChecker::printState(raw_ostream &Out,
ProgramStateRef State,
Checker->NoDiagnoseCallsToSystemHeaders =
\
Checker->NoDiagnoseCallsToSystemHeaders ||
\
Mgr.getAnalyzerOptions().getCheckerBooleanOption(
\
- Mgr.getCurrentCheckerName(), "NoDiagnoseCallsToSystemHeaders",
\
- true);
\
+ CHECKER_GROUP_NAME, "NoDiagnoseCallsToSystemHeaders", true);
\
}
\
\
bool ento::shouldRegister##Part##Checker(const CheckerManager &) {
\
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
