public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v4 0/7] Thread handle to thread info mapping
@ 2017-08-16 16:25 Kevin Buettner
  2017-08-16 17:35 ` [PATCH v4 1/7] Add target method for converting thread handle to thread_info struct pointer Kevin Buettner
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Kevin Buettner @ 2017-08-16 16:25 UTC (permalink / raw)
  To: gdb-patches

This patch set introduces support for mapping thread handles to the
thread_info structs which GDB uses to keep track of threads in the
inferiors which it's debugging.  I need this in order to find the GDB
thread which corresponds to a saved thread handle (e.g.  pthread_t)
within an implementation of a thread library built atop pthreads.  The
mechanism is general enough however to support other thread handle
representations as well.

Simon reviewed the non-documentation parts of the v3 version of this
patch set.  I think I have addressed all of his concerns from that
review.

Eli approved the two documentation patches.  There have been no
changes from v3 to v4, but I'm including them here for completeness.

Part 1 introduces a target method which maps a thread handle to
the corresponding internal GDB thread object, i.e. something of type
`struct thread_info *'.  An implementation of this new method is
provided for the Linux thread target.

Part 2 adds a python interface for the mechanism introduced in part 1.

Part 3 is a documentation patch.  As noted above, Eli has already
approved this patch which is unchanged from the v3 patch set.

Part 4 adds a test case.  I've incorporated all of Simon's suggestions
from his review of the v3 patch.  I've also added three new tests
which check the behavior of thread_from_thread_handle when passing
a bad thread handle.  After making this change, I found a problem
in part 6.

Part 5 is a bug fix for a problem discovered while working on part 6.

Part 6 adds support for remote targets.  In addition to making several
v3 changes requested by Simon, I adjusted remote_thread_handle_to_thread_info
so that would throw an exception (via a call to error()) when the handle
sizes are mismatched.

Part 7 is a documentation patch for the remote protocol changes that
were implemented in part 6.  This patch is unchanged from the v3 version
and has already been approved by Eli.

