[v3] gdbserver, remote: introduce "id_str" in the "qXfer:threads:read" XML

Message ID 20240828115807.1154869-1-tankut.baris.aktemur@intel.com
State New
Headers
Series [v3] gdbserver, remote: introduce "id_str" in the "qXfer:threads:read" XML |

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-aarch64 success Test passed

Commit Message

Aktemur, Tankut Baris Aug. 28, 2024, 11:58 a.m. UTC
  GDB prints the target id of a thread in various places such as the
output of the "info threads" command in the "Target Id" column or when
switching to a thread.  A target can define what to print for a given
ptid by overriding the `pid_to_str` method.

The remote target is a gateway behind which one of many various
targets could be running.  The remote target converts a given ptid to
a string in a uniform way, without consulting the low target at the
server-side.

In this patch we introduce a new attribute in the XML that is sent in
response to the "qXfer:threads:read" RSP packet, so that a low target
at the server side, if it wishes, can specify what to print as the
target id of a thread.

Note that the existing "name" attribute or the "extra" text provided
in the XML are not sufficient for the server-side low target to
achieve the goal.  Those attributes, when present, are simply appended
to the target id by GDB.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
---
 gdb/NEWS            |  9 +++++++++
 gdb/doc/gdb.texinfo |  7 +++++--
 gdb/remote.c        | 26 +++++++++++++++++++++++++-
 gdbserver/server.cc |  4 ++++
 gdbserver/target.cc |  6 ++++++
 gdbserver/target.h  | 13 +++++++++++++
 6 files changed, 62 insertions(+), 3 deletions(-)
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 38807dfcf18..db7cd58b833 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -53,6 +53,15 @@  maintenance info blocks [ADDRESS]
   are listed starting at the inner global block out to the most inner
   block.
 
+* Changed remote packets
+
+qXfer:threads:read
+
+  The XML that is sent as a response can now include an "id_str"
+  attribute for a thread element.  The attribute indicates what GDB
+  should print as the target ID of the thread, for example in the
+  "info threads" command or when switching to the thread.
+
 *** 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 8cde2f22637..9d25fc1abab 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -48268,7 +48268,7 @@  the following structure:
 @smallexample
 <?xml version="1.0"?>
 <threads>
-    <thread id="id" core="0" name="name" handle="1a2b3c">
+    <thread id="id" core="0" name="name" id_str="Thread 12.34" handle="1a2b3c">
     ... description ...
     </thread>
 </threads>
@@ -48280,7 +48280,10 @@  identifies the thread (@pxref{thread-id syntax}).  The
 the thread was last executing on.  The @samp{name} attribute, if
 present, specifies the human-readable name of the thread.  The content
 of the of @samp{thread} element is interpreted as human-readable
-auxiliary information.  The @samp{handle} attribute, if present,
+auxiliary information.  The @samp{id_str} attribute, if present,
+specifies what @value{GDBN} should print as the target ID of the
+thread (e.g.@: in the @samp{info threads} command or when switching
+to the thread).  The @samp{handle} attribute, if present,
 is a hex encoded representation of the thread handle.
 
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 2c3988cb507..fbc687d6484 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1600,6 +1600,12 @@  struct remote_thread_info : public private_thread_info
   std::string name;
   int core = -1;
 
+  /* The string representation for the thread's id.
+
+     The target specifies this if they want to display the thread id
+     in a specific way.  If empty, the default approach is used.  */
+  std::string id_str;
+
   /* Thread handle, perhaps a pthread_t or thread_t value, stored as a
      sequence of bytes.  */
   gdb::byte_vector thread_handle;
@@ -4009,6 +4015,9 @@  struct thread_item
   /* The thread's name.  */
   std::string name;
 
+  /* The thread's id, translated to a string for displaying.  */
+  std::string id_str;
+
   /* The core the thread was running on.  -1 if not known.  */
   int core = -1;
 
