public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 2/3] Read pseudo registers from frame instead of regcache
  2018-10-24  1:43 [PATCH v2 0/3] Read pseudo registers from frame instead of regcache Simon Marchi
@ 2018-10-24  1:43 ` Simon Marchi
  2019-02-08 16:53   ` John Baldwin
  2018-10-24  1:43 ` [PATCH v2 1/3] Extract register_reader and register_readwriter interfaces from regcache Simon Marchi
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2018-10-24  1:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand, Tom Tromey, Simon Marchi

From: Simon Marchi <simon.marchi@ericsson.com>

Reading pseudo registers (made of one or more raw register) from an
unwound stack frame (frame number > #0) does not work.  The raw
registers needed to compose the pseudo registers are always read from
the current thread's regcache, which is effectively frame #0.

Here's an example using the x86-64 test included in this patch (with
some output remove for brevity):

  (gdb) tb break_here_asm
  (gdb) r
  (gdb) info registers rbx ebx
  rbx            0x2021222324252627  2315169217770759719
  ebx            0x24252627          606414375
  (gdb) up
  (gdb) info registers rbx ebx
  rbx            0x1011121314151617  1157726452361532951
  ebx            0x24252627          606414375

We would expect the last ebx value to be 0x14151617, half of the rbx value in
the same frame.

The problem comes from the fact that the gdbarch hooks:

  - pseudo_register_read
  - pseudo_register_read_value
  - pseudo_register_write

receive a regcache as a parameter, as a way to get the values of the raw
registers required to compute the pseudo register value.  However, the
regcache object always contains the current register values, not the
values unwound to the frame we're interested in.  That's why ebx in
frame #1 appears to have the value it has in frame #0.

This patch introduces new implementations of the register_reader and
register_readwriter interfaces which access register values by unwinding
them from a particular frame.  It therefore allows computing pseudo
register values using the raw register values of that frame.

My initial implementation [1] made it so raw registers were always
unwound using the frame unwinder (generally goes to
dwarf2_frame_prev_register) and pseudo registers were always unwound
through gdbarch_pseudo_register_read.  The problem with this, as pointed
out by Ulrich, is that some DWARF info may contain unwind info for
pseudo registers.

An example of this are the S (single-precision) registers on ARM, where
two 32-bits S registers form one 64-bits D (double-precision) register.
Today, according to the "DWARF for the ARM architecture" document [2],
producers should describe the location of the saved D registers in the
DWARF info and the value of the S registers should be inferred from
them.  In GDB words, S are pseudo registers built from D, which are raw
registers.  So we would unwind D registers using the frame unwinder
(i.e. DWARF info) and unwind S registers using
gdbarch_pseudo_register_read, which will actually read the right D
register and derive the S register value from that.

However, before the introduction of the D registers, the S registers had
register numbers assigned (64-95).  So it's possible to find binaries in
the wild whose DWARF info have rules to unwind S registers.  Using my
initial approach described previously wouldn't work then.  When trying
to read an S register, we would try to read the matching D register, for
which we don't have any unwind information, and conclude its value
hasn't been saved.  We would miss the unwind info specific to the S
register, and end up with a wrong result.

So what this patch does is first try to unwind pseudo registers using
the frame unwinder, for both raw and pseudo registers.  If we
specifically have info for that register, great, we use that.
Typically, for pseudo registers, we won't find anything.  So we'll fall
back to going through gdbarch_pseudo_register_read.

[1] https://sourceware.org/ml/gdb-patches/2018-05/msg00705.html
[2] http://infocenter.arm.com/help/topic/com.arm.doc.ihi0040b/IHI0040B_aadwarf.pdf
---
 gdb/dwarf2-frame.c |  12 ++++-
 gdb/frame.c        | 122 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 124 insertions(+), 10 deletions(-)

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 2201c636590d..2b698fbf6caa 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -1290,8 +1290,16 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
 	 registers are actually undefined (which is different to CFI
 	 "undefined").  Code above issues a complaint about this.
 	 Here just fudge the books, assume GCC, and that the value is
-	 more inner on the stack.  */
-      return frame_unwind_got_register (this_frame, regnum, regnum);
+	 more inner on the stack.
+
+	 If the register is a pseudo one, it's possible that we don't have
+	 unwind info for it, but we have unwind info for the raw registers
+	 that compose it.  Return nullptr, so that frame_unwind_register_value
+	 attempts reconstructing the value from raw registers. */
+      if (regnum < gdbarch_num_regs (gdbarch))
+	return frame_unwind_got_register (this_frame, regnum, regnum);
+      else
+	return nullptr;
 
     case DWARF2_FRAME_REG_SAME_VALUE:
       return frame_unwind_got_register (this_frame, regnum, regnum);
diff --git a/gdb/frame.c b/gdb/frame.c
index 4624eada87b2..cd7149ee38b2 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1180,14 +1180,74 @@ get_frame_register (struct frame_info *frame,
   frame_unwind_register (frame->next, regnum, buf);
 }
 
+/* Implement the register_reader interface, but read the registers of a given
+   frame.  */
+
+struct frame_register_reader : public virtual register_reader
+{
+  frame_register_reader (frame_info *next_frame)
+  : m_next_frame (next_frame)
+  {}
+
+  gdbarch *arch () const override
+  {
+    return get_frame_arch (m_next_frame);
+  }
+
+  register_status raw_read (int regnum, gdb_byte *buf) override
+  {
+    return cooked_read (regnum, buf);
+  }
+
+  register_status cooked_read (int regnum, gdb_byte *buf) override
+  {
+    TRY
+      {
+	frame_unwind_register (m_next_frame, regnum, buf);
+	return REG_VALID;
+      }
+    CATCH (ex, RETURN_MASK_ALL)
+      {
+	return REG_UNAVAILABLE;
+      }
+    END_CATCH
+  }
+
+protected:
+  frame_info *m_next_frame;
+};
+
+/* Implement the register_readwriter interface, but read/write the registers of
+   a given frame.  */
+
+struct frame_register_readwriter : public frame_register_reader,
+				   public register_readwriter
+{
+  frame_register_readwriter (frame_info *next_frame)
+  : frame_register_reader (next_frame)
+  {}
+
+  void raw_write (int regnum, const gdb_byte *buf) override
+  {
+    cooked_write (regnum, buf);
+  }
+
+  void cooked_write (int regnum, const gdb_byte *buf) override
+  {
+    frame_info *this_frame = get_prev_frame (m_next_frame);
+    put_frame_register (this_frame, regnum, buf);
+  }
+};
+
 struct value *
 frame_unwind_register_value (frame_info *next_frame, int regnum)
 {
-  struct gdbarch *gdbarch;
-  struct value *value;
-
   gdb_assert (next_frame != NULL);
-  gdbarch = frame_unwind_arch (next_frame);
+
+  struct gdbarch *gdbarch = frame_unwind_arch (next_frame);
+
+  gdb_assert (regnum >= 0);
+  gdb_assert (regnum < gdbarch_num_cooked_regs (gdbarch));
 
   if (frame_debug)
     {
@@ -1203,9 +1263,41 @@ frame_unwind_register_value (frame_info *next_frame, int regnum)
     frame_unwind_find_by_frame (next_frame, &next_frame->prologue_cache);
 
   /* Ask this frame to unwind its register.  */
-  value = next_frame->unwind->prev_register (next_frame,
-					     &next_frame->prologue_cache,
-					     regnum);
+  struct value *value
+    = next_frame->unwind->prev_register (next_frame,
+					 &next_frame->prologue_cache, regnum);
+
+  if (value == nullptr)
+    {
+      frame_register_reader reg_read (next_frame);
+
+      if (gdbarch_pseudo_register_read_value_p (gdbarch))
+	{
+	  /* This is a pseudo register, we don't know how how what raw registers
+	     this pseudo register is made of.  Ask the gdbarch to read the
+	     value, it will itself ask the next frame to unwind the values of
+	     the raw registers it needs to compose the value of the pseudo
+	     register.  */
+	  value
+	    = gdbarch_pseudo_register_read_value (gdbarch, &reg_read, regnum);
+	  VALUE_LVAL (value) = not_lval;
+	}
+      else if (gdbarch_pseudo_register_read_p (gdbarch))
+	{
+	  value = allocate_value (register_type (gdbarch, regnum));
+	  VALUE_LVAL (value) = not_lval;
+
+	  register_status st
+	    = gdbarch_pseudo_register_read (gdbarch, &reg_read, regnum,
+					    value_contents_raw (value));
+	  if (st == REG_UNAVAILABLE)
+	    mark_value_bytes_unavailable (value, 0,
+					  TYPE_LENGTH (value_type (value)));
+	}
+      else
+	error (_("Can't unwind value of register %d (%s)"), regnum,
+	       user_reg_map_regnum_to_name (gdbarch, regnum));
+    }
 
   if (frame_debug)
     {
@@ -1353,10 +1445,14 @@ put_frame_register (struct frame_info *frame, int regnum,
   enum lval_type lval;
   CORE_ADDR addr;
 
+  gdb_assert (regnum >= 0);
+  gdb_assert (regnum < gdbarch_num_cooked_regs (gdbarch));
+
   frame_register (frame, regnum, &optim, &unavail,
 		  &lval, &addr, &realnum, NULL);
   if (optim)
     error (_("Attempt to assign to a register that was not saved."));
+
   switch (lval)
     {
     case lval_memory:
@@ -1368,7 +1464,17 @@ put_frame_register (struct frame_info *frame, int regnum,
       get_current_regcache ()->cooked_write (realnum, buf);
       break;
     default:
-      error (_("Attempt to assign to an unmodifiable value."));
+      {
+	if (regnum < gdbarch_num_regs (gdbarch))
+	  error (_("Attempt to assign to an unmodifiable value."));
+
+	frame_info *next_frame = get_next_frame_sentinel_okay (frame);
+	frame_register_readwriter frame_readwriter (next_frame);
+
+	/* This is a pseudo-register, the arch will find out which raw registers
+	   to modify and update them.  */
+	gdbarch_pseudo_register_write (gdbarch, &frame_readwriter, regnum, buf);
+      }
     }
 }
 
-- 
2.19.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 3/3] Add tests for unwinding of pseudo registers
  2018-10-24  1:43 [PATCH v2 0/3] Read pseudo registers from frame instead of regcache Simon Marchi
  2018-10-24  1:43 ` [PATCH v2 2/3] " Simon Marchi
  2018-10-24  1:43 ` [PATCH v2 1/3] Extract register_reader and register_readwriter interfaces from regcache Simon Marchi
@ 2018-10-24  1:43 ` Simon Marchi
  2019-02-08 13:47 ` [PATCH v2 0/3] Read pseudo registers from frame instead of regcache Simon Marchi
  2019-02-11 16:56 ` Ulrich Weigand
  4 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2018-10-24  1:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand, Tom Tromey, Simon Marchi

This patch adds tests to exercise the previous patch's changes.

All three tests:

* aarch64-pseudo-unwind
* amd64-pseudo-unwind
* arm-pseudo-unwind

follow the same pattern, just with different registers.

The other test, arm-pseudo-unwind-legacy, tests the special case where
the unwind information contains an entry for a register considered a
pseudo-register by GDB.

gdb/testsuite/ChangeLog:

	* gdb.arch/aarch64-pseudo-unwind-asm.S: New file.
	* gdb.arch/aarch64-pseudo-unwind.c: New file.
	* gdb.arch/aarch64-pseudo-unwind.exp: New file.
	* gdb.arch/amd64-pseudo-unwind-asm.S: New file.
	* gdb.arch/amd64-pseudo-unwind.c: New file.
	* gdb.arch/amd64-pseudo-unwind.exp: New file.
	* gdb.arch/arm-pseudo-unwind-asm.S: New file.
	* gdb.arch/arm-pseudo-unwind-legacy-asm.S: New file.
	* gdb.arch/arm-pseudo-unwind-legacy.c: New file.
	* gdb.arch/arm-pseudo-unwind-legacy.exp: New file.
	* gdb.arch/arm-pseudo-unwind.c: New file.
	* gdb.arch/arm-pseudo-unwind.exp: New file.
---
 .../gdb.arch/aarch64-pseudo-unwind-asm.S      | 78 ++++++++++++++++
 .../gdb.arch/aarch64-pseudo-unwind.c          | 33 +++++++
 .../gdb.arch/aarch64-pseudo-unwind.exp        | 89 ++++++++++++++++++
 .../gdb.arch/amd64-pseudo-unwind-asm.S        | 63 +++++++++++++
 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c  | 33 +++++++
 .../gdb.arch/amd64-pseudo-unwind.exp          | 90 +++++++++++++++++++
 .../gdb.arch/arm-pseudo-unwind-asm.S          | 76 ++++++++++++++++
 .../gdb.arch/arm-pseudo-unwind-legacy-asm.S   | 79 ++++++++++++++++
 .../gdb.arch/arm-pseudo-unwind-legacy.c       | 33 +++++++
 .../gdb.arch/arm-pseudo-unwind-legacy.exp     | 85 ++++++++++++++++++
 gdb/testsuite/gdb.arch/arm-pseudo-unwind.c    | 33 +++++++
 gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp  | 87 ++++++++++++++++++
 12 files changed, 779 insertions(+)
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind-asm.S
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.exp
 create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
 create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c
 create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.exp
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-asm.S
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy-asm.S
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.c
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.exp
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind.c
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp

