[v2,6/7] Add thread_handle_to_thread_info support for remote targets

Message ID 20170408230733.2ac60bcb@pinnacle.lan
State New, archived
Headers

Commit Message

Kevin Buettner April 9, 2017, 6:07 a.m. UTC
  This patch adds support to remote targets for converting a thread
handle to a thread_info struct pointer.

A thread handle is fetched via a "handle" attribute which has been
added to the qXfer:threads:read query packet.  An implementation is
provided in gdbserver for targets using the Linux kernel.

gdb/gdbserver/ChangeLog:
    
    	* linux-low.h (struct lwp_info): Add new field, thread_handle.
    	(thread_db_thread_handle): Declare.
    	* linux-low.c (linux_target_ops): Initialize thread_handle.
    	* server.c (handle_qxfer_threads_worker): Add support for
    	"handle" attribute.
    	* target.h (struct target_ops): Add new function pointer,
    	thread_handle.
    	(target_thread_handle): Define.
    	* thread-db.c (find_one_thread, attach_thread): Set thread_handle
    	field in lwp.
    	(thread_db_thread_handle): New function.
    
gdb/ChangeLog:
    
    	* remote.c (vector): Include.
    	(struct private_thread_info): Add field, thread_handle.
    	(free_private_thread_info): Deallocate storage associated with
    	thread handle.
    	(get_private_info_thread): Initialize `thread_handle' field.
    	(struct thread_item): Add field, thread_handle.
    	(clear_threads_listing_context): Deallocate storage associated
    	with thread handle.
    	(start_thread): Add support for "handle" attribute.
    	(thread_attributes): Add "handle".
    	(remote_update_thread_list): Update thread_handle.
    	(remote_thread_handle_to_thread_info): New function.
    	(init_remote_ops): Initialize to_thread_handle_to_thread_info.
---
 gdb/gdbserver/linux-low.c |  5 +++++
 gdb/gdbserver/linux-low.h |  3 +++
 gdb/gdbserver/server.c    | 10 +++++++++
 gdb/gdbserver/target.h    | 10 +++++++++
 gdb/gdbserver/thread-db.c | 30 +++++++++++++++++++++++++++
 gdb/remote.c              | 53 +++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 111 insertions(+)
  

Comments

Simon Marchi May 6, 2017, 2:28 a.m. UTC | #1
On 2017-04-09 02:07, Kevin Buettner wrote:
> This patch adds support to remote targets for converting a thread
> handle to a thread_info struct pointer.
> 
> A thread handle is fetched via a "handle" attribute which has been
> added to the qXfer:threads:read query packet.  An implementation is
> provided in gdbserver for targets using the Linux kernel.
> 
> gdb/gdbserver/ChangeLog:
> 
>     	* linux-low.h (struct lwp_info): Add new field, thread_handle.
>     	(thread_db_thread_handle): Declare.
>     	* linux-low.c (linux_target_ops): Initialize thread_handle.
>     	* server.c (handle_qxfer_threads_worker): Add support for
>     	"handle" attribute.
>     	* target.h (struct target_ops): Add new function pointer,
>     	thread_handle.
>     	(target_thread_handle): Define.
>     	* thread-db.c (find_one_thread, attach_thread): Set thread_handle
>     	field in lwp.
>     	(thread_db_thread_handle): New function.
> 
> gdb/ChangeLog:
> 
>     	* remote.c (vector): Include.
>     	(struct private_thread_info): Add field, thread_handle.
>     	(free_private_thread_info): Deallocate storage associated with
>     	thread handle.
>     	(get_private_info_thread): Initialize `thread_handle' field.
>     	(struct thread_item): Add field, thread_handle.
>     	(clear_threads_listing_context): Deallocate storage associated
>     	with thread handle.
>     	(start_thread): Add support for "handle" attribute.
>     	(thread_attributes): Add "handle".
>     	(remote_update_thread_list): Update thread_handle.
>     	(remote_thread_handle_to_thread_info): New function.
>     	(init_remote_ops): Initialize to_thread_handle_to_thread_info.

