From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15572 invoked by alias); 27 Jun 2011 17:35:00 -0000 Received: (qmail 15251 invoked by uid 22791); 27 Jun 2011 17:34:57 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,T_TO_NO_BRKTS_FREEMAIL X-Spam-Check-By: sourceware.org Received: from mail-vx0-f169.google.com (HELO mail-vx0-f169.google.com) (209.85.220.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 27 Jun 2011 17:34:25 +0000 Received: by vxg38 with SMTP id 38so4501384vxg.0 for ; Mon, 27 Jun 2011 10:34:24 -0700 (PDT) Received: by 10.220.5.210 with SMTP id 18mr2462991vcw.83.1309196064134; Mon, 27 Jun 2011 10:34:24 -0700 (PDT) MIME-Version: 1.0 Received: by 10.220.188.75 with HTTP; Mon, 27 Jun 2011 10:34:04 -0700 (PDT) In-Reply-To: References: From: Kevin Pouget Date: Mon, 27 Jun 2011 17:35:00 -0000 Message-ID: Subject: Re: [patch] [python] Implement Inferior.current_inferior To: pmuldoon@redhat.com Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-06/txt/msg00406.txt.bz2 Hi, I actually proposed and got approved a patch to solve this bug two months a= go, 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 wrote: > > This patch implements the current_inferior function for the Python > Inferior class. =A0It also fixes a bug that I found along the way. =A0I > 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. =A0The above > statement creates a very short-lived reference to a Python object. =A0In > doing so, as with all inferiors, we register py_cleanup_inferior with > set_inferior_data. =A0However 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. =A0In 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. =A0Should I call clear_inferior_data there? =A0I looked at th= at > function and it appears to clear out all of the inferior data, which is > something we do not want. =A0I instead opted to set the inferior data > matched to the key to NULL. =A0I 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. =A0Previously that function could > return a new reference or a borrowed reference and the caller would > have to incref/decref as needed. =A0The 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). =A0This in > particular was a problem for gdb.current_inferior. > > Cheers, > > Phil > > -- > > 2011-06-27 =A0Phil Muldoon =A0 > > =A0 =A0 =A0 =A0PR/Python 12692 > > =A0 =A0 =A0 =A0* python/py-inferior.c (gdbpy_current_inferior): New funct= ion. > =A0 =A0 =A0 =A0(infpy_dealloc): Ditto. > =A0 =A0 =A0 =A0(inferior_to_inferior_object): Return a new object, or a > =A0 =A0 =A0 =A0new reference to the existing object. > =A0 =A0 =A0 =A0(find_thread_object): Cleanup references to inferior. > =A0 =A0 =A0 =A0(delete_thread_object): Ditto. > =A0 =A0 =A0 =A0* python/py-infthread.c (create_thread_object): Do not inc= rement > =A0 =A0 =A0 =A0inferior reference count. > =A0 =A0 =A0 =A0* python/python.c (inferior_object_type): Register current > =A0 =A0 =A0 =A0inferior function. > =A0 =A0 =A0 =A0* python/python-internal.h: Define gdbpy_current_inferior. > > 2011-06-27 =A0Phil Muldoon > > =A0 =A0 =A0 =A0* gdb.texinfo (Inferiors In Python): Document current_infe= rior > =A0 =A0 =A0 =A0function. > > 2011-06-27 =A0Phil Muldoon =A0 > > =A0 =A0 =A0 =A0* 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: > =A0Return a tuple containing all inferior objects. > =A0@end defun > > +@defun current_inferior > +Return a @code{gdb.Inferior} object containing the current inferior > +selected in @value{GDBN}. > +@end defun > + > =A0A @code{gdb.Inferior} object has the following attributes: > > =A0@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) > =A0 do_cleanups (cleanup); > =A0} > > -/* Return a borrowed reference to the Python object of type Inferior > +/* Return a reference to the Python object of type Inferior > =A0 =A0representing INFERIOR. =A0If the object has already been created, > - =A0 return it, =A0otherwise, create it. =A0Return NULL on failure. =A0*/ > + =A0 return it and increment the reference count, =A0otherwise, create i= t. > + =A0 Return NULL on failure. =A0*/ > =A0PyObject * > =A0inferior_to_inferior_object (struct inferior *inferior) > =A0{ > @@ -154,13 +155,14 @@ inferior_to_inferior_object (struct inferior *infer= ior) > > =A0 =A0 =A0 do_cleanups (cleanup); > =A0 =A0 } > + =A0else > + =A0 =A0Py_INCREF ((PyObject *)inf_obj); > > =A0 return (PyObject *) inf_obj; > =A0} > > =A0/* Finds the Python Inferior object for the given PID. =A0Returns a > - =A0 borrowed reference, or NULL if PID does not match any inferior > - =A0 object. =A0*/ > + =A0 reference, or NULL if PID does not match any inferior object. */ > > =A0PyObject * > =A0find_inferior_object (int pid) > @@ -180,6 +182,7 @@ find_thread_object (ptid_t ptid) > =A0 int pid; > =A0 struct threadlist_entry *thread; > =A0 PyObject *inf_obj; > + =A0thread_object *found =3D NULL; > > =A0 pid =3D PIDGET (ptid); > =A0 if (pid =3D=3D 0) > @@ -187,11 +190,21 @@ find_thread_object (ptid_t ptid) > > =A0 inf_obj =3D find_inferior_object (pid); > > - =A0if (inf_obj) > - =A0 =A0for (thread =3D ((inferior_object *)inf_obj)->threads; thread; > - =A0 =A0 =A0 =A0thread =3D thread->next) > - =A0 =A0 =A0if (ptid_equal (thread->thread_obj->thread->ptid, ptid)) > - =A0 =A0 =A0 return thread->thread_obj; > + =A0if (! inf_obj) > + =A0 =A0return NULL; > + > + =A0for (thread =3D ((inferior_object *)inf_obj)->threads; thread; > + =A0 =A0 =A0 thread =3D thread->next) > + =A0 =A0if (ptid_equal (thread->thread_obj->thread->ptid, ptid)) > + =A0 =A0 =A0{ > + =A0 =A0 =A0 found =3D thread->thread_obj; > + =A0 =A0 =A0 break; > + =A0 =A0 =A0} > + > + =A0Py_DECREF (inf_obj); > + > + =A0if (found) > + =A0 =A0return found; > > =A0 return NULL; > =A0} > @@ -245,7 +258,10 @@ delete_thread_object (struct thread_info *tp, int ig= nore) > =A0 =A0 =A0 break; > > =A0 if (!*entry) > - =A0 =A0return; > + =A0 =A0{ > + =A0 =A0 =A0Py_DECREF (inf_obj); > + =A0 =A0 =A0return; > + =A0 =A0} > > =A0 cleanup =3D ensure_python_env (python_gdbarch, python_language); > > @@ -256,6 +272,7 @@ delete_thread_object (struct thread_info *tp, int ign= ore) > =A0 inf_obj->nthreads--; > > =A0 Py_DECREF (tmp->thread_obj); > + =A0Py_DECREF (inf_obj); > =A0 xfree (tmp); > > =A0 do_cleanups (cleanup); > @@ -321,8 +338,15 @@ build_inferior_list (struct inferior *inf, void *arg) > =A0{ > =A0 PyObject *list =3D arg; > =A0 PyObject *inferior =3D inferior_to_inferior_object (inf); > + =A0int success =3D 0; > > - =A0if (PyList_Append (list, inferior)) > + =A0if (! inferior) > + =A0 =A0return 0; > + > + =A0success =3D PyList_Append (list, inferior); > + =A0Py_DECREF (inferior); > + > + =A0if (success) > =A0 =A0 return 1; > > =A0 return 0; > @@ -350,6 +374,22 @@ gdbpy_inferiors (PyObject *unused, PyObject *unused2) > =A0 return PyList_AsTuple (list); > =A0} > > +/* Returns the current inferior in GDB as a Gdb.Inferior object. =A0*/ > +PyObject * > +gdbpy_current_inferior (PyObject *self, PyObject *args) > +{ > + =A0struct inferior *inf =3D current_inferior (); > + =A0PyObject *inferior =3D NULL; > + > + =A0/* There should always be an inferior entry, even when there are no > + =A0 =A0 inferiors loaded into GDB. =A0*/ > + =A0gdb_assert (inf !=3D NULL); > + > + =A0inferior =3D inferior_to_inferior_object (inf); > + > + =A0return inferior; > +} > + > =A0/* Membuf and memory manipulation. =A0*/ > > =A0/* Implementation of gdb.read_memory (address, length). > @@ -617,6 +657,17 @@ infpy_is_valid (PyObject *self, PyObject *args) > =A0 Py_RETURN_TRUE; > =A0} > > +static void > +infpy_dealloc (PyObject *obj) > +{ > + =A0inferior_object *inf_obj =3D (inferior_object *) obj; > + =A0struct inferior *inf =3D inf_obj->inferior; > + > + =A0if (! inf) > + =A0 =A0return; > + > + =A0set_inferior_data (inf, infpy_inf_data_key, NULL); > +} > > =A0/* Clear the INFERIOR pointer in an Inferior object and clear the > =A0 =A0thread list. =A0*/ > @@ -714,7 +765,7 @@ static PyTypeObject inferior_object_type =3D > =A0 "gdb.Inferior", =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* tp_name */ > =A0 sizeof (inferior_object), =A0 =A0 =A0/* tp_basicsize */ > =A0 0, =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* tp_item= size */ > - =A00, =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* tp_dea= lloc */ > + =A0infpy_dealloc, =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* tp_dealloc */ > =A0 0, =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* tp_prin= t */ > =A0 0, =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* tp_geta= ttr */ > =A0 0, =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* tp_seta= ttr */ > 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) > > =A0 thread_obj->thread =3D tp; > =A0 thread_obj->inf_obj =3D find_inferior_object (PIDGET (tp->ptid)); > - =A0Py_INCREF (thread_obj->inf_obj); > > =A0 return thread_obj; > =A0} > 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, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 const char *encoding, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 struct type *type); > =A0PyObject *gdbpy_inferiors (PyObject *unused, PyObject *unused2); > +PyObject *gdbpy_current_inferior (PyObject *self, PyObject *args); > =A0PyObject *gdbpy_selected_thread (PyObject *self, PyObject *args); > =A0PyObject *gdbpy_string_to_argv (PyObject *self, PyObject *args); > =A0PyObject *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." }, > =A0 { "inferiors", gdbpy_inferiors, METH_NOARGS, > =A0 =A0 "inferiors () -> (gdb.Inferior, ...).\n\ > =A0Return a tuple containing all inferiors." }, > + =A0{ "current_inferior", gdbpy_current_inferior, METH_NOARGS, > + =A0 =A0"current_inferior () -> gdb.Inferior.\n\ > +Return the current GDB inferior object." }, > + > =A0 {NULL, NULL, 0, NULL} > =A0}; > > 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()" "Fa= lse" \ > =A0 =A0 =A0 =A0 =A0"Check inferior validity" > =A0gdb_test "python print inf_list\[1\].is_valid()" "True" \ > =A0 =A0 =A0 =A0 =A0"Check inferior validity" > + > +# Test current_inferior () functionality. > +# Start with a fresh gdb. > +clean_restart ${testfile} > +if ![runto_main] then { > + =A0 =A0fail "Can't run to main" > + =A0 =A0return 0 > +} > + > +gdb_test_no_output "python i =3D gdb.current_inferior()" \ > + =A0 =A0"Check the current inferior" > + > +gdb_test "python print i" \ > + =A0 =A0"" \ > + =A0 =A0"Verify inferior from current_inferior" > +gdb_test "python print 'result =3D', i.num" " =3D \[0-9\]+" \ > + =A0 =A0"Test current_inferior Inferior.num" > +gdb_test "python print 'result =3D', i.pid" " =3D \[0-9\]+" \ > + =A0 =A0"Test current_inferior Inferior.pid" > +gdb_test_no_output "python t =3D i.threads()" \ > + =A0 =A0"Allocate threads from the current_inferior" > +gdb_test "python print len(t)" "1" \ > + =A0 =A0"There should only be one thread from current_inferior" > +gdb_test "python print t\[0\].name" ".*py-inferior.*" \ > + =A0 =A0"The name of the current_inferior thread" > + > +clean_restart ${testfile} > + > +# Test empty inferior > +gdb_test_no_output "python ie =3D gdb.current_inferior()" \ > + =A0 =A0"Check the current empty inferior" > + > +gdb_test "python print ie" \ > + =A0 =A0"" \ > + =A0 =A0"Verify empty inferior from current_inferior" > +gdb_test "python print 'result =3D', ie.num" " =3D 1" \ > + =A0 =A0"Test current_inferior Inferior.num" > +gdb_test "python print 'result =3D', ie.pid" " =3D 0" \ > + =A0 =A0"Test current_inferior Inferior.pid" > +gdb_test_no_output "python te =3D ie.threads()" \ > + =A0 =A0"Allocate threads from the current_inferior" > +gdb_test "python print len(te)" "0" \ > + =A0 =A0"There should not be any threads from empty inferior" >