This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH v2 2/3] Display names of remote threads


This patch adds support for thread names in the remote protocol, and
updates gdb/gdbserver to use it.  The information is added to the XML
description sent in response to the qXfer:threads:read packet.

Note for Eli:

  You commented on the original patch that the last sentence of the doc
  paragraph doesn't make sense and should be removed.  Regardless if that
  is true or not, the sentence was already there and is not added by this
  patch.  I left it there for now, if we want to remove it, it will be in
  a separate patch.

Note for Pedro:

  I think I went over all the comments you made on the original patch.
  However, the code changed quite a bit, so it needs a full review anyway.

gdb/ChangeLog:

	* linux-nat.c (linux_nat_thread_name): Replace implementation by call
	to linux_proc_tid_get_name.
	* nat/linux-procfs.c (linux_proc_tid_get_name): New function,
	implementation inspired by linux_nat_thread_name.
	* nat/linux-procfs.h (linux_proc_tid_get_name): New declaration.
	* remote.c (struct private_thread_info) <name>: New field.
	(free_private_thread_info): Free name field.
	(remote_thread_name): New function.
	(thread_item_t) <name>: New field.
	(clear_threads_listing_context): Free name field.
	(start_thread): Get name xml attribute.
	(thread_attributes): Add "name" attribute.
	(remote_update_thread_list): Copy name field.
	(init_remote_ops): Assign remote_thread_name callback.
	* target.h (target_thread_name): Update comment.
	* NEWS: Mention remote thread name support.

gdb/gdbserver/ChangeLog:

	* linux-low.c (linux_target_ops): Use linux_proc_tid_get_name.
	* server.c (handle_qxfer_threads_worker): Refactor to include thread
	name in reply.
	* target.h (struct target_ops) <thread_name>: New field.
	(target_thread_name): New macro.

gdb/doc/ChangeLog:

	* gdb.texinfo (Thread List Format): Mention thread names.
---
 gdb/NEWS                  |  5 +++++
 gdb/doc/gdb.texinfo       |  8 +++++---
 gdb/gdbserver/linux-low.c |  3 ++-
 gdb/gdbserver/server.c    | 16 +++++++++-------
 gdb/gdbserver/target.h    |  8 ++++++++
 gdb/linux-nat.c           | 33 +--------------------------------
 gdb/nat/linux-procfs.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
 gdb/nat/linux-procfs.h    |  7 +++++++
 gdb/remote.c              | 29 ++++++++++++++++++++++++++++-
 gdb/target.h              |  4 ++--
 10 files changed, 109 insertions(+), 46 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 31072b7..5f704fe 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -94,6 +94,11 @@ set remote exec-event-feature-packet
 show remote exec-event-feature-packet
   Set/show the use of the remote exec event feature.
 
+ * Thread names in remote protocol
+
+   The reply to qXfer:threads:read may now include a name attribute for each
+   thread.
+
 *** Changes in GDB 7.10
 
 * Support for process record-replay and reverse debugging on aarch64*-linux*
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1917008..5840c47 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39542,7 +39542,7 @@ the following structure:
 @smallexample
 <?xml version="1.0"?>
 <threads>
-    <thread id="id" core="0">
+    <thread id="id" core="0" name="name">
     ... description ...
     </thread>
 </threads>
@@ -39551,8 +39551,10 @@ the following structure:
 Each @samp{thread} element must have the @samp{id} attribute that
 identifies the thread (@pxref{thread-id syntax}).  The
 @samp{core} attribute, if present, specifies which processor core
-the thread was last executing on.  The content of the of @samp{thread}
-element is interpreted as human-readable auxilliary information.
+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
+auxilliary information.
 
 @node Traceframe Info Format
 @section Traceframe Info Format
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index e3a56a7..a70868c 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -7043,7 +7043,8 @@ static struct target_ops linux_target_ops = {
   linux_mntns_unlink,
   linux_mntns_readlink,
   linux_breakpoint_kind_from_pc,
-  linux_sw_breakpoint_from_kind
+  linux_sw_breakpoint_from_kind,
+  linux_proc_tid_get_name,
 };
 
 static void
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 7d6c9cc..0105b99 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1456,20 +1456,22 @@ handle_qxfer_threads_worker (struct inferior_list_entry *inf, void *arg)
   char ptid_s[100];
   int core = target_core_of_thread (ptid);
   char core_s[21];
+  const char *name = target_thread_name (ptid);
 
   write_ptid (ptid_s, ptid);
 
