public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle
@ 2018-09-16  5:08 Kevin Buettner
  2018-09-16  5:16 ` [PATCH 1/4] Add and implement thread_to_thread_handle Kevin Buettner
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Kevin Buettner @ 2018-09-16  5:08 UTC (permalink / raw)
  To: gdb-patches

This four part series adds a python method named "thread_handle"
which is used to fetch the thread handle from a thread object.

It will eventually be used as part of the implementation of the
"thread parent" operation for the OpenMP work that I've been doing.

Kevin

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

* [PATCH 1/4] Add and implement thread_to_thread_handle
  2018-09-16  5:08 [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
@ 2018-09-16  5:16 ` Kevin Buettner
  2018-09-16  5:17 ` [PATCH 3/4] Tests for gdb.InfThread.thread_handle Kevin Buettner
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Kevin Buettner @ 2018-09-16  5:16 UTC (permalink / raw)
  To: gdb-patches

This patch adds a thread_to_thread_handle method to the target vector.
It also implements this functionality for remote targets and linux
native threads.

gdb/ChangeLog:
    
    	* gdbthread.h (thread_to_thread_handle): Declare.
    	* thread.c (gdbtypes.h): Include.
    	(thread_to_thread_handle): New function.
    
    	* target.h (struct target_ops): Add thread_info_to_thread_handle.
    	(target_thread_info_to_thread_handle): Declare.
    	* target.c (target_thread_info_to_thread_handle): New function.
    	* target-debug.h, target-delegates.c: Regenerate.
    
    	* linux-thread-db.c (class thread_db_target): Add method
    	thread_info_to_thread_handle.
    	(thread_db_target::thread_info_to_thread_handle): Define.
    	* remote.c (class remote_target): Add new method
    	thread_info_to_thread_handle.
    	(remote_target::thread_info_to_thread_handle): Define.
---
 gdb/gdbthread.h        |  5 +++++
 gdb/linux-thread-db.c  | 19 +++++++++++++++++++
 gdb/remote.c           | 10 ++++++++++
 gdb/target-debug.h     |  2 ++
 gdb/target-delegates.c | 28 ++++++++++++++++++++++++++++
 gdb/target.c           |  6 ++++++
 gdb/target.h           |  6 ++++++
 gdb/thread.c           | 22 ++++++++++++++++++++++
 8 files changed, 98 insertions(+)

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 5950512..45a2259 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -459,6 +459,11 @@ struct thread_info *find_thread_by_handle (struct value *thread_handle,
 /* Finds the first thread of the specified inferior.  */
 extern struct thread_info *first_thread_of_inferior (inferior *inf);
 
+/* Return thread handle of type TYPE given thread THR.  Returns NULL if
+   there is no handle and throw an error if there is a size mismatch.  */
+struct value *thread_to_thread_handle (struct thread_info *thr,
+                                       struct type *type);
+
 /* Find thread parent.  */
 struct thread_info * thread_parent (struct thread_info *thread);
 
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index ad193d6..689e0f7 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -105,6 +105,7 @@ public:
   thread_info *thread_handle_to_thread_info (const gdb_byte *thread_handle,
 					     int handle_len,
 					     inferior *inf) override;
+  gdb::byte_vector thread_info_to_thread_handle (struct thread_info *) override;
 };
 
 thread_db_target::thread_db_target ()
@@ -1695,6 +1696,24 @@ thread_db_target::thread_handle_to_thread_info (const gdb_byte *thread_handle,
   return NULL;
 }
 
+/* Return the thread handle associated the thread_info pointer TP.  */
+
+gdb::byte_vector
+thread_db_target::thread_info_to_thread_handle (struct thread_info *tp)
+{
+  thread_db_thread_info *priv = get_thread_db_thread_info (tp);
+
+  if (priv == NULL)
+    return gdb::byte_vector ();
+
+  int handle_size = sizeof (priv->tid);
+  gdb::byte_vector rv (handle_size);
+
+  memcpy (rv.data (), &priv->tid, handle_size);
+
+  return rv;
+}
+
 /* 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.  */
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 3b19da7..e46575d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -499,6 +499,9 @@ public:
 					     int handle_len,
 					     inferior *inf) override;
 
+  gdb::byte_vector thread_info_to_thread_handle (struct thread_info *tp)
+						 override;
+
   void stop (ptid_t) override;
 
   void interrupt () override;
@@ -14093,6 +14096,13 @@ remote_target::thread_handle_to_thread_info (const gdb_byte *thread_handle,
   return NULL;
 }
 
+gdb::byte_vector
+remote_target::thread_info_to_thread_handle (struct thread_info *tp)
+{
+  remote_thread_info *priv = get_remote_thread_info (tp);
+  return priv->thread_handle;
+}
+
 bool
 remote_target::can_async_p ()
 {
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 1e904b9..503482a 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -184,6 +184,8 @@
   target_debug_do_print (host_address_to_string (X))
 #define target_debug_print_thread_info_pp(X)		\
   target_debug_do_print (host_address_to_string (X))
+#define target_debug_print_gdb_byte_vector(X)	\
+  target_debug_do_print (host_address_to_string (X.data ()))
 
 static void
 target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status)
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 03136df..76519f3 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -70,6 +70,7 @@ struct dummy_target : public target_ops
   const char *extra_thread_info (thread_info *arg0) override;
   const char *thread_name (thread_info *arg0) override;
   thread_info *thread_handle_to_thread_info (const gdb_byte *arg0, int arg1, inferior *arg2) override;
+  gdb::byte_vector thread_info_to_thread_handle (struct thread_info *arg0) override;
   void stop (ptid_t arg0) override;
   void interrupt () override;
   void pass_ctrlc () override;
@@ -237,6 +238,7 @@ struct debug_target : public target_ops
   const char *extra_thread_info (thread_info *arg0) override;
   const char *thread_name (thread_info *arg0) override;
   thread_info *thread_handle_to_thread_info (const gdb_byte *arg0, int arg1, inferior *arg2) override;
+  gdb::byte_vector thread_info_to_thread_handle (struct thread_info *arg0) override;
   void stop (ptid_t arg0) override;
   void interrupt () override;
   void pass_ctrlc () override;
@@ -1858,6 +1860,32 @@ debug_target::thread_handle_to_thread_info (const gdb_byte *arg0, int arg1, infe
   return result;
 }
 
+gdb::byte_vector
+target_ops::thread_info_to_thread_handle (struct thread_info *arg0)
+{
+  return this->beneath ()->thread_info_to_thread_handle (arg0);
+}
+
+gdb::byte_vector
+dummy_target::thread_info_to_thread_handle (struct thread_info *arg0)
+{
+  return gdb::byte_vector ();
+}
+
+gdb::byte_vector
+debug_target::thread_info_to_thread_handle (struct thread_info *arg0)
+{
+  gdb::byte_vector result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->thread_info_to_thread_handle (...)\n", this->beneath ()->shortname ());
+  result = this->beneath ()->thread_info_to_thread_handle (arg0);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->thread_info_to_thread_handle (", this->beneath ()->shortname ());
+  target_debug_print_struct_thread_info_p (arg0);
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_gdb_byte_vector (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
 void
 target_ops::stop (ptid_t arg0)
 {
diff --git a/gdb/target.c b/gdb/target.c
index 2d98954..8af2368 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2147,6 +2147,12 @@ target_thread_handle_to_thread_info (const gdb_byte *thread_handle,
 						     handle_len, inf);
 }
 
+gdb::byte_vector
+target_thread_info_to_thread_handle (struct thread_info *tip)
+{
+  return current_top_target ()->thread_info_to_thread_handle (tip);
+}
+
 void
 target_resume (ptid_t ptid, int step, enum gdb_signal signal)
 {
diff --git a/gdb/target.h b/gdb/target.h
index a3000c8..3ee744b 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -658,6 +658,8 @@ struct target_ops
 						       int,
 						       inferior *inf)
       TARGET_DEFAULT_RETURN (NULL);
+    virtual gdb::byte_vector thread_info_to_thread_handle (struct thread_info *)
+      TARGET_DEFAULT_RETURN (gdb::byte_vector ());
     virtual void stop (ptid_t)
       TARGET_DEFAULT_IGNORE ();
     virtual void interrupt ()
@@ -1863,6 +1865,10 @@ extern const char *target_thread_name (struct thread_info *);
 extern struct thread_info *target_thread_handle_to_thread_info
   (const gdb_byte *thread_handle, int handle_len, struct inferior *inf);
 
+/* Given a thread, return the thread handle.  */
+extern gdb::byte_vector target_thread_info_to_thread_handle
+  (struct thread_info *);
+
 /* 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 6c17923..8ac8b97 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -46,6 +46,7 @@
 #include <algorithm>
 #include "common/gdb_optional.h"
 #include "extension.h"
+#include "gdbtypes.h"
 
 /* Definition of struct thread_info exported to gdbthread.h.  */
 
@@ -532,6 +533,27 @@ find_thread_by_handle (struct value *thread_handle, struct inferior *inf)
 	    inf);
 }
 
+/* See gdbthread.h.  */
+
+struct value *
+thread_to_thread_handle (struct thread_info *thr, struct type *type)
+{
+  gdb::byte_vector hv = target_thread_info_to_thread_handle (thr);
+
+  if (hv.size () == 0)
+    return NULL;
+
+  check_typedef (type);
+  if (hv.size () != TYPE_LENGTH (type))
+    error (_("Thread handle size mismatch: Expecting %d, got %zu instead"),
+           TYPE_LENGTH (type), hv.size ());
+
+  struct value *rv = allocate_value (type);
+  memcpy (value_contents_raw (rv), hv.data (), hv.size ());
+
+  return rv;
+}
+
 /*
  * Thread iterator function.
  *

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

* [PATCH 2/4] Add python method gdb.InferiorThread.thread_handle
  2018-09-16  5:08 [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
  2018-09-16  5:16 ` [PATCH 1/4] Add and implement thread_to_thread_handle Kevin Buettner
  2018-09-16  5:17 ` [PATCH 3/4] Tests for gdb.InfThread.thread_handle Kevin Buettner
@ 2018-09-16  5:17 ` Kevin Buettner
  2018-09-16  5:19 ` [PATCH 4/4] Documentation for python method InferiorThread.thread_handle Kevin Buettner
  2018-09-18 11:03 ` [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle Tom Tromey
  4 siblings, 0 replies; 10+ messages in thread
From: Kevin Buettner @ 2018-09-16  5:17 UTC (permalink / raw)
  To: gdb-patches

gdb/ChangeLog:
    
    	* py-infthread.c (thpy_thread_handle): New function.
    	(thread_object_methods): Add thpy_thread_handle.
---
 gdb/python/py-infthread.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 36ae71b..9073231 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -253,6 +253,32 @@ thpy_is_valid (PyObject *self, PyObject *args)
   Py_RETURN_TRUE;
 }
 
