[PATCH v2 1/3] rtems-debugger: Fixed 32bit pointers

[Joel Sherrill]

After picking on Ryan, Alex, and Sebastian, you get it next. :)

"Fix" or "Fixed" in the short commit message title is not useful
when you browse the log of commits:

https://git.rtems.org/rtems/log/

Something like "Use uint32_t not uintptr_t for portability to 64-bit CPUs"
says a lot more. Think of yourself reading that list of commits and what
messages tell you enough and which leave you thinking that you need
to look at the change to know what was done.



On Thu, Mar 18, 2021 at 12:33 PM 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.
> ---
>  cpukit/libdebugger/rtems-debugger-server.c | 4 ++--
>  cpukit/libdebugger/rtems-debugger-target.c | 2 +-
>  cpukit/libdebugger/rtems-debugger-target.h | 2 +-
>  3 files changed, 4 insertions(+), 4 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]);


I think the changes OK but Chris should comment what
happens on 64-bit address targets.

I think this may be decoding the gdb protocol message and we need
to know if the field coming in is OK to decode as a uint.

Your patch is OK but there may be an issue interfacing with the protocol
that this points out.


> 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);
>
>  /**
> --
> 2.27.0
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210318/1fb40d49/attachment.html>