[RFC PATCH] Read pseudo registers from frame instead of regcache

[RFC PATCH] Read pseudo registers from frame instead of regcache

[Simon Marchi]

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

I would like to fix the problem described below.  The solution I
currently have would require me to change some gdbarch hooks and
therefore touch every architecture.  It will take quite some time and
it's not always trivial, so before doing this, I'd like to gather
comments to know if my approach looks right or wrong.

NOTE: Don't use you --enable-targets=all build for this, it will only
build for x86-64, i386, ARM and AVR.

The problem: Reading pseudo registers from an upper stack frame 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's registers.

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 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.  Instead, I think
the "read" hooks should take the "next frame" and read the register
values it needs from it.  In the example above,
amd64_pseudo_register_read_value would properly get the value of rbx for
frame #1 (which is at that point saved on the stack).

Similarly, the write hook would receive a frame and read/write raw
registers through that.

Another thing I'm wondering about is error handling.  In
amd64_pseudo_register_read_value, I changed functions that returned a
register_status for functions that throw errors when values are
unavailable.  I'm not sure if we should keep handling
unavailable/optimized out values in each arch hook, or let the hooks
throw and handle it in the callers.

Finally, this change affected the cooked_write_test and cooked_read_test
tests.  I had to set up a bit more fake global state to get them
to pass.  This is because reading/writing pseudo registers now involve
frames, and in particular the sentinel frame, which was not created
before.

