public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Add python method gdb.InferiorThread.thread_handle
@ 2019-02-28  2:24 Kevin Buettner
  2019-02-28  2:30 ` [PATCH v2 1/5] Introduce target_ops method thread_info_to_thread_handle Kevin Buettner
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Kevin Buettner @ 2019-02-28  2:24 UTC (permalink / raw)
  To: gdb-patches

This five 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.

While thread handles are normally opaque, for my OpenMP work, it's
become necessary to convert a handle to a form upon which arithmetic
may be performed.  Via some simple arithmetic manipulations, it's
possible to find the handle associated with a GOMP thread.  If you want
details on this, see:

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

This is version 2 of a series that I first posted in Sept, 2018.

In v1, InferiorThread.thread_handle was passed a type and returned
a gdb.Value object representing the handle (of the type passed in).

In v2, InferiorThread.thread_handle is a nullary method.  It returns
the thread handle represented as a Python bytes object.  If it's
necessary to pierce the opacity of this object, as it is for my use
case, the two argument form of the gdb.Value constructor may be used
to make an object of some suitable type.

Kevin

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

* [PATCH v2 1/5] Introduce target_ops method thread_info_to_thread_handle
  2019-02-28  2:24 [PATCH v2 0/5] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
@ 2019-02-28  2:30 ` Kevin Buettner
  2019-03-05 21:44   ` Tom Tromey
  2019-02-28  2:31 ` [PATCH v2 2/5] Add python method InferiorThread.thread_handle Kevin Buettner
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Kevin Buettner @ 2019-02-28  2:30 UTC (permalink / raw)
  To: gdb-patches

This patch adds a thread_info_to_thread_handle method to the target_ops
struct.  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/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 ++++++
 6 files changed, 71 insertions(+)

diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index fd67dcb17b..fe9f44046a 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;
 };
 
 static char *libthread_db_search_path;
@@ -1698,6 +1699,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 36136e3e3e..546b0f36f7 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -495,6 +495,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;
@@ -14047,6 +14050,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 d9f7d46dd9..23a8d25420 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 a25851e9cf..ffa90d966f 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;
@@ -1854,6 +1856,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 116510e8cb..8bfedb3619 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2082,6 +2082,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 c95151a404..4920f00eeb 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -659,6 +659,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 ()
@@ -1851,6 +1853,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.
 

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

* [PATCH v2 2/5] Add python method InferiorThread.thread_handle
  2019-02-28  2:24 [PATCH v2 0/5] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
  2019-02-28  2:30 ` [PATCH v2 1/5] Introduce target_ops method thread_info_to_thread_handle Kevin Buettner
@ 2019-02-28  2:31 ` Kevin Buettner
  2019-03-05 21:48   ` Tom Tromey
  2019-02-28  2:32 ` [PATCH v2 3/5] Support buffer objects as handles in Inferior.thread_from_thread_handle() Kevin Buettner
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Kevin Buettner @ 2019-02-28  2:31 UTC (permalink / raw)
  To: gdb-patches

gdb/ChangeLog:

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

diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index bf90d08ae6..13e11b16e1 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -257,6 +257,24 @@ thpy_is_valid (PyObject *self, PyObject *args)
   Py_RETURN_TRUE;
 }
 
+/* Implementation of gdb.InferiorThread.thread_handle (self) -> handle. */
+
+static PyObject *
+thpy_thread_handle (PyObject *self, PyObject *args)
+{
+  thread_object *thread_obj = (thread_object *) self;
+  THPY_REQUIRE_VALID (thread_obj);
+
+  gdb::byte_vector hv = target_thread_info_to_thread_handle (thread_obj->thread);
+
+  if (hv.size () == 0)
+    return NULL;
+
+  PyObject *object = PyBytes_FromStringAndSize ((const char *) hv.data (),
+				                hv.size());
+  return object;
+}
+
 /* Return a reference to a new Python object representing a ptid_t.
    The object is a tuple containing (pid, lwp, tid). */
 PyObject *
@@ -336,6 +354,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_NOARGS,
+    "thread_handle  () -> handle\n\
+Return thread handle for thread." },
 
   { NULL }
 };

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

