[clang] [analyzer] Conversion to CheckerFamily: NSOrCFErrorDerefChecker (PR #151171)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 29 08:24:28 PDT 2025
https://github.com/NagyDonat created https://github.com/llvm/llvm-project/pull/151171
This commit converts the class `NSOrCFErrorDerefChecker` to the checker family framework and simplifies some parts of the implementation (e.g. removes two very trivial subclasses of `BugType`).
This commit is almost NFC, the only technically "functional" change is that it removes the hidden modeling checker `NSOrCFErrorDerefChecker` which was only relevant as an implementation detail of the old checker registration procedure.
>From 036f1e61900c19b0142c20580a0bc5894f509921 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 29 Jul 2025 17:18:34 +0200
Subject: [PATCH] [analyzer] Conversion to CheckerFamily:
NSOrCFErrorDerefChecker
This commit converts the class `NSOrCFErrorDerefChecker` to the checker
family framework and simplifies some parts of the implementation (e.g.
removes two very trivial subclasses of `BugType`).
This commit is almost NFC, the only technically "functional" change is
that it removes the hidden modeling checker `NSOrCFErrorDerefChecker`
which was only relevant as an implementation detail of the old checker
registration procedure.
---
.../clang/StaticAnalyzer/Checkers/Checkers.td | 15 +--
.../Checkers/NSErrorChecker.cpp | 97 ++++++-------------
2 files changed, 34 insertions(+), 78 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 6225977e980cc..543edaaff5871 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1047,11 +1047,6 @@ def RetainCountBase : Checker<"RetainCountBase">,
let ParentPackage = OSX in {
-def NSOrCFErrorDerefChecker : Checker<"NSOrCFErrorDerefChecker">,
- HelpText<"Implementation checker for NSErrorChecker and CFErrorChecker">,
- Documentation<NotDocumented>,
- Hidden;
-
def NumberObjectConversionChecker : Checker<"NumberObjectConversion">,
HelpText<"Check for erroneous conversions of objects representing numbers "
"into numbers">,
@@ -1149,9 +1144,8 @@ def ObjCSuperCallChecker : Checker<"MissingSuperCall">,
Documentation<NotDocumented>;
def NSErrorChecker : Checker<"NSError">,
- HelpText<"Check usage of NSError** parameters">,
- Dependencies<[NSOrCFErrorDerefChecker]>,
- Documentation<HasDocumentation>;
+ HelpText<"Check usage of NSError** parameters">,
+ Documentation<HasDocumentation>;
def RetainCountChecker : Checker<"RetainCount">,
HelpText<"Check for leaks and improper reference count management">,
@@ -1253,9 +1247,8 @@ def CFRetainReleaseChecker : Checker<"CFRetainRelease">,
Documentation<HasDocumentation>;
def CFErrorChecker : Checker<"CFError">,
- HelpText<"Check usage of CFErrorRef* parameters">,
- Dependencies<[NSOrCFErrorDerefChecker]>,
- Documentation<HasDocumentation>;
+ HelpText<"Check usage of CFErrorRef* parameters">,
+ Documentation<HasDocumentation>;
} // end "osx.coreFoundation"
diff --git a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
index 15fd9a0b76cc3..ae1675406b798 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -142,34 +142,18 @@ void CFErrorFunctionChecker::checkASTDecl(const FunctionDecl *D,
//===----------------------------------------------------------------------===//
namespace {
+class NSOrCFErrorDerefChecker
+ : public CheckerFamily<check::Location,
+ check::Event<ImplicitNullDerefEvent>> {
+ mutable IdentifierInfo *NSErrorII = nullptr, *CFErrorII = nullptr;
-class NSErrorDerefBug : public BugType {
-public:
- NSErrorDerefBug(const CheckerNameRef Checker)
- : BugType(Checker, "NSError** null dereference",
- "Coding conventions (Apple)") {}
-};
-
-class CFErrorDerefBug : public BugType {
public:
- CFErrorDerefBug(const CheckerNameRef Checker)
- : BugType(Checker, "CFErrorRef* null dereference",
- "Coding conventions (Apple)") {}
-};
-
-}
+ CheckerFrontendWithBugType NSError{"NSError** null dereference",
+ "Coding conventions (Apple)"};
+ CheckerFrontendWithBugType CFError{"CFErrorRef* null dereference",
+ "Coding conventions (Apple)"};
-namespace {
-class NSOrCFErrorDerefChecker
- : public Checker< check::Location,
- check::Event<ImplicitNullDerefEvent> > {
- mutable IdentifierInfo *NSErrorII, *CFErrorII;
- mutable std::unique_ptr<NSErrorDerefBug> NSBT;
- mutable std::unique_ptr<CFErrorDerefBug> CFBT;
-public:
- bool ShouldCheckNSError = false, ShouldCheckCFError = false;
- CheckerNameRef NSErrorName, CFErrorName;
- NSOrCFErrorDerefChecker() : NSErrorII(nullptr), CFErrorII(nullptr) {}
+ StringRef getDebugTag() const override { return "NSOrCFErrorDerefChecker"; }
void checkLocation(SVal loc, bool isLoad, const Stmt *S,
CheckerContext &C) const;
@@ -236,12 +220,12 @@ void NSOrCFErrorDerefChecker::checkLocation(SVal loc, bool isLoad,
if (!CFErrorII)
CFErrorII = &Ctx.Idents.get("CFErrorRef");
- if (ShouldCheckNSError && IsNSError(parmT, NSErrorII)) {
+ if (NSError.isEnabled() && IsNSError(parmT, NSErrorII)) {
setFlag<NSErrorOut>(state, state->getSVal(loc.castAs<Loc>()), C);
return;
}
- if (ShouldCheckCFError && IsCFError(parmT, CFErrorII)) {
+ if (CFError.isEnabled() && IsCFError(parmT, CFErrorII)) {
setFlag<CFErrorOut>(state, state->getSVal(loc.castAs<Loc>()), C);
return;
}
@@ -274,19 +258,9 @@ void NSOrCFErrorDerefChecker::checkEvent(ImplicitNullDerefEvent event) const {
os << " may be null";
- BugType *bug = nullptr;
- if (isNSError) {
- if (!NSBT)
- NSBT.reset(new NSErrorDerefBug(NSErrorName));
- bug = NSBT.get();
- }
- else {
- if (!CFBT)
- CFBT.reset(new CFErrorDerefBug(CFErrorName));
- bug = CFBT.get();
- }
+ const BugType &BT = isNSError ? NSError : CFError;
BR.emitReport(
- std::make_unique<PathSensitiveBugReport>(*bug, os.str(), event.SinkNode));
+ std::make_unique<PathSensitiveBugReport>(BT, os.str(), event.SinkNode));
}
static bool IsNSError(QualType T, IdentifierInfo *II) {
@@ -320,32 +294,21 @@ static bool IsCFError(QualType T, IdentifierInfo *II) {
return TT->getDecl()->getIdentifier() == II;
}
-void ento::registerNSOrCFErrorDerefChecker(CheckerManager &mgr) {
- mgr.registerChecker<NSOrCFErrorDerefChecker>();
-}
-
-bool ento::shouldRegisterNSOrCFErrorDerefChecker(const CheckerManager &mgr) {
- return true;
-}
-
-void ento::registerNSErrorChecker(CheckerManager &mgr) {
- mgr.registerChecker<NSErrorMethodChecker>();
- NSOrCFErrorDerefChecker *checker = mgr.getChecker<NSOrCFErrorDerefChecker>();
- checker->ShouldCheckNSError = true;
- checker->NSErrorName = mgr.getCurrentCheckerName();
-}
-
-bool ento::shouldRegisterNSErrorChecker(const CheckerManager &mgr) {
- return true;
-}
-
-void ento::registerCFErrorChecker(CheckerManager &mgr) {
- mgr.registerChecker<CFErrorFunctionChecker>();
- NSOrCFErrorDerefChecker *checker = mgr.getChecker<NSOrCFErrorDerefChecker>();
- checker->ShouldCheckCFError = true;
- checker->CFErrorName = mgr.getCurrentCheckerName();
-}
+// This source file implements two user-facing checkers ("osx.cocoa.NSError"
+// and "osx.coreFoundation.CFError") which are both implemented as the
+// combination of two `CheckerFrontend`s that are registered under the same
+// name (but otherwise act independently). Among these 2+2 `CheckerFrontend`s
+// two are coming from the checker family `NSOrCFErrorDerefChecker` while the
+// other two (the `ADDITIONAL_PART`s) are small standalone checkers.
+#define REGISTER_CHECKER(NAME, ADDITIONAL_PART) \
+ void ento::register##NAME##Checker(CheckerManager &Mgr) { \
+ Mgr.getChecker<NSOrCFErrorDerefChecker>()->NAME.enable(Mgr); \
+ Mgr.registerChecker<ADDITIONAL_PART>(); \
+ } \
+ \
+ bool ento::shouldRegister##NAME##Checker(const CheckerManager &) { \
+ return true; \
+ }
-bool ento::shouldRegisterCFErrorChecker(const CheckerManager &mgr) {
- return true;
-}
+REGISTER_CHECKER(NSError, NSErrorMethodChecker)
+REGISTER_CHECKER(CFError, CFErrorFunctionChecker)
More information about the cfe-commits
mailing list