diff --git a/gdb/testsuite/gdb.arch/aarch64-pseudo-unwind-asm.S b/gdb/testsuite/gdb.arch/aarch64-pseudo-unwind-asm.S
new file mode 100644
index 000000000000..6bd9d46bbfd8
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-pseudo-unwind-asm.S
@@ -0,0 +1,78 @@
+/* Copyright 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+value_callee:
+.quad 0x2021222324252627
+value_caller:
+.quad 0x1011121314151617
+
+.global callee
+callee:
+	/* Standard prologue:
+	    - push fp (x29) and lr (x30) to the stack.
+	    - mov sp to fp  */
+.cfi_startproc
+	stp x29, x30, [sp, -16]!
+.cfi_def_cfa 29, 16
+.cfi_offset 29, -16
+.cfi_offset 30, -8
+	mov x29, sp
+
+	/* Save caller's q8 value on the stack.  */
+.cfi_offset 72, -32
+	str q8, [sp, -16]!
+
+	/* Put our own q8 value.  */
+	adr x0, value_callee
+	ld1 { v8.1d }, [x0]
+break_here_asm:
+
+	/* Restore caller's q8 value.  */
+	ldr q8, [sp], 16
+	
+	/* Standard epilogue:
+	    - pop fo (x29) and lr (x30) from the stack  */
+	ldp x29, x30, [sp], 16
+	ret
+.cfi_endproc
+
+
+.global caller
+caller:
+	/* Standard prologue.  */
+.cfi_startproc
+	stp x29, x30, [sp, -16]!
+.cfi_def_cfa 29, 16
+.cfi_offset x29, -16
+.cfi_offset x30, -8
+	add x29, sp, 0
+
+	/* Put our own q8 value.  */
+	adr x0, value_caller
+	ld1 { v8.1d }, [x0]
+
+	/* Call callee.  */
+	bl callee
+
+	/* Store our q8 value in x0 to return it.  */
+	str q8, [sp, -16]!
+	ldr x0, [sp], 16
+
+	/* Standard epilogue.  */
+	ldp x29, x30, [sp], 16
+	ret
+.cfi_endproc
diff --git a/gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.c b/gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.c
new file mode 100644
index 000000000000..c4553f844e9f
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.c
@@ -0,0 +1,33 @@
+/* Copyright 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdint.h>
+
+uint64_t caller (void);
+
+static void
+break_here_c (uint64_t value)
+{
+}
+
+int
+main (void)
+{
+  uint64_t value = caller ();
+  break_here_c (value);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.exp b/gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.exp
new file mode 100644
index 000000000000..0f92319c8a20
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.exp
@@ -0,0 +1,89 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This test is equivalent to amd64-pseudo-unwind, but specific to AArch64.  We
+# use the raw register v8/q8 (it's the same register, but referred to differently
+# depending on the instruction) which is 128 bits long (although we only load
+# a 64-bits value, it's enough for the test).  We use pseudo register s8, which
+# is the low 32-bits of v8/q8.
+
+if { ![istarget aarch64-*-* ] } {
+    verbose "Skipping aarch64 pseudo register unwind."
+    return
+}
+
+standard_testfile aarch64-pseudo-unwind.c aarch64-pseudo-unwind-asm.S
+
+if { [prepare_for_testing "failed to prepare" ${testfile} "${srcfile} ${srcfile2}" {debug}] } {
+    return -1
+}
+
+clean_restart ${binfile}
+
+if ![runto_main] then {
+    fail "could not run to main"
+}
+
+gdb_breakpoint break_here_asm temporary
+gdb_continue_to_breakpoint "continue to callee"
+
+# Verify the value of v8/s8 in the inner frame (callee).
+with_test_prefix "callee, before change" {
+    gdb_test "p/x \$v8.q.u" " = \\{0x2021222324252627\\}"
+    gdb_test "p/x \$s8.u" " = 0x24252627"
+}
+
+# Verify that we can change the value of the pseudo register (s8) in the inner
+# frame (callee).
+gdb_test_no_output "set \$s8.u = 0x34353637"
+
+# Verify the value of v8/s8 in the inner frame (callee) after the change.
+with_test_prefix "callee, after change" {
+    gdb_test "p/x \$v8.q.u" " = \\{0x34353637\\}"
+    gdb_test "p/x \$s8.u" " = 0x34353637"
+}
+
+# Go up one frame (to caller) and do the same.
+gdb_test "up"
+
+# Verify the value of v8/s8 in the outer frame (caller).
+with_test_prefix "caller, before change" {
+    gdb_test "p/x \$v8.q.u" " = \\{0x1011121314151617\\}"
+    gdb_test "p/x \$s8.u" " = 0x14151617"
+}
+
+# Verify that we can change the value of the pseudo register (s8) in the outer
+# frame (caller).
+gdb_test_no_output "set \$s8.u = 0x44454647"
+
+# Verify the value of v8/s8 in the outer frame (caller) after the change.
+with_test_prefix "caller, after change" {
+    gdb_test "p/x \$v8.q.u" " = \\{0x44454647\\}"
+    gdb_test "p/x \$s8.u" " = 0x44454647"
+}
+
+# Go back to frame 0 (callee), check that the change to the outer frame didn't
+# mess up anything there.
+gdb_test "down"
+with_test_prefix "callee, after change in caller" {
+    gdb_test "p/x \$v8.q.u" " = \\{0x34353637\\}"
+    gdb_test "p/x \$s8.u" " = 0x34353637"
+}
+
+# Verify that the value of the saved v8 we changed is correctly seen by the
+# inferior.
+gdb_breakpoint break_here_c temporary
+gdb_continue_to_breakpoint "continue to break_here_c"
+gdb_test "p/x value" " = 0x44454647"
diff --git a/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S b/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
new file mode 100644
index 000000000000..316e8ca007f6
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
@@ -0,0 +1,63 @@
+/* Copyright 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+.global callee
+callee:
+	/* Standard prologue.  */
+.cfi_startproc
+	push %rbp
+.cfi_def_cfa rbp, 16
+	mov %rsp, %rbp
+
+	/* Save caller's rbx value on the stack.  */
+.cfi_offset rbx, -24
+	push %rbx
+
+	/* Put our own rbx value.  */
+	mov $0x2021222324252627, %rbx
+break_here_asm:
+
+	/* Restore caller's rbx value.  */
+	pop %rbx
+
+	/* Standard epilogue.  */
+	pop %rbp
+	ret
+.cfi_endproc
+
+
+.global caller
+caller:
+.cfi_startproc
+	/* Standard prologue.  */
+	push %rbp
+.cfi_def_cfa_offset 16
+	mov %rsp, %rbp
+
+	/* Put our own rbx value.  */
+	mov $0x1011121314151617, %rbx
+
+	/* Call callee.  */
+	call callee
+
+	/* Store our rbx value in rax to return it.  */
+	mov %rbx, %rax
+
+	/* Standard epilogue.  */
+	pop %rbp
+	ret
+.cfi_endproc
diff --git a/gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c b/gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c
new file mode 100644
index 000000000000..c4553f844e9f
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c
@@ -0,0 +1,33 @@
+/* Copyright 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdint.h>
+
+uint64_t caller (void);
+
+static void
+break_here_c (uint64_t value)
+{
+}
+
+int
+main (void)
+{
+  uint64_t value = caller ();
+  break_here_c (value);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-pseudo-unwind.exp b/gdb/testsuite/gdb.arch/amd64-pseudo-unwind.exp
new file mode 100644
index 000000000000..379b05c9d4fa
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-pseudo-unwind.exp
@@ -0,0 +1,90 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This test verifies that we can read and write the value of a pseudo register
+# in unwound frames.  For the test, we choose one raw register, rbx, and one
+# pseudo register that is backed by rbx, ebx.  We have two frames (the inner one,
+# #0 and the outer one, #1) that each set a value for rbx.  We verify that we
+# can read both rbx and ebx correctly for each frame, and that when we write to
+# ebx, rbx for that frame is correctly updated.
+
+if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
+    verbose "Skipping amd64 pseudo register unwind."
+    return
+}
+
+standard_testfile amd64-pseudo-unwind.c amd64-pseudo-unwind-asm.S
+
+if { [prepare_for_testing "failed to prepare" ${testfile} "${srcfile} ${srcfile2}" {debug}] } {
+    return -1
+}
+
+clean_restart ${binfile}
+
+if ![runto_main] then {
+    fail "could not run to main"
+}
+
+gdb_breakpoint break_here_asm temporary
+gdb_continue_to_breakpoint "continue to callee"
+
+# Verify the value of rbx/ebx in the inner frame (callee).
+with_test_prefix "callee, before change" {
+    gdb_test "p/x \$rbx" " = 0x2021222324252627"
+    gdb_test "p/x \$ebx" " = 0x24252627"
+}
+
+# Verify that we can change the value of the pseudo register (ebx) in the inner
+# frame (callee).
+gdb_test_no_output "set \$ebx = 0x34353637"
+
+# Verify the value of rbx/ebx in the inner frame (callee) after the change.
+with_test_prefix "callee, after change" {
+    gdb_test "p/x \$rbx" " = 0x2021222334353637"
+    gdb_test "p/x \$ebx" " = 0x34353637"
+}
+
+# Go up one frame, and do the same.
+gdb_test "up"
+
+# Verify the value of rbx/ebx in the outer frame (caller).
+with_test_prefix "caller, before change" {
+    gdb_test "p/x \$rbx" " = 0x1011121314151617"
+    gdb_test "p/x \$ebx" " = 0x14151617"
+}
+
+# Verify that we can change the value of the pseudo register (ebx) in the outer
+# frame (caller).
+gdb_test_no_output "set \$ebx = 0x44454647"
+
+# Verify the value of rbx/ebx in the outer frame (caller) after the change.
+with_test_prefix "caller, after change" {
+    gdb_test "p/x \$rbx" " = 0x1011121344454647"
+    gdb_test "p/x \$ebx" " = 0x44454647"
+}
+
+# Go back to frame 0 (callee), check that the change to the outer frame didn't
+# mess up anything there.
+gdb_test "down"
+with_test_prefix "callee, after change in caller" {
+    gdb_test "p/x \$rbx" " = 0x2021222334353637"
+    gdb_test "p/x \$ebx" " = 0x34353637"
+}
+
+# Verify that the value of the saved rbx we changed is correctly seen by the
+# inferior.
+gdb_breakpoint break_here_c temporary
+gdb_continue_to_breakpoint "continue to break_here_c"
+gdb_test "p/x value" " = 0x1011121344454647"
diff --git a/gdb/testsuite/gdb.arch/arm-pseudo-unwind-asm.S b/gdb/testsuite/gdb.arch/arm-pseudo-unwind-asm.S
new file mode 100644
index 000000000000..fe2346bf3cb2
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-pseudo-unwind-asm.S
@@ -0,0 +1,76 @@
+/* Copyright 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+value_callee:
+.quad 0x2021222324252627
+value_caller:
+.quad 0x1011121314151617
+
+.global callee
+callee:
+	/* Standard prologue.  */
+.cfi_startproc
+	push {fp, lr}
+.cfi_def_cfa fp, 4
+.cfi_offset fp, -8
+.cfi_offset lr, -4
+	add fp, sp, #4
+
+	/* Save caller's d8 value on the stack.  */
+.cfi_offset d8, -16
+	vpush {d8}
+
+	/* Put our own d8 value.  */
+	ldr r0, =value_callee
+	vldr d8, [r0]
+break_here_asm:
+
+	/* Restore caller's d8 value.  */
+	vpop {d8}
+	
+	/* Standard epilogue.  */
+	pop {fp, pc}
+.cfi_endproc
+
+
+.global caller
+caller:
+	/* Standard prologue.  */
+.cfi_startproc
+	push {fp, lr}
+.cfi_def_cfa fp, 4
+.cfi_offset fp, -8
+.cfi_offset lr, -4
+	add fp, sp, #4
+
+	/* Put our own d8 value.  */
+	ldr r0, =value_caller
+	vldr d8, [r0]
+
+	/* Call callee.  */
+	bl callee
+
+	/* Store our d8 value in r0-r1 to return it.  */
+	vpush {d8}
+	pop {r0}
+	pop {r1}
+
+	/* Standard epilogue.  */
+	pop {fp, pc}	
+.cfi_endproc
+
+
diff --git a/gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy-asm.S b/gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy-asm.S
new file mode 100644
index 000000000000..d8f383e53223
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy-asm.S
@@ -0,0 +1,79 @@
+/* Copyright 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* The difference between this and arm-pseudo-unwind is that here, the CFI
+   directives use the obsolete DWARF number for the s16 register (a
+   pseudo-register in GDB), whereas arm-pseudo-unwind uses the number for the d8
+   register (the underlying raw register for s16).  */
+
+value_callee:
+.quad 0x20212223
+value_caller:
+.quad 0x10111213
+
+.global callee
+callee:
+.cfi_startproc
+	/* Standard prologue.  */
+	push {fp, lr}
+.cfi_def_cfa fp, 4
+.cfi_offset fp, -8
+.cfi_offset lr, -4
+	add fp, sp, #4
+
+	/* Save caller's s16 value on the stack.  */
+.cfi_offset 80, -12
+	vpush {s16}
+
+	/* Put our own s16 value.  */
+	ldr r0, =value_callee
+	vldr s16, [r0]
+break_here_asm:
+
+	/* Restore caller's s16 value.  */
+	vpop {s16}
+	
+	/* Standard epilogue.  */
+	pop {fp, pc}
+.cfi_endproc
+
+
+.global caller
+caller:
+.cfi_startproc
+	/* Standard prologue.  */
+	push {fp, lr}
+.cfi_def_cfa fp, 4
+.cfi_offset fp, -8
+.cfi_offset lr, -4
+	add fp, sp, #4
+
+	/* Put our own s16 value.  */
+	ldr r0, =value_caller
+	vldr s16, [r0]
+
+	/* Call callee.  */
+	bl callee
+	
+	/* Store our s16 value in r0-r1 to return it.  */
+	vpush {s16}
+	pop {r0}
+	mov r1, #0
+
+	/* Standard epilogue.  */
+	pop {fp, pc}	
+.cfi_endproc
diff --git a/gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.c b/gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.c
new file mode 100644
index 000000000000..c4553f844e9f
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.c
@@ -0,0 +1,33 @@
+/* Copyright 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdint.h>
+
+uint64_t caller (void);
+
+static void
+break_here_c (uint64_t value)
+{
+}
+
+int
+main (void)
+{
+  uint64_t value = caller ();
+  break_here_c (value);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.exp b/gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.exp
new file mode 100644
index 000000000000..6deab8833b21
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.exp
@@ -0,0 +1,85 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This test is in the same vein as amd64-pseudo-unwind, making sure we can
+# read write pseudo registers in outer frames.  However, it tests a special
+# case where the debug info includes unwind information for a pseudo register
+# but not the underlying raw register.  This can happen for the pseudo register
+# s16, which is the bottom half of the raw register d8.
+#
+# See "DWARF for the ARM architecture":
+#   http://infocenter.arm.com/help/topic/com.arm.doc.ihi0040b/IHI0040B_aadwarf.pdf
+
+if { ![istarget arm*-*-* ] } {
+    verbose "Skipping arm pseudo register unwind."
+    return
+}
+
+standard_testfile arm-pseudo-unwind-legacy.c arm-pseudo-unwind-legacy-asm.S
+
+if { [prepare_for_testing "failed to prepare" ${testfile} "${srcfile} ${srcfile2}" {debug}] } {
+    return -1
+}
+
+clean_restart ${binfile}
+
+if ![runto_main] then {
+    fail "could not run to main"
+}
+
+gdb_breakpoint break_here_asm temporary
+gdb_continue_to_breakpoint "continue to callee"
+
+# Verify the value of s16 in the inner frame (callee).
+with_test_prefix "callee, before change" {
+    gdb_test "info registers s16" "raw 0x20212223\\)"
+}
+
+# Verify that we can change the value of s16 in the inner frame (callee).
+gdb_test_no_output "set \$s16 = 1.0"
+
+# Verify the value of s16 in the inner frame (callee) after the change.
+with_test_prefix "callee, after change" {
+    gdb_test "info registers s16" "raw 0x3f800000\\)"
+}
+
+# Go up one frame, and do the same
+gdb_test "up"
+
+# Verify the value of s16 in the outer frame (caller).
+with_test_prefix "caller, before change" {
+    gdb_test "info registers s16" "raw 0x10111213\\)"
+}
+
+# Verify that we can change the value of s16 in the outer frame (caller).
+gdb_test_no_output "set \$s16 = 2.0"
+
+# Verify the value of s16 in the outer frame (caller) after the change.
+with_test_prefix "caller, after change" {
+    gdb_test "info registers s16" "raw 0x40000000\\)"
+}
+
+# Go back to frame 0 (callee), check that the change to the outer frame didn't
+# mess up anything there.
+gdb_test "down"
+with_test_prefix "callee, after change in caller" {
+    gdb_test "info registers s16" "raw 0x3f800000\\)"
+}
+
+# Verify that the value of the saved s16 we changed is correctly seen by the
+# inferior.
+gdb_breakpoint break_here_c temporary
+gdb_continue_to_breakpoint "continue to break_here_c"
+gdb_test "p/x value" " = 0x40000000"
diff --git a/gdb/testsuite/gdb.arch/arm-pseudo-unwind.c b/gdb/testsuite/gdb.arch/arm-pseudo-unwind.c
new file mode 100644
index 000000000000..c4553f844e9f
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-pseudo-unwind.c
@@ -0,0 +1,33 @@
+/* Copyright 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdint.h>
+
+uint64_t caller (void);
+
+static void
+break_here_c (uint64_t value)
+{
+}
+
+int
+main (void)
+{
+  uint64_t value = caller ();
+  break_here_c (value);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp b/gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp
new file mode 100644
index 000000000000..cc64fab7aa39
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp
@@ -0,0 +1,87 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This test is equivalent to amd64-pseudo-unwind, but specific to ARM.  We
+# use the raw register d8 which is 64 bits long.  We use pseudo register s16,
+# which is the low 32 bits of d8.
+
+if { ![istarget arm*-*-* ] } {
+    verbose "Skipping arm pseudo register unwind."
+    return
+}
+
+standard_testfile arm-pseudo-unwind.c arm-pseudo-unwind-asm.S
+
+if { [prepare_for_testing "failed to prepare" ${testfile} "${srcfile} ${srcfile2}" {debug}] } {
+    return -1
+}
+
+clean_restart ${binfile}
+
+if ![runto_main] then {
+    fail "could not run to main"
+}
+
+gdb_breakpoint break_here_asm temporary
+gdb_continue_to_breakpoint "continue to callee"
+
+# Verify the value of d8/s16 in the inner frame (callee).
+with_test_prefix "callee, before change" {
+    gdb_test "p/x \$d8.u64" " = 0x2021222324252627"
+    gdb_test "info registers s16" "raw 0x24252627\\)"
+}
+
+# Verify that we can change the value of the pseudo register (s16) in the inner
+# frame (callee).
+gdb_test_no_output "set \$s16 = 1.0"
+
+# Verify the value of d8/s16 in the inner frame (callee) after the change.
+with_test_prefix "callee, after change" {
+    gdb_test "p/x \$d8.u64" " = 0x202122233f800000"
+    gdb_test "info registers s16" "raw 0x3f800000\\)"
+}
+
+# Go up one frame (to caller), and do the same.
+gdb_test "up"
+
+# Verify the value of d8/s16 in the outer frame (caller).
+with_test_prefix "caller, before change" {
+    gdb_test "p/x \$d8.u64" " = 0x1011121314151617"
+    gdb_test "info registers s16" "raw 0x14151617\\)"
+}
+
+# Verify that we can change the value of the pseudo register (s16) in the outer
+# frame (caller).
+gdb_test_no_output "set \$s16 = 2.0"
+
+# Verify the value of d8/s16 in the outer frame (caller) after the change.
+with_test_prefix "caller, after change" {
+    gdb_test "p/x \$d8.u64" " = 0x1011121340000000"
+    gdb_test "info registers s16" "raw 0x40000000\\)"
+}
+
+# Go back to frame 0 (callee), check that the change to the outer frame didn't
+# mess up anything there.
+gdb_test "down"
+with_test_prefix "callee, after change in caller" {
+    gdb_test "p/x \$d8.u64" " = 0x202122233f800000"
+    gdb_test "info registers s16" "raw 0x3f800000\\)"
+}
+
+# Verify that the value of the saved d8 we changed is correctly seen by the
+# inferior.
+gdb_breakpoint break_here_c temporary
+gdb_continue_to_breakpoint "continue to break_here_c"
+gdb_test "p/x value" " = 0x1011121340000000"
-- 
2.19.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/3] Extract register_reader and register_readwriter interfaces from regcache
  2018-10-24  1:43 [PATCH v2 0/3] Read pseudo registers from frame instead of regcache Simon Marchi
  2018-10-24  1:43 ` [PATCH v2 2/3] " Simon Marchi
@ 2018-10-24  1:43 ` Simon Marchi
  2019-02-08 16:47   ` John Baldwin
  2018-10-24  1:43 ` [PATCH v2 3/3] Add tests for unwinding of pseudo registers Simon Marchi
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2018-10-24  1:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand, Tom Tromey, Simon Marchi

From: Simon Marchi <simon.marchi@ericsson.com>

In the following patch, we make gdbarch pseudo-registers hooks read and
write required registers from a frame instead of from the current
regcache.  To avoid having to change the implementation of all
architectures to use a different interface, we can re-use the regcache
interface.

This patch extracts two interfaces, register_reader and
register_readwriter, and make respectively readable_regcache and
regcache inherit from them.  register_reader is "something from which
you can read register values from".  It can of course be a regcache, but
it will also be (in the following patch) something that unwinds
registers for a particular frame.  As you would expect,
register_readwriter is "something you can read and write registers
from/to".

Some notes about the implementation.  This results in diamond
inheritance: regcache inherits from both readable_regcache and
register_readwriter, which both inherit from register_reader.  It is
therefore necessary to use virtual inheritance (use "public virtual"),
otherwises we end up with errors like:

  /home/emaisin/src/binutils-gdb/gdb/regcache.c:210:20: error: request
  for member ‘cooked_read’ is ambiguous

Also, the raw_read method in readable_regcache hides the raw_read
template method in register_reader.  So it's necessary to use "using
register_reader::raw_read" so that clients of readable_regcache are able
to call register_reader's raw_read.  Same thing for some cooked_read,
raw_write and cooked_write.

All corresponding gdbarch hooks are updated to use register_reader or
register_readwriter instead of readable_regcache and regcache, but
otherwise they are not changed.

With this patch, no behavior change is expected.

gdb/ChangeLog:

	* regcache.h (struct register_reader,
	struct register_readwriter): Forward-declare.
	(regcache_raw_read_signed, regcache_raw_write_signed,
	regcache_raw_write_unsigned, regcache_cooked_read_signed,
	regcache_cooked_read_unsigned, regcache_cooked_write_signed,
	regcache_cooked_write_unsigned): Use
	register_reader/register_readwriter.
	(struct register_reader, struct register_readwriter): New.
	(class readable_regcache): Extend register_reader.
	<arch>: New.
	<raw_read (int regnum, gdb_byte *buf)>: Override parent method.
	<raw_read (int regnum, T *val)>: Move to register_reader.
	<cooked_read (int regnum, gdb_byte *buf)>: Override parent
	method.
	<cooked_read (int regnum, T *val)>: Move to register_reader.
	<cooked_read_part, cooked_read_value, read_part>: Move to
	register_reader.
	<raw_update>: New.
	(class regcache): Extend register_readwriter.
	<arch, invalidate>: New.
	<raw_write (int regnum, const gdb_byte *buf)>: Override parent
	method.
	<raw_write (int regnum, T val)>: Move to register_readwriter.
	<cooked_write (int regnum, const gdb_byte *buf)>: Override
	parent method.
	<cooked_write (int regnum, T val)>: Move to register_readwriter.
	<raw_write_part, cooked_write_parti, write_part>: Move to
	register_readwriter.
	* regcache.c (readable_regcache::raw_read): Move asserts here.
	Rename other overload to ...
	(register_reader::raw_read): ... this, adjust.
	(regcache_raw_read_signed): Change regcache parameter to
	readable_regcache.
	(regcache_raw_write_signed): Change regcache parameter to
	register_readwriter.
	(regcache::raw_write): Rename to ...
	(register_readwriter::raw_write): ... this, adjust.
	(regcache_raw_write_unsigned): Change regcache parameter to
	register_readwriter.
	(readable_regcache::cooked_read_value): Rename to ...
	(register_reader::cooked_read_value): ... this, and adjust.
	(regcache_cooked_read_signed): Change regcache parameter type to
	register_reader.
	(readable_regcache::cooked_read): Rename to ...
	(register_reader::cooked_read): ... this, adjust.
	(regcache_cooked_read_unsigned): Replace regcache parameter type
	to register_reader.
	(regcache_cooked_write_signed): Replace regcache parameter type
	to register_readwriter.
	(regcache::cooked_write): Rename to ...
	(register_readwriter::cooked_write): ... this, adjust.
	(regcache_cooked_write_unsigned): Replace regcache parameter
	type to register_readwriter.
	(readable_regcache::read_part): Rename to ...
	(register_reader::read_part): ... this.
	(regcache::write_part): Rename to ...
	(register_readwriter::write_part): ... this.
	(readable_regcache::raw_read_part): Rename to ...
	(register_reader::raw_read_part): ... this, adjust.
	(regcache::raw_write_part): Rename to ...
	(register_readwriter::raw_write_part): ... this, adjust.
	(readable_regcache::cooked_read_part): Rename to ...
	(register_reader::cooked_read_part): ... this, adjust.
	(regcache::cooked_write_part): Rename to ...
	(register_readwriter::cooked_write_part): ... this, adjust.
	(reg_buffer::num_raw_registers):
	* aarch64-tdep.c (aarch64_pseudo_read_value_1): Use
	register_reader/register_readwriter.
	(aarch64_pseudo_read_value): Likewise.
	(aarch64_pseudo_write_1): Likewise.
	(aarch64_pseudo_write): Likewise.
	* amd64-tdep.c (amd64_pseudo_register_read_value): Likewise.
	(amd64_pseudo_register_write): Likewise.
	* arm-tdep.c (arm_neon_quad_read): Likewise.
	(arm_neon_quad_write): Likewise.
	(arm_pseudo_read): Likewise.
	(arm_pseudo_write): Likewise.
	* avr-tdep.c (avr_pseudo_register_read): Likewise.
	(avr_pseudo_register_write): Likewise.
	* bfin-tdep.c (bfin_pseudo_register_read): Likewise.
	(bfin_pseudo_register_write): Likewise.
	* frv-tdep.c (frv_pseudo_register_read): Likewise.
	(frv_pseudo_register_write): Likewise.
	* gdbarch.sh (gdbarch_pseudo_register_read,
	gdbarch_pseudo_register_read_value): Use register_reader.
	(gdbarch_pseudo_register_write): Use register_readwriter.
	* gdbarch.c: Re-generate.
	* gdbarch.h: Re-generate.
	* h8300-tdep.c (pseudo_from_raw_register): Likewise.
	(raw_from_pseudo_register): Likewise.
	(h8300_pseudo_register_read): Likewise.
	(h8300_pseudo_register_write): Likewise.
	* hppa-tdep.c (hppa_pseudo_register_read): Likewise.
	* i386-tdep.c (i386_mmx_regnum_to_fp_regnum): Likewise.
	(i386_pseudo_register_read_into_value): Likewise.
	(i386_pseudo_register_read_value): Likewise.
	(i386_pseudo_register_write): Likewise.
	* i386-tdep.h (i386_pseudo_register_read_into_value): Likewise.
	(i386_pseudo_register_write): Likewise.
	* ia64-libunwind-tdep.c (libunwind_get_reg_special): Likewise.
	* ia64-libunwind-tdep.h (libunwind_get_reg_special): Likewise.
	* ia64-tdep.c (ia64_pseudo_register_read): Likewise.
	(ia64_pseudo_register_write): Likewise.
	* m32c-tdep.c (register_status): Likewise.
	(m32c_raw_read): Likewise.
	(m32c_raw_write): Likewise.
	(m32c_read_flg): Likewise.
	(m32c_banked_register): Likewise.
	(m32c_banked_read): Likewise.
	(m32c_banked_write): Likewise.
	(m32c_sb_read): Likewise.
	(m32c_sb_write): Likewise.
	(m32c_part_read): Likewise.
	(m32c_part_write): Likewise.
	(m32c_cat_read): Likewise.
	(m32c_cat_write): Likewise.
	(m32c_r3r2r1r0_read): Likewise.
	(m32c_r3r2r1r0_write): Likewise.
	(m32c_pseudo_register_read): Likewise.
	(m32c_pseudo_register_write): Likewise.
	* m68hc11-tdep.c (m68hc11_pseudo_register_read): Likewise.
	(m68hc11_pseudo_register_write): Likewise.
	* mep-tdep.c (mep_pseudo_cr32_read): Likewise.
	(mep_pseudo_cr64_read): Likewise.
	(mep_pseudo_register_read): Likewise.
	(mep_pseudo_csr_write): Likewise.
	(mep_pseudo_cr32_write): Likewise.
	(mep_pseudo_cr64_write): Likewise.
	(mep_pseudo_register_write): Likewise.
	* mips-tdep.c (mips_pseudo_register_read): Likewise.
	(mips_pseudo_register_write): Likewise.
	* msp430-tdep.c (msp430_pseudo_register_read): Likewise.
	(msp430_pseudo_register_write): Likewise.
	* nds32-tdep.c (nds32_pseudo_register_read): Likewise.
	(nds32_pseudo_register_write): Likewise.
	* rl78-tdep.c (rl78_pseudo_register_read): Likewise.
	(rl78_pseudo_register_write): Likewise.
	* rs6000-tdep.c (e500_move_ev_register): Likewise.
	(do_regcache_raw_write): Likewise.
	(e500_pseudo_register_read): Likewise.
	(e500_pseudo_register_write): Likewise.
	(dfp_pseudo_register_read): Likewise.
	(dfp_pseudo_register_write): Likewise.
	(vsx_pseudo_register_read): Likewise.
	(vsx_pseudo_register_write): Likewise.
	(efpr_pseudo_register_read): Likewise.
	(efpr_pseudo_register_write): Likewise.
	(rs6000_pseudo_register_read): Likewise.
	(rs6000_pseudo_register_write): Likewise.
	* s390-tdep.c (s390_pseudo_register_read): Likewise.
	(s390_pseudo_register_write): Likewise.
	* sh-tdep.c (pseudo_register_read_portions): Likewise.
	(sh_pseudo_register_read): Likewise.
	(sh_pseudo_register_write): Likewise.
	* sparc-tdep.c (sparc32_pseudo_register_read): Likewise.
	(sparc32_pseudo_register_write): Likewise.
	* sparc64-tdep.c (sparc64_pseudo_register_read): Likewise.
	(sparc64_pseudo_register_write): Likewise.
	* spu-tdep.c (spu_pseudo_register_read_spu): Likewise.
	(spu_pseudo_register_read): Likewise.
	(spu_pseudo_register_write_spu): Likewise.
	(spu_pseudo_register_write): Likewise.
	* xtensa-tdep.c (xtensa_register_write_masked): Likewise.
	(xtensa_register_read_masked): Likewise.
	(xtensa_pseudo_register_read): Likewise.
	(xtensa_pseudo_register_write): Likewise.
---
 gdb/aarch64-tdep.c        |  12 +--
 gdb/amd64-tdep.c          |   4 +-
 gdb/arm-tdep.c            |  12 +--
 gdb/avr-tdep.c            |   4 +-
 gdb/bfin-tdep.c           |   5 +-
 gdb/frv-tdep.c            |   7 +-
 gdb/gdbarch.c             |  12 +--
 gdb/gdbarch.h             |  12 +--
 gdb/gdbarch.sh            |   6 +-
 gdb/h8300-tdep.c          |   9 ++-
 gdb/hppa-tdep.c           |   2 +-
 gdb/i386-tdep.c           |   9 ++-
 gdb/i386-tdep.h           |   4 +-
 gdb/ia64-libunwind-tdep.c |   2 +-
 gdb/ia64-libunwind-tdep.h |   2 +-
 gdb/ia64-tdep.c           |   5 +-
 gdb/m32c-tdep.c           |  38 ++++-----
 gdb/m68hc11-tdep.c        |   4 +-
 gdb/mep-tdep.c            |  20 ++---
 gdb/mips-tdep.c           |   4 +-
 gdb/msp430-tdep.c         |   4 +-
 gdb/nds32-tdep.c          |   4 +-
 gdb/regcache.c            | 118 ++++++++++++++--------------
 gdb/regcache.h            | 161 ++++++++++++++++++++++++--------------
 gdb/rl78-tdep.c           |   8 +-
 gdb/rs6000-tdep.c         |  34 ++++----
 gdb/s390-tdep.c           |  11 +--
 gdb/sh-tdep.c             |   6 +-
 gdb/sparc-tdep.c          |   4 +-
 gdb/sparc64-tdep.c        |   6 +-
 gdb/spu-tdep.c            |  14 ++--
 gdb/xtensa-tdep.c         |  14 ++--
 32 files changed, 306 insertions(+), 251 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 023e8eb45393..c0c67aa211da 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2352,9 +2352,9 @@ aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
 /* Helper for aarch64_pseudo_read_value.  */
 
 static struct value *
