<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, May 5, 2021 at 12:35 AM Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, May 4, 2021 at 1:34 PM Stephen Clark <<a href="mailto:stephen.clark@oarcorp.com" target="_blank">stephen.clark@oarcorp.com</a>> wrote:<br>
><br>
> Using 32bit types like uint32_t for pointers creates issues on 64 bit<br>
> architectures like AArch64. Replaced occurrences of these with uintptr_t,<br>
> which will work for both 32 and 64 bit architectures. Added hex_decode_addr<br>
> function to rtems-debugger.<br>
> ---<br>
> .../rtems/debugger/rtems-debugger-server.h | 5 ++++<br>
> cpukit/libdebugger/rtems-debugger-server.c | 30 +++++++++++++++----<br>
> cpukit/libdebugger/rtems-debugger-target.c | 2 +-<br>
> cpukit/libdebugger/rtems-debugger-target.h | 2 +-<br>
> 4 files changed, 32 insertions(+), 7 deletions(-)<br>
><br>
> diff --git a/cpukit/include/rtems/debugger/rtems-debugger-server.h b/cpukit/include/rtems/debugger/rtems-debugger-server.h<br>
> index 2189aac873..beff302641 100644<br>
> --- a/cpukit/include/rtems/debugger/rtems-debugger-server.h<br>
> +++ b/cpukit/include/rtems/debugger/rtems-debugger-server.h<br>
> @@ -50,6 +50,11 @@ extern "C" {<br>
> */<br>
> #define DB_UINT uint32_t<br>
><br>
> +/**<br>
> + * Data type for addresses. Here for 64bit support.<br>
> + */<br>
> +#define DB_ADDR uintptr_t<br>
> +<br>
> /*<br>
> * Debugger signals.<br>
> */<br>
> diff --git a/cpukit/libdebugger/rtems-debugger-server.c b/cpukit/libdebugger/rtems-debugger-server.c<br>
> index 975ec23a30..2ada418a79 100644<br>
> --- a/cpukit/libdebugger/rtems-debugger-server.c<br>
> +++ b/cpukit/libdebugger/rtems-debugger-server.c<br>
> @@ -154,6 +154,26 @@ hex_encode(int val)<br>
> return "0123456789abcdef"[val & 0xf];<br>
> }<br>
><br>
> +static inline DB_ADDR<br>
> +hex_decode_addr(const uint8_t* data)<br>
> +{<br>
> + DB_ADDR ui = 0;<br>
> + size_t i;<br>
> + if (data[0] == '-') {<br>
> + if (data[1] == '1')<br>
> + ui = (DB_ADDR) -1;<br>
> + }<br>
> + else {<br>
> + for (i = 0; i < (sizeof(ui) * 2); ++i) {<br>
> + int v = hex_decode(data[i]);<br>
> + if (v < 0)<br>
> + break;<br>
> + ui = (ui << 4) | v;<br>
> + }<br>
> + }<br>
> + return ui;<br>
> +}<br>
> +<br>
This function body is identical to the following hex_decode_uint()<br>
except for the type of DB_ADDR. They could probably be merged. Is<br>
there a reason not to combine them and avoid the copy-paste?<br></blockquote><div><br></div><div>DB_ADDR aka uintptr_t is just stated as being large enough to hold</div><div>an address. It is not listed in the set of types the C Standard makes </div><div>size guarantees about. <span style="font-family:"Segoe UI",system-ui,"Apple Color Emoji","Segoe UI Emoji",sans-serif;font-size:14px">See </span></div><div><a href="http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf" title="http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf" style="font-family:"Segoe UI",system-ui,"Apple Color Emoji","Segoe UI Emoji",sans-serif;font-size:14px">http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf</a><span style="font-family:"Segoe UI",system-ui,"Apple Color Emoji","Segoe UI Emoji",sans-serif;font-size:14px"> in </span></div><div><span style="font-family:"Segoe UI",system-ui,"Apple Color Emoji","Segoe UI Emoji",sans-serif;font-size:14px">section 5.2.4.2.1 for the relationship between types that are guaranteed. </span></div><div><span style="font-family:"Segoe UI",system-ui,"Apple Color Emoji","Segoe UI Emoji",sans-serif;font-size:14px">Notice that uintptr_t is just described in English when it shows up </span></div><div><span style="font-family:"Segoe UI",system-ui,"Apple Color Emoji","Segoe UI Emoji",sans-serif;font-size:14px">in the standard and not in this list. </span> </div><div><br></div><div>I don't think we can assume any specific integer type is equivalent to </div><div>uintptr_t. </div><div><br></div><div>It is a good example for function templates but they are not in C.</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> static inline DB_UINT<br>
> hex_decode_uint(const uint8_t* data)<br>
> {<br>
> @@ -1438,10 +1458,10 @@ remote_read_memory(uint8_t* buffer, int size)<br>
> if (comma == NULL)<br>
> remote_packet_out_str(r_E01);<br>
> else {<br>
> - DB_UINT addr;<br>
> + uintptr_t addr;<br>
> DB_UINT length;<br>
> int r;<br>
> - addr = hex_decode_uint(&buffer[1]);<br>
> + addr = hex_decode_addr(&buffer[1]);<br>
> length = hex_decode_uint((const uint8_t*) comma + 1);<br>
> remote_packet_out_reset();<br>
> r = rtems_debugger_target_start_memory_access();<br>
> @@ -1468,10 +1488,10 @@ remote_write_memory(uint8_t* buffer, int size)<br>
> comma = strchr((const char*) buffer, ',');<br>
> colon = strchr((const char*) buffer, ':');<br>
> if (comma != NULL && colon != NULL) {<br>
> - DB_UINT addr;<br>
> + uintptr_t addr;<br>
> DB_UINT length;<br>
> int r;<br>
> - addr = hex_decode_uint(&buffer[1]);<br>
> + addr = hex_decode_addr(&buffer[1]);<br>
> length = hex_decode_uint((const uint8_t*) comma + 1);<br>
> r = rtems_debugger_target_start_memory_access();<br>
> if (r == 0) {<br>
> @@ -1521,7 +1541,7 @@ remote_breakpoints(bool insert, uint8_t* buffer, int size)<br>
> uint32_t capabilities;<br>
> DB_UINT addr;<br>
> DB_UINT kind;<br>
> - addr = hex_decode_uint((const uint8_t*) comma1 + 1);<br>
> + addr = hex_decode_addr((const uint8_t*) comma1 + 1);<br>
> kind = hex_decode_uint((const uint8_t*)comma2 + 1);<br>
> capabilities = rtems_debugger_target_capabilities();<br>
> switch (buffer[1]) {<br>
> diff --git a/cpukit/libdebugger/rtems-debugger-target.c b/cpukit/libdebugger/rtems-debugger-target.c<br>
> index bf7579700d..34e4e84d2f 100644<br>
> --- a/cpukit/libdebugger/rtems-debugger-target.c<br>
> +++ b/cpukit/libdebugger/rtems-debugger-target.c<br>
> @@ -168,7 +168,7 @@ rtems_debugger_target_reg_table_size(void)<br>
> }<br>
><br>
> int<br>
> -rtems_debugger_target_swbreak_control(bool insert, DB_UINT addr, DB_UINT kind)<br>
> +rtems_debugger_target_swbreak_control(bool insert, uintptr_t addr, DB_UINT kind)<br>
why not use DB_ADDR?<br></blockquote><div><br></div><div>That should be DB_ADDR unless there is another sweep to eliminate DB_ADDR.</div><div>But I wouldn't do that on this pass.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> {<br>
> rtems_debugger_target* target = rtems_debugger->target;<br>
> rtems_debugger_target_swbreak* swbreaks;<br>
> diff --git a/cpukit/libdebugger/rtems-debugger-target.h b/cpukit/libdebugger/rtems-debugger-target.h<br>
> index f2abbe5fd3..db356e1f07 100644<br>
> --- a/cpukit/libdebugger/rtems-debugger-target.h<br>
> +++ b/cpukit/libdebugger/rtems-debugger-target.h<br>
> @@ -200,7 +200,7 @@ extern void rtems_debugger_target_exception_print(CPU_Exception_frame* frame);<br>
> * Software breakpoints. These are also referred to as memory breakpoints.<br>
> */<br>
> extern int rtems_debugger_target_swbreak_control(bool insert,<br>
> - DB_UINT addr,<br>
> + uintptr_t addr,<br>
ditto<br>
<br>
> DB_UINT kind);<br>
><br>
> /**<br>
> --<br>
> 2.27.0<br>
><br>
> _______________________________________________<br>
> devel mailing list<br>
> <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div></div>