+/* Implementation of gdb.InferiorThread.thread_handle (self, gdbtype) -> handle. */
+
+static PyObject *
+thpy_thread_handle (PyObject *self, PyObject *args)
+{
+  PyObject *type_obj;
+  struct type *type;
+  thread_object *thread_obj = (thread_object *) self;
+
+  THPY_REQUIRE_VALID (thread_obj);
+
+  if (! PyArg_ParseTuple (args, "O", &type_obj))
+    return NULL;
+
+  type = type_object_to_type (type_obj);
+  if (type == nullptr)
+    {
+      PyErr_SetString (PyExc_RuntimeError,
+		       _("Argument must be a type."));
+      return NULL;
+    }
+
+  return value_to_value_object
+           (thread_to_thread_handle (thread_obj->thread, type));
+}
+
 /* Return a reference to a new Python object representing a ptid_t.
    The object is a tuple containing (pid, lwp, tid). */
 PyObject *
@@ -340,6 +366,9 @@ Return whether the thread is running." },
   { "is_exited", thpy_is_exited, METH_NOARGS,
     "is_exited () -> Boolean\n\
 Return whether the thread is exited." },
+  { "thread_handle", thpy_thread_handle, METH_VARARGS,
+    "thread_handle  (gdbtype) -> handle\n\
+Return thread handle for thread." },
 
   { NULL }
 };

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