* [PATCH v2 3/5] Support buffer objects as handles in Inferior.thread_from_thread_handle()
  2019-02-28  2:24 [PATCH v2 0/5] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
  2019-02-28  2:30 ` [PATCH v2 1/5] Introduce target_ops method thread_info_to_thread_handle Kevin Buettner
  2019-02-28  2:31 ` [PATCH v2 2/5] Add python method InferiorThread.thread_handle Kevin Buettner
@ 2019-02-28  2:32 ` Kevin Buettner
  2019-03-05 21:51   ` Tom Tromey
  2019-02-28  2:34 ` [PATCH v2 4/5] Tests for gdb.InferiorThread.thread_handle Kevin Buettner
  2019-02-28  2:35 ` [PATCH v2 5/5] Documentation for python method InferiorThread.thread_handle Kevin Buettner
  4 siblings, 1 reply; 14+ messages in thread
From: Kevin Buettner @ 2019-02-28  2:32 UTC (permalink / raw)
  To: gdb-patches

InferiorThread.thread_handle() returns a python bytes object.  We'd
like to be able to pass the output of thread_handle() to
Inferior.thread_from_thread_handle().  Up to now,
thread_from_thread_handle() expects to receive a gdb.Value input.
This commit adds support to also allow a python buffer object to be
passed as the handle.

infpy_thread_from_thread_handle() calls find_thread_by_handle() which
has the obvious functionality.  It used to pass the thread handle via
a struct value pointer.  I've revised this interface to instead pass a
gdb_byte pointer and length.  I considered using a vector, but it
seemed more straightforward to just pass a pointer and length.

gdb/ChangeLog:

	* gdbthread.h (find_thread_by_handle): Revise declaration.
	* thread.c (find_thread_by_handle): Likewise.  Adjust
	implementation too.
	* python/py-inferior.c (infpy_thread_from_thread_handle): Add
	support for buffer objects as handles.
---
 gdb/gdbthread.h          |  3 ++-
 gdb/python/py-inferior.c | 23 ++++++++++++++++++++---
 gdb/thread.c             |  8 +++-----
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 7808569a8a..9a133eb45e 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -486,7 +486,8 @@ extern struct thread_info *find_thread_ptid (inferior *inf, ptid_t ptid);
 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 thread_info *find_thread_by_handle (const gdb_byte *thread_handle,
+					   size_t handle_len,
 					   struct inferior *inf);
 
 /* Finds the first thread of the specified inferior.  */
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 72fbf6d90b..7e20343cfd 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -774,7 +774,25 @@ infpy_thread_from_thread_handle (PyObject *self, PyObject *args, PyObject *kw)
   if (! gdb_PyArg_ParseTupleAndKeywords (args, kw, "O", keywords, &handle_obj))
     return NULL;
 
-  if (!gdbpy_is_value_object (handle_obj))
+  const gdb_byte *bytes;
+  size_t bytes_len;
+  Py_buffer_up buffer_up;
+  Py_buffer py_buf;
+
+  if (PyObject_CheckBuffer (handle_obj)
+      && PyObject_GetBuffer (handle_obj, &py_buf, PyBUF_SIMPLE) == 0)
+    {
+      buffer_up.reset (&py_buf);
+      bytes = (const gdb_byte *) py_buf.buf;
+      bytes_len = py_buf.len;
+    }
+  else if (gdbpy_is_value_object (handle_obj))
+    {
+      struct value *val = value_object_to_value (handle_obj);
+      bytes = value_contents_all (val);
+      bytes_len = TYPE_LENGTH (value_type (val));
+    }
+  else
     {
       PyErr_SetString (PyExc_TypeError,
 		       _("Argument 'handle_obj' must be a thread handle object."));
@@ -785,9 +803,8 @@ infpy_thread_from_thread_handle (PyObject *self, PyObject *args, PyObject *kw)
   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);
+      thread_info = find_thread_by_handle (bytes, bytes_len, inf_obj->inferior);
       if (thread_info != NULL)
 	return thread_to_thread_object (thread_info).release ();
     }
diff --git a/gdb/thread.c b/gdb/thread.c
index 90999d3d0b..ac3d758c76 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -541,12 +541,10 @@ find_thread_ptid (inferior *inf, ptid_t ptid)
 /* See gdbthread.h.  */
 
 struct thread_info *
