[12/26] gdbserver: convert supply_regblock to regcache::supply_regblock

Message ID 1281d173376d23b5da5e09529aafeb6618672fd6.1677582744.git.tankut.baris.aktemur@intel.com
State New
Headers
Series gdbserver: refactor regcache and allow gradually populating |

Commit Message

Aktemur, Tankut Baris Feb. 28, 2023, 11:28 a.m. UTC
  Convert the supply_regblock function into a method of the regcache
struct.  Also do some minor code modernization.
---
 gdbserver/regcache.cc   | 32 ++++++++------------------------
 gdbserver/regcache.h    |  7 +++++--
 gdbserver/tracepoint.cc |  8 ++++----
 3 files changed, 17 insertions(+), 30 deletions(-)
  

Comments

Simon Marchi Dec. 21, 2023, 9:23 p.m. UTC | #1
On 2/28/23 06:28, Tankut Baris Aktemur via Gdb-patches wrote:
> Convert the supply_regblock function into a method of the regcache
> struct.  Also do some minor code modernization.
> ---
>  gdbserver/regcache.cc   | 32 ++++++++------------------------
>  gdbserver/regcache.h    |  7 +++++--
>  gdbserver/tracepoint.cc |  8 ++++----
>  3 files changed, 17 insertions(+), 30 deletions(-)
> 
> diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
> index 156fda7068d..31f1e7bb3dc 100644
> --- a/gdbserver/regcache.cc
> +++ b/gdbserver/regcache.cc
> @@ -376,39 +376,23 @@ supply_register_by_name_zeroed (struct regcache *regcache,
>  
>  #endif
>  
> -/* Supply the whole register set whose contents are stored in BUF, to
> -   REGCACHE.  If BUF is NULL, all the registers' values are recorded
> -   as unavailable.  */
> -
>  void
> -supply_regblock (struct regcache *regcache, const void *buf)
> +regcache::supply_regblock (const void *buf)

Add `/* See regcache.h.  */`.

Can you investigate if the buf == nullptr case is really needed?  It
seems to me like the only times we call supply_regblock with nullptr is
just after creating a fresh regcache.  Since the regcache is created
with all registers in the REG_UNAVAILABLE state, perhaps those spots can
just omit calling supply_regblock, and supply_regblock can be
simplified?

Simon
  

Patch

diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
index 156fda7068d..31f1e7bb3dc 100644
--- a/gdbserver/regcache.cc
+++ b/gdbserver/regcache.cc
@@ -376,39 +376,23 @@  supply_register_by_name_zeroed (struct regcache *regcache,
 
 #endif
 
-/* Supply the whole register set whose contents are stored in BUF, to
-   REGCACHE.  If BUF is NULL, all the registers' values are recorded
-   as unavailable.  */
-
 void
-supply_regblock (struct regcache *regcache, const void *buf)
+regcache::supply_regblock (const void *buf)
 {
-  if (buf)
+  if (buf != nullptr)
     {
-      const struct target_desc *tdesc = regcache->tdesc;
-
-      memcpy (regcache->registers, buf, tdesc->registers_size);
+      memcpy (registers, buf, tdesc->registers_size);
 #ifndef IN_PROCESS_AGENT
-      {
-	int i;
-
-	for (i = 0; i < tdesc->reg_defs.size (); i++)
-	  regcache->register_status[i] = REG_VALID;
-      }
+      for (int i = 0; i < tdesc->reg_defs.size (); i++)
+	register_status[i] = REG_VALID;
 #endif
     }
   else
     {
-      const struct target_desc *tdesc = regcache->tdesc;
-
-      memset (regcache->registers, 0, tdesc->registers_size);
+      memset (registers, 0, tdesc->registers_size);
 #ifndef IN_PROCESS_AGENT
-      {
-	int i;
-
-	for (i = 0; i < tdesc->reg_defs.size (); i++)
-	  regcache->register_status[i] = REG_UNAVAILABLE;
-      }
+      for (int i = 0; i < tdesc->reg_defs.size (); i++)
+	register_status[i] = REG_UNAVAILABLE;
 #endif
     }
 }
diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h
index 6aa03a70352..944718070b4 100644
--- a/gdbserver/regcache.h
+++ b/gdbserver/regcache.h
@@ -91,6 +91,11 @@  struct regcache : public reg_buffer_common
 
   /* Convert a string to register values and fill our register cache.  */
   void registers_from_string (const char *buf);
+
+  /* Supply the whole register set whose contents are stored in BUF,
+     to this regcache.  If BUF is NULL, all the registers' values are
+     recorded as unavailable.  */
+  void supply_regblock (const void *buf);
 };
 
 regcache *get_thread_regcache (thread_info *thread, bool fetch = true);
@@ -138,8 +143,6 @@  void supply_register_by_name (struct regcache *regcache,
 void supply_register_by_name_zeroed (struct regcache *regcache,
 				     const char *name);
 
-void supply_regblock (struct regcache *regcache, const void *buf);
-
 void collect_register (struct regcache *regcache, int n, void *buf);
 
 void collect_register_as_string (struct regcache *regcache, int n, char *buf);
diff --git a/gdbserver/tracepoint.cc b/gdbserver/tracepoint.cc
index 26003422a70..9833e7c3b0f 100644
--- a/gdbserver/tracepoint.cc
+++ b/gdbserver/tracepoint.cc
@@ -4707,7 +4707,7 @@  get_context_regcache (struct tracepoint_hit_ctx *ctx)
 	{
 	  fctx->regcache_initted = 1;
 	  fctx->regcache.initialize (ipa_tdesc, fctx->regspace);
-	  supply_regblock (&fctx->regcache, NULL);
+	  fctx->regcache.supply_regblock (nullptr);
 	  supply_fast_tracepoint_registers (&fctx->regcache, fctx->regs);
 	}
       regcache = &fctx->regcache;
@@ -4722,7 +4722,7 @@  get_context_regcache (struct tracepoint_hit_ctx *ctx)
 	{
 	  sctx->regcache_initted = 1;
 	  sctx->regcache.initialize (ipa_tdesc, sctx->regspace);
-	  supply_regblock (&sctx->regcache, NULL);
+	  sctx->regcache.supply_regblock (nullptr);
 	  /* Pass down the tracepoint address, because REGS doesn't
 	     include the PC, but we know what it must have been.  */
 	  supply_static_tracepoint_registers (&sctx->regcache,
@@ -5180,7 +5180,7 @@  fetch_traceframe_registers (int tfnum, struct regcache *regcache, int regnum)
   if (dataptr == NULL)
     {
       /* Mark registers unavailable.  */
-      supply_regblock (regcache, NULL);
+      regcache->supply_regblock (nullptr);
 
       /* We can generally guess at a PC, although this will be
 	 misleading for while-stepping frames and multi-location
@@ -5190,7 +5190,7 @@  fetch_traceframe_registers (int tfnum, struct regcache *regcache, int regnum)
 	regcache_write_pc (regcache, tpoint->address);
     }
   else
-    supply_regblock (regcache, dataptr);
+    regcache->supply_regblock (dataptr);
 
   return 0;
 }