-aarch64_pseudo_read_value_1 (struct gdbarch *gdbarch,
-			     readable_regcache *regcache, int regnum_offset,
-			     int regsize, struct value *result_value)
+aarch64_pseudo_read_value_1 (struct gdbarch *gdbarch, register_reader *regcache,
+			     int regnum_offset, int regsize,
+			     struct value *result_value)
 {
   unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
 
@@ -2374,7 +2374,7 @@ aarch64_pseudo_read_value_1 (struct gdbarch *gdbarch,
 /* Implement the "pseudo_register_read_value" gdbarch method.  */
 
 static struct value *
-aarch64_pseudo_read_value (struct gdbarch *gdbarch, readable_regcache *regcache,
+aarch64_pseudo_read_value (struct gdbarch *gdbarch, register_reader *regcache,
 			   int regnum)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -2422,7 +2422,7 @@ aarch64_pseudo_read_value (struct gdbarch *gdbarch, readable_regcache *regcache,
 /* Helper for aarch64_pseudo_write.  */
 
 static void
-aarch64_pseudo_write_1 (struct gdbarch *gdbarch, struct regcache *regcache,
+aarch64_pseudo_write_1 (struct gdbarch *gdbarch, register_readwriter *regcache,
 			int regnum_offset, int regsize, const gdb_byte *buf)
 {
   unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
@@ -2444,7 +2444,7 @@ aarch64_pseudo_write_1 (struct gdbarch *gdbarch, struct regcache *regcache,
 /* Implement the "pseudo_register_write" gdbarch method.  */
 
 static void
-aarch64_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
+aarch64_pseudo_write (struct gdbarch *gdbarch, register_readwriter *regcache,
 		      int regnum, const gdb_byte *buf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index abf3e4d91904..1c6a1e6dfb27 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -349,7 +349,7 @@ amd64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
 
 static struct value *
 amd64_pseudo_register_read_value (struct gdbarch *gdbarch,
-				  readable_regcache *regcache,
+				  register_reader *regcache,
 				  int regnum)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -409,7 +409,7 @@ amd64_pseudo_register_read_value (struct gdbarch *gdbarch,
 
 static void
 amd64_pseudo_register_write (struct gdbarch *gdbarch,
-			     struct regcache *regcache,
+			     register_readwriter *regcache,
 			     int regnum, const gdb_byte *buf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 53eee7692632..b9f546d047dc 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -227,10 +227,10 @@ static void show_disassembly_style_sfunc (struct ui_file *, int,
 					  const char *);
 
 static enum register_status arm_neon_quad_read (struct gdbarch *gdbarch,
-						readable_regcache *regcache,
+						register_reader *regs,
 						int regnum, gdb_byte *buf);
 static void arm_neon_quad_write (struct gdbarch *gdbarch,
-				 struct regcache *regcache,
+				 register_readwriter *regcache,
 				 int regnum, const gdb_byte *buf);
 
 static CORE_ADDR
@@ -8665,7 +8665,7 @@ arm_write_pc (struct regcache *regcache, CORE_ADDR pc)
    the quad register, in [0, 15].  */
 
 static enum register_status
-arm_neon_quad_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+arm_neon_quad_read (struct gdbarch *gdbarch, register_reader *regcache,
 		    int regnum, gdb_byte *buf)
 {
   char name_buf[4];
@@ -8698,7 +8698,7 @@ arm_neon_quad_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 }
 
 static enum register_status
-arm_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+arm_pseudo_read (struct gdbarch *gdbarch, register_reader *regcache,
 		 int regnum, gdb_byte *buf)
 {
   const int num_regs = gdbarch_num_regs (gdbarch);
@@ -8744,7 +8744,7 @@ arm_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
    of the quad register, in [0, 15].  */
 
 static void
-arm_neon_quad_write (struct gdbarch *gdbarch, struct regcache *regcache,
+arm_neon_quad_write (struct gdbarch *gdbarch, register_readwriter *regcache,
 		     int regnum, const gdb_byte *buf)
 {
   char name_buf[4];
@@ -8766,7 +8766,7 @@ arm_neon_quad_write (struct gdbarch *gdbarch, struct regcache *regcache,
 }
 
 static void
-arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
+arm_pseudo_write (struct gdbarch *gdbarch, register_readwriter *regcache,
 		  int regnum, const gdb_byte *buf)
 {
   const int num_regs = gdbarch_num_regs (gdbarch);
diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index 9e14007fc023..3a7ebdbdc4d6 100644
--- a/gdb/avr-tdep.c
+++ b/gdb/avr-tdep.c
@@ -383,7 +383,7 @@ avr_write_pc (struct regcache *regcache, CORE_ADDR val)
 }
 
 static enum register_status
-avr_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+avr_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
                           int regnum, gdb_byte *buf)
 {
   ULONGEST val;
@@ -404,7 +404,7 @@ avr_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 }
 
 static void
-avr_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
+avr_pseudo_register_write (struct gdbarch *gdbarch, register_readwriter *regcache,
                            int regnum, const gdb_byte *buf)
 {
   ULONGEST val;
diff --git a/gdb/bfin-tdep.c b/gdb/bfin-tdep.c
index c84625c89480..855ae52ccf4d 100644
--- a/gdb/bfin-tdep.c
+++ b/gdb/bfin-tdep.c
@@ -688,7 +688,7 @@ bfin_register_name (struct gdbarch *gdbarch, int i)
 }
 
 static enum register_status
-bfin_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+bfin_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
 			   int regnum, gdb_byte *buffer)
 {
   gdb_byte buf[BFIN_MAX_REGISTER_SIZE];
@@ -709,7 +709,8 @@ bfin_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 }
 
 static void
-bfin_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
+bfin_pseudo_register_write (struct gdbarch *gdbarch,
+			    register_readwriter *regcache,
 			    int regnum, const gdb_byte *buffer)
 {
   gdb_byte buf[BFIN_MAX_REGISTER_SIZE];
diff --git a/gdb/frv-tdep.c b/gdb/frv-tdep.c
index dafab756543b..ee388fe74d27 100644
--- a/gdb/frv-tdep.c
+++ b/gdb/frv-tdep.c
@@ -295,7 +295,7 @@ frv_register_type (struct gdbarch *gdbarch, int reg)
 }
 
 static enum register_status
-frv_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+frv_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
                           int reg, gdb_byte *buffer)
 {
   enum register_status status;
@@ -332,8 +332,9 @@ frv_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 }
 
 static void
-frv_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
-                          int reg, const gdb_byte *buffer)
+frv_pseudo_register_write (struct gdbarch *gdbarch,
+			   register_readwriter *regcache,
+			   int reg, const gdb_byte *buffer)
 {
   if (reg == iacc0_regnum)
     {
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index e2abf263b3a9..c1ab53c5793b 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -1959,13 +1959,13 @@ gdbarch_pseudo_register_read_p (struct gdbarch *gdbarch)
 }
 
 enum register_status
-gdbarch_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum, gdb_byte *buf)
+gdbarch_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regs, int cookednum, gdb_byte *buf)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->pseudo_register_read != NULL);
   if (gdbarch_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "gdbarch_pseudo_register_read called\n");
-  return gdbarch->pseudo_register_read (gdbarch, regcache, cookednum, buf);
+  return gdbarch->pseudo_register_read (gdbarch, regs, cookednum, buf);
 }
 
 void
@@ -1983,13 +1983,13 @@ gdbarch_pseudo_register_read_value_p (struct gdbarch *gdbarch)
 }
 
 struct value *
-gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum)
+gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, register_reader *regs, int cookednum)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->pseudo_register_read_value != NULL);
   if (gdbarch_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "gdbarch_pseudo_register_read_value called\n");
-  return gdbarch->pseudo_register_read_value (gdbarch, regcache, cookednum);
+  return gdbarch->pseudo_register_read_value (gdbarch, regs, cookednum);
 }
 
 void
@@ -2007,13 +2007,13 @@ gdbarch_pseudo_register_write_p (struct gdbarch *gdbarch)
 }
 
 void