-find_thread_by_handle (struct value *thread_handle, struct inferior *inf)
+find_thread_by_handle (const gdb_byte *thread_handle, size_t handle_len,
+                       struct inferior *inf)
 {
-  return target_thread_handle_to_thread_info
-	   (value_contents_all (thread_handle),
-	    TYPE_LENGTH (value_type (thread_handle)),
-	    inf);
+  return target_thread_handle_to_thread_info (thread_handle, handle_len, inf);
 }
 
 /*

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

* [PATCH v2 4/5] Tests for gdb.InferiorThread.thread_handle
  2019-02-28  2:24 [PATCH v2 0/5] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
                   ` (2 preceding siblings ...)
  2019-02-28  2:32 ` [PATCH v2 3/5] Support buffer objects as handles in Inferior.thread_from_thread_handle() Kevin Buettner
@ 2019-02-28  2:34 ` Kevin Buettner
  2019-03-05 21:54   ` Tom Tromey
  2019-02-28  2:35 ` [PATCH v2 5/5] Documentation for python method InferiorThread.thread_handle Kevin Buettner
  4 siblings, 1 reply; 14+ messages in thread
From: Kevin Buettner @ 2019-02-28  2:34 UTC (permalink / raw)
  To: gdb-patches

gdb/testsuite/ChangeLog:

	* gdb.python/py-thrhandle.exp: Add tests for
	gdb.InferiorThread.thread_handle.
---
 gdb/testsuite/gdb.python/py-thrhandle.exp | 42 ++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.python/py-thrhandle.exp b/gdb/testsuite/gdb.python/py-thrhandle.exp
index 2c905b444f..ec6852b333 100644
--- a/gdb/testsuite/gdb.python/py-thrhandle.exp
+++ b/gdb/testsuite/gdb.python/py-thrhandle.exp
@@ -16,11 +16,12 @@
 # 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.
+# This file verifies that methods Inferior.thread_from_thread_handle
+# and InferiorThread.thread_handle work as expected.
 
-standard_testfile
+load_lib gdb-python.exp
 
+standard_testfile
 
 if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable debug] != "" } {
     return -1
@@ -104,3 +105,38 @@ 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_py_test_silent_cmd "python tp=gdb.lookup_type('pthread_t')" \
+                       "Get pthread_t type" 0
+gdb_py_test_silent_cmd "python inf=gdb.selected_inferior()" "Get inferior" 0
+
+foreach thrN {0 1 2} {
+    with_test_prefix "thread $thrN" {
+
+	gdb_py_test_silent_cmd \
+	    "python hand = gdb.parse_and_eval('thrs\[$thrN\]')" \
+	    "fetch thread handle from inferior" \
+	    1
+
+	gdb_py_test_silent_cmd \
+	    "python hand_bytes = inf.thread_from_thread_handle(hand).thread_handle()" \
+	    "fetch thread handle from thread" \
+	    1
+
+
+	# It'd be nice to be able to use this comparison expression:
+	#
+	#    hand == hand_bytes
+	#
+	# But this won't work because hand is a gdb.Value and hand_bytes
+	# is a Python bytes object.  Therefore, we convert the bytes
+	# object into a gdb.value by calling the two argument form of
+	# its constructor.
+
+        gdb_test "python print(gdb.Value(hand_bytes, tp) == hand)" \
+	         "True" \
+	         "verify that handles are the same"
+    }
+}

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

* [PATCH v2 5/5] Documentation for python method InferiorThread.thread_handle
  2019-02-28  2:24 [PATCH v2 0/5] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
                   ` (3 preceding siblings ...)
  2019-02-28  2:34 ` [PATCH v2 4/5] Tests for gdb.InferiorThread.thread_handle Kevin Buettner
@ 2019-02-28  2:35 ` Kevin Buettner
  2019-02-28 18:06   ` Eli Zaretskii
  4 siblings, 1 reply; 14+ messages in thread
From: Kevin Buettner @ 2019-02-28  2:35 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 7f6f52c4df..cc663486a2 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3240,6 +3240,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 ()
+Return the thread object's handle, represented as a Python @code{bytes}
+object.  A @code{gdb.Value} representation of the handle may be
+constructed via @code{gdb.Value(bufobj, type)} where @var{bufobj} is
+the Python @code{bytes} representation of the handle and @var{type} is
+a @code{gdb.Type} for the handle type.
+@end defun
+
 @node Recordings In Python
 @subsubsection Recordings In Python
 @cindex recordings in python

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

* Re: [PATCH v2 5/5] Documentation for python method InferiorThread.thread_handle
  2019-02-28  2:35 ` [PATCH v2 5/5] Documentation for python method InferiorThread.thread_handle Kevin Buettner
@ 2019-02-28 18:06   ` Eli Zaretskii
  0 siblings, 0 replies; 14+ messages in thread
From: Eli Zaretskii @ 2019-02-28 18:06 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

> Date: Wed, 27 Feb 2019 19:35:00 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> 
> gdb/doc/ChangeLog:
> 
> 	* python.texi (Threads In Python): Add description for method
> 	InferiorThread.thread_handle.

OK, thanks.

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

* Re: [PATCH v2 1/5] Introduce target_ops method thread_info_to_thread_handle
  2019-02-28  2:30 ` [PATCH v2 1/5] Introduce target_ops method thread_info_to_thread_handle Kevin Buettner
@ 2019-03-05 21:44   ` Tom Tromey
  0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2019-03-05 21:44 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

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

Kevin> +gdb::byte_vector
Kevin> +target_thread_info_to_thread_handle (struct thread_info *tip)
Kevin> +{
Kevin> +  return current_top_target ()->thread_info_to_thread_handle (tip);
Kevin> +}

It's the gdb style to stick /* See target.h.  */ before the definition.