+  buffer_xml_printf (buffer, "<thread id=\"%s\"", ptid_s);
+
   if (core != -1)
     {
       sprintf (core_s, "%d", core);
-      buffer_xml_printf (buffer, "<thread id=\"%s\" core=\"%s\"/>\n",
-			 ptid_s, core_s);
-    }
-  else
-    {
-      buffer_xml_printf (buffer, "<thread id=\"%s\"/>\n",
-			 ptid_s);
+      buffer_xml_printf (buffer, " core=\"%s\"", core_s);
     }
+
+  if (name != NULL)
+    buffer_xml_printf (buffer, " name=\"%s\"", name);
+
+  buffer_xml_printf (buffer, "/>\n");
 }
 
 /* Helper for handle_qxfer_threads.  */
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 358a8ab..8903da5 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -453,6 +453,10 @@ struct target_ops
      specific meaning like the Z0 kind parameter.
      SIZE is set to the software breakpoint's length in memory.  */
   const gdb_byte *(*sw_breakpoint_from_kind) (int kind, int *size);
+
+  /* Return the thread's name, or NULL if the target is unable to determine it.
+     The returned value must not be freed by the caller.  */
+  const char *(*thread_name) (ptid_t thread);
 };
 
 extern struct target_ops *the_target;
@@ -663,6 +667,10 @@ ptid_t mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
   (the_target->core_of_thread ? (*the_target->core_of_thread) (ptid) \
    : -1)
 
+#define target_thread_name(ptid)                                \
+  (the_target->thread_name ? (*the_target->thread_name) (ptid)  \
+   : NULL)
+
 int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len);
 
 int write_inferior_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 2421687..9bc1324 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4100,38 +4100,7 @@ linux_nat_pid_to_str (struct target_ops *ops, ptid_t ptid)
 static const char *
 linux_nat_thread_name (struct target_ops *self, struct thread_info *thr)
 {
-  int pid = ptid_get_pid (thr->ptid);
-  long lwp = ptid_get_lwp (thr->ptid);
-#define FORMAT "/proc/%d/task/%ld/comm"
-  char buf[sizeof (FORMAT) + 30];
-  FILE *comm_file;
-  char *result = NULL;
-
-  snprintf (buf, sizeof (buf), FORMAT, pid, lwp);
-  comm_file = gdb_fopen_cloexec (buf, "r");
-  if (comm_file)
-    {
-      /* Not exported by the kernel, so we define it here.  */
-#define COMM_LEN 16
-      static char line[COMM_LEN + 1];
-
-      if (fgets (line, sizeof (line), comm_file))
-	{
-	  char *nl = strchr (line, '\n');
-
-	  if (nl)
-	    *nl = '\0';
-	  if (*line != '\0')
-	    result = line;
-	}
-
-      fclose (comm_file);
-    }
-
-#undef COMM_LEN
-#undef FORMAT
-
-  return result;
+  return linux_proc_tid_get_name (thr->ptid);
 }
 
 /* Accepts an integer PID; Returns a string representing a file that
diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index 24bcb01..78cbb03 100644
--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -187,6 +187,48 @@ linux_proc_pid_is_zombie (pid_t pid)
 
 /* See linux-procfs.h.  */
 
+const char *
+linux_proc_tid_get_name (ptid_t ptid)
+{
+#define TASK_COMM_LEN 16  /* As defined in the kernel's sched.h.  */
+
+  static char comm_buf[TASK_COMM_LEN];
+  char comm_path[100];
+  FILE *comm_file;
+  const char *comm_val;
+  pid_t pid = ptid_get_pid (ptid);
+  pid_t tid = ptid_lwp_p (ptid) ? ptid_get_lwp (ptid) : ptid_get_pid (ptid);
+
+  xsnprintf (comm_path, sizeof (comm_path),
+	     "/proc/%ld/task/%ld/comm", (long) pid, (long) tid);
+
+  comm_file = gdb_fopen_cloexec (comm_path, "r");
+  if (comm_file == NULL)
+    return NULL;
+
+  comm_val = fgets (comm_buf, sizeof (comm_buf), comm_file);
+  fclose (comm_file);
+
+  if (comm_val != NULL)
+    {
+      int i;
+
+      /* Make sure there is no newline at the end.  */
+      for (i = 0; i < sizeof (comm_buf); i++)
+	{
+	  if (comm_buf[i] == '\n')
+	    {
+	      comm_buf[i] = '\0';
+	      break;
+	    }
+	}
+    }
+
+  return comm_val;
+}
+
+/* See linux-procfs.h.  */
+
 void
 linux_proc_attach_tgid_threads (pid_t pid,
 				linux_proc_attach_lwp_func attach_lwp)
