diff mbox series

[RFC,v4,11/15] gdbserver: Add concept of variable-size registers to the regcache

Message ID 20241102025635.586759-12-thiago.bauermann@linaro.org
State New
Headers show
Series gdbserver improvements for AArch64 SVE support | expand

Commit Message

Thiago Jung Bauermann Nov. 2, 2024, 2:56 a.m. UTC
This is the gdbserver equivalent of the previous patch, and many of the
same remarks also apply:

1. The contents of variable-size registers are placed in a separate buffer.
2. Some places needed to be changed from calling he register_size ()
   function to calling the register_size () method instead.
3. There are FIXMEs related to the const_casts needed to remove the const
   from the "this" pointer when calling initialize_variable_size_registers ().

Probably the main thing to note in this patch is that it adds a puny DWARF
location expression parser which supports only the subset of opcodes
needed for variable-size registers.

Finally, this patch doesn't add support for handling variable-size
registers in tracepoints.  This causes some FAILs in
gdb.base/internal-string-values.exp when it uses tracepoints.
---
 gdb/regformats/regdef.h   |  19 ++++-
 gdbserver/ax.cc           |   2 +-
 gdbserver/regcache.cc     | 162 ++++++++++++++++++++++++++++++++------
 gdbserver/regcache.h      |  18 ++++-
 gdbserver/remote-utils.cc |   2 +-
 gdbserver/tdesc.cc        |  80 ++++++++++++++++++-
 gdbserver/tdesc.h         |   4 +-
 gdbserver/tracepoint.cc   |   4 +-
 8 files changed, 253 insertions(+), 38 deletions(-)
diff mbox series

Patch

diff --git a/gdb/regformats/regdef.h b/gdb/regformats/regdef.h
index 09339d495209..fcf7b177f04e 100644
--- a/gdb/regformats/regdef.h
+++ b/gdb/regformats/regdef.h
@@ -27,6 +27,8 @@  struct reg
     : name (""),
       offset (_offset),
       size (0),
+      element_bitsize (0),
+      count_locexpr (nullptr),
       load_early (false)
   {}
 
@@ -34,6 +36,8 @@  struct reg
     : name (_name),
       offset (_offset),
       size (_size),
+      element_bitsize (0),
+      count_locexpr (nullptr),
       load_early (_load_early)
   {}
 
@@ -48,9 +52,20 @@  struct reg
   /* The offset (in bits) of the value of this register in the buffer.  */
   int offset;
 
-  /* The size (in bits) of the value of this register, as transmitted.  */
+  /* The size (in bits) of the value of this register, as transmitted.  If
+     it's TDESC_REG_VARIABLE_SIZE then this register contains a vector with
+     variable number of elements given by COUNT_LOCEXPR, and each element
+     has a size of ELEMENT_BITSIZE.  */
   int size;
 
+  /* Size of each element in the vector.  Only valid if
+     size == TDESC_REG_VARIABLE_SIZE.  */
+  int element_bitsize;
+
+  /* DWARF location expression for number of elements in the vector.  Only
+     valid if size == TDESC_REG_VARIABLE_SIZE.  */
+  gdb::byte_vector *count_locexpr;
+
   /* Whether this register needs to be loaded early in the register cache,
      because variable-size registers depend on it to calculate their
      size.  */
@@ -61,6 +76,8 @@  struct reg
     return (strcmp (name, other.name) == 0
 	    && offset == other.offset
 	    && size == other.size
+	    && element_bitsize == other.element_bitsize
+	    && count_locexpr == other.count_locexpr
 	    && load_early == other.load_early);
   }
 
