[v3,6/7] Add thread_handle_to_thread_info support for remote targets
Commit Message
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_get_threads_with_qthreadinfo): Initialize thread_handle
field.
(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 | 4 +++-
gdb/gdbserver/server.c | 10 +++++++++
gdb/gdbserver/target.h | 10 +++++++++
gdb/gdbserver/thread-db.c | 30 ++++++++++++++++++++++++++
gdb/remote.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 112 insertions(+), 1 deletion(-)
Comments
Hi Kevin,
On 2017-07-19 02:55, 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_get_threads_with_qthreadinfo): Initialize thread_handle
> field.
> (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 | 4 +++-
> gdb/gdbserver/server.c | 10 +++++++++
> gdb/gdbserver/target.h | 10 +++++++++
> gdb/gdbserver/thread-db.c | 30 ++++++++++++++++++++++++++
> gdb/remote.c | 54
> +++++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 112 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 9d831e7..1a15a74 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -7696,6 +7696,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
Given your implementation of thread_db_thread_handle, I think we could
always install the callback and it will just return false. If we can
avoid a preprocessor conditional, I think it's preferable.
> };
>
> #ifdef HAVE_LINUX_REGSETS
> diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
> index 86cfe51..8e3eecb 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;
New line between the two fields?
> + /* The pthread_t handle. */
> + thread_t thread_handle;
> #endif
>
> /* Arch-specific additions. */
> @@ -415,6 +417,6 @@ int thread_db_look_up_one_symbol (const char
> *name, CORE_ADDR *addrp);
> whatever is required have the clone under thread_db's control. */
> 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);
> +bool 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 3838351..ac8f812 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -1526,6 +1526,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);
bool
>
> write_ptid (ptid_s, ptid);
>
> @@ -1540,6 +1543,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 be89258..ebff52e 100644
> --- a/gdb/gdbserver/target.h
> +++ b/gdb/gdbserver/target.h
> @@ -475,6 +475,11 @@ struct target_ops
>
> /* Return tdesc index for IPA. */
> int (*get_ipa_tdesc_idx) (void);
> +
> + /* 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. */
> + bool (*thread_handle) (ptid_t ptid, gdb_byte **handle, int
> *handle_len);
> };
>
> extern struct target_ops *the_target;
> @@ -693,6 +698,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)
Nit: this should be false.
> +
> 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..3a9ebbb 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;
> }
>
Add
/* See linux-low.h. */
> +bool
> +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 false;
> +
> + thread_db = get_thread_process (thread)->priv->thread_db;
> +
> + if (thread_db == NULL)
> + return false;
> +
> + lwp = get_thread_lwp (thread);
> +
> + if (!lwp->thread_known && !find_one_thread (thread->entry.id))
> + return false;
> +
> + gdb_assert (lwp->thread_known);
> +
> + *handle = (gdb_byte *) &lwp->thread_handle;
> + *handle_len = sizeof (lwp->thread_handle);
> + return true;
> +}
> +
> #ifdef USE_LIBTHREAD_DB_DIRECTLY
>
> static int
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 8e8ee6f..8cf65e7 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;
We now have a gdb::byte_vector (common/byte-vector.h) to use in these
situations. It should be a drop-in replacement.
> @@ -13515,6 +13543,30 @@ 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). */
New line here.
> +static struct thread_info *
> +remote_thread_handle_to_thread_info (struct target_ops *ops,
> + const gdb_byte *thread_handle,
> + int handle_len,
> + struct inferior *inf)
> +{
> + struct thread_info *tp;
> +
> + ALL_NON_EXITED_THREADS (tp)
> + {
> + struct private_thread_info *priv = get_private_info_thread (tp);
> +
> + if (tp->inf == inf && 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)
> {
Thanks!
Simon
@@ -7696,6 +7696,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
@@ -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. */
@@ -415,6 +417,6 @@ int thread_db_look_up_one_symbol (const char *name, CORE_ADDR *addrp);
whatever is required have the clone under thread_db's control. */
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);
+bool thread_db_thread_handle (ptid_t ptid, gdb_byte **handle, int *handle_len);
extern int have_ptrace_getregset;
@@ -1526,6 +1526,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);
@@ -1540,6 +1543,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");
}
@@ -475,6 +475,11 @@ struct target_ops
/* Return tdesc index for IPA. */
int (*get_ipa_tdesc_idx) (void);
+
+ /* 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. */
+ bool (*thread_handle) (ptid_t ptid, gdb_byte **handle, int *handle_len);
};
extern struct target_ops *the_target;
@@ -693,6 +698,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,
@@ -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;
}
+bool
+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 false;
+
+ thread_db = get_thread_process (thread)->priv->thread_db;
+
+ if (thread_db == NULL)
+ return false;
+
+ lwp = get_thread_lwp (thread);
+
+ if (!lwp->thread_known && !find_one_thread (thread->entry.id))
+ return false;
+
+ gdb_assert (lwp->thread_known);
+
+ *handle = (gdb_byte *) &lwp->thread_handle;
+ *handle_len = sizeof (lwp->thread_handle);
+ return true;
+}
+
#ifdef USE_LIBTHREAD_DB_DIRECTLY
static int
@@ -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);
}
@@ -1968,6 +1974,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;
@@ -2994,6 +3001,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);
@@ -3021,6 +3032,7 @@ clear_threads_listing_context (void *p)
{
xfree (item->extra);
xfree (item->name);
+ delete item->thread_handle;
}
VEC_free (thread_item_t, context->items);
@@ -3059,6 +3071,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);
@@ -3129,6 +3142,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);
@@ -3150,6 +3174,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 }
};
@@ -3225,6 +3250,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);
}
@@ -3330,6 +3356,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;
}
}
}
@@ -13515,6 +13543,30 @@ 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,
+ struct inferior *inf)
+{
+ struct thread_info *tp;
+
+ ALL_NON_EXITED_THREADS (tp)
+ {
+ struct private_thread_info *priv = get_private_info_thread (tp);
+
+ if (tp->inf == inf && 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)
{
@@ -13663,6 +13715,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