[v2,14/24] gdb: read pseudo register through frame

Message ID 20231124212656.96801-15-simon.marchi@efficios.com
State New
Headers
Series Fix reading and writing pseudo registers in non-current frames |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Simon Marchi Nov. 24, 2023, 9:26 p.m. UTC
  Change gdbarch_pseudo_register_read_value to take a frame instead of a
regcache.  The frame (and formerly the regcache) is used to read raw
registers needed to make up the pseudo register value.  The problem with
using the regcache is that it always provides raw register values for
the current frame (frame 0).

Let's say the user wants to read the ebx register on amd64.  ebx is a pseudo
register, obtained by reading the bottom half (bottom 4 bytes) of the
rbx register, which is a raw register.  If the currently selected frame
is frame 0, it works fine:

    (gdb) frame 0
    #0  break_here_asm () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:36
    36      in /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
    (gdb) p/x $ebx
    $1 = 0x24252627
    (gdb) p/x $rbx
    $2 = 0x2021222324252627

But if the user is looking at another frame, and the raw register behind
the pseudo register has been saved at some point in the call stack, then
we get a wrong answer:

    (gdb) frame 1
    #1  0x000055555555517d in caller () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:56
    56      in /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
    (gdb) p/x $ebx
    $3 = 0x24252627
    (gdb) p/x $rbx
    $4 = 0x1011121314151617

Here, the value of ebx was computed using the value of rbx in frame 0
(through the regcache), it should have been computed using the value of
rbx in frame 1.

In other to make this work properly, make the following changes:

 - Make dwarf2_frame_prev_register return nullptr if it doesn't know how
   to unwind a register and that register is a pseudo register.
   Previously, it returned `frame_unwind_got_register`, meaning, in our
   example, "the value of ebx in frame 1 is the same as the value of ebx
   in frame 0", which is obviously false.  Return nullptr as a way to
   say "I don't know".

 - In frame_unwind_register_value, when prev_register (for instance
   dwarf2_frame_prev_register) returns nullptr, and we are trying to
   read a pseudo register, try to get the register value through
   gdbarch_pseudo_register_read_value or gdbarch_pseudo_register_read.
   If using gdbarch_pseudo_register_read, the behavior is known to be
   broken.  Implementations should be migrated to use
   gdbarch_pseudo_register_read_value to fix that.

 - Change gdbarch_pseudo_register_read_value to take a frame_info
   instead of a regcache, update implementations (aarch64, amd64, i386).
   In i386-tdep.c, I made a copy of i386_mmx_regnum_to_fp_regnum that
   uses a frame instead of a regcache.  The version using the regcache
   is still used by i386_pseudo_register_write.  It will get removed in
   a subsequent patch.

 - Add some helpers in value.{c,h} to implement the common cases of
   pseudo registers: taking part of a raw register and concatenating
   multiple raw registers.

 - Update readable_regcache::{cooked_read,cooked_read_value} to pass the
   current frame to gdbarch_pseudo_register_read_value.  Passing the
   current frame will give the same behavior as before: for frame 0, raw
   registers will be read from the current thread's regcache.

Notes:

 - I do not plan on changing gdbarch_pseudo_register_read to receive a
   frame instead of a regcache. That method is considered deprecated.
   Instead, we should be working on migrating implementations to use
   gdbarch_pseudo_register_read_value instead.

 - In frame_unwind_register_value, we still ask the unwinder to try to
   unwind pseudo register values.  It's apparently possible for the
   debug info to provide information about [1] pseudo registers, so we
   want to try that first, before falling back to computing them
   ourselves.

[1] https://inbox.sourceware.org/gdb-patches/20180528174715.A954AD804AD@oc3748833570.ibm.com/

Change-Id: Id6ef1c64e19090a183dec050e4034d8c2394e7ca
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
---
 gdb/aarch64-tdep.c        | 148 +++++++++++--------------
 gdb/amd64-tdep.c          |  43 ++------
 gdb/dwarf2/frame.c        |   5 +-
 gdb/frame.c               |  36 +++++-
 gdb/gdbarch-gen.h         |   4 +-
 gdb/gdbarch.c             |   4 +-
 gdb/gdbarch_components.py |   2 +-
 gdb/i386-tdep.c           | 224 ++++++++++++++------------------------
 gdb/i386-tdep.h           |   7 +-
 gdb/regcache.c            |  38 +++++--
 gdb/value.c               |  70 ++++++++++++
 gdb/value.h               |  26 +++++
 12 files changed, 322 insertions(+), 285 deletions(-)
  

Patch

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 50bfaa41eee2..341a6a19de5f 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -3104,25 +3104,14 @@  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)
+static value *
+aarch64_pseudo_read_value_1 (frame_info_ptr next_frame,
+			     const int pseudo_reg_num, int raw_regnum_offset)
 {
-  unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
-
-  /* Enough space for a full vector register.  */
-  gdb_byte reg_buf[register_size (gdbarch, AARCH64_V0_REGNUM)];
-  gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM);
+  unsigned v_regnum = AARCH64_V0_REGNUM + raw_regnum_offset;
 
