[clang] [clang][analyzer] Add checker 'alpha.core.FixedAddressDereference' (PR #127191)
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 14 02:50:55 PST 2025
https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/127191
>From 1f2ad6d5ce6f11fb031ec2175527f56ea86761ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Mon, 3 Feb 2025 15:35:31 +0100
Subject: [PATCH 1/2] [clang][analyzer] Add checker
'alpha.core.FixedAddressDereference'
---
clang/docs/analyzer/checkers.rst | 38 ++++
.../clang/StaticAnalyzer/Checkers/Checkers.td | 26 +--
.../Checkers/DereferenceChecker.cpp | 63 ++++++-
clang/test/Analysis/concrete-address.c | 162 +++++++++++++++++-
clang/test/Analysis/misc-ps.m | 4 +-
5 files changed, 271 insertions(+), 22 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 707067358fdfe..c5e10f5a8bbf2 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -129,6 +129,8 @@ The ``SuppressAddressSpaces`` option suppresses
warnings for null dereferences of all pointers with address spaces. You can
disable this behavior with the option
``-analyzer-config core.NullDereference:SuppressAddressSpaces=false``.
+Value of this option is used for checker :ref:`_core-FixedAddressDereference`
+too.
*Defaults to true*.
.. code-block:: objc
@@ -2919,6 +2921,42 @@ Check for assignment of a fixed address to a pointer.
p = (int *) 0x10000; // warn
}
+.. _alpha-core-FixedAddressDereference:
+
+alpha.core.FixedAddressDereference (C, C++, ObjC)
+"""""""""""""""""""""""""""""""""""""""""""""""""
+Check for dereferences of fixed values used as pointers.
+
+Similarly as at :ref:`_core-NullDereference`, the checker specifically does
+not report dereferences for x86 and x86-64 targets when the
+address space is 256 (x86 GS Segment), 257 (x86 FS Segment), or 258 (x86 SS
+segment). (See `X86/X86-64 Language Extensions
+<https://clang.llvm.org/docs/LanguageExtensions.html#memory-references-to-specified-segments>`__
+for reference.)
+
+If you want to disable this behavior, set the ``SuppressAddressSpaces`` option
+of checker ``core.NullDereference`` to false, like
+``-analyzer-config core.NullDereference:SuppressAddressSpaces=false``. The value
+of this option is used for both checkers.
+
+.. code-block:: c
+
+ void test1() {
+ int *p = (int *)0x020;
+ int x = p[0]; // warn
+ }
+
+ void test2(int *p) {
+ if (p == (int *)-1)
+ *p = 0; // warn
+ }
+
+ void test3() {
+ int (*p_function)(char, char);
+ p_function = (int (*)(char, char))0x04080;
+ int x = (*p_function)('x', 'y'); // NO warning yet at functon pointer calls
+ }
+
.. _alpha-core-PointerArithm:
alpha.core.PointerArithm (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 9bf491eac1e0e..44ca28c003b06 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -211,17 +211,15 @@ def DereferenceModeling : Checker<"DereferenceModeling">,
Documentation<NotDocumented>,
Hidden;
-def NullDereferenceChecker : Checker<"NullDereference">,
- HelpText<"Check for dereferences of null pointers">,
- CheckerOptions<[
- CmdLineOption<Boolean,
- "SuppressAddressSpaces",
- "Suppresses warning when pointer dereferences an address space",
- "true",
- Released>
- ]>,
- Documentation<HasDocumentation>,
- Dependencies<[DereferenceModeling]>;
+def NullDereferenceChecker
+ : Checker<"NullDereference">,
+ HelpText<"Check for dereferences of null pointers">,
+ CheckerOptions<[CmdLineOption<
+ Boolean, "SuppressAddressSpaces",
+ "Suppresses warning when pointer dereferences an address space",
+ "true", Released>]>,
+ Documentation<HasDocumentation>,
+ Dependencies<[DereferenceModeling]>;
def NonNullParamChecker : Checker<"NonNullParamChecker">,
HelpText<"Check for null pointers passed as arguments to a function whose "
@@ -285,6 +283,12 @@ def FixedAddressChecker : Checker<"FixedAddr">,
HelpText<"Check for assignment of a fixed address to a pointer">,
Documentation<HasDocumentation>;
+def FixedAddressDereferenceChecker
+ : Checker<"FixedAddressDereference">,
+ HelpText<"Check for dereferences of fixed values used as pointers">,
+ Documentation<HasDocumentation>,
+ Dependencies<[DereferenceModeling]>;
+
def PointerArithChecker : Checker<"PointerArithm">,
HelpText<"Check for pointer arithmetic on locations other than array "
"elements">,
diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index e9e2771c739b6..6a3f70e62e77b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -31,7 +31,12 @@ class DereferenceChecker
: public Checker< check::Location,
check::Bind,
EventDispatcher<ImplicitNullDerefEvent> > {
- enum DerefKind { NullPointer, UndefinedPointerValue, AddressOfLabel };
+ enum DerefKind {
+ NullPointer,
+ UndefinedPointerValue,
+ AddressOfLabel,
+ FixedAddress
+ };
void reportBug(DerefKind K, ProgramStateRef State, const Stmt *S,
CheckerContext &C) const;
@@ -52,10 +57,12 @@ class DereferenceChecker
bool SuppressAddressSpaces = 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;
};
} // end anonymous namespace
@@ -155,30 +162,47 @@ 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:
+ 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();
+ 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) {
+ 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.
@@ -289,6 +313,13 @@ 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);
+ return;
+ }
+
// From this point forward, we know that the location is not null.
C.addTransition(notNullState);
}
@@ -337,6 +368,13 @@ 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);
+ return;
+ }
+
// Unlike a regular null dereference, initializing a reference with a
// dereferenced null pointer does not actually cause a runtime exception in
// Clang's implementation of references.
@@ -383,3 +421,16 @@ void ento::registerNullDereferenceChecker(CheckerManager &Mgr) {
bool ento::shouldRegisterNullDereferenceChecker(const CheckerManager &) {
return true;
}
+
+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));
+}
+
+bool ento::shouldRegisterFixedAddressDereferenceChecker(
+ const CheckerManager &) {
+ return true;
+}
diff --git a/clang/test/Analysis/concrete-address.c b/clang/test/Analysis/concrete-address.c
index 346f5093e44f7..af99aa43a98a8 100644
--- a/clang/test/Analysis/concrete-address.c
+++ b/clang/test/Analysis/concrete-address.c
@@ -1,7 +1,163 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -verify %s
-// expected-no-diagnostics
-void foo(void) {
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+ unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+
+#define assert(expr) \
+ ((expr) ? (void)(0) : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+typedef unsigned long uintptr_t;
+
+void f0(void) {
int *p = (int*) 0x10000; // Should not crash here.
- *p = 3;
+ *p = 3; // expected-warning{{Dereference of a fixed address}}
+}
+
+void f1(int *p) {
+ if (p != (int *)-1)
+ *p = 1;
+ else
+ *p = 0; // expected-warning{{Dereference of a fixed address}}
+}
+
+struct f2_struct {
+ int x;
+};
+
+int f2(struct f2_struct* p) {
+
+ if (p != (struct f2_struct *)1)
+ p->x = 1;
+
+ return p->x++; // expected-warning{{Access to field 'x' results in a dereference of a fixed address (loaded from variable 'p')}}
+}
+
+int f3_1(char* x) {
+ int i = 2;
+
+ if (x != (char *)1)
+ return x[i - 1];
+
+ return x[i+1]; // expected-warning{{Array access (from variable 'x') results in a dereference of a fixed address}}
+}
+
+int f3_2(char* x) {
+ int i = 2;
+
+ if (x != (char *)1)
+ return x[i - 1];
+
+ return x[i+1]++; // expected-warning{{Array access (from variable 'x') results in a dereference of a fixed address}}
+}
+
+int f4_1(int *p) {
+ uintptr_t x = (uintptr_t) p;
+
+ if (x != (uintptr_t)1)
+ return 1;
+
+ int *q = (int*) x;
+ return *q; // expected-warning{{Dereference of a fixed address (loaded from variable 'q')}}
+}
+
+int f4_2(void) {
+ short array[2];
+ uintptr_t x = (uintptr_t)array;
+ short *p = (short *)x;
+
+ // The following branch should be infeasible.
+ if (!(p == &array[0])) {
+ p = (short *)1;
+ *p = 1; // no-warning
+ }
+
+ if (p != (short *)1) {
+ *p = 5; // no-warning
+ p = (short *)1; // expected-warning {{Using a fixed address is not portable}}
+ }
+ else return 1;
+
+ *p += 10; // expected-warning{{Dereference of a fixed}}
+ return 0;
+}
+
+int f5(void) {
+ char *s = "hello world";
+ return s[0]; // no-warning
+}
+
+void f6(int *p, int *q) {
+ if (p != (int *)1)
+ if (p == (int *)1)
+ *p = 1; // no-warning
+
+ if (q == (int *)1)
+ if (q != (int *)1)
+ *q = 1; // no-warning
+}
+
+int* qux(int);
+
+int f7_1(unsigned len) {
+ assert (len != 0);
+ int *p = (int *)1;
+ unsigned i;
+
+ for (i = 0; i < len; ++i)
+ p = qux(i);
+
+ return *p++; // no-warning
+}
+
+int f7_2(unsigned len) {
+ assert (len > 0); // note use of '>'
+ int *p = (int *)1;
+ unsigned i;
+
+ for (i = 0; i < len; ++i)
+ p = qux(i);
+
+ return *p++; // no-warning
+}
+
+struct f8_s {
+ int x;
+ int y[2];
+};
+
+void f8(struct f8_s *s, int coin) {
+ if (s != (struct f8_s *)7)
+ return;
+
+ if (coin)
+ s->x = 5; // expected-warning{{Access to field 'x' results in a dereference of a fixed address (loaded from variable 's')}}
+ else
+ s->y[1] = 6; // expected-warning{{Array access (via field 'y') results in a dereference of a fixed address}}
+}
+
+void f9() {
+ int (*p_function) (char, char) = (int (*)(char, char))0x04040; // FIXME: warn at this initialization
+ p_function = (int (*)(char, char))0x04080; // expected-warning {{Using a fixed address is not portable}}
+ // FIXME: there should be a warning from calling the function pointer with fixed address
+ int x = (*p_function) ('x', 'y');
+}
+
+#define AS_ATTRIBUTE volatile __attribute__((address_space(256)))
+#define _get_base() ((void * AS_ATTRIBUTE *)0x10)
+
+void* test_address_space_array(unsigned long slot) {
+ return _get_base()[slot]; // no-warning
+}
+void test_address_space_condition(int AS_ATTRIBUTE *cpu_data) {
+ if (cpu_data == (int *)0x10) {
+ *cpu_data = 3; // no-warning
+ }
+}
+struct X { int member; };
+int test_address_space_member(void) {
+ struct X AS_ATTRIBUTE *data = (struct X AS_ATTRIBUTE *)0x10UL;
+ int ret;
+ ret = data->member; // no-warning
+ return ret;
}
diff --git a/clang/test/Analysis/misc-ps.m b/clang/test/Analysis/misc-ps.m
index 0a8a30cb6175c..841f618fbdfab 100644
--- a/clang/test/Analysis/misc-ps.m
+++ b/clang/test/Analysis/misc-ps.m
@@ -1,6 +1,6 @@
// NOTE: Use '-fobjc-gc' to test the analysis being run twice, and multiple reports are not issued.
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -analyzer-disable-checker=alpha.core.FixedAddressDereference -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -analyzer-disable-checker=alpha.core.FixedAddressDereference -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
#ifndef __clang_analyzer__
#error __clang_analyzer__ not defined
>From d60d9c7f64ba501f80ee24db7afa9be9beec2cf6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Fri, 14 Feb 2025 11:50:27 +0100
Subject: [PATCH 2/2] fix link in documentation
---
clang/docs/analyzer/checkers.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index c5e10f5a8bbf2..6480c423bdce7 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -129,8 +129,8 @@ The ``SuppressAddressSpaces`` option suppresses
warnings for null dereferences of all pointers with address spaces. You can
disable this behavior with the option
``-analyzer-config core.NullDereference:SuppressAddressSpaces=false``.
-Value of this option is used for checker :ref:`_core-FixedAddressDereference`
-too.
+Value of this option is used for checker
+:ref:`_alpha-core-FixedAddressDereference` too.
*Defaults to true*.
.. code-block:: objc
More information about the cfe-commits
mailing list