-gdbarch_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum, const gdb_byte *buf)
+gdbarch_pseudo_register_write (struct gdbarch *gdbarch, register_readwriter *regs, int cookednum, const gdb_byte *buf)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->pseudo_register_write != NULL);
   if (gdbarch_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "gdbarch_pseudo_register_write called\n");
-  gdbarch->pseudo_register_write (gdbarch, regcache, cookednum, buf);
+  gdbarch->pseudo_register_write (gdbarch, regs, cookednum, buf);
 }
 
 void
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 2cb69610837d..79bfe5df5651 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -262,8 +262,8 @@ extern void set_gdbarch_virtual_frame_pointer (struct gdbarch *gdbarch, gdbarch_
 
 extern int gdbarch_pseudo_register_read_p (struct gdbarch *gdbarch);
 
-typedef enum register_status (gdbarch_pseudo_register_read_ftype) (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum, gdb_byte *buf);
-extern enum register_status gdbarch_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum, gdb_byte *buf);
+typedef enum register_status (gdbarch_pseudo_register_read_ftype) (struct gdbarch *gdbarch, register_reader *regs, int cookednum, gdb_byte *buf);
+extern enum register_status gdbarch_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regs, int cookednum, gdb_byte *buf);
 extern void set_gdbarch_pseudo_register_read (struct gdbarch *gdbarch, gdbarch_pseudo_register_read_ftype *pseudo_register_read);
 
 /* Read a register into a new struct value.  If the register is wholly
@@ -273,14 +273,14 @@ extern void set_gdbarch_pseudo_register_read (struct gdbarch *gdbarch, gdbarch_p
 
 extern int gdbarch_pseudo_register_read_value_p (struct gdbarch *gdbarch);
 
-typedef struct value * (gdbarch_pseudo_register_read_value_ftype) (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum);
-extern struct value * gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum);
+typedef struct value * (gdbarch_pseudo_register_read_value_ftype) (struct gdbarch *gdbarch, register_reader *regs, int cookednum);
+extern struct value * gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, register_reader *regs, int cookednum);
 extern void set_gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, gdbarch_pseudo_register_read_value_ftype *pseudo_register_read_value);
 
 extern int gdbarch_pseudo_register_write_p (struct gdbarch *gdbarch);
 
-typedef void (gdbarch_pseudo_register_write_ftype) (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum, const gdb_byte *buf);
-extern void gdbarch_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum, const gdb_byte *buf);
+typedef void (gdbarch_pseudo_register_write_ftype) (struct gdbarch *gdbarch, register_readwriter *regs, int cookednum, const gdb_byte *buf);
+extern void gdbarch_pseudo_register_write (struct gdbarch *gdbarch, register_readwriter *regs, int cookednum, const gdb_byte *buf);
 extern void set_gdbarch_pseudo_register_write (struct gdbarch *gdbarch, gdbarch_pseudo_register_write_ftype *pseudo_register_write);
 
 extern int gdbarch_num_regs (struct gdbarch *gdbarch);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index bbfa8d220583..ca413333bc68 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -426,13 +426,13 @@ F;void;write_pc;struct regcache *regcache, CORE_ADDR val;regcache, val
 # serious shakedown.
 m;void;virtual_frame_pointer;CORE_ADDR pc, int *frame_regnum, LONGEST *frame_offset;pc, frame_regnum, frame_offset;0;legacy_virtual_frame_pointer;;0
 #
-M;enum register_status;pseudo_register_read;readable_regcache *regcache, int cookednum, gdb_byte *buf;regcache, cookednum, buf
+M;enum register_status;pseudo_register_read;register_reader *regs, int cookednum, gdb_byte *buf;regs, cookednum, buf
 # Read a register into a new struct value.  If the register is wholly
 # or partly unavailable, this should call mark_value_bytes_unavailable
 # as appropriate.  If this is defined, then pseudo_register_read will
 # never be called.
-M;struct value *;pseudo_register_read_value;readable_regcache *regcache, int cookednum;regcache, cookednum
-M;void;pseudo_register_write;struct regcache *regcache, int cookednum, const gdb_byte *buf;regcache, cookednum, buf
+M;struct value *;pseudo_register_read_value;register_reader *regs, int cookednum;regs, cookednum
+M;void;pseudo_register_write;register_readwriter *regs, int cookednum, const gdb_byte *buf;regs, cookednum, buf
 #
 v;int;num_regs;;;0;-1
 # This macro gives the number of pseudo-registers that live in the
diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index 2334582260d3..b3b4de590043 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -1159,7 +1159,7 @@ h8300_register_type (struct gdbarch *gdbarch, int regno)
    raw registers.  These helpers extend/narrow the values.  */
 
 static enum register_status
