PR gdb/22286: linux-nat-trad: Support arbitrary register widths

Message ID alpine.DEB.2.00.1805160319400.10896@tp.orcam.me.uk
State Committed
Headers

Commit Message

Maciej W. Rozycki May 16, 2018, 5:14 p.m. UTC
  Update `fetch_register' and `store_register' code to support arbitrary 
register widths rather than only ones that are a multiply of the size of 
the `ptrace' data type used with PTRACE_PEEKUSR and PTRACE_POKEUSR 
requests to access registers.  Remove associated assertions, correcting 
an issue with accessing the DSPControl (`$dspctl') register on n64 MIPS 
native targets:

(gdb) print /x $dspctl
.../gdb/linux-nat-trad.c:50: internal-error: void linux_nat_trad_target::fetch_register(regcache*, int): Assertion `(size % sizeof (PTRACE_TYPE_RET)) == 0' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) n

This is a bug, please report it.  For instructions, see:
<http://www.gnu.org/software/gdb/bugs/>.

.../gdb/linux-nat-trad.c:50: internal-error: void linux_nat_trad_target::fetch_register(regcache*, int): Assertion `(size % sizeof (PTRACE_TYPE_RET)) == 0' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Create a core file of GDB? (y or n) n
Command aborted.
(gdb)

All registers are now reported correctly and their architectural 
hardware widths respected:

(gdb) print /x $dspctl
$1 = 0x55aa33cc
(gdb) info registers
                  zero               at               v0               v1
 R0   0000000000000000 0000000000000001 000000fff7ffeb20 0000000000000000
                    a0               a1               a2               a3
 R4   0000000000000001 000000ffffffeaf8 000000ffffffeb08 0000000000000000
                    a4               a5               a6               a7
 R8   000000fff7ee3800 000000fff7ede8f0 000000ffffffeaf0 2f2f2f2f2f2f2f2f
                    t0               t1               t2               t3
 R12  0000000000000437 0000000000000002 000000fff7ffd000 0000000120000ad0
                    s0               s1               s2               s3
 R16  000000fff7ee2068 0000000120000e60 0000000000000000 0000000000000000
                    s4               s5               s6               s7
 R20  0000000000521ec8 0000000000522608 0000000000000000 0000000000000000
                    t8               t9               k0               k1
 R24  0000000000000000 0000000120000d9c 0000000000000000 0000000000000000
                    gp               sp               s8               ra
 R28  0000000120019030 000000ffffffe990 000000ffffffe990 000000fff7d5b88c
                status               lo               hi         badvaddr
      0000000000109cf3 0000000000005ea5 0000000000000211 000000fff7fc6fe0
                 cause               pc
      0000000000800024 0000000120000dbc
                  fcsr              fir              hi1              lo1
              00000000         00f30000 0000000000000000 0101010101010101
                   hi2              lo2              hi3              lo3
      0202020202020202 0303030303030303 0404040404040404 0505050505050505
                dspctl          restart
              55aa33cc 0000000000000000
(gdb)

