* [PATCH v2] gdbserver: dump 'xx...x' in collect_register_as_string for unavailable register (was: gdbserver: extract code out of regcache's registers_to_string) @ 2023-06-20 15:54 Tankut Baris Aktemur 2023-11-21 19:53 ` Aktemur, Tankut Baris 0 siblings, 1 reply; 4+ messages in thread From: Tankut Baris Aktemur @ 2023-06-20 15:54 UTC (permalink / raw) To: gdb-patches; +Cc: tom Fix 'collect_register_as_string' so that unavailable registers are dumped as 'xx...x' instead of arbitrary values. This gives the opportunity that we can reuse 'collect_register_as_string' in 'registers_to_string' for additional code simplification. --- gdbserver/regcache.cc | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc index 0b1141662ac..c0a6d6eb0a4 100644 --- a/gdbserver/regcache.cc +++ b/gdbserver/regcache.cc @@ -210,24 +210,13 @@ find_register_by_number (const struct target_desc *tdesc, int n) void registers_to_string (struct regcache *regcache, char *buf) { - unsigned char *registers = regcache->registers; const struct target_desc *tdesc = regcache->tdesc; for (int i = 0; i < tdesc->reg_defs.size (); ++i) { - if (regcache->register_status[i] == REG_VALID) - { - bin2hex (registers, buf, register_size (tdesc, i)); - buf += register_size (tdesc, i) * 2; - } - else - { - memset (buf, 'x', register_size (tdesc, i) * 2); - buf += register_size (tdesc, i) * 2; - } - registers += register_size (tdesc, i); + collect_register_as_string (regcache, i, buf); + buf += register_size (tdesc, i) * 2; } - *buf = '\0'; } void @@ -472,8 +461,15 @@ regcache_raw_get_unsigned_by_name (struct regcache *regcache, void collect_register_as_string (struct regcache *regcache, int n, char *buf) { - bin2hex (register_data (regcache, n), buf, - register_size (regcache->tdesc, n)); + int reg_size = register_size (regcache->tdesc, n); + + if (regcache->register_status[n] == REG_VALID) + bin2hex (register_data (regcache, n), buf, reg_size); + else + memset (buf, 'x', reg_size * 2); + + buf += reg_size * 2; + *buf = '\0'; } void -- 2.25.1 Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] gdbserver: dump 'xx...x' in collect_register_as_string for unavailable register (was: gdbserver: extract code out of regcache's registers_to_string) 2023-06-20 15:54 [PATCH v2] gdbserver: dump 'xx...x' in collect_register_as_string for unavailable register (was: gdbserver: extract code out of regcache's registers_to_string) Tankut Baris Aktemur @ 2023-11-21 19:53 ` Aktemur, Tankut Baris 2023-11-24 11:21 ` [PATCH v2] gdbserver: dump 'xx...x' in collect_register_as_string for unavailable register Luis Machado 0 siblings, 1 reply; 4+ messages in thread From: Aktemur, Tankut Baris @ 2023-11-21 19:53 UTC (permalink / raw) To: gdb-patches; +Cc: tom Kindly pinging. Regards -Baris On Tuesday, June 20, 2023 5:55 PM, Aktemur, Tankut Baris wrote: > Fix 'collect_register_as_string' so that unavailable registers are > dumped as 'xx...x' instead of arbitrary values. This gives the > opportunity that we can reuse 'collect_register_as_string' in > 'registers_to_string' for additional code simplification. > --- > gdbserver/regcache.cc | 26 +++++++++++--------------- > 1 file changed, 11 insertions(+), 15 deletions(-) > > diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc > index 0b1141662ac..c0a6d6eb0a4 100644 > --- a/gdbserver/regcache.cc > +++ b/gdbserver/regcache.cc > @@ -210,24 +210,13 @@ find_register_by_number (const struct target_desc *tdesc, int > n) > void > registers_to_string (struct regcache *regcache, char *buf) > { > - unsigned char *registers = regcache->registers; > const struct target_desc *tdesc = regcache->tdesc; > > for (int i = 0; i < tdesc->reg_defs.size (); ++i) > { > - if (regcache->register_status[i] == REG_VALID) > - { > - bin2hex (registers, buf, register_size (tdesc, i)); > - buf += register_size (tdesc, i) * 2; > - } > - else > - { > - memset (buf, 'x', register_size (tdesc, i) * 2); > - buf += register_size (tdesc, i) * 2; > - } > - registers += register_size (tdesc, i); > + collect_register_as_string (regcache, i, buf); > + buf += register_size (tdesc, i) * 2; > } > - *buf = '\0'; > } > > void > @@ -472,8 +461,15 @@ regcache_raw_get_unsigned_by_name (struct regcache *regcache, > void > collect_register_as_string (struct regcache *regcache, int n, char *buf) > { > - bin2hex (register_data (regcache, n), buf, > - register_size (regcache->tdesc, n)); > + int reg_size = register_size (regcache->tdesc, n); > + > + if (regcache->register_status[n] == REG_VALID) > + bin2hex (register_data (regcache, n), buf, reg_size); > + else > + memset (buf, 'x', reg_size * 2); > + > + buf += reg_size * 2; > + *buf = '\0'; > } > > void > -- > 2.25.1 Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] gdbserver: dump 'xx...x' in collect_register_as_string for unavailable register 2023-11-21 19:53 ` Aktemur, Tankut Baris @ 2023-11-24 11:21 ` Luis Machado 2023-11-28 8:24 ` Aktemur, Tankut Baris 0 siblings, 1 reply; 4+ messages in thread From: Luis Machado @ 2023-11-24 11:21 UTC (permalink / raw) To: Aktemur, Tankut Baris, gdb-patches; +Cc: tom Hi, On 11/21/23 19:53, Aktemur, Tankut Baris wrote: > Kindly pinging. > > Regards > -Baris > > On Tuesday, June 20, 2023 5:55 PM, Aktemur, Tankut Baris wrote: >> Fix 'collect_register_as_string' so that unavailable registers are >> dumped as 'xx...x' instead of arbitrary values. This gives the >> opportunity that we can reuse 'collect_register_as_string' in >> 'registers_to_string' for additional code simplification. >> --- >> gdbserver/regcache.cc | 26 +++++++++++--------------- >> 1 file changed, 11 insertions(+), 15 deletions(-) >> >> diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc >> index 0b1141662ac..c0a6d6eb0a4 100644 >> --- a/gdbserver/regcache.cc >> +++ b/gdbserver/regcache.cc >> @@ -210,24 +210,13 @@ find_register_by_number (const struct target_desc *tdesc, int >> n) >> void >> registers_to_string (struct regcache *regcache, char *buf) >> { >> - unsigned char *registers = regcache->registers; >> const struct target_desc *tdesc = regcache->tdesc; >> >> for (int i = 0; i < tdesc->reg_defs.size (); ++i) >> { >> - if (regcache->register_status[i] == REG_VALID) >> - { >> - bin2hex (registers, buf, register_size (tdesc, i)); >> - buf += register_size (tdesc, i) * 2; >> - } >> - else >> - { >> - memset (buf, 'x', register_size (tdesc, i) * 2); >> - buf += register_size (tdesc, i) * 2; >> - } >> - registers += register_size (tdesc, i); >> + collect_register_as_string (regcache, i, buf); >> + buf += register_size (tdesc, i) * 2; >> } >> - *buf = '\0'; >> } >> >> void >> @@ -472,8 +461,15 @@ regcache_raw_get_unsigned_by_name (struct regcache *regcache, >> void >> collect_register_as_string (struct regcache *regcache, int n, char *buf) >> { >> - bin2hex (register_data (regcache, n), buf, >> - register_size (regcache->tdesc, n)); >> + int reg_size = register_size (regcache->tdesc, n); >> + >> + if (regcache->register_status[n] == REG_VALID) >> + bin2hex (register_data (regcache, n), buf, reg_size); >> + else >> + memset (buf, 'x', reg_size * 2); >> + >> + buf += reg_size * 2; >> + *buf = '\0'; >> } I don't mind this improvement, but I was wondering if using the std::string variant of bin2hex would allow further simplification of this code. You wouldn't have to handle the \0 explicitly. I guess the slightly confusing part is that collect_register_as_string finishes the string with \0, but registers_to_string ignores it and overwrites it. In any case, I think this is a nice improvement. Reviewed-By: Luis Machado <luis.machado@arm.com> >> >> void >> -- >> 2.25.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] gdbserver: dump 'xx...x' in collect_register_as_string for unavailable register 2023-11-24 11:21 ` [PATCH v2] gdbserver: dump 'xx...x' in collect_register_as_string for unavailable register Luis Machado @ 2023-11-28 8:24 ` Aktemur, Tankut Baris 0 siblings, 0 replies; 4+ messages in thread From: Aktemur, Tankut Baris @ 2023-11-28 8:24 UTC (permalink / raw) To: Luis Machado, gdb-patches; +Cc: tom On Friday, November 24, 2023 12:22 PM, Luis Machado wrote: > > Hi, > > On 11/21/23 19:53, Aktemur, Tankut Baris wrote: > > Kindly pinging. > > > > Regards > > -Baris > > > > On Tuesday, June 20, 2023 5:55 PM, Aktemur, Tankut Baris wrote: > >> Fix 'collect_register_as_string' so that unavailable registers are > >> dumped as 'xx...x' instead of arbitrary values. This gives the > >> opportunity that we can reuse 'collect_register_as_string' in > >> 'registers_to_string' for additional code simplification. > >> --- > >> gdbserver/regcache.cc | 26 +++++++++++--------------- > >> 1 file changed, 11 insertions(+), 15 deletions(-) > >> > >> diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc > >> index 0b1141662ac..c0a6d6eb0a4 100644 > >> --- a/gdbserver/regcache.cc > >> +++ b/gdbserver/regcache.cc > >> @@ -210,24 +210,13 @@ find_register_by_number (const struct target_desc *tdesc, int > >> n) > >> void > >> registers_to_string (struct regcache *regcache, char *buf) > >> { > >> - unsigned char *registers = regcache->registers; > >> const struct target_desc *tdesc = regcache->tdesc; > >> > >> for (int i = 0; i < tdesc->reg_defs.size (); ++i) > >> { > >> - if (regcache->register_status[i] == REG_VALID) > >> - { > >> - bin2hex (registers, buf, register_size (tdesc, i)); > >> - buf += register_size (tdesc, i) * 2; > >> - } > >> - else > >> - { > >> - memset (buf, 'x', register_size (tdesc, i) * 2); > >> - buf += register_size (tdesc, i) * 2; > >> - } > >> - registers += register_size (tdesc, i); > >> + collect_register_as_string (regcache, i, buf); > >> + buf += register_size (tdesc, i) * 2; > >> } > >> - *buf = '\0'; > >> } > >> > >> void > >> @@ -472,8 +461,15 @@ regcache_raw_get_unsigned_by_name (struct regcache *regcache, > >> void > >> collect_register_as_string (struct regcache *regcache, int n, char *buf) > >> { > >> - bin2hex (register_data (regcache, n), buf, > >> - register_size (regcache->tdesc, n)); > >> + int reg_size = register_size (regcache->tdesc, n); > >> + > >> + if (regcache->register_status[n] == REG_VALID) > >> + bin2hex (register_data (regcache, n), buf, reg_size); > >> + else > >> + memset (buf, 'x', reg_size * 2); > >> + > >> + buf += reg_size * 2; > >> + *buf = '\0'; > >> } > > I don't mind this improvement, but I was wondering if using the std::string variant of > bin2hex would > allow further simplification of this code. You wouldn't have to handle the \0 explicitly. > > I guess the slightly confusing part is that collect_register_as_string finishes the string > with \0, > but registers_to_string ignores it and overwrites it. > > In any case, I think this is a nice improvement. > > Reviewed-By: Luis Machado <luis.machado@arm.com> Hi Luis, Thanks for the comments. Overwriting the null terminator does indeed look odd, but using the std::string variant of bin2hex would require creating intermediary copies of pieces of the overall string. To avoid this increased cost, I thought we could still prefer the lower-level approach above. (The BUF buffer we are writing into is in practice the client_state buffer.) Regards -Baris Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-11-28 8:24 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-20 15:54 [PATCH v2] gdbserver: dump 'xx...x' in collect_register_as_string for unavailable register (was: gdbserver: extract code out of regcache's registers_to_string) Tankut Baris Aktemur 2023-11-21 19:53 ` Aktemur, Tankut Baris 2023-11-24 11:21 ` [PATCH v2] gdbserver: dump 'xx...x' in collect_register_as_string for unavailable register Luis Machado 2023-11-28 8:24 ` Aktemur, Tankut Baris
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).