Kevin> +    virtual gdb::byte_vector thread_info_to_thread_handle (struct thread_info *)
Kevin> +      TARGET_DEFAULT_RETURN (gdb::byte_vector ());

This should probably have a comment explaining what is meant by the
return value.

Kevin> +/* Given a thread, return the thread handle.  */
Kevin> +extern gdb::byte_vector target_thread_info_to_thread_handle
Kevin> +  (struct thread_info *);

Explaining that the thread handle is just some target-specific bytes
would be good.

Tom

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

* Re: [PATCH v2 2/5] Add python method InferiorThread.thread_handle
  2019-02-28  2:31 ` [PATCH v2 2/5] Add python method InferiorThread.thread_handle Kevin Buettner
@ 2019-03-05 21:48   ` Tom Tromey
  2019-03-20 20:24     ` Kevin Buettner
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2019-03-05 21:48 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

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

Kevin> +/* Implementation of gdb.InferiorThread.thread_handle (self) -> handle. */

What would you think of just the name "handle"?  The "thread" part
seemed redundant to me.

Kevin> +  gdb::byte_vector hv = target_thread_info_to_thread_handle (thread_obj->thread);

In general, calls into gdb from Python require a try/catch.
It can be ok if you're 100% sure a throw is not only not possible but
will never be possible.  (I feel like we normally ignore out-of-memory
exceptions when deciding this, but that's probably a bad idea.)

Kevin> +  PyObject *object = PyBytes_FromStringAndSize ((const char *) hv.data (),
Kevin> +				                hv.size());

I thought this wasn't available in Python 2 but I see it is used
unconditionally in py-record-btrace.c.

Tom

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

* Re: [PATCH v2 3/5] Support buffer objects as handles in Inferior.thread_from_thread_handle()
  2019-02-28  2:32 ` [PATCH v2 3/5] Support buffer objects as handles in Inferior.thread_from_thread_handle() Kevin Buettner
@ 2019-03-05 21:51   ` Tom Tromey
  0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2019-03-05 21:51 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

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

Kevin> I've revised this interface to instead pass a gdb_byte pointer
Kevin> and length.  I considered using a vector, but it seemed more
Kevin> straightforward to just pass a pointer and length.

How about a gdb::array_view instead?

The rest seems fine to me, thanks.

Tom

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

* Re: [PATCH v2 4/5] Tests for gdb.InferiorThread.thread_handle
  2019-02-28  2:34 ` [PATCH v2 4/5] Tests for gdb.InferiorThread.thread_handle Kevin Buettner