NB due to the lack of access to 64-bit DSP hardware all DSP register 
values in the dumps are artificial and have been created with a debug 
change applied to the kernel handler of the `ptrace' syscall.

The use of `store_unsigned_integer' and `extract_unsigned_integer' 
unconditionally in all cases rather than when actual data occupies a 
part of the data quantity exchanged with `ptrace' makes code perhaps 
marginally slower, however I think avoiding it is not worth code 
obfuscation it would cause.  If this turns out unfounded, then there 
should be no problem with optimizing this code later.

	gdb/
	PR gdb/22286
	* linux-nat-trad.c (linux_nat_trad_target::fetch_register):
	Also handle registers whose width is not a multiple of
	PTRACE_TYPE_RET.
	(linux_nat_trad_target::store_register). Likewise.
---
Hi,

 This has been verified not to cause regressions with native o32 and n64 
MIPS/Linux targets, using a small debug patch to disable the use of the 
more modern `ptrace' PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS and 
PTRACE_SETFPREGS requests (by wiring `have_ptrace_regsets' to 0 rather 
than 1), so that the handlers modified here have been thoroughly tested.

 This in turn triggered many issues elsewhere, now handled, in GDB:
<https://patchwork.sourceware.org/patch/27272/>, and the Linux kernel:
<https://patchwork.kernel.org/patch/10398781/>, 
<https://patchwork.kernel.org/patch/10404237/>.

 Additionally DSP support is unusable for n64 due to 32-bit truncation at 
kernel context switching: <https://patchwork.kernel.org/patch/10402159/>, 
and neither is for n32, for the same reason as well as the lack of a 
64-bit `ptrace' interface to access DSP accumulators.  The latter problem 
can only be addressed by adding support for a new `ptrace' API, that is 
for a DSP regset: <https://patchwork.kernel.org/patch/10402171/>, which 
will also provide DSP state in core dumps across all the three of o32, n32 
and n64 Linux ABIs.  Implementing GDB support for DSP access through this 
new DSP regset will have to happen separately though.

 Meanwhile OK to apply?

  Maciej
---
 gdb/linux-nat-trad.c |   28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

gdb-inf-ptrace-fetch-store-register-uneven.diff
  

Comments

Pedro Alves May 16, 2018, 6:53 p.m. UTC | #1
On 05/16/2018 06:14 PM, Maciej W. Rozycki wrote:

>  Meanwhile OK to apply?

OK.

Thanks,
Pedro Alves
  
Maciej W. Rozycki May 16, 2018, 7:45 p.m. UTC | #2
On Wed, 16 May 2018, Pedro Alves wrote:

> >  Meanwhile OK to apply?
> 
> OK.

 Thanks.  Applied, with a minor formatting update.

  Maciej
  

Patch

Index: gdb/gdb/linux-nat-trad.c
===================================================================
--- gdb.orig/gdb/linux-nat-trad.c	2018-05-10 22:13:05.000000000 +0100
+++ gdb/gdb/linux-nat-trad.c	2018-05-10 23:42:58.720836631 +0100
@@ -29,9 +29,10 @@  void
 linux_nat_trad_target::fetch_register (struct regcache *regcache, int regnum)
 {
   struct gdbarch *gdbarch = regcache->arch ();
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR addr;
+  gdb_byte *buf;
   size_t size;
-  PTRACE_TYPE_RET *buf;
   pid_t pid;
   int i;
 
@@ -47,19 +48,22 @@  linux_nat_trad_target::fetch_register (s
   pid = get_ptrace_pid (regcache_get_ptid (regcache));
 
   size = register_size (gdbarch, regnum);
-  gdb_assert ((size % sizeof (PTRACE_TYPE_RET)) == 0);
-  buf = (PTRACE_TYPE_RET *) alloca (size);
+  buf = (gdb_byte *) alloca (size);
 
   /* Read the register contents from the inferior a chunk at a time.  */
-  for (i = 0; i < size / sizeof (PTRACE_TYPE_RET); i++)
+  for (i = 0; i < size; i += sizeof (PTRACE_TYPE_RET))
     {
+      size_t chunk = std::min (sizeof (PTRACE_TYPE_RET), size - i);
+      PTRACE_TYPE_RET val;
+
       errno = 0;
-      buf[i] = ptrace (PT_READ_U, pid, (PTRACE_TYPE_ARG3)(uintptr_t)addr, 0);
+      val = ptrace (PT_READ_U, pid, (PTRACE_TYPE_ARG3) (uintptr_t) addr, 0);
       if (errno != 0)
 	error (_("Couldn't read register %s (#%d): %s."),
 	       gdbarch_register_name (gdbarch, regnum),
 	       regnum, safe_strerror (errno));
 
+      store_unsigned_integer (buf + i, chunk, byte_order, val);
       addr += sizeof (PTRACE_TYPE_RET);
     }
   regcache_raw_supply (regcache, regnum, buf);
@@ -87,9 +91,10 @@  linux_nat_trad_target::store_register (c
 				       int regnum)
 {
   struct gdbarch *gdbarch = regcache->arch ();
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR addr;
   size_t size;
-  PTRACE_TYPE_RET *buf;
+  gdb_byte *buf;
   pid_t pid;
   int i;
 
@@ -102,15 +107,18 @@  linux_nat_trad_target::store_register (c
   pid = get_ptrace_pid (regcache_get_ptid (regcache));
 
   size = register_size (gdbarch, regnum);
-  gdb_assert ((size % sizeof (PTRACE_TYPE_RET)) == 0);
-  buf = (PTRACE_TYPE_RET *) alloca (size);
+  buf = (gdb_byte *) alloca (size);
 
   /* Write the register contents into the inferior a chunk at a time.  */
   regcache_raw_collect (regcache, regnum, buf);
-  for (i = 0; i < size / sizeof (PTRACE_TYPE_RET); i++)
+  for (i = 0; i < size; i += sizeof (PTRACE_TYPE_RET))
     {
+      size_t chunk = std::min (sizeof (PTRACE_TYPE_RET), size - i);
+      PTRACE_TYPE_RET val;
+
+      val = extract_unsigned_integer (buf + i, chunk, byte_order);
       errno = 0;
-      ptrace (PT_WRITE_U, pid, (PTRACE_TYPE_ARG3)(uintptr_t)addr, buf[i]);
+      ptrace (PT_WRITE_U, pid, (PTRACE_TYPE_ARG3) (uintptr_t) addr, val);
       if (errno != 0)
 	error (_("Couldn't write register %s (#%d): %s."),
 	       gdbarch_register_name (gdbarch, regnum),