https://github.com/NagyDonat updated
https://github.com/llvm/llvm-project/pull/150442
From 4403a6e50ba82d3d2cc4bb50f1cbc5ecf7eeee18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Thu, 24 Jul 2025 17:17:05 +0200
Subject: [PATCH 1/2] [analyzer] Conversion to CheckerFamily:
DereferenceChecker
This commit converts the class DereferenceChecker to the checker family
framework and simplifies some parts of the implementation.
This commit is almost NFC, the only technically "functional" change is
that it removes the hidden modeling checker `DereferenceModeling` which
was only relevant as an implementation detail of the old checker
registration procedure.
---
.../clang/StaticAnalyzer/Checkers/Checkers.td | 16 +-
.../Checkers/DereferenceChecker.cpp | 186 +++++++-----------
.../test/Analysis/analyzer-enabled-checkers.c | 1 -
...c-library-functions-arg-enabled-checkers.c | 1 -
4 files changed, 79 insertions(+), 125 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 38584c98fcc91..6225977e980cc 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -206,21 +206,15 @@ def CallAndMessageChecker : Checker<"CallAndMessage">,
Documentation<HasDocumentation>,
Dependencies<[CallAndMessageModeling]>;
-def DereferenceModeling : Checker<"DereferenceModeling">,
- HelpText<"General support for dereference related checkers">,
- Documentation<NotDocumented>,
- Hidden;
-
def FixedAddressDereferenceChecker
: Checker<"FixedAddressDereference">,
HelpText<"Check for dereferences of fixed addresses">,
- Documentation<HasDocumentation>,
- Dependencies<[DereferenceModeling]>;
+ Documentation<HasDocumentation>;
-def NullDereferenceChecker : Checker<"NullDereference">,
- HelpText<"Check for dereferences of null pointers">,
- Documentation<HasDocumentation>,
- Dependencies<[DereferenceModeling]>;
+def NullDereferenceChecker
+ : Checker<"NullDereference">,
+ HelpText<"Check for dereferences of null pointers">,
+ Documentation<HasDocumentation>;
def NonNullParamChecker : Checker<"NonNullParamChecker">,
HelpText<"Check for null pointers passed as arguments to a function whose "
diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index d7eea7e0b0db7..d572c4aba70a7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -25,18 +25,22 @@ using namespace clang;
using namespace ento;
namespace {
+
+class DerefBugType : public BugType {
+ StringRef ArrayMsg, FieldMsg;
+
+public:
+ DerefBugType(CheckerFrontend *FE, StringRef Desc, const char *RMsg,
+ const char *FMsg = nullptr)
+ : BugType(FE, Desc), ArrayMsg(RMsg), FieldMsg(FMsg ? FMsg : RMsg) {}
+ StringRef getArrayMsg() const { return ArrayMsg; }
+ StringRef getFieldMsg() const { return FieldMsg; }
+};
+
class DereferenceChecker
- : public Checker< check::Location,
- check::Bind,
- EventDispatcher<ImplicitNullDerefEvent> > {
- enum DerefKind {
- NullPointer,
- UndefinedPointerValue,
- AddressOfLabel,
- FixedAddress,
- };
-
- void reportBug(DerefKind K, ProgramStateRef State, const Stmt *S,
+ : public CheckerFamily<check::Location, check::Bind,
+ EventDispatcher<ImplicitNullDerefEvent>> {
+ void reportBug(const DerefBugType &BT, ProgramStateRef State, const Stmt *S,
CheckerContext &C) const;
bool suppressReport(CheckerContext &C, const Expr *E) const;
@@ -52,13 +56,23 @@ class DereferenceChecker
const LocationContext *LCtx,
bool loadedFrom = false);
- bool CheckNullDereference = false;
- bool CheckFixedDereference = false;
-
- std::unique_ptr<BugType> BT_Null;
- std::unique_ptr<BugType> BT_Undef;
- std::unique_ptr<BugType> BT_Label;
- std::unique_ptr<BugType> BT_FixedAddress;
+ CheckerFrontend NullDerefChecker, FixedDerefChecker;
+ const DerefBugType NullBug{&NullDerefChecker, "Dereference of null pointer",
+ "a null pointer dereference",
+ "a dereference of a null pointer"};
+ const DerefBugType UndefBug{&NullDerefChecker,
+ "Dereference of undefined pointer value",
+ "an undefined pointer dereference",
+ "a dereference of an undefined pointer value"};
+ const DerefBugType LabelBug{&NullDerefChecker,
+ "Dereference of the address of a label",
+ "an undefined pointer dereference",
+ "a dereference of an address of a label"};
+ const DerefBugType FixedAddressBug{&FixedDerefChecker,
+ "Dereference of a fixed address",
+ "a dereference of a fixed address"};
+
+ StringRef getDebugTag() const override { return "DereferenceChecker"; }
};
} // end anonymous namespace
@@ -158,115 +172,85 @@ static bool isDeclRefExprToReference(const Expr *E) {
return false;
}
-void DereferenceChecker::reportBug(DerefKind K, ProgramStateRef State,
- const Stmt *S, CheckerContext &C) const {
- const BugType *BT = nullptr;
- llvm::StringRef DerefStr1;
- llvm::StringRef DerefStr2;
- switch (K) {
- case DerefKind::NullPointer:
- if (!CheckNullDereference) {
- C.addSink();
- return;
- }
- BT = BT_Null.get();
- DerefStr1 = " results in a null pointer dereference";
- DerefStr2 = " results in a dereference of a null pointer";
- break;
- case DerefKind::UndefinedPointerValue:
- if (!CheckNullDereference) {
- C.addSink();
+void DereferenceChecker::reportBug(const DerefBugType &BT,
+ ProgramStateRef State, const Stmt *S,
+ CheckerContext &C) const {
+ if (&BT == &FixedAddressBug) {
+ if (!FixedDerefChecker.isEnabled())
return;
- }
- BT = BT_Undef.get();
- DerefStr1 = " results in an undefined pointer dereference";
- DerefStr2 = " results in a dereference of an undefined pointer value";
- break;
- case DerefKind::AddressOfLabel:
- if (!CheckNullDereference) {
+ } else {
+ if (!NullDerefChecker.isEnabled()) {
C.addSink();
return;
}
- BT = BT_Label.get();
- DerefStr1 = " results in an undefined pointer dereference";
- DerefStr2 = " results in a dereference of an address of a label";
- break;
- case DerefKind::FixedAddress:
- // Deliberately don't add a sink node if check is disabled.
- // This situation may be valid in special cases.
- if (!CheckFixedDereference)
- return;
-
- BT = BT_FixedAddress.get();
- DerefStr1 = " results in a dereference of a fixed address";
- DerefStr2 = " results in a dereference of a fixed address";
- break;
- };
+ }
// Generate an error node.
ExplodedNode *N = C.generateErrorNode(State);
if (!N)
return;
- SmallString<100> buf;
- llvm::raw_svector_ostream os(buf);
+ SmallString<100> Buf;
+ llvm::raw_svector_ostream Out(Buf);
SmallVector<SourceRange, 2> Ranges;
switch (S->getStmtClass()) {
case Stmt::ArraySubscriptExprClass: {
- os << "Array access";
+ Out << "Array access";
const ArraySubscriptExpr *AE = cast<ArraySubscriptExpr>(S);
- AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
- State.get(), N->getLocationContext());
- os << DerefStr1;
+ AddDerefSource(Out, Ranges, AE->getBase()->IgnoreParenCasts(), State.get(),
+ N->getLocationContext());
+ Out << " results in " << BT.getArrayMsg();
break;
}
case Stmt::ArraySectionExprClass: {
- os << "Array access";
+ Out << "Array access";
const ArraySectionExpr *AE = cast<ArraySectionExpr>(S);
- AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
- State.get(), N->getLocationContext());
- os << DerefStr1;
+ AddDerefSource(Out, Ranges, AE->getBase()->IgnoreParenCasts(), State.get(),
+ N->getLocationContext());
+ Out << " results in " << BT.getArrayMsg();
break;
}
case Stmt::UnaryOperatorClass: {
- os << BT->getDescription();
+ Out << BT.getDescription();
const UnaryOperator *U = cast<UnaryOperator>(S);
- AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(),
- State.get(), N->getLocationContext(), true);
+ AddDerefSource(Out, Ranges, U->getSubExpr()->IgnoreParens(), State.get(),
+ N->getLocationContext(), true);
break;
}
case Stmt::MemberExprClass: {
const MemberExpr *M = cast<MemberExpr>(S);
if (M->isArrow() || isDeclRefExprToReference(M->getBase())) {
- os << "Access to field '" << M->getMemberNameInfo() << "'" << DerefStr2;
- AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(),
- State.get(), N->getLocationContext(), true);
+ Out << "Access to field '" << M->getMemberNameInfo() << "' results in "
+ << BT.getFieldMsg();
+ AddDerefSource(Out, Ranges, M->getBase()->IgnoreParenCasts(),
State.get(),
+ N->getLocationContext(), true);
}
break;
}
case Stmt::ObjCIvarRefExprClass: {
const ObjCIvarRefExpr *IV = cast<ObjCIvarRefExpr>(S);
- os << "Access to instance variable '" << *IV->getDecl() << "'" <<
DerefStr2;
- AddDerefSource(os, Ranges, IV->getBase()->IgnoreParenCasts(),
- State.get(), N->getLocationContext(), true);
+ Out << "Access to instance variable '" << *IV->getDecl() << "' results in "
+ << BT.getFieldMsg();
+ AddDerefSource(Out, Ranges, IV->getBase()->IgnoreParenCasts(), State.get(),
+ N->getLocationContext(), true);
break;
}
default:
break;
}
- auto report = std::make_unique<PathSensitiveBugReport>(
- *BT, buf.empty() ? BT->getDescription() : buf.str(), N);
+ auto BR = std::make_unique<PathSensitiveBugReport>(
+ BT, Buf.empty() ? BT.getDescription() : Buf.str(), N);
- bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
+ bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *BR);
for (SmallVectorImpl<SourceRange>::iterator
I = Ranges.begin(), E = Ranges.end(); I!=E; ++I)
- report->addRange(*I);
+ BR->addRange(*I);
- C.emitReport(std::move(report));
+ C.emitReport(std::move(BR));
}
void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
@@ -275,7 +259,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad,
const Stmt* S,
if (l.isUndef()) {
const Expr *DerefExpr = getDereferenceExpr(S);
if (!suppressReport(C, DerefExpr))
- reportBug(DerefKind::UndefinedPointerValue, C.getState(), DerefExpr, C);
+ reportBug(UndefBug, C.getState(), DerefExpr, C);
return;
}
@@ -296,7 +280,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad,
const Stmt* S,
// we call an "explicit" null dereference.
const Expr *expr = getDereferenceExpr(S);
if (!suppressReport(C, expr)) {
- reportBug(DerefKind::NullPointer, nullState, expr, C);
+ reportBug(NullBug, nullState, expr, C);
return;
}
}
@@ -314,7 +298,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad,
const Stmt* S,
if (location.isConstant()) {
const Expr *DerefExpr = getDereferenceExpr(S, isLoad);
if (!suppressReport(C, DerefExpr))
- reportBug(DerefKind::FixedAddress, notNullState, DerefExpr, C);
+ reportBug(FixedAddressBug, notNullState, DerefExpr, C);
return;
}
@@ -330,7 +314,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const
Stmt *S,
// One should never write to label addresses.
if (auto Label = L.getAs<loc::GotoLabel>()) {
- reportBug(DerefKind::AddressOfLabel, C.getState(), S, C);
+ reportBug(LabelBug, C.getState(), S, C);
return;
}
@@ -351,7 +335,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const
Stmt *S,
if (!StNonNull) {
const Expr *expr = getDereferenceExpr(S, /*IsBind=*/true);
if (!suppressReport(C, expr)) {
- reportBug(DerefKind::NullPointer, StNull, expr, C);
+ reportBug(NullBug, StNull, expr, C);
return;
}
}
@@ -369,7 +353,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const
Stmt *S,
if (V.isConstant()) {
const Expr *DerefExpr = getDereferenceExpr(S, true);
if (!suppressReport(C, DerefExpr))
- reportBug(DerefKind::FixedAddress, State, DerefExpr, C);
+ reportBug(FixedAddressBug, State, DerefExpr, C);
return;
}
@@ -392,26 +376,8 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const
Stmt *S,
C.addTransition(State, this);
}
-void ento::registerDereferenceModeling(CheckerManager &Mgr) {
- Mgr.registerChecker<DereferenceChecker>();
-}
-
-bool ento::shouldRegisterDereferenceModeling(const CheckerManager &) {
- return true;
-}
-
void ento::registerNullDereferenceChecker(CheckerManager &Mgr) {
- auto *Chk = Mgr.getChecker<DereferenceChecker>();
- Chk->CheckNullDereference = true;
- Chk->BT_Null.reset(new BugType(Mgr.getCurrentCheckerName(),
- "Dereference of null pointer",
- categories::LogicError));
- Chk->BT_Undef.reset(new BugType(Mgr.getCurrentCheckerName(),
- "Dereference of undefined pointer value",
- categories::LogicError));
- Chk->BT_Label.reset(new BugType(Mgr.getCurrentCheckerName(),
- "Dereference of the address of a label",
- categories::LogicError));
+ Mgr.getChecker<DereferenceChecker>()->NullDerefChecker.enable(Mgr);
}
bool ento::shouldRegisterNullDereferenceChecker(const CheckerManager &) {
@@ -419,11 +385,7 @@ bool ento::shouldRegisterNullDereferenceChecker(const
CheckerManager &) {
}
void ento::registerFixedAddressDereferenceChecker(CheckerManager &Mgr) {
- auto *Chk = Mgr.getChecker<DereferenceChecker>();
- Chk->CheckFixedDereference = true;
- Chk->BT_FixedAddress.reset(new BugType(Mgr.getCurrentCheckerName(),
- "Dereference of a fixed address",
- categories::LogicError));
+ Mgr.getChecker<DereferenceChecker>()->FixedDerefChecker.enable(Mgr);
}
bool ento::shouldRegisterFixedAddressDereferenceChecker(
diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c
b/clang/test/Analysis/analyzer-enabled-checkers.c
index 78ee00deea18d..a632b70fa843a 100644
--- a/clang/test/Analysis/analyzer-enabled-checkers.c
+++ b/clang/test/Analysis/analyzer-enabled-checkers.c
@@ -14,7 +14,6 @@
// CHECK-NEXT: core.BitwiseShift
// CHECK-NEXT: core.CallAndMessageModeling
// CHECK-NEXT: core.CallAndMessage
-// CHECK-NEXT: core.DereferenceModeling
// CHECK-NEXT: core.DivideZero
// CHECK-NEXT: core.DynamicTypePropagation
// CHECK-NEXT: core.FixedAddressDereference
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 7f9c9ff4c9fd7..b388c312ba957 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
@@ -22,7 +22,6 @@
// CHECK-NEXT: core.BitwiseShift
// CHECK-NEXT: core.CallAndMessageModeling
// CHECK-NEXT: core.CallAndMessage
-// CHECK-NEXT: core.DereferenceModeling
// CHECK-NEXT: core.DivideZero
// CHECK-NEXT: core.DynamicTypePropagation
// CHECK-NEXT: core.FixedAddressDereference
From 343cd4762777f7db970a061bdea98f7aaed85cfb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Fri, 25 Jul 2025 11:00:17 +0200
Subject: [PATCH 2/2] Preserve comment that was "lost" in reorganization
---
clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index d572c4aba70a7..51cf318206589 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -177,6 +177,8 @@ void DereferenceChecker::reportBug(const DerefBugType &BT,
CheckerContext &C) const {
if (&BT == &FixedAddressBug) {
if (!FixedDerefChecker.isEnabled())
+ // Deliberately don't add a sink node if check is disabled.
+ // This situation may be valid in special cases.
return;
} else {
if (!NullDerefChecker.isEnabled()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
