This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] ppc64le/gdbserver: Fix ppc_collect/supply_ptrace_register() routines
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Edjunior Barbosa Machado <emachado at linux dot vnet dot ibm dot com>
- Cc: gdb-patches at sourceware dot org, Ulrich Weigand <uweigand at de dot ibm dot com>
- Date: Fri, 05 Sep 2014 18:54:41 -0400
- Subject: Re: [PATCH] ppc64le/gdbserver: Fix ppc_collect/supply_ptrace_register() routines
- Authentication-results: sourceware.org; auth=none
- References: <1409947102-32166-1-git-send-email-emachado at linux dot vnet dot ibm dot com>
On Friday, September 05 2014, Edjunior Barbosa Machado wrote:
> Hi,
>
> this patch fixes the routines to collect and supply ptrace registers on ppc64le
> gdbserver. Originally written for big endian arch, they were causing several
> issues on little endian. With this fix, the number of unexpected failures in
> the testsuite dropped from 263 to 72 on ppc64le.
> Tested on ppc64{,le}. Ok?
Heya!
Thanks for the patch. A few comments below.
> gdb/gdbserver/
> 2014-09-05 Edjunior Barbosa Machado <emachado@linux.vnet.ibm.com>
>
> * linux-ppc-low.c (ppc_collect_ptrace_register): Adjust routine to take
> endianness into account.
> (ppc_supply_ptrace_register): Likewise.
>
> ---
> gdb/gdbserver/linux-ppc-low.c | 37 ++++++++++++++++++++++++++++---------
> 1 file changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/gdb/gdbserver/linux-ppc-low.c b/gdb/gdbserver/linux-ppc-low.c
> index d743311..1898741 100644
> --- a/gdb/gdbserver/linux-ppc-low.c
> +++ b/gdb/gdbserver/linux-ppc-low.c
> @@ -202,25 +202,44 @@ ppc_cannot_fetch_register (int regno)
> static void
> ppc_collect_ptrace_register (struct regcache *regcache, int regno, char *buf)
> {
> - int size = register_size (regcache->tdesc, regno);
> -
> memset (buf, 0, sizeof (long));
>
> - if (size < sizeof (long))
> - collect_register (regcache, regno, buf + sizeof (long) - size);
> + if (__BYTE_ORDER == __LITTLE_ENDIAN)
Why not use gdbarch_byte_order here? We don't use __BYTE_ORDER anywhere
in the code.
> + {
> + /* Little-endian values always sit at the left end of the buffer. */
> + collect_register (regcache, regno, buf);
> + }
> + else if (__BYTE_ORDER == __BIG_ENDIAN)
> + {
> + /* Big-endian values sit at the right end of the buffer. In case of
"... of the buffer. In case..."
Two spaces after period :-).
> + registers whose size is smaller than sizeof (long), we must use a
I'd prefer:
s/size is/sizes are/
> + padding to access it correctly. */
s/it/them/
> + int padding = (sizeof (long) - register_size (regcache->tdesc, regno));
> + collect_register (regcache, regno, buf + padding);
Please leave an empty newline between the declaration of the variable
and the rest of the code.
> + }
> else
> - collect_register (regcache, regno, buf);
> + perror_with_name ("Unexpected byte order");
> }
>
> static void
> ppc_supply_ptrace_register (struct regcache *regcache,
> int regno, const char *buf)
> {
> - int size = register_size (regcache->tdesc, regno);
> - if (size < sizeof (long))
> - supply_register (regcache, regno, buf + sizeof (long) - size);
> + if (__BYTE_ORDER == __LITTLE_ENDIAN)
> + {
> + /* Little-endian values always sit at the left end of the buffer. */
> + supply_register (regcache, regno, buf);
> + }
> + else if (__BYTE_ORDER == __BIG_ENDIAN)
> + {
> + /* Big-endian values sit at the right end of the buffer. In case of
> + registers whose size is smaller than sizeof (long), we must use a
> + padding to access it correctly. */
> + int padding = (sizeof (long) - register_size (regcache->tdesc, regno));
> + supply_register (regcache, regno, buf + padding);
> + }
> else
> - supply_register (regcache, regno, buf);
> + perror_with_name ("Unexpected byte order");
> }
Same applies for this chunk.
Otherwise, looks good (it's not an approval).
Cheers,
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/