That looks good to me.  I was a bit thrown off by the std::vector 
pointers in the remote structures, but I think it's a good trade-off 
until we make the structures apt to have non trivial fields.

> diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
> index 3cc2bc4..db8b8d7 100644
> --- a/gdb/gdbserver/target.h
> +++ b/gdb/gdbserver/target.h
> @@ -474,6 +474,11 @@ struct target_ops
> 
>    /* Return tdesc index for IPA.  */
>    int (*get_ipa_tdesc_idx) (void);
> +
> +  /* Thread ID to (numeric) thread handle: Return a non-zero status on
> +     success, 0 for failure.  Return pointer to thread handle via 
> HANDLE
> +     and the handle's length via HANDLE_LEN.  */
> +  int (*thread_handle) (ptid_t ptid, gdb_byte **handle, int 
> *handle_len);

Return a bool?

> @@ -13505,6 +13533,29 @@ remote_execution_direction (struct target_ops 
> *self)
>    return rs->last_resume_exec_dir;
>  }
> 
> +/* Return pointer to the thread_info struct which corresponds to
> +   THREAD_HANDLE (having length HANDLE_LEN).  */
> +static struct thread_info *
> +remote_thread_handle_to_thread_info (struct target_ops *ops,
> +				     const gdb_byte *thread_handle,
> +				     int handle_len)
> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());

byte_order is unused.

> +  struct thread_info *tp;
> +
> +  ALL_NON_EXITED_THREADS (tp)
> +    {
> +      struct private_thread_info *priv = get_private_info_thread (tp);
> +
> +      if (priv != NULL && handle_len == priv->thread_handle->size ()
> +          && memcmp (thread_handle, priv->thread_handle->data (),
> +	             handle_len) == 0)
> +        return tp;
> +    }
> +
> +  return NULL;
> +}
> +

Thanks,

Simon
  

Patch

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 6f06eb8..4ed5a63 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -7700,6 +7700,11 @@  static struct target_ops linux_target_ops = {
   linux_supports_software_single_step,
   linux_supports_catch_syscall,
   linux_get_ipa_tdesc_idx,
+#if USE_THREAD_DB
+  thread_db_thread_handle,
+#else
+  NULL,
+#endif
 };
 
 #ifdef HAVE_LINUX_REGSETS
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 87ce53a..5d6ee40 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -374,6 +374,8 @@  struct lwp_info
   /* The thread handle, used for e.g. TLS access.  Only valid if
      THREAD_KNOWN is set.  */
   td_thrhandle_t th;
+  /* The pthread_t handle.  */
+  thread_t thread_handle;
 #endif
 
   /* Arch-specific additions.  */
@@ -410,5 +412,6 @@  int thread_db_get_tls_address (struct thread_info *thread, CORE_ADDR offset,
 			       CORE_ADDR load_module, CORE_ADDR *address);
 int thread_db_look_up_one_symbol (const char *name, CORE_ADDR *addrp);
 void thread_db_notice_clone (struct process_info *proc, ptid_t lwp);
+int thread_db_thread_handle (ptid_t ptid, gdb_byte **handle, int *handle_len);
 
 extern int have_ptrace_getregset;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 4bc7f71..0ad6c65 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1572,6 +1572,9 @@  handle_qxfer_threads_worker (struct inferior_list_entry *inf, void *arg)
   int core = target_core_of_thread (ptid);
   char core_s[21];
   const char *name = target_thread_name (ptid);
+  int handle_len;
+  gdb_byte *handle;
+  int handle_status = target_thread_handle (ptid, &handle, &handle_len);
 
   write_ptid (ptid_s, ptid);
 
@@ -1586,6 +1589,13 @@  handle_qxfer_threads_worker (struct inferior_list_entry *inf, void *arg)
   if (name != NULL)
     buffer_xml_printf (buffer, " name=\"%s\"", name);
 
