gdb, gdbserver: introduce the 'x' RSP packet for binary memory read

Introduce an RSP packet, 'x', for reading from the remote server
memory in binary format.  The binary write packet, 'X' already exists.
The 'x' packet is essentially the same as 'm', except that the
returned data is in binary format.  For transferring relatively large
data from the memory of the remote process, the 'x' packet can reduce the
transfer costs.

For example, without this patch, fetching ~100MB of data from a remote
target takes

  (gdb) dump binary memory temp.o 0x00007f3ba4c576c0 0x00007f3bab709400
  2024-03-13 16:17:42.626 - command started
  2024-03-13 16:18:24.151 - command finished
  Command execution time: 32.136785 (cpu), 41.525515 (wall)
  (gdb)

whereas with this patch, we obtain

  (gdb) dump binary memory temp.o 0x00007fec39fce6c0 0x00007fec40a80400
  2024-03-13 16:20:48.609 - command started
  2024-03-13 16:21:16.873 - command finished
  Command execution time: 20.447970 (cpu), 28.264202 (wall)
  (gdb)

We see improvements not only when reading bulk data as above, but also
when making a large number of small memory access requests.

For example, without this patch:

  (gdb) pipe x/100000xw $pc | wc -l
  2024-03-13 16:04:57.112 - command started
  25000
  2024-03-13 16:05:10.798 - command finished
  Command execution time: 9.952364 (cpu), 13.686581 (wall)

With this patch:

  (gdb) pipe x/100000xw $pc | wc -l
  2024-03-13 16:06:48.160 - command started
  25000
  2024-03-13 16:06:57.750 - command finished
  Command execution time: 6.541425 (cpu), 9.589839 (wall)
  (gdb)

Another example, where we create a core file of a GDB process.

  (gdb) gcore /tmp/core.1
  ...
  Command execution time: 85.496967 (cpu), 133.224373 (wall)

vs.

  (gdb) gcore /tmp/core.1
  ...
  Command execution time: 48.328885 (cpu), 115.032289 (wall)

Regression-tested on X86-64 using the unix (default) and
native-extended-gdbserver board files.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/NEWS b/gdb/NEWS
index 814cccbe716c6c3154095c4147755455010cd021..3b9cd00a9a0df9e7e9038bd942637a957f6acedf 100644 (file)
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -196,6 +196,12 @@ vFile:stat
   vFile:fstat but takes a filename rather than an open file
   descriptor.
 
+x addr,length
+  Given ADDR and LENGTH, fetch LENGTH units from the memory at address
+  ADDR and send the fetched data in binary format.  This packet is
+  equivalent to 'm', except that the data in the response are in
+  binary format.
+
 *** Changes in GDB 15
 
 * The MPX commands "show/set mpx bound" have been deprecated, as Intel

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 20742d22a3ff5398380bae48f6e296453b66ad17..7b6000abbea59fad5989615715f66b536ada3aa2 100644 (file)
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -43451,6 +43451,33 @@ for success (@pxref{Stop Reply Packets})
 @cindex @samp{vStopped} packet
 @xref{Notification Packets}.
 
+@item x @var{addr},@var{length}
+@anchor{x packet}
+@cindex @samp{x} packet
+Read @var{length} addressable memory units starting at address @var{addr}
+(@pxref{addressable memory unit}).  Note that @var{addr} does not have to
+be aligned to any particular boundary.
+
+@cindex alignment of remote memory accesses
+@cindex size of remote memory accesses
+@cindex memory, alignment and size of remote accesses
+The stub need not use any particular size or alignment when gathering
+data from memory for the response; even if @var{addr} is word-aligned
+and @var{length} is a multiple of the word size, the stub is free to
+use byte accesses, or not.  For this reason, this packet may not be
+suitable for accessing memory-mapped I/O devices.
+
+Reply:
+@table @samp
+@item b @var{XX@dots{}}
+Memory contents as binary data (@pxref{Binary Data}).
+The reply may contain fewer addressable memory units than requested if the
+server was reading from a trace frame memory and was able to read only part
+of the region of memory.
+@item E @var{NN}
+for an error
+@end table
+
 @item X @var{addr},@var{length}:@var{XX@dots{}}
 @anchor{X packet}
 @cindex @samp{X} packet

