* Re: [PATCH v3 2/7] Add `thread_from_thread_handle' method to (Python) gdb.Inferior
2017-07-19 0:42 [PATCH v3 0/7] Thread handle to thread info mapping Kevin Buettner
@ 2017-07-19 0:54 ` Kevin Buettner
2017-07-23 20:53 ` Simon Marchi
2017-07-19 0:54 ` [PATCH v3 1/7] Add target method for converting thread handle to thread_info struct pointer Kevin Buettner
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Kevin Buettner @ 2017-07-19 0:54 UTC (permalink / raw)
To: gdb-patches
gdb/ChangeLog:
* python/py-inferior.c (gdbpy_thread_from_thread_handle): New
function.
(inferior_object_methods): Add gdbpy_thread_from_thread_handle.
* python/python-internal.h (thread_object_type): Declare.
---
gdb/python/py-inferior.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
gdb/python/python-internal.h | 2 ++
2 files changed, 49 insertions(+)
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index f6a24a0..082ded0 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -751,6 +751,49 @@ infpy_is_valid (PyObject *self, PyObject *args)
Py_RETURN_TRUE;
}
+/* Implementation of gdb.Inferior.thread_from_thread_handle (self, handle)
+ -> gdb.InferiorThread. */
+
+PyObject *
+gdbpy_thread_from_thread_handle (PyObject *self, PyObject *args, PyObject *kw)
+{
+ PyObject *handle_obj, *result;
+ inferior_object *inf_obj = (inferior_object *) self;
+ static const char *keywords[] = { "thread_handle", NULL };
+
+ INFPY_REQUIRE_VALID (inf_obj);
+
+ if (! gdb_PyArg_ParseTupleAndKeywords (args, kw, "O", keywords, &handle_obj))
+ return NULL;
+
+ result = Py_None;
+
+ if (gdbpy_is_value_object (handle_obj))
+ {
+ TRY
+ {
+ struct thread_info *thread_info;
+ struct value *val = value_object_to_value (handle_obj);
+
+ thread_info = find_thread_by_handle (val, inf_obj->inferior);
+ if (thread_info != NULL)
+ {
+ result = (PyObject *) find_thread_object (thread_info->ptid);
+ if (result != NULL)
+ Py_INCREF (result);
+ }
+ }
+ CATCH (except, RETURN_MASK_ALL)
+ {
+ GDB_PY_HANDLE_EXCEPTION (except);
+ }
+ END_CATCH
+ }
+
+ return result;
+}
+
+
static void
infpy_dealloc (PyObject *obj)
{
@@ -861,6 +904,10 @@ Write the given buffer object to the inferior's memory." },
METH_VARARGS | METH_KEYWORDS,
"search_memory (address, length, pattern) -> long\n\
Return a long with the address of a match, or None." },
+ { "thread_from_thread_handle", (PyCFunction) gdbpy_thread_from_thread_handle,
+ METH_VARARGS | METH_KEYWORDS,
+ "thread_from_thread_handle (handle) -> gdb.InferiorThread.\n\
+Return thread object corresponding to thread handle." },
{ NULL }
};
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index ebb83f0..40e64f0 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -379,6 +379,8 @@ extern PyTypeObject breakpoint_object_type
CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_object");
extern PyTypeObject frame_object_type
CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("frame_object");
+extern PyTypeObject thread_object_type
+ CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("thread_object");
typedef struct gdbpy_breakpoint_object
{
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 2/7] Add `thread_from_thread_handle' method to (Python) gdb.Inferior
2017-07-19 0:54 ` [PATCH v3 2/7] Add `thread_from_thread_handle' method to (Python) gdb.Inferior Kevin Buettner
@ 2017-07-23 20:53 ` Simon Marchi
0 siblings, 0 replies; 15+ messages in thread
From: Simon Marchi @ 2017-07-23 20:53 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
Hi Kevin,
On 2017-07-19 02:54, Kevin Buettner wrote:
> gdb/ChangeLog:
> * python/py-inferior.c (gdbpy_thread_from_thread_handle): New
> function.
> (inferior_object_methods): Add gdbpy_thread_from_thread_handle.
> * python/python-internal.h (thread_object_type): Declare.
> ---
> gdb/python/py-inferior.c | 47
> ++++++++++++++++++++++++++++++++++++++++++++
> gdb/python/python-internal.h | 2 ++
> 2 files changed, 49 insertions(+)
>
> diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
> index f6a24a0..082ded0 100644
> --- a/gdb/python/py-inferior.c
> +++ b/gdb/python/py-inferior.c
> @@ -751,6 +751,49 @@ infpy_is_valid (PyObject *self, PyObject *args)
> Py_RETURN_TRUE;
> }
>
> +/* Implementation of gdb.Inferior.thread_from_thread_handle (self,
> handle)
Spurious whitespace at the end of this line.
> + -> gdb.InferiorThread. */
> +
> +PyObject *
> +gdbpy_thread_from_thread_handle (PyObject *self, PyObject *args,
> PyObject *kw)
> +{
> + PyObject *handle_obj, *result;
> + inferior_object *inf_obj = (inferior_object *) self;
> + static const char *keywords[] = { "thread_handle", NULL };
> +
> + INFPY_REQUIRE_VALID (inf_obj);
> +
> + if (! gdb_PyArg_ParseTupleAndKeywords (args, kw, "O", keywords,
> &handle_obj))
> + return NULL;
> +
> + result = Py_None;
> +
> + if (gdbpy_is_value_object (handle_obj))
> + {
> + TRY
> + {
> + struct thread_info *thread_info;
> + struct value *val = value_object_to_value (handle_obj);
> +
> + thread_info = find_thread_by_handle (val, inf_obj->inferior);
> + if (thread_info != NULL)
> + {
> + result = (PyObject *) find_thread_object (thread_info->ptid);
> + if (result != NULL)
> + Py_INCREF (result);
> + }
> + }
> + CATCH (except, RETURN_MASK_ALL)
> + {
> + GDB_PY_HANDLE_EXCEPTION (except);
> + }
> + END_CATCH
> + }
Should we raise a TypeError if the argument has the wrong type? As a
user not understanding why I always receive None because I made a silly
typo, I would like if GDB told me I'm passing the wrong thing.
TypeError seems to be exactly for that:
https://docs.python.org/3/library/exceptions.html#TypeError
> +
> + return result;
> +}
> +
> +
> static void
> infpy_dealloc (PyObject *obj)
> {
> @@ -861,6 +904,10 @@ Write the given buffer object to the inferior's
> memory." },
> METH_VARARGS | METH_KEYWORDS,
> "search_memory (address, length, pattern) -> long\n\
> Return a long with the address of a match, or None." },
> + { "thread_from_thread_handle", (PyCFunction)
> gdbpy_thread_from_thread_handle,
Should the function now be named infpy_ to match the current style?
> + METH_VARARGS | METH_KEYWORDS,
> + "thread_from_thread_handle (handle) -> gdb.InferiorThread.\n\
These last two lines should be indented with one less space.
> +Return thread object corresponding to thread handle." },
> { NULL }
> };
>
> diff --git a/gdb/python/python-internal.h
> b/gdb/python/python-internal.h
> index ebb83f0..40e64f0 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -379,6 +379,8 @@ extern PyTypeObject breakpoint_object_type
> CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_object");
> extern PyTypeObject frame_object_type
> CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("frame_object");
> +extern PyTypeObject thread_object_type
> + CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("thread_object");
>
> typedef struct gdbpy_breakpoint_object
> {
Thanks,
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/7] Add target method for converting thread handle to thread_info struct pointer
2017-07-19 0:42 [PATCH v3 0/7] Thread handle to thread info mapping Kevin Buettner
2017-07-19 0:54 ` [PATCH v3 2/7] Add `thread_from_thread_handle' method to (Python) gdb.Inferior Kevin Buettner
@ 2017-07-19 0:54 ` Kevin Buettner
2017-07-23 20:39 ` Simon Marchi
2017-07-19 0:55 ` [PATCH v3 4/7] Test case for Inferior.thread_from_thread_handle Kevin Buettner
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Kevin Buettner @ 2017-07-19 0:54 UTC (permalink / raw)
To: gdb-patches
This patch adds a target method named `to_thread_handle_to_thread_info'.
It is intended to map a thread library specific thread handle (such as
pthread_t for the pthread library) to the corresponding GDB internal
thread_info struct (pointer).
An implementation is provided for Linux pthreads; see linux-thread-db.c.
gdb/ChangeLog:
* target.h (struct target_ops): Add to_thread_handle_to_thread_info.
(target_thread_handle_to_thread_info): Declare.
* target.c (target_thread_handle_to_thread_info): New function.
* target-delegates.c: Regenerate.
* gdbthread.h (find_thread_by_handle): Declare.
* thread.c (find_thread_by_handle): New function.
* linux-thread-db.c (thread_db_thread_handle_to_thread_info): New
function.
(init_thread_db_ops): Register thread_db_thread_handle_to_thread_info.
---
gdb/gdbthread.h | 4 ++++
gdb/linux-thread-db.c | 31 +++++++++++++++++++++++++++++++
gdb/target-delegates.c | 37 +++++++++++++++++++++++++++++++++++++
gdb/target.c | 9 +++++++++
gdb/target.h | 11 +++++++++++
gdb/thread.c | 10 ++++++++++
6 files changed, 102 insertions(+)
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 046bf95..c16a0d2 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -448,6 +448,10 @@ extern struct thread_info *find_thread_ptid (ptid_t ptid);
/* Find thread by GDB global thread ID. */
struct thread_info *find_thread_global_id (int global_id);
+/* Find thread by thread library specific handle in inferior INF. */
+struct thread_info *find_thread_by_handle (struct value *thread_handle,
+ struct inferior *inf);
+
/* Finds the first thread of the inferior given by PID. If PID is -1,
returns the first thread in the list. */
struct thread_info *first_thread_of_process (int pid);
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 86254f8..14dc5d2 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1410,6 +1410,36 @@ thread_db_extra_thread_info (struct target_ops *self,
return NULL;
}
+/* Return pointer to the thread_info struct which corresponds to
+ THREAD_HANDLE (having length HANDLE_LEN). */
+static struct thread_info *
+thread_db_thread_handle_to_thread_info (struct target_ops *ops,
+ const gdb_byte *thread_handle,
+ int handle_len,
+ struct inferior *inf)
+{
+ struct thread_info *tp;
+ thread_t handle_tid;
+
+ /* Thread handle sizes must match in order to proceed. We don't use an
+ assert here because the resulting internal error will cause GDB to
+ exit. This isn't necessarily an internal error due to the possibility
+ of garbage being passed as the thread handle via the python interface. */
+ if (handle_len != sizeof (handle_tid))
+ error (_("Thread handle size mismatch: %d vs %d (from libthread_db)"),
+ handle_len, (int) sizeof (handle_tid));
+
+ handle_tid = * (const thread_t *) thread_handle;
+
+ ALL_NON_EXITED_THREADS (tp)
+ {
+ if (tp->inf == inf && tp->priv != NULL && handle_tid == tp->priv->tid)
+ return tp;
+ }
+
+ return NULL;
+}
+
/* Get the address of the thread local variable in load module LM which
is stored at OFFSET within the thread local storage for thread PTID. */
@@ -1687,6 +1717,7 @@ init_thread_db_ops (void)
= thread_db_get_thread_local_address;
thread_db_ops.to_extra_thread_info = thread_db_extra_thread_info;
thread_db_ops.to_get_ada_task_ptid = thread_db_get_ada_task_ptid;
+ thread_db_ops.to_thread_handle_to_thread_info = thread_db_thread_handle_to_thread_info;
thread_db_ops.to_magic = OPS_MAGIC;
complete_target_initialization (&thread_db_ops);
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 88e3e0b..dea1d9f 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1583,6 +1583,39 @@ debug_thread_name (struct target_ops *self, struct thread_info *arg1)
return result;
}
+static struct thread_info *
+delegate_thread_handle_to_thread_info (struct target_ops *self, const gdb_byte *arg1, int arg2, struct inferior *arg3)
+{
+ self = self->beneath;
+ return self->to_thread_handle_to_thread_info (self, arg1, arg2, arg3);
+}
+
+static struct thread_info *
+tdefault_thread_handle_to_thread_info (struct target_ops *self, const gdb_byte *arg1, int arg2, struct inferior *arg3)
+{
+ return NULL;
+}
+
+static struct thread_info *
+debug_thread_handle_to_thread_info (struct target_ops *self, const gdb_byte *arg1, int arg2, struct inferior *arg3)
+{
+ struct thread_info * result;
+ fprintf_unfiltered (gdb_stdlog, "-> %s->to_thread_handle_to_thread_info (...)\n", debug_target.to_shortname);
+ result = debug_target.to_thread_handle_to_thread_info (&debug_target, arg1, arg2, arg3);
+ fprintf_unfiltered (gdb_stdlog, "<- %s->to_thread_handle_to_thread_info (", debug_target.to_shortname);
+ target_debug_print_struct_target_ops_p (&debug_target);
+ fputs_unfiltered (", ", gdb_stdlog);
+ target_debug_print_const_gdb_byte_p (arg1);
+ fputs_unfiltered (", ", gdb_stdlog);
+ target_debug_print_int (arg2);
+ fputs_unfiltered (", ", gdb_stdlog);
+ target_debug_print_struct_inferior_p (arg3);
+ fputs_unfiltered (") = ", gdb_stdlog);
+ target_debug_print_struct_thread_info_p (result);
+ fputs_unfiltered ("\n", gdb_stdlog);
+ return result;
+}
+
static void
delegate_stop (struct target_ops *self, ptid_t arg1)
{
@@ -4267,6 +4300,8 @@ install_delegators (struct target_ops *ops)
ops->to_extra_thread_info = delegate_extra_thread_info;
if (ops->to_thread_name == NULL)
ops->to_thread_name = delegate_thread_name;
+ if (ops->to_thread_handle_to_thread_info == NULL)
+ ops->to_thread_handle_to_thread_info = delegate_thread_handle_to_thread_info;
if (ops->to_stop == NULL)
ops->to_stop = delegate_stop;
if (ops->to_interrupt == NULL)
@@ -4522,6 +4557,7 @@ install_dummy_methods (struct target_ops *ops)
ops->to_pid_to_str = default_pid_to_str;
ops->to_extra_thread_info = tdefault_extra_thread_info;
ops->to_thread_name = tdefault_thread_name;
+ ops->to_thread_handle_to_thread_info = tdefault_thread_handle_to_thread_info;
ops->to_stop = tdefault_stop;
ops->to_interrupt = tdefault_interrupt;
ops->to_pass_ctrlc = default_target_pass_ctrlc;
@@ -4681,6 +4717,7 @@ init_debug_target (struct target_ops *ops)
ops->to_pid_to_str = debug_pid_to_str;
ops->to_extra_thread_info = debug_extra_thread_info;
ops->to_thread_name = debug_thread_name;
+ ops->to_thread_handle_to_thread_info = debug_thread_handle_to_thread_info;
ops->to_stop = debug_stop;
ops->to_interrupt = debug_interrupt;
ops->to_pass_ctrlc = debug_pass_ctrlc;
diff --git a/gdb/target.c b/gdb/target.c
index e526bcc..23f4b9f 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2315,6 +2315,15 @@ target_thread_name (struct thread_info *info)
return current_target.to_thread_name (¤t_target, info);
}
+struct thread_info *
+target_thread_handle_to_thread_info (const gdb_byte * thread_handle,
+ int handle_len,
+ struct inferior *inf)
+{
+ return current_target.to_thread_handle_to_thread_info
+ (¤t_target, thread_handle, handle_len, inf);
+}
+
void
target_resume (ptid_t ptid, int step, enum gdb_signal signal)
{
diff --git a/gdb/target.h b/gdb/target.h
index c0155c1..1bb8c0f 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -646,6 +646,11 @@ struct target_ops
TARGET_DEFAULT_RETURN (NULL);
const char *(*to_thread_name) (struct target_ops *, struct thread_info *)
TARGET_DEFAULT_RETURN (NULL);
+ struct thread_info *(*to_thread_handle_to_thread_info) (struct target_ops *,
+ const gdb_byte *,
+ int,
+ struct inferior *inf)
+ TARGET_DEFAULT_RETURN (NULL);
void (*to_stop) (struct target_ops *, ptid_t)
TARGET_DEFAULT_IGNORE ();
void (*to_interrupt) (struct target_ops *, ptid_t)
@@ -1857,6 +1862,12 @@ extern const char *normal_pid_to_str (ptid_t ptid);
extern const char *target_thread_name (struct thread_info *);
+/* Given a pointer to a thread library specific thread handle and
+ its length, return a pointer to the corresponding thread_info struct. */
+
+extern struct thread_info *target_thread_handle_to_thread_info
+ (const gdb_byte * thread_handle, int handle_len, struct inferior *);
+
/* Attempts to find the pathname of the executable file
that was run to create a specified process.
diff --git a/gdb/thread.c b/gdb/thread.c
index 3cf1b94..456b1ad 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -534,6 +534,16 @@ find_thread_ptid (ptid_t ptid)
return NULL;
}
+/* Find a thread_info by matching thread libary specific handle. */
+struct thread_info *
+find_thread_by_handle (struct value *thread_handle, struct inferior *inf)
+{
+ return target_thread_handle_to_thread_info
+ (value_contents_all (thread_handle),
+ TYPE_LENGTH (value_type (thread_handle)),
+ inf);
+}
+
/*
* Thread iterator function.
*
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 1/7] Add target method for converting thread handle to thread_info struct pointer
2017-07-19 0:54 ` [PATCH v3 1/7] Add target method for converting thread handle to thread_info struct pointer Kevin Buettner
@ 2017-07-23 20:39 ` Simon Marchi
0 siblings, 0 replies; 15+ messages in thread
From: Simon Marchi @ 2017-07-23 20:39 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
Hi Kevin,
Just some formatting stuff, otherwise the patch LGTM.
On 2017-07-19 02:54, Kevin Buettner wrote:
> This patch adds a target method named
> `to_thread_handle_to_thread_info'.
> It is intended to map a thread library specific thread handle (such as
> pthread_t for the pthread library) to the corresponding GDB internal
> thread_info struct (pointer).
>
> An implementation is provided for Linux pthreads; see
> linux-thread-db.c.
>
> gdb/ChangeLog:
>
> * target.h (struct target_ops): Add
> to_thread_handle_to_thread_info.
> (target_thread_handle_to_thread_info): Declare.
> * target.c (target_thread_handle_to_thread_info): New function.
> * target-delegates.c: Regenerate.
> * gdbthread.h (find_thread_by_handle): Declare.
> * thread.c (find_thread_by_handle): New function.
> * linux-thread-db.c (thread_db_thread_handle_to_thread_info): New
> function.
> (init_thread_db_ops): Register
> thread_db_thread_handle_to_thread_info.
> ---
> gdb/gdbthread.h | 4 ++++
> gdb/linux-thread-db.c | 31 +++++++++++++++++++++++++++++++
> gdb/target-delegates.c | 37 +++++++++++++++++++++++++++++++++++++
> gdb/target.c | 9 +++++++++
> gdb/target.h | 11 +++++++++++
> gdb/thread.c | 10 ++++++++++
> 6 files changed, 102 insertions(+)
>
> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
> index 046bf95..c16a0d2 100644
> --- a/gdb/gdbthread.h
> +++ b/gdb/gdbthread.h
> @@ -448,6 +448,10 @@ extern struct thread_info *find_thread_ptid
> (ptid_t ptid);
> /* Find thread by GDB global thread ID. */
> struct thread_info *find_thread_global_id (int global_id);
>
> +/* Find thread by thread library specific handle in inferior INF. */
> +struct thread_info *find_thread_by_handle (struct value
> *thread_handle,
> + struct inferior *inf);
> +
> /* Finds the first thread of the inferior given by PID. If PID is -1,
> returns the first thread in the list. */
> struct thread_info *first_thread_of_process (int pid);
> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
> index 86254f8..14dc5d2 100644
> --- a/gdb/linux-thread-db.c
> +++ b/gdb/linux-thread-db.c
> @@ -1410,6 +1410,36 @@ thread_db_extra_thread_info (struct target_ops
> *self,
> return NULL;
> }
>
> +/* Return pointer to the thread_info struct which corresponds to
> + THREAD_HANDLE (having length HANDLE_LEN). */
Please insert a newline between the comment and function (I know there
are tons of bad examples).
> +static struct thread_info *
> +thread_db_thread_handle_to_thread_info (struct target_ops *ops,
> + const gdb_byte *thread_handle,
> + int handle_len,
> + struct inferior *inf)
> +{
> + struct thread_info *tp;
> + thread_t handle_tid;
> +
> + /* Thread handle sizes must match in order to proceed. We don't use
> an
> + assert here because the resulting internal error will cause GDB
> to
> + exit. This isn't necessarily an internal error due to the
> possibility
> + of garbage being passed as the thread handle via the python
> interface. */
> + if (handle_len != sizeof (handle_tid))
> + error (_("Thread handle size mismatch: %d vs %d (from
> libthread_db)"),
> + handle_len, (int) sizeof (handle_tid));
You could use %zu for printing the sizeof. I just learned that the z
length modifier is standard in C++11, so we might as well use it.
> +
> + handle_tid = * (const thread_t *) thread_handle;
> +
> + ALL_NON_EXITED_THREADS (tp)
> + {
> + if (tp->inf == inf && tp->priv != NULL && handle_tid ==
> tp->priv->tid)
> + return tp;
> + }
> +
> + return NULL;
> +}
> +
> /* Get the address of the thread local variable in load module LM
> which
> is stored at OFFSET within the thread local storage for thread
> PTID. */
>
> @@ -1687,6 +1717,7 @@ init_thread_db_ops (void)
> = thread_db_get_thread_local_address;
> thread_db_ops.to_extra_thread_info = thread_db_extra_thread_info;
> thread_db_ops.to_get_ada_task_ptid = thread_db_get_ada_task_ptid;
> + thread_db_ops.to_thread_handle_to_thread_info =
> thread_db_thread_handle_to_thread_info;
> thread_db_ops.to_magic = OPS_MAGIC;
>
> complete_target_initialization (&thread_db_ops);
> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
> index 88e3e0b..dea1d9f 100644
> --- a/gdb/target-delegates.c
> +++ b/gdb/target-delegates.c
> @@ -1583,6 +1583,39 @@ debug_thread_name (struct target_ops *self,
> struct thread_info *arg1)
> return result;
> }
>
> +static struct thread_info *
> +delegate_thread_handle_to_thread_info (struct target_ops *self, const
> gdb_byte *arg1, int arg2, struct inferior *arg3)
> +{
> + self = self->beneath;
> + return self->to_thread_handle_to_thread_info (self, arg1, arg2,
> arg3);
> +}
> +
> +static struct thread_info *
> +tdefault_thread_handle_to_thread_info (struct target_ops *self, const
> gdb_byte *arg1, int arg2, struct inferior *arg3)
> +{
> + return NULL;
> +}
> +
> +static struct thread_info *
> +debug_thread_handle_to_thread_info (struct target_ops *self, const
> gdb_byte *arg1, int arg2, struct inferior *arg3)
> +{
> + struct thread_info * result;
> + fprintf_unfiltered (gdb_stdlog, "->
> %s->to_thread_handle_to_thread_info (...)\n",
> debug_target.to_shortname);
> + result = debug_target.to_thread_handle_to_thread_info
> (&debug_target, arg1, arg2, arg3);
> + fprintf_unfiltered (gdb_stdlog, "<-
> %s->to_thread_handle_to_thread_info (", debug_target.to_shortname);
> + target_debug_print_struct_target_ops_p (&debug_target);
> + fputs_unfiltered (", ", gdb_stdlog);
> + target_debug_print_const_gdb_byte_p (arg1);
> + fputs_unfiltered (", ", gdb_stdlog);
> + target_debug_print_int (arg2);
> + fputs_unfiltered (", ", gdb_stdlog);
> + target_debug_print_struct_inferior_p (arg3);
> + fputs_unfiltered (") = ", gdb_stdlog);
> + target_debug_print_struct_thread_info_p (result);
> + fputs_unfiltered ("\n", gdb_stdlog);
> + return result;
> +}
> +
> static void
> delegate_stop (struct target_ops *self, ptid_t arg1)
> {
> @@ -4267,6 +4300,8 @@ install_delegators (struct target_ops *ops)
> ops->to_extra_thread_info = delegate_extra_thread_info;
> if (ops->to_thread_name == NULL)
> ops->to_thread_name = delegate_thread_name;
> + if (ops->to_thread_handle_to_thread_info == NULL)
> + ops->to_thread_handle_to_thread_info =
> delegate_thread_handle_to_thread_info;
> if (ops->to_stop == NULL)
> ops->to_stop = delegate_stop;
> if (ops->to_interrupt == NULL)
> @@ -4522,6 +4557,7 @@ install_dummy_methods (struct target_ops *ops)
> ops->to_pid_to_str = default_pid_to_str;
> ops->to_extra_thread_info = tdefault_extra_thread_info;
> ops->to_thread_name = tdefault_thread_name;
> + ops->to_thread_handle_to_thread_info =
> tdefault_thread_handle_to_thread_info;
> ops->to_stop = tdefault_stop;
> ops->to_interrupt = tdefault_interrupt;
> ops->to_pass_ctrlc = default_target_pass_ctrlc;
> @@ -4681,6 +4717,7 @@ init_debug_target (struct target_ops *ops)
> ops->to_pid_to_str = debug_pid_to_str;
> ops->to_extra_thread_info = debug_extra_thread_info;
> ops->to_thread_name = debug_thread_name;
> + ops->to_thread_handle_to_thread_info =
> debug_thread_handle_to_thread_info;
> ops->to_stop = debug_stop;
> ops->to_interrupt = debug_interrupt;
> ops->to_pass_ctrlc = debug_pass_ctrlc;
> diff --git a/gdb/target.c b/gdb/target.c
> index e526bcc..23f4b9f 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -2315,6 +2315,15 @@ target_thread_name (struct thread_info *info)
> return current_target.to_thread_name (¤t_target, info);
> }
>
> +struct thread_info *
> +target_thread_handle_to_thread_info (const gdb_byte * thread_handle,
No space after *.
> + int handle_len,
> + struct inferior *inf)
> +{
> + return current_target.to_thread_handle_to_thread_info
> + (¤t_target, thread_handle, handle_len, inf);
> +}
> +
> void
> target_resume (ptid_t ptid, int step, enum gdb_signal signal)
> {
> diff --git a/gdb/target.h b/gdb/target.h
> index c0155c1..1bb8c0f 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -646,6 +646,11 @@ struct target_ops
> TARGET_DEFAULT_RETURN (NULL);
> const char *(*to_thread_name) (struct target_ops *, struct
> thread_info *)
> TARGET_DEFAULT_RETURN (NULL);
> + struct thread_info *(*to_thread_handle_to_thread_info) (struct
> target_ops *,
> + const
> gdb_byte *,
> + int,
> + struct inferior *inf)
> + TARGET_DEFAULT_RETURN (NULL);
> void (*to_stop) (struct target_ops *, ptid_t)
> TARGET_DEFAULT_IGNORE ();
> void (*to_interrupt) (struct target_ops *, ptid_t)
> @@ -1857,6 +1862,12 @@ extern const char *normal_pid_to_str (ptid_t
> ptid);
>
> extern const char *target_thread_name (struct thread_info *);
>
> +/* Given a pointer to a thread library specific thread handle and
> + its length, return a pointer to the corresponding thread_info
> struct. */
> +
> +extern struct thread_info *target_thread_handle_to_thread_info
> + (const gdb_byte * thread_handle, int handle_len, struct inferior *);
No space after *. Could you name the last parameter as well?
> +
> /* Attempts to find the pathname of the executable file
> that was run to create a specified process.
>
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 3cf1b94..456b1ad 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -534,6 +534,16 @@ find_thread_ptid (ptid_t ptid)
> return NULL;
> }
>
> +/* Find a thread_info by matching thread libary specific handle. */
That comment should only refer to the header, which already contains a
comment for that function.
/* See gdbthread.h. */
Also, newline after the comment.
> +struct thread_info *
> +find_thread_by_handle (struct value *thread_handle, struct inferior
> *inf)
> +{
> + return target_thread_handle_to_thread_info
> + (value_contents_all (thread_handle),
> + TYPE_LENGTH (value_type (thread_handle)),
> + inf);
Those last two lines should be intended with one more space (aligned
with value_contents_all).
> +}
> +
> /*
> * Thread iterator function.
> *
Thanks!
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 4/7] Test case for Inferior.thread_from_thread_handle
2017-07-19 0:42 [PATCH v3 0/7] Thread handle to thread info mapping Kevin Buettner
2017-07-19 0:54 ` [PATCH v3 2/7] Add `thread_from_thread_handle' method to (Python) gdb.Inferior Kevin Buettner
2017-07-19 0:54 ` [PATCH v3 1/7] Add target method for converting thread handle to thread_info struct pointer Kevin Buettner
@ 2017-07-19 0:55 ` Kevin Buettner
2017-07-23 21:17 ` Simon Marchi
2017-07-19 0:55 ` [PATCH v3 3/7] Documentation " Kevin Buettner
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Kevin Buettner @ 2017-07-19 0:55 UTC (permalink / raw)
To: gdb-patches
As the title says, this is a test case for
Inferior.thread_from_thread_handle, a python method which will,
given a thread library dependent thread handle, find the GDB thread
which corresponds to that thread handle (in the inferior under
consideration).
The C file for this test case causes the thread handles for the
main thread and two child threads to be placed into an array. The
test case runs to one of the functions (do_something()) at which point,
it retrieves the thread handles from the array and attempts to find the
correponding thread in GDB's internal thread list.
I use barriers to make sure that both threads have actually started;
execution will stop when one of the threads breaks at do_something.
Thanks to Simon Marchi for suggestions for forcing the thread
numbering to be stable.
gdb/testsuite/ChangeLog:
* gdb.python/py-thrhandle.c, gdb.python/py-thrhandle.exp: New
files.
---
gdb/testsuite/gdb.python/py-thrhandle.c | 97 +++++++++++++++++++++++++++++++
gdb/testsuite/gdb.python/py-thrhandle.exp | 72 +++++++++++++++++++++++
2 files changed, 169 insertions(+)
diff --git a/gdb/testsuite/gdb.python/py-thrhandle.c b/gdb/testsuite/gdb.python/py-thrhandle.c
new file mode 100644
index 0000000..10e0222
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-thrhandle.c
@@ -0,0 +1,97 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2017 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <pthread.h>
+#include <unistd.h>
+#include <memory.h>
+
+#define NTHR 3
+#define NBOGUSTHR 2
+
+int thr_data[NTHR];
+
+/* Thread handles for each thread plus some "bogus" threads. */
+pthread_t thrs[NTHR + NBOGUSTHR];
+
+/* The thread children will meet at this barrier. */
+pthread_barrier_t c_barrier;
+
+/* The main thread and child thread will meet at this barrier. */
+pthread_barrier_t mc_barrier;
+
+void
+do_something (int n)
+{
+}
+
+void *
+do_work (void *data)
+{
+ int num = * (int *) data;
+
+ /* As the child threads are created, they'll meet the main thread
+ at this barrier. We do this to ensure that threads end up in
+ GDB's thread list in the order in which they were created. Having
+ this ordering makes it easier to write the test. */
+ pthread_barrier_wait (&mc_barrier);
+
+ /* All of the child threads will meet at this barrier before proceeding.
+ This ensures that all threads will be active (not exited) and in
+ roughly the same state when the first one hits the breakpoint in
+ do_something(). */
+ pthread_barrier_wait (&c_barrier);
+
+ do_something (num);
+
+ pthread_exit (NULL);
+}
+
+void
+after_mc_barrier (void)
+{
+}
+
+int
+main (int argc, char **argv)
+{
+ int i;
+
+ pthread_barrier_init (&c_barrier, NULL, NTHR - 1);
+
+ thrs[0] = pthread_self ();
+ thr_data[0] = 1;
+
+ /* Create two bogus thread handles. */
+ memset (&thrs[NTHR], 0, sizeof (pthread_t));
+ memset (&thrs[NTHR + 1], 0xaa, sizeof (pthread_t));
+
+ for (i = 1; i < NTHR; i++)
+ {
+ thr_data[i] = i + 1;
+
+ pthread_barrier_init (&mc_barrier, NULL, 2);
+
+ pthread_create (&thrs[i], NULL, do_work, &thr_data[i]);
+
+ pthread_barrier_wait (&mc_barrier);
+ pthread_barrier_destroy (&mc_barrier);
+ after_mc_barrier ();
+ }
+
+ for (i = 1; i < NTHR; i++)
+ pthread_join (thrs[i], NULL);
+}
diff --git a/gdb/testsuite/gdb.python/py-thrhandle.exp b/gdb/testsuite/gdb.python/py-thrhandle.exp
new file mode 100644
index 0000000..033737d
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-thrhandle.exp
@@ -0,0 +1,72 @@
+# Copyright (C) 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file verifies that gdb.thread_from_thread_handle works as expected.
+
+standard_testfile
+
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable debug] != "" } {
+ return -1
+}
+
+clean_restart ${binfile}
+runto_main
+
+gdb_test "break after_mc_barrier" \
+ "Breakpoint 2 at .*: file .*${srcfile}, line .*" \
+ "breakpoint on after_mc_barrier"
+
+gdb_test "break do_something" \
+ "Breakpoint 3 at .*: file .*${srcfile}, line .*" \
+ "breakpoint on do_something"
+
+gdb_test "continue" \
+ "Breakpoint 2, after_mc_barrier .*" \
+ "run to after_mc_barrier (1)"
+
+gdb_test_no_output "del 2" "delete after_mc_barrier breakpoint"
+
+gdb_test "continue" \
+ "Breakpoint 3, do_something .*" \
+ "run to do_something"
+
+# The current thread, indicated by '*' in the "info threads" output
+# should be stopped in do_something() with a value of n which is the
+# same as the number reported in the "Id" column. If it's not, then
+# something went wrong with the start up sequence which should cause
+# the main thread to be thread 1, the first child thread to be thread
+# 2, and the second child thread to be thread 3.
+
+gdb_test "info threads" \
+ {.*[\r\n]+\* +([0-9]+) +Thread[^\r\n]* do_something \(n=\1\) at.*}
+
+gdb_test "python print gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[0\]')).num" \
+ "1"
+
+gdb_test "python print gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[1\]')).num" \
+ "2"
+
+gdb_test "python print gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[2\]')).num" \
+ "3"
+
+gdb_test "python print gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[3\]'))" \
+ "None"
+
+gdb_test "python print gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[4\]'))" \
+ "None"
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 4/7] Test case for Inferior.thread_from_thread_handle
2017-07-19 0:55 ` [PATCH v3 4/7] Test case for Inferior.thread_from_thread_handle Kevin Buettner
@ 2017-07-23 21:17 ` Simon Marchi
0 siblings, 0 replies; 15+ messages in thread
From: Simon Marchi @ 2017-07-23 21:17 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
Hi Kevin,
On 2017-07-19 02:55, Kevin Buettner wrote:
> As the title says, this is a test case for
> Inferior.thread_from_thread_handle, a python method which will,
> given a thread library dependent thread handle, find the GDB thread
> which corresponds to that thread handle (in the inferior under
> consideration).
>
> The C file for this test case causes the thread handles for the
> main thread and two child threads to be placed into an array. The
> test case runs to one of the functions (do_something()) at which point,
> it retrieves the thread handles from the array and attempts to find the
> correponding thread in GDB's internal thread list.
"correponding"
>
> I use barriers to make sure that both threads have actually started;
> execution will stop when one of the threads breaks at do_something.
>
> Thanks to Simon Marchi for suggestions for forcing the thread
> numbering to be stable.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.python/py-thrhandle.c, gdb.python/py-thrhandle.exp: New
> files.
> ---
> gdb/testsuite/gdb.python/py-thrhandle.c | 97
> +++++++++++++++++++++++++++++++
> gdb/testsuite/gdb.python/py-thrhandle.exp | 72 +++++++++++++++++++++++
> 2 files changed, 169 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.python/py-thrhandle.c
> b/gdb/testsuite/gdb.python/py-thrhandle.c
> new file mode 100644
> index 0000000..10e0222
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-thrhandle.c
> @@ -0,0 +1,97 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2017 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or
> modify
> + it under the terms of the GNU General Public License as published
> by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see
> <http://www.gnu.org/licenses/>. */
> +
> +#include <pthread.h>
> +#include <unistd.h>
> +#include <memory.h>
> +
> +#define NTHR 3
> +#define NBOGUSTHR 2
> +
> +int thr_data[NTHR];
> +
> +/* Thread handles for each thread plus some "bogus" threads. */
> +pthread_t thrs[NTHR + NBOGUSTHR];
> +
> +/* The thread children will meet at this barrier. */
> +pthread_barrier_t c_barrier;
> +
> +/* The main thread and child thread will meet at this barrier. */
> +pthread_barrier_t mc_barrier;
> +
> +void
> +do_something (int n)
> +{
> +}
> +
> +void *
> +do_work (void *data)
> +{
> + int num = * (int *) data;
> +
> + /* As the child threads are created, they'll meet the main thread
> + at this barrier. We do this to ensure that threads end up in
> + GDB's thread list in the order in which they were created.
> Having
> + this ordering makes it easier to write the test. */
> + pthread_barrier_wait (&mc_barrier);
> +
> + /* All of the child threads will meet at this barrier before
> proceeding.
> + This ensures that all threads will be active (not exited) and in
> + roughly the same state when the first one hits the breakpoint in
> + do_something(). */
> + pthread_barrier_wait (&c_barrier);
> +
> + do_something (num);
> +
> + pthread_exit (NULL);
> +}
> +
> +void
> +after_mc_barrier (void)
> +{
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> + int i;
> +
> + pthread_barrier_init (&c_barrier, NULL, NTHR - 1);
> +
> + thrs[0] = pthread_self ();
> + thr_data[0] = 1;
> +
> + /* Create two bogus thread handles. */
> + memset (&thrs[NTHR], 0, sizeof (pthread_t));
> + memset (&thrs[NTHR + 1], 0xaa, sizeof (pthread_t));
> +
> + for (i = 1; i < NTHR; i++)
> + {
> + thr_data[i] = i + 1;
> +
> + pthread_barrier_init (&mc_barrier, NULL, 2);
I don't think it's wrong to init/destroy the barrier repeatedly, but
it's not necessary. After the barrier has "opened", it is reset with
the count it was initialized with. So you can just call _wait again on
it.
> +
> + pthread_create (&thrs[i], NULL, do_work, &thr_data[i]);
> +
> + pthread_barrier_wait (&mc_barrier);
> + pthread_barrier_destroy (&mc_barrier);
> + after_mc_barrier ();
> + }
> +
> + for (i = 1; i < NTHR; i++)
> + pthread_join (thrs[i], NULL);
> +}
> diff --git a/gdb/testsuite/gdb.python/py-thrhandle.exp
> b/gdb/testsuite/gdb.python/py-thrhandle.exp
> new file mode 100644
> index 0000000..033737d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-thrhandle.exp
> @@ -0,0 +1,72 @@
> +# Copyright (C) 2017 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see
> <http://www.gnu.org/licenses/>.
> +
> +# Please email any bugs, comments, and/or additions to this file to:
> +# bug-gdb@gnu.org
> +
> +# This file verifies that gdb.thread_from_thread_handle works as
> expected.
This is now gdb.Inferior.thread_from_thread_handle.
> +
> +standard_testfile
> +
> +
> +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}"
> "${binfile}" executable debug] != "" } {
> + return -1
> +}
> +
> +clean_restart ${binfile}
> +runto_main
> +
> +gdb_test "break after_mc_barrier" \
> + "Breakpoint 2 at .*: file .*${srcfile}, line .*" \
> + "breakpoint on after_mc_barrier"
> +
> +gdb_test "break do_something" \
> + "Breakpoint 3 at .*: file .*${srcfile}, line .*" \
> + "breakpoint on do_something"
> +
> +gdb_test "continue" \
> + "Breakpoint 2, after_mc_barrier .*" \
> + "run to after_mc_barrier (1)"
Do not use parentheses at the end of the test name:
https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages
Alternatively, you can use gdb_continue_to_breakpoint or gdb_continue
(not sure why we have both).
> +
> +gdb_test_no_output "del 2" "delete after_mc_barrier breakpoint"
> +
> +gdb_test "continue" \
> + "Breakpoint 3, do_something .*" \
> + "run to do_something"
> +
> +# The current thread, indicated by '*' in the "info threads" output
> +# should be stopped in do_something() with a value of n which is the
> +# same as the number reported in the "Id" column. If it's not, then
> +# something went wrong with the start up sequence which should cause
> +# the main thread to be thread 1, the first child thread to be thread
> +# 2, and the second child thread to be thread 3.
> +
> +gdb_test "info threads" \
> + {.*[\r\n]+\* +([0-9]+) +Thread[^\r\n]* do_something \(n=\1\) at.*}
I didn't understand the comment when reading it at first, then wondered
for way too long how we could end up with n=1 :).
> +gdb_test "python print
> gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[0\]')).num"
> \
> + "1"
> +
> +gdb_test "python print
> gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[1\]')).num"
> \
> + "2"
> +
> +gdb_test "python print
> gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[2\]')).num"
> \
> + "3"
> +
> +gdb_test "python print
> gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[3\]'))"
> \
> + "None"
> +
> +gdb_test "python print
> gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[4\]'))"
> \
> + "None"
Please use parentheses with print, so it works with Python 3. But then,
your test name will have tail parentheses, so it might be a good idea to
give them explicit names.
Thanks,
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/7] Documentation for Inferior.thread_from_thread_handle
2017-07-19 0:42 [PATCH v3 0/7] Thread handle to thread info mapping Kevin Buettner
` (2 preceding siblings ...)
2017-07-19 0:55 ` [PATCH v3 4/7] Test case for Inferior.thread_from_thread_handle Kevin Buettner
@ 2017-07-19 0:55 ` Kevin Buettner
2017-07-19 2:33 ` Eli Zaretskii
2017-07-19 0:55 ` [PATCH v3 5/7] Add thread_db_notice_clone to gdbserver Kevin Buettner
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Kevin Buettner @ 2017-07-19 0:55 UTC (permalink / raw)
To: gdb-patches
gdb/doc/ChangeLog:
* python.texi (Inferiors In Python): Add description for method
Inferior.thread_from_thread_handle.
---
gdb/doc/python.texi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 32d7939..f60d366 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2785,6 +2785,13 @@ containing the address where the pattern was found, or @code{None} if
the pattern could not be found.
@end defun
+@findex Inferior.thread_from_thread_handle
+@defun Inferior.thread_from_thread_handle (thread_handle)
+Return the thread object corresponding to @var{thread_handle}, a thread
+library specific data structure such as @code{pthread_t} for pthreads
+library implementations.
+@end defun
+
@node Events In Python
@subsubsection Events In Python
@cindex inferior events in Python
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 5/7] Add thread_db_notice_clone to gdbserver
2017-07-19 0:42 [PATCH v3 0/7] Thread handle to thread info mapping Kevin Buettner
` (3 preceding siblings ...)
2017-07-19 0:55 ` [PATCH v3 3/7] Documentation " Kevin Buettner
@ 2017-07-19 0:55 ` Kevin Buettner
2017-07-23 21:27 ` Simon Marchi
2017-07-19 0:56 ` [PATCH v3 7/7] Documentation for qXfer:threads:read handle attribute Kevin Buettner
2017-07-19 0:56 ` [PATCH v3 6/7] Add thread_handle_to_thread_info support for remote targets Kevin Buettner
6 siblings, 1 reply; 15+ messages in thread
From: Kevin Buettner @ 2017-07-19 0:55 UTC (permalink / raw)
To: gdb-patches
While working on a patch for fetching a thread handle in gdbserver, I
ran into a circumstance in which tests in gdb.mi/mi-nsmoribund.exp
would occasionally fail. Over a large enough number of runs, it would
fail roughly 2% of the time.
That thread handle patch caused find_one_thread() to be called on
every stop. find_one_thread() calls td_ta_map_lwp2thr() which, in
turn, can cause ps_get_thread_area() to be called.
ps_get_thread_area() makes a call to ptrace() for getting the thread
area address. If this should happen when the thread is not stopped,
the call to ptrace will return error which in turn propogates back to
find_one_thread(). find_one_thread() calls error() in this instance
which causes the program to die.
This patch causes find_one_thread() to be called upon reciept of a
clone event. Since the clone is stopped, the circumstances described
above cannot occur.
gdb/gdbserver/ChangeLog:
* linux-low.c (handle_extended_wait): Call thread_db_notice_clone().
* linux-low.h (thread_db_notice_clone): Declare.
* thread-db.c (thread_db_notice_clone): New function.
---
gdb/gdbserver/linux-low.c | 2 ++
gdb/gdbserver/linux-low.h | 7 +++++++
gdb/gdbserver/thread-db.c | 14 ++++++++++++++
3 files changed, 23 insertions(+)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 3d7cfe3..9d831e7 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -656,6 +656,8 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat)
new_lwp->status_pending = status;
}
+ thread_db_notice_clone (get_thread_process (event_thr), ptid);
+
/* Don't report the event. */
return 1;
}
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 6328da0..86cfe51 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -410,4 +410,11 @@ 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);
+/* Called from linux-low.c when a clone event is detected. Upon entry,
+ both the clone and the parent should be stopped. This function does
+ 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);
+
extern int have_ptrace_getregset;
diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
index 1ffb79d..eff1914 100644
--- a/gdb/gdbserver/thread-db.c
+++ b/gdb/gdbserver/thread-db.c
@@ -864,3 +864,17 @@ thread_db_handle_monitor_command (char *mon)
/* Tell server.c to perform default processing. */
return 0;
}
+
+void
+thread_db_notice_clone (struct process_info *proc, ptid_t ptid)
+{
+ struct thread_db *thread_db = proc->priv->thread_db;
+
+ /* If the thread layer isn't initialized, return. It may just
+ be that the program uses clone, but does not use libthread_db. */
+ if (thread_db == NULL || !thread_db->all_symbols_looked_up)
+ return;
+
+ if (!find_one_thread (ptid))
+ warning ("Cannot find thread after clone.\n");
+}
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 5/7] Add thread_db_notice_clone to gdbserver
2017-07-19 0:55 ` [PATCH v3 5/7] Add thread_db_notice_clone to gdbserver Kevin Buettner
@ 2017-07-23 21:27 ` Simon Marchi
0 siblings, 0 replies; 15+ messages in thread
From: Simon Marchi @ 2017-07-23 21:27 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
Hi Kevin,
On 2017-07-19 02:55, Kevin Buettner wrote:
> While working on a patch for fetching a thread handle in gdbserver, I
> ran into a circumstance in which tests in gdb.mi/mi-nsmoribund.exp
> would occasionally fail. Over a large enough number of runs, it would
> fail roughly 2% of the time.
>
> That thread handle patch caused find_one_thread() to be called on
> every stop. find_one_thread() calls td_ta_map_lwp2thr() which, in
> turn, can cause ps_get_thread_area() to be called.
> ps_get_thread_area() makes a call to ptrace() for getting the thread
> area address. If this should happen when the thread is not stopped,
> the call to ptrace will return error which in turn propogates back to
> find_one_thread(). find_one_thread() calls error() in this instance
> which causes the program to die.
>
> This patch causes find_one_thread() to be called upon reciept of a
> clone event. Since the clone is stopped, the circumstances described
> above cannot occur.
>
> gdb/gdbserver/ChangeLog:
>
> * linux-low.c (handle_extended_wait): Call
> thread_db_notice_clone().
> * linux-low.h (thread_db_notice_clone): Declare.
> * thread-db.c (thread_db_notice_clone): New function.
> ---
> gdb/gdbserver/linux-low.c | 2 ++
> gdb/gdbserver/linux-low.h | 7 +++++++
> gdb/gdbserver/thread-db.c | 14 ++++++++++++++
> 3 files changed, 23 insertions(+)
>
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 3d7cfe3..9d831e7 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -656,6 +656,8 @@ handle_extended_wait (struct lwp_info
> **orig_event_lwp, int wstat)
> new_lwp->status_pending = status;
> }
>
> + thread_db_notice_clone (get_thread_process (event_thr), ptid);
> +
> /* Don't report the event. */
> return 1;
> }
> diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
> index 6328da0..86cfe51 100644
> --- a/gdb/gdbserver/linux-low.h
> +++ b/gdb/gdbserver/linux-low.h
> @@ -410,4 +410,11 @@ 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);
>
> +/* Called from linux-low.c when a clone event is detected. Upon
> entry,
> + both the clone and the parent should be stopped. This function
> does
> + whatever is required have the clone under thread_db's control. */
New line here.
> +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);
This last declaration doesn't belong in this patch I think.
> extern int have_ptrace_getregset;
> diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
> index 1ffb79d..eff1914 100644
> --- a/gdb/gdbserver/thread-db.c
> +++ b/gdb/gdbserver/thread-db.c
> @@ -864,3 +864,17 @@ thread_db_handle_monitor_command (char *mon)
> /* Tell server.c to perform default processing. */
> return 0;
> }
> +
Just add a
/* See linux-low.h. */
to be fully GDB-style-compliant :)
> +void
> +thread_db_notice_clone (struct process_info *proc, ptid_t ptid)
> +{
> + struct thread_db *thread_db = proc->priv->thread_db;
> +
> + /* If the thread layer isn't initialized, return. It may just
> + be that the program uses clone, but does not use libthread_db.
> */
> + if (thread_db == NULL || !thread_db->all_symbols_looked_up)
> + return;
> +
> + if (!find_one_thread (ptid))
> + warning ("Cannot find thread after clone.\n");
> +}
Again, this patch LGTM, but if somebody else could look at it I'd
appreciate it.
Thanks!
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 7/7] Documentation for qXfer:threads:read handle attribute
2017-07-19 0:42 [PATCH v3 0/7] Thread handle to thread info mapping Kevin Buettner
` (4 preceding siblings ...)
2017-07-19 0:55 ` [PATCH v3 5/7] Add thread_db_notice_clone to gdbserver Kevin Buettner
@ 2017-07-19 0:56 ` Kevin Buettner
2017-07-19 2:34 ` Eli Zaretskii
2017-07-19 0:56 ` [PATCH v3 6/7] Add thread_handle_to_thread_info support for remote targets Kevin Buettner
6 siblings, 1 reply; 15+ messages in thread
From: Kevin Buettner @ 2017-07-19 0:56 UTC (permalink / raw)
To: gdb-patches
gdb/doc/ChangeLog:
* gdb.texinfo (qXfer:threads:read): Add documentation for handle
attribute.
---
gdb/doc/gdb.texinfo | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c167a86..4024a40 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -40433,7 +40433,9 @@ 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.
+auxiliary information. The @samp{handle} attribute, if present,
+is a hex encoded representation of the thread handle.
+
@node Traceframe Info Format
@section Traceframe Info Format
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 6/7] Add thread_handle_to_thread_info support for remote targets
2017-07-19 0:42 [PATCH v3 0/7] Thread handle to thread info mapping Kevin Buettner
` (5 preceding siblings ...)
2017-07-19 0:56 ` [PATCH v3 7/7] Documentation for qXfer:threads:read handle attribute Kevin Buettner
@ 2017-07-19 0:56 ` Kevin Buettner
2017-07-23 21:47 ` Simon Marchi
6 siblings, 1 reply; 15+ messages in thread
From: Kevin Buettner @ 2017-07-19 0:56 UTC (permalink / raw)
To: gdb-patches
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
};
#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;
+ /* 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);
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)
+
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;
}
+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;
+
/* 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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 6/7] Add thread_handle_to_thread_info support for remote targets
2017-07-19 0:56 ` [PATCH v3 6/7] Add thread_handle_to_thread_info support for remote targets Kevin Buettner
@ 2017-07-23 21:47 ` Simon Marchi
0 siblings, 0 replies; 15+ messages in thread
From: Simon Marchi @ 2017-07-23 21:47 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
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
^ permalink raw reply [flat|nested] 15+ messages in thread