Kevin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/7] Add target method for converting thread handle to thread_info struct pointer
  2017-08-16 16:25 [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
@ 2017-08-16 17:35 ` Kevin Buettner
  2017-08-16 17:37 ` [PATCH v4 2/7] Add `thread_from_thread_handle' method to (Python) gdb.Inferior Kevin Buettner
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Buettner @ 2017-08-16 17:35 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  | 32 ++++++++++++++++++++++++++++++++
 gdb/target-delegates.c | 37 +++++++++++++++++++++++++++++++++++++
 gdb/target.c           |  9 +++++++++
 gdb/target.h           | 11 +++++++++++
 gdb/thread.c           | 12 ++++++++++++
 6 files changed, 105 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 cf68013..3a5414b 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1410,6 +1410,37 @@ 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 %zu (from libthread_db)"),
+	   handle_len, 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.  */
 
@@ -1691,6 +1722,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 9b63ab7..c6ca00e 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2308,6 +2308,15 @@ target_thread_name (struct thread_info *info)
   return current_target.to_thread_name (&current_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
+           (&current_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..2396719 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 *inf);
+
 /* 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..3ecb555 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -522,6 +522,7 @@ find_thread_id (struct inferior *inf, int thr_num)
 }
 
 /* Find a thread_info by matching PTID.  */
+
 struct thread_info *
 find_thread_ptid (ptid_t ptid)
 {
@@ -534,6 +535,17 @@ find_thread_ptid (ptid_t ptid)
   return NULL;
 }
 
+/* See gdbthread.h.  */
+
+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] 13+ messages in thread

* Re: [PATCH v4 2/7] Add `thread_from_thread_handle' method to (Python) gdb.Inferior
  2017-08-16 16:25 [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
  2017-08-16 17:35 ` [PATCH v4 1/7] Add target method for converting thread handle to thread_info struct pointer Kevin Buettner
@ 2017-08-16 17:37 ` Kevin Buettner
  2017-08-16 17:38 ` [PATCH v4 3/7] Documentation for Inferior.thread_from_thread_handle Kevin Buettner
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Buettner @ 2017-08-16 17:37 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     | 54 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h |  2 ++
 2 files changed, 56 insertions(+)

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index f6a24a0..67a7e6e 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -751,6 +751,56 @@ infpy_is_valid (PyObject *self, PyObject *args)
   Py_RETURN_TRUE;
 }
 
+/* Implementation of gdb.Inferior.thread_from_thread_handle (self, handle)
+                        ->  gdb.InferiorThread.  */
+
+PyObject *
+infpy_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))
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       _("Argument 'handle_obj' must be a thread handle object."));
+
+      return NULL;
+    }
+  else
+    {
+      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 +911,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) infpy_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] 13+ messages in thread

* Re: [PATCH v4 3/7] Documentation for Inferior.thread_from_thread_handle
  2017-08-16 16:25 [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
  2017-08-16 17:35 ` [PATCH v4 1/7] Add target method for converting thread handle to thread_info struct pointer Kevin Buettner
  2017-08-16 17:37 ` [PATCH v4 2/7] Add `thread_from_thread_handle' method to (Python) gdb.Inferior Kevin Buettner
@ 2017-08-16 17:38 ` Kevin Buettner
  2017-08-16 17:40 ` [PATCH v4 4/7] Test case " Kevin Buettner
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Buettner @ 2017-08-16 17:38 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] 13+ messages in thread

* Re: [PATCH v4 4/7] Test case for Inferior.thread_from_thread_handle
  2017-08-16 16:25 [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
                   ` (2 preceding siblings ...)
  2017-08-16 17:38 ` [PATCH v4 3/7] Documentation for Inferior.thread_from_thread_handle Kevin Buettner
@ 2017-08-16 17:40 ` Kevin Buettner
  2017-08-16 17:41 ` [PATCH v4 5/7] Add thread_db_notice_clone to gdbserver Kevin Buettner
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Buettner @ 2017-08-16 17:40 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
corresponding 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   |  94 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-thrhandle.exp | 102 ++++++++++++++++++++++++++++++
 2 files changed, 196 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..0dd974f
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-thrhandle.c
@@ -0,0 +1,94 @@
+/* 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);
+  pthread_barrier_init (&mc_barrier, NULL, 2);
+
+  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_create (&thrs[i], NULL, do_work, &thr_data[i]);
+      pthread_barrier_wait (&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..66b0472
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-thrhandle.exp
@@ -0,0 +1,102 @@
+# 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.Inferior.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"
+
+gdb_test_no_output "del 2" "delete after_mc_barrier breakpoint"
+
+gdb_test "continue" \
+	"Breakpoint 3, do_something .*" \
+	"run to do_something"
+
+# The test case has been constructed so that 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.
+#
+# Note that \1 in the RE below is a backreference to the thread id
+# reported in the "Id" column.
+
+gdb_test "info threads"  \
+	{.*[\r\n]+\* +([0-9]+) +Thread[^\r\n]* do_something \(n=\1\) at.*}
+
+# Check for expected results when passing a valid thread handle to
+# thread_from_thread_handle().
+
+gdb_test "python print(gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[0\]')).num)" \
+	"1" "print thread id for thrs\[0\]"
+
+gdb_test "python print(gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[1\]')).num)" \
+	"2" "print thread id for thrs\[1\]"
+
+gdb_test "python print(gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[2\]')).num)" \
+	"3" "print thread id for thrs\[2\]"
+
+# Objects which are of the correct size, but which are bogus thread
+# handles should return None.  For the first test (using thrs[3]), we
+# use 0.  For the second (thrs[4]), we use an unlikely bit pattern.
+
+gdb_test "python print(gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[3\]')))" \
+	"None" "print thread for bogus handle thrs\[3\]"
+
+gdb_test "python print(gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs\[4\]')))" \
+	"None" "print thread for bogus handle thrs\[4\]"
+
+# We should see an exception when passing an object of the wrong type.
+
+gdb_test "python print(gdb.selected_inferior().thread_from_thread_handle(gdb.lookup_symbol('main')))" \
+         ".*TypeError: Argument 'handle_obj' must be a thread handle object.*" \
+	 "TypeError when passing a symbol object to thread_from_thread_handle"
+
+# We should see an exception when passing too large of an object.
+
+gdb_test "python print(gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('thrs')))" \
+         ".*Thread handle size mismatch.*" \
+	 "Pass overly large object to thread_from_thread_handle"
+
+# We should see an exception when passing too small of an object.
+
+gdb_test "python print(gdb.selected_inferior().thread_from_thread_handle(gdb.parse_and_eval('\"S\"')))" \
+         ".*Thread handle size mismatch.*" \
+	 "Pass too small of an object to thread_from_thread_handle"

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 5/7] Add thread_db_notice_clone to gdbserver
  2017-08-16 16:25 [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
                   ` (3 preceding siblings ...)
  2017-08-16 17:40 ` [PATCH v4 4/7] Test case " Kevin Buettner
@ 2017-08-16 17:41 ` Kevin Buettner
  2017-09-29 12:24   ` Pedro Alves
  2017-08-16 17:43 ` [PATCH v4 6/7] Add thread_handle_to_thread_info support for remote targets Kevin Buettner
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 13+ messages in thread
From: Kevin Buettner @ 2017-08-16 17:41 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 |  6 ++++++
 gdb/gdbserver/thread-db.c | 16 ++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index fd46d85..2b43f51 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 2bf7e7c..3d5cb66 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -410,4 +410,10 @@ 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);
+
 extern int have_ptrace_getregset;
diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
index 1ffb79d..b4a5188 100644
--- a/gdb/gdbserver/thread-db.c
+++ b/gdb/gdbserver/thread-db.c
@@ -864,3 +864,19 @@ thread_db_handle_monitor_command (char *mon)
   /* Tell server.c to perform default processing.  */
   return 0;
 }
+
+/* See linux-low.h.  */
+
+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] 13+ messages in thread

