This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] [python] Implement Inferior.current_inferior


Hi,

I actually proposed and got approved a patch to solve this bug two months ago,

http://sources.redhat.com/ml/gdb-patches/2011-04/msg00389.html

but I'm still waiting for these crazy copyright assignment papers to
cross the ocean, it looks like that they won't never reach the old
continent, too bad ...

cheers,

Kevin

On Mon, Jun 27, 2011 at 4:10 PM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>
> This patch implements the current_inferior function for the Python
> Inferior class. ?It also fixes a bug that I found along the way. ?I
> did not break out the bug into a separate patch as it is related to
> the current inferior functionality.
>
> The bug, in brief:
>
> In the case of: python print gdb.selected_inferior()
>
> When GDB does not actually have an inferior loaded into GDB, this causes
> a bug where we do not clear the inferior cleanup data. ?The above
> statement creates a very short-lived reference to a Python object. ?In
> doing so, as with all inferiors, we register py_cleanup_inferior with
> set_inferior_data. ?However as this object is deallocated soon after,
> but the empty GDB inferior is still around, that old registration still
> exists and points to a now deallocated object. ?In this case, I simply
> added a Python deconstructor that clears that data.
>
> A point I am not sure on is the call to set_inferior_data in the
> destructor. ?Should I call clear_inferior_data there? ?I looked at that
> function and it appears to clear out all of the inferior data, which is
> something we do not want. ?I instead opted to set the inferior data
> matched to the key to NULL. ?I am not sure if this is correct.
>
> I've also had to change the behaviour of inferior_to_inferior_object to
> always return a new reference, or NULL. ?Previously that function could
> return a new reference or a borrowed reference and the caller would
> have to incref/decref as needed. ?The problem with this approach is that
> the caller does not know if the reference is new (do not increment
> the reference), or borrowed (increment the reference). ?This in
> particular was a problem for gdb.current_inferior.
>
> Cheers,
>
> Phil
>
> --
>
> 2011-06-27 ?Phil Muldoon ?<pmuldoon@redhat.com>
>
> ? ? ? ?PR/Python 12692
>
> ? ? ? ?* python/py-inferior.c (gdbpy_current_inferior): New function.
> ? ? ? ?(infpy_dealloc): Ditto.
> ? ? ? ?(inferior_to_inferior_object): Return a new object, or a
> ? ? ? ?new reference to the existing object.
> ? ? ? ?(find_thread_object): Cleanup references to inferior.
> ? ? ? ?(delete_thread_object): Ditto.
> ? ? ? ?* python/py-infthread.c (create_thread_object): Do not increment
> ? ? ? ?inferior reference count.
> ? ? ? ?* python/python.c (inferior_object_type): Register current
> ? ? ? ?inferior function.
> ? ? ? ?* python/python-internal.h: Define gdbpy_current_inferior.
>
> 2011-06-27 ?Phil Muldoon <pmuldoon@redhat.com>
>
> ? ? ? ?* gdb.texinfo (Inferiors In Python): Document current_inferior
> ? ? ? ?function.
>
> 2011-06-27 ?Phil Muldoon ?<pmuldoon@redhat.com>
>
> ? ? ? ?* gdb.python/py-inferior.exp: Add current_inferior tests.
>
> --
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 3a705c2..343fd7c 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -21988,6 +21988,11 @@ module:
> ?Return a tuple containing all inferior objects.
> ?@end defun
>
> +@defun current_inferior
> +Return a @code{gdb.Inferior} object containing the current inferior
> +selected in @value{GDBN}.
> +@end defun
> +
> ?A @code{gdb.Inferior} object has the following attributes:
>
> ?@table @code
> diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
> index f226501..92b28b7 100644
> --- a/gdb/python/py-inferior.c
> +++ b/gdb/python/py-inferior.c
> @@ -125,9 +125,10 @@ python_inferior_exit (struct inferior *inf)
> ? do_cleanups (cleanup);
> ?}
>
> -/* Return a borrowed reference to the Python object of type Inferior
> +/* Return a reference to the Python object of type Inferior
> ? ?representing INFERIOR. ?If the object has already been created,
> - ? return it, ?otherwise, create it. ?Return NULL on failure. ?*/
> + ? return it and increment the reference count, ?otherwise, create it.
> + ? Return NULL on failure. ?*/
> ?PyObject *
> ?inferior_to_inferior_object (struct inferior *inferior)
> ?{
> @@ -154,13 +155,14 @@ inferior_to_inferior_object (struct inferior *inferior)
>
> ? ? ? do_cleanups (cleanup);
> ? ? }
> + ?else
> + ? ?Py_INCREF ((PyObject *)inf_obj);
>
> ? return (PyObject *) inf_obj;
> ?}
>
> ?/* Finds the Python Inferior object for the given PID. ?Returns a
> - ? borrowed reference, or NULL if PID does not match any inferior
> - ? object. ?*/
> + ? reference, or NULL if PID does not match any inferior object. */
>
> ?PyObject *
> ?find_inferior_object (int pid)
> @@ -180,6 +182,7 @@ find_thread_object (ptid_t ptid)
> ? int pid;
> ? struct threadlist_entry *thread;
> ? PyObject *inf_obj;
> + ?thread_object *found = NULL;
>
> ? pid = PIDGET (ptid);
> ? if (pid == 0)
> @@ -187,11 +190,21 @@ find_thread_object (ptid_t ptid)
>
> ? inf_obj = find_inferior_object (pid);
>
> - ?if (inf_obj)
> - ? ?for (thread = ((inferior_object *)inf_obj)->threads; thread;
> - ? ? ? ?thread = thread->next)
> - ? ? ?if (ptid_equal (thread->thread_obj->thread->ptid, ptid))
> - ? ? ? return thread->thread_obj;
> + ?if (! inf_obj)
> + ? ?return NULL;
> +
> + ?for (thread = ((inferior_object *)inf_obj)->threads; thread;
> + ? ? ? thread = thread->next)
> + ? ?if (ptid_equal (thread->thread_obj->thread->ptid, ptid))
> + ? ? ?{
> + ? ? ? found = thread->thread_obj;
> + ? ? ? break;
> + ? ? ?}
> +
> + ?Py_DECREF (inf_obj);
> +
> + ?if (found)
> + ? ?return found;
>
> ? return NULL;
> ?}
> @@ -245,7 +258,10 @@ delete_thread_object (struct thread_info *tp, int ignore)
> ? ? ? break;
>
> ? if (!*entry)
> - ? ?return;
> + ? ?{
> + ? ? ?Py_DECREF (inf_obj);
> + ? ? ?return;
> + ? ?}
>
> ? cleanup = ensure_python_env (python_gdbarch, python_language);
>
> @@ -256,6 +272,7 @@ delete_thread_object (struct thread_info *tp, int ignore)
> ? inf_obj->nthreads--;
>
> ? Py_DECREF (tmp->thread_obj);
> + ?Py_DECREF (inf_obj);
> ? xfree (tmp);
>
> ? do_cleanups (cleanup);
> @@ -321,8 +338,15 @@ build_inferior_list (struct inferior *inf, void *arg)
> ?{
> ? PyObject *list = arg;
> ? PyObject *inferior = inferior_to_inferior_object (inf);
> + ?int success = 0;
>
> - ?if (PyList_Append (list, inferior))
> + ?if (! inferior)
> + ? ?return 0;
> +
> + ?success = PyList_Append (list, inferior);
> + ?Py_DECREF (inferior);
> +
> + ?if (success)
> ? ? return 1;
>
> ? return 0;
> @@ -350,6 +374,22 @@ gdbpy_inferiors (PyObject *unused, PyObject *unused2)
> ? return PyList_AsTuple (list);
> ?}
>
> +/* Returns the current inferior in GDB as a Gdb.Inferior object. ?*/
> +PyObject *
> +gdbpy_current_inferior (PyObject *self, PyObject *args)
> +{
> + ?struct inferior *inf = current_inferior ();
> + ?PyObject *inferior = NULL;
> +
> + ?/* There should always be an inferior entry, even when there are no
> + ? ? inferiors loaded into GDB. ?*/
> + ?gdb_assert (inf != NULL);
> +
> + ?inferior = inferior_to_inferior_object (inf);
> +
> + ?return inferior;
> +}
> +
> ?/* Membuf and memory manipulation. ?*/
>
> ?/* Implementation of gdb.read_memory (address, length).
> @@ -617,6 +657,17 @@ infpy_is_valid (PyObject *self, PyObject *args)
> ? Py_RETURN_TRUE;
> ?}
>
> +static void
> +infpy_dealloc (PyObject *obj)
> +{
> + ?inferior_object *inf_obj = (inferior_object *) obj;
> + ?struct inferior *inf = inf_obj->inferior;
> +
> + ?if (! inf)
> + ? ?return;
> +
> + ?set_inferior_data (inf, infpy_inf_data_key, NULL);
> +}
>
> ?/* Clear the INFERIOR pointer in an Inferior object and clear the
> ? ?thread list. ?*/
> @@ -714,7 +765,7 @@ static PyTypeObject inferior_object_type =
> ? "gdb.Inferior", ? ? ? ? ? ? ? ?/* tp_name */
> ? sizeof (inferior_object), ? ? ?/* tp_basicsize */
> ? 0, ? ? ? ? ? ? ? ? ? ? ? ? ? ? /* tp_itemsize */
> - ?0, ? ? ? ? ? ? ? ? ? ? ? ? ? ? /* tp_dealloc */
> + ?infpy_dealloc, ? ? ? ? ? ? ? ? /* tp_dealloc */
> ? 0, ? ? ? ? ? ? ? ? ? ? ? ? ? ? /* tp_print */
> ? 0, ? ? ? ? ? ? ? ? ? ? ? ? ? ? /* tp_getattr */
> ? 0, ? ? ? ? ? ? ? ? ? ? ? ? ? ? /* tp_setattr */
> diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
> index b37c53c..cb714c7 100644
> --- a/gdb/python/py-infthread.c
> +++ b/gdb/python/py-infthread.c
> @@ -49,7 +49,6 @@ create_thread_object (struct thread_info *tp)
>
> ? thread_obj->thread = tp;
> ? thread_obj->inf_obj = find_inferior_object (PIDGET (tp->ptid));
> - ?Py_INCREF (thread_obj->inf_obj);
>
> ? return thread_obj;
> ?}
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> index b65109d..ab83a47 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -147,6 +147,7 @@ PyObject *gdbpy_create_lazy_string_object (CORE_ADDR address, long length,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const char *encoding,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct type *type);
> ?PyObject *gdbpy_inferiors (PyObject *unused, PyObject *unused2);
> +PyObject *gdbpy_current_inferior (PyObject *self, PyObject *args);
> ?PyObject *gdbpy_selected_thread (PyObject *self, PyObject *args);
> ?PyObject *gdbpy_string_to_argv (PyObject *self, PyObject *args);
> ?PyObject *gdbpy_parameter (PyObject *self, PyObject *args);
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index ddfe9ba..787b0ff 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -1283,6 +1283,10 @@ Return the selected thread object." },
> ? { "inferiors", gdbpy_inferiors, METH_NOARGS,
> ? ? "inferiors () -> (gdb.Inferior, ...).\n\
> ?Return a tuple containing all inferiors." },
> + ?{ "current_inferior", gdbpy_current_inferior, METH_NOARGS,
> + ? ?"current_inferior () -> gdb.Inferior.\n\
> +Return the current GDB inferior object." },
> +
> ? {NULL, NULL, 0, NULL}
> ?};
>
> diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
> index b853c79..51b31c9 100644
> --- a/gdb/testsuite/gdb.python/py-inferior.exp
> +++ b/gdb/testsuite/gdb.python/py-inferior.exp
> @@ -211,3 +211,46 @@ gdb_test "python print inf_list\[0\].is_valid()" "False" \
> ? ? ? ? ?"Check inferior validity"
> ?gdb_test "python print inf_list\[1\].is_valid()" "True" \
> ? ? ? ? ?"Check inferior validity"
> +
> +# Test current_inferior () functionality.
> +# Start with a fresh gdb.
> +clean_restart ${testfile}
> +if ![runto_main] then {
> + ? ?fail "Can't run to main"
> + ? ?return 0
> +}
> +
> +gdb_test_no_output "python i = gdb.current_inferior()" \
> + ? ?"Check the current inferior"
> +
> +gdb_test "python print i" \
> + ? ?"<gdb.Inferior object at 0x\[\[:xdigit:\]\]+>" \
> + ? ?"Verify inferior from current_inferior"
> +gdb_test "python print 'result =', i.num" " = \[0-9\]+" \
> + ? ?"Test current_inferior Inferior.num"
> +gdb_test "python print 'result =', i.pid" " = \[0-9\]+" \
> + ? ?"Test current_inferior Inferior.pid"
> +gdb_test_no_output "python t = i.threads()" \
> + ? ?"Allocate threads from the current_inferior"
> +gdb_test "python print len(t)" "1" \
> + ? ?"There should only be one thread from current_inferior"
> +gdb_test "python print t\[0\].name" ".*py-inferior.*" \
> + ? ?"The name of the current_inferior thread"
> +
> +clean_restart ${testfile}
> +
> +# Test empty inferior
> +gdb_test_no_output "python ie = gdb.current_inferior()" \
> + ? ?"Check the current empty inferior"
> +
> +gdb_test "python print ie" \
> + ? ?"<gdb.Inferior object at 0x\[\[:xdigit:\]\]+>" \
> + ? ?"Verify empty inferior from current_inferior"
> +gdb_test "python print 'result =', ie.num" " = 1" \
> + ? ?"Test current_inferior Inferior.num"
> +gdb_test "python print 'result =', ie.pid" " = 0" \
> + ? ?"Test current_inferior Inferior.pid"
> +gdb_test_no_output "python te = ie.threads()" \
> + ? ?"Allocate threads from the current_inferior"
> +gdb_test "python print len(te)" "0" \
> + ? ?"There should not be any threads from empty inferior"
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]