Comments on any of this are very welcome!
---
 gdb/amd64-tdep.c                              |  60 ++---
 gdb/arm-tdep.c                                |  20 +-
 gdb/avr-tdep.c                                |  19 +-
 gdb/frame.c                                   |  83 ++++---
 gdb/gdbarch.c                                 |   8 +-
 gdb/gdbarch.h                                 |   8 +-
 gdb/gdbarch.sh                                |   4 +-
 gdb/i386-tdep.c                               | 209 ++++++------------
 gdb/i386-tdep.h                               |   4 +-
 gdb/regcache.c                                |  45 +++-
 .../gdb.arch/amd64-pseudo-unwind-asm.S        |  43 ++++
 gdb/testsuite/gdb.arch/amd64-pseudo-unwind.c  |  33 +++
 .../gdb.arch/amd64-pseudo-unwind.exp          |  81 +++++++
 gdb/value.c                                   |  16 +-
 gdb/value.h                                   |   3 +
 15 files changed, 397 insertions(+), 239 deletions(-)
 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

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index d555465c2f96..12ec8294e4ee 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -349,19 +349,15 @@ amd64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
 
 static struct value *
 amd64_pseudo_register_read_value (struct gdbarch *gdbarch,
-				  readable_regcache *regcache,
+				  frame_info *next_frame,
 				  int regnum)
 {
   gdb_byte *raw_buf = (gdb_byte *) alloca (register_size (gdbarch, regnum));
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  enum register_status status;
-  struct value *result_value;
-  gdb_byte *buf;
-
-  result_value = allocate_value (register_type (gdbarch, regnum));
-  VALUE_LVAL (result_value) = lval_register;
-  VALUE_REGNUM (result_value) = regnum;
-  buf = value_contents_raw (result_value);
+  value *result_value
+    = allocate_register_value (gdbarch, regnum, next_frame);
+  gdb_byte *buf = value_contents_raw (result_value);
+  set_value_lazy (result_value, 0);
 
   if (i386_byte_regnum_p (gdbarch, regnum))
     {
@@ -371,37 +367,25 @@ amd64_pseudo_register_read_value (struct gdbarch *gdbarch,
       if (gpnum >= AMD64_NUM_LOWER_BYTE_REGS)
 	{
 	  /* Special handling for AH, BH, CH, DH.  */
-	  status = regcache->raw_read (gpnum - AMD64_NUM_LOWER_BYTE_REGS,
-				       raw_buf);
-	  if (status == REG_VALID)
-	    memcpy (buf, raw_buf + 1, 1);
-	  else
-	    mark_value_bytes_unavailable (result_value, 0,
-					  TYPE_LENGTH (value_type (result_value)));
+	  frame_unwind_register (next_frame, gpnum - AMD64_NUM_LOWER_BYTE_REGS,
+				 raw_buf);
+	  memcpy (buf, raw_buf + 1, 1);
 	}
       else
 	{
-	  status = regcache->raw_read (gpnum, raw_buf);
-	  if (status == REG_VALID)
-	    memcpy (buf, raw_buf, 1);
-	  else
-	    mark_value_bytes_unavailable (result_value, 0,
-					  TYPE_LENGTH (value_type (result_value)));
+	  frame_unwind_register (next_frame, gpnum, raw_buf);
+	  memcpy (buf, raw_buf, 1);
 	}
     }
   else if (i386_dword_regnum_p (gdbarch, regnum))
     {
       int gpnum = regnum - tdep->eax_regnum;
       /* Extract (always little endian).  */
-      status = regcache->raw_read (gpnum, raw_buf);
-      if (status == REG_VALID)
-	memcpy (buf, raw_buf, 4);
-      else
-	mark_value_bytes_unavailable (result_value, 0,
-				      TYPE_LENGTH (value_type (result_value)));
+      frame_unwind_register (next_frame, gpnum, raw_buf);
+      memcpy (buf, raw_buf, 4);
     }
   else
-    i386_pseudo_register_read_into_value (gdbarch, regcache, regnum,
+    i386_pseudo_register_read_into_value (gdbarch, next_frame, regnum,
 					  result_value);
 
   return result_value;
@@ -409,7 +393,7 @@ amd64_pseudo_register_read_value (struct gdbarch *gdbarch,
 
 static void
 amd64_pseudo_register_write (struct gdbarch *gdbarch,
-			     struct regcache *regcache,
+			     frame_info *this_frame,
 			     int regnum, const gdb_byte *buf)
 {
   gdb_byte *raw_buf = (gdb_byte *) alloca (register_size (gdbarch, regnum));
@@ -422,22 +406,20 @@ amd64_pseudo_register_write (struct gdbarch *gdbarch,
       if (gpnum >= AMD64_NUM_LOWER_BYTE_REGS)
 	{
 	  /* Read ... AH, BH, CH, DH.  */
-	  regcache_raw_read (regcache,
-			     gpnum - AMD64_NUM_LOWER_BYTE_REGS, raw_buf);
+	  get_frame_register (this_frame, gpnum - AMD64_NUM_LOWER_BYTE_REGS, raw_buf);
 	  /* ... Modify ... (always little endian).  */
 	  memcpy (raw_buf + 1, buf, 1);
 	  /* ... Write.  */
-	  regcache_raw_write (regcache,
-			      gpnum - AMD64_NUM_LOWER_BYTE_REGS, raw_buf);
+	  put_frame_register (this_frame, gpnum - AMD64_NUM_LOWER_BYTE_REGS, raw_buf);
 	}
       else
 	{
 	  /* Read ...  */
-	  regcache_raw_read (regcache, gpnum, raw_buf);
+	  get_frame_register (this_frame, gpnum, raw_buf);
 	  /* ... Modify ... (always little endian).  */
 	  memcpy (raw_buf, buf, 1);
 	  /* ... Write.  */
-	  regcache_raw_write (regcache, gpnum, raw_buf);
+	  put_frame_register (this_frame, gpnum, raw_buf);
 	}
     }
   else if (i386_dword_regnum_p (gdbarch, regnum))
@@ -445,14 +427,14 @@ amd64_pseudo_register_write (struct gdbarch *gdbarch,
       int gpnum = regnum - tdep->eax_regnum;
 
       /* Read ...  */
-      regcache_raw_read (regcache, gpnum, raw_buf);
+      get_frame_register (this_frame, gpnum, raw_buf);
       /* ... Modify ... (always little endian).  */
       memcpy (raw_buf, buf, 4);
       /* ... Write.  */
-      regcache_raw_write (regcache, gpnum, raw_buf);
+      put_frame_register (this_frame, gpnum, raw_buf);
     }
   else
-    i386_pseudo_register_write (gdbarch, regcache, regnum, buf);
+    i386_pseudo_register_write (gdbarch, this_frame, regnum, buf);
 }
 
 /* Implement the 'ax_pseudo_register_collect' gdbarch method.  */
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 382080a714b6..123ab9e2a03d 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -230,7 +230,7 @@ static enum register_status arm_neon_quad_read (struct gdbarch *gdbarch,
 						readable_regcache *regcache,
 						int regnum, gdb_byte *buf);
 static void arm_neon_quad_write (struct gdbarch *gdbarch,
-				 struct regcache *regcache,
+				 frame_info *this_frame,
 				 int regnum, const gdb_byte *buf);
 
 static CORE_ADDR
@@ -3796,7 +3796,7 @@ arm_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		  char name_buf[4];
 		  int regnum;
 		  if (reg_char == 'q')
-		    arm_neon_quad_write (gdbarch, regcache, reg_scaled + i,
+		    arm_neon_quad_write (gdbarch, get_current_frame (), reg_scaled + i,
 					 val + i * unit_length);
 		  else
 		    {
@@ -8198,7 +8198,7 @@ arm_return_value (struct gdbarch *gdbarch, struct value *function,
 	  if (reg_char == 'q')
 	    {
 	      if (writebuf)
-		arm_neon_quad_write (gdbarch, regcache, i,
+		arm_neon_quad_write (gdbarch, get_current_frame (), i,
 				     writebuf + i * unit_length);
 
 	      if (readbuf)
@@ -8749,7 +8749,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, frame_info *this_frame,
 		     int regnum, const gdb_byte *buf)
 {
   char name_buf[4];
@@ -8765,13 +8765,13 @@ arm_neon_quad_write (struct gdbarch *gdbarch, struct regcache *regcache,
   else
     offset = 0;
 
-  regcache_raw_write (regcache, double_regnum, buf + offset);
+  put_frame_register (this_frame, double_regnum, buf + offset);
   offset = 8 - offset;
-  regcache_raw_write (regcache, double_regnum + 1, buf + offset);
+  put_frame_register (this_frame, double_regnum + 1, buf + offset);
 }
 
 static void
-arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
+arm_pseudo_write (struct gdbarch *gdbarch, frame_info *this_frame,
 		  int regnum, const gdb_byte *buf)
 {
   const int num_regs = gdbarch_num_regs (gdbarch);
@@ -8784,7 +8784,7 @@ arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
 
   if (gdbarch_tdep (gdbarch)->have_neon_pseudos && regnum >= 32 && regnum < 48)
     /* Quad-precision register.  */
-    arm_neon_quad_write (gdbarch, regcache, regnum - 32, buf);
+    arm_neon_quad_write (gdbarch, this_frame, regnum - 32, buf);
   else
     {
       /* Single-precision register.  */
@@ -8800,9 +8800,9 @@ arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
       double_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
 						   strlen (name_buf));
 
-      regcache_raw_read (regcache, double_regnum, reg_buf);
+      get_frame_register (this_frame, double_regnum, reg_buf);
       memcpy (reg_buf + offset, buf, 4);
-      regcache_raw_write (regcache, double_regnum, reg_buf);
+      put_frame_register (this_frame, double_regnum, reg_buf);
     }
 }
 
diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index 5aa61ba2d9e7..d1addbad72b3 100644
--- a/gdb/avr-tdep.c
+++ b/gdb/avr-tdep.c
@@ -404,18 +404,23 @@ 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, frame_info *this_frame,
                            int regnum, const gdb_byte *buf)
 {
-  ULONGEST val;
-
   switch (regnum)
     {
     case AVR_PSEUDO_PC_REGNUM:
-      val = extract_unsigned_integer (buf, 4, gdbarch_byte_order (gdbarch));
-      val <<= 1;
-      regcache_raw_write_unsigned (regcache, AVR_PC_REGNUM, val);
-      break;
+      {
+	bfd_endian endianness = gdbarch_byte_order (gdbarch);
+	ULONGEST val = extract_unsigned_integer (buf, 4, endianness);
+	val <<= 1;
+
+	int pc_size = register_size (gdbarch, AVR_PC_REGNUM);
+	gdb_byte pc_buf[pc_size];
+	store_unsigned_integer (pc_buf, pc_size, endianness, val);
+	put_frame_register (this_frame, AVR_PC_REGNUM, pc_buf);
+	break;
+      }
     default:
       internal_error (__FILE__, __LINE__, _("invalid regnum"));
     }
diff --git a/gdb/frame.c b/gdb/frame.c
index c0f6e64dfc25..0181655b2d60 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1183,29 +1183,48 @@ get_frame_register (struct frame_info *frame,
 }
 
 struct value *
-frame_unwind_register_value (struct frame_info *frame, int regnum)
+frame_unwind_register_value (struct frame_info *next_frame, int regnum)
 {
-  struct gdbarch *gdbarch;
-  struct value *value;
+  gdb_assert (next_frame != NULL);
 
-  gdb_assert (frame != NULL);
-  gdbarch = frame_unwind_arch (frame);
+  struct gdbarch *gdbarch = frame_unwind_arch (next_frame);
+
+  gdb_assert (regnum >= 0);
+  gdb_assert (regnum < (gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch)));
 
   if (frame_debug)
     {
       fprintf_unfiltered (gdb_stdlog,
 			  "{ frame_unwind_register_value "
 			  "(frame=%d,regnum=%d(%s),...) ",
-			  frame->level, regnum,
+			  next_frame->level, regnum,
 			  user_reg_map_regnum_to_name (gdbarch, regnum));
     }
 
   /* Find the unwinder.  */
-  if (frame->unwind == NULL)
-    frame_unwind_find_by_frame (frame, &frame->prologue_cache);
+  if (next_frame->unwind == NULL)
+    frame_unwind_find_by_frame (next_frame, &next_frame->prologue_cache);
 
-  /* Ask this frame to unwind its register.  */
-  value = frame->unwind->prev_register (frame, &frame->prologue_cache, regnum);
+  struct value *value;
+  if (regnum < gdbarch_num_regs (gdbarch))
+    {
+      /* This is a raw register, we can directly ask the next frame to unwind
+         the register.  */
+      value = next_frame->unwind->prev_register (next_frame,
+						 &next_frame->prologue_cache,
+						 regnum);
+    }
+  else 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, next_frame, regnum);
+    }
+  else
+    error (_("Can't unwind value of register %d (%s)"), regnum,
+	   user_reg_map_regnum_to_name (gdbarch, regnum));
 
   if (frame_debug)
     {
@@ -1353,22 +1372,36 @@ put_frame_register (struct frame_info *frame, int regnum,
   enum lval_type lval;
   CORE_ADDR addr;
 
-  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)
+  gdb_assert (regnum >= 0);
+  gdb_assert (regnum < (gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch)));
+
+  if (regnum < gdbarch_num_regs (gdbarch))
     {
-    case lval_memory:
-      {
-	write_memory (addr, buf, register_size (gdbarch, regnum));
-	break;
-      }
-    case lval_register:
-      regcache_cooked_write (get_current_regcache (), realnum, buf);
-      break;
-    default:
-      error (_("Attempt to assign to an unmodifiable value."));
+      /* This is a raw register, we can find directly where it is saved
+         currently and update it.  */
+      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:
+	  {
+	    write_memory (addr, buf, register_size (gdbarch, regnum));
+	    break;
+	  }
+	case lval_register:
+	  regcache_cooked_write (get_current_regcache (), realnum, buf);
+	  break;
+	default:
+	  error (_("Attempt to assign to an unmodifiable value."));
+	}
+    }
+  else
+    {
+      /* This is a pseudo-register, the arch will find out which raw registers
+         to modify and update them.  */
+      gdbarch_pseudo_register_write (gdbarch, frame, regnum, buf);
     }
 }
 
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index c430ebe52a4f..a284a4bfc298 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -1979,13 +1979,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, frame_info *this_frame, 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, this_frame, cookednum);
 }
 
 void
@@ -2003,13 +2003,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, frame_info *this_frame, 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, this_frame, cookednum, buf);
 }
 
 void
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 09edcd5eb282..4a315bb8eb24 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -271,14 +271,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, frame_info *this_frame, int cookednum);
+extern struct value * gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, frame_info *this_frame, 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, frame_info *this_frame, int cookednum, const gdb_byte *buf);
+extern void gdbarch_pseudo_register_write (struct gdbarch *gdbarch, frame_info *this_frame, 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 733043093742..5a1b53ee9d1b 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -431,8 +431,8 @@ M;enum register_status;pseudo_register_read;readable_regcache *regcache, int coo
 # 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;frame_info *next_frame, int cookednum;next_frame, cookednum
+M;void;pseudo_register_write;frame_info *this_frame, int cookednum, const gdb_byte *buf;this_frame, cookednum, buf
 #
 v;int;num_regs;;;0;-1
 # This macro gives the number of pseudo-registers that live in the
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 646f305edc51..73a87a613f7d 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3249,17 +3249,14 @@ 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 (frame_info *next_frame, int regnum)
 {
-  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
-  int mmxreg, fpreg;
-  ULONGEST fstat;
-  int tos;
-
-  mmxreg = regnum - tdep->mm0_regnum;
-  regcache->raw_read (I387_FSTAT_REGNUM (tdep), &fstat);
-  tos = (fstat >> 11) & 0x7;
-  fpreg = (mmxreg + tos) % 8;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (get_frame_arch (next_frame));
+  int mmxreg = regnum - tdep->mm0_regnum;
+  ULONGEST fstat
+    = frame_unwind_register_unsigned (next_frame, I387_FSTAT_REGNUM (tdep));
+  int tos = (fstat >> 11) & 0x7;
+  int fpreg = (mmxreg + tos) % 8;
 
   return (I387_ST0_REGNUM (tdep) + fpreg);
 }
@@ -3270,25 +3267,20 @@ i386_mmx_regnum_to_fp_regnum (readable_regcache *regcache, int regnum)
 
 void
 i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
-				      readable_regcache *regcache,
+				      frame_info *next_frame,
 				      int regnum,
 				      struct value *result_value)
 {
   gdb_byte raw_buf[I386_MAX_REGISTER_SIZE];
-  enum register_status status;
   gdb_byte *buf = value_contents_raw (result_value);
 
   if (i386_mmx_regnum_p (gdbarch, regnum))
     {
-      int fpnum = i386_mmx_regnum_to_fp_regnum (regcache, regnum);
+      int fpnum = i386_mmx_regnum_to_fp_regnum (next_frame, regnum);
 
       /* Extract (always little endian).  */
-      status = regcache->raw_read (fpnum, raw_buf);
-      if (status != REG_VALID)
-	mark_value_bytes_unavailable (result_value, 0,
-				      TYPE_LENGTH (value_type (result_value)));
-      else
-	memcpy (buf, raw_buf, register_size (gdbarch, regnum));
+      frame_unwind_register (next_frame, fpnum, raw_buf);
+      memcpy (buf, raw_buf, register_size (gdbarch, regnum));
     }
   else
     {
@@ -3298,34 +3290,26 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
 	  regnum -= tdep->bnd0_regnum;
 
 	  /* Extract (always little endian).  Read lower 128bits.  */
-	  status = regcache->raw_read (I387_BND0R_REGNUM (tdep) + regnum,
-				       raw_buf);
-	  if (status != REG_VALID)
-	    mark_value_bytes_unavailable (result_value, 0, 16);
-	  else
-	    {
-	      enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
-	      LONGEST upper, lower;
-	      int size = TYPE_LENGTH (builtin_type (gdbarch)->builtin_data_ptr);
+	  frame_unwind_register (next_frame, I387_BND0R_REGNUM (tdep) + regnum, raw_buf);
 
-	      lower = extract_unsigned_integer (raw_buf, 8, byte_order);
-	      upper = extract_unsigned_integer (raw_buf + 8, 8, byte_order);
-	      upper = ~upper;
+	  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
+	  LONGEST upper, lower;
+	  int size = TYPE_LENGTH (builtin_type (gdbarch)->builtin_data_ptr);
 
-	      memcpy (buf, &lower, size);
-	      memcpy (buf + size, &upper, size);
-	    }
+	  lower = extract_unsigned_integer (raw_buf, 8, byte_order);
+	  upper = extract_unsigned_integer (raw_buf + 8, 8, byte_order);
+	  upper = ~upper;
+
+	  memcpy (buf, &lower, size);
+	  memcpy (buf + size, &upper, size);
 	}
       else if (i386_k_regnum_p (gdbarch, regnum))
 	{
 	  regnum -= tdep->k0_regnum;
 
 	  /* Extract (always little endian).  */
-	  status = regcache->raw_read (tdep->k0_regnum + regnum, raw_buf);
-	  if (status != REG_VALID)
-	    mark_value_bytes_unavailable (result_value, 0, 8);
-	  else
-	    memcpy (buf, raw_buf, 8);
+	  frame_unwind_register (next_frame, tdep->k0_regnum + regnum, raw_buf);
+	  memcpy (buf, raw_buf, 8);
 	}
       else if (i386_zmm_regnum_p (gdbarch, regnum))
 	{
@@ -3334,98 +3318,57 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
 	  if (regnum < num_lower_zmm_regs)
 	    {
 	      /* Extract (always little endian).  Read lower 128bits.  */
-	      status = regcache->raw_read (I387_XMM0_REGNUM (tdep) + regnum,
-					   raw_buf);
-	      if (status != REG_VALID)
-		mark_value_bytes_unavailable (result_value, 0, 16);
-	      else
-		memcpy (buf, raw_buf, 16);
+	      frame_unwind_register (next_frame, I387_XMM0_REGNUM (tdep) + regnum, raw_buf);
+	      memcpy (buf, raw_buf, 16);
 
 	      /* Extract (always little endian).  Read upper 128bits.  */
-	      status = regcache->raw_read (tdep->ymm0h_regnum + regnum,
-					   raw_buf);
-	      if (status != REG_VALID)
-		mark_value_bytes_unavailable (result_value, 16, 16);
-	      else
-		memcpy (buf + 16, raw_buf, 16);
+	      frame_unwind_register (next_frame, tdep->ymm0h_regnum + regnum, raw_buf);
+	      memcpy (buf + 16, raw_buf, 16);
 	    }
 	  else
 	    {
 	      /* Extract (always little endian).  Read lower 128bits.  */
-	      status = regcache->raw_read (I387_XMM16_REGNUM (tdep) + regnum
-					   - num_lower_zmm_regs,
-					   raw_buf);
-	      if (status != REG_VALID)
-		mark_value_bytes_unavailable (result_value, 0, 16);
-	      else
-		memcpy (buf, raw_buf, 16);
+	      frame_unwind_register (next_frame, I387_XMM16_REGNUM (tdep) + regnum, raw_buf);
+	      memcpy (buf, raw_buf, 16);
 
 	      /* Extract (always little endian).  Read upper 128bits.  */
-	      status = regcache->raw_read (I387_YMM16H_REGNUM (tdep) + regnum
-					   - num_lower_zmm_regs,
-					   raw_buf);
-	      if (status != REG_VALID)
-		mark_value_bytes_unavailable (result_value, 16, 16);
-	      else
-		memcpy (buf + 16, raw_buf, 16);
+	      frame_unwind_register (next_frame, I387_YMM16H_REGNUM (tdep) + regnum, raw_buf);
+	      memcpy (buf + 16, raw_buf, 16);
 	    }
 
 	  /* Read upper 256bits.  */
-	  status = regcache->raw_read (tdep->zmm0h_regnum + regnum,
-				       raw_buf);
-	  if (status != REG_VALID)
-	    mark_value_bytes_unavailable (result_value, 32, 32);
-	  else
-	    memcpy (buf + 32, raw_buf, 32);
+	  frame_unwind_register (next_frame, tdep->zmm0h_regnum + regnum, raw_buf);
+	  memcpy (buf + 32, raw_buf, 32);
 	}
       else if (i386_ymm_regnum_p (gdbarch, regnum))
 	{
 	  regnum -= tdep->ymm0_regnum;
 
 	  /* Extract (always little endian).  Read lower 128bits.  */
-	  status = regcache->raw_read (I387_XMM0_REGNUM (tdep) + regnum,
-				       raw_buf);
-	  if (status != REG_VALID)
-	    mark_value_bytes_unavailable (result_value, 0, 16);
-	  else
-	    memcpy (buf, raw_buf, 16);
+	  frame_unwind_register (next_frame, I387_XMM0_REGNUM (tdep) + regnum, raw_buf);
+	  memcpy (buf, raw_buf, 16);
+
 	  /* Read upper 128bits.  */
-	  status = regcache->raw_read (tdep->ymm0h_regnum + regnum,
-				       raw_buf);
-	  if (status != REG_VALID)
-	    mark_value_bytes_unavailable (result_value, 16, 32);
-	  else
-	    memcpy (buf + 16, raw_buf, 16);
+	  frame_unwind_register (next_frame, tdep->ymm0h_regnum + regnum, raw_buf);
+	  memcpy (buf + 16, raw_buf, 16);
 	}
       else if (i386_ymm_avx512_regnum_p (gdbarch, regnum))
 	{
 	  regnum -= tdep->ymm16_regnum;
 	  /* Extract (always little endian).  Read lower 128bits.  */
-	  status = regcache->raw_read (I387_XMM16_REGNUM (tdep) + regnum,
-				       raw_buf);
-	  if (status != REG_VALID)
-	    mark_value_bytes_unavailable (result_value, 0, 16);
-	  else
-	    memcpy (buf, raw_buf, 16);
+	  frame_unwind_register (next_frame, I387_XMM16_REGNUM (tdep) + regnum, raw_buf);
+	  memcpy (buf, raw_buf, 16);
 	  /* Read upper 128bits.  */
-	  status = regcache->raw_read (tdep->ymm16h_regnum + regnum,
-				       raw_buf);
-	  if (status != REG_VALID)
-	    mark_value_bytes_unavailable (result_value, 16, 16);
-	  else
-	    memcpy (buf + 16, raw_buf, 16);
+	  frame_unwind_register (next_frame, tdep->ymm16h_regnum + regnum, raw_buf);
+	  memcpy (buf + 16, raw_buf, 16);
 	}
       else if (i386_word_regnum_p (gdbarch, regnum))
 	{
 	  int gpnum = regnum - tdep->ax_regnum;
 
 	  /* Extract (always little endian).  */
-	  status = regcache->raw_read (gpnum, raw_buf);
-	  if (status != REG_VALID)
-	    mark_value_bytes_unavailable (result_value, 0,
-					  TYPE_LENGTH (value_type (result_value)));
-	  else
-	    memcpy (buf, raw_buf, 2);
+	  frame_unwind_register (next_frame, gpnum, raw_buf);
+	  memcpy (buf, raw_buf, 2);
 	}
       else if (i386_byte_regnum_p (gdbarch, regnum))
 	{
@@ -3433,11 +3376,8 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
 
 	  /* Extract (always little endian).  We read both lower and
 	     upper registers.  */
-	  status = regcache->raw_read (gpnum % 4, raw_buf);
-	  if (status != REG_VALID)
-	    mark_value_bytes_unavailable (result_value, 0,
-					  TYPE_LENGTH (value_type (result_value)));
-	  else if (gpnum >= 4)
+	  frame_unwind_register (next_frame, gpnum % 4, raw_buf);
+	  if (gpnum >= 4)
 	    memcpy (buf, raw_buf + 1, 1);
 	  else
 	    memcpy (buf, raw_buf, 1);
@@ -3449,36 +3389,34 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
 
 static struct value *
 i386_pseudo_register_read_value (struct gdbarch *gdbarch,
-				 readable_regcache *regcache,
+				 frame_info *next_frame,
 				 int regnum)
 {
-  struct value *result;
-
-  result = allocate_value (register_type (gdbarch, regnum));
-  VALUE_LVAL (result) = lval_register;
-  VALUE_REGNUM (result) = regnum;
+  struct value *result = allocate_register_value (gdbarch, regnum, next_frame);
+  set_value_lazy (result, 0);
 
-  i386_pseudo_register_read_into_value (gdbarch, regcache, regnum, result);
+  i386_pseudo_register_read_into_value (gdbarch, next_frame, regnum, result);
 
   return result;
 }
 
 void
-i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
+i386_pseudo_register_write (struct gdbarch *gdbarch, frame_info *this_frame,
 			    int regnum, const gdb_byte *buf)
 {
   gdb_byte raw_buf[I386_MAX_REGISTER_SIZE];
 
   if (i386_mmx_regnum_p (gdbarch, regnum))
     {
-      int fpnum = i386_mmx_regnum_to_fp_regnum (regcache, regnum);
+      frame_info *next_frame = get_next_frame_sentinel_okay (this_frame);
+      int fpnum = i386_mmx_regnum_to_fp_regnum (next_frame, regnum);
 
       /* Read ...  */
-      regcache_raw_read (regcache, fpnum, raw_buf);
+      get_frame_register (this_frame, fpnum, raw_buf);
       /* ... Modify ... (always little endian).  */
       memcpy (raw_buf, buf, register_size (gdbarch, regnum));
       /* ... Write.  */
-      regcache_raw_write (regcache, fpnum, raw_buf);
+      put_frame_register (this_frame, fpnum, raw_buf);
     }
   else
     {
@@ -3496,9 +3434,9 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 	  upper = extract_unsigned_integer (buf + size, size, byte_order);
 
 	  /* Fetching register buffer.  */
-	  regcache_raw_read (regcache,
-			     I387_BND0R_REGNUM (tdep) + regnum,
-			     raw_buf);
+	  get_frame_register (this_frame,
+			      I387_BND0R_REGNUM (tdep) + regnum,
+			      raw_buf);
 
 	  upper = ~upper;
 
@@ -3506,8 +3444,7 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 	  memcpy (raw_buf, &lower, 8);
 	  memcpy (raw_buf + 8, &upper, 8);
 
-
-	  regcache_raw_write (regcache,
+	  put_frame_register (this_frame,
 			      I387_BND0R_REGNUM (tdep) + regnum,
 			      raw_buf);
 	}
@@ -3515,7 +3452,7 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 	{
 	  regnum -= tdep->k0_regnum;
 
-	  regcache_raw_write (regcache,
+	  put_frame_register (this_frame,
 			      tdep->k0_regnum + regnum,
 			      buf);
 	}
@@ -3526,29 +3463,29 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 	  if (regnum < num_lower_zmm_regs)
 	    {
 	      /* Write lower 128bits.  */
-	      regcache_raw_write (regcache,
+	      put_frame_register (this_frame,
 				  I387_XMM0_REGNUM (tdep) + regnum,
 				  buf);
 	      /* Write upper 128bits.  */
-	      regcache_raw_write (regcache,
+	      put_frame_register (this_frame,
 				  I387_YMM0_REGNUM (tdep) + regnum,
 				  buf + 16);
 	    }
 	  else
 	    {
 	      /* Write lower 128bits.  */
-	      regcache_raw_write (regcache,
+	      put_frame_register (this_frame,
 				  I387_XMM16_REGNUM (tdep) + regnum
 				  - num_lower_zmm_regs,
 				  buf);
 	      /* Write upper 128bits.  */
-	      regcache_raw_write (regcache,
+	      put_frame_register (this_frame,
 				  I387_YMM16H_REGNUM (tdep) + regnum
 				  - num_lower_zmm_regs,
 				  buf + 16);
 	    }
 	  /* Write upper 256bits.  */
-	  regcache_raw_write (regcache,
+	  put_frame_register (this_frame,
 			      tdep->zmm0h_regnum + regnum,
 			      buf + 32);
 	}
@@ -3557,11 +3494,11 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 	  regnum -= tdep->ymm0_regnum;
 
 	  /* ... Write lower 128bits.  */
-	  regcache_raw_write (regcache,
+	  put_frame_register (this_frame,
 			     I387_XMM0_REGNUM (tdep) + regnum,
 			     buf);
 	  /* ... Write upper 128bits.  */
-	  regcache_raw_write (regcache,
+	  put_frame_register (this_frame,
 			     tdep->ymm0h_regnum + regnum,
 			     buf + 16);
 	}
@@ -3570,11 +3507,11 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 	  regnum -= tdep->ymm16_regnum;
 
 	  /* ... Write lower 128bits.  */
-	  regcache_raw_write (regcache,
+	  put_frame_register (this_frame,
 			      I387_XMM16_REGNUM (tdep) + regnum,
 			      buf);
 	  /* ... Write upper 128bits.  */
-	  regcache_raw_write (regcache,
+	  put_frame_register (this_frame,
 			      tdep->ymm16h_regnum + regnum,
 			      buf + 16);
 	}
@@ -3583,25 +3520,25 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 	  int gpnum = regnum - tdep->ax_regnum;
 
 	  /* Read ...  */
-	  regcache_raw_read (regcache, gpnum, raw_buf);
+	  get_frame_register (this_frame, gpnum, raw_buf);
 	  /* ... Modify ... (always little endian).  */
 	  memcpy (raw_buf, buf, 2);
 	  /* ... Write.  */
-	  regcache_raw_write (regcache, gpnum, raw_buf);
+	  put_frame_register (this_frame, gpnum, raw_buf);
 	}
       else if (i386_byte_regnum_p (gdbarch, regnum))
 	{
 	  int gpnum = regnum - tdep->al_regnum;
 
 	  /* Read ...  We read both lower and upper registers.  */
-	  regcache_raw_read (regcache, gpnum % 4, raw_buf);
+	  get_frame_register (this_frame, gpnum % 4, raw_buf);
 	  /* ... Modify ... (always little endian).  */
 	  if (gpnum >= 4)
 	    memcpy (raw_buf + 1, buf, 1);
 	  else
 	    memcpy (raw_buf, buf, 1);
 	  /* ... Write.  */
-	  regcache_raw_write (regcache, gpnum % 4, raw_buf);
+	  put_frame_register (this_frame, gpnum % 4, raw_buf);
 	}
       else
 	internal_error (__FILE__, __LINE__, _("invalid regnum"));
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 81a93f11af5a..238bb1f533cd 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,
+						  frame_info *next_frame,
 						  int regnum,
 						  struct value *result);
 
 extern void i386_pseudo_register_write (struct gdbarch *gdbarch,
-					struct regcache *regcache,
+					frame_info *this_frame,
 					int regnum, const gdb_byte *buf);
 
 extern int i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 83bcbd22a374..c6c6330bcec6 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -663,7 +663,7 @@ readable_regcache::cooked_read (int regnum, gdb_byte *buf)
       mark = value_mark ();
 
       computed = gdbarch_pseudo_register_read_value (m_descr->gdbarch,
-						     this, regnum);
+						     frame_find_by_id (sentinel_frame_id), regnum);
       if (value_entirely_available (computed))
 	memcpy (buf, value_contents_raw (computed),
 		m_descr->sizeof_register[regnum]);
@@ -716,7 +716,7 @@ readable_regcache::cooked_read_value (int regnum)
     }
   else
     return gdbarch_pseudo_register_read_value (m_descr->gdbarch,
-					       this, regnum);
+					       frame_find_by_id (sentinel_frame_id), regnum);
 }
 
 enum register_status
@@ -838,7 +838,7 @@ regcache::cooked_write (int regnum, const gdb_byte *buf)
   if (regnum < num_raw_registers ())
     raw_write (regnum, buf);
   else
-    gdbarch_pseudo_register_write (m_descr->gdbarch, this,
+    gdbarch_pseudo_register_write (m_descr->gdbarch, get_current_frame (),
 				   regnum, buf);
 }
 
@@ -1598,6 +1598,8 @@ cooked_read_test (struct gdbarch *gdbarch)
   scoped_restore restore_inferior_ptid
     = make_scoped_restore (&inferior_ptid, mock_ptid);
 
+  frame_info *f = get_current_frame ();
+
   /* Test that read one raw register from regcache_no_target will go
      to the target layer.  */
   int regnum;
@@ -1609,7 +1611,9 @@ cooked_read_test (struct gdbarch *gdbarch)
 	break;
     }
 
-  readwrite_regcache readwrite (gdbarch);
+  //readwrite_regcache readwrite (gdbarch);
+  regcache *r = get_thread_regcache (mock_ptid);
+  regcache &readwrite = *r;
   gdb::def_vector<gdb_byte> buf (register_size (gdbarch, regnum));
 
   readwrite.raw_read (regnum, buf.data ());
@@ -1731,12 +1735,29 @@ cooked_write_test (struct gdbarch *gdbarch)
   if (target_stack->to_stratum >= process_stratum)
     error (_("target already pushed"));
 
-  /* Create a mock environment.  A process_stratum target pushed.  */
-
   target_ops_no_register mock_target;
+  ptid_t mock_ptid (1, 1);
+  inferior mock_inferior (mock_ptid.pid ());
+  address_space mock_aspace
+    { };
+  mock_inferior.gdbarch = gdbarch;
+  mock_inferior.aspace = &mock_aspace;
+  thread_info mock_thread (&mock_inferior, mock_ptid);
+
+  scoped_restore restore_thread_list = make_scoped_restore (&thread_list,
+							    &mock_thread);
+
+  /* Add the mock inferior to the inferior list so that look ups by
+   target+ptid can find it.  */
+  scoped_restore restore_inferior_list = make_scoped_restore (&inferior_list);
+  inferior_list = &mock_inferior;
+
+  /* Switch to the mock inferior.  */
+  scoped_restore_current_inferior restore_current_inferior;
+  set_current_inferior (&mock_inferior);
 
   /* Push the process_stratum target so we can mock accessing
-     registers.  */
+   registers.  */
   push_target (&mock_target);
 
   /* Pop it again on exit (return/exception).  */
@@ -1748,7 +1769,15 @@ cooked_write_test (struct gdbarch *gdbarch)
     }
   } pop_targets;
 
-  readwrite_regcache readwrite (gdbarch);
+  /* Switch to the mock thread.  */
+  scoped_restore restore_inferior_ptid = make_scoped_restore (&inferior_ptid,
+							      mock_ptid);
+
+  frame_info *f = get_current_frame ();
+
+
+  regcache *r = get_thread_regcache (mock_ptid);
+  regcache &readwrite = *r;
 
   const int num_regs = (gdbarch_num_regs (gdbarch)
 			+ gdbarch_num_pseudo_regs (gdbarch));
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..390de7c32692
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
@@ -0,0 +1,43 @@
+.global callee
+callee:
+.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:
+
+	pop %rbx
+	pop %rbp	
+	ret
+.cfi_endproc
+
+
+.global caller
+caller:
+.cfi_startproc
+	push %rbp
+.cfi_def_cfa_offset 16
+	mov %rsp, %rbp
+
+	# Save caller's rbx value on the stack.
+.cfi_offset rbx, -24
+	push %rbx
+
+	# Put our own rbx value.
+	mov $0x1011121314151617, %rbx
+	call callee
+	
+	# Return the value we put in rbx.
+	mov %rbx, %rax
+
+	pop %rbx
+	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..619f44405e82
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-pseudo-unwind.exp
@@ -0,0 +1,81 @@
+# 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/>.
+
+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"
+
+with_test_prefix "callee, after change" {
+    # Verify the value of rbx/ebx in the inner frame (callee) after the 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, 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/value.c b/gdb/value.c
index 89ccac7cfc6e..6ea7c200af41 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1049,6 +1049,19 @@ allocate_repeat_value (struct type *type, int count)
   return allocate_value (array_type);
 }
 
+struct value *
+allocate_register_value (struct gdbarch *gdbarch, int regnum, frame_info *next_frame)
+{
+  struct type *type = register_type (gdbarch, regnum);
+  value *v = allocate_value_lazy (type);
+
+  VALUE_LVAL (v) = lval_register;
+  v->location.reg.regnum = regnum;
+  v->location.reg.next_frame_id = get_frame_id (next_frame);
+
+  return v;
+}
+
 struct value *
 allocate_computed_value (struct type *type,
                          const struct lval_funcs *funcs,
@@ -3758,7 +3771,6 @@ value_fetch_lazy_memory (struct value *val)
 static void
 value_fetch_lazy_register (struct value *val)
 {
-  struct frame_info *next_frame;
   int regnum;
   struct type *type = check_typedef (value_type (val));
   struct value *new_val = val, *mark = value_mark ();
@@ -3771,7 +3783,7 @@ value_fetch_lazy_register (struct value *val)
     {
       struct frame_id next_frame_id = VALUE_NEXT_FRAME_ID (new_val);
 
-      next_frame = frame_find_by_id (next_frame_id);
+      frame_info *next_frame = frame_find_by_id (next_frame_id);
       regnum = VALUE_REGNUM (new_val);
 
       gdb_assert (next_frame != NULL);
diff --git a/gdb/value.h b/gdb/value.h
index 4d75c966edd6..2335edb0f5eb 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -238,6 +238,9 @@ extern void set_value_pointed_to_offset (struct value *value, LONGEST val);
 extern LONGEST value_embedded_offset (const struct value *value);
 extern void set_value_embedded_offset (struct value *value, LONGEST val);
 
+extern struct value *allocate_register_value (struct gdbarch *gdbarch,
+					      int regnum, frame_info *frame);
+
 /* For lval_computed values, this structure holds functions used to
    retrieve and set the value (or portions of the value).
 
-- 
2.17.0

Re: [RFC PATCH] Read pseudo registers from frame instead of regcache

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> The problem: Reading pseudo registers from an upper stack frame does not
Simon> work.  The raw registers needed to compose the pseudo registers are
Simon> always read from the current thread's regcache, which is effectively
Simon> frame #0's registers.
[...]
Simon> The gdbarch hooks

Simon>   - pseudo_register_read
Simon>   - pseudo_register_read_value
Simon>   - pseudo_register_write

Simon> receive a regcache as a parameter, as a way to get the values of the raw
Simon> registers required to compute the pseudo register value.  However, the
Simon> regcache object always contains the current register values, not the
Simon> values unwound to the frame we're interested in.

I don't know this area very well, so apologies if this is really off
target; but I am wondering why not just make a regcache for the desired
frame and pass that to the gdbarch hooks?

Another thing I am curious about is how far back this bug goes.  Like,
if it were introduced in one of the last few releases, then it might be
worth bisecting to find out what happened.

Tom

Re: [RFC PATCH] Read pseudo registers from frame instead of regcache

[Simon Marchi]

On 2018-05-26 23:42, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> The problem: Reading pseudo registers from an upper stack frame 
> does not
> Simon> work.  The raw registers needed to compose the pseudo registers 
> are
> Simon> always read from the current thread's regcache, which is 
> effectively
> Simon> frame #0's registers.
> [...]
> Simon> The gdbarch hooks
> 
> Simon>   - pseudo_register_read
> Simon>   - pseudo_register_read_value
> Simon>   - pseudo_register_write
> 
> Simon> receive a regcache as a parameter, as a way to get the values of 
> the raw
> Simon> registers required to compute the pseudo register value.  
> However, the
> Simon> regcache object always contains the current register values, not 
> the
> Simon> values unwound to the frame we're interested in.
> 
> I don't know this area very well, so apologies if this is really off
> target; but I am wondering why not just make a regcache for the desired
> frame and pass that to the gdbarch hooks?

That sounds like a good idea, at least to avoid changing the gdbarch 
interface.  The current regcache always fetches raw registers using 
target_fetch_registers (which gets the current values of the thread's 
registers), but we could have another kind of regcache that instead gets 
raw register values by unwinding from the previous frame.

> Another thing I am curious about is how far back this bug goes.  Like,
> if it were introduced in one of the last few releases, then it might be
> worth bisecting to find out what happened.

The oldest version of GDB with which I could print $ebx is 7.2 (did 
pseudo registers not exist before?), and it exhibits the same problem.  
So I would think that the problem exists since "forever".

Simon

Re: [RFC PATCH] Read pseudo registers from frame instead of regcache

[Ulrich Weigand]

Simon Marchi wrote:

> The problem: Reading pseudo registers from an upper stack frame 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's registers.

I think this may have been by design at some point.  The idea being:
for the innermost frame, you construct your register set using the ptrace
(or whatever) interface, possibly using arch-specific constructions
(the "pseudo" registers).  But for higher frames, you construct *all*
registers directly via the unwinding logic.

For example, on a platform where a floating-point register was extended
to a vector register at some point, your ptrace interface may be split
between the original FP part, and the "remaining" piece, so to construct
the full vector register, you'd have to make two ptrace requests and
combine them (using a "pseudo" register).  But if you want to find the
value of the vector register in a higher frame, there will be unwind
info for the full vector register, and not the two pieces.

(This construct actually exists on Intel as well, e.g. with the ymmh
register parts.  However, since ymm is not call-saved in the Linux ABI,
we don't have unwind info either way ... In some other ABI, this could
be an actual problem, however.)

So this change:

> -  /* Ask this frame to unwind its register.  */
> -  value = frame->unwind->prev_register (frame, &frame->prologue_cache, regnum);
> +  struct value *value;
> +  if (regnum < gdbarch_num_regs (gdbarch))
> +    {
> +      /* This is a raw register, we can directly ask the next frame to unwind
> +         the register.  */
> +      value = next_frame->unwind->prev_register (next_frame,
> +						 &next_frame->prologue_cache,
> +						 regnum);
> +    }
> +  else 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, next_frame, regnum);
> +    }

in effect changes what unwind info GDB expects.  Now maybe it is still the
correct change, but this at least needs a review on how pseudo register
unwind info is currently handled across all architectures ...

Bye,
Ulrich

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

Re: [RFC PATCH] Read pseudo registers from frame instead of regcache

[Simon Marchi]

Hi Ulrich,

Thanks for the feedback.

On 2018-05-28 13:47, Ulrich Weigand wrote:
> Simon Marchi wrote:
> 
>> The problem: Reading pseudo registers from an upper stack frame 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's registers.
> 
> I think this may have been by design at some point.  The idea being:
> for the innermost frame, you construct your register set using the 
> ptrace
> (or whatever) interface, possibly using arch-specific constructions
> (the "pseudo" registers).  But for higher frames, you construct *all*
> registers directly via the unwinding logic.

That make sense, I don't think my patch really changes that.  What my 
patch changes is that we can reconstruct pseudo register values from 
unwound raw register values, which we couldn't before.  In the simple 
rbx/ebx case I tried, next_frame will never have info about ebx (trying 
to use it in cfi_offset gives "bad register expression"), so it doesn't 
make sense to query it about the location of ebx.

> For example, on a platform where a floating-point register was extended
> to a vector register at some point, your ptrace interface may be split
> between the original FP part, and the "remaining" piece, so to 
> construct
> the full vector register, you'd have to make two ptrace requests and
> combine them (using a "pseudo" register).  But if you want to find the
> value of the vector register in a higher frame, there will be unwind
> info for the full vector register, and not the two pieces.
> 
> (This construct actually exists on Intel as well, e.g. with the ymmh
> register parts.  However, since ymm is not call-saved in the Linux ABI,
> we don't have unwind info either way ... In some other ABI, this could
> be an actual problem, however.)

Ok, I was assuming that it was never possible for the debug info to 
describe how pseudo registers are saved, only raw registers.  Do you 
have an architecture in mind where it's possible to have a pseudo 
register mentioned in the unwind information?  GNU as doesn't accept at 
all "ymm0" or "ymm0h" as an argument to .cfi_offset, so I don't know how 
ymm0 would be represented if we wanted to save it (despite it not being 
callee saved according to the ABI).

If the unwind info can indeed contain explicit information about pseudo 
registers (and therefore implicit info about the raw registers that 
compose it), can't it lead to very difficult situations?  There is 
normally only one way to reconstruct a pseudo register value from raw 
registers.  But a raw register can be part of many pseudo registers.  So 
if we need to unwind a raw register value for which the next frame has 
no explicit information, then we need to check if it has info about all 
the possible pseudo registers this raw register is part of?

> So this change:
> 
>> -  /* Ask this frame to unwind its register.  */
>> -  value = frame->unwind->prev_register (frame, 
>> &frame->prologue_cache, regnum);
>> +  struct value *value;
>> +  if (regnum < gdbarch_num_regs (gdbarch))
>> +    {
>> +      /* This is a raw register, we can directly ask the next frame 
>> to unwind
>> +         the register.  */
>> +      value = next_frame->unwind->prev_register (next_frame,
>> +						 &next_frame->prologue_cache,
>> +						 regnum);
>> +    }
>> +  else 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, 
>> next_frame, regnum);
>> +    }
> 
> in effect changes what unwind info GDB expects.  Now maybe it is still 
> the
> correct change, but this at least needs a review on how pseudo register
> unwind info is currently handled across all architectures ...

I'll do what I can, but don't expect me to be an expert of all CPU 
architectures anytime soon :).

