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

[Joel Sherrill]

On Sat, Mar 20, 2021, 1:21 AM Gedare Bloom <gedare at rtems.org> wrote:

> Hi Stephen, Joel:
>
> On Fri, Mar 19, 2021 at 9:35 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.
> > ---
> >  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]);
>
> I still have concerns that just changing the variable types is not
> addressing any possible fundamental type errors that get hidden by
> later type coercion, for example, here we see addr = hex_decode_uint()
> that returns type DB_UINT, so now we have a type mismatch in 64-bit
> targets. Now, maybe it is fine, because the addresses are limited to
> 32-bits in practice, but I would like some discussion of the analysis
> that has been done on these type changes.
>

I commented earlier in this thread that Chris needs to comment on the gdb
protocol. Switching to uintptr_t should be correct but, as you point out,
whether or not the gdb protocol uses 64 bit addresses in the messages is
unknown to us both.

Looking at the gdb manual, it seems to use addr a lot without explaining
this point.

My money says there needs to be a decode address which returns a uintptr_t.
And that combined with this patch and a review of where the variables are
used is needed.

I suspect the answer is in something like remote.c in the gdb source.

But we really need guidance from Chris.


> > @@ -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);
> >
> >  /**
> > --
> > 2.27.0
> >
> > _______________________________________________
> > devel mailing list
> > devel at rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel
> _______________________________________________
> 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/20210320/d7d93bfb/attachment.html>