* [PATCH 3/4] Tests for gdb.InfThread.thread_handle
  2018-09-16  5:08 [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
  2018-09-16  5:16 ` [PATCH 1/4] Add and implement thread_to_thread_handle Kevin Buettner
@ 2018-09-16  5:17 ` Kevin Buettner
  2018-09-16  5:17 ` [PATCH 2/4] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Kevin Buettner @ 2018-09-16  5:17 UTC (permalink / raw)
  To: gdb-patches

gdb/testsuite/ChangeLog:
    
    	* gdb.python/py-thrhandle.exp: Add tests for
    	gdb.InfThread.thread_handle.
---
 gdb/testsuite/gdb.python/py-thrhandle.exp | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gdb/testsuite/gdb.python/py-thrhandle.exp b/gdb/testsuite/gdb.python/py-thrhandle.exp
index 49aa97c..b3caaf5 100644
--- a/gdb/testsuite/gdb.python/py-thrhandle.exp
+++ b/gdb/testsuite/gdb.python/py-thrhandle.exp
@@ -104,3 +104,17 @@ gdb_test "python print(gdb.selected_inferior().thread_from_thread_handle(gdb.par
 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"
+
+# Test the thread_handle method
+
+gdb_test "python h=gdb.parse_and_eval('thrs\[0\]');print(gdb.selected_inferior().thread_from_thread_handle(h)).thread_handle(gdb.lookup_type('pthread_t'))==h" \
+         "True" \
+	 "Check operation of thread_handle method for thrs\[0\]"
+
+gdb_test "python h=gdb.parse_and_eval('thrs\[1\]');print(gdb.selected_inferior().thread_from_thread_handle(h)).thread_handle(gdb.lookup_type('pthread_t'))==h" \
+         "True" \
+	 "Check operation of thread_handle method for thrs\[1\]"
+
+gdb_test "python h=gdb.parse_and_eval('thrs\[2\]');print(gdb.selected_inferior().thread_from_thread_handle(h)).thread_handle(gdb.lookup_type('pthread_t'))==h" \
+         "True" \
+	 "Check operation of thread_handle method for thrs\[2\]"

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

* [PATCH 4/4] Documentation for python method InferiorThread.thread_handle
  2018-09-16  5:08 [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
                   ` (2 preceding siblings ...)
  2018-09-16  5:17 ` [PATCH 2/4] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
@ 2018-09-16  5:19 ` Kevin Buettner
  2018-09-16 16:04   ` Eli Zaretskii
  2018-09-18 11:16   ` Tom Tromey
  2018-09-18 11:03 ` [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle Tom Tromey
  4 siblings, 2 replies; 10+ messages in thread
From: Kevin Buettner @ 2018-09-16  5:19 UTC (permalink / raw)
  To: gdb-patches

gdb/doc/ChangeLog:

    	* python.texi (Threads In Python): Add description for method
    	InferiorThread.thread_handle.
---
 gdb/doc/python.texi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index aca6ec8..87e1762 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3220,6 +3220,14 @@ Return a Boolean indicating whether the thread is running.
 Return a Boolean indicating whether the thread is exited.
 @end defun
 
+@defun InferiorThread.thread_handle (type)
+Return the thread object's handle.  Since handles are generally opaque
+objects, the type @var{type} is used to ensure that the correct amount
+of space is allocated for the returned handle value.  When using handles
+associated with the pthreads library, the value passed for @var{type}
+should be the result of invoking @code{gdb.lookup_type('pthread_t')}.
+@end defun
+
 @node Recordings In Python
 @subsubsection Recordings In Python
 @cindex recordings in python

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

* Re: [PATCH 4/4] Documentation for python method InferiorThread.thread_handle
  2018-09-16  5:19 ` [PATCH 4/4] Documentation for python method InferiorThread.thread_handle Kevin Buettner
@ 2018-09-16 16:04   ` Eli Zaretskii
  2018-09-18 11:16   ` Tom Tromey
  1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2018-09-16 16:04 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

> Date: Sat, 15 Sep 2018 22:18:47 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> 
> +@defun InferiorThread.thread_handle (type)
> +Return the thread object's handle.  Since handles are generally opaque
> +objects, the type @var{type} is used to ensure that the correct amount

The phrase "type TYPE is used ..." is awkward.  I suggest to use just
"@var{type", as the name speaks for itself.

Actually, since we want to avoid passive tense, I'd rephrase

  Since various thread handles have different sizes, a Python program
  should supply a suitable @var{type} argument, to ensure @value{GDBN}
  allocates the correct amount of memory for the handle.  E.g., for
  thread handles associated with the @code{pthreads} library, you
  should use @code{gdb.lookup_type('pthread_t')} for @var{type}.

Thanks.

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

* Re: [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle
  2018-09-16  5:08 [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
                   ` (3 preceding siblings ...)
  2018-09-16  5:19 ` [PATCH 4/4] Documentation for python method InferiorThread.thread_handle Kevin Buettner
@ 2018-09-18 11:03 ` Tom Tromey
  2018-09-19 23:53   ` Kevin Buettner
  4 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2018-09-18 11:03 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:

Kevin> This four part series adds a python method named "thread_handle"
Kevin> which is used to fetch the thread handle from a thread object.

Kevin> It will eventually be used as part of the implementation of the
Kevin> "thread parent" operation for the OpenMP work that I've been doing.

Could you explain more about thread handles -- what they are and why
they are needed?  I do not really understand the motivation for this,
but I would like to.

Tom

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

* Re: [PATCH 4/4] Documentation for python method InferiorThread.thread_handle
  2018-09-16  5:19 ` [PATCH 4/4] Documentation for python method InferiorThread.thread_handle Kevin Buettner
  2018-09-16 16:04   ` Eli Zaretskii
@ 2018-09-18 11:16   ` Tom Tromey
  2018-09-20  0:03     ` Kevin Buettner
  1 sibling, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2018-09-18 11:16 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:

Kevin> +@defun InferiorThread.thread_handle (type)
Kevin> +Return the thread object's handle.  Since handles are generally opaque
Kevin> +objects, the type @var{type} is used to ensure that the correct amount
Kevin> +of space is allocated for the returned handle value.  When using handles
Kevin> +associated with the pthreads library, the value passed for @var{type}
Kevin> +should be the result of invoking @code{gdb.lookup_type('pthread_t')}.
Kevin> +@end defun

This API requires the Python programmer to know the type of the
inferior's thread ID.  However, it seems to me that the target also
knows it, or should know it, and so there should be no reason to pass in
the type.

But if a handle is supposed to be opaque, why have a type at all?  It
could just be a bunch of bytes, or even just some completely opaque
object...  but if a completely opaque object would do, why not use the
InferiorThread itself?

I think the code patches all look good.  I didn't even have any nits.
However, exposing some API to Python is more of a commitment and so I'd
like to understand this area and come to an agreement about how it
should work before anything here goes in.

thanks,
Tom

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

* Re: [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle
  2018-09-18 11:03 ` [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle Tom Tromey
@ 2018-09-19 23:53   ` Kevin Buettner
  0 siblings, 0 replies; 10+ messages in thread
From: Kevin Buettner @ 2018-09-19 23:53 UTC (permalink / raw)
  To: gdb-patches

On Tue, 18 Sep 2018 05:02:40 -0600
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:  
> 
> Kevin> This four part series adds a python method named "thread_handle"
> Kevin> which is used to fetch the thread handle from a thread object.  
> 
> Kevin> It will eventually be used as part of the implementation of the
> Kevin> "thread parent" operation for the OpenMP work that I've been doing.  
> 
> Could you explain more about thread handles -- what they are and why
> they are needed?  I do not really understand the motivation for this,
> but I would like to.

Hi Tom,

I'm working on a bug in which GDB is sometimes unable to find and
print variables which should be visible from an OpenMP parallel
region.  During compilation, the parallel region has been transformed
into an outlined function and, when GDB is stopped within that
function, the GDB user is not able to examine all variables that
should be visible.

Part of the solution was a change to the DWARF generated by GCC which
caused outlined OpenMP functions to be placed within a lexical scope
which causes the variables in question to become visible.  But work
was/is also needed on the GDB side.  The variables might not be on the
stack of the current thread, but may instead be found on the stack of
the thread which created the current thread.  I'm calling that thread,
the creator thread, the "thread parent".  GDB needs a way to find the
thread parent.  If the variable in question is not found on the stack
of the current thread, GDB will try to find it in the thread parent
(and other ancestorors too - which is needed for nested parallel
regions).

The libgomp library contains some data structures describing the
threads that it manages.  One would hope that that data structure
contains a thread handle which will allow the debugger examining those
data structures to map the handle to whatever data structures the
debugger is using to represent the threads.  For a pthread based
system, the thread handle will be of type pthread_t.  Such handles are
generally opaque, might not fit in an integer, and sometimes even
require the use of a comparison function to determine whether two
handles are identical.

When examining that data structure (which I'm doing via Python code),
I need to be able find the thread associated with a particular handle. 
To this end, I've already added Inferior.thread_from_thread_handle. 
It is passed a thread handle and will return the corresponding thread
if such can be found.

So... now things start to get complicated.

It turned out the libgomp wasn't storing the thread handle in its
thread management data structures.  I submitted a patch which added
the handle and caused it to be initialized for each thread.  According
to Jakub, some architectures will require that patch, but most
architectures (with Linux as the OS) use -ftls-model=initial-exec TLS
model for libgomp.  Again, according to Jakub, this means that we
should be able derive the handle from the address of gomp_tls_data,
which is the address of a struct in thread local storage.

After some investigation, I verified that this does indeed work, but
in order to make it work, I needed to be able to find the thread handle
associated with the current thread.

Handles in this case are NOT opaque.  My python code needs to be be
able to convert a handle to an integer (or address) and do arithmetic
on it eventually resulting in a different handle.  So my remark in the
documentation patch (part 4) was not correct.  I had written this code
a while back and had forgotten some of these details.

In order to make things more concrete, below is my current implementation
of the thread parent operation.  As should be evident by some of the
comments, it's not completely finished yet, but you'll at least be able
to see how I'm using the two methods which map handles to threads and
vice versa.

# -*- python -*-

""" This module is an implementation of `thread parent'.

    GDB calls thread_parent when it wishes to search ancestor threads for
    variables when debugging an OpenMP program.  (It might be useful in
    other circumstances too, but so far this is the only use case.)

    Given a thread, thread_parent is expected to return either None or the
    thread corresponding to the parent thread.

    At the moment, this module will not (always) find the correct parent
    thread for the nested case.  More work is required to find the parents
    of nested threads.

    This file should be named (at the moment) libgomp.so.1.0.0-gdb.py.  It
    should be placed in the same directory as the libgomp shared library.
    Moreover, libgomp.so.1.0.0 needs to have intact DWARF debug
    symbols.

    It should eventually be made part of libgomp and installed along with
    the libgomp shared library.

"""

import gdb
from gdb.thread_parent import ThreadParent

class GompThreadParent(ThreadParent):

    def __init__(self):
	ThreadParent.__init__(self, "GOMP Thread Parent")
        self.offset = None
	self.no_pthread_id = False
	self.long_long_type = gdb.lookup_type("long long")
	self.pthread_t_type = gdb.lookup_type("pthread_t")

    def parent_from_handle(self, thr, h):
        parent_thr = gdb.selected_inferior().thread_from_thread_handle(h)
        if parent_thr == thr:
            return None
        else:
            return parent_thr

    def thread_parent_via_pthread_id_field(self, thr):
        # Uncomment line below for deployment.
        #ph = gdb.parse_and_eval("gomp_tls_data->thread_pool.threads[0].pthread_id")
        # For testing only: pthread_idddd does not exist.
        ph = gdb.parse_and_eval("gomp_tls_data->thread_pool.threads[0].pthread_idddd")
        return self.parent_from_handle (thr, ph)

    def thread_parent_via_offset(self, thr):
        if self.offset is None:
            gomp_tls_addr =  gdb.parse_and_eval("&gomp_tls_data").cast(self.long_long_type)
            self.offset = gdb.selected_thread().thread_handle(self.pthread_t_type) - gomp_tls_addr
        ph = gdb.parse_and_eval("gomp_tls_data->thread_pool.threads[0]") \
                 .cast(self.long_long_type) \
             + self.offset
        return self.parent_from_handle (thr, ph)

    def thread_parent(self, thr):

        if not self.no_pthread_id:
            try:
                # print("Trying to fetch parent via thread_id")
                parent = self.thread_parent_via_pthread_id_field(thr)
            except gdb.error:
                if self.no_pthread_id:
                    # Clean this up
                    print("Got an exception after already successfully fetching pthread_id")
                self.no_pthread_id = True
            else:
                return parent

        # Should we even bother with the try / except here?  Maybe print a
        # warning if there is an exception?
        try:
            parent = self.thread_parent_via_offset(thr)
        except:
            pass
        else:
            return parent

        return None

    __call__ = thread_parent

gdb.thread_parent.register_thread_parent(None, GompThreadParent())

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

* Re: [PATCH 4/4] Documentation for python method InferiorThread.thread_handle
  2018-09-18 11:16   ` Tom Tromey
@ 2018-09-20  0:03     ` Kevin Buettner
  0 siblings, 0 replies; 10+ messages in thread
From: Kevin Buettner @ 2018-09-20  0:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

On Tue, 18 Sep 2018 05:16:01 -0600
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:  
> 
> Kevin> +@defun InferiorThread.thread_handle (type)
> Kevin> +Return the thread object's handle.  Since handles are generally opaque
> Kevin> +objects, the type @var{type} is used to ensure that the correct amount
> Kevin> +of space is allocated for the returned handle value.  When using handles
> Kevin> +associated with the pthreads library, the value passed for @var{type}
> Kevin> +should be the result of invoking @code{gdb.lookup_type('pthread_t')}.
> Kevin> +@end defun  
> 
> This API requires the Python programmer to know the type of the
> inferior's thread ID.  However, it seems to me that the target also
> knows it, or should know it, and so there should be no reason to pass in
> the type.
> 
> But if a handle is supposed to be opaque, why have a type at all?  It
> could just be a bunch of bytes, or even just some completely opaque
> object...  but if a completely opaque object would do, why not use the
> InferiorThread itself?

As discussed in my other reply, I (unfortunately) require the handle to
not be opaque.  I made an error when I wrote the documentation patch.

With regard to passing the type, when debugging a remote target, GDB
won't know the handle type, though it does know the size.  If I were
truly able to work with an opaque object, I wouldn't need the type.
It may still be the case that I don't need it; I'm still pondering this.

I'll eventually post an updated doc patch (and updates to the rest
of the patch series if needed) which will describe the actual state
of affairs with regard to opacity.

> I think the code patches all look good.  I didn't even have any nits.
> However, exposing some API to Python is more of a commitment and so I'd
> like to understand this area and come to an agreement about how it
> should work before anything here goes in.

I've tried to provide the necessary background here:

https://sourceware.org/ml/gdb-patches/2018-09/msg00719.html

Kevin

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

end of thread, other threads:[~2018-09-20  0:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-16  5:08 [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
2018-09-16  5:16 ` [PATCH 1/4] Add and implement thread_to_thread_handle Kevin Buettner
2018-09-16  5:17 ` [PATCH 3/4] Tests for gdb.InfThread.thread_handle Kevin Buettner
2018-09-16  5:17 ` [PATCH 2/4] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
2018-09-16  5:19 ` [PATCH 4/4] Documentation for python method InferiorThread.thread_handle Kevin Buettner
2018-09-16 16:04   ` Eli Zaretskii
2018-09-18 11:16   ` Tom Tromey
2018-09-20  0:03     ` Kevin Buettner
2018-09-18 11:03 ` [PATCH 0/4] Add python method gdb.InferiorThread.thread_handle Tom Tromey
2018-09-19 23:53   ` 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).