[PATCH v1 1/2] cpukit: Fixed 32bit pointers

[Gedare Bloom]

On Thu, Mar 18, 2021 at 8:18 AM Stephen Clark <stephen.clark at oarcorp.com> 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(&buffer[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(&buffer[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(bool    insert,
> -                                                 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( &the_thread->Registers );
>
>    printer = arg;
>    _Thread_Get_name( the_thread, name, sizeof( name ) );
>    Stack_check_Dump_stack_usage(
>      &the_thread->Start.Initial_stack,
> -    (void *) _CPU_Context_Get_SP( &the_thread->Registers ),
> +    (void *) sp,
>      name,
>      the_thread->Object.id,
>      printer
> --
> 2.27.0
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel