diff mbox series

gdb, gdbserver, gdbsupport: turn regcache_register_size into methods

Message ID 20250107212936.2759850-1-simon.marchi@polymtl.ca
State New
Headers
Series gdb, gdbserver, gdbsupport: turn regcache_register_size into methods |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 fail Patch failed to apply

Commit Message

Simon Marchi Jan. 7, 2025, 9:29 p.m. UTC 
  From: Simon Marchi <simon.marchi@polymtl.ca>

Add an abstract virtual method "register_size" to reg_buffer_common, and
implement in both sides (gdb and gdbserver).

Change-Id: I1f55dd6f409a6b8798d57366505fdad2bda61ea8
---
 gdb/amd64-windows-tdep.c     |  2 +-
 gdb/i387-tdep.c              |  4 ++--
 gdb/nat/aarch64-hw-point.c   |  2 +-
 gdb/regcache-dump.c          |  2 +-
 gdb/regcache.c               | 20 +++++++++-----------
 gdb/regcache.h               |  3 +++
 gdbserver/regcache.cc        |  7 +++----
 gdbserver/regcache.h         |  3 +++
 gdbsupport/common-regcache.h | 14 +++++---------
 9 files changed, 28 insertions(+), 29 deletions(-)


base-commit: 69053cf7eba1001f1a24a78f30d6334a88ec5a56

Comments

Thiago Jung Bauermann Jan. 8, 2025, 1:32 a.m. UTC | #1 
Hello Simon,

simon.marchi@polymtl.ca writes:

> From: Simon Marchi <simon.marchi@polymtl.ca>
>
> Add an abstract virtual method "register_size" to reg_buffer_common, and
> implement in both sides (gdb and gdbserver).
>
> Change-Id: I1f55dd6f409a6b8798d57366505fdad2bda61ea8

This is very close to patch 1 in my latest SVE variable size registers
series¹. :)

Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>

--
Thiago

¹ https://inbox.sourceware.org/gdb-patches/20241102025635.586759-2-thiago.bauermann@linaro.org/
Simon Marchi Jan. 8, 2025, 5:06 p.m. UTC | #2 
On 2025-01-07 20:32, Thiago Jung Bauermann wrote:
> Hello Simon,
> 
> simon.marchi@polymtl.ca writes:
> 
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>>
>> Add an abstract virtual method "register_size" to reg_buffer_common, and
>> implement in both sides (gdb and gdbserver).
>>
>> Change-Id: I1f55dd6f409a6b8798d57366505fdad2bda61ea8
> 
> This is very close to patch 1 in my latest SVE variable size registers
> series¹. :)
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> 
> --
> Thiago
> 
> ¹ https://inbox.sourceware.org/gdb-patches/20241102025635.586759-2-thiago.bauermann@linaro.org/

Oh, well I don't want to rob the credit for that patch from you, so I'll
rebase/update and push your patch.

For the record, I had this patch lying around for a while [1] (I'm
trying to flush those pending patches little by little), but you posted
yours first.

Simon

[1] https://review.lttng.org/c/binutils-gdb/+/13106
Thiago Jung Bauermann Jan. 8, 2025, 9:26 p.m. UTC | #3 
Simon Marchi <simon.marchi@polymtl.ca> writes:

> On 2025-01-07 20:32, Thiago Jung Bauermann wrote:
>> Hello Simon,
>>
>> simon.marchi@polymtl.ca writes:
>>
>>> From: Simon Marchi <simon.marchi@polymtl.ca>
>>>
>>> Add an abstract virtual method "register_size" to reg_buffer_common, and
>>> implement in both sides (gdb and gdbserver).
>>>
>>> Change-Id: I1f55dd6f409a6b8798d57366505fdad2bda61ea8
>>
>> This is very close to patch 1 in my latest SVE variable size registers
>> series¹. :)
>>
>> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
>>
>> --
>> Thiago
>>
>> ¹
>> https://inbox.sourceware.org/gdb-patches/20241102025635.586759-2-thiago.bauermann@linaro.org/
>
> Oh, well I don't want to rob the credit for that patch from you, so I'll
> rebase/update and push your patch.
>
> For the record, I had this patch lying around for a while [1] (I'm
> trying to flush those pending patches little by little), but you posted
> yours first.

I didn't mind actually, and was expecting you to commit your patch, and
I would rebase on top of it.

Thank you!

--
Thiago
diff mbox series

Patch

diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index e3e815e1123a..9e255bb2d433 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -211,7 +211,7 @@  amd64_windows_store_arg_in_reg (struct regcache *regcache,
 
   gdb_assert (valbuf.size () <= 8);
   std::copy (valbuf.begin (), valbuf.end (), buf.begin ());
-  size_t reg_size = regcache_register_size (regcache, regno);
+  size_t reg_size = regcache->register_size (regno);
   gdb_assert (reg_size <= buf.size ());
   gdb::array_view<gdb_byte> view (buf);
   regcache->cooked_write (regno, view.slice (0, reg_size));
diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index 96869613d103..35a039ed659b 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -1564,7 +1564,7 @@  i387_collect_xsave (const struct regcache *regcache, int regnum,
 					byte_order, I387_FCTRL_INIT_VAL);
 	      else
 		memset (FXSAVE_ADDR (tdep, regs, i), 0,
-			regcache_register_size (regcache, i));
+			regcache->register_size (i));
 	    }
 	}
     }
@@ -1887,7 +1887,7 @@  i387_collect_xsave (const struct regcache *regcache, int regnum,
 	    int regsize;
 
 	    regcache->raw_collect (i, raw);
-	    regsize = regcache_register_size (regcache, i);
+	    regsize = regcache->register_size (i);
 	    p = FXSAVE_ADDR (tdep, regs, i);
 	    if (memcmp (raw, p, regsize))
 	      {
diff --git a/gdb/nat/aarch64-hw-point.c b/gdb/nat/aarch64-hw-point.c
index 6acee0fb814c..8ab91fe85142 100644
--- a/gdb/nat/aarch64-hw-point.c
+++ b/gdb/nat/aarch64-hw-point.c
@@ -166,7 +166,7 @@  aarch64_point_is_aligned (ptid_t ptid, int is_watchpoint, CORE_ADDR addr,
       /* Set alignment to 2 only if the current process is 32-bit,
 	 since thumb instruction can be 2-byte aligned.  Otherwise, set
 	 alignment to AARCH64_HBP_ALIGNMENT.  */
-      if (regcache_register_size (regcache, 0) == 8)
+      if (regcache->register_size (0) == 8)
 	alignment = AARCH64_HBP_ALIGNMENT;
       else
 	alignment = 2;
diff --git a/gdb/regcache-dump.c b/gdb/regcache-dump.c
index 037a232189c7..fd39f95734f2 100644
--- a/gdb/regcache-dump.c
+++ b/gdb/regcache-dump.c
@@ -113,7 +113,7 @@  class register_dump_reg_buffer : public register_dump, reg_buffer
   {
     if (regnum < gdbarch_num_regs (m_gdbarch) || m_has_pseudo)
       {
-	auto size = register_size (m_gdbarch, regnum);
+	auto size = ::register_size (m_gdbarch, regnum);
 
 	if (size == 0)
 	  return;
diff --git a/gdb/regcache.c b/gdb/regcache.c
index f2c403b88d0c..23552977bb9b 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -176,13 +176,12 @@  register_size (struct gdbarch *gdbarch, int regnum)
   return size;
 }
 
-/* See gdbsupport/common-regcache.h.  */
+/* See regcache.h.  */
 
 int
-regcache_register_size (const reg_buffer_common *regcache, int n)
+reg_buffer::register_size (int n) const
 {
-  return register_size
-    (gdb::checked_static_cast<const struct regcache *> (regcache)->arch (), n);
+  return ::register_size (this->arch (), n);
 }
 
 reg_buffer::reg_buffer (gdbarch *gdbarch, bool has_pseudo)
@@ -939,7 +938,7 @@  register_status
 readable_regcache::read_part (int regnum, int offset,
 			      gdb::array_view<gdb_byte> dst, bool is_raw)
 {
-  int reg_size = register_size (arch (), regnum);
+  int reg_size = this->register_size (regnum);
 
   gdb_assert (offset >= 0);
   gdb_assert (offset + dst.size () <= reg_size);
@@ -983,7 +982,7 @@  void
 reg_buffer::raw_collect_part (int regnum, int offset,
 			      gdb::array_view<gdb_byte> dst) const
 {
-  int reg_size = register_size (arch (), regnum);
+  int reg_size = this->register_size (regnum);
 
   gdb_assert (offset >= 0);
   gdb_assert (offset + dst.size () <= reg_size);
@@ -1013,7 +1012,7 @@  register_status
 regcache::write_part (int regnum, int offset,
 		      gdb::array_view<const gdb_byte> src, bool is_raw)
 {
-  int reg_size = register_size (arch (), regnum);
+  int reg_size = this->register_size (regnum);
 
   gdb_assert (offset >= 0);
   gdb_assert (offset + src.size () <= reg_size);
@@ -1065,7 +1064,7 @@  void
 reg_buffer::raw_supply_part (int regnum, int offset,
 			     gdb::array_view<const gdb_byte> src)
 {
-  int reg_size = register_size (arch (), regnum);
+  int reg_size = this->register_size (regnum);
 
   gdb_assert (offset >= 0);
   gdb_assert (offset + src.size () <= reg_size);
@@ -1236,8 +1235,7 @@  regcache::transfer_regset_register (struct regcache *out_regcache, int regnum,
 				    const gdb_byte *in_buf, gdb_byte *out_buf,
 				    int slot_size, int offs) const
 {
-  struct gdbarch *gdbarch = arch ();
-  int reg_size = std::min (register_size (gdbarch, regnum), slot_size);
+  int reg_size = std::min (this->register_size (regnum), slot_size);
 
   /* Use part versions and reg_size to prevent possible buffer overflows when
      accessing the regcache.  */
@@ -1254,7 +1252,7 @@  regcache::transfer_regset_register (struct regcache *out_regcache, int regnum,
   else if (in_buf != nullptr)
     {
       /* Zero-extend the register value if the slot is smaller than the register.  */
-      if (slot_size < register_size (gdbarch, regnum))
+      if (slot_size < this->register_size (regnum))
 	out_regcache->raw_supply_zeroed (regnum);
       out_regcache->raw_supply_part (regnum, 0,
 				     gdb::make_array_view (in_buf + offs,
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 71f17b349c9d..0e76e8da09fb 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -199,6 +199,9 @@  class reg_buffer : public reg_buffer_common
   /* See gdbsupport/common-regcache.h.  */
   enum register_status get_register_status (int regnum) const override;
 
+  /* See gdbsupport/common-regcache.h.  */
+  int register_size (int n) const override final;
+
   /* See gdbsupport/common-regcache.h.  */
   void raw_collect (int regnum, gdb::array_view<gdb_byte> dst) const override;
 
diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
index 5b064ae04d1e..acd42089ca98 100644
--- a/gdbserver/regcache.cc
+++ b/gdbserver/regcache.cc
@@ -303,13 +303,12 @@  register_size (const struct target_desc *tdesc, int n)
   return find_register_by_number (tdesc, n).size / 8;
 }
 
-/* See gdbsupport/common-regcache.h.  */
+/* See regcache.h.  */
 
 int
-regcache_register_size (const reg_buffer_common *regcache, int n)
+regcache::register_size (int n) const
 {
-  return register_size
-    (gdb::checked_static_cast<const struct regcache *> (regcache)->tdesc, n);
+  return ::register_size (this->tdesc, n);
 }
 
 static gdb::array_view<gdb_byte>
diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h
index 12345a3439cd..90fa5be4f145 100644
--- a/gdbserver/regcache.h
+++ b/gdbserver/regcache.h
@@ -50,6 +50,9 @@  struct regcache : public reg_buffer_common
   /* See gdbsupport/common-regcache.h.  */
   enum register_status get_register_status (int regnum) const override;
 
+  /* See gdbsupport/common-regcache.h.  */
+  int register_size (int n) const override;
+
   /* See gdbsupport/common-regcache.h.  */
   void raw_supply (int regnum, gdb::array_view<const gdb_byte> src) override;
 
diff --git a/gdbsupport/common-regcache.h b/gdbsupport/common-regcache.h
index cd30ae949cad..b3a498189c83 100644
--- a/gdbsupport/common-regcache.h
+++ b/gdbsupport/common-regcache.h
@@ -48,11 +48,6 @@  enum register_status : signed char
 
 extern reg_buffer_common *get_thread_regcache_for_ptid (ptid_t ptid);
 
-/* Return the size of register numbered N in REGCACHE.  This function
-   must be provided by the client.  */
-
-extern int regcache_register_size (const reg_buffer_common *regcache, int n);
-
 /* Read the PC register.  This function must be provided by the
    client.  */
 
@@ -78,6 +73,9 @@  struct reg_buffer_common
      buffer.  */
   virtual register_status get_register_status (int regnum) const = 0;
 
+/* Return the size of register numbered N in this buffer.  */
+  virtual int register_size (int n) const = 0;
+
   /* Supply register REGNUM, whose contents are stored in SRC, to this register
      buffer.  */
   virtual void raw_supply (int regnum, gdb::array_view<const gdb_byte> src)
@@ -92,8 +90,7 @@  struct reg_buffer_common
   void raw_supply (int regnum, const gdb_byte *src)
   {
     raw_supply (regnum,
-		gdb::make_array_view (src,
-				      regcache_register_size (this, regnum)));
+		gdb::make_array_view (src, this->register_size (regnum)));
   }
 
   /* Supply part of register REGNUM with zeroed value.  Start at OFFSET in
@@ -116,8 +113,7 @@  struct reg_buffer_common
   void raw_collect (int regnum, gdb_byte *dst)
   {
     raw_collect (regnum,
-		 gdb::make_array_view (dst,
-				       regcache_register_size (this, regnum)));
+		 gdb::make_array_view (dst, this->register_size (regnum)));
   }
 
   /* Compare the contents of the register stored in the regcache (ignoring the