-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang][analyzer] Add checker 'alpha.core.FixedAddressDereference' #127191
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-clang Author: Balázs Kéri (balazske) ChangesFull diff: https://github.com/llvm/llvm-project/pull/127191.diff 5 Files Affected:
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
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Balázs Kéri (balazske) ChangesFull diff: https://github.com/llvm/llvm-project/pull/127191.diff 5 Files Affected:
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
|
The checker is alpha because there are known problems with it which I plan to fix later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had the impression that the core dereference checker already checks this.
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part of the doc feels less relevant than an example. I'd rather move this after the examples, as this fine-tunes the behavior.
It may cause confusion that "NullDereference" checker checks not only null dereference but undefined pointer and label address too. Probably these checks (specially label address) can be moved into this checker. (Or add the new check to NullDereference without a new checker?) |
I see. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good. Thank you!
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Value of this option is used for checker | |
Value of this option is also used for checker |
It's really weird to see that the option of a different checker influences some other checker.
That that point maybe the checker option should be hoisted into an analyzer config option instead.
WDYT?
|
||
alpha.core.FixedAddressDereference (C, C++, ObjC) | ||
""""""""""""""""""""""""""""""""""""""""""""""""" | ||
Check for dereferences of fixed values used as pointers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check for dereferences of fixed values used as pointers. | |
Check for dereferences of fixed addresses. |
""""""""""""""""""""""""""""""""""""""""""""""""" | ||
Check for dereferences of fixed values used as pointers. | ||
|
||
Similarly as at :ref:`_core-NullDereference`, the checker specifically does |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly as at :ref:`_core-NullDereference`, the checker specifically does | |
Similarly to :ref:`_core-NullDereference`, the checker intentionally does |
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
segment). (See `X86/X86-64 Language Extensions | |
Segment). (See `X86/X86-64 Language Extensions |
All the previous "segment" words were capitalized.
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]>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hunk didn't change semantic behavior. I'd prefer keeping as it was before.
@@ -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">, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HelpText<"Check for dereferences of fixed values used as pointers">, | |
HelpText<"Check for dereferences of fixed addresses">, |
NullPointer, | ||
UndefinedPointerValue, | ||
AddressOfLabel, | ||
FixedAddress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FixedAddress | |
FixedAddress, |
Let's have a trailing comma.
// Deliberately don't add a sink node if check is disabled. | ||
// This situation may be valid in special cases. | ||
if (!CheckFixedDereference) | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In those cases wouldn't we get UndefinedVal for the loaded value?
That usually anyways halt the analysis pretty early. Would it be better to still add a sink here in that case?
int x = (*p_function) ('x', 'y'); | ||
} | ||
|
||
#define AS_ATTRIBUTE volatile __attribute__((address_space(256))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use this address space attribute yet we didn't pin the target to x86.
// 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to disable this checker explicitly?
No description provided.