Simon

Re: [RFC PATCH] Read pseudo registers from frame instead of regcache

[Simon Marchi]

On 2018-05-28 15:13, Simon Marchi wrote:
> Ok, I was assuming that it was never possible for the debug info to
> describe how pseudo registers are saved, only raw registers.  Do you
> have an architecture in mind where it's possible to have a pseudo
> register mentioned in the unwind information?  GNU as doesn't accept
> at all "ymm0" or "ymm0h" as an argument to .cfi_offset, so I don't
> know how ymm0 would be represented if we wanted to save it (despite it
> not being callee saved according to the ABI).

I looked at ARMv7, and one case where this might happen is the VFP 
registers.  The dX registers (64-bits) are the raw ones and the sX 
(32-bits) are the pseudo ones (where two sX registers make up one dX 
register).  The GNU assembler accepts using s0 with .cfi_offset (for 
backwards compatibility, I guess?), so we end up with it in the unwind 
info.  However, this is an obsolete way of doing it according to the 
"DWARF for the ARM Architecture" document [1].  They recommend to 
describe the sX registers as bit pieces of the dX registers.  That 
recommendation would be in line with the "only raw registers in unwind 
info" assumption.  But it means that we can encounter this in the wild.

Simon

[1] 
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0040b/IHI0040B_aadwarf.pdf

