[binutils-gdb] gdb: testsuite: Test whether PC register is expedited in gdb.server/server-run.exp

Thiago Bauermann bauermann@sourceware.org
Tue Sep 24 18:19:49 GMT 2024


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=94aedcf7ea5b1dbd8bf79247a8ed247b75b3dfb0

commit 94aedcf7ea5b1dbd8bf79247a8ed247b75b3dfb0
Author: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Date:   Thu Sep 5 01:42:02 2024 -0300

    gdb: testsuite: Test whether PC register is expedited in gdb.server/server-run.exp
    
    One thing GDB always does when the inferior stops is finding out where
    it's stopped at, by way of querying the value of the program counter
    register.
    
    To save a packet round trip, the remote target can send the PC
    value (often alongside other frequently consulted registers such as the
    stack pointer) in the stop reply packet as an "expedited register".
    
    Test that this is actually done for the targets where gdbserver is
    supposed to.
    
    Extend the "maintenance print remote-registers" command output with an
    "Expedited" column which says "yes" if the register was seen by GDB in
    the last stop reply packet it received, and is left blank otherwise.
    
    Tested for regressions on aarch64-linux-gnu native-extended-remote.
    
    The testcase was tested on aarch64-linux-gnu, i686-linux-gnu and
    x86_64-linux-gnu native-remote and native-extended-remote targets.
    
    Reviewed-By: Eli Zaretskii <eliz@gnu.org>
    Approved-By: Tom Tromey <tom@tromey.com>

Diff:
---
 gdb/NEWS                                |  5 +++++
 gdb/doc/gdb.texinfo                     |  6 +++++-
 gdb/regcache-dump.c                     | 19 +++++++++++++------
 gdb/remote.c                            | 27 ++++++++++++++++++++++++++-
 gdb/remote.h                            |  5 +++++
 gdb/testsuite/gdb.server/server-run.exp | 33 +++++++++++++++++++++++++++++++++
 6 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 5e0ac313f0a..26dd4de82a2 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -90,6 +90,11 @@ save gdb-index
   it contains white space or quote characters.  If the argument
   contains no such special characters then quoting is not required.
 
+maintenance print remote-registers
+  Add an "Expedited" column to the output of the command.  It indicates
+  which registers were included in the last stop reply packet received by
+  GDB.
+
 * New remote packets
 
 vFile:stat
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 891344b1bf3..143e498b742 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -42082,7 +42082,11 @@ including registers which aren't available on the target nor visible
 to user; the command @code{maint print register-groups} includes the
 groups that each register is a member of; and the command @code{maint
 print remote-registers} includes the remote target's register numbers
-and offsets in the `G' packets.
+and offsets in the `G' packets, as well as an indication of which
+registers were included in the last stop reply packet received by
+@value{GDBN} (@pxref{Stop Reply Packets}).  Please note that the list
+of registers included in a stop reply can change from one stop to the
+next.
 
 These commands take an optional parameter, a file name to which to
 write the information.
diff --git a/gdb/regcache-dump.c b/gdb/regcache-dump.c
index bc665dc08a6..6b711bf6c2a 100644
--- a/gdb/regcache-dump.c
+++ b/gdb/regcache-dump.c
@@ -162,7 +162,7 @@ class register_dump_remote : public register_dump
   {
     if (regnum < 0)
       {
-	gdb_printf (file, "Rmt Nr  g/G Offset");
+	gdb_printf (file, "Rmt Nr  g/G Offset  Expedited");
       }
     else if (regnum < gdbarch_num_regs (m_gdbarch))
       {
@@ -170,7 +170,12 @@ class register_dump_remote : public register_dump
 
 	if (remote_register_number_and_offset (m_gdbarch, regnum,
 					       &pnum, &poffset))
-	  gdb_printf (file, "%7d %11d", pnum, poffset);
+	  {
+	    if (remote_register_is_expedited (regnum))
+	      gdb_printf (file, "%7d %11d  yes", pnum, poffset);
+	    else
+	      gdb_printf (file, "%7d %11d", pnum, poffset);
+	  }
       }
   }
 };
@@ -324,9 +329,11 @@ _initialize_regcache_dump ()
 	     "Takes an optional file parameter."),
 	   &maintenanceprintlist);
   add_cmd ("remote-registers", class_maintenance,
-	   maintenance_print_remote_registers, _("\
-Print the internal register configuration including remote register number "
-"and g/G packets offset.\n\
-Takes an optional file parameter."),
+	   maintenance_print_remote_registers,
+	   _("Print the internal register configuration including remote "
+	     "register number and g/G packets offset.\n"
+	     "Also prints which registers were sent in the last stop reply "
+	     "packet (i.e. expedited).\n"
+	     "Takes an optional file parameter."),
 	   &maintenanceprintlist);
 }