diff --git a/gdbserver/ax.cc b/gdbserver/ax.cc
index ff42795582ff..121ea6b420a3 100644
--- a/gdbserver/ax.cc
+++ b/gdbserver/ax.cc
@@ -1191,7 +1191,7 @@  gdb_eval_agent_expr (struct eval_agent_expr_context *ctx,
 	    int regnum = arg;
 	    struct regcache *regcache = ctx->regcache;
 
-	    switch (register_size (regcache->tdesc, regnum))
+	    switch (regcache->register_size (regnum))
 	      {
 	      case 8:
 		collect_register (regcache, regnum, cnv.u64.bytes);
diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
index cd1ee2e5f145..8d491590d995 100644
--- a/gdbserver/regcache.cc
+++ b/gdbserver/regcache.cc
@@ -19,6 +19,7 @@ 
 #include "regdef.h"
 #include "gdbthread.h"
 #include "tdesc.h"
+#include "locexpr.h"
 #include "gdbsupport/rsp-low.h"
 #include "gdbsupport/gdb-checked-static-cast.h"
 
@@ -56,6 +57,7 @@  get_thread_regcache (struct thread_info *thread, int fetch)
       /* Invalidate all registers, to prevent stale left-overs.  */
       memset (regcache->register_status, REG_UNAVAILABLE,
 	      regcache->tdesc->reg_defs.size ());
+      regcache->variable_size_registers.reset ();
       fetch_inferior_registers (regcache, -1);
       regcache->registers_valid = 1;
     }
@@ -127,13 +129,14 @@  init_register_cache (struct regcache *regcache,
 	 fetches.  This way they'll read as zero instead of
 	 garbage.  */
       regcache->tdesc = tdesc;
-      regcache->registers
-	= (unsigned char *) xcalloc (1, tdesc->registers_size);
+      regcache->fixed_size_registers
+	= (unsigned char *) xcalloc (1, tdesc->fixed_registers_size);
       regcache->registers_owned = 1;
       regcache->register_status
 	= (unsigned char *) xmalloc (tdesc->reg_defs.size ());
       memset ((void *) regcache->register_status, REG_UNAVAILABLE,
 	      tdesc->reg_defs.size ());
+      regcache->variable_size_registers.reset ();
 #else
       gdb_assert_not_reached ("can't allocate memory from the heap");
 #endif
@@ -141,10 +144,11 @@  init_register_cache (struct regcache *regcache,
   else
     {
       regcache->tdesc = tdesc;
-      regcache->registers = regbuf;
+      regcache->fixed_size_registers = regbuf;
       regcache->registers_owned = 0;
 #ifndef IN_PROCESS_AGENT
       regcache->register_status = NULL;
+      regcache->variable_size_registers.reset ();
 #endif
     }
 
@@ -160,7 +164,7 @@  new_register_cache (const struct target_desc *tdesc)
 {
   struct regcache *regcache = new struct regcache;
 
-  gdb_assert (tdesc->registers_size != 0);
+  gdb_assert (tdesc->fixed_registers_size != 0);
 
   return init_register_cache (regcache, tdesc, NULL);
 }
@@ -171,7 +175,7 @@  free_register_cache (struct regcache *regcache)
   if (regcache)
     {
       if (regcache->registers_owned)
-	free (regcache->registers);
+	free (regcache->fixed_size_registers);
       free (regcache->register_status);
       delete regcache;
     }
@@ -186,7 +190,7 @@  regcache_cpy (struct regcache *dst, struct regcache *src)
   gdb_assert (src->tdesc == dst->tdesc);
   gdb_assert (src != dst);
 
-  memcpy (dst->registers, src->registers, src->tdesc->registers_size);
+  memcpy (dst->fixed_size_registers, src->fixed_size_registers, src->tdesc->fixed_registers_size);
 #ifndef IN_PROCESS_AGENT
   if (dst->register_status != NULL && src->register_status != NULL)
     memcpy (dst->register_status, src->register_status,
@@ -255,22 +259,34 @@  register_from_string (struct regcache *regcache, int regnum, char *buf)
 void
 registers_to_string (struct regcache *regcache, char *buf)
 {
-  unsigned char *registers = regcache->registers;
+  unsigned char *registers = regcache->fixed_size_registers;
+  unsigned char *var_size_registers = regcache->variable_size_registers.get ();
   const struct target_desc *tdesc = regcache->tdesc;
 
   for (int i = 0; i < tdesc->reg_defs.size (); ++i)
     {
+      int reg_size = regcache->register_size (i);
       if (regcache->register_status[i] == REG_VALID)
 	{
-	  bin2hex (registers, buf, register_size (tdesc, i));
-	  buf += register_size (tdesc, i) * 2;
+	  unsigned char *regs;
+	  if (tdesc->reg_defs[i].size == TDESC_REG_VARIABLE_SIZE)
+	    regs = var_size_registers;
+	  else
+	    regs = registers;
+
+	  bin2hex (regs, buf, reg_size);
+	  buf += reg_size * 2;
 	}
       else
 	{
-	  memset (buf, 'x', register_size (tdesc, i) * 2);
-	  buf += register_size (tdesc, i) * 2;
+	  memset (buf, 'x', reg_size * 2);
+	  buf += reg_size * 2;
 	}
-      registers += register_size (tdesc, i);
+
+      if (tdesc->reg_defs[i].size == TDESC_REG_VARIABLE_SIZE)
+	var_size_registers += reg_size;
+      else
+	registers += reg_size;
     }
   *buf = '\0';
 }
@@ -279,17 +295,32 @@  void
 registers_from_string (struct regcache *regcache, char *buf)
 {
   int len = strlen (buf);
-  unsigned char *registers = regcache->registers;
+  unsigned char *registers = regcache->fixed_size_registers;
+  unsigned char *var_size_registers = regcache->variable_size_registers.get ();
   const struct target_desc *tdesc = regcache->tdesc;
+  int expected_len = (tdesc->fixed_registers_size + regcache->variable_registers_size) * 2;
 
-  if (len != tdesc->registers_size * 2)
+  if (len != expected_len)
     {
       warning ("Wrong sized register packet (expected %d bytes, got %d)",
-	       2 * tdesc->registers_size, len);
-      if (len > tdesc->registers_size * 2)
-	len = tdesc->registers_size * 2;
+	       expected_len, len);
+      if (len > expected_len)
+        len = expected_len;
+    }
+
+  for (int i = 0; i < tdesc->reg_defs.size (); ++i)
+    {
+      int reg_size = regcache->register_size (i);
+      unsigned char *regs;
+
+      if (tdesc->reg_defs[i].size == TDESC_REG_VARIABLE_SIZE)
+	regs = var_size_registers;
+      else
+	regs = registers;
+
+      hex2bin (buf, regs, reg_size);
+      buf += reg_size * 2;
     }
-  hex2bin (buf, registers, len / 2);
 }
 
 /* See regcache.h */
@@ -337,16 +368,56 @@  regcache_release (void)
 }
 #endif
 
+#ifndef IN_PROCESS_AGENT
+/* See regcache.h */
+
+void
+regcache::initialize_variable_size_registers ()
+{
+  const unsigned int num_regs = tdesc->reg_defs.size ();
+  int total_size = 0;
+
+  variable_size_sizes.resize (num_regs);
+  variable_size_offsets.resize (num_regs);
+
+  for (int i = 0; i < num_regs; i++)
+    {
+      const gdb::reg &reg = tdesc->reg_defs[i];
+      if (reg.size != TDESC_REG_VARIABLE_SIZE)
+	{
+	  variable_size_sizes[i] = -1;
+	  variable_size_offsets[i] = -1;
+	  continue;
+	}
+
+      long size = (evaluate_locexpr (*reg.count_locexpr, this)
+		   * reg.element_bitsize / 8);
+      variable_size_sizes[i] = size;
+      variable_size_offsets[i] = total_size;
+      total_size += size;
+    }
+
+  variable_size_registers.reset (new unsigned char[total_size]);
+  variable_registers_size = total_size;
+}
+#endif
+
 int
 register_cache_size (const struct target_desc *tdesc)
 {
-  return tdesc->registers_size;
+  /* Variable-size registers aren't considered here because this function is
+     only used for tracepoints.  */
+  return tdesc->fixed_registers_size;
 }
 
 int
 register_size (const struct target_desc *tdesc, int n)
 {
-  return find_register_by_number (tdesc, n).size / 8;
+  const gdb::reg &reg = find_register_by_number (tdesc, n);
+
+  gdb_assert (reg.size != TDESC_REG_VARIABLE_SIZE);
+
+  return reg.size / 8;
 }
 
 /* See gdbsupport/common-regcache.h.  */
@@ -354,15 +425,46 @@  register_size (const struct target_desc *tdesc, int n)
 int
 regcache::register_size (int regnum) const
 {
-  return ::register_size (tdesc, regnum);
+  const gdb::reg &reg = find_register_by_number (tdesc, regnum);
+
+  if (reg.size == TDESC_REG_VARIABLE_SIZE)
+    {
+#ifndef IN_PROCESS_AGENT
+      if (!variable_size_registers)
+	// FIXME: Remove cast.
+	const_cast<struct regcache *> (this)->initialize_variable_size_registers ();
+
+      return variable_size_sizes[regnum];
+#else
+      gdb_assert_not_reached ("Variable-size registers not supported.");
+#endif
+    }
+  else
+    return reg.size / 8;
 }
 
 static gdb::array_view<gdb_byte>
 register_data (const struct regcache *regcache, int n)
 {
   const gdb::reg &reg = find_register_by_number (regcache->tdesc, n);
-  return gdb::make_array_view (regcache->registers + reg.offset / 8,
-			       reg.size / 8);
+
+  if (reg.size == TDESC_REG_VARIABLE_SIZE)
+    {
+#ifndef IN_PROCESS_AGENT
+      if (!regcache->variable_size_registers)
+	// FIXME: Remove cast.
+	const_cast<struct regcache *> (regcache)->initialize_variable_size_registers ();
+
+      return gdb::make_array_view (regcache->variable_size_registers.get ()
+				   + regcache->variable_size_offsets[n],
+				   regcache->variable_size_sizes[n]);
+#else
+      gdb_assert_not_reached ("Variable-size registers not supported.");
+#endif
+    }
+  else
+    return gdb::make_array_view (regcache->fixed_size_registers + reg.offset / 8,
+				 reg.size / 8);
 }
 
 void
@@ -396,6 +498,12 @@  regcache::raw_supply (int n, gdb::array_view<const gdb_byte> src)
 	register_status[n] = REG_UNAVAILABLE;
 #endif
     }
+
+#ifndef IN_PROCESS_AGENT
+  if (this->is_load_early_register (n))
+    /* Invalidate variable-size registers.  */
+    this->initialize_variable_size_registers ();
+#endif
 }
 
 /* Supply register N with value zero to REGCACHE.  */
