Re: [PATCH v1 1/2] cpukit: Fixed 32bit pointers
On Thu, Mar 18, 2021 at 9:18 AM Stephen Clark
wrote:
> Using 32bit types like uint32_t for pointers creates issues on 64 bit
> architectures like AArch64. Replaced occurrences of these with uintptr_t,
> which will work for both 32 and 64 bit architectures.
> ---
> cpukit/libdebugger/rtems-debugger-server.c | 4 ++--
> cpukit/libdebugger/rtems-debugger-target.c | 2 +-
> cpukit/libdebugger/rtems-debugger-target.h | 2 +-
> cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c | 4 ++--
> cpukit/libmisc/stackchk/check.c| 3 ++-
> 5 files changed, 8 insertions(+), 7 deletions(-)
>
This is the same basic issue but it is in three completely different areas.
This patch should be split into a series for rtems-debugger, rtems-fdt,
and stackchk.
That should make most of it OK. Using uint32_t to hold pointers
is wrong but has been a valid assumption for almost every architecture
RTEMS has ever supported. I suspect these warnings also showed up
on the sparc64 port but no one noticed them.
But the cases Gedare points out where they then get cast again
needs examination. Splitting it will help. I think different people are
the expert on each of the three areas.
--joel
>
> diff --git a/cpukit/libdebugger/rtems-debugger-server.c
> b/cpukit/libdebugger/rtems-debugger-server.c
> index 975ec23a30..f8c485a794 100644
> --- a/cpukit/libdebugger/rtems-debugger-server.c
> +++ b/cpukit/libdebugger/rtems-debugger-server.c
> @@ -1438,7 +1438,7 @@ remote_read_memory(uint8_t* buffer, int size)
>if (comma == NULL)
> remote_packet_out_str(r_E01);
>else {
> -DB_UINT addr;
> +uintptr_t addr;
> DB_UINT length;
> int r;
> addr = hex_decode_uint([1]);
> @@ -1468,7 +1468,7 @@ remote_write_memory(uint8_t* buffer, int size)
>comma = strchr((const char*) buffer, ',');
>colon = strchr((const char*) buffer, ':');
>if (comma != NULL && colon != NULL) {
> -DB_UINT addr;
> +uintptr_t addr;
> DB_UINT length;
> int r;
> addr = hex_decode_uint([1]);
> diff --git a/cpukit/libdebugger/rtems-debugger-target.c
> b/cpukit/libdebugger/rtems-debugger-target.c
> index bf7579700d..34e4e84d2f 100644
> --- a/cpukit/libdebugger/rtems-debugger-target.c
> +++ b/cpukit/libdebugger/rtems-debugger-target.c
> @@ -168,7 +168,7 @@ rtems_debugger_target_reg_table_size(void)
> }
>
> int
> -rtems_debugger_target_swbreak_control(bool insert, DB_UINT addr, DB_UINT
> kind)
> +rtems_debugger_target_swbreak_control(bool insert, uintptr_t addr,
> DB_UINT kind)
> {
>rtems_debugger_target* target = rtems_debugger->target;
>rtems_debugger_target_swbreak* swbreaks;
> diff --git a/cpukit/libdebugger/rtems-debugger-target.h
> b/cpukit/libdebugger/rtems-debugger-target.h
> index f2abbe5fd3..db356e1f07 100644
> --- a/cpukit/libdebugger/rtems-debugger-target.h
> +++ b/cpukit/libdebugger/rtems-debugger-target.h
> @@ -200,7 +200,7 @@ extern void
> rtems_debugger_target_exception_print(CPU_Exception_frame* frame);
> * Software breakpoints. These are also referred to as memory breakpoints.
> */
> extern int rtems_debugger_target_swbreak_control(boolinsert,
> - DB_UINT addr,
> + uintptr_t addr,
> DB_UINT kind);
>
> /**
> diff --git a/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
> b/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
> index b0894c1d38..61f20765e0 100644
> --- a/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
> +++ b/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
> @@ -52,14 +52,14 @@ static long rtems_fdt_test_timeout = 5;
> static rtems_fdt_handle cmd_fdt_handle;
>
> static void
> -rtems_fdt_write (uint32_t address, uint32_t value)
> +rtems_fdt_write (uintptr_t address, uint32_t value)
> {
>volatile uint32_t* ap = (uint32_t*) address;
>*ap = value;
> }
>
> static uint32_t
> -rtems_fdt_read (uint32_t address)
> +rtems_fdt_read (uintptr_t address)
> {
>volatile uint32_t* ap = (uint32_t*) address;
>return *ap;
> diff --git a/cpukit/libmisc/stackchk/check.c
> b/cpukit/libmisc/stackchk/check.c
> index ab08155c92..a2b63345d9 100644
> --- a/cpukit/libmisc/stackchk/check.c
> +++ b/cpukit/libmisc/stackchk/check.c
> @@ -447,12 +447,13 @@ static bool Stack_check_Dump_threads_usage(
> {
>char name[ 22 ];
>const rtems_printer *printer;
> + uintptr_t sp = _CPU_Context_Get_SP( _thread->Registers );
>
>printer = arg;
>_Thread_Get_name( the_thread, name, sizeof( name ) );
>Stack_check_Dump_stack_usage(
> _thread->Start.Initial_stack,
> -(void *) _CPU_Context_Get_SP( _thread->Registers ),
> +(void *) sp,
> name,
> the_thread->Object.id,
> printer
> --
> 2.27.0
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
___
devel mailing list
Re: [PATCH v1 1/2] cpukit: Fixed 32bit pointers
On Thu, Mar 18, 2021 at 8:18 AM Stephen Clark wrote:
>
> Using 32bit types like uint32_t for pointers creates issues on 64 bit
> architectures like AArch64. Replaced occurrences of these with uintptr_t,
> which will work for both 32 and 64 bit architectures.
I'd like Chris' opinion on this patch. I have a couple notes below.
Also, split this from the long double patch.
> ---
> cpukit/libdebugger/rtems-debugger-server.c | 4 ++--
> cpukit/libdebugger/rtems-debugger-target.c | 2 +-
> cpukit/libdebugger/rtems-debugger-target.h | 2 +-
> cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c | 4 ++--
> cpukit/libmisc/stackchk/check.c| 3 ++-
> 5 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/cpukit/libdebugger/rtems-debugger-server.c
> b/cpukit/libdebugger/rtems-debugger-server.c
> index 975ec23a30..f8c485a794 100644
> --- a/cpukit/libdebugger/rtems-debugger-server.c
> +++ b/cpukit/libdebugger/rtems-debugger-server.c
> @@ -1438,7 +1438,7 @@ remote_read_memory(uint8_t* buffer, int size)
>if (comma == NULL)
> remote_packet_out_str(r_E01);
>else {
> -DB_UINT addr;
> +uintptr_t addr;
> DB_UINT length;
> int r;
> addr = hex_decode_uint([1]);
> @@ -1468,7 +1468,7 @@ remote_write_memory(uint8_t* buffer, int size)
>comma = strchr((const char*) buffer, ',');
>colon = strchr((const char*) buffer, ':');
>if (comma != NULL && colon != NULL) {
> -DB_UINT addr;
> +uintptr_t addr;
> DB_UINT length;
> int r;
> addr = hex_decode_uint([1]);
> diff --git a/cpukit/libdebugger/rtems-debugger-target.c
> b/cpukit/libdebugger/rtems-debugger-target.c
> index bf7579700d..34e4e84d2f 100644
> --- a/cpukit/libdebugger/rtems-debugger-target.c
> +++ b/cpukit/libdebugger/rtems-debugger-target.c
> @@ -168,7 +168,7 @@ rtems_debugger_target_reg_table_size(void)
> }
>
> int
> -rtems_debugger_target_swbreak_control(bool insert, DB_UINT addr, DB_UINT
> kind)
> +rtems_debugger_target_swbreak_control(bool insert, uintptr_t addr, DB_UINT
> kind)
> {
>rtems_debugger_target* target = rtems_debugger->target;
>rtems_debugger_target_swbreak* swbreaks;
> diff --git a/cpukit/libdebugger/rtems-debugger-target.h
> b/cpukit/libdebugger/rtems-debugger-target.h
> index f2abbe5fd3..db356e1f07 100644
> --- a/cpukit/libdebugger/rtems-debugger-target.h
> +++ b/cpukit/libdebugger/rtems-debugger-target.h
> @@ -200,7 +200,7 @@ extern void
> rtems_debugger_target_exception_print(CPU_Exception_frame* frame);
> * Software breakpoints. These are also referred to as memory breakpoints.
> */
> extern int rtems_debugger_target_swbreak_control(boolinsert,
> - DB_UINT addr,
> + uintptr_t addr,
> DB_UINT kind);
>
> /**
> diff --git a/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
> b/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
> index b0894c1d38..61f20765e0 100644
> --- a/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
> +++ b/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
> @@ -52,14 +52,14 @@ static long rtems_fdt_test_timeout = 5;
> static rtems_fdt_handle cmd_fdt_handle;
>
> static void
> -rtems_fdt_write (uint32_t address, uint32_t value)
> +rtems_fdt_write (uintptr_t address, uint32_t value)
> {
>volatile uint32_t* ap = (uint32_t*) address;
This looks suspicious. Making the type a 64-bit address, and then
casting it down to a 32-bit int, just hides the possible type
mismatch. I don't think it is going to be as simple as changing the
interface to be 64-bit clean, you may also need to dig into the
implementation details a bit.
>*ap = value;
> }
>
> static uint32_t
> -rtems_fdt_read (uint32_t address)
> +rtems_fdt_read (uintptr_t address)
> {
>volatile uint32_t* ap = (uint32_t*) address;
ditto
>return *ap;
> diff --git a/cpukit/libmisc/stackchk/check.c b/cpukit/libmisc/stackchk/check.c
> index ab08155c92..a2b63345d9 100644
> --- a/cpukit/libmisc/stackchk/check.c
> +++ b/cpukit/libmisc/stackchk/check.c
> @@ -447,12 +447,13 @@ static bool Stack_check_Dump_threads_usage(
> {
>char name[ 22 ];
>const rtems_printer *printer;
> + uintptr_t sp = _CPU_Context_Get_SP( _thread->Registers );
>
>printer = arg;
>_Thread_Get_name( the_thread, name, sizeof( name ) );
>Stack_check_Dump_stack_usage(
> _thread->Start.Initial_stack,
> -(void *) _CPU_Context_Get_SP( _thread->Registers ),
> +(void *) sp,
> name,
> the_thread->Object.id,
> printer
> --
> 2.27.0
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 1/2] cpukit: Fixed 32bit pointers
Using 32bit types like uint32_t for pointers creates issues on 64 bit
architectures like AArch64. Replaced occurrences of these with uintptr_t,
which will work for both 32 and 64 bit architectures.
---
cpukit/libdebugger/rtems-debugger-server.c | 4 ++--
cpukit/libdebugger/rtems-debugger-target.c | 2 +-
cpukit/libdebugger/rtems-debugger-target.h | 2 +-
cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c | 4 ++--
cpukit/libmisc/stackchk/check.c| 3 ++-
5 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/cpukit/libdebugger/rtems-debugger-server.c
b/cpukit/libdebugger/rtems-debugger-server.c
index 975ec23a30..f8c485a794 100644
--- a/cpukit/libdebugger/rtems-debugger-server.c
+++ b/cpukit/libdebugger/rtems-debugger-server.c
@@ -1438,7 +1438,7 @@ remote_read_memory(uint8_t* buffer, int size)
if (comma == NULL)
remote_packet_out_str(r_E01);
else {
-DB_UINT addr;
+uintptr_t addr;
DB_UINT length;
int r;
addr = hex_decode_uint([1]);
@@ -1468,7 +1468,7 @@ remote_write_memory(uint8_t* buffer, int size)
comma = strchr((const char*) buffer, ',');
colon = strchr((const char*) buffer, ':');
if (comma != NULL && colon != NULL) {
-DB_UINT addr;
+uintptr_t addr;
DB_UINT length;
int r;
addr = hex_decode_uint([1]);
diff --git a/cpukit/libdebugger/rtems-debugger-target.c
b/cpukit/libdebugger/rtems-debugger-target.c
index bf7579700d..34e4e84d2f 100644
--- a/cpukit/libdebugger/rtems-debugger-target.c
+++ b/cpukit/libdebugger/rtems-debugger-target.c
@@ -168,7 +168,7 @@ rtems_debugger_target_reg_table_size(void)
}
int
-rtems_debugger_target_swbreak_control(bool insert, DB_UINT addr, DB_UINT kind)
+rtems_debugger_target_swbreak_control(bool insert, uintptr_t addr, DB_UINT
kind)
{
rtems_debugger_target* target = rtems_debugger->target;
rtems_debugger_target_swbreak* swbreaks;
diff --git a/cpukit/libdebugger/rtems-debugger-target.h
b/cpukit/libdebugger/rtems-debugger-target.h
index f2abbe5fd3..db356e1f07 100644
--- a/cpukit/libdebugger/rtems-debugger-target.h
+++ b/cpukit/libdebugger/rtems-debugger-target.h
@@ -200,7 +200,7 @@ extern void
rtems_debugger_target_exception_print(CPU_Exception_frame* frame);
* Software breakpoints. These are also referred to as memory breakpoints.
*/
extern int rtems_debugger_target_swbreak_control(boolinsert,
- DB_UINT addr,
+ uintptr_t addr,
DB_UINT kind);
/**
diff --git a/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
b/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
index b0894c1d38..61f20765e0 100644
--- a/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
+++ b/cpukit/libmisc/rtems-fdt/rtems-fdt-shell.c
@@ -52,14 +52,14 @@ static long rtems_fdt_test_timeout = 5;
static rtems_fdt_handle cmd_fdt_handle;
static void
-rtems_fdt_write (uint32_t address, uint32_t value)
+rtems_fdt_write (uintptr_t address, uint32_t value)
{
volatile uint32_t* ap = (uint32_t*) address;
*ap = value;
}
static uint32_t
-rtems_fdt_read (uint32_t address)
+rtems_fdt_read (uintptr_t address)
{
volatile uint32_t* ap = (uint32_t*) address;
return *ap;
diff --git a/cpukit/libmisc/stackchk/check.c b/cpukit/libmisc/stackchk/check.c
index ab08155c92..a2b63345d9 100644
--- a/cpukit/libmisc/stackchk/check.c
+++ b/cpukit/libmisc/stackchk/check.c
@@ -447,12 +447,13 @@ static bool Stack_check_Dump_threads_usage(
{
char name[ 22 ];
const rtems_printer *printer;
+ uintptr_t sp = _CPU_Context_Get_SP( _thread->Registers );
printer = arg;
_Thread_Get_name( the_thread, name, sizeof( name ) );
Stack_check_Dump_stack_usage(
_thread->Start.Initial_stack,
-(void *) _CPU_Context_Get_SP( _thread->Registers ),
+(void *) sp,
name,
the_thread->Object.id,
printer
--
2.27.0
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
