Author: Donát Nagy Date: 2025-06-17T11:51:09+02:00 New Revision: 4c8f43440955c93a54b9547421513867bc81788a
URL: https://github.com/llvm/llvm-project/commit/4c8f43440955c93a54b9547421513867bc81788a DIFF: https://github.com/llvm/llvm-project/commit/4c8f43440955c93a54b9547421513867bc81788a.diff LOG: [analyzer] Conversion to CheckerFamily: NullabilityChecker (#143735) This commit converts NullabilityChecker to the new checker family framework that was introduced in the recent commit 6833076a5d9f5719539a24e900037da5a3979289 This commit removes the dummy checker `nullability.NullabilityBase` because it was hidden from the users and didn't have any useful role except for helping the registration of the checker parts in the old ad-hoc system (which is replaced by the new standardized framework). Except for the removal of this dummy checker, no functional changes intended. Added: Modified: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp clang/test/Analysis/analyzer-enabled-checkers.c clang/test/Analysis/bugfix-124477.m clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c Removed: ################################################################################ diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 2a96df80d1001..211ce585fbac8 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -326,39 +326,34 @@ 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 461d01b452fd0..9744d1abf7790 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -81,11 +81,12 @@ enum class ErrorKind : int { }; class NullabilityChecker - : public Checker<check::Bind, check::PreCall, check::PreStmt<ReturnStmt>, - check::PostCall, check::PostStmt<ExplicitCastExpr>, - check::PostObjCMessage, check::DeadSymbols, eval::Assume, - check::Location, check::Event<ImplicitNullDerefEvent>, - check::BeginFunction> { + : public CheckerFamily< + check::Bind, check::PreCall, check::PreStmt<ReturnStmt>, + check::PostCall, check::PostStmt<ExplicitCastExpr>, + check::PostObjCMessage, check::DeadSymbols, eval::Assume, + check::Location, check::Event<ImplicitNullDerefEvent>, + check::BeginFunction> { public: // If true, the checker will not diagnose nullabilility issues for calls @@ -113,25 +114,21 @@ 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 - }; - - 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]; - } + StringRef getDebugTag() const override { return "NullabilityChecker"; } + + // FIXME: All bug types share the same Description ("Nullability") since the + // creation of this checker. We should write more descriptive descriptions... + // or just eliminate the Description field if it is meaningless? + CheckerFrontendWithBugType NullPassedToNonnull{"Nullability", + categories::MemoryError}; + CheckerFrontendWithBugType NullReturnedFromNonnull{"Nullability", + categories::MemoryError}; + CheckerFrontendWithBugType NullableDereferenced{"Nullability", + categories::MemoryError}; + CheckerFrontendWithBugType NullablePassedToNonnull{"Nullability", + categories::MemoryError}; + CheckerFrontendWithBugType NullableReturnedFromNonnull{ + "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 @@ -164,17 +161,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, + const BugType &BT, 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, const BugType &BT, + 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>(BT, Msg, N); if (Region) { R->markInteresting(Region); R->addVisitor<NullabilityBugVisitor>(Region); @@ -480,7 +476,7 @@ static bool checkInvariantViolation(ProgramStateRef State, ExplodedNode *N, } void NullabilityChecker::reportBugIfInvariantHolds( - StringRef Msg, ErrorKind Error, CheckKind CK, 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(); @@ -492,7 +488,7 @@ void NullabilityChecker::reportBugIfInvariantHolds( N = C.addTransition(OriginalState, N); } - reportBug(Msg, Error, CK, N, Region, C.getBugReporter(), ValueExpr); + reportBug(Msg, Error, BT, N, Region, C.getBugReporter(), ValueExpr); } /// Cleaning up the program state. @@ -546,19 +542,19 @@ void NullabilityChecker::checkEvent(ImplicitNullDerefEvent Event) const { if (!TrackedNullability) return; - if (ChecksEnabled[CK_NullableDereferenced] && + if (NullableDereferenced.isEnabled() && 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, NullableDereferenced, Event.SinkNode, Region, BR); else { reportBug("Nullable pointer is passed to a callee that requires a " "non-null", - ErrorKind::NullablePassedToNonnull, CK_NullableDereferenced, + ErrorKind::NullablePassedToNonnull, NullableDereferenced, Event.SinkNode, Region, BR); } } @@ -710,29 +706,28 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, Nullability RetExprTypeLevelNullability = getNullabilityAnnotation(lookThroughImplicitCasts(RetExpr)->getType()); - bool NullReturnedFromNonNull = (RequiredNullability == Nullability::Nonnull && - Nullness == NullConstraint::IsNull); - if (ChecksEnabled[CK_NullReturnedFromNonnull] && NullReturnedFromNonNull && - RetExprTypeLevelNullability != Nullability::Nonnull && - !InSuppressedMethodFamily) { - ExplodedNode *N = C.generateErrorNode(State); - if (!N) - return; + if (RequiredNullability == Nullability::Nonnull && + Nullness == NullConstraint::IsNull) { + if (NullReturnedFromNonnull.isEnabled() && + RetExprTypeLevelNullability != Nullability::Nonnull && + !InSuppressedMethodFamily) { + ExplodedNode *N = C.generateErrorNode(State); + if (!N) + return; - SmallString<256> SBuf; - llvm::raw_svector_ostream OS(SBuf); - OS << (RetExpr->getType()->isObjCObjectPointerType() ? "nil" : "Null"); - 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, - RetExpr); - return; - } + SmallString<256> SBuf; + llvm::raw_svector_ostream OS(SBuf); + OS << (RetExpr->getType()->isObjCObjectPointerType() ? "nil" : "Null"); + OS << " returned from a " << C.getDeclDescription(D) + << " that is expected to return a non-null value"; + reportBugIfInvariantHolds(OS.str(), ErrorKind::NilReturnedToNonnull, + NullReturnedFromNonnull, N, nullptr, C, + RetExpr); + return; + } - // If null was returned from a non-null function, mark the nullability - // invariant as violated even if the diagnostic was suppressed. - if (NullReturnedFromNonNull) { + // If null was returned from a non-null function, mark the nullability + // invariant as violated even if the diagnostic was suppressed. State = State->set<InvariantViolated>(true); C.addTransition(State); return; @@ -746,7 +741,7 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, State->get<NullabilityMap>(Region); if (TrackedNullability) { Nullability TrackedNullabValue = TrackedNullability->getValue(); - if (ChecksEnabled[CK_NullableReturnedFromNonnull] && + if (NullableReturnedFromNonnull.isEnabled() && Nullness != NullConstraint::IsNotNull && TrackedNullabValue == Nullability::Nullable && RequiredNullability == Nullability::Nonnull) { @@ -758,7 +753,7 @@ 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); + NullableReturnedFromNonnull, N, Region, C); } return; } @@ -809,8 +804,7 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call, unsigned ParamIdx = Param->getFunctionScopeIndex() + 1; - if (ChecksEnabled[CK_NullPassedToNonnull] && - Nullness == NullConstraint::IsNull && + if (NullPassedToNonnull.isEnabled() && Nullness == NullConstraint::IsNull && ArgExprTypeLevelNullability != Nullability::Nonnull && RequiredNullability == Nullability::Nonnull && isDiagnosableCall(Call)) { @@ -824,7 +818,7 @@ 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, + NullPassedToNonnull, N, nullptr, C, ArgExpr, /*SuppressPath=*/false); return; } @@ -841,7 +835,7 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call, TrackedNullability->getValue() != Nullability::Nullable) continue; - if (ChecksEnabled[CK_NullablePassedToNonnull] && + if (NullablePassedToNonnull.isEnabled() && RequiredNullability == Nullability::Nonnull && isDiagnosableCall(Call)) { ExplodedNode *N = C.addTransition(State); @@ -850,17 +844,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, + NullablePassedToNonnull, N, Region, C, ArgExpr, /*SuppressPath=*/true); return; } - if (ChecksEnabled[CK_NullableDereferenced] && + if (NullableDereferenced.isEnabled() && Param->getType()->isReferenceType()) { ExplodedNode *N = C.addTransition(State); - reportBugIfInvariantHolds("Nullable pointer is dereferenced", - ErrorKind::NullableDereferenced, - CK_NullableDereferenced, N, Region, C, - ArgExpr, /*SuppressPath=*/true); + reportBugIfInvariantHolds( + "Nullable pointer is dereferenced", ErrorKind::NullableDereferenced, + NullableDereferenced, N, Region, C, ArgExpr, /*SuppressPath=*/true); return; } continue; @@ -1294,7 +1287,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 (NullPassedToNonnull.isEnabled() && NullAssignedToNonNull && ValNullability != Nullability::Nonnull && ValueExprTypeLevelNullability != Nullability::Nonnull && !isARCNilInitializedLocal(C, S)) { @@ -1312,7 +1305,7 @@ 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); + NullPassedToNonnull, N, nullptr, C, ValueStmt); return; } @@ -1338,13 +1331,13 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S, if (RhsNullness == NullConstraint::IsNotNull || TrackedNullability->getValue() != Nullability::Nullable) return; - if (ChecksEnabled[CK_NullablePassedToNonnull] && + if (NullablePassedToNonnull.isEnabled() && LocNullability == Nullability::Nonnull) { ExplodedNode *N = C.addTransition(State, C.getPredecessor()); reportBugIfInvariantHolds("Nullable pointer is assigned to a pointer " "which is expected to have non-null value", ErrorKind::NullableAssignedToNonnull, - CK_NullablePassedToNonnull, N, ValueRegion, C); + NullablePassedToNonnull, N, ValueRegion, C); } return; } @@ -1391,28 +1384,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); \ +// The checker group "nullability" (which consists of the checkers that are +// implemented in this file) has a group-level configuration option which +// affects all the checkers in the group. As this is a completely unique +// remnant of old design (this is the only group option in the analyzer), there +// is no machinery to inject the group name from `Checkers.td`, so it is simply +// hardcoded here: +constexpr llvm::StringLiteral GroupName = "nullability"; +constexpr llvm::StringLiteral GroupOptName = "NoDiagnoseCallsToSystemHeaders"; + +#define REGISTER_CHECKER(NAME, TRACKING_REQUIRED) \ + void ento::register##NAME##Checker(CheckerManager &Mgr) { \ + NullabilityChecker *Chk = Mgr.getChecker<NullabilityChecker>(); \ + Chk->NAME.enable(Mgr); \ + Chk->NeedTracking = Chk->NeedTracking || TRACKING_REQUIRED; \ + Chk->NoDiagnoseCallsToSystemHeaders = \ + Mgr.getAnalyzerOptions().getCheckerBooleanOption(GroupName, \ + GroupOptName, true); \ } \ \ - bool ento::shouldRegister##name##Checker(const CheckerManager &mgr) { \ + bool ento::shouldRegister##NAME##Checker(const CheckerManager &) { \ return true; \ } diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c index 66b9be9795f12..78ee00deea18d 100644 --- a/clang/test/Analysis/analyzer-enabled-checkers.c +++ b/clang/test/Analysis/analyzer-enabled-checkers.c @@ -34,7 +34,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 8c6078a49c231..7f9c9ff4c9fd7 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 @@ -42,7 +42,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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