* Re: [PATCH v4 6/7] Add thread_handle_to_thread_info support for remote targets
  2017-08-16 16:25 [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
                   ` (4 preceding siblings ...)
  2017-08-16 17:41 ` [PATCH v4 5/7] Add thread_db_notice_clone to gdbserver Kevin Buettner
@ 2017-08-16 17:43 ` Kevin Buettner
  2017-08-16 17:44 ` [PATCH v4 7/7] Documentation for qXfer:threads:read handle attribute Kevin Buettner
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Buettner @ 2017-08-16 17:43 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 |  5 ++++
 gdb/gdbserver/server.c    | 10 ++++++++
 gdb/gdbserver/target.h    | 10 ++++++++
 gdb/gdbserver/thread-db.c | 32 +++++++++++++++++++++++++
 gdb/remote.c              | 59 +++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 121 insertions(+)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 2b43f51..b0aea5c 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -7709,6 +7709,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 3d5cb66..f1d461a 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -374,6 +374,9 @@ 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.  */
@@ -416,4 +419,6 @@ 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);
 
+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..dfb9f48 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;
+  bool 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..ed8b830 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) \
+   : 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 b4a5188..1eb8cb8 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,36 @@ thread_db_get_tls_address (struct thread_info *thread, CORE_ADDR offset,
     return err;
 }
 
+/* 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 ff59a0f..4574fc0 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -73,6 +73,7 @@
 #include "record-btrace.h"
 #include <algorithm>
 #include "common/scoped_restore.h"
+#include "common/byte-vector.h"
 
 /* Temp hacks for tracepoint encoding migration.  */
 static char *target_buf;
@@ -452,6 +453,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.  */
+  gdb::byte_vector *thread_handle;
+
   /* Whether the target stopped for a breakpoint/watchpoint.  */
   enum target_stop_reason stop_reason;
 
@@ -483,6 +488,7 @@ free_private_thread_info (struct private_thread_info *info)
 {
   xfree (info->extra);
   xfree (info->name);
+  delete info->thread_handle;
   xfree (info);
 }
 
