[2/3] python: Add Progspace.objfiles method
Commit Message
This patch adds an objfiles method, which returns a sequence of the
objfiles associated to that program space. I chose a method rather than
a property for symmetry with gdb.objfiles().
Question:
When we try to access a property of an Inferior object that has
become invalid, for example, we raise an exception ("Inferior no longer
exists."). When doing the same with a Progspace object, we return None
(the only case for now is its filename property). For
Progspace.objfiles(), I made it return None too, but perhaps it should
throw an exception instead? Especially that None is not iterable, so
trying to do:
for obj in pspace.objfiles():
...
will fail horribly if we return None... so should I introduce a macro
similar to INFPY_REQUIRE_VALID?
gdb/ChangeLog:
* python/py-progspace.c (pspy_get_objfiles): New function.
(progspace_object_methods): New.
(pspace_object_type): Add tp_methods callback.
* python/python-internal.h (build_objfiles_list): New
declaration.
* python/python.c (build_objfiles_list): New function.
(gdbpy_objfiles): Implement using build_objfiles_list.
* NEWS: Mention the Progspace.objfiles method.
gdb/doc/ChangeLog:
* python.texi (Program Spaces In Python): Document the
Progspace.objfiles method.
gdb/testsuite/ChangeLog:
* gdb.python/py-progspace.exp: Test the Progspace.objfiles
method.
---
gdb/NEWS | 3 +++
gdb/doc/python.texi | 6 +++++
gdb/python/py-progspace.c | 23 +++++++++++++++--
gdb/python/python-internal.h | 4 +++
gdb/python/python.c | 28 +++++++++++++--------
gdb/testsuite/gdb.python/py-progspace.exp | 30 +++++++++++++++++++++++
6 files changed, 82 insertions(+), 12 deletions(-)
Comments
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> Question:
Simon> When we try to access a property of an Inferior object that has
Simon> become invalid, for example, we raise an exception ("Inferior no longer
Simon> exists."). When doing the same with a Progspace object, we return None
Simon> (the only case for now is its filename property). For
Simon> Progspace.objfiles(), I made it return None too, but perhaps it should
Simon> throw an exception instead? Especially that None is not iterable, so
Simon> trying to do:
Simon> for obj in pspace.objfiles():
Simon> ...
Simon> will fail horribly if we return None... so should I introduce a macro
Simon> similar to INFPY_REQUIRE_VALID?
There are two approaches to modeling gdb objects in the Python layer.
One is taken by objects like Value whose lifetime can be arbitrarily
"extended". For these objects, Python simply holds a reference to the
underlying gdb object.
The other approach is for objects whose lifetime can be controlled by
the user or other external (to Python) events. For example, a
breakpoint can be deleted by the user, leaving behind the gdb.Breakpoint
representation.
For these we have generally had the Python object keep a sort of weak
reference to the gdb object; when the gdb object is destroyed, the
Python wrapper enters a special invalid state. These objects have an
is_valid method; and generally all other methods and attributes throw an
exception if the object is invalid -- but I think this is not a
hard-and-fast rule and can be broken where there is an obvious decent
non-exception result.
In sum I think INFPY_REQUIRE_VALID is fine if you happen to need it at
some spot in the inferior wrapper. I wouldn't go out of my way to avoid
it.
Normally Python code has to know not to work with an invalid object (and
anyway why would it want to); but I think in this case, I would be fine
with objfiles() returning an empty sequence. That is what my old patch
did, I would assume intentionally, though I don't recall.
Finally, I have another ancient and unfinished series that adds a bunch
of methods to Inferior. If you're working in this area I can send it;
I'd be happy to rebase it.
Tom
On 2018-09-12 18:01, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
> Simon> Question:
>
> Simon> When we try to access a property of an Inferior object that has
> Simon> become invalid, for example, we raise an exception ("Inferior no
> longer
> Simon> exists."). When doing the same with a Progspace object, we
> return None
> Simon> (the only case for now is its filename property). For
> Simon> Progspace.objfiles(), I made it return None too, but perhaps it
> should
> Simon> throw an exception instead? Especially that None is not
> iterable, so
> Simon> trying to do:
>
> Simon> for obj in pspace.objfiles():
> Simon> ...
>
> Simon> will fail horribly if we return None... so should I introduce a
> macro
> Simon> similar to INFPY_REQUIRE_VALID?
>
> There are two approaches to modeling gdb objects in the Python layer.
>
> One is taken by objects like Value whose lifetime can be arbitrarily
> "extended". For these objects, Python simply holds a reference to the
> underlying gdb object.
>
> The other approach is for objects whose lifetime can be controlled by
> the user or other external (to Python) events. For example, a
> breakpoint can be deleted by the user, leaving behind the
> gdb.Breakpoint
> representation.
>
> For these we have generally had the Python object keep a sort of weak
> reference to the gdb object; when the gdb object is destroyed, the
> Python wrapper enters a special invalid state. These objects have an
> is_valid method; and generally all other methods and attributes throw
> an
> exception if the object is invalid -- but I think this is not a
> hard-and-fast rule and can be broken where there is an obvious decent
> non-exception result.
>
> In sum I think INFPY_REQUIRE_VALID is fine if you happen to need it at
> some spot in the inferior wrapper. I wouldn't go out of my way to
> avoid
> it.
This is already how the progspace object works (the weak reference
thing), and I see you've added a PSPY_REQUIRE_VALID in your old patch.
> Normally Python code has to know not to work with an invalid object
> (and
> anyway why would it want to); but I think in this case, I would be fine
> with objfiles() returning an empty sequence. That is what my old patch
> did, I would assume intentionally, though I don't recall.
For consistency with the rest of the API, I would lean towards an
exception. I will update the patch to do that. The existing "filename"
property returns None in that case, unfortunately.
> Finally, I have another ancient and unfinished series that adds a bunch
> of methods to Inferior. If you're working in this area I can send it;
> I'd be happy to rebase it.
Sure!
Thanks,
Simon
> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: Simon Marchi <simon.marchi@ericsson.com>
> Date: Wed, 12 Sep 2018 15:36:16 -0400
>
> This patch adds an objfiles method, which returns a sequence of the
> objfiles associated to that program space. I chose a method rather than
> a property for symmetry with gdb.objfiles().
>
> Question:
>
> When we try to access a property of an Inferior object that has
> become invalid, for example, we raise an exception ("Inferior no longer
> exists."). When doing the same with a Progspace object, we return None
> (the only case for now is its filename property). For
> Progspace.objfiles(), I made it return None too, but perhaps it should
> throw an exception instead? Especially that None is not iterable, so
> trying to do:
>
> for obj in pspace.objfiles():
> ...
>
> will fail horribly if we return None... so should I introduce a macro
> similar to INFPY_REQUIRE_VALID?
>
> gdb/ChangeLog:
>
> * python/py-progspace.c (pspy_get_objfiles): New function.
> (progspace_object_methods): New.
> (pspace_object_type): Add tp_methods callback.
> * python/python-internal.h (build_objfiles_list): New
> declaration.
> * python/python.c (build_objfiles_list): New function.
> (gdbpy_objfiles): Implement using build_objfiles_list.
> * NEWS: Mention the Progspace.objfiles method.
>
> gdb/doc/ChangeLog:
>
> * python.texi (Program Spaces In Python): Document the
> Progspace.objfiles method.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.python/py-progspace.exp: Test the Progspace.objfiles
> method.
OK for the documentation parts.
Thanks.
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
>> Finally, I have another ancient and unfinished series that adds a bunch
>> of methods to Inferior. If you're working in this area I can send it;
>> I'd be happy to rebase it.
Simon> Sure!
I looked at this and it's in rougher shape. See
submit/python/inferior-additions on github. So maybe it can be mined as
a source for some things but it isn't close to landing.
It adds these methods to Inferior: attach, continue, stop, kill, and select.
It also adds a constructor so you can make a new inferior.
However nowadays I tend to think that inferior-control things, like
stop, should return some kind of promise that resolves when the request
completes. I think the code on the branch, on the other hand, supposes
that your code will wait for an event after making a request. My
experience from JS is that the promise-based approach is much simpler to
program and reason about, and now that Python has async+await, I think
gdb should try to follow. (If possible, I haven't looked at how
extensions deal with this stuff. And I used the JS terms which are
probably different in Python.)
Maybe the constructor patch could be completed pretty easily though.
Tom
@@ -89,6 +89,9 @@ CSKY GNU/LINUX csky*-*-linux
** The program space associated to an inferior is now accessible through a
new "progspace" attribute of gdb.Inferior.
+ ** The gdb.Progspace type has a new objfiles method, which returns the list
+ of objfiles associated to that program space.
+
*** Changes in GDB 8.2
* The 'set disassembler-options' command now supports specifying options
@@ -4073,6 +4073,12 @@ Hello.
[Inferior 1 (process 4242) exited normally]
@end smallexample
+A @code{gdb.Progspace} object has the following methods:
+
+@defun Progspace.objfiles ()
+Return a sequence of all the objfiles associated to this program space (@pxref{Objfiles In Python}).
+@end defun
+
@node Objfiles In Python
@subsubsection Objfiles In Python
@@ -314,7 +314,19 @@ pspy_set_type_printers (PyObject *o, PyObject *value, void *ignore)
return 0;
}
-
+/* Implement the objfiles method. */
+
+PyObject *
+pspy_get_objfiles (PyObject *o, PyObject *unused2)
+{
+ pspace_object *self = (pspace_object *) o;
+
+ program_space *pspace = self->pspace;
+ if (pspace == nullptr)
+ Py_RETURN_NONE;
+
+ return build_objfiles_list (pspace).release ();
+}
/* Clear the PSPACE pointer in a Pspace object and remove the reference. */
@@ -397,6 +409,13 @@ static gdb_PyGetSetDef pspace_getset[] =
{ NULL }
};
+static PyMethodDef progspace_object_methods[] =
+{
+ { "objfiles", pspy_get_objfiles, METH_NOARGS,
+ "Return a sequence of objfiles associated to this program space." },
+ { NULL }
+};
+
PyTypeObject pspace_object_type =
{
PyVarObject_HEAD_INIT (NULL, 0)
@@ -426,7 +445,7 @@ PyTypeObject pspace_object_type =
0, /* tp_weaklistoffset */
0, /* tp_iter */
0, /* tp_iternext */
- 0, /* tp_methods */
+ progspace_object_methods, /* tp_methods */
0, /* tp_members */
pspace_getset, /* tp_getset */
0, /* tp_base */
@@ -549,6 +549,10 @@ struct symtab_and_line *sal_object_to_symtab_and_line (PyObject *obj);
struct frame_info *frame_object_to_frame_info (PyObject *frame_obj);
struct gdbarch *arch_object_to_gdbarch (PyObject *obj);
+/* Return a Python list containing an Objfile object for each objfile in
+ PSPACE. */
+gdbpy_ref<> build_objfiles_list (program_space *pspace);
+
void gdbpy_initialize_gdb_readline (void);
int gdbpy_initialize_auto_load (void)
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
@@ -1437,10 +1437,10 @@ gdbpy_get_current_objfile (PyObject *unused1, PyObject *unused2)
return result;
}
-/* Return a sequence holding all the Objfiles. */
+/* See python-internal.h. */
-static PyObject *
-gdbpy_objfiles (PyObject *unused1, PyObject *unused2)
+gdbpy_ref<>
+build_objfiles_list (program_space *pspace)
{
struct objfile *objf;
@@ -1448,15 +1448,23 @@ gdbpy_objfiles (PyObject *unused1, PyObject *unused2)
if (list == NULL)
return NULL;
- ALL_OBJFILES (objf)
- {
- PyObject *item = objfile_to_objfile_object (objf);
+ ALL_PSPACE_OBJFILES (pspace, objf)
+ {
+ PyObject *item = objfile_to_objfile_object (objf);
- if (!item || PyList_Append (list.get (), item) == -1)
- return NULL;
- }
+ if (item == nullptr || PyList_Append (list.get (), item) == -1)
+ return NULL;
+ }
- return list.release ();
+ return list;
+}
+
+/* Return a sequence holding all the Objfiles. */
+
+static PyObject *
+gdbpy_objfiles (PyObject *unused1, PyObject *unused2)
+{
+ return build_objfiles_list (current_program_space).release ();
}
/* Compute the list of active python type printers and store them in
@@ -51,3 +51,33 @@ gdb_py_test_silent_cmd "python progspace.random_attribute = 42" \
"Set random attribute in progspace" 1
gdb_test "python print (progspace.random_attribute)" "42" \
"Verify set of random attribute in progspace"
+
+if {![runto_main]} {
+ fail "can't run to main"
+ return
+}
+
+# With a single inferior, progspace.objfiles () and gdb.objfiles () should
+# be identical.
+gdb_test "python print (progspace.objfiles () == gdb.objfiles ())" "True"
+
+gdb_test "add-inferior"
+gdb_test "inferior 2"
+
+gdb_load ${binfile}
+
+# With a second (non-started) inferior, we should have a single objfile - the
+# main one.
+gdb_test "python print (len (gdb.objfiles ())) == 1"
+
+# And the gdb.objfiles() list should now be different from the objfiles of the
+# prog space of inferior 1.
+gdb_test "python print (progspace.objfiles () != gdb.objfiles ())" "True"
+
+# Delete inferior 2 (and therefore the second progspace), check that the Python
+# object reacts sensibly.
+gdb_py_test_silent_cmd "python progspace2 = gdb.current_progspace()" \
+ "save progspace 2" 1
+gdb_test "inferior 1" "Switching to inferior 1.*"
+gdb_test_no_output "remove-inferiors 2"
+gdb_test "python print (progspace2.objfiles ())" "None"