-pseudo_from_raw_register (struct gdbarch *gdbarch, readable_regcache *regcache,
+pseudo_from_raw_register (struct gdbarch *gdbarch, register_reader *regcache,
 			  gdb_byte *buf, int pseudo_regno, int raw_regno)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1177,7 +1177,8 @@ pseudo_from_raw_register (struct gdbarch *gdbarch, readable_regcache *regcache,
 /* See pseudo_from_raw_register.  */
 
 static void
-raw_from_pseudo_register (struct gdbarch *gdbarch, struct regcache *regcache,
+raw_from_pseudo_register (struct gdbarch *gdbarch,
+			  register_readwriter *regcache,
 			  const gdb_byte *buf, int raw_regno, int pseudo_regno)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1190,7 +1191,7 @@ raw_from_pseudo_register (struct gdbarch *gdbarch, struct regcache *regcache,
 
 static enum register_status
 h8300_pseudo_register_read (struct gdbarch *gdbarch,
-			    readable_regcache *regcache, int regno,
+			    register_reader *regcache, int regno,
 			    gdb_byte *buf)
 {
   if (regno == E_PSEUDO_CCR_REGNUM (gdbarch))
@@ -1209,7 +1210,7 @@ h8300_pseudo_register_read (struct gdbarch *gdbarch,
 
 static void
 h8300_pseudo_register_write (struct gdbarch *gdbarch,
-			     struct regcache *regcache, int regno,
+			     register_readwriter *regcache, int regno,
 			     const gdb_byte *buf)
 {
   if (regno == E_PSEUDO_CCR_REGNUM (gdbarch))
diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 319096e056f2..1635a6df9500 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -2738,7 +2738,7 @@ hppa_fetch_pointer_argument (struct frame_info *frame, int argi,
 }
 
 static enum register_status
-hppa_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+hppa_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
 			   int regnum, gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a34a3374a4d7..30ef278d6370 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3249,7 +3249,7 @@ i386_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
    the MMX registers need to be mapped onto floating point registers.  */
 
 static int
-i386_mmx_regnum_to_fp_regnum (readable_regcache *regcache, int regnum)
+i386_mmx_regnum_to_fp_regnum (register_reader *regcache, int regnum)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
   int mmxreg, fpreg;
@@ -3270,7 +3270,7 @@ i386_mmx_regnum_to_fp_regnum (readable_regcache *regcache, int regnum)
 
 void
 i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
-				      readable_regcache *regcache,
+				      register_reader *regcache,
 				      int regnum,
 				      struct value *result_value)
 {
@@ -3449,7 +3449,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
 
 static struct value *
 i386_pseudo_register_read_value (struct gdbarch *gdbarch,
-				 readable_regcache *regcache,
+				 register_reader *regcache,
 				 int regnum)
 {
   struct value *result;
@@ -3464,7 +3464,8 @@ i386_pseudo_register_read_value (struct gdbarch *gdbarch,
 }
 
 void
-i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
+i386_pseudo_register_write (struct gdbarch *gdbarch,
+			    register_readwriter *regcache,
 			    int regnum, const gdb_byte *buf)
 {
   gdb_byte raw_buf[I386_MAX_REGISTER_SIZE];
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 81a93f11af5a..929ba7512807 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -364,12 +364,12 @@ extern struct type *i386_pseudo_register_type (struct gdbarch *gdbarch,
 					       int regnum);
 
 extern void i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
-						  readable_regcache *regcache,
+						  register_reader *regcache,
 						  int regnum,
 						  struct value *result);
 
 extern void i386_pseudo_register_write (struct gdbarch *gdbarch,
-					struct regcache *regcache,
+					register_readwriter *regcache,
 					int regnum, const gdb_byte *buf);
 
 extern int i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
diff --git a/gdb/ia64-libunwind-tdep.c b/gdb/ia64-libunwind-tdep.c
index f6d954abdcac..7959e692af95 100644
--- a/gdb/ia64-libunwind-tdep.c
+++ b/gdb/ia64-libunwind-tdep.c
@@ -451,7 +451,7 @@ libunwind_sigtramp_frame_sniffer (const struct frame_unwind *self,
    are usually located at BOF, this is not always true and only the libunwind
    info can decipher where they actually are.  */
 int
-libunwind_get_reg_special (struct gdbarch *gdbarch, readable_regcache *regcache,
+libunwind_get_reg_special (struct gdbarch *gdbarch, register_reader *regcache,
 			   int regnum, void *buf)
 {
   unw_cursor_t cursor;
diff --git a/gdb/ia64-libunwind-tdep.h b/gdb/ia64-libunwind-tdep.h
index dc7ec9cf46ea..f6bd3458c2d1 100644
--- a/gdb/ia64-libunwind-tdep.h
+++ b/gdb/ia64-libunwind-tdep.h
@@ -71,7 +71,7 @@ unw_word_t libunwind_find_dyn_list (unw_addr_space_t, unw_dyn_info_t *,
 				    void *);
 
 int libunwind_get_reg_special (struct gdbarch *gdbarch,
-			       readable_regcache *regcache,
+			       register_reader *regcache,
 			       int regnum, void *buf);
 
 #endif /* IA64_LIBUNWIND_TDEP_H */
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index d381ecc74f3a..bf49d83b825f 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -928,7 +928,7 @@ rse_address_add(CORE_ADDR addr, int nslots)
 }
 
 static enum register_status
-ia64_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+ia64_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
                            int regnum, gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1084,7 +1084,8 @@ ia64_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 }
 
 static void
-ia64_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
+ia64_pseudo_register_write (struct gdbarch *gdbarch,
+			    register_readwriter *regcache,
 			    int regnum, const gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/m32c-tdep.c b/gdb/m32c-tdep.c
index 6fa24452da96..6189084ebfa7 100644
--- a/gdb/m32c-tdep.c
+++ b/gdb/m32c-tdep.c
@@ -47,11 +47,11 @@ struct m32c_reg;
 /* The type of a function that moves the value of REG between CACHE or
    BUF --- in either direction.  */
 typedef enum register_status (m32c_write_reg_t) (struct m32c_reg *reg,
-						 struct regcache *cache,
+						 register_readwriter *cache,
 						 const gdb_byte *buf);
 
 typedef enum register_status (m32c_read_reg_t) (struct m32c_reg *reg,
-						readable_regcache *cache,
+						register_reader *cache,
 						gdb_byte *buf);
 
 struct m32c_reg
@@ -310,7 +310,7 @@ static m32c_write_reg_t m32c_r3r2r1r0_write;
 
 /* Copy the value of the raw register REG from CACHE to BUF.  */
 static enum register_status
-m32c_raw_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
+m32c_raw_read (struct m32c_reg *reg, register_reader *cache, gdb_byte *buf)
 {
   return cache->raw_read (reg->num, buf);
 }
@@ -318,7 +318,7 @@ m32c_raw_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
 
 /* Copy the value of the raw register REG from BUF to CACHE.  */
 static enum register_status
-m32c_raw_write (struct m32c_reg *reg, struct regcache *cache,
+m32c_raw_write (struct m32c_reg *reg, register_readwriter *cache,
 		const gdb_byte *buf)
 {
   cache->raw_write (reg->num, buf);
@@ -329,7 +329,7 @@ m32c_raw_write (struct m32c_reg *reg, struct regcache *cache,
 
 /* Return the value of the 'flg' register in CACHE.  */
 static int
-m32c_read_flg (readable_regcache *cache)
+m32c_read_flg (register_reader *cache)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (cache->arch ());
   ULONGEST flg;
@@ -341,7 +341,7 @@ m32c_read_flg (readable_regcache *cache)
 
 /* Evaluate the real register number of a banked register.  */
 static struct m32c_reg *
-m32c_banked_register (struct m32c_reg *reg, readable_regcache *cache)
+m32c_banked_register (struct m32c_reg *reg, register_reader *cache)
 {
   return ((m32c_read_flg (cache) & reg->n) ? reg->ry : reg->rx);
 }
@@ -352,7 +352,7 @@ m32c_banked_register (struct m32c_reg *reg, readable_regcache *cache)
    masked in REG->n set, then read REG->ry.  Otherwise, read
    REG->rx.  */
 static enum register_status
-m32c_banked_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
+m32c_banked_read (struct m32c_reg *reg, register_reader *cache, gdb_byte *buf)
 {
   struct m32c_reg *bank_reg = m32c_banked_register (reg, cache);
   return cache->raw_read (bank_reg->num, buf);
@@ -364,7 +364,8 @@ m32c_banked_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
    masked in REG->n set, then write REG->ry.  Otherwise, write
    REG->rx.  */
 static enum register_status
-m32c_banked_write (struct m32c_reg *reg, struct regcache *cache,
+m32c_banked_write (struct m32c_reg *reg,
+		   register_readwriter *cache,
 		   const gdb_byte *buf)
 {
   struct m32c_reg *bank_reg = m32c_banked_register (reg, cache);
@@ -377,7 +378,7 @@ m32c_banked_write (struct m32c_reg *reg, struct regcache *cache,
 /* Move the value of SB from CACHE to BUF.  On bfd_mach_m32c, SB is a
    banked register; on bfd_mach_m16c, it's not.  */
 static enum register_status
-m32c_sb_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
+m32c_sb_read (struct m32c_reg *reg, register_reader *cache, gdb_byte *buf)
 {
   if (gdbarch_bfd_arch_info (reg->arch)->mach == bfd_mach_m16c)
     return m32c_raw_read (reg->rx, cache, buf);
@@ -389,7 +390,8 @@ m32c_sb_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
 /* Move the value of SB from BUF to CACHE.  On bfd_mach_m32c, SB is a
    banked register; on bfd_mach_m16c, it's not.  */
 static enum register_status
-m32c_sb_write (struct m32c_reg *reg, struct regcache *cache, const gdb_byte *buf)
+m32c_sb_write (struct m32c_reg *reg, register_readwriter *cache,
+	       const gdb_byte *buf)
 {
   if (gdbarch_bfd_arch_info (reg->arch)->mach == bfd_mach_m16c)
     m32c_raw_write (reg->rx, cache, buf);
@@ -442,7 +444,7 @@ m32c_find_part (struct m32c_reg *reg, int *offset_p, int *len_p)
    REG->type values, where higher indices refer to more significant
    bits, read the value of the REG->n'th element.  */
 static enum register_status
-m32c_part_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
+m32c_part_read (struct m32c_reg *reg, register_reader *cache, gdb_byte *buf)
 {
   int offset, len;
 
@@ -457,7 +459,7 @@ m32c_part_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
    values, where higher indices refer to more significant bits, write
    the value of the REG->n'th element.  */
 static enum register_status
-m32c_part_write (struct m32c_reg *reg, struct regcache *cache,
+m32c_part_write (struct m32c_reg *reg, register_readwriter *cache,
 		 const gdb_byte *buf)
 {
   int offset, len;
@@ -473,7 +475,7 @@ m32c_part_write (struct m32c_reg *reg, struct regcache *cache,
    concatenation of the values of the registers REG->rx and REG->ry,
    with REG->rx contributing the more significant bits.  */
 static enum register_status
-m32c_cat_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
+m32c_cat_read (struct m32c_reg *reg, register_reader *cache, gdb_byte *buf)
 {
   int high_bytes = TYPE_LENGTH (reg->rx->type);
   int low_bytes  = TYPE_LENGTH (reg->ry->type);
@@ -501,7 +503,7 @@ m32c_cat_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
    concatenation of the values of the registers REG->rx and REG->ry,
    with REG->rx contributing the more significant bits.  */
 static enum register_status
-m32c_cat_write (struct m32c_reg *reg, struct regcache *cache,
+m32c_cat_write (struct m32c_reg *reg, register_readwriter *cache,
 		const gdb_byte *buf)
 {
   int high_bytes = TYPE_LENGTH (reg->rx->type);
@@ -528,7 +530,7 @@ m32c_cat_write (struct m32c_reg *reg, struct regcache *cache,
    the concatenation (from most significant to least) of r3, r2, r1,
    and r0.  */
 static enum register_status
-m32c_r3r2r1r0_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
+m32c_r3r2r1r0_read (struct m32c_reg *reg, register_reader *cache, gdb_byte *buf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (reg->arch);
   int len = TYPE_LENGTH (tdep->r0->type);
@@ -563,7 +565,7 @@ m32c_r3r2r1r0_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *bu
    the concatenation (from most significant to least) of r3, r2, r1,
    and r0.  */
 static enum register_status
-m32c_r3r2r1r0_write (struct m32c_reg *reg, struct regcache *cache,
+m32c_r3r2r1r0_write (struct m32c_reg *reg, register_readwriter *cache,
 		     const gdb_byte *buf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (reg->arch);
@@ -590,7 +592,7 @@ m32c_r3r2r1r0_write (struct m32c_reg *reg, struct regcache *cache,
 
 static enum register_status
 m32c_pseudo_register_read (struct gdbarch *arch,
-			   readable_regcache *cache,
+			   register_reader *cache,
 			   int cookednum,
 			   gdb_byte *buf)
 {
@@ -608,7 +610,7 @@ m32c_pseudo_register_read (struct gdbarch *arch,
 
 static void
 m32c_pseudo_register_write (struct gdbarch *arch,
-			    struct regcache *cache,
+			    register_readwriter *cache,
 			    int cookednum,
 			    const gdb_byte *buf)
 {
diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
index b6e8f00a0ba1..fbbe15cbfc1e 100644
--- a/gdb/m68hc11-tdep.c
+++ b/gdb/m68hc11-tdep.c
@@ -279,7 +279,7 @@ m68hc11_which_soft_register (CORE_ADDR addr)
    fetch into a memory read.  */
 static enum register_status
 m68hc11_pseudo_register_read (struct gdbarch *gdbarch,
-			      readable_regcache *regcache,
+			      register_reader *regcache,
 			      int regno, gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -327,7 +327,7 @@ m68hc11_pseudo_register_read (struct gdbarch *gdbarch,
    into a memory write.  */
 static void
 m68hc11_pseudo_register_write (struct gdbarch *gdbarch,
-			       struct regcache *regcache,
+			       register_readwriter *regcache,
 			       int regno, const gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/mep-tdep.c b/gdb/mep-tdep.c
index ae9c4debcae9..f4355e3c85ba 100644
--- a/gdb/mep-tdep.c
+++ b/gdb/mep-tdep.c
@@ -1114,7 +1114,7 @@ mep_register_type (struct gdbarch *gdbarch, int reg_nr)
 
 static enum register_status
 mep_pseudo_cr32_read (struct gdbarch *gdbarch,
-		      readable_regcache *regcache,
+		      register_reader *regcache,
                       int cookednum,
                       gdb_byte *buf)
 {
@@ -1140,7 +1140,7 @@ mep_pseudo_cr32_read (struct gdbarch *gdbarch,
 
 static enum register_status
 mep_pseudo_cr64_read (struct gdbarch *gdbarch,
-                      readable_regcache *regcache,
+		      register_reader *regcache,
                       int cookednum,
                       gdb_byte *buf)
 {
@@ -1150,7 +1150,7 @@ mep_pseudo_cr64_read (struct gdbarch *gdbarch,
 
 static enum register_status
 mep_pseudo_register_read (struct gdbarch *gdbarch,
-			  readable_regcache *regcache,
+			  register_reader *regcache,
                           int cookednum,
                           gdb_byte *buf)
 {
@@ -1170,7 +1170,7 @@ mep_pseudo_register_read (struct gdbarch *gdbarch,
 
 static void
 mep_pseudo_csr_write (struct gdbarch *gdbarch,
-                      struct regcache *regcache,
+		      register_readwriter *regcache,
                       int cookednum,
                       const gdb_byte *buf)
 {
@@ -1190,7 +1190,7 @@ mep_pseudo_csr_write (struct gdbarch *gdbarch,
       ULONGEST new_bits;
       ULONGEST mixed_bits;
           
-      regcache_raw_read_unsigned (regcache, r->raw, &old_bits);
+      regcache->raw_read (r->raw, &old_bits);
       new_bits = extract_unsigned_integer (buf, size, byte_order);
       mixed_bits = ((r->writeable_bits & new_bits)
                     | (~r->writeable_bits & old_bits));
@@ -1201,7 +1201,7 @@ mep_pseudo_csr_write (struct gdbarch *gdbarch,
 
 static void
 mep_pseudo_cr32_write (struct gdbarch *gdbarch,
-                       struct regcache *regcache,
+		       register_readwriter *regcache,
                        int cookednum,
                        const gdb_byte *buf)
 {
@@ -1222,9 +1222,9 @@ mep_pseudo_cr32_write (struct gdbarch *gdbarch,
 
 static void
 mep_pseudo_cr64_write (struct gdbarch *gdbarch,
-                     struct regcache *regcache,
-                     int cookednum,
-                     const gdb_byte *buf)
+		       register_readwriter *regcache,
+		       int cookednum,
+		       const gdb_byte *buf)
 {
   regcache->raw_write (mep_pseudo_to_raw[cookednum], buf);
 }
@@ -1232,7 +1232,7 @@ mep_pseudo_cr64_write (struct gdbarch *gdbarch,
 
 static void
 mep_pseudo_register_write (struct gdbarch *gdbarch,
-                           struct regcache *regcache,
+			   register_readwriter *regcache,
                            int cookednum,
                            const gdb_byte *buf)
 {
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 2c8726214f39..61f1fd165038 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -747,7 +747,7 @@ mips_tdesc_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
    registers.  Take care of alignment and size problems.  */
 
 static enum register_status
-mips_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+mips_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
 			   int cookednum, gdb_byte *buf)
 {
   int rawnum = cookednum % gdbarch_num_regs (gdbarch);
@@ -778,7 +778,7 @@ mips_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 
 static void
 mips_pseudo_register_write (struct gdbarch *gdbarch,
-			    struct regcache *regcache, int cookednum,
+			    register_readwriter *regcache, int cookednum,
 			    const gdb_byte *buf)
 {
   int rawnum = cookednum % gdbarch_num_regs (gdbarch);
diff --git a/gdb/msp430-tdep.c b/gdb/msp430-tdep.c
index 427f58c0ed08..43d16f5bcbe0 100644
--- a/gdb/msp430-tdep.c
+++ b/gdb/msp430-tdep.c
@@ -218,7 +218,7 @@ msp430_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
 
 static enum register_status
 msp430_pseudo_register_read (struct gdbarch *gdbarch,
-			     readable_regcache *regcache,
+			     register_reader *regcache,
 			     int regnum, gdb_byte *buffer)
 {
   if (MSP430_NUM_REGS <= regnum && regnum < MSP430_NUM_TOTAL_REGS)
@@ -243,7 +243,7 @@ msp430_pseudo_register_read (struct gdbarch *gdbarch,
 
 static void
 msp430_pseudo_register_write (struct gdbarch *gdbarch,
-			      struct regcache *regcache,
+			      register_readwriter *regcache,
 			      int regnum, const gdb_byte *buffer)
 {
   if (MSP430_NUM_REGS <= regnum && regnum < MSP430_NUM_TOTAL_REGS)
diff --git a/gdb/nds32-tdep.c b/gdb/nds32-tdep.c
index 05b82def0261..e5241a58eb13 100644
--- a/gdb/nds32-tdep.c
+++ b/gdb/nds32-tdep.c
@@ -437,7 +437,7 @@ nds32_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
 
 static enum register_status
 nds32_pseudo_register_read (struct gdbarch *gdbarch,
-			    readable_regcache *regcache, int regnum,
+			    register_reader *regcache, int regnum,
 			    gdb_byte *buf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -476,7 +476,7 @@ nds32_pseudo_register_read (struct gdbarch *gdbarch,
 
 static void
 nds32_pseudo_register_write (struct gdbarch *gdbarch,
-			     struct regcache *regcache, int regnum,
+			     register_readwriter *regcache, int regnum,
 			     const gdb_byte *buf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 946035ae67ae..a0067aa87331 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -519,6 +519,8 @@ enum register_status
 readable_regcache::raw_read (int regnum, gdb_byte *buf)
 {
   gdb_assert (buf != NULL);
+  gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (arch ()));
+
   raw_update (regnum);
 
   if (m_register_status[regnum] != REG_VALID)
@@ -531,7 +533,7 @@ readable_regcache::raw_read (int regnum, gdb_byte *buf)
 }
 
 enum register_status
-regcache_raw_read_signed (struct regcache *regcache, int regnum, LONGEST *val)
+regcache_raw_read_signed (register_reader *regcache, int regnum, LONGEST *val)
 {
   gdb_assert (regcache != NULL);
   return regcache->raw_read (regnum, val);
@@ -539,20 +541,19 @@ regcache_raw_read_signed (struct regcache *regcache, int regnum, LONGEST *val)
 
 template<typename T, typename>
 enum register_status
-readable_regcache::raw_read (int regnum, T *val)
+register_reader::raw_read (int regnum, T *val)
 {
-  gdb_byte *buf;
-  enum register_status status;
+  gdbarch *arch = this->arch ();
+  int reg_size = register_size (arch, regnum);
+  gdb_byte buf[reg_size];
+
+  register_status status = raw_read (regnum, buf);
 
-  assert_regnum (regnum);
-  buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
-  status = raw_read (regnum, buf);
   if (status == REG_VALID)
-    *val = extract_integer<T> (buf,
-			       m_descr->sizeof_register[regnum],
-			       gdbarch_byte_order (m_descr->gdbarch));
+    *val = extract_integer<T> (buf, reg_size, gdbarch_byte_order (arch));
   else
     *val = 0;
+
   return status;
 }
 
@@ -565,7 +566,7 @@ regcache_raw_read_unsigned (struct regcache *regcache, int regnum,
 }
 
 void
-regcache_raw_write_signed (struct regcache *regcache, int regnum, LONGEST val)
+regcache_raw_write_signed (register_readwriter *regcache, int regnum, LONGEST val)
 {
   gdb_assert (regcache != NULL);
   regcache->raw_write (regnum, val);
@@ -573,19 +574,18 @@ regcache_raw_write_signed (struct regcache *regcache, int regnum, LONGEST val)
 
 template<typename T, typename>
 void
-regcache::raw_write (int regnum, T val)
+register_readwriter::raw_write (int regnum, T val)
 {
-  gdb_byte *buf;
+  gdbarch *arch = this->arch ();
+  int reg_size = register_size (arch, regnum);
+  gdb_byte buf[reg_size];
 
-  assert_regnum (regnum);
-  buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
-  store_integer (buf, m_descr->sizeof_register[regnum],
-		 gdbarch_byte_order (m_descr->gdbarch), val);
+  store_integer (buf, reg_size, gdbarch_byte_order (arch), val);
   raw_write (regnum, buf);
 }
 
 void
-regcache_raw_write_unsigned (struct regcache *regcache, int regnum,
+regcache_raw_write_unsigned (register_readwriter *regcache, int regnum,
 			     ULONGEST val)
 {
   gdb_assert (regcache != NULL);
@@ -651,18 +651,17 @@ readable_regcache::cooked_read (int regnum, gdb_byte *buf)
 }
 
 struct value *
-readable_regcache::cooked_read_value (int regnum)
+register_reader::cooked_read_value (int regnum)
 {
-  gdb_assert (regnum >= 0);
-  gdb_assert (regnum < m_descr->nr_cooked_registers);
+  gdbarch *arch = this->arch ();
+  gdb_assert (regnum >= 0 && regnum < gdbarch_num_cooked_regs (arch));
 
-  if (regnum < num_raw_registers ()
-      || (m_has_pseudo && m_register_status[regnum] != REG_UNKNOWN)
-      || !gdbarch_pseudo_register_read_value_p (m_descr->gdbarch))
+  if (regnum < gdbarch_num_regs (arch)
+      || !gdbarch_pseudo_register_read_value_p (arch))
     {
       struct value *result;
 
-      result = allocate_value (register_type (m_descr->gdbarch, regnum));
+      result = allocate_value (register_type (arch, regnum));
       VALUE_LVAL (result) = lval_register;
       VALUE_REGNUM (result) = regnum;
 
@@ -677,12 +676,11 @@ readable_regcache::cooked_read_value (int regnum)
       return result;
     }
   else
-    return gdbarch_pseudo_register_read_value (m_descr->gdbarch,
-					       this, regnum);
+    return gdbarch_pseudo_register_read_value (arch, this, regnum);
 }
 
 enum register_status
-regcache_cooked_read_signed (struct regcache *regcache, int regnum,
+regcache_cooked_read_signed (register_reader *regcache, int regnum,
 			     LONGEST *val)
 {
   gdb_assert (regcache != NULL);
@@ -691,24 +689,24 @@ regcache_cooked_read_signed (struct regcache *regcache, int regnum,
 
 template<typename T, typename>
 enum register_status
-readable_regcache::cooked_read (int regnum, T *val)
+register_reader::cooked_read (int regnum, T *val)
 {
-  enum register_status status;
-  gdb_byte *buf;
+  gdbarch *arch = this->arch ();
+  int reg_size = register_size (arch, regnum);
+  gdb_byte buf[reg_size];
+
+  register_status status = cooked_read (regnum, buf);
 
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers);
-  buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
-  status = cooked_read (regnum, buf);
   if (status == REG_VALID)
-    *val = extract_integer<T> (buf, m_descr->sizeof_register[regnum],
-			       gdbarch_byte_order (m_descr->gdbarch));
+    *val = extract_integer<T> (buf, reg_size, gdbarch_byte_order (arch));
   else
     *val = 0;
+
   return status;
 }
 
 enum register_status
-regcache_cooked_read_unsigned (struct regcache *regcache, int regnum,
+regcache_cooked_read_unsigned (register_reader *regcache, int regnum,
 			       ULONGEST *val)
 {
   gdb_assert (regcache != NULL);
@@ -716,7 +714,7 @@ regcache_cooked_read_unsigned (struct regcache *regcache, int regnum,
 }
 
 void
-regcache_cooked_write_signed (struct regcache *regcache, int regnum,
+regcache_cooked_write_signed (register_readwriter *regcache, int regnum,
 			      LONGEST val)
 {
   gdb_assert (regcache != NULL);
@@ -725,19 +723,18 @@ regcache_cooked_write_signed (struct regcache *regcache, int regnum,
 
 template<typename T, typename>
 void
-regcache::cooked_write (int regnum, T val)
+register_readwriter::cooked_write (int regnum, T val)
 {
-  gdb_byte *buf;
+  gdbarch *arch = this->arch ();
+  int reg_size = register_size (arch, regnum);
+  gdb_byte buf[reg_size];
 
-  gdb_assert (regnum >=0 && regnum < m_descr->nr_cooked_registers);
-  buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
-  store_integer (buf, m_descr->sizeof_register[regnum],
-		 gdbarch_byte_order (m_descr->gdbarch), val);
+  store_integer (buf, reg_size, gdbarch_byte_order (arch), val);
   cooked_write (regnum, buf);
 }
 
 void
-regcache_cooked_write_unsigned (struct regcache *regcache, int regnum,
+regcache_cooked_write_unsigned (register_readwriter *regcache, int regnum,
 				ULONGEST val)
 {
   gdb_assert (regcache != NULL);
@@ -792,8 +789,8 @@ regcache::cooked_write (int regnum, const gdb_byte *buf)
 /* See regcache.h.  */
 
 enum register_status
-readable_regcache::read_part (int regnum, int offset, int len,
-			      gdb_byte *out, bool is_raw)
+register_reader::read_part (int regnum, int offset, int len,
+			    gdb_byte *out, bool is_raw)
 {
   int reg_size = register_size (arch (), regnum);
 
@@ -859,8 +856,8 @@ reg_buffer::raw_collect_part (int regnum, int offset, int len,
 /* See regcache.h.  */
 
 enum register_status
-regcache::write_part (int regnum, int offset, int len,
-		      const gdb_byte *in, bool is_raw)
+register_readwriter::write_part (int regnum, int offset, int len,
+				 const gdb_byte *in, bool is_raw)
 {
   int reg_size = register_size (arch (), regnum);
 
@@ -930,40 +927,43 @@ reg_buffer::raw_supply_part (int regnum, int offset, int len,
 }
 
 enum register_status
-readable_regcache::raw_read_part (int regnum, int offset, int len,
-				  gdb_byte *buf)
+register_reader::raw_read_part (int regnum, int offset, int len, gdb_byte *buf)
 {
-  assert_regnum (regnum);
+  gdbarch *arch = this->arch ();
+  gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (arch));
   return read_part (regnum, offset, len, buf, true);
 }
 
 /* See regcache.h.  */
 
 void
-regcache::raw_write_part (int regnum, int offset, int len,
+register_readwriter::raw_write_part (int regnum, int offset, int len,
 			  const gdb_byte *buf)
 {
-  assert_regnum (regnum);
+  gdbarch *arch = this->arch ();
+  gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (arch));
   write_part (regnum, offset, len, buf, true);
 }
 
 /* See regcache.h.  */
 
 enum register_status
-readable_regcache::cooked_read_part (int regnum, int offset, int len,
+register_reader::cooked_read_part (int regnum, int offset, int len,
 				     gdb_byte *buf)
 {
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers);
+  gdbarch *arch = this->arch ();
+  gdb_assert (regnum >= 0 && regnum < gdbarch_num_cooked_regs (arch));
   return read_part (regnum, offset, len, buf, false);
 }
 
 /* See regcache.h.  */
 
 void
-regcache::cooked_write_part (int regnum, int offset, int len,
-			     const gdb_byte *buf)
+register_readwriter::cooked_write_part (int regnum, int offset, int len,
+					const gdb_byte *buf)
 {
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers);
+  gdbarch *arch = this->arch ();
+  gdb_assert (regnum >= 0 && regnum < gdbarch_num_cooked_regs (arch));
   write_part (regnum, offset, len, buf, false);
 }
 
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 374826cdb822..762a08797eb3 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -23,6 +23,8 @@
 #include "common-regcache.h"
 #include <forward_list>
 
+struct register_reader;
+struct register_readwriter;
 struct regcache;
 struct regset;
 struct gdbarch;
@@ -40,12 +42,12 @@ extern struct regcache *get_thread_arch_aspace_regcache (ptid_t,
 							 struct address_space *);
 
 extern enum register_status
-  regcache_raw_read_signed (struct regcache *regcache,
+  regcache_raw_read_signed (register_reader *regcache,
 			    int regnum, LONGEST *val);
 
-extern void regcache_raw_write_signed (struct regcache *regcache,
+extern void regcache_raw_write_signed (register_readwriter *regcache,
 				       int regnum, LONGEST val);
-extern void regcache_raw_write_unsigned (struct regcache *regcache,
+extern void regcache_raw_write_unsigned (register_readwriter *regcache,
 					 int regnum, ULONGEST val);
 
 /* Return the register's value in signed or throw if it's not
@@ -56,14 +58,14 @@ extern LONGEST regcache_raw_get_signed (struct regcache *regcache,
 
 /* Read a register as a signed/unsigned quantity.  */
 extern enum register_status
-  regcache_cooked_read_signed (struct regcache *regcache,
+  regcache_cooked_read_signed (register_reader *regcache,
 			       int regnum, LONGEST *val);
 extern enum register_status
-  regcache_cooked_read_unsigned (struct regcache *regcache,
+  regcache_cooked_read_unsigned (register_reader *regcache,
 				 int regnum, ULONGEST *val);
-extern void regcache_cooked_write_signed (struct regcache *regcache,
+extern void regcache_cooked_write_signed (register_readwriter *regcache,
 					  int regnum, LONGEST val);
-extern void regcache_cooked_write_unsigned (struct regcache *regcache,
+extern void regcache_cooked_write_unsigned (register_readwriter *regcache,
 					    int regnum, ULONGEST val);
 
 /* Special routines to read/write the PC.  */
@@ -256,49 +258,101 @@ protected:
   friend class detached_regcache;
 };
 
-/* An abstract class which only has methods doing read.  */
+/* Interface for something you can read a register from.  */
 
-class readable_regcache : public reg_buffer
+struct register_reader
 {
-public:
-  readable_regcache (gdbarch *gdbarch, bool has_pseudo)
-    : reg_buffer (gdbarch, has_pseudo)
-  {}
+  virtual ~register_reader () = default;
 
-  /* Transfer a raw register [0..NUM_REGS) from core-gdb to this regcache,
-     return its value in *BUF and return its availability status.  */
-
-  enum register_status raw_read (int regnum, gdb_byte *buf);
-  template<typename T, typename = RequireLongest<T>>
-  enum register_status raw_read (int regnum, T *val);
-
-  /* Partial transfer of raw registers.  Return the status of the register.  */
-  enum register_status raw_read_part (int regnum, int offset, int len,
-				      gdb_byte *buf);
+  virtual gdbarch *arch () const = 0;
 
-  /* Make certain that the register REGNUM is up-to-date.  */
-  virtual void raw_update (int regnum) = 0;
+  virtual register_status raw_read (int regnum, gdb_byte *buf) = 0;
+  virtual register_status cooked_read (int regnum, gdb_byte *buf) = 0;
 
-  /* Transfer a raw register [0..NUM_REGS+NUM_PSEUDO_REGS) from core-gdb to
-     this regcache, return its value in *BUF and return its availability status.  */
-  enum register_status cooked_read (int regnum, gdb_byte *buf);
+  /* Variants of the above to read the value in a LONGEST or ULONGEST.  */
+  template<typename T, typename = RequireLongest<T>>
+  register_status raw_read (int regnum, T *val);
   template<typename T, typename = RequireLongest<T>>
-  enum register_status cooked_read (int regnum, T *val);
+  register_status cooked_read (int regnum, T *val);
 
+  /* Partial transfer of raw registers.  Return the status of the register.  */
+  register_status raw_read_part (int regnum, int offset, int len,
+				 gdb_byte *buf);
   /* Partial transfer of a cooked register.  */
-  enum register_status cooked_read_part (int regnum, int offset, int len,
-					 gdb_byte *buf);
+  register_status cooked_read_part (int regnum, int offset, int len,
+				    gdb_byte *buf);
 
   /* Read register REGNUM from the regcache and return a new value.  This
      will call mark_value_bytes_unavailable as appropriate.  */
   struct value *cooked_read_value (int regnum);
 
-protected:
+private:
+  register_status read_part (int regnum, int offset, int len, gdb_byte *in,
+			     bool is_raw);
+};
+
+/* Interface for something you can read/write a register from/to.  */
+
+struct register_readwriter : public virtual register_reader
+{
+  virtual ~register_readwriter () = default;
+
+  /* Update the value of raw register REGNUM (in the range [0..NUM_REGS)) and
+     transfer its value to core-gdb.  */
+  virtual void raw_write (int regnum, const gdb_byte *buf) = 0;
+
+  /* Transfer of pseudo-registers.  */
+  virtual void cooked_write (int regnum, const gdb_byte *buf) = 0;
+
+  /* Variants of the above to read the value in a LONGEST or ULONGEST.  */
+  template<typename T, typename = RequireLongest<T>>
+  void raw_write (int regnum, T val);
+  template<typename T, typename = RequireLongest<T>>
+  void cooked_write (int regnum, T val);
 
+  /* Partial transfer of raw registers.  Perform read, modify, write style
+     operations.  */
+  void raw_write_part (int regnum, int offset, int len, const gdb_byte *buf);
+
+  /* Partial transfer of a cooked register.  Perform read, modify, write style
+     operations.  */
+  void cooked_write_part (int regnum, int offset, int len,
+			  const gdb_byte *buf);
+
+  virtual void invalidate (int regnum)
+  {};
+
+private:
   /* Perform a partial register transfer using a read, modify, write
-     operation.  Will fail if register is currently invalid.  */
-  enum register_status read_part (int regnum, int offset, int len,
-				  gdb_byte *out, bool is_raw);
+     operation.  */
+  register_status write_part (int regnum, int offset, int len,
+			      const gdb_byte *in, bool is_raw);
+};
+
+/* An abstract class which only has methods doing read.  */
+
+class readable_regcache : public reg_buffer, public virtual register_reader
+{
+public:
+  readable_regcache (gdbarch *gdbarch, bool has_pseudo)
+    : reg_buffer (gdbarch, has_pseudo)
+  {}
+
+  gdbarch *arch () const override
+  { return reg_buffer::arch (); }
+
+  /* Transfer a raw register [0..NUM_REGS) from core-gdb to this regcache,
+     return its value in *BUF and return its availability status.  */
+  virtual register_status raw_read (int regnum, gdb_byte *buf) override;
+  using register_reader::raw_read;
+
+  /* Transfer a raw register [0..NUM_REGS+NUM_PSEUDO_REGS) from core-gdb to
+     this regcache, return its value in *BUF and return its availability status.  */
+  virtual register_status cooked_read (int regnum, gdb_byte *buf) override;
+  using register_reader::cooked_read;
+
+  /* Make certain that the register REGNUM is up-to-date.  */
+  virtual void raw_update (int regnum) = 0;
 };
 
 /* Buffer of registers, can be read and written.  */
@@ -320,7 +374,7 @@ class readonly_detached_regcache;
 
 /* The register cache for storing raw register values.  */
 
-class regcache : public detached_regcache
+class regcache : public detached_regcache, public register_readwriter
 {
 public:
   DISABLE_COPY_AND_ASSIGN (regcache);
@@ -331,6 +385,14 @@ public:
     return m_aspace;
   }
 
+  gdbarch *arch () const override
+  { return readable_regcache::arch (); }
+
+  void invalidate (int regnum) override
+  {
+    detached_regcache::invalidate (regnum);
+  }
+
   /* Restore 'this' regcache.  The set of registers restored into
      the regcache determined by the restore_reggroup.
      Writes to regcache will go through to the target.  SRC is a
@@ -339,29 +401,15 @@ public:
 
   /* Update the value of raw register REGNUM (in the range [0..NUM_REGS)) and
      transfer its value to core-gdb.  */
-
-  void raw_write (int regnum, const gdb_byte *buf);
-
-  template<typename T, typename = RequireLongest<T>>
-  void raw_write (int regnum, T val);
+  void raw_write (int regnum, const gdb_byte *buf) override;
+  using register_readwriter::raw_write;
 
   /* Transfer of pseudo-registers.  */
-  void cooked_write (int regnum, const gdb_byte *buf);
-
-  template<typename T, typename = RequireLongest<T>>
-  void cooked_write (int regnum, T val);
+  void cooked_write (int regnum, const gdb_byte *buf) override;
+  using register_readwriter::cooked_write;
 
   void raw_update (int regnum) override;
 
-  /* Partial transfer of raw registers.  Perform read, modify, write style
-     operations.  */
-  void raw_write_part (int regnum, int offset, int len, const gdb_byte *buf);
-
-  /* Partial transfer of a cooked register.  Perform read, modify, write style
-     operations.  */
-  void cooked_write_part (int regnum, int offset, int len,
-			  const gdb_byte *buf);
-
   void supply_regset (const struct regset *regset,
 		      int regnum, const void *buf, size_t size);
 
@@ -408,11 +456,6 @@ private:
 			int regnum, const gdb_byte *in_buf,
 			gdb_byte *out_buf, size_t size) const;
 
-  /* Perform a partial register transfer using a read, modify, write
-     operation.  */
-  enum register_status write_part (int regnum, int offset, int len,
-				   const gdb_byte *in, bool is_raw);
-
   /* The address space of this register cache (for registers where it
      makes sense, like PC or SP).  */
   const address_space * const m_aspace;
diff --git a/gdb/rl78-tdep.c b/gdb/rl78-tdep.c
index ace01b1171a2..42d0eb3e67d1 100644
--- a/gdb/rl78-tdep.c
+++ b/gdb/rl78-tdep.c
@@ -640,7 +640,7 @@ rl78_make_data_address (CORE_ADDR addr)
 
 static enum register_status
 rl78_pseudo_register_read (struct gdbarch *gdbarch,
-			   readable_regcache *regcache,
+			   register_reader *regcache,
                            int reg, gdb_byte *buffer)
 {
   enum register_status status;
@@ -722,7 +722,7 @@ rl78_pseudo_register_read (struct gdbarch *gdbarch,
 
 static void
 rl78_pseudo_register_write (struct gdbarch *gdbarch,
-                            struct regcache *regcache,
+			    register_readwriter *regcache,
                             int reg, const gdb_byte *buffer)
 {
   if (RL78_BANK0_R0_REGNUM <= reg && reg <= RL78_BANK3_R7_REGNUM)
@@ -767,7 +767,7 @@ rl78_pseudo_register_write (struct gdbarch *gdbarch,
       int bank;
       int raw_regnum;
 
-      regcache_raw_read_unsigned (regcache, RL78_PSW_REGNUM, &psw);
+      regcache->raw_read (RL78_PSW_REGNUM, &psw);
       bank = ((psw >> 3) & 1) | ((psw >> 4) & 1);
       /* RSB0 is at bit 3; RSBS1 is at bit 5.  */
       raw_regnum = RL78_RAW_BANK0_R0_REGNUM + bank * RL78_REGS_PER_BANK
@@ -779,7 +779,7 @@ rl78_pseudo_register_write (struct gdbarch *gdbarch,
       ULONGEST psw;
       int bank, raw_regnum;
 
-      regcache_raw_read_unsigned (regcache, RL78_PSW_REGNUM, &psw);
+      regcache->raw_read (RL78_PSW_REGNUM, &psw);
       bank = ((psw >> 3) & 1) | ((psw >> 4) & 1);
       /* RSB0 is at bit 3; RSBS1 is at bit 5.  */
       raw_regnum = RL78_RAW_BANK0_R0_REGNUM + bank * RL78_REGS_PER_BANK
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 80d8504b2334..e3d8347447eb 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -2487,7 +2487,7 @@ rs6000_value_to_register (struct frame_info *frame,
 
  /* The type of a function that moves the value of REG between CACHE
     or BUF --- in either direction.  */
-typedef enum register_status (*move_ev_register_func) (struct regcache *,
+typedef enum register_status (*move_ev_register_func) (register_readwriter *,
 						       int, void *);
 
 /* Move SPE vector register values between a 64-bit buffer and the two
@@ -2516,7 +2516,7 @@ typedef enum register_status (*move_ev_register_func) (struct regcache *,
 
 static enum register_status
 e500_move_ev_register (move_ev_register_func move,
-		       struct regcache *regcache, int ev_reg, void *buffer)
+		       register_readwriter *regcache, int ev_reg, void *buffer)
 {
   struct gdbarch *arch = regcache->arch ();
   struct gdbarch_tdep *tdep = gdbarch_tdep (arch); 
@@ -2548,7 +2548,7 @@ e500_move_ev_register (move_ev_register_func move,
 }
 
 static enum register_status
-do_regcache_raw_write (struct regcache *regcache, int regnum, void *buffer)
+do_regcache_raw_write (register_readwriter *regcache, int regnum, void *buffer)
 {
   regcache->raw_write (regnum, (const gdb_byte *) buffer);
 
@@ -2556,7 +2556,7 @@ do_regcache_raw_write (struct regcache *regcache, int regnum, void *buffer)
 }
 
 static enum register_status
-e500_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+e500_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
 			   int ev_reg, gdb_byte *buffer)
 {
   struct gdbarch *arch = regcache->arch ();
@@ -2589,7 +2589,8 @@ e500_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 }
 
 static void
-e500_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
+e500_pseudo_register_write (struct gdbarch *gdbarch,
+			    register_readwriter *regcache,
 			    int reg_nr, const gdb_byte *buffer)
 {
   e500_move_ev_register (do_regcache_raw_write, regcache,
@@ -2598,7 +2599,7 @@ e500_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 
 /* Read method for DFP pseudo-registers.  */
 static enum register_status
-dfp_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+dfp_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
 			   int reg_nr, gdb_byte *buffer)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -2628,8 +2629,9 @@ dfp_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 
 /* Write method for DFP pseudo-registers.  */
 static void
-dfp_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
-			    int reg_nr, const gdb_byte *buffer)
+dfp_pseudo_register_write (struct gdbarch *gdbarch,
+			   register_readwriter *regcache,
+			   int reg_nr, const gdb_byte *buffer)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int reg_index = reg_nr - tdep->ppc_dl0_regnum;
@@ -2654,7 +2656,7 @@ dfp_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 
 /* Read method for POWER7 VSX pseudo-registers.  */
 static enum register_status
-vsx_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+vsx_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
 			   int reg_nr, gdb_byte *buffer)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -2689,8 +2691,9 @@ vsx_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 
 /* Write method for POWER7 VSX pseudo-registers.  */
 static void
-vsx_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
-			    int reg_nr, const gdb_byte *buffer)
+vsx_pseudo_register_write (struct gdbarch *gdbarch,
+			   register_readwriter *regcache,
+			   int reg_nr, const gdb_byte *buffer)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int reg_index = reg_nr - tdep->ppc_vsr0_regnum;
@@ -2719,7 +2722,7 @@ vsx_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 
 /* Read method for POWER7 Extended FP pseudo-registers.  */
 static enum register_status
-efpr_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+efpr_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
 			   int reg_nr, gdb_byte *buffer)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -2734,7 +2737,8 @@ efpr_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 
 /* Write method for POWER7 Extended FP pseudo-registers.  */
 static void
-efpr_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
+efpr_pseudo_register_write (struct gdbarch *gdbarch,
+			    register_readwriter *regcache,
 			    int reg_nr, const gdb_byte *buffer)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -2748,7 +2752,7 @@ efpr_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 
 static enum register_status
 rs6000_pseudo_register_read (struct gdbarch *gdbarch,
-			     readable_regcache *regcache,
+			     register_reader *regcache,
 			     int reg_nr, gdb_byte *buffer)
 {
   struct gdbarch *regcache_arch = regcache->arch ();
@@ -2773,7 +2777,7 @@ rs6000_pseudo_register_read (struct gdbarch *gdbarch,
 
 static void
 rs6000_pseudo_register_write (struct gdbarch *gdbarch,
-			      struct regcache *regcache,
+			      register_readwriter *regcache,
 			      int reg_nr, const gdb_byte *buffer)
 {
   struct gdbarch *regcache_arch = regcache->arch ();
diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
index 23689aa71a15..4808a13db830 100644
--- a/gdb/s390-tdep.c
+++ b/gdb/s390-tdep.c
@@ -1285,7 +1285,7 @@ s390_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
 /* Implement pseudo_register_read gdbarch method.  */
 
 static enum register_status
-s390_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+s390_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
 			   int regnum, gdb_byte *buf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -1333,7 +1333,7 @@ s390_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
       status = regcache->raw_read (S390_R0_REGNUM + regnum, &val);
       if (status == REG_VALID)
 	status = regcache->raw_read (S390_R0_UPPER_REGNUM + regnum,
-				     &val_upper);
+					      &val_upper);
       if (status == REG_VALID)
 	{
 	  val |= val_upper << 32;
@@ -1360,7 +1360,8 @@ s390_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 /* Implement pseudo_register_write gdbarch method.  */
 
 static void
-s390_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
+s390_pseudo_register_write (struct gdbarch *gdbarch,
+			    register_readwriter *regcache,
 			    int regnum, const gdb_byte *buf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -1373,7 +1374,7 @@ s390_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
       val = extract_unsigned_integer (buf, regsize, byte_order);
       if (register_size (gdbarch, S390_PSWA_REGNUM) == 4)
 	{
-	  regcache_raw_read_unsigned (regcache, S390_PSWA_REGNUM, &psw);
+	  regcache->raw_read (S390_PSWA_REGNUM, &psw);
 	  val = (psw & 0x80000000) | (val & 0x7fffffff);
 	}
       regcache_raw_write_unsigned (regcache, S390_PSWA_REGNUM, val);
@@ -1383,7 +1384,7 @@ s390_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
   if (regnum == tdep->cc_regnum)
     {
       val = extract_unsigned_integer (buf, regsize, byte_order);
-      regcache_raw_read_unsigned (regcache, S390_PSWM_REGNUM, &psw);
+      regcache->raw_read (S390_PSWM_REGNUM, &psw);
       if (register_size (gdbarch, S390_PSWA_REGNUM) == 4)
 	val = (psw & ~((ULONGEST)3 << 12)) | ((val & 3) << 12);
       else
diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
index fe64cf979a01..d2ef46a9a782 100644
--- a/gdb/sh-tdep.c
+++ b/gdb/sh-tdep.c
@@ -1627,7 +1627,7 @@ dr_reg_base_num (struct gdbarch *gdbarch, int dr_regnum)
 
 static enum register_status
 pseudo_register_read_portions (struct gdbarch *gdbarch,
-			       readable_regcache *regcache,
+			       register_reader *regcache,
 			       int portions,
 			       int base_regnum, gdb_byte *buffer)
 {
@@ -1648,7 +1648,7 @@ pseudo_register_read_portions (struct gdbarch *gdbarch,
 }
 
 static enum register_status
-sh_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+sh_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
 			 int reg_nr, gdb_byte *buffer)
 {
   int base_regnum;
@@ -1688,7 +1688,7 @@ sh_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 }
 
 static void
-sh_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
+sh_pseudo_register_write (struct gdbarch *gdbarch, register_readwriter *regcache,
 			  int reg_nr, const gdb_byte *buffer)
 {
   int base_regnum, portion;
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index 7a50a8d4a97d..6f560810dc9f 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -523,7 +523,7 @@ sparc32_register_type (struct gdbarch *gdbarch, int regnum)
 
 static enum register_status
 sparc32_pseudo_register_read (struct gdbarch *gdbarch,
-			      readable_regcache *regcache,
+			      register_reader *regcache,
 			      int regnum, gdb_byte *buf)
 {
   enum register_status status;
@@ -540,7 +540,7 @@ sparc32_pseudo_register_read (struct gdbarch *gdbarch,
 
 static void
 sparc32_pseudo_register_write (struct gdbarch *gdbarch,
-			       struct regcache *regcache,
+			       register_readwriter *regcache,
 			       int regnum, const gdb_byte *buf)
 {
   regnum -= gdbarch_num_regs (gdbarch);
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index b1ee6c1b57d0..5a707e8133c6 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -900,7 +900,7 @@ sparc64_register_type (struct gdbarch *gdbarch, int regnum)
 
 static enum register_status
 sparc64_pseudo_register_read (struct gdbarch *gdbarch,
-			      readable_regcache *regcache,
+			      register_reader *regcache,
 			      int regnum, gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -979,7 +979,7 @@ sparc64_pseudo_register_read (struct gdbarch *gdbarch,
 
 static void
 sparc64_pseudo_register_write (struct gdbarch *gdbarch,
-			       struct regcache *regcache,
+			       register_readwriter *regcache,
 			       int regnum, const gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1018,7 +1018,7 @@ sparc64_pseudo_register_write (struct gdbarch *gdbarch,
     {
       ULONGEST state, bits;
 
-      regcache_raw_read_unsigned (regcache, SPARC64_STATE_REGNUM, &state);
+      regcache->raw_read (SPARC64_STATE_REGNUM, &state);
       bits = extract_unsigned_integer (buf, 8, byte_order);
       switch (regnum)
 	{
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 6ae37f58c7a4..01e3dc135a43 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -182,7 +182,7 @@ spu_register_type (struct gdbarch *gdbarch, int reg_nr)
 /* Pseudo registers for preferred slots - stack pointer.  */
 
 static enum register_status
-spu_pseudo_register_read_spu (readable_regcache *regcache, const char *regname,
+spu_pseudo_register_read_spu (register_reader *regcache, const char *regname,
 			      gdb_byte *buf)
 {
   struct gdbarch *gdbarch = regcache->arch ();
@@ -207,7 +207,7 @@ spu_pseudo_register_read_spu (readable_regcache *regcache, const char *regname,
 }
 
 static enum register_status
-spu_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+spu_pseudo_register_read (struct gdbarch *gdbarch, register_reader *regcache,
                           int regnum, gdb_byte *buf)
 {
   gdb_byte reg[16];
@@ -250,7 +250,8 @@ spu_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 }
 
 static void
-spu_pseudo_register_write_spu (struct regcache *regcache, const char *regname,
+spu_pseudo_register_write_spu (register_readwriter *regcache,
+			       const char *regname,
 			       const gdb_byte *buf)
 {
   struct gdbarch *gdbarch = regcache->arch ();
@@ -259,7 +260,7 @@ spu_pseudo_register_write_spu (struct regcache *regcache, const char *regname,
   char annex[32];
   ULONGEST id;
 
-  regcache_raw_read_unsigned (regcache, SPU_ID_REGNUM, &id);
+  regcache->raw_read (SPU_ID_REGNUM, &id);
   xsnprintf (annex, sizeof annex, "%d/%s", (int) id, regname);
   xsnprintf (reg, sizeof reg, "0x%s",
 	     phex_nz (extract_unsigned_integer (buf, 4, byte_order), 4));
@@ -268,7 +269,8 @@ spu_pseudo_register_write_spu (struct regcache *regcache, const char *regname,
 }
 
 static void
-spu_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
+spu_pseudo_register_write (struct gdbarch *gdbarch,
+			   register_readwriter *regcache,
                            int regnum, const gdb_byte *buf)
 {
   gdb_byte reg[16];
@@ -284,7 +286,7 @@ spu_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
       break;
 
     case SPU_FPSCR_REGNUM:
-      regcache_raw_read_unsigned (regcache, SPU_ID_REGNUM, &id);
+      regcache->raw_read (SPU_ID_REGNUM, &id);
       xsnprintf (annex, sizeof annex, "%d/fpcr", (int) id);
       target_write (current_top_target (), TARGET_OBJECT_SPU, annex, buf, 0, 16);
       break;
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index 4f29557fbf65..9b92fe8ace51 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -362,7 +362,7 @@ xtensa_reg_to_regnum (struct gdbarch *gdbarch, int regnum)
    than or equal to 32 bits.  */
 
 static void
-xtensa_register_write_masked (struct regcache *regcache,
+xtensa_register_write_masked (register_readwriter *regcache,
 			      xtensa_register_t *reg, const gdb_byte *buffer)
 {
   unsigned int value[(XTENSA_MAX_REGISTER_SIZE + 3) / 4];
@@ -446,7 +446,7 @@ xtensa_register_write_masked (struct regcache *regcache,
    of the registers and assemble them into a single value.  */
 
 static enum register_status
-xtensa_register_read_masked (readable_regcache *regcache,
+xtensa_register_read_masked (register_reader *regcache,
 			     xtensa_register_t *reg, gdb_byte *buffer)
 {
   unsigned int value[(XTENSA_MAX_REGISTER_SIZE + 3) / 4];
@@ -540,7 +540,7 @@ xtensa_register_read_masked (readable_regcache *regcache,
 
 static enum register_status
 xtensa_pseudo_register_read (struct gdbarch *gdbarch,
-			     readable_regcache *regcache,
+			     register_reader *regcache,
 			     int regnum,
 			     gdb_byte *buffer)
 {
@@ -555,8 +555,7 @@ xtensa_pseudo_register_read (struct gdbarch *gdbarch,
       ULONGEST value;
       enum register_status status;
 
-      status = regcache->raw_read (gdbarch_tdep (gdbarch)->wb_regnum,
-				   &value);
+      status = regcache->raw_read (gdbarch_tdep (gdbarch)->wb_regnum, &value);
       if (status != REG_VALID)
 	return status;
       regnum = arreg_number (gdbarch, regnum, value);
@@ -631,7 +630,7 @@ xtensa_pseudo_register_read (struct gdbarch *gdbarch,
 
 static void
 xtensa_pseudo_register_write (struct gdbarch *gdbarch,
-			      struct regcache *regcache,
+			      register_readwriter *regcache,
 			      int regnum,
 			      const gdb_byte *buffer)
 {
@@ -644,8 +643,7 @@ xtensa_pseudo_register_write (struct gdbarch *gdbarch,
       && (regnum <= gdbarch_tdep (gdbarch)->a0_base + 15))
     {
       ULONGEST value;
-      regcache_raw_read_unsigned (regcache,
-				  gdbarch_tdep (gdbarch)->wb_regnum, &value);
+      regcache->raw_read (gdbarch_tdep (gdbarch)->wb_regnum, &value);
       regnum = arreg_number (gdbarch, regnum, value);
     }
 
-- 
2.19.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 0/3] Read pseudo registers from frame instead of regcache
@ 2018-10-24  1:43 Simon Marchi
  2018-10-24  1:43 ` [PATCH v2 2/3] " Simon Marchi
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Simon Marchi @ 2018-10-24  1:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand, Tom Tromey, Simon Marchi

Hi,

This is a follow-up of

  https://sourceware.org/ml/gdb-patches/2018-05/msg00705.html

The main changes are:

- Re-use the regcache interface to read registers from frames
- Deal with debug info that contains unwind info for pseudo registers

Simon Marchi (3):
  Extract register_reader and register_readwriter interfaces from
    regcache
  Read pseudo registers from frame instead of regcache
  Add tests for unwinding of pseudo registers

 gdb/aarch64-tdep.c                            |  12 +-
 gdb/amd64-tdep.c                              |   4 +-
 gdb/arm-tdep.c                                |  12 +-
 gdb/avr-tdep.c                                |   4 +-
 gdb/bfin-tdep.c                               |   5 +-
 gdb/dwarf2-frame.c                            |  12 +-
 gdb/frame.c                                   | 122 ++++++++++++-
 gdb/frv-tdep.c                                |   7 +-
 gdb/gdbarch.c                                 |  12 +-
 gdb/gdbarch.h                                 |  12 +-
 gdb/gdbarch.sh                                |   6 +-
 gdb/h8300-tdep.c                              |   9 +-
 gdb/hppa-tdep.c                               |   2 +-
 gdb/i386-tdep.c                               |   9 +-
 gdb/i386-tdep.h                               |   4 +-
 gdb/ia64-libunwind-tdep.c                     |   2 +-
 gdb/ia64-libunwind-tdep.h                     |   2 +-
 gdb/ia64-tdep.c                               |   5 +-
 gdb/m32c-tdep.c                               |  38 +++--
 gdb/m68hc11-tdep.c                            |   4 +-
 gdb/mep-tdep.c                                |  20 +--
 gdb/mips-tdep.c                               |   4 +-
 gdb/msp430-tdep.c                             |   4 +-
 gdb/nds32-tdep.c                              |   4 +-
 gdb/regcache.c                                | 118 ++++++-------
 gdb/regcache.h                                | 161 +++++++++++-------
 gdb/rl78-tdep.c                               |   8 +-
 gdb/rs6000-tdep.c                             |  34 ++--
 gdb/s390-tdep.c                               |  11 +-
 gdb/sh-tdep.c                                 |   6 +-
 gdb/sparc-tdep.c                              |   4 +-
 gdb/sparc64-tdep.c                            |   6 +-
 gdb/spu-tdep.c                                |  14 +-
 .../gdb.arch/aarch64-pseudo-unwind-asm.S      |  78 +++++++++
 .../gdb.arch/aarch64-pseudo-unwind.c          |  33 ++++
 .../gdb.arch/aarch64-pseudo-unwind.exp        |  89 ++++++++++
 .../gdb.arch/amd64-pseudo-unwind-asm.S        |  63 +++++++
 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c  |  33 ++++
 .../gdb.arch/amd64-pseudo-unwind.exp          |  90 ++++++++++
 .../gdb.arch/arm-pseudo-unwind-asm.S          |  76 +++++++++
 .../gdb.arch/arm-pseudo-unwind-legacy-asm.S   |  79 +++++++++
 .../gdb.arch/arm-pseudo-unwind-legacy.c       |  33 ++++
 .../gdb.arch/arm-pseudo-unwind-legacy.exp     |  85 +++++++++
 gdb/testsuite/gdb.arch/arm-pseudo-unwind.c    |  33 ++++
 gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp  |  87 ++++++++++
 gdb/xtensa-tdep.c                             |  14 +-
 46 files changed, 1209 insertions(+), 261 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind-asm.S
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-pseudo-unwind.exp
 create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
 create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c
 create mode 100644 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.exp
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-asm.S
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy-asm.S
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.c
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind-legacy.exp
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind.c
 create mode 100644 gdb/testsuite/gdb.arch/arm-pseudo-unwind.exp

-- 
2.19.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/3] Read pseudo registers from frame instead of regcache
  2018-10-24  1:43 [PATCH v2 0/3] Read pseudo registers from frame instead of regcache Simon Marchi
                   ` (2 preceding siblings ...)
  2018-10-24  1:43 ` [PATCH v2 3/3] Add tests for unwinding of pseudo registers Simon Marchi
@ 2019-02-08 13:47 ` Simon Marchi
  2019-02-11 16:56 ` Ulrich Weigand
  4 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2019-02-08 13:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand, Tom Tromey

On 2018-10-23 21:43, Simon Marchi wrote:
> Hi,
> 
> This is a follow-up of
> 
>   https://sourceware.org/ml/gdb-patches/2018-05/msg00705.html
> 
> The main changes are:
> 
> - Re-use the regcache interface to read registers from frames
> - Deal with debug info that contains unwind info for pseudo registers
> 
> Simon Marchi (3):
>   Extract register_reader and register_readwriter interfaces from
>     regcache
>   Read pseudo registers from frame instead of regcache
>   Add tests for unwinding of pseudo registers

Ping.  I would appreciate if somebody could take a look at this, since 
it's not something I would be comfortable merging unilaterally / without 
reviews.

Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/3] Extract register_reader and register_readwriter interfaces from regcache
  2018-10-24  1:43 ` [PATCH v2 1/3] Extract register_reader and register_readwriter interfaces from regcache Simon Marchi
@ 2019-02-08 16:47   ` John Baldwin
  2019-02-09  5:24     ` Simon Marchi
  0 siblings, 1 reply; 9+ messages in thread
From: John Baldwin @ 2019-02-08 16:47 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches; +Cc: Ulrich Weigand, Tom Tromey, Simon Marchi

On 10/23/18 6:43 PM, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@ericsson.com>
> 
> In the following patch, we make gdbarch pseudo-registers hooks read and
> write required registers from a frame instead of from the current
> regcache.  To avoid having to change the implementation of all
> architectures to use a different interface, we can re-use the regcache
> interface.
> 
> This patch extracts two interfaces, register_reader and
> register_readwriter, and make respectively readable_regcache and
> regcache inherit from them.  register_reader is "something from which
> you can read register values from".  It can of course be a regcache, but
> it will also be (in the following patch) something that unwinds
> registers for a particular frame.  As you would expect,
> register_readwriter is "something you can read and write registers
> from/to".
> 
> Some notes about the implementation.  This results in diamond
> inheritance: regcache inherits from both readable_regcache and
> register_readwriter, which both inherit from register_reader.  It is
> therefore necessary to use virtual inheritance (use "public virtual"),
> otherwises we end up with errors like:
> 
>   /home/emaisin/src/binutils-gdb/gdb/regcache.c:210:20: error: request
>   for member ‘cooked_read’ is ambiguous
> 
> Also, the raw_read method in readable_regcache hides the raw_read
> template method in register_reader.  So it's necessary to use "using
> register_reader::raw_read" so that clients of readable_regcache are able
> to call register_reader's raw_read.  Same thing for some cooked_read,
> raw_write and cooked_write.
> 
> All corresponding gdbarch hooks are updated to use register_reader or
> register_readwriter instead of readable_regcache and regcache, but
> otherwise they are not changed.
> 
> With this patch, no behavior change is expected.
> 
> @@ -539,20 +541,19 @@ regcache_raw_read_signed (struct regcache *regcache, int regnum, LONGEST *val)
>  
>  template<typename T, typename>
>  enum register_status
> -readable_regcache::raw_read (int regnum, T *val)
> +register_reader::raw_read (int regnum, T *val)
>  {
> -  gdb_byte *buf;
> -  enum register_status status;
> +  gdbarch *arch = this->arch ();
> +  int reg_size = register_size (arch, regnum);
> +  gdb_byte buf[reg_size];
> +
> +  register_status status = raw_read (regnum, buf);
>  
> -  assert_regnum (regnum);
> -  buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
> -  status = raw_read (regnum, buf);

This "loses" the assert_regnum().  Is that replaced for regcache's by the
gdb_assert() you added in readable_regcache::raw_read()?  Did you consider
moving that assertion to here instead so it would also be enforced in the
frame version of register_reader?  (Or does the frame version also add its
own assertion in the following patches?)

Similar question about register_readwriter::raw_write,
register_reader::cooked_read, and register_readerwriter::cooked_write.

The only other general comment is that while I appreciate that this doesn't
require changing the tdep files very much (which is a good thing), it does
have the odd result that the variable names in the gdbarch methods are still
named 'regcache' even though they aren't regcache's anymore.  It's perhaps
not worth it, but it might be nice to do a followup pass eventually to
rename 'register_reader *regcache' to 'reader' and something similar for
the writer case?  It seems tedious though, and I don't think it should be a 
requirement/blocker, just a suggestion for the future perhaps.

-- 
John Baldwin

                                                                            

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/3] Read pseudo registers from frame instead of regcache
  2018-10-24  1:43 ` [PATCH v2 2/3] " Simon Marchi
@ 2019-02-08 16:53   ` John Baldwin
  0 siblings, 0 replies; 9+ messages in thread
From: John Baldwin @ 2019-02-08 16:53 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches; +Cc: Ulrich Weigand, Tom Tromey, Simon Marchi

On 10/23/18 6:43 PM, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@ericsson.com>
> 
> Reading pseudo registers (made of one or more raw register) from an
> unwound stack frame (frame number > #0) does not work.  The raw
> registers needed to compose the pseudo registers are always read from
> the current thread's regcache, which is effectively frame #0.
>
> ...
> 
> @@ -1203,9 +1263,41 @@ frame_unwind_register_value (frame_info *next_frame, int regnum)
>      frame_unwind_find_by_frame (next_frame, &next_frame->prologue_cache);
>  
>    /* Ask this frame to unwind its register.  */
> -  value = next_frame->unwind->prev_register (next_frame,
> -					     &next_frame->prologue_cache,
> -					     regnum);
> +  struct value *value
> +    = next_frame->unwind->prev_register (next_frame,
> +					 &next_frame->prologue_cache, regnum);
> +
> +  if (value == nullptr)
> +    {
> +      frame_register_reader reg_read (next_frame);
> +
> +      if (gdbarch_pseudo_register_read_value_p (gdbarch))
> +	{
> +	  /* This is a pseudo register, we don't know how how what raw registers
> +	     this pseudo register is made of.  Ask the gdbarch to read the
> +	     value, it will itself ask the next frame to unwind the values of
> +	     the raw registers it needs to compose the value of the pseudo
> +	     register.  */
> +	  value
> +	    = gdbarch_pseudo_register_read_value (gdbarch, &reg_read, regnum);
> +	  VALUE_LVAL (value) = not_lval;
> +	}
> +      else if (gdbarch_pseudo_register_read_p (gdbarch))
> +	{
> +	  value = allocate_value (register_type (gdbarch, regnum));
> +	  VALUE_LVAL (value) = not_lval;
> +
> +	  register_status st
> +	    = gdbarch_pseudo_register_read (gdbarch, &reg_read, regnum,
> +					    value_contents_raw (value));
> +	  if (st == REG_UNAVAILABLE)
> +	    mark_value_bytes_unavailable (value, 0,
> +					  TYPE_LENGTH (value_type (value)));
> +	}
> +      else
> +	error (_("Can't unwind value of register %d (%s)"), regnum,
> +	       user_reg_map_regnum_to_name (gdbarch, regnum));
> +    }

The only thought I have here (which is a bit orthogonal I think) is if
gdbarch shouldn't have a default implementation of
gdbarch_pseudo_register_read_value that does what your else branch does
above?

-- 
John Baldwin

                                                                            

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/3] Extract register_reader and register_readwriter interfaces from regcache
  2019-02-08 16:47   ` John Baldwin
@ 2019-02-09  5:24     ` Simon Marchi
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2019-02-09  5:24 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches, Ulrich Weigand, Tom Tromey, Simon Marchi

On 2019-02-08 11:46, John Baldwin wrote:
> On 10/23/18 6:43 PM, Simon Marchi wrote:
>> From: Simon Marchi <simon.marchi@ericsson.com>
>> 
>> In the following patch, we make gdbarch pseudo-registers hooks read 
>> and
>> write required registers from a frame instead of from the current
>> regcache.  To avoid having to change the implementation of all
>> architectures to use a different interface, we can re-use the regcache
>> interface.
>> 
>> This patch extracts two interfaces, register_reader and
>> register_readwriter, and make respectively readable_regcache and
>> regcache inherit from them.  register_reader is "something from which
>> you can read register values from".  It can of course be a regcache, 
>> but
>> it will also be (in the following patch) something that unwinds
>> registers for a particular frame.  As you would expect,
>> register_readwriter is "something you can read and write registers
>> from/to".
>> 
>> Some notes about the implementation.  This results in diamond
>> inheritance: regcache inherits from both readable_regcache and
>> register_readwriter, which both inherit from register_reader.  It is
>> therefore necessary to use virtual inheritance (use "public virtual"),
>> otherwises we end up with errors like:
>> 
>>   /home/emaisin/src/binutils-gdb/gdb/regcache.c:210:20: error: request
>>   for member ‘cooked_read’ is ambiguous
>> 
>> Also, the raw_read method in readable_regcache hides the raw_read
>> template method in register_reader.  So it's necessary to use "using
>> register_reader::raw_read" so that clients of readable_regcache are 
>> able
>> to call register_reader's raw_read.  Same thing for some cooked_read,
>> raw_write and cooked_write.
>> 
>> All corresponding gdbarch hooks are updated to use register_reader or
>> register_readwriter instead of readable_regcache and regcache, but
>> otherwise they are not changed.
>> 
>> With this patch, no behavior change is expected.
>> 
>> @@ -539,20 +541,19 @@ regcache_raw_read_signed (struct regcache 
>> *regcache, int regnum, LONGEST *val)
>> 
>>  template<typename T, typename>
>>  enum register_status
>> -readable_regcache::raw_read (int regnum, T *val)
>> +register_reader::raw_read (int regnum, T *val)
>>  {
>> -  gdb_byte *buf;
>> -  enum register_status status;
>> +  gdbarch *arch = this->arch ();
>> +  int reg_size = register_size (arch, regnum);
>> +  gdb_byte buf[reg_size];
>> +
>> +  register_status status = raw_read (regnum, buf);
>> 
>> -  assert_regnum (regnum);
>> -  buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
>> -  status = raw_read (regnum, buf);
> 
> This "loses" the assert_regnum().  Is that replaced for regcache's by 
> the
> gdb_assert() you added in readable_regcache::raw_read()?  Did you 
> consider
> moving that assertion to here instead so it would also be enforced in 
> the
> frame version of register_reader?  (Or does the frame version also add 
> its
> own assertion in the following patches?)

Good point.  I don't recall exactly, but the idea was probably to have 
the assert in a single place on the code path, to avoid asserting the 
same thing multiple times in a row, if possible.  I don't think that the 
frame version adds a similar assert, I'll look into adding it there.

> Similar question about register_readwriter::raw_write,
> register_reader::cooked_read, and register_readerwriter::cooked_write.
> 
> The only other general comment is that while I appreciate that this 
> doesn't
> require changing the tdep files very much (which is a good thing), it 
> does
> have the odd result that the variable names in the gdbarch methods are 
> still
> named 'regcache' even though they aren't regcache's anymore.  It's 
> perhaps
> not worth it, but it might be nice to do a followup pass eventually to
> rename 'register_reader *regcache' to 'reader' and something similar 
> for
> the writer case?  It seems tedious though, and I don't think it should 
> be a
> requirement/blocker, just a suggestion for the future perhaps.

I agree.  I'll look into doing this as a follow-up pass (it would just 
add noise to this patch).

Thanks,

Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/3] Read pseudo registers from frame instead of regcache
  2018-10-24  1:43 [PATCH v2 0/3] Read pseudo registers from frame instead of regcache Simon Marchi
                   ` (3 preceding siblings ...)
  2019-02-08 13:47 ` [PATCH v2 0/3] Read pseudo registers from frame instead of regcache Simon Marchi
@ 2019-02-11 16:56 ` Ulrich Weigand
  4 siblings, 0 replies; 9+ messages in thread
From: Ulrich Weigand @ 2019-02-11 16:56 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, Tom Tromey, Simon Marchi

Simon Marchi wrote:

> This is a follow-up of
> 
>   https://sourceware.org/ml/gdb-patches/2018-05/msg00705.html
> 
> The main changes are:
> 
> - Re-use the regcache interface to read registers from frames
> - Deal with debug info that contains unwind info for pseudo registers

Sorry for not looking at this earlier ...

I'm not sure I fully agree with this approach, it seems to make the
frame vs. regcache distinction even more confusing (at least to me)
than it already was :-)

We used to have pseudo-registers as a property of the regcache, and
at the frame level there was really no such distinction.  Now, there
may be good reasons to have pseudo-registers instead a property of
the frame, so that they can be independently constructed at each
frame level.  But what seems a bit confusing is to have them as
properties of *both* layers ...

From that perspective, I actually liked your original approach
better, where you modified the pseudo register read/write
routines to operate on a frame instead of a regcache.  Why did
you decide to move away from that?  Is it just to avoid having
to change all back ends?

In case, maybe we can have a gradual transition here by allowing
targets to introduce new per-frame pseudo-register accessors,
and fall back to the per-regcache accessors if we're at the
sentinel frame and the gdbarch has no per-frame accessors.

(As an aside, the new interfaces should all be value-based, so
we'd merge that transition into the gdbarch_pseudo_register_read
vs. gdbarch_pesudo_register_read_value transition.)

This would also make the new register_readers interfaces
unnecessary.  While I'm not really the C++ expert, I'm not
sure I like the use of multiple (in particular virtual)
inheritance here; I thought this was typically one of the
features that people are trying to avoid ...  (B.t.w. do
we have a formal coding style documenting which C++ features
we want to use vs. avoid in GDB code?)

As a minor thing that occurred to me in the patch itself:

+	  value
+	    = gdbarch_pseudo_register_read_value (gdbarch, &reg_read, regnum);
+	  VALUE_LVAL (value) = not_lval;

We really want to get away from "resetting" VALUE_LVAL to
something else.  Instead, values should be created correctly
once in the first place.  This is another of those decades-long
transitions we have, but we should probably avoid making it worse.

Note that the implementation in amd64_pseudo_register_read_value
already does:
  value *result_value = allocate_value (register_type (gdbarch, regnum));
  VALUE_LVAL (result_value) = lval_register;
  VALUE_REGNUM (result_value) = regnum;
  gdb_byte *buf = value_contents_raw (result_value);

which also violates the rule, and is actually wrong since lval_register
values really should always be based relative to a *frame*.  This is
another argument b.t.w. for making the _pseudo_register_read_value
routines be frame-based ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-02-11 16:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-24  1:43 [PATCH v2 0/3] Read pseudo registers from frame instead of regcache Simon Marchi
2018-10-24  1:43 ` [PATCH v2 2/3] " Simon Marchi
2019-02-08 16:53   ` John Baldwin
2018-10-24  1:43 ` [PATCH v2 1/3] Extract register_reader and register_readwriter interfaces from regcache Simon Marchi
2019-02-08 16:47   ` John Baldwin
2019-02-09  5:24     ` Simon Marchi
2018-10-24  1:43 ` [PATCH v2 3/3] Add tests for unwinding of pseudo registers Simon Marchi
2019-02-08 13:47 ` [PATCH v2 0/3] Read pseudo registers from frame instead of regcache Simon Marchi
2019-02-11 16:56 ` Ulrich Weigand

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).