Re: [RFC PATCH] Read pseudo registers from frame instead of regcache

[Ulrich Weigand]

Simon Marchi wrote:
> On 2018-05-28 15:13, Simon Marchi wrote:
> > Ok, I was assuming that it was never possible for the debug info to
> > describe how pseudo registers are saved, only raw registers.  Do you
> > have an architecture in mind where it's possible to have a pseudo
> > register mentioned in the unwind information?  GNU as doesn't accept
> > at all "ymm0" or "ymm0h" as an argument to .cfi_offset, so I don't
> > know how ymm0 would be represented if we wanted to save it (despite it
> > not being callee saved according to the ABI).
> 
> I looked at ARMv7, and one case where this might happen is the VFP 
> registers.  The dX registers (64-bits) are the raw ones and the sX 
> (32-bits) are the pseudo ones (where two sX registers make up one dX 
> register).  The GNU assembler accepts using s0 with .cfi_offset (for 
> backwards compatibility, I guess?), so we end up with it in the unwind 
> info.  However, this is an obsolete way of doing it according to the 
> "DWARF for the ARM Architecture" document [1].  They recommend to 
> describe the sX registers as bit pieces of the dX registers.  That 
> recommendation would be in line with the "only raw registers in unwind 
> info" assumption.  But it means that we can encounter this in the wild.

Another consideration is where GDB itself massages the DWARF info to
use pseudo register numbers; one way to check for that is to look
for instances of gdbarch_dwarf2_reg_to_regnum where the returned
value is a pseudo register number.

At a quick glance, it seems that MEP does that (for pretty much all
registers?), SPU does it for the special SP register, h8300 does it
for a special CCR register, xtensa seems to do it for some registers
(driven by a table) etc.  (I didn't review all archictures.  Also, not
all of those registers are necessarily used for *unwinding*, some DWARF
registers may only be used for DWARF debug info / locations.  But in
at least in the SPU case, I know that SP *is* used for unwinding.).

Now for the internal uses, I guess those could all be fixed by GDB
internal changes to those targets in one way or the other.  But still,
this would need to be done to prevent your patch from breaking
existing platforms ...

Bye,
Ulrich

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