-  if (regcache->raw_read (v_regnum, reg_buf) != REG_VALID)
-    result_value->mark_bytes_unavailable (0,
-					  result_value->type ()->length ());
-  else
-    memcpy (result_value->contents_raw ().data (), reg_buf, regsize);
-
-  return result_value;
- }
+  return pseudo_from_raw_part (next_frame, pseudo_reg_num, v_regnum, 0);
+}
 
 /* Helper function for reading/writing ZA pseudo-registers.  Given REGNUM,
    a ZA pseudo-register number, return, in OFFSETS, the information on positioning
@@ -3205,54 +3194,47 @@  aarch64_za_offsets_from_regnum (struct gdbarch *gdbarch, int regnum,
 
 /* Given REGNUM, a SME pseudo-register number, return its value in RESULT.  */
 
-static struct value *
-aarch64_sme_pseudo_register_read (struct gdbarch *gdbarch,
-				  readable_regcache *regcache, int regnum,
-				  struct value *result)
+static value *
+aarch64_sme_pseudo_register_read (gdbarch *gdbarch, frame_info_ptr next_frame,
+				  const int pseudo_reg_num)
 {
   aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
 
   gdb_assert (tdep->has_sme ());
   gdb_assert (tdep->sme_svq > 0);
-  gdb_assert (tdep->sme_pseudo_base <= regnum);
-  gdb_assert (regnum < tdep->sme_pseudo_base + tdep->sme_pseudo_count);
+  gdb_assert (tdep->sme_pseudo_base <= pseudo_reg_num);
+  gdb_assert (pseudo_reg_num < tdep->sme_pseudo_base + tdep->sme_pseudo_count);
 
   /* Fetch the offsets that we need in order to read from the correct blocks
      of ZA.  */
   struct za_offsets offsets;
-  aarch64_za_offsets_from_regnum (gdbarch, regnum, offsets);
+  aarch64_za_offsets_from_regnum (gdbarch, pseudo_reg_num, offsets);
 
   /* Fetch the contents of ZA.  */
-  size_t svl = sve_vl_from_vq (tdep->sme_svq);
-  gdb::byte_vector za (std::pow (svl, 2));
-  regcache->raw_read (tdep->sme_za_regnum, za.data ());
+  value *za_value = value_of_register (tdep->sme_za_regnum, next_frame);
+  value *result = value::allocate_register (next_frame, pseudo_reg_num);
 
   /* Copy the requested data.  */
   for (int chunks = 0; chunks < offsets.chunks; chunks++)
     {
-      const gdb_byte *source
-	= za.data () + offsets.starting_offset + chunks * offsets.stride_size;
-      gdb_byte *destination
-	= result->contents_raw ().data () + chunks * offsets.chunk_size;
-
-      memcpy (destination, source, offsets.chunk_size);
+      int src_offset = offsets.starting_offset + chunks * offsets.stride_size;
+      int dst_offset = chunks * offsets.chunk_size;
+      za_value->contents_copy (result, dst_offset, src_offset,
+			       offsets.chunk_size);
     }
+
   return result;
 }
 
 /* Implement the "pseudo_register_read_value" gdbarch method.  */
 