diff --git a/gdb/remote.c b/gdb/remote.c
index 8e704a770ce..4ec25ee2342 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -696,6 +696,10 @@ class remote_state
      qSupported.  */
   gdb_thread_options supported_thread_options = 0;
 
+  /* Contains the regnums of the expedited registers in the last stop
+     reply packet.  */
+  std::set<int> last_seen_expedited_registers;
+
 private:
   /* Asynchronous signal handle registered as event loop source for
      when we have pending events ready to be passed to the core.  */
@@ -1493,6 +1497,20 @@ is_remote_target (process_stratum_target *target)
   return as_remote_target (target) != nullptr;
 }
 
+/* See remote.h.  */
+
+bool
+remote_register_is_expedited (int regnum)
+{
+  remote_target *rt = as_remote_target (current_inferior ()->process_target ());
+
+  if (rt == nullptr)
+    return false;
+
+  remote_state *rs = rt->get_remote_state ();
+  return rs->last_seen_expedited_registers.count (regnum) > 0;
+}
+
 /* Per-program-space data key.  */
 static const registry<program_space>::key<char, gdb::xfree_deleter<char>>
   remote_pspace_data;
@@ -8523,6 +8541,10 @@ remote_target::process_stop_reply (stop_reply_up stop_reply,
 {
   *status = stop_reply->ws;
   ptid_t ptid = stop_reply->ptid;
+  struct remote_state *rs = get_remote_state ();
+
+  /* Forget about last reply's expedited registers.  */
+  rs->last_seen_expedited_registers.clear ();
 
   /* If no thread/process was reported by the stub then select a suitable
      thread/process.  */
@@ -8549,7 +8571,10 @@ remote_target::process_stop_reply (stop_reply_up stop_reply,
 					stop_reply->arch);
 
 	  for (cached_reg_t &reg : stop_reply->regcache)
-	    regcache->raw_supply (reg.num, reg.data.get ());
+	    {
+	      regcache->raw_supply (reg.num, reg.data.get ());
+	      rs->last_seen_expedited_registers.insert (reg.num);
+	    }
 	}
 
       remote_thread_info *remote_thr = get_remote_thread_info (this, ptid);
diff --git a/gdb/remote.h b/gdb/remote.h
index cb0a66da66e..bfe3c65b463 100644
--- a/gdb/remote.h
+++ b/gdb/remote.h
@@ -121,4 +121,9 @@ extern void send_remote_packet (gdb::array_view<const char> &buf,
 
 extern bool is_remote_target (process_stratum_target *target);
 
+/* Return true if REGNUM was returned as an expedited register in the last
+   stop reply we received.  */
+
+extern bool remote_register_is_expedited (int regnum);
+
 #endif
diff --git a/gdb/testsuite/gdb.server/server-run.exp b/gdb/testsuite/gdb.server/server-run.exp
index 92eb38bd9db..af5a5f53ca0 100644
--- a/gdb/testsuite/gdb.server/server-run.exp
+++ b/gdb/testsuite/gdb.server/server-run.exp
@@ -52,3 +52,36 @@ if { [istarget *-*-linux*] } {
 
 gdb_breakpoint main
 gdb_test "continue" "Breakpoint.* main .*" "continue to main"
+
+if { [istarget "aarch64*-*-*"]
+     || [istarget "arm*-*-*"]
+     || [istarget "csky*-*-*"]
+     || [istarget "loongarch*-*-*"]
+     || [istarget "riscv*-*-*"] } {
+    set pc_regname "pc"
+} elseif { [is_amd64_regs_target] } {
+    set pc_regname "rip"
+} elseif { [is_x86_like_target] } {
+    set pc_regname "eip"
+} elseif { [istarget "tic6x-*-*"] } {
+    set pc_regname "PC"
+}
+
+# Sending the PC register in advance is good practice.  Test that this is
+# actually done for the targets where gdbserver is supposed to.
+set expedited_pc_test_name "send PC as expedited register in stop reply"
+if { [info exists pc_regname] } {
+    set seen_line false
+    gdb_test_multiple "maintenance print remote-registers" \
+	     $expedited_pc_test_name -lbl {
+	-re " ${pc_regname}\[\[:space:\]\]+${decimal}.*${decimal}  yes" {
+	    set seen_line true
+	    exp_continue
+	}
+	-re "\r\n$gdb_prompt $" {
+	    gdb_assert { $seen_line } $gdb_test_name
+	}
+    }
+} else {
+    untested $expedited_pc_test_name
+}


More information about the Gdb-cvs mailing list