+  if (handle_status)
+    {
+      char *handle_s = (char *) alloca (handle_len * 2 + 1);
+      bin2hex (handle, handle_s, handle_len);
+      buffer_xml_printf (buffer, " handle=\"%s\"", handle_s);
+    }
+
   buffer_xml_printf (buffer, "/>\n");
 }
 
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 3cc2bc4..db8b8d7 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -474,6 +474,11 @@  struct target_ops
 
   /* Return tdesc index for IPA.  */
   int (*get_ipa_tdesc_idx) (void);
+
+  /* Thread ID to (numeric) thread handle: Return a non-zero status on
+     success, 0 for failure.  Return pointer to thread handle via HANDLE
+     and the handle's length via HANDLE_LEN.  */
+  int (*thread_handle) (ptid_t ptid, gdb_byte **handle, int *handle_len);
 };
 
 extern struct target_ops *the_target;
@@ -692,6 +697,11 @@  void done_accessing_memory (void);
   (the_target->thread_name ? (*the_target->thread_name) (ptid)  \
    : NULL)
 
+#define target_thread_handle(ptid, handle, handle_len) \
+   (the_target->thread_handle ? (*the_target->thread_handle) \
+                                  (ptid, handle, handle_len) \
+   : 0)
+
 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/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
index eff1914..8f8bf20 100644
--- a/gdb/gdbserver/thread-db.c
+++ b/gdb/gdbserver/thread-db.c
@@ -200,6 +200,7 @@  find_one_thread (ptid_t ptid)
 
   lwp->thread_known = 1;
   lwp->th = th;
+  lwp->thread_handle = ti.ti_tid;
 
   return 1;
 }
@@ -231,6 +232,7 @@  attach_thread (const td_thrhandle_t *th_p, td_thrinfo_t *ti_p)
   gdb_assert (lwp != NULL);
   lwp->thread_known = 1;
   lwp->th = *th_p;
+  lwp->thread_handle = ti_p->ti_tid;
 
   return 1;
 }
@@ -439,6 +441,34 @@  thread_db_get_tls_address (struct thread_info *thread, CORE_ADDR offset,
     return err;
 }
 
+int
+thread_db_thread_handle (ptid_t ptid, gdb_byte **handle, int *handle_len)
+{
+  struct thread_db *thread_db;
+  struct lwp_info *lwp;
+  struct thread_info *thread
+    = (struct thread_info *) find_inferior_id (&all_threads, ptid);
+
+  if (thread == NULL)
+    return 0;
+
+  thread_db = get_thread_process (thread)->priv->thread_db;
+
+  if (thread_db == NULL)
+    return 0;
+
+  lwp = get_thread_lwp (thread);
+
+  if (!lwp->thread_known && !find_one_thread (thread->entry.id))
+    return 0;
+
+  gdb_assert (lwp->thread_known);
+
+  *handle = (gdb_byte *) &lwp->thread_handle;
+  *handle_len = sizeof (lwp->thread_handle);
+  return 1;
+}
+
 #ifdef USE_LIBTHREAD_DB_DIRECTLY
 
 static int
diff --git a/gdb/remote.c b/gdb/remote.c
index 73a2e51..cdac7a1 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -72,6 +72,7 @@ 
 #include "btrace.h"
 #include "record-btrace.h"
 #include <algorithm>
+#include <vector>
 
 /* Temp hacks for tracepoint encoding migration.  */
 static char *target_buf;
@@ -451,6 +452,10 @@  struct private_thread_info
   char *name;
   int core;
 
+  /* Thread handle, perhaps a pthread_t or thread_t value, stored as a
+     sequence of bytes.  */
+  std::vector<gdb_byte> *thread_handle;
+
   /* Whether the target stopped for a breakpoint/watchpoint.  */
   enum target_stop_reason stop_reason;
 
@@ -482,6 +487,7 @@  free_private_thread_info (struct private_thread_info *info)
 {
   xfree (info->extra);
   xfree (info->name);
+  delete info->thread_handle;
   xfree (info);
 }
 
@@ -1967,6 +1973,7 @@  get_private_info_thread (struct thread_info *thread)
       priv->last_resume_step = 0;
       priv->last_resume_sig = GDB_SIGNAL_0;
       priv->vcont_resumed = 0;