@ 2019-03-05 21:54   ` Tom Tromey
  0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2019-03-05 21:54 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

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

Kevin> +gdb_py_test_silent_cmd "python tp=gdb.lookup_type('pthread_t')" \
Kevin> +                       "Get pthread_t type" 0

This indentation is off, but looking at the file I see it is already not
really in line with the official gdb test suite style.  So, no big deal
I suppose.

So, this seems fine to me.

Tom

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

* Re: [PATCH v2 2/5] Add python method InferiorThread.thread_handle
  2019-03-05 21:48   ` Tom Tromey
@ 2019-03-20 20:24     ` Kevin Buettner
  2019-03-20 20:39       ` Tom Tromey
  0 siblings, 1 reply; 14+ messages in thread
From: Kevin Buettner @ 2019-03-20 20:24 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

On Tue, 05 Mar 2019 14:48:34 -0700
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:  
> 
> Kevin> +/* Implementation of gdb.InferiorThread.thread_handle (self) -> handle. */  
> 
> What would you think of just the name "handle"?  The "thread" part
> seemed redundant to me.

I like this idea, though to maintain consistency, I think we should
also rename the existing Inferior.thread_from_thread_handle function
to Inferior.thread_from_handle.  I'll add another commit which does
this to the upcoming v3 series which I'm working on.

Kevin

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

* Re: [PATCH v2 2/5] Add python method InferiorThread.thread_handle
  2019-03-20 20:24     ` Kevin Buettner
@ 2019-03-20 20:39       ` Tom Tromey
  2019-03-20 20:48         ` Kevin Buettner
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2019-03-20 20:39 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches, Tom Tromey

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

Kevin> I like this idea, though to maintain consistency, I think we should
Kevin> also rename the existing Inferior.thread_from_thread_handle function
Kevin> to Inferior.thread_from_handle.  I'll add another commit which does
Kevin> this to the upcoming v3 series which I'm working on.

I think we should probably keep the old name as an alias, at least.
I'd rather not break existing scripts (if there are any using this)
unless we really have to.

Tom

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

* Re: [PATCH v2 2/5] Add python method InferiorThread.thread_handle
  2019-03-20 20:39       ` Tom Tromey
@ 2019-03-20 20:48         ` Kevin Buettner
  0 siblings, 0 replies; 14+ messages in thread
From: Kevin Buettner @ 2019-03-20 20:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

On Wed, 20 Mar 2019 14:39:44 -0600
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:  
> 
> Kevin> I like this idea, though to maintain consistency, I think we should
> Kevin> also rename the existing Inferior.thread_from_thread_handle function
> Kevin> to Inferior.thread_from_handle.  I'll add another commit which does
> Kevin> this to the upcoming v3 series which I'm working on.  
> 
> I think we should probably keep the old name as an alias, at least.
> I'd rather not break existing scripts (if there are any using this)
> unless we really have to.

Okay, I'll retain the old name as an alias.

Kevin

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

end of thread, other threads:[~2019-03-20 20:48 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-28  2:24 [PATCH v2 0/5] Add python method gdb.InferiorThread.thread_handle Kevin Buettner
2019-02-28  2:30 ` [PATCH v2 1/5] Introduce target_ops method thread_info_to_thread_handle Kevin Buettner
2019-03-05 21:44   ` Tom Tromey
2019-02-28  2:31 ` [PATCH v2 2/5] Add python method InferiorThread.thread_handle Kevin Buettner
2019-03-05 21:48   ` Tom Tromey
2019-03-20 20:24     ` Kevin Buettner
2019-03-20 20:39       ` Tom Tromey
2019-03-20 20:48         ` Kevin Buettner
2019-02-28  2:32 ` [PATCH v2 3/5] Support buffer objects as handles in Inferior.thread_from_thread_handle() Kevin Buettner
2019-03-05 21:51   ` Tom Tromey
2019-02-28  2:34 ` [PATCH v2 4/5] Tests for gdb.InferiorThread.thread_handle Kevin Buettner
2019-03-05 21:54   ` Tom Tromey
2019-02-28  2:35 ` [PATCH v2 5/5] Documentation for python method InferiorThread.thread_handle Kevin Buettner
2019-02-28 18:06   ` Eli Zaretskii

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).