diff --git a/gdb/remote.c b/gdb/remote.c
index 8e3819bdb75de46d35e39776b4e047dcc70eaa61..d9ad6974366a514bfa46349a3b0eaf9e1f1f63b1 100644 (file)
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -233,6 +233,7 @@ private:
 enum {
   PACKET_vCont = 0,
   PACKET_X,
+  PACKET_x,
   PACKET_qSymbol,
   PACKET_P,
   PACKET_p,
@@ -9669,7 +9670,6 @@ remote_target::remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr,
 {
   struct remote_state *rs = get_remote_state ();
   int buf_size_bytes;          /* Max size of packet output buffer.  */
-  char *p;
   int todo_units;
   int decoded_bytes;
 
@@ -9681,23 +9681,79 @@ remote_target::remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr,
   todo_units = std::min (len_units,
                         (ULONGEST) (buf_size_bytes / unit_size) / 2);
 
-  /* Construct "m"<memaddr>","<len>".  */
   memaddr = remote_address_masked (memaddr);
-  p = rs->buf.data ();
-  *p++ = 'm';
-  p += hexnumstr (p, (ULONGEST) memaddr);
-  *p++ = ',';
-  p += hexnumstr (p, (ULONGEST) todo_units);
-  *p = '\0';
-  putpkt (rs->buf);
-  getpkt (&rs->buf);
+
+  /* Construct "m/x"<memaddr>","<len>".  */
+  auto send_request = [this, rs, memaddr, todo_units] (char format) -> void
+    {
+      char *buffer = rs->buf.data ();
+      *buffer++ = format;
+      buffer += hexnumstr (buffer, (ULONGEST) memaddr);
+      *buffer++ = ',';
+      buffer += hexnumstr (buffer, (ULONGEST) todo_units);
+      *buffer = '\0';
+      putpkt (rs->buf);
+    };
+
+  /* Determine which packet format to use.  The target's support for
+     'x' may be unknown.  We just try.  If it doesn't work, we try
+     again using 'm'.  */
+  char packet_format;
+  if (m_features.packet_support (PACKET_x) == PACKET_DISABLE)
+    packet_format = 'm';
+  else
+    packet_format = 'x';
+
+  send_request (packet_format);
+  int packet_len = getpkt (&rs->buf);
+  if (packet_len < 0)
+    return TARGET_XFER_E_IO;
+
+  if (m_features.packet_support (PACKET_x) == PACKET_SUPPORT_UNKNOWN)
+    {
+      if (rs->buf[0] == '\0')
+       {
+         remote_debug_printf ("binary uploading NOT supported by target");
+         m_features.m_protocol_packets[PACKET_x].support = PACKET_DISABLE;
+
+         /* Try again using 'm'.  */
+         packet_format = 'm';
+         send_request (packet_format);
+         packet_len = getpkt (&rs->buf);
+         if (packet_len < 0)
+           return TARGET_XFER_E_IO;
+       }
+      else
+       {
+         remote_debug_printf ("binary uploading supported by target");
+         m_features.m_protocol_packets[PACKET_x].support = PACKET_ENABLE;
+       }
+    }
+
   packet_result result = packet_check_result (rs->buf);
   if (result.status () == PACKET_ERROR)
     return TARGET_XFER_E_IO;
-  /* Reply describes memory byte by byte, each byte encoded as two hex
-     characters.  */
-  p = rs->buf.data ();
-  decoded_bytes = hex2bin (p, myaddr, todo_units * unit_size);
+
+  char *p = rs->buf.data ();
+  if (packet_format == 'x')
+    {
+      if (*p != 'b')
+       return TARGET_XFER_E_IO;
+
+      /* Adjust for 'b'.  */
+      p++;
+      packet_len--;
+      decoded_bytes = remote_unescape_input ((const gdb_byte *) p,
+                                            packet_len, myaddr,
+                                            todo_units * unit_size);
+    }
+  else
+    {
+      /* Reply describes memory byte by byte, each byte encoded as two hex
+        characters.  */
+      decoded_bytes = hex2bin (p, myaddr, todo_units * unit_size);
+    }
+
   /* Return what we have.  Let higher layers handle partial reads.  */
   *xfered_len_units = (ULONGEST) (decoded_bytes / unit_size);
   return (*xfered_len_units != 0) ? TARGET_XFER_OK : TARGET_XFER_EOF;
@@ -16226,6 +16282,8 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
 
   add_packet_config_cmd (PACKET_X, "X", "binary-download", 1);
 
+  add_packet_config_cmd (PACKET_x, "x", "binary-upload", 0);
+
   add_packet_config_cmd (PACKET_vCont, "vCont", "verbose-resume", 0);
 
   add_packet_config_cmd (PACKET_QPassSignals, "QPassSignals", "pass-signals",

diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc
index df606c71bba8e2f4721a66373f8729d9f50d0bd3..67225c50f81bbd6c198dea9b5a6af9189bbbaa84 100644 (file)
--- a/gdbserver/remote-utils.cc
+++ b/gdbserver/remote-utils.cc
@@ -1337,6 +1337,13 @@ decode_M_packet (const char *from, CORE_ADDR *mem_addr_ptr,
   hex2bin (from, *to_p, *len_ptr);
 }
 
+void
+decode_x_packet (const char *from, CORE_ADDR *mem_addr_ptr,
+                unsigned int *len_ptr)
+{
+  decode_m_packet_params (from, mem_addr_ptr, len_ptr, '\0');
+}
+
 int
 decode_X_packet (char *from, int packet_len, CORE_ADDR *mem_addr_ptr,
                 unsigned int *len_ptr, unsigned char **to_p)
@@ -1481,12 +1488,13 @@ look_up_one_symbol (const char *name, CORE_ADDR *addrp, int may_ask_gdb)
      while it figures out the address of the symbol.  */
   while (1)
     {
+      CORE_ADDR mem_addr;
+      unsigned char *mem_buf;
+      unsigned int mem_len;
+      int new_len = -1;
+
       if (cs.own_buf[0] == 'm')
        {
-         CORE_ADDR mem_addr;
-         unsigned char *mem_buf;
-         unsigned int mem_len;
-
          decode_m_packet (&cs.own_buf[1], &mem_addr, &mem_len);
          mem_buf = (unsigned char *) xmalloc (mem_len);
          if (read_inferior_memory (mem_addr, mem_buf, mem_len) == 0)
@@ -1497,9 +1505,42 @@ look_up_one_symbol (const char *name, CORE_ADDR *addrp, int may_ask_gdb)
          if (putpkt (cs.own_buf) < 0)
            return -1;
        }
+      else if (cs.own_buf[0] == 'x')
+       {
+         decode_x_packet (&cs.own_buf[1], &mem_addr, &mem_len);
+         mem_buf = (unsigned char *) xmalloc (mem_len);
+         if (read_inferior_memory (mem_addr, mem_buf, mem_len) == 0)
+           {
+             gdb_byte *buffer = (gdb_byte *) cs.own_buf;
+             *buffer++ = 'b';
+
+             int out_len_units;
+             new_len = remote_escape_output (mem_buf, mem_len, 1,
+                                             buffer,
+                                             &out_len_units,
+                                             PBUFSIZ);
+             new_len++; /* For the 'b' marker.  */
+
+             if (out_len_units != mem_len)
+               {
+                 write_enn (cs.own_buf);
+                 new_len = -1;
+               }
+             else
+               suppress_next_putpkt_log ();
+           }
+         else
+           write_enn (cs.own_buf);
+
+         free (mem_buf);
+         int res = ((new_len == -1)
+                    ? putpkt (cs.own_buf)
+                    : putpkt_binary (cs.own_buf, new_len));
+         if (res < 0)
+           return -1;
+       }
       else if (cs.own_buf[0] == 'v')
        {
-         int new_len = -1;
          handle_v_requests (cs.own_buf, len, &new_len);
          if (new_len != -1)
            putpkt_binary (cs.own_buf, new_len);
@@ -1574,11 +1615,13 @@ relocate_instruction (CORE_ADDR *to, CORE_ADDR oldloc)
      wait for the qRelocInsn "response".  That requires re-entering
      the main loop.  For now, this is an adequate approximation; allow
      GDB to access memory.  */
-  while (cs.own_buf[0] == 'm' || cs.own_buf[0] == 'M' || cs.own_buf[0] == 'X')
+  while (cs.own_buf[0] == 'm' || cs.own_buf[0] == 'M'
+        || cs.own_buf[0] == 'X' || cs.own_buf[0] == 'x')
     {
       CORE_ADDR mem_addr;
       unsigned char *mem_buf = NULL;
       unsigned int mem_len;
+      int new_len = -1;
 
       if (cs.own_buf[0] == 'm')
        {
@@ -1589,6 +1632,33 @@ relocate_instruction (CORE_ADDR *to, CORE_ADDR oldloc)
          else
            write_enn (cs.own_buf);
        }
+      else if (cs.own_buf[0] == 'x')
+       {
+         decode_x_packet (&cs.own_buf[1], &mem_addr, &mem_len);
+         mem_buf = (unsigned char *) xmalloc (mem_len);
+         if (read_inferior_memory (mem_addr, mem_buf, mem_len) == 0)
+           {
+             gdb_byte *buffer = (gdb_byte *) cs.own_buf;
+             *buffer++ = 'b';
+
+             int out_len_units;
+             new_len = remote_escape_output (mem_buf, mem_len, 1,
+                                             buffer,
+                                             &out_len_units,
+                                             PBUFSIZ);
+             new_len++; /* For the 'b' marker.  */
+
+             if (out_len_units != mem_len)
+               {
+                 write_enn (cs.own_buf);
+                 new_len = -1;
+               }
+             else
+               suppress_next_putpkt_log ();
+           }
+         else
+           write_enn (cs.own_buf);
+       }
       else if (cs.own_buf[0] == 'X')
        {
          if (decode_X_packet (&cs.own_buf[1], len - 1, &mem_addr,
@@ -1607,7 +1677,11 @@ relocate_instruction (CORE_ADDR *to, CORE_ADDR oldloc)
            write_enn (cs.own_buf);
        }
       free (mem_buf);
-      if (putpkt (cs.own_buf) < 0)
+
+      int res = ((new_len == -1)
+                ? putpkt (cs.own_buf)
+                : putpkt_binary (cs.own_buf, new_len));
+      if (res < 0)
        return -1;
       len = getpkt (cs.own_buf);
       if (len < 0)

diff --git a/gdbserver/remote-utils.h b/gdbserver/remote-utils.h
index 4681ca12f550eee4861bca3e1c7cb2f3a53d5be2..65ce604f9dc64958f04b76de604092e3610ecf90 100644 (file)
--- a/gdbserver/remote-utils.h
+++ b/gdbserver/remote-utils.h
@@ -57,6 +57,8 @@ void decode_m_packet (const char *from, CORE_ADDR * mem_addr_ptr,
                      unsigned int *len_ptr);
 void decode_M_packet (const char *from, CORE_ADDR * mem_addr_ptr,
                      unsigned int *len_ptr, unsigned char **to_p);
+void decode_x_packet (const char *from, CORE_ADDR *mem_addr_ptr,
+                     unsigned int *len_ptr);
 int decode_X_packet (char *from, int packet_len, CORE_ADDR * mem_addr_ptr,
                     unsigned int *len_ptr, unsigned char **to_p);
 int decode_xfer_write (char *buf, int packet_len,

diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index e5bdb3792348828a387cea947214dffe193305da..55898f59556b2d16bebe319e48321cdfb0cc9f89 100644 (file)
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -4763,6 +4763,35 @@ process_serial_event (void)
       else
        write_enn (cs.own_buf);
       break;
+    case 'x':
+      {
+       require_running_or_break (cs.own_buf);
+       decode_x_packet (&cs.own_buf[1], &mem_addr, &len);
+       int res = gdb_read_memory (mem_addr, mem_buf, len);
+       if (res < 0)
+         write_enn (cs.own_buf);
+       else
+         {
+           gdb_byte *buffer = (gdb_byte *) cs.own_buf;
+           *buffer++ = 'b';
+
+           int out_len_units;
+           new_packet_len = remote_escape_output (mem_buf, res, 1,
+                                                  buffer,
+                                                  &out_len_units,
+                                                  PBUFSIZ);
+           new_packet_len++; /* For the 'b' marker.  */
+
+           if (out_len_units != res)
+             {
+               write_enn (cs.own_buf);
+               new_packet_len = -1;
+             }
+           else
+             suppress_next_putpkt_log ();
+         }
+      }
+      break;
     case 'X':
       require_running_or_break (cs.own_buf);
       if (decode_X_packet (&cs.own_buf[1], packet_len - 1,