subject:"\[PATCH v4\] rtems\-debugger\: Fixed 32bit pointers"

Re: [PATCH v4] rtems-debugger: Fixed 32bit pointers

2021-05-06 Thread Gedare Bloom

On Thu, May 6, 2021 at 10:13 AM Joel Sherrill  wrote:
>
>
>
> On Wed, May 5, 2021 at 12:35 AM Gedare Bloom  wrote:
>>
>> On Tue, May 4, 2021 at 1:34 PM Stephen Clark  
>> 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.
>
OK. probably this can be done with macros, but I won't push for that :)

>
>>
>> >  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([1]);
>> > +addr = hex_decode_addr([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([1]);
>> > +addr = hex_decode_addr([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
>> >

Re: [PATCH v4] rtems-debugger: Fixed 32bit pointers

2021-05-06 Thread Joel Sherrill

On Wed, May 5, 2021 at 12:35 AM Gedare Bloom  wrote:

> On Tue, May 4, 2021 at 1:34 PM Stephen Clark 
> 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([1]);
> > +addr = hex_decode_addr([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([1]);
> > +addr = hex_decode_addr([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.


>
>

Re: [PATCH v4] rtems-debugger: Fixed 32bit pointers

2021-05-04 Thread Gedare Bloom

On Tue, May 4, 2021 at 1:34 PM Stephen Clark  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?

>  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([1]);
> +addr = hex_decode_addr([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([1]);
> +addr = hex_decode_addr([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?

>  {
>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(boolinsert,
> - DB_UINT addr,
> + uintptr_t addr,
ditto

>   DB_UINT kind);
>
>  /**
> --
> 2.27.0
>
> ___
> devel mailing list
> devel@rtems.org
>

[PATCH v4] rtems-debugger: Fixed 32bit pointers

2021-05-04 Thread Stephen Clark

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;
+}
+
 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([1]);
+addr = hex_decode_addr([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([1]);
+addr = hex_decode_addr([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)
 {
   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(boolinsert,
- DB_UINT addr,
+ uintptr_t addr,
  DB_UINT kind);
 
 /**
-- 
2.27.0

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v4] rtems-debugger: Fixed 32bit pointers

Re: [PATCH v4] rtems-debugger: Fixed 32bit pointers

Re: [PATCH v4] rtems-debugger: Fixed 32bit pointers

[PATCH v4] rtems-debugger: Fixed 32bit pointers

4 matches

Site Navigation

Mail list logo

Footer information