From 135839a993f1cad32d65d21c6aeaef6f074a1997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= Date: Wed, 8 Jan 2025 17:22:10 +0100 Subject: [PATCH 1/3] [clang][analyzer] Split NullDereferenceChecker into a modeling and checker part. The checker currently reports beneath the null dereference dereferences of undefined value and of label addresses. If we want to add more kinds of invalid dereferences (or split the existing functionality) it is more useful to make it separate checkers. To make this possible the existing checker is split into a DereferenceModeling part and a NullDereference checker that actually only switches on the check of null dereference. This is similar architecture as in MallocChecker and CStringChecker. The change is almost NFC but a new (modeling) checker is added. If the NullDereference checker is turned off the found invalid dereferences will still stop the analysis without emitted warning (this is different compared to the old behavior). --- .../clang/StaticAnalyzer/Checkers/Checkers.td | 10 ++++- .../Checkers/DereferenceChecker.cpp | 45 +++++++++++++------ .../test/Analysis/analyzer-enabled-checkers.c | 1 + ...c-library-functions-arg-enabled-checkers.c | 1 + 4 files changed, 41 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index b34e940682fc5..fa2af57f1a09b 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -206,7 +206,12 @@ def CallAndMessageChecker : Checker<"CallAndMessage">, Documentation, Dependencies<[CallAndMessageModeling]>; -def DereferenceChecker : Checker<"NullDereference">, +def DereferenceModeling : Checker<"DereferenceModeling">, + HelpText<"General support for dereference related checkers">, + Documentation, + Hidden; + +def NullDereferenceChecker : Checker<"NullDereference">, HelpText<"Check for dereferences of null pointers">, CheckerOptions<[ CmdLineOption, "true", Released> ]>, - Documentation; + Documentation, + Dependencies<[DereferenceModeling]>; 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 0355eede75eae..dbcfe1f9b06fb 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -33,12 +33,6 @@ class DereferenceChecker EventDispatcher > { enum DerefKind { NullPointer, UndefinedPointerValue, AddressOfLabel }; - BugType BT_Null{this, "Dereference of null pointer", categories::LogicError}; - BugType BT_Undef{this, "Dereference of undefined pointer value", - categories::LogicError}; - BugType BT_Label{this, "Dereference of the address of a label", - categories::LogicError}; - void reportBug(DerefKind K, ProgramStateRef State, const Stmt *S, CheckerContext &C) const; @@ -56,6 +50,12 @@ class DereferenceChecker bool loadedFrom = false); bool SuppressAddressSpaces = false; + + bool CheckNullDereference = false; + + std::unique_ptr BT_Null; + std::unique_ptr BT_Undef; + std::unique_ptr BT_Label; }; } // end anonymous namespace @@ -155,22 +155,27 @@ static bool isDeclRefExprToReference(const Expr *E) { void DereferenceChecker::reportBug(DerefKind K, ProgramStateRef State, const Stmt *S, CheckerContext &C) const { + if (!CheckNullDereference) { + C.addSink(); + return; + } + const BugType *BT = nullptr; llvm::StringRef DerefStr1; llvm::StringRef DerefStr2; switch (K) { case DerefKind::NullPointer: - BT = &BT_Null; + BT = BT_Null.get(); DerefStr1 = " results in a null pointer dereference"; DerefStr2 = " results in a dereference of a null pointer"; break; case DerefKind::UndefinedPointerValue: - BT = &BT_Undef; + 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: - BT = &BT_Label; + BT = BT_Label.get(); DerefStr1 = " results in an undefined pointer dereference"; DerefStr2 = " results in a dereference of an address of a label"; break; @@ -351,12 +356,24 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, C.addTransition(State, this); } -void ento::registerDereferenceChecker(CheckerManager &mgr) { - auto *Chk = mgr.registerChecker(); - Chk->SuppressAddressSpaces = mgr.getAnalyzerOptions().getCheckerBooleanOption( - mgr.getCurrentCheckerName(), "SuppressAddressSpaces"); +void ento::registerDereferenceModeling(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterDereferenceModeling(const CheckerManager &Mgr) { + return true; +} + +void ento::registerNullDereferenceChecker(CheckerManager &Mgr) { + auto *Chk = Mgr.getChecker(); + Chk->CheckNullDereference = true; + Chk->SuppressAddressSpaces = Mgr.getAnalyzerOptions().getCheckerBooleanOption( + Mgr.getCurrentCheckerName(), "SuppressAddressSpaces"); + 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)); } -bool ento::shouldRegisterDereferenceChecker(const CheckerManager &mgr) { +bool ento::shouldRegisterNullDereferenceChecker(const CheckerManager &Mgr) { return true; } diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c index 160e35c77462d..c70aeb21ab045 100644 --- a/clang/test/Analysis/analyzer-enabled-checkers.c +++ b/clang/test/Analysis/analyzer-enabled-checkers.c @@ -14,6 +14,7 @@ // 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.NonNullParamChecker 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 0910f030b0f07..faf0a8f19d919 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,6 +22,7 @@ // 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.NonNullParamChecker From 580b8647ee345539605abc5d8d967245b00c7a88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= Date: Wed, 8 Jan 2025 18:18:03 +0100 Subject: [PATCH 2/3] fixed formatting error --- .../StaticAnalyzer/Checkers/DereferenceChecker.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index dbcfe1f9b06fb..b7dad644c9cff 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -369,9 +369,15 @@ void ento::registerNullDereferenceChecker(CheckerManager &Mgr) { Chk->CheckNullDereference = true; Chk->SuppressAddressSpaces = Mgr.getAnalyzerOptions().getCheckerBooleanOption( Mgr.getCurrentCheckerName(), "SuppressAddressSpaces"); - 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)); + 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)); } bool ento::shouldRegisterNullDereferenceChecker(const CheckerManager &Mgr) { From a8053dfdb38e4208c41d90e88010a20693f33556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= Date: Thu, 9 Jan 2025 17:33:46 +0100 Subject: [PATCH 3/3] fixed unused variable and checker specification --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index fa2af57f1a09b..1361da46c3c81 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -208,7 +208,7 @@ def CallAndMessageChecker : Checker<"CallAndMessage">, def DereferenceModeling : Checker<"DereferenceModeling">, HelpText<"General support for dereference related checkers">, - Documentation, + Documentation, Hidden; def NullDereferenceChecker : Checker<"NullDereference">, diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index b7dad644c9cff..e9e2771c739b6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -360,7 +360,7 @@ void ento::registerDereferenceModeling(CheckerManager &Mgr) { Mgr.registerChecker(); } -bool ento::shouldRegisterDereferenceModeling(const CheckerManager &Mgr) { +bool ento::shouldRegisterDereferenceModeling(const CheckerManager &) { return true; } @@ -380,6 +380,6 @@ void ento::registerNullDereferenceChecker(CheckerManager &Mgr) { categories::LogicError)); } -bool ento::shouldRegisterNullDereferenceChecker(const CheckerManager &Mgr) { +bool ento::shouldRegisterNullDereferenceChecker(const CheckerManager &) { return true; }