@@ -435,7 +543,7 @@  supply_regblock (struct regcache *regcache, const void *buf)
     {
       const struct target_desc *tdesc = regcache->tdesc;
 
-      memcpy (regcache->registers, buf, tdesc->registers_size);
+      memcpy (regcache->fixed_size_registers, buf, tdesc->fixed_registers_size);
 #ifndef IN_PROCESS_AGENT
       {
 	int i;
@@ -449,7 +557,7 @@  supply_regblock (struct regcache *regcache, const void *buf)
     {
       const struct target_desc *tdesc = regcache->tdesc;
 
-      memset (regcache->registers, 0, tdesc->registers_size);
+      memset (regcache->fixed_size_registers, 0, tdesc->fixed_registers_size);
 #ifndef IN_PROCESS_AGENT
       {
 	int i;
@@ -457,6 +565,8 @@  supply_regblock (struct regcache *regcache, const void *buf)
 	for (i = 0; i < tdesc->reg_defs.size (); i++)
 	  regcache->register_status[i] = REG_UNAVAILABLE;
       }
+
+      regcache->variable_size_registers.reset ();
 #endif
     }
 }
@@ -498,7 +608,7 @@  regcache_raw_read_unsigned (reg_buffer_common *reg_buf, int regnum,
 
   gdb_assert (regcache != NULL);
 
-  size = register_size (regcache->tdesc, regnum);
+  size = regcache->register_size (regnum);
 
   if (size > (int) sizeof (ULONGEST))
     error (_("That operation is not available on integers of more than"
diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h
index 07e48a7432b7..e1202d708a6a 100644
--- a/gdbserver/regcache.h
+++ b/gdbserver/regcache.h
@@ -40,10 +40,26 @@  struct regcache : public reg_buffer_common
      in a traceframe.  For that, check REGISTER_STATUS below.  */
   int registers_valid = 0;
   int registers_owned = 0;
-  unsigned char *registers = nullptr;
+  unsigned char *fixed_size_registers = nullptr;
 #ifndef IN_PROCESS_AGENT
+  /* The variable-size register buffers (if any).  */
+  std::unique_ptr<unsigned char[]> variable_size_registers;
+
+  /* Size of the variable_size_registers buffer, if there is one.  */
+  int variable_registers_size;
+
   /* One of REG_UNAVAILABLE or REG_VALID.  */
   unsigned char *register_status = nullptr;
+
+  /* Offsets into variable_size_registers.  */
+  std::vector<long> variable_size_offsets;
+
+  /* Size of each variable-size register.  */
+  std::vector<long> variable_size_sizes;
+
+  /* Initialize (or reset) information about variable-size registers in this
+     regcache.  */
+  void initialize_variable_size_registers ();
 #endif
 
   /* See gdbsupport/common-regcache.h.  */
diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc
index 98c34e91220c..b4e4b6145eb4 100644
--- a/gdbserver/remote-utils.cc
+++ b/gdbserver/remote-utils.cc
@@ -1043,7 +1043,7 @@  outreg (struct regcache *regcache, int regno, char *buf)
   *buf++ = tohex (regno & 0xf);
   *buf++ = ':';
   collect_register_as_string (regcache, regno, buf);
-  buf += 2 * register_size (regcache->tdesc, regno);
+  buf += 2 * regcache->register_size (regno);
   *buf++ = ';';
 
   return buf;
diff --git a/gdbserver/tdesc.cc b/gdbserver/tdesc.cc
index fbaf756f5050..bf5a5c6e79a4 100644
--- a/gdbserver/tdesc.cc
+++ b/gdbserver/tdesc.cc
@@ -51,6 +51,69 @@  void target_desc::accept (tdesc_element_visitor &v) const
 #endif
 }
 
+/* Return bit size of given TYPE.  */
+
+static int
+tdesc_type_bitsize (const tdesc_type *type)
+{
+  switch (type->kind) {
+  case TDESC_TYPE_INT8:
+  case TDESC_TYPE_UINT8:
+    return 8;
+  case TDESC_TYPE_IEEE_HALF:
+  case TDESC_TYPE_INT16:
+  case TDESC_TYPE_UINT16:
+  case TDESC_TYPE_BFLOAT16:
+    return 16;
+  case TDESC_TYPE_IEEE_SINGLE:
+  case TDESC_TYPE_INT32:
+  case TDESC_TYPE_UINT32:
+    return 32;
+  case TDESC_TYPE_IEEE_DOUBLE:
+  case TDESC_TYPE_INT64:
+  case TDESC_TYPE_UINT64:
+    return 64;
+  case TDESC_TYPE_I387_EXT:
+    return 80;
+  case TDESC_TYPE_ARM_FPA_EXT:
+    return 96;
+  case TDESC_TYPE_INT128:
+  case TDESC_TYPE_UINT128:
+    return 128;
+  default:
+    /* The other types require a gdbarch to determine their size.  */
+    error ("Target description uses unsupported type in variable-size register.");
+  }
+}
+
+/* Sets up REG size information using TYPE.  */
+
+static void
+setup_variable_size_reg (tdesc_type *type, gdb::reg &reg)
+{
+  /* We only support vector types or unions containing vector types as
+     variable-size.  */
+  gdb_assert (type->kind == TDESC_TYPE_UNION
+	      || type->kind == TDESC_TYPE_VECTOR);
+
+  if (type->kind == TDESC_TYPE_VECTOR)
+    {
+      tdesc_type_vector *vec_type = static_cast<tdesc_type_vector *> (type);
+
+      reg.element_bitsize = tdesc_type_bitsize (vec_type->element_type);
+      reg.count_locexpr = &vec_type->locexpr;
+    }
+  else
+    {
+      tdesc_type_with_fields *union_type
+	= static_cast<tdesc_type_with_fields *> (type);
+
+      /* We assume that all fields in the union have the same size, so
+	 just get the first one.  */
+      setup_variable_size_reg (union_type->fields.front ().type, reg);
+    }
+}
+
 void
 init_target_desc (struct target_desc *tdesc,
 		  const char **expedite_regs)
@@ -73,17 +136,26 @@  init_target_desc (struct target_desc *tdesc,
 
 	tdesc->reg_defs.emplace_back (treg->name.c_str (), offset,
 				      treg->bitsize, treg->load_early);
-	offset += treg->bitsize;
+
+	if (treg->bitsize == TDESC_REG_VARIABLE_SIZE)
+	  {
+	    tdesc_type *type
+		= tdesc_named_type (feature.get (), treg->type.c_str ());
+
+	    setup_variable_size_reg (type, tdesc->reg_defs.back ());
+	  }
+	else
+	  offset += treg->bitsize;
 
 	if (treg->load_early)
 	  expedite_from_features.push_back (treg->name.c_str ());
       }
 
-  tdesc->registers_size = offset / 8;
+  tdesc->fixed_registers_size = offset / 8;
 
   /* Make sure PBUFSIZ is large enough to hold a full register
      packet.  */
-  gdb_assert (2 * tdesc->registers_size + 32 <= PBUFSIZ);
+  gdb_assert (2 * tdesc->fixed_registers_size + 32 <= PBUFSIZ);
 
 #ifndef IN_PROCESS_AGENT
   /* Drop the contents of the previous vector, if any.  */
@@ -127,7 +199,7 @@  copy_target_description (struct target_desc *dest,
 {
   dest->reg_defs = src->reg_defs;
   dest->expedite_regs = src->expedite_regs;
-  dest->registers_size = src->registers_size;
+  dest->fixed_registers_size = src->fixed_registers_size;
   dest->xmltarget = src->xmltarget;
 
   if (src->arch)
diff --git a/gdbserver/tdesc.h b/gdbserver/tdesc.h
index 4796b50b4d13..b7a0166c6e65 100644
--- a/gdbserver/tdesc.h
+++ b/gdbserver/tdesc.h
@@ -34,7 +34,7 @@  struct target_desc final : tdesc_element
   std::vector<struct gdb::reg> reg_defs;
 
   /* The register cache size, in bytes.  */
-  int registers_size;
+  int fixed_registers_size;
 
   /* XML features in this target description.  */
   std::vector<tdesc_feature_up> features;
@@ -61,7 +61,7 @@  struct target_desc final : tdesc_element
 
 public:
   target_desc ()
-    : registers_size (0)
+    : fixed_registers_size (0)
   {}
 
   bool operator== (const target_desc &other) const;
diff --git a/gdbserver/tracepoint.cc b/gdbserver/tracepoint.cc
index b7d0ef2c2932..1cbf0fddc1e6 100644
--- a/gdbserver/tracepoint.cc
+++ b/gdbserver/tracepoint.cc
@@ -5097,7 +5097,7 @@  traceframe_walk_blocks (unsigned char *database, unsigned int datasize,
 	{
 	case 'R':
 	  /* Skip over the registers block.  */
-	  dataptr += current_target_desc ()->registers_size;
+	  dataptr += current_target_desc ()->fixed_registers_size;
 	  break;
 	case 'M':
 	  /* Skip over the memory block.  */
@@ -5776,7 +5776,7 @@  gdb_collect (struct tracepoint *tpoint, unsigned char *regs)
   ctx.regcache_initted = 0;
   /* Wrap the regblock in a register cache (in the stack, we don't
      want to malloc here).  */
-  ctx.regspace = (unsigned char *) alloca (ipa_tdesc->registers_size);
+  ctx.regspace = (unsigned char *) alloca (ipa_tdesc->fixed_registers_size);
   if (ctx.regspace == NULL)
     {
       trace_debug ("Trace buffer block allocation failed, skipping");