From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Date: Sat, 5 Oct 2024 03:19:53 +0000 (-0300)
Subject: gdbserver: Implement p and P packets
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=40f8939b622d28a681221fe2f9e460fd6bca08ad;p=thirdparty%2Fbinutils-gdb.git

gdbserver: Implement p and P packets

GDB will need the p packet to individually request "load early" registers
before using the g packet.

Not sure if P is necessary, but if p is supported, why not implement P?

Alternatively, to be more efficient there could be a packet where GDB can
specify a list of registers it wants do load or set.  Or there could be a
register to request/set the expedited registers.

Reviewed-By: Luis Machado <luis.machado@arm.com>
---

diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
index a937a34b8d0..1485d223c95 100644
--- a/gdbserver/regcache.cc
+++ b/gdbserver/regcache.cc
@@ -185,6 +185,30 @@ find_register_by_number (const struct target_desc *tdesc, int n)
 
 #ifndef IN_PROCESS_AGENT
 
+static gdb::array_view<gdb_byte> register_data (const struct regcache *regcache,
+						int n);
+
+/* See regcache.h.  */
+
+bool
+register_from_string (struct regcache *regcache, int regnum,
+		      gdb::array_view<const char> buf)
+{
+  gdb::array_view<gdb_byte> value = register_data (regcache, regnum);
+  int expected_len = value.size () * 2;
+
+  if (buf.size () != expected_len)
+    {
+      warning (_("Wrong sized packet for register %d (expected %d bytes, got %zu)"),
+	       regnum, expected_len, buf.size ());
+      return false;
+    }
+
+  hex2bin (buf.data (), value.data (), buf.size () / 2);
+
+  return true;
+}
+
 void
 registers_to_string (struct regcache *regcache, char *buf)
 {
diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h
index 89e801f5b26..03317bf9a2d 100644
--- a/gdbserver/regcache.h
+++ b/gdbserver/regcache.h
@@ -117,6 +117,13 @@ void regcache_invalidate (void);
 
 void regcache_release (void);
 
+/* Set contents of register REGNUM from BUF, interpreted as an hexadecimal
+   string.
+   Return true on success, false if there is an error.  */
+
+bool register_from_string (struct regcache *regcache, int regnum,
+			   gdb::array_view<const char> buf);
+
 /* Convert all registers to a string in the currently specified remote
    format.  */
 
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 0ce40ee50d1..2900e55c730 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -4598,6 +4598,19 @@ process_point_options (struct gdb_breakpoint *bp, const char **packet)
   *packet = dataptr;
 }
 
+static bool
+can_access_registers (client_state &cs)
+{
+  if (!target_running () || cs.current_traceframe >= 0
+      || !set_desired_thread ())
+    {
+      write_enn (cs.own_buf);
+      return false;
+    }
+
+  return true;
+}
+
 /* Event loop callback that handles a serial event.  The first byte in
    the serial buffer gets us here.  We expect characters to arrive at
    a brisk pace, so we read the rest of the packet with a blocking
@@ -4732,22 +4745,81 @@ process_serial_event (void)
 	}
       break;
     case 'G':
-      require_running_or_break (cs.own_buf);
-      if (cs.current_traceframe >= 0)
-	write_enn (cs.own_buf);
-      else
-	{
-	  struct regcache *regcache;
+      {
+	if (!can_access_registers (cs))
+	  break;
 
-	  if (!set_desired_thread ())
+	regcache *regcache = get_thread_regcache (current_thread, 1);
+	registers_from_string (regcache, &cs.own_buf[1]);
+	write_ok (cs.own_buf);
+      }
+      break;
+    case 'p':
+      {
+	if (!can_access_registers (cs))
+	  break;
+
+	int i = 1, regnum = 0;
+	char c;
+	while ((c = cs.own_buf[i++]) != '\0')
+	  {
+	    regnum = regnum << 4;
+	    regnum |= fromhex (c) & 0x0f;
+	  }
+
+	struct regcache *regcache = get_thread_regcache (current_thread, true);
+
+	if (regnum < 0 || regnum >= regcache->tdesc->reg_defs.size ())
+	  {
 	    write_enn (cs.own_buf);
-	  else
-	    {
-	      regcache = get_thread_regcache (current_thread);
-	      registers_from_string (regcache, &cs.own_buf[1]);
-	      write_ok (cs.own_buf);
-	    }
-	}
+	    break;
+	  }
+
+	fetch_inferior_registers (regcache, regnum);
+	collect_register_as_string (regcache, regnum, cs.own_buf);
+      }
+      break;
+    case 'P':
+      {
+	if (!can_access_registers (cs))
+	  break;
+
+	if (strchr (cs.own_buf, '=') == nullptr)
+	  {
+	    write_enn (cs.own_buf);
+	    break;
+	  }
+
+	int i = 1, regnum = 0;
+	char c;
+	while ((c = cs.own_buf[i++]) != '=')
+	  {
+	    regnum = regnum << 4;
+	    regnum |= fromhex (c) & 0x0f;
+	  }
+
+	struct regcache *regcache = get_thread_regcache (current_thread, true);
+
+	if (regnum < 0 || regnum >= regcache->tdesc->reg_defs.size ())
+	  {
+	    write_enn (cs.own_buf);
+	    break;
+	  }
+
+	len = strlen (&cs.own_buf[i]);
+	bool ret = register_from_string (regcache, regnum,
+					 gdb::make_array_view (&cs.own_buf[i],
+							       len));
+	if (!ret)
+	  {
+	    write_enn (cs.own_buf);
+	    break;
+	  }
+
+	/* FIXME: Why doesn't the G packet need this as well?  */
+	store_inferior_registers (regcache, regnum);
+	write_ok (cs.own_buf);
+      }
       break;
     case 'm':
       {