@@ -1969,6 +1975,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;
@@ -2995,6 +3002,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.  */
+  gdb::byte_vector *thread_handle;
+
 } thread_item_t;
 DEF_VEC_O(thread_item_t);
 
@@ -3022,6 +3033,7 @@ clear_threads_listing_context (void *p)
     {
       xfree (item->extra);
       xfree (item->name);
+      delete item->thread_handle;
     }
 
   VEC_free (thread_item_t, context->items);
@@ -3060,6 +3072,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);
 
@@ -3130,6 +3143,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 gdb::byte_vector
+                             (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);
@@ -3151,6 +3175,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 }
 };
 
@@ -3226,6 +3251,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);
 		}
@@ -3331,6 +3357,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;
 	    }
 	}
     }
@@ -13464,6 +13492,35 @@ 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)
+        {
+	  if (handle_len != priv->thread_handle->size ())
+	    error (_("Thread handle size mismatch: %d vs %zu (from remote)"),
+	           handle_len, priv->thread_handle->size ());
+	  if (memcmp (thread_handle, priv->thread_handle->data (),
+	              handle_len) == 0)
+	    return tp;
+	}
+    }
+
+  return NULL;
+}
+
 static void
 init_remote_ops (void)
 {
@@ -13612,6 +13669,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] 13+ messages in thread