diff --git a/gdb/nat/linux-procfs.h b/gdb/nat/linux-procfs.h
index f9cad39..f28c1ba 100644
--- a/gdb/nat/linux-procfs.h
+++ b/gdb/nat/linux-procfs.h
@@ -54,6 +54,13 @@ extern int linux_proc_pid_is_zombie_nowarn (pid_t pid);
 
 extern int linux_proc_pid_is_gone (pid_t pid);
 
+/* Return a string giving the thread's name or NULL if the
+   information is unavailable.  The returned value points to a statically
+   allocated buffer.  The value therefore becomes invalid at the next
+   linux_proc_tid_get_name call.  */
+
+extern const char *linux_proc_tid_get_name (ptid_t ptid);
+
 /* Callback function for linux_proc_attach_tgid_threads.  If the PTID
    thread is not yet known, try to attach to it and return true,
    otherwise return false.  */
diff --git a/gdb/remote.c b/gdb/remote.c
index 2bbab62..71f8628 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -437,6 +437,7 @@ struct remote_state
 struct private_thread_info
 {
   char *extra;
+  char *name;
   int core;
 };
 
@@ -444,6 +445,7 @@ static void
 free_private_thread_info (struct private_thread_info *info)
 {
   xfree (info->extra);
+  xfree (info->name);
   xfree (info);
 }
 
@@ -2141,6 +2143,18 @@ remote_thread_alive (struct target_ops *ops, ptid_t ptid)
   return (rs->buf[0] == 'O' && rs->buf[1] == 'K');
 }
 
+/* Return a pointer to a thread name if we know it and NULL otherwise.
+   The thread_info object owns the memory for the name.  */
+
+static const char *
+remote_thread_name (struct target_ops *ops, struct thread_info *info)
+{
+  if (info != NULL && info->priv != NULL)
+    return info->priv->name;
+
+  return NULL;
+}
+
 /* About these extended threadlist and threadinfo packets.  They are
    variable length packets but, the fields within them are often fixed
    length.  They are redundent enough to send over UDP as is the
@@ -2821,6 +2835,9 @@ typedef struct thread_item
   /* The thread's extra info.  May be NULL.  */
   char *extra;
 
+  /* The thread's name.  May be NULL.  */
+  char *name;
+
   /* The core the thread was running on.  -1 if not known.  */
   int core;
 } thread_item_t;
@@ -2847,7 +2864,10 @@ clear_threads_listing_context (void *p)
   struct thread_item *item;
 
   for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
-    xfree (item->extra);
+    {
+      xfree (item->extra);
+      xfree (item->name);
+    }
 
   VEC_free (thread_item_t, context->items);
 }
@@ -2951,6 +2971,9 @@ start_thread (struct gdb_xml_parser *parser,
   else
     item.core = -1;
 
+  attr = xml_find_attribute (attributes, "name");
+  item.name = attr != NULL ? xstrdup (attr->value) : NULL;
+
   item.extra = 0;
 
   VEC_safe_push (thread_item_t, data->items, &item);
@@ -2971,6 +2994,7 @@ end_thread (struct gdb_xml_parser *parser,
 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 },
   { NULL, GDB_XML_AF_NONE, NULL, NULL }
 };
 
@@ -3149,6 +3173,8 @@ remote_update_thread_list (struct target_ops *ops)
 	      info->core = item->core;
 	      info->extra = item->extra;
 	      item->extra = NULL;
+	      info->name = item->name;
+	      item->name = NULL;
 	    }
 	}
     }
@@ -12738,6 +12764,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_pass_signals = remote_pass_signals;
   remote_ops.to_program_signals = remote_program_signals;
   remote_ops.to_thread_alive = remote_thread_alive;
+  remote_ops.to_thread_name = remote_thread_name;
   remote_ops.to_update_thread_list = remote_update_thread_list;
   remote_ops.to_pid_to_str = remote_pid_to_str;
   remote_ops.to_extra_thread_info = remote_threads_extra_info;
diff --git a/gdb/target.h b/gdb/target.h
index ac28a41..65b717c 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1820,8 +1820,8 @@ extern char *normal_pid_to_str (ptid_t ptid);
 #define target_extra_thread_info(TP) \
      (current_target.to_extra_thread_info (&current_target, TP))
 
-/* Return the thread's name.  A NULL result means that the target
-   could not determine this thread's name.  */
+/* Return the thread's name, or NULL if the target is unable to determine it.
+   The returned value must not be freed by the caller.  */
 
 extern const char *target_thread_name (struct thread_info *);
 
-- 
2.5.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]