@@ -4135,6 +4144,10 @@  start_thread (struct gdb_xml_parser *parser,
   if (attr != NULL)
     item.name = (const char *) attr->value.get ();
 
+  attr = xml_find_attribute (attributes, "id_str");
+  if (attr != nullptr)
+    item.id_str = (const char *) attr->value.get ();
+
   attr = xml_find_attribute (attributes, "handle");
   if (attr != NULL)
     item.thread_handle = hex2bin ((const char *) attr->value.get ());
@@ -4156,6 +4169,7 @@  const struct gdb_xml_attribute thread_attributes[] = {
   { "id", GDB_XML_AF_NONE, NULL, NULL },
   { "core", GDB_XML_AF_OPTIONAL, gdb_xml_parse_attr_ulongest, NULL },
   { "name", GDB_XML_AF_OPTIONAL, NULL, NULL },
+  { "id_str", GDB_XML_AF_OPTIONAL, NULL, NULL },
   { "handle", GDB_XML_AF_OPTIONAL, NULL, NULL },
   { NULL, GDB_XML_AF_NONE, NULL, NULL }
 };
@@ -4340,6 +4354,7 @@  remote_target::update_thread_list ()
 	      info->core = item.core;
 	      info->extra = std::move (item.extra);
 	      info->name = std::move (item.name);
+	      info->id_str = std::move (item.id_str);
 	      info->thread_handle = std::move (item.thread_handle);
 	    }
 	}
@@ -12307,7 +12322,16 @@  remote_target::pid_to_str (ptid_t ptid)
     {
       if (magic_null_ptid == ptid)
 	return "Thread <main>";
-      else if (m_features.remote_multi_process_p ())
+
+      thread_info *thread = this->find_thread (ptid);
+      if ((thread != nullptr) && (thread->priv != nullptr))
+	{
+	  remote_thread_info *priv = get_remote_thread_info (thread);
+	  if (!priv->id_str.empty ())
+	    return priv->id_str;
+	}
+
+      if (m_features.remote_multi_process_p ())
 	if (ptid.lwp () == 0)
 	  return normal_pid_to_str (ptid);
 	else
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index ee537760cfe..06d2de027b5 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -1996,6 +1996,7 @@  handle_qxfer_threads_worker (thread_info *thread, std::string *buffer)
   int core = target_core_of_thread (ptid);
   char core_s[21];
   const char *name = target_thread_name (ptid);
+  std::string id_str = target_thread_id_str (thread);
   int handle_len;
   gdb_byte *handle;
   bool handle_status = target_thread_handle (ptid, &handle, &handle_len);
@@ -2020,6 +2021,9 @@  handle_qxfer_threads_worker (thread_info *thread, std::string *buffer)
   if (name != NULL)
     string_xml_appendf (*buffer, " name=\"%s\"", name);
 
+  if (!id_str.empty ())
+    string_xml_appendf (*buffer, " id_str=\"%s\"", id_str.c_str ());
+
   if (handle_status)
     {
       char *handle_s = (char *) alloca (handle_len * 2 + 1);
diff --git a/gdbserver/target.cc b/gdbserver/target.cc
index 6db32da2e95..a26e1599975 100644
--- a/gdbserver/target.cc
+++ b/gdbserver/target.cc
@@ -807,6 +807,12 @@  process_stratum_target::thread_name (ptid_t thread)
   return nullptr;
 }
 
+std::string
+process_stratum_target::thread_id_str (thread_info *thread)
+{
+  return "";
+}
+
 bool
 process_stratum_target::thread_handle (ptid_t ptid, gdb_byte **handle,
 				       int *handle_len)
diff --git a/gdbserver/target.h b/gdbserver/target.h
index 3643b9110da..a46b3fa47ff 100644
--- a/gdbserver/target.h
+++ b/gdbserver/target.h
@@ -475,6 +475,13 @@  class process_stratum_target
      caller.  */
   virtual const char *thread_name (ptid_t thread);
 
+  /* Return the string translation for THREAD's id.  This gives the
+     target a chance to completely re-interpret the thread id and
+     present a target-specific description for displaying to the user.
+     Return empty if the target is fine with how an id is displayed
+     by default.  */
+  virtual std::string thread_id_str (thread_info *thread);
+
   /* Thread ID to (numeric) thread handle: Return true on success and
      false for failure.  Return pointer to thread handle via HANDLE
      and the handle's length via HANDLE_LEN.  */
@@ -735,4 +742,10 @@  bool set_desired_process ();
 
 std::string target_pid_to_str (ptid_t);
 
+static inline std::string
+target_thread_id_str (thread_info *thread)
+{
+  return the_target->thread_id_str (thread);
+}
+
 #endif /* GDBSERVER_TARGET_H */