-static struct value *
-aarch64_pseudo_read_value (struct gdbarch *gdbarch, readable_regcache *regcache,
-			   int regnum)
+static value *
+aarch64_pseudo_read_value (gdbarch *gdbarch, frame_info_ptr next_frame,
+			   const int pseudo_reg_num)
 {
   aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
-  struct value *result_value = value::allocate (register_type (gdbarch, regnum));
 
-  result_value->set_lval (lval_register);
-  VALUE_REGNUM (result_value) = regnum;
-
-  if (is_w_pseudo_register (gdbarch, regnum))
+  if (is_w_pseudo_register (gdbarch, pseudo_reg_num))
     {
       enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
       /* Default offset for little endian.  */
@@ -3262,53 +3244,49 @@  aarch64_pseudo_read_value (struct gdbarch *gdbarch, readable_regcache *regcache,
 	offset = 4;
 
       /* Find the correct X register to extract the data from.  */
-      int x_regnum = AARCH64_X0_REGNUM + (regnum - tdep->w_pseudo_base);
-      gdb_byte data[4];
+      int x_regnum
+	= AARCH64_X0_REGNUM + (pseudo_reg_num - tdep->w_pseudo_base);
 
       /* Read the bottom 4 bytes of X.  */
-      if (regcache->raw_read_part (x_regnum, offset, 4, data) != REG_VALID)
-	result_value->mark_bytes_unavailable (0, 4);
-      else
-	memcpy (result_value->contents_raw ().data (), data, 4);
-
-      return result_value;
+      return pseudo_from_raw_part (next_frame, pseudo_reg_num, x_regnum,
+				   offset);
     }
-  else if (is_sme_pseudo_register (gdbarch, regnum))
-    return aarch64_sme_pseudo_register_read (gdbarch, regcache, regnum,
-					     result_value);
-
-  regnum -= gdbarch_num_regs (gdbarch);
-
-  if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
-    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
-					regnum - AARCH64_Q0_REGNUM,
-					Q_REGISTER_SIZE, result_value);
-
-  if (regnum >= AARCH64_D0_REGNUM && regnum < AARCH64_D0_REGNUM + 32)
-    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
-					regnum - AARCH64_D0_REGNUM,
-					D_REGISTER_SIZE, result_value);
-
-  if (regnum >= AARCH64_S0_REGNUM && regnum < AARCH64_S0_REGNUM + 32)
-    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
-					regnum - AARCH64_S0_REGNUM,
-					S_REGISTER_SIZE, result_value);
-
-  if (regnum >= AARCH64_H0_REGNUM && regnum < AARCH64_H0_REGNUM + 32)
-    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
-					regnum - AARCH64_H0_REGNUM,
-					H_REGISTER_SIZE, result_value);
-
-  if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
-    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
-					regnum - AARCH64_B0_REGNUM,
-					B_REGISTER_SIZE, result_value);
-
-  if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
-      && regnum < AARCH64_SVE_V0_REGNUM + 32)
-    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
-					regnum - AARCH64_SVE_V0_REGNUM,
-					V_REGISTER_SIZE, result_value);
+  else if (is_sme_pseudo_register (gdbarch, pseudo_reg_num))
+    return aarch64_sme_pseudo_register_read (gdbarch, next_frame,
+					     pseudo_reg_num);
+
+  /* Offset in the "pseudo-register space".  */
+  int pseudo_offset = pseudo_reg_num - gdbarch_num_regs (gdbarch);
+
+  if (pseudo_offset >= AARCH64_Q0_REGNUM
+      && pseudo_offset < AARCH64_Q0_REGNUM + 32)
+    return aarch64_pseudo_read_value_1 (next_frame, pseudo_reg_num,
+					pseudo_offset - AARCH64_Q0_REGNUM);
+
+  if (pseudo_offset >= AARCH64_D0_REGNUM
+      && pseudo_offset < AARCH64_D0_REGNUM + 32)
+    return aarch64_pseudo_read_value_1 (next_frame, pseudo_reg_num,
+					pseudo_offset - AARCH64_D0_REGNUM);
+
+  if (pseudo_offset >= AARCH64_S0_REGNUM
+      && pseudo_offset < AARCH64_S0_REGNUM + 32)
+    return aarch64_pseudo_read_value_1 (next_frame, pseudo_reg_num,
+					pseudo_offset - AARCH64_S0_REGNUM);
+
+  if (pseudo_offset >= AARCH64_H0_REGNUM
+      && pseudo_offset < AARCH64_H0_REGNUM + 32)
+    return aarch64_pseudo_read_value_1 (next_frame, pseudo_reg_num,
+					pseudo_offset - AARCH64_H0_REGNUM);
+
+  if (pseudo_offset >= AARCH64_B0_REGNUM
+      && pseudo_offset < AARCH64_B0_REGNUM + 32)
+    return aarch64_pseudo_read_value_1 (next_frame, pseudo_reg_num,
+					pseudo_offset - AARCH64_B0_REGNUM);
+
+  if (tdep->has_sve () && pseudo_offset >= AARCH64_SVE_V0_REGNUM
+      && pseudo_offset < AARCH64_SVE_V0_REGNUM + 32)
+    return aarch64_pseudo_read_value_1 (next_frame, pseudo_reg_num,
+					pseudo_offset - AARCH64_SVE_V0_REGNUM);
 
   gdb_assert_not_reached ("regnum out of bound");
 }
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index e6feee677b3d..955a6860631d 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -348,18 +348,12 @@  amd64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
     return i386_pseudo_register_name (gdbarch, regnum);
 }
 