+      priv->thread_handle = nullptr;
     }
 
   return thread->priv;
@@ -2993,6 +3000,10 @@  typedef struct thread_item
 
   /* The core the thread was running on.  -1 if not known.  */
   int core;
+
+  /* The thread handle associated with the thread.  */
+  std::vector<gdb_byte> *thread_handle;
+
 } thread_item_t;
 DEF_VEC_O(thread_item_t);
 
@@ -3020,6 +3031,7 @@  clear_threads_listing_context (void *p)
     {
       xfree (item->extra);
       xfree (item->name);
+      delete item->thread_handle;
     }
 
   VEC_free (thread_item_t, context->items);
@@ -3058,6 +3070,7 @@  remote_newthread_step (threadref *ref, void *data)
   item.core = -1;
   item.name = NULL;
   item.extra = NULL;
+  item.thread_handle = nullptr;
 
   VEC_safe_push (thread_item_t, context->items, &item);
 
@@ -3128,6 +3141,17 @@  start_thread (struct gdb_xml_parser *parser,
   attr = xml_find_attribute (attributes, "name");
   item.name = attr != NULL ? xstrdup ((const char *) attr->value) : NULL;
 
+  attr = xml_find_attribute (attributes, "handle");
+  if (attr != NULL)
+    {
+      item.thread_handle = new std::vector<gdb_byte>
+                             (strlen ((const char *) attr->value) / 2);
+      hex2bin ((const char *) attr->value, item.thread_handle->data (),
+               item.thread_handle->size ());
+    }
+  else
+    item.thread_handle = nullptr;
+
   item.extra = 0;
 
   VEC_safe_push (thread_item_t, data->items, &item);
@@ -3149,6 +3173,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 },
+  { "handle", GDB_XML_AF_OPTIONAL, NULL, NULL },
   { NULL, GDB_XML_AF_NONE, NULL, NULL }
 };
 
@@ -3224,6 +3249,7 @@  remote_get_threads_with_qthreadinfo (struct target_ops *ops,
 		  item.core = -1;
 		  item.name = NULL;
 		  item.extra = NULL;
+		  item.thread_handle = nullptr;
 
 		  VEC_safe_push (thread_item_t, context->items, &item);
 		}
@@ -3329,6 +3355,8 @@  remote_update_thread_list (struct target_ops *ops)
 	      item->extra = NULL;
 	      info->name = item->name;
 	      item->name = NULL;
+	      info->thread_handle = item->thread_handle;
+	      item->thread_handle = nullptr;
 	    }
 	}
     }
@@ -13505,6 +13533,29 @@  remote_execution_direction (struct target_ops *self)
   return rs->last_resume_exec_dir;
 }
 
+/* Return pointer to the thread_info struct which corresponds to
+   THREAD_HANDLE (having length HANDLE_LEN).  */
+static struct thread_info *
+remote_thread_handle_to_thread_info (struct target_ops *ops,
+				     const gdb_byte *thread_handle,
+				     int handle_len)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
+  struct thread_info *tp;
+
+  ALL_NON_EXITED_THREADS (tp)
+    {
+      struct private_thread_info *priv = get_private_info_thread (tp);
+
+      if (priv != NULL && handle_len == priv->thread_handle->size ()
+          && memcmp (thread_handle, priv->thread_handle->data (),
+	             handle_len) == 0)
+        return tp;
+    }
+
+  return NULL;
+}
+
 static void
 init_remote_ops (void)
 {
@@ -13653,6 +13704,8 @@  Specify the serial device it is connected to\n\
   remote_ops.to_insert_exec_catchpoint = remote_insert_exec_catchpoint;
   remote_ops.to_remove_exec_catchpoint = remote_remove_exec_catchpoint;
   remote_ops.to_execution_direction = remote_execution_direction;
+  remote_ops.to_thread_handle_to_thread_info =
+    remote_thread_handle_to_thread_info;
 }
 
 /* Set up the extended remote vector by making a copy of the standard