* Re: [PATCH v4 7/7] Documentation for qXfer:threads:read handle attribute
  2017-08-16 16:25 [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
                   ` (5 preceding siblings ...)
  2017-08-16 17:43 ` [PATCH v4 6/7] Add thread_handle_to_thread_info support for remote targets Kevin Buettner
@ 2017-08-16 17:44 ` Kevin Buettner
  2017-09-13  0:00 ` [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Buettner @ 2017-08-16 17:44 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 d098eba..43ce3bf 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -40443,7 +40443,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] 13+ messages in thread

* Re: [PATCH v4 0/7] Thread handle to thread info mapping
  2017-08-16 16:25 [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
                   ` (6 preceding siblings ...)
  2017-08-16 17:44 ` [PATCH v4 7/7] Documentation for qXfer:threads:read handle attribute Kevin Buettner
@ 2017-09-13  0:00 ` Kevin Buettner
  2017-09-18 20:17 ` Simon Marchi
  2017-09-21 18:57 ` Kevin Buettner
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Buettner @ 2017-09-13  0:00 UTC (permalink / raw)
  To: gdb-patches

Ping.

Any (further) comments on this patch set?

Kevin

On Wed, 16 Aug 2017 09:25:42 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> This patch set introduces support for mapping thread handles to the
> thread_info structs which GDB uses to keep track of threads in the
> inferiors which it's debugging.  I need this in order to find the GDB
> thread which corresponds to a saved thread handle (e.g.  pthread_t)
> within an implementation of a thread library built atop pthreads.  The
> mechanism is general enough however to support other thread handle
> representations as well.
> 
> Simon reviewed the non-documentation parts of the v3 version of this
> patch set.  I think I have addressed all of his concerns from that
> review.
> 
> Eli approved the two documentation patches.  There have been no
> changes from v3 to v4, but I'm including them here for completeness.
> 
> Part 1 introduces a target method which maps a thread handle to
> the corresponding internal GDB thread object, i.e. something of type
> `struct thread_info *'.  An implementation of this new method is
> provided for the Linux thread target.
> 
> Part 2 adds a python interface for the mechanism introduced in part 1.
> 
> Part 3 is a documentation patch.  As noted above, Eli has already
> approved this patch which is unchanged from the v3 patch set.
> 
> Part 4 adds a test case.  I've incorporated all of Simon's suggestions
> from his review of the v3 patch.  I've also added three new tests
> which check the behavior of thread_from_thread_handle when passing
> a bad thread handle.  After making this change, I found a problem
> in part 6.
> 
> Part 5 is a bug fix for a problem discovered while working on part 6.
> 
> Part 6 adds support for remote targets.  In addition to making several
> v3 changes requested by Simon, I adjusted remote_thread_handle_to_thread_info
> so that would throw an exception (via a call to error()) when the handle
> sizes are mismatched.
> 
> Part 7 is a documentation patch for the remote protocol changes that
> were implemented in part 6.  This patch is unchanged from the v3 version
> and has already been approved by Eli.
> 
> Kevin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/7] Thread handle to thread info mapping
  2017-08-16 16:25 [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
                   ` (7 preceding siblings ...)
  2017-09-13  0:00 ` [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
@ 2017-09-18 20:17 ` Simon Marchi
  2017-09-21 18:56   ` Kevin Buettner
  2017-09-21 18:57 ` Kevin Buettner
  9 siblings, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2017-09-18 20:17 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

On 2017-08-16 18:25, Kevin Buettner wrote:
> This patch set introduces support for mapping thread handles to the
> thread_info structs which GDB uses to keep track of threads in the
> inferiors which it's debugging.  I need this in order to find the GDB
> thread which corresponds to a saved thread handle (e.g.  pthread_t)
> within an implementation of a thread library built atop pthreads.  The
> mechanism is general enough however to support other thread handle
> representations as well.
> 
> Simon reviewed the non-documentation parts of the v3 version of this
> patch set.  I think I have addressed all of his concerns from that
> review.
> 
> Eli approved the two documentation patches.  There have been no
> changes from v3 to v4, but I'm including them here for completeness.
> 
> Part 1 introduces a target method which maps a thread handle to
> the corresponding internal GDB thread object, i.e. something of type
> `struct thread_info *'.  An implementation of this new method is
> provided for the Linux thread target.
> 
> Part 2 adds a python interface for the mechanism introduced in part 1.
> 
> Part 3 is a documentation patch.  As noted above, Eli has already
> approved this patch which is unchanged from the v3 patch set.
> 
> Part 4 adds a test case.  I've incorporated all of Simon's suggestions
> from his review of the v3 patch.  I've also added three new tests
> which check the behavior of thread_from_thread_handle when passing
> a bad thread handle.  After making this change, I found a problem
> in part 6.
> 
> Part 5 is a bug fix for a problem discovered while working on part 6.
> 
> Part 6 adds support for remote targets.  In addition to making several
> v3 changes requested by Simon, I adjusted 
> remote_thread_handle_to_thread_info
> so that would throw an exception (via a call to error()) when the 
> handle
> sizes are mismatched.
> 
> Part 7 is a documentation patch for the remote protocol changes that
> were implemented in part 6.  This patch is unchanged from the v3 
> version
> and has already been approved by Eli.
> 
> Kevin

My previous comments were mostly nits.  I took a quick look, and it 
LGTM.

Thanks!

Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/7] Thread handle to thread info mapping
  2017-09-18 20:17 ` Simon Marchi
@ 2017-09-21 18:56   ` Kevin Buettner
  0 siblings, 0 replies; 13+ messages in thread
From: Kevin Buettner @ 2017-09-21 18:56 UTC (permalink / raw)
  To: gdb-patches

On Mon, 18 Sep 2017 22:17:38 +0200
Simon Marchi <simon.marchi@polymtl.ca> wrote:

> My previous comments were mostly nits.  I took a quick look, and it 
> LGTM.

Thanks for looking it over again!  Thanks too for all earlier reviews.
They were very much appreciated!

Kevin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/7] Thread handle to thread info mapping
  2017-08-16 16:25 [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
                   ` (8 preceding siblings ...)
  2017-09-18 20:17 ` Simon Marchi
@ 2017-09-21 18:57 ` Kevin Buettner
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Buettner @ 2017-09-21 18:57 UTC (permalink / raw)
  To: gdb-patches

I've pushed these changes.

Kevin

On Wed, 16 Aug 2017 09:25:42 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> This patch set introduces support for mapping thread handles to the
> thread_info structs which GDB uses to keep track of threads in the
> inferiors which it's debugging.  I need this in order to find the GDB
> thread which corresponds to a saved thread handle (e.g.  pthread_t)
> within an implementation of a thread library built atop pthreads.  The
> mechanism is general enough however to support other thread handle
> representations as well.
> 
> Simon reviewed the non-documentation parts of the v3 version of this
> patch set.  I think I have addressed all of his concerns from that
> review.
> 
> Eli approved the two documentation patches.  There have been no
> changes from v3 to v4, but I'm including them here for completeness.
> 
> Part 1 introduces a target method which maps a thread handle to
> the corresponding internal GDB thread object, i.e. something of type
> `struct thread_info *'.  An implementation of this new method is
> provided for the Linux thread target.
> 
> Part 2 adds a python interface for the mechanism introduced in part 1.
> 
> Part 3 is a documentation patch.  As noted above, Eli has already
> approved this patch which is unchanged from the v3 patch set.
> 
> Part 4 adds a test case.  I've incorporated all of Simon's suggestions
> from his review of the v3 patch.  I've also added three new tests
> which check the behavior of thread_from_thread_handle when passing
> a bad thread handle.  After making this change, I found a problem
> in part 6.
> 
> Part 5 is a bug fix for a problem discovered while working on part 6.
> 
> Part 6 adds support for remote targets.  In addition to making several
> v3 changes requested by Simon, I adjusted remote_thread_handle_to_thread_info
> so that would throw an exception (via a call to error()) when the handle
> sizes are mismatched.
> 
> Part 7 is a documentation patch for the remote protocol changes that
> were implemented in part 6.  This patch is unchanged from the v3 version
> and has already been approved by Eli.
> 
> Kevin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 5/7] Add thread_db_notice_clone to gdbserver
  2017-08-16 17:41 ` [PATCH v4 5/7] Add thread_db_notice_clone to gdbserver Kevin Buettner
@ 2017-09-29 12:24   ` Pedro Alves
  0 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2017-09-29 12:24 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches

On 08/16/2017 06:41 PM, 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.
> 

FYI, issues like the described above still happened with the
patch as it was.  While both parent/child are stopped, we
weren't making sure that one of parent/child is the
current thread.  That lead to libthread_db -> proc-service
trying to access memory from a running or non-existing thread.

For me that resulted in occasional
gdb.threads/multi-create-ns-info-thr.exp failures.

The gdbserver buildslaves have been reporting the same
regression too, e.g.:
  https://sourceware.org/ml/gdb-testers/2017-q3/msg05653.html

Should be fixed now, with:
 https://sourceware.org/ml/gdb-patches/2017-09/msg00903.html
 https://sourceware.org/ml/gdb-patches/2017-09/msg00904.html

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-09-29 12:24 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-16 16:25 [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
2017-08-16 17:35 ` [PATCH v4 1/7] Add target method for converting thread handle to thread_info struct pointer Kevin Buettner
2017-08-16 17:37 ` [PATCH v4 2/7] Add `thread_from_thread_handle' method to (Python) gdb.Inferior Kevin Buettner
2017-08-16 17:38 ` [PATCH v4 3/7] Documentation for Inferior.thread_from_thread_handle Kevin Buettner
2017-08-16 17:40 ` [PATCH v4 4/7] Test case " Kevin Buettner
2017-08-16 17:41 ` [PATCH v4 5/7] Add thread_db_notice_clone to gdbserver Kevin Buettner
2017-09-29 12:24   ` Pedro Alves
2017-08-16 17:43 ` [PATCH v4 6/7] Add thread_handle_to_thread_info support for remote targets Kevin Buettner
2017-08-16 17:44 ` [PATCH v4 7/7] Documentation for qXfer:threads:read handle attribute Kevin Buettner
2017-09-13  0:00 ` [PATCH v4 0/7] Thread handle to thread info mapping Kevin Buettner
2017-09-18 20:17 ` Simon Marchi
2017-09-21 18:56   ` Kevin Buettner
2017-09-21 18:57 ` Kevin Buettner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).