[v2,2/7] Add `thread_from_thread_handle' function to (Python) gdb module

Message ID 20170408230657.556b99e4@pinnacle.lan
State New, archived
Headers

Commit Message

Kevin Buettner April 9, 2017, 6:06 a.m. UTC
  gdb/ChangeLog:
    	* python/py-infthread.c (gdbpy_thread_from_thread_handle): New
    	function.
    	* python/python-internal.h (thread_object_type): Declare.
    	(gdbpy_thread_from_thread_handle): Declare.
    	* python/python.c (thread_from_thread_handle): Register.
---
 gdb/python/py-infthread.c    | 41 +++++++++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h |  3 +++
 gdb/python/python.c          |  4 ++++
 3 files changed, 48 insertions(+)
  

Comments

Phil Muldoon April 13, 2017, 4:11 p.m. UTC | #1
On 09/04/17 07:06, Kevin Buettner wrote:
> gdb/ChangeLog:
>     	* python/py-infthread.c (gdbpy_thread_from_thread_handle): New
>     	function.
>     	* python/python-internal.h (thread_object_type): Declare.
>     	(gdbpy_thread_from_thread_handle): Declare.
>     	* python/python.c (thread_from_thread_handle): Register.
> ---
>  gdb/python/py-infthread.c    | 41 +++++++++++++++++++++++++++++++++++++++++
>  gdb/python/python-internal.h |  3 +++
>  gdb/python/python.c          |  4 ++++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
> index 5482bf9..5739984 100644
> --- a/gdb/python/py-infthread.c
> +++ b/gdb/python/py-infthread.c
> @@ -294,6 +294,47 @@ gdbpy_selected_thread (PyObject *self, PyObject *args)
>    Py_RETURN_NONE;
>  }
>  
> +/* Implementation of gdb.thread_from_thread_handle (handle) 
> +                        ->  gdb.InferiorThread.  */
> +
> +PyObject *
> +gdbpy_thread_from_thread_handle (PyObject *self, PyObject *args, PyObject *kw)
> +{
> +  PyObject *handle_obj, *result;
> +  static char *keywords[] = { "thread_handle", NULL };
> +
> +  if (! PyArg_ParseTupleAndKeywords (args, kw, "O", keywords, &handle_obj))
> +    return NULL;
> +
> +  result = Py_None;
> +
> +  if (gdbpy_is_value_object (handle_obj))
> +    {
> +      TRY
> +	{
> +	  struct thread_info *thread_info;
> +	  struct value *val = value_object_to_value (handle_obj);
> +
> +	  thread_info = find_thread_by_handle (val);
> +	  if (thread_info != NULL)
> +	    {
> +	      result = (PyObject *) find_thread_object (thread_info->ptid);
> +	      if (result)
> +		Py_INCREF (result);

Initially this caught me off guard and wondered why find_thread_object required a reference count. But it does return a borrowed reference, though it does not seem to be documented as such.  I think the code surrounding this should be documented, but not in this patch. I'll add it to one of my TODOs.
> +	    }
> +	}
> +      CATCH (except, RETURN_MASK_ALL)
> +	{
> +	  if (except.reason < 0)
> +	    gdbpy_convert_exception (except);
> +	  return NULL;
> +	}
> +      END_CATCH
> +    }
> +
> +  return result;
> +}

The Python bits look good to me, as do the tests and documentation.  Please wait on a maintainer to give the overall patch a yes/no before proceeding.

Cheers

Phil
  
Simon Marchi May 5, 2017, 3:54 a.m. UTC | #2
On 2017-04-09 02:06, Kevin Buettner wrote:
> gdb/ChangeLog:
>     	* python/py-infthread.c (gdbpy_thread_from_thread_handle): New
>     	function.
>     	* python/python-internal.h (thread_object_type): Declare.
>     	(gdbpy_thread_from_thread_handle): Declare.
>     	* python/python.c (thread_from_thread_handle): Register.
> ---
>  gdb/python/py-infthread.c    | 41 
> +++++++++++++++++++++++++++++++++++++++++
>  gdb/python/python-internal.h |  3 +++
>  gdb/python/python.c          |  4 ++++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
> index 5482bf9..5739984 100644
> --- a/gdb/python/py-infthread.c
> +++ b/gdb/python/py-infthread.c
> @@ -294,6 +294,47 @@ gdbpy_selected_thread (PyObject *self, PyObject 
> *args)
>    Py_RETURN_NONE;
>  }
> 
> +/* Implementation of gdb.thread_from_thread_handle (handle)
> +                        ->  gdb.InferiorThread.  */
> +
> +PyObject *
> +gdbpy_thread_from_thread_handle (PyObject *self, PyObject *args, 
> PyObject *kw)
> +{
> +  PyObject *handle_obj, *result;
> +  static char *keywords[] = { "thread_handle", NULL };
> +
> +  if (! PyArg_ParseTupleAndKeywords (args, kw, "O", keywords, 
> &handle_obj))
> +    return NULL;

To adapt the patch to the current GDB master, you'll need to make 
keyword a const char * [], and use gdb_PyArg_ParseTupleAndKeywords.

> +
> +  result = Py_None;
> +
> +  if (gdbpy_is_value_object (handle_obj))
> +    {
> +      TRY
> +	{
> +	  struct thread_info *thread_info;
> +	  struct value *val = value_object_to_value (handle_obj);
> +
> +	  thread_info = find_thread_by_handle (val);
> +	  if (thread_info != NULL)
> +	    {
> +	      result = (PyObject *) find_thread_object (thread_info->ptid);
> +	      if (result)

if (result != NULL)

> +		Py_INCREF (result);
> +	    }
> +	}
> +      CATCH (except, RETURN_MASK_ALL)
> +	{
> +	  if (except.reason < 0)
> +	    gdbpy_convert_exception (except);
> +	  return NULL;

Should you use GDB_PY_HANDLE_EXCEPTION?  The behavior is a little bit 
different than your code, in that it will only return if an exception 
was set.  The Python doc says: "If the return value is NULL, an 
exception shall have been set.".  Here, it's possible that we return 
NULL without having set an exception.

Thanks,

Simon
  

Patch

diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 5482bf9..5739984 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -294,6 +294,47 @@  gdbpy_selected_thread (PyObject *self, PyObject *args)
   Py_RETURN_NONE;
 }
 
+/* Implementation of gdb.thread_from_thread_handle (handle) 
+                        ->  gdb.InferiorThread.  */
+
+PyObject *
+gdbpy_thread_from_thread_handle (PyObject *self, PyObject *args, PyObject *kw)
+{
+  PyObject *handle_obj, *result;
+  static char *keywords[] = { "thread_handle", NULL };
+
+  if (! PyArg_ParseTupleAndKeywords (args, kw, "O", keywords, &handle_obj))
+    return NULL;
+
+  result = Py_None;
+
+  if (gdbpy_is_value_object (handle_obj))
+    {
+      TRY
+	{
+	  struct thread_info *thread_info;
+	  struct value *val = value_object_to_value (handle_obj);
+
+	  thread_info = find_thread_by_handle (val);
+	  if (thread_info != NULL)
+	    {
+	      result = (PyObject *) find_thread_object (thread_info->ptid);
+	      if (result)
+		Py_INCREF (result);
+	    }
+	}
+      CATCH (except, RETURN_MASK_ALL)
+	{
+	  if (except.reason < 0)
+	    gdbpy_convert_exception (except);
+	  return NULL;
+	}
+      END_CATCH
+    }
+
+  return result;
+}
+
 int
 gdbpy_initialize_thread (void)
 {
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 4dd413d..68c3a8e 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -258,6 +258,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
 {
@@ -385,6 +387,7 @@  PyObject *gdbpy_create_lazy_string_object (CORE_ADDR address, long length,
 PyObject *gdbpy_inferiors (PyObject *unused, PyObject *unused2);
 PyObject *gdbpy_create_ptid_object (ptid_t ptid);
 PyObject *gdbpy_selected_thread (PyObject *self, PyObject *args);
+PyObject *gdbpy_thread_from_thread_handle (PyObject *self, PyObject *args, PyObject *kw);
 PyObject *gdbpy_selected_inferior (PyObject *self, PyObject *args);
 PyObject *gdbpy_string_to_argv (PyObject *self, PyObject *args);
 PyObject *gdbpy_parameter_value (enum var_types type, void *var);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index a7aff53..972def2 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1993,6 +1993,10 @@  Arguments are separate by spaces and may be quoted."
   { "selected_thread", gdbpy_selected_thread, METH_NOARGS,
     "selected_thread () -> gdb.InferiorThread.\n\
 Return the selected thread object." },
+  { "thread_from_thread_handle", (PyCFunction) gdbpy_thread_from_thread_handle,
+     METH_VARARGS | METH_KEYWORDS,
+     "thread_from_thread_handle (handle) -> gdb.InferiorThread.\n\
+Return thread object corresponding to thread handle." },
   { "selected_inferior", gdbpy_selected_inferior, METH_NOARGS,
     "selected_inferior () -> gdb.Inferior.\n\
 Return the selected inferior object." },