-static struct value *
-amd64_pseudo_register_read_value (struct gdbarch *gdbarch,
-				  readable_regcache *regcache,
+static value *
+amd64_pseudo_register_read_value (gdbarch *gdbarch, frame_info_ptr next_frame,
 				  int regnum)
 {
   i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (gdbarch);
 
-  value *result_value = value::allocate (register_type (gdbarch, regnum));
-  result_value->set_lval (lval_register);
-  VALUE_REGNUM (result_value) = regnum;
-  gdb_byte *buf = result_value->contents_raw ().data ();
-
   if (i386_byte_regnum_p (gdbarch, regnum))
     {
       int gpnum = regnum - tdep->al_regnum;
@@ -368,44 +362,21 @@  amd64_pseudo_register_read_value (struct gdbarch *gdbarch,
       if (gpnum >= AMD64_NUM_LOWER_BYTE_REGS)
 	{
 	  gpnum -= AMD64_NUM_LOWER_BYTE_REGS;
-	  gdb_byte raw_buf[register_size (gdbarch, gpnum)];
 
 	  /* Special handling for AH, BH, CH, DH.  */
-	  register_status status = regcache->raw_read (gpnum, raw_buf);
-	  if (status == REG_VALID)
-	    memcpy (buf, raw_buf + 1, 1);
-	  else
-	    result_value->mark_bytes_unavailable (0,
-						  result_value->type ()->length ());
+	  return pseudo_from_raw_part (next_frame, regnum, gpnum, 1);
 	}
       else
-	{
-	  gdb_byte raw_buf[register_size (gdbarch, gpnum)];
-	  register_status status = regcache->raw_read (gpnum, raw_buf);
-	  if (status == REG_VALID)
-	    memcpy (buf, raw_buf, 1);
-	  else
-	    result_value->mark_bytes_unavailable (0,
-						  result_value->type ()->length ());
-	}
+	return pseudo_from_raw_part (next_frame, regnum, gpnum, 0);
     }
   else if (i386_dword_regnum_p (gdbarch, regnum))
     {
       int gpnum = regnum - tdep->eax_regnum;
-      gdb_byte raw_buf[register_size (gdbarch, gpnum)];
-      /* Extract (always little endian).  */
-      register_status status = regcache->raw_read (gpnum, raw_buf);
-      if (status == REG_VALID)
-	memcpy (buf, raw_buf, 4);
-      else
-	result_value->mark_bytes_unavailable (0,
-					      result_value->type ()->length ());
+
+      return pseudo_from_raw_part (next_frame, regnum, gpnum, 0);
     }
   else
-    i386_pseudo_register_read_into_value (gdbarch, regcache, regnum,
-					  result_value);
-
-  return result_value;
+    return i386_pseudo_register_read_value (gdbarch, next_frame, regnum);
 }
 
 static void
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index d72dd0ad971d..d3d1ecdf1f5b 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -1220,7 +1220,10 @@  dwarf2_frame_prev_register (frame_info_ptr this_frame, void **this_cache,
 	 "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);
+      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 019faad37dda..5a510f322f26 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1257,9 +1257,39 @@  frame_unwind_register_value (frame_info_ptr next_frame, int regnum)
     frame_unwind_find_by_frame (next_frame, &next_frame->prologue_cache);
 
   /* Ask this frame to unwind its register.  */
-  value *value = next_frame->unwind->prev_register (next_frame,
-						    &next_frame->prologue_cache,
-						    regnum);
+  value *value
+    = next_frame->unwind->prev_register (next_frame,
+					 &next_frame->prologue_cache, regnum);
+  if (value == nullptr)
+    {
+      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 if (gdbarch_pseudo_register_read_p (gdbarch))
+	{
+	  value = value::allocate_register (next_frame, regnum);
+
+	  /* Passing the current regcache is known to be broken, the pseudo
+	     register value will be constructed using the current raw registers,
+	     rather than reading them using NEXT_FRAME.  Architectures should be
+	     migrated to gdbarch_pseudo_register_read_value.  */
+	  register_status status = gdbarch_pseudo_register_read
+	    (gdbarch, get_thread_regcache (inferior_thread ()), regnum,
+	     value->contents_writeable ().data ());
+	  if (status == REG_UNAVAILABLE)
+	    value->mark_bytes_unavailable (0, value->type ()->length ());
+	}
+      else
+	error (_("Can't unwind value of register %d (%s)"), regnum,
+	       user_reg_map_regnum_to_name (gdbarch, regnum));
+    }
 
   if (frame_debug)
     {
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index 9f468bd1f618..c70bfc97634a 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -196,8 +196,8 @@  extern void set_gdbarch_pseudo_register_read (struct gdbarch *gdbarch, gdbarch_p
 
 extern bool 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_ptr next_frame, int cookednum);
+extern struct value * gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, frame_info_ptr next_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 bool gdbarch_pseudo_register_write_p (struct gdbarch *gdbarch);
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index ea6e4c647b10..06ff52576bdf 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -1886,13 +1886,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_ptr next_frame, int cookednum)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->pseudo_register_read_value != NULL);
   if (gdbarch_debug >= 2)
     gdb_printf (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, next_frame, cookednum);
 }
 
 void
diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
index 694ac3660235..c597b38ad5a9 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -414,7 +414,7 @@  never be called.
 """,
     type="struct value *",
     name="pseudo_register_read_value",
-    params=[("readable_regcache *", "regcache"), ("int", "cookednum")],
+    params=[("frame_info_ptr", "next_frame"), ("int", "cookednum")],
     predicate=True,
 )
 
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 79c1dfac435b..c74e5146c609 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3411,195 +3411,131 @@  i386_mmx_regnum_to_fp_regnum (readable_regcache *regcache, int regnum)
   return (I387_ST0_REGNUM (tdep) + fpreg);
 }
 
+/* Map a cooked register onto a raw register or memory.  For the i386,
+   the MMX registers need to be mapped onto floating point registers.  */
+
+static int
+i386_mmx_regnum_to_fp_regnum (frame_info_ptr next_frame, int regnum)
+{
+  gdbarch *arch = frame_unwind_arch (next_frame);
+  i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (arch);
+  ULONGEST fstat
+    = frame_unwind_register_unsigned (next_frame, I387_FSTAT_REGNUM (tdep));
+  int tos = (fstat >> 11) & 0x7;
+  int mmxreg = regnum - tdep->mm0_regnum;
+  int fpreg = (mmxreg + tos) % 8;
+
+  return (I387_ST0_REGNUM (tdep) + fpreg);
+}
+
 /* A helper function for us by i386_pseudo_register_read_value and
    amd64_pseudo_register_read_value.  It does all the work but reads
    the data into an already-allocated value.  */
 
-void
-i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
-				      readable_regcache *regcache,
-				      int regnum,
-				      struct value *result_value)
+value *
+i386_pseudo_register_read_value (gdbarch *gdbarch, frame_info_ptr next_frame,
+				 const int pseudo_reg_num)
 {
-  gdb_byte raw_buf[I386_MAX_REGISTER_SIZE];
-  enum register_status status;
-  gdb_byte *buf = result_value->contents_raw ().data ();
-
-  if (i386_mmx_regnum_p (gdbarch, regnum))
+  if (i386_mmx_regnum_p (gdbarch, pseudo_reg_num))
     {
-      int fpnum = i386_mmx_regnum_to_fp_regnum (regcache, regnum);
+      int fpnum = i386_mmx_regnum_to_fp_regnum (next_frame, pseudo_reg_num);
 
       /* Extract (always little endian).  */
-      status = regcache->raw_read (fpnum, raw_buf);
-      if (status != REG_VALID)
-	result_value->mark_bytes_unavailable (0,
-					      result_value->type ()->length ());
-      else
-	memcpy (buf, raw_buf, register_size (gdbarch, regnum));
+      return pseudo_from_raw_part (next_frame, pseudo_reg_num, fpnum, 0);
     }
   else
     {
       i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (gdbarch);
-      if (i386_bnd_regnum_p (gdbarch, regnum))
+      if (i386_bnd_regnum_p (gdbarch, pseudo_reg_num))
 	{
-	  regnum -= tdep->bnd0_regnum;
+	  int i = pseudo_reg_num - 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)
-	    result_value->mark_bytes_unavailable (0, 16);
-	  else
+	  value *bndr_value
+	    = value_of_register (I387_BND0R_REGNUM (tdep) + i, next_frame);
+	  int size = builtin_type (gdbarch)->builtin_data_ptr->length ();
+	  value *result
+	    = value::allocate_register (next_frame, pseudo_reg_num);
+
+	  /* Copy the lower. */
+	  bndr_value->contents_copy (result, 0, 0, size);
+
+	  /* Copy the upper.  */
+	  bndr_value->contents_copy (result, size, 8, size);
+
+	  /* If upper bytes are available, compute ones' complement.  */
+	  if (result->bytes_available (size, size))
 	    {
 	      bfd_endian byte_order
-		= gdbarch_byte_order (current_inferior ()->arch ());
-	      LONGEST upper, lower;
-	      int size = builtin_type (gdbarch)->builtin_data_ptr->length ();
-
-	      lower = extract_unsigned_integer (raw_buf, 8, byte_order);
-	      upper = extract_unsigned_integer (raw_buf + 8, 8, byte_order);
+		= gdbarch_byte_order (frame_unwind_arch (next_frame));
+	      gdb::array_view<gdb_byte> upper_bytes
+		= result->contents_raw ().slice (size, size);
+	      ULONGEST upper
+		= extract_unsigned_integer (upper_bytes, byte_order);
 	      upper = ~upper;
-
-	      memcpy (buf, &lower, size);
-	      memcpy (buf + size, &upper, size);
+	      store_unsigned_integer (upper_bytes, byte_order, upper);
 	    }
+
+	  return result;
 	}
-      else if (i386_zmm_regnum_p (gdbarch, regnum))
+      else if (i386_zmm_regnum_p (gdbarch, pseudo_reg_num))
 	{
-	  regnum -= tdep->zmm0_regnum;
-
-	  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)
-		result_value->mark_bytes_unavailable (0, 16);
-	      else
-		memcpy (buf, raw_buf, 16);
+	  /* Which register is it, relative to zmm0.  */
+	  int i_0 = pseudo_reg_num - tdep->zmm0_regnum;
 
-	      /* Extract (always little endian).  Read upper 128bits.  */
-	      status = regcache->raw_read (tdep->ymm0h_regnum + regnum,
-					   raw_buf);
-	      if (status != REG_VALID)
-		result_value->mark_bytes_unavailable (16, 16);
-	      else
-		memcpy (buf + 16, raw_buf, 16);
-	    }
+	  if (i_0 < num_lower_zmm_regs)
+	    return pseudo_from_concat_raw (next_frame, pseudo_reg_num,
+					   I387_XMM0_REGNUM (tdep) + i_0,
+					   tdep->ymm0h_regnum + i_0,
+					   tdep->zmm0h_regnum + i_0);
 	  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)
-		result_value->mark_bytes_unavailable (0, 16);
-	      else
-		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)
-		result_value->mark_bytes_unavailable (16, 16);
-	      else
-		memcpy (buf + 16, raw_buf, 16);
-	    }
+	      /* Which register is it, relative to zmm16.  */
+	      int i_16 = i_0 - num_lower_zmm_regs;
 
-	  /* Read upper 256bits.  */
-	  status = regcache->raw_read (tdep->zmm0h_regnum + regnum,
-				       raw_buf);
-	  if (status != REG_VALID)
-	    result_value->mark_bytes_unavailable (32, 32);
-	  else
-	    memcpy (buf + 32, raw_buf, 32);
+	      return pseudo_from_concat_raw (next_frame, pseudo_reg_num,
+					     I387_XMM16_REGNUM (tdep) + i_16,
+					     I387_YMM16H_REGNUM (tdep) + i_16,
+					     tdep->zmm0h_regnum + i_0);
+	    }
 	}
-      else if (i386_ymm_regnum_p (gdbarch, regnum))
+      else if (i386_ymm_regnum_p (gdbarch, pseudo_reg_num))
 	{
-	  regnum -= tdep->ymm0_regnum;
+	  int i = pseudo_reg_num - 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)
-	    result_value->mark_bytes_unavailable (0, 16);
-	  else
-	    memcpy (buf, raw_buf, 16);
-	  /* Read upper 128bits.  */
-	  status = regcache->raw_read (tdep->ymm0h_regnum + regnum,
-				       raw_buf);
-	  if (status != REG_VALID)
-	    result_value->mark_bytes_unavailable (16, 32);
-	  else
-	    memcpy (buf + 16, raw_buf, 16);
+	  return pseudo_from_concat_raw (next_frame, pseudo_reg_num,
+					 I387_XMM0_REGNUM (tdep) + i,
+					 tdep->ymm0h_regnum + i);
 	}
-      else if (i386_ymm_avx512_regnum_p (gdbarch, regnum))
+      else if (i386_ymm_avx512_regnum_p (gdbarch, pseudo_reg_num))
 	{
-	  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)
-	    result_value->mark_bytes_unavailable (0, 16);
-	  else
-	    memcpy (buf, raw_buf, 16);
-	  /* Read upper 128bits.  */
-	  status = regcache->raw_read (tdep->ymm16h_regnum + regnum,
-				       raw_buf);
-	  if (status != REG_VALID)
-	    result_value->mark_bytes_unavailable (16, 16);
-	  else
-	    memcpy (buf + 16, raw_buf, 16);
+	  int i = pseudo_reg_num - tdep->ymm16_regnum;
+
+	  return pseudo_from_concat_raw (next_frame, pseudo_reg_num,
+					 I387_XMM16_REGNUM (tdep) + i,
+					 tdep->ymm16h_regnum + i);
 	}
-      else if (i386_word_regnum_p (gdbarch, regnum))
+      else if (i386_word_regnum_p (gdbarch, pseudo_reg_num))
 	{
-	  int gpnum = regnum - tdep->ax_regnum;
+	  int gpnum = pseudo_reg_num - tdep->ax_regnum;
 
 	  /* Extract (always little endian).  */
-	  status = regcache->raw_read (gpnum, raw_buf);
-	  if (status != REG_VALID)
-	    result_value->mark_bytes_unavailable (0,
-						  result_value->type ()->length ());
-	  else
-	    memcpy (buf, raw_buf, 2);
+	  return pseudo_from_raw_part (next_frame, pseudo_reg_num, gpnum, 0);
 	}
-      else if (i386_byte_regnum_p (gdbarch, regnum))
+      else if (i386_byte_regnum_p (gdbarch, pseudo_reg_num))
 	{
-	  int gpnum = regnum - tdep->al_regnum;
+	  int gpnum = pseudo_reg_num - tdep->al_regnum;
 
 	  /* Extract (always little endian).  We read both lower and
 	     upper registers.  */
-	  status = regcache->raw_read (gpnum % 4, raw_buf);
-	  if (status != REG_VALID)
-	    result_value->mark_bytes_unavailable (0,
-						  result_value->type ()->length ());
-	  else if (gpnum >= 4)
-	    memcpy (buf, raw_buf + 1, 1);
-	  else
-	    memcpy (buf, raw_buf, 1);
+	  return pseudo_from_raw_part (next_frame, pseudo_reg_num, gpnum % 4,
+				       gpnum >= 4 ? 1 : 0);
 	}
       else
 	internal_error (_("invalid regnum"));
     }
 }
 
-static struct value *
-i386_pseudo_register_read_value (struct gdbarch *gdbarch,
-				 readable_regcache *regcache,
-				 int regnum)
-{
-  struct value *result;
-
-  result = value::allocate (register_type (gdbarch, regnum));
-  result->set_lval (lval_register);
-  VALUE_REGNUM (result) = regnum;
-
-  i386_pseudo_register_read_into_value (gdbarch, regcache, regnum, result);
-
-  return result;
-}
-
 void
 i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 			    int regnum, const gdb_byte *buf)
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 4283a52c9292..970dc8904f2a 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -376,10 +376,9 @@  extern const char *i386_pseudo_register_name (struct gdbarch *gdbarch,
 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,
-						  int regnum,
-						  struct value *result);
+extern value *i386_pseudo_register_read_value (gdbarch *gdbarch,
+					       frame_info_ptr next_frame,
+					       int regnum);
 
 extern void i386_pseudo_register_write (struct gdbarch *gdbarch,
 					struct regcache *regcache,
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 218257aa085f..09e376fe3861 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -733,8 +733,9 @@  readable_regcache::cooked_read (int regnum, gdb::array_view<gdb_byte> dst)
     {
       register_status result = REG_VALID;
       scoped_value_mark mark;
-      value *computed
-	= gdbarch_pseudo_register_read_value (m_descr->gdbarch, this, regnum);
+      value *computed = gdbarch_pseudo_register_read_value
+	(m_descr->gdbarch, get_next_frame_sentinel_okay (get_current_frame ()),
+	 regnum);
 
       if (computed->entirely_available ())
 	copy (computed->contents_raw (), dst);
@@ -786,8 +787,9 @@  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
+      (m_descr->gdbarch, get_next_frame_sentinel_okay (get_current_frame ()),
+       regnum);
 }
 
 enum register_status
@@ -1933,10 +1935,18 @@  cooked_read_test (struct gdbarch *gdbarch)
 	break;
     }
 
-  readwrite_regcache readwrite (&mockctx.mock_inferior, gdbarch);
+  /* Install this regcache in the regcaches global structure, so that.  */
+  pid_ptid_regcache_map &x = regcaches[&mockctx.mock_target];
+  ptid_regcache_map &y = x[mockctx.mock_ptid.pid ()];
+  regcache &readwrite
+    = *y.emplace (std::make_pair (mockctx.mock_ptid,
+				  std::make_unique<readwrite_regcache> (
+				    &mockctx.mock_inferior, gdbarch)))
+	 ->second;
+
   readwrite.set_ptid (mockctx.mock_ptid);
-  gdb::byte_vector buf (register_size (gdbarch, nonzero_regnum));
 
+  gdb::byte_vector buf (register_size (gdbarch, nonzero_regnum));
   readwrite.raw_read (nonzero_regnum, buf);
 
   /* raw_read calls target_fetch_registers.  */
@@ -2034,6 +2044,8 @@  cooked_read_test (struct gdbarch *gdbarch)
 
       mockctx.mock_target.reset ();
     }
+
+  regcaches.erase (&mockctx.mock_target);
 }
 
 /* Test regcache::cooked_write by writing some expected contents to
@@ -2048,7 +2060,17 @@  cooked_write_test (struct gdbarch *gdbarch)
 
   /* Create a mock environment.  A process_stratum target pushed.  */
   scoped_mock_context<target_ops_no_register> ctx (gdbarch);
-  readwrite_regcache readwrite (&ctx.mock_inferior, gdbarch);
+
+
+  /* Install this regcache in the regcaches global structure, so that.  */
+  pid_ptid_regcache_map &x = regcaches[&ctx.mock_target];
+  ptid_regcache_map &y = x[ctx.mock_ptid.pid ()];
+  regcache &readwrite
+    = *y.emplace (std::make_pair (ctx.mock_ptid,
+				  std::make_unique<readwrite_regcache> (
+				    &ctx.mock_inferior, gdbarch)))
+	 ->second;
+
   readwrite.set_ptid (ctx.mock_ptid);
   const int num_regs = gdbarch_num_cooked_regs (gdbarch);
 
@@ -2129,6 +2151,8 @@  cooked_write_test (struct gdbarch *gdbarch)
       SELF_CHECK (readwrite.cooked_read (regnum, buf) == REG_VALID);
       SELF_CHECK (expected == buf);
     }
+
+  regcaches.erase (&ctx.mock_target);
 }
 
 /* Verify that when two threads with the same ptid exist (from two different
diff --git a/gdb/value.c b/gdb/value.c
index a2acd1f26e03..cddd5216e9f9 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -4041,6 +4041,76 @@  value::fetch_lazy ()
   set_lazy (false);
 }
 
+/* See value.h.  */
+
+value *
+pseudo_from_raw_part (frame_info_ptr next_frame, int pseudo_reg_num,
+		      int raw_reg_num, int raw_offset)
+{
+  value *pseudo_reg_val
+    = value::allocate_register (next_frame, pseudo_reg_num);
+  value *raw_reg_val = value_of_register (raw_reg_num, next_frame);
+  raw_reg_val->contents_copy (pseudo_reg_val, 0, raw_offset,
+			      pseudo_reg_val->type ()->length ());
+  return pseudo_reg_val;
+}
+
+/* See value.h.  */
+
+value *
+pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num,
+			int raw_reg_1_num, int raw_reg_2_num)
+{
+  value *pseudo_reg_val
+    = value::allocate_register (next_frame, pseudo_reg_num);
+  int dst_offset = 0;
+
+  value *raw_reg_1_val = value_of_register (raw_reg_1_num, next_frame);
+  raw_reg_1_val->contents_copy (pseudo_reg_val, dst_offset, 0,
+				raw_reg_1_val->type ()->length ());
+  dst_offset += raw_reg_1_val->type ()->length ();
+
+  value *raw_reg_2_val = value_of_register (raw_reg_2_num, next_frame);
+  raw_reg_2_val->contents_copy (pseudo_reg_val, dst_offset, 0,
+				raw_reg_2_val->type ()->length ());
+  dst_offset += raw_reg_2_val->type ()->length ();
+
+  gdb_assert (dst_offset == pseudo_reg_val->type ()->length ());
+
+  return pseudo_reg_val;
+}
+
+/* See value.h.  */
+
+value *
+pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num,
+			int raw_reg_1_num, int raw_reg_2_num,
+			int raw_reg_3_num)
+{
+  value *pseudo_reg_val
+    = value::allocate_register (next_frame, pseudo_reg_num);
+  int dst_offset = 0;
+
+  value *raw_reg_1_val = value_of_register (raw_reg_1_num, next_frame);
+  raw_reg_1_val->contents_copy (pseudo_reg_val, dst_offset, 0,
+				raw_reg_1_val->type ()->length ());
+  dst_offset += raw_reg_1_val->type ()->length ();
+
+  value *raw_reg_2_val = value_of_register (raw_reg_2_num, next_frame);
+  raw_reg_2_val->contents_copy (pseudo_reg_val, dst_offset, 0,
+				raw_reg_2_val->type ()->length ());
+  dst_offset += raw_reg_2_val->type ()->length ();
+
+  value *raw_reg_3_val = value_of_register (raw_reg_3_num, next_frame);
+  raw_reg_3_val->contents_copy (pseudo_reg_val, dst_offset, 0,
+				raw_reg_3_val->type ()->length ());
+  dst_offset += raw_reg_3_val->type ()->length ();
+
+  gdb_assert (dst_offset == pseudo_reg_val->type ()->length ());
+
+  return pseudo_reg_val;
+}
+
 /* Implementation of the convenience function $_isvoid.  */
 
 static struct value *
diff --git a/gdb/value.h b/gdb/value.h
index 2f3b41e26ea4..935d9ebfd8c9 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1650,4 +1650,30 @@  struct scoped_array_length_limiting
   std::optional<int> m_old_value;
 };
 
+/* Helpers for building pseudo register values from raw registers.  */
+
+/* Create a value for pseudo register PSEUDO_REG_NUM by using bytes from
+   raw register RAW_REG_NUM starting at RAW_OFFSET.
+
+   The size of the pseudo register specifies how many bytes to use.  The
+   offset plus the size must not overflow the raw register's size.  */
+
+value *pseudo_from_raw_part (frame_info_ptr next_frame, int pseudo_reg_num,
+			     int raw_reg_num, int raw_offset);
+
+/* Create a value for pseudo register PSEUDO_REG_NUM by concatenating raw
+   registers RAW_REG_1_NUM and RAW_REG_2_NUM.
+
+   The sum of the sizes of raw registers must be equal to the size of the
+   pseudo register.  */
+
+value *pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num,
+			       int raw_reg_1_num, int raw_reg_2_num);
+
+/* Same as the above, but with three raw registers.  */
+
+value *pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num,
+			       int raw_reg_1_num, int raw_reg_2_num,
+			       int raw_reg_3_num);
+
 #endif /* !defined (VALUE_H) */