On Wed, May 5, 2021 at 12:35 AM Gedare Bloom <ged...@rtems.org> wrote:
> On Tue, May 4, 2021 at 1:34 PM Stephen Clark <stephen.cl...@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. Added > hex_decode_addr > > function to rtems-debugger. > > --- > > .../rtems/debugger/rtems-debugger-server.h | 5 ++++ > > cpukit/libdebugger/rtems-debugger-server.c | 30 +++++++++++++++---- > > cpukit/libdebugger/rtems-debugger-target.c | 2 +- > > cpukit/libdebugger/rtems-debugger-target.h | 2 +- > > 4 files changed, 32 insertions(+), 7 deletions(-) > > > > diff --git a/cpukit/include/rtems/debugger/rtems-debugger-server.h > b/cpukit/include/rtems/debugger/rtems-debugger-server.h > > index 2189aac873..beff302641 100644 > > --- a/cpukit/include/rtems/debugger/rtems-debugger-server.h > > +++ b/cpukit/include/rtems/debugger/rtems-debugger-server.h > > @@ -50,6 +50,11 @@ extern "C" { > > */ > > #define DB_UINT uint32_t > > > > +/** > > + * Data type for addresses. Here for 64bit support. > > + */ > > +#define DB_ADDR uintptr_t > > + > > /* > > * Debugger signals. > > */ > > diff --git a/cpukit/libdebugger/rtems-debugger-server.c > b/cpukit/libdebugger/rtems-debugger-server.c > > index 975ec23a30..2ada418a79 100644 > > --- a/cpukit/libdebugger/rtems-debugger-server.c > > +++ b/cpukit/libdebugger/rtems-debugger-server.c > > @@ -154,6 +154,26 @@ hex_encode(int val) > > return "0123456789abcdef"[val & 0xf]; > > } > > > > +static inline DB_ADDR > > +hex_decode_addr(const uint8_t* data) > > +{ > > + DB_ADDR ui = 0; > > + size_t i; > > + if (data[0] == '-') { > > + if (data[1] == '1') > > + ui = (DB_ADDR) -1; > > + } > > + else { > > + for (i = 0; i < (sizeof(ui) * 2); ++i) { > > + int v = hex_decode(data[i]); > > + if (v < 0) > > + break; > > + ui = (ui << 4) | v; > > + } > > + } > > + return ui; > > +} > > + > This function body is identical to the following hex_decode_uint() > except for the type of DB_ADDR. They could probably be merged. Is > there a reason not to combine them and avoid the copy-paste? > DB_ADDR aka uintptr_t is just stated as being large enough to hold an address. It is not listed in the set of types the C Standard makes size guarantees about. See http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf in section 5.2.4.2.1 for the relationship between types that are guaranteed. Notice that uintptr_t is just described in English when it shows up in the standard and not in this list. I don't think we can assume any specific integer type is equivalent to uintptr_t. It is a good example for function templates but they are not in C. > > static inline DB_UINT > > hex_decode_uint(const uint8_t* data) > > { > > @@ -1438,10 +1458,10 @@ 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]); > > + addr = hex_decode_addr(&buffer[1]); > > length = hex_decode_uint((const uint8_t*) comma + 1); > > remote_packet_out_reset(); > > r = rtems_debugger_target_start_memory_access(); > > @@ -1468,10 +1488,10 @@ 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]); > > + addr = hex_decode_addr(&buffer[1]); > > length = hex_decode_uint((const uint8_t*) comma + 1); > > r = rtems_debugger_target_start_memory_access(); > > if (r == 0) { > > @@ -1521,7 +1541,7 @@ remote_breakpoints(bool insert, uint8_t* buffer, > int size) > > uint32_t capabilities; > > DB_UINT addr; > > DB_UINT kind; > > - addr = hex_decode_uint((const uint8_t*) comma1 + 1); > > + addr = hex_decode_addr((const uint8_t*) comma1 + 1); > > kind = hex_decode_uint((const uint8_t*)comma2 + 1); > > capabilities = rtems_debugger_target_capabilities(); > > switch (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) > why not use DB_ADDR? > That should be DB_ADDR unless there is another sweep to eliminate DB_ADDR. But I wouldn't do that on this pass. > > > { > > 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, > ditto > > > DB_UINT kind); > > > > /** > > -- > > 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 >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel