public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 3/3] python: Fix erroneous doc about gdb.objfiles()
  2018-09-12 19:37 [PATCH 1/3] python: Add Inferior.progspace property Simon Marchi
@ 2018-09-12 19:36 ` Simon Marchi
  2018-09-12 21:46   ` Tom Tromey
  2018-09-13  2:37   ` Eli Zaretskii
  2018-09-12 19:37 ` [PATCH 2/3] python: Add Progspace.objfiles method Simon Marchi
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Simon Marchi @ 2018-09-12 19:36 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

The code implementing gdb.objfiles() returns a list of objfiles for the
current program space (the program space of the selected inferior).  The
documentation for the gdb.objfiles() Python method, however, states:

    Return a sequence of all the objfiles current known to GDB.

That sounds wrong to me.  I tried to phrase to be more precise.

gdb/doc/ChangeLog:

	* python.texi (Objfiles In Python): Update gdb.objfiles() doc.
---
 gdb/doc/python.texi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index abda135e17d..a9de6bd815e 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4104,8 +4104,9 @@ this function returns @code{None}.
 
 @findex gdb.objfiles
 @defun gdb.objfiles ()
-Return a sequence of all the objfiles current known to @value{GDBN}.
-@xref{Objfiles In Python}.
+Return a sequence of objfiles for the current program space (and by extension,
+for the selected inferior) @xref{Objfiles In Python} and
+@pxref{Progspaces In Python}.
 @end defun
 
 @findex gdb.lookup_objfile
-- 
2.19.0

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

* [PATCH 2/3] python: Add Progspace.objfiles method
  2018-09-12 19:37 [PATCH 1/3] python: Add Inferior.progspace property Simon Marchi
  2018-09-12 19:36 ` [PATCH 3/3] python: Fix erroneous doc about gdb.objfiles() Simon Marchi
@ 2018-09-12 19:37 ` Simon Marchi
  2018-09-12 22:01   ` Tom Tromey
  2018-09-13  2:38   ` Eli Zaretskii
  2018-09-12 21:42 ` [PATCH 1/3] python: Add Inferior.progspace property Tom Tromey
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Simon Marchi @ 2018-09-12 19:37 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

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

diff --git a/gdb/NEWS b/gdb/NEWS
index 21da6ae4ba8..f4d281eef0e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -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
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d4f03439f10..abda135e17d 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -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
 
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 3eaa466666d..64587ab16aa 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -314,7 +314,19 @@ pspy_set_type_printers (PyObject *o, PyObject *value, void *ignore)
   return 0;
 }
 
-\f
+/* 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 */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 3874fdc5e79..785ad171511 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -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;
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 6f798a0e461..371f4a57529 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -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
diff --git a/gdb/testsuite/gdb.python/py-progspace.exp b/gdb/testsuite/gdb.python/py-progspace.exp
index f0cafa834e0..53920c16fb0 100644
--- a/gdb/testsuite/gdb.python/py-progspace.exp
+++ b/gdb/testsuite/gdb.python/py-progspace.exp
@@ -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"
-- 
2.19.0

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

* [PATCH 1/3] python: Add Inferior.progspace property
@ 2018-09-12 19:37 Simon Marchi
  2018-09-12 19:36 ` [PATCH 3/3] python: Fix erroneous doc about gdb.objfiles() Simon Marchi
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Simon Marchi @ 2018-09-12 19:37 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patch adds a progspace property to the gdb.Inferior type, which
allows getting the gdb.Progspace object associated to that inferior.
In conjunction with the following patch, this will allow scripts iterate
on objfiles associated with a particular inferior.

While modifying py-inferior.exp, I added some checks for the other
Inferior properties, when the Inferior is no longer valid.  This doesn't
seem tested at the moment.

gdb/ChangeLog:

	* python/py-inferior.c (infpy_get_progspace): New function.
	(inferior_object_getset): Add progspace property.
	* NEWS: Mention the new property.

gdb/doc/ChangeLog:

	* python.texi (Inferiors In Python): Document
	Inferior.progspace.

gdb/testsuite/ChangeLog:

	* gdb.python/py-inferior.exp: Add tests for Inferior.progspace
	and a few other Inferior properties when the Inferior is no
	longer valid.
---
 gdb/NEWS                                 |  5 +++++
 gdb/doc/python.texi                      |  4 ++++
 gdb/python/py-inferior.c                 | 18 ++++++++++++++++++
 gdb/testsuite/gdb.python/py-inferior.exp | 18 ++++++++++++++++++
 4 files changed, 45 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 4e4f12d8d13..21da6ae4ba8 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -84,6 +84,11 @@ GNU/Linux/RISC-V		riscv*-*-linux*
 CSKY ELF			csky*-*-elf
 CSKY GNU/LINUX			csky*-*-linux
 
+* Python API
+
+  ** The program space associated to an inferior is now accessible through a
+     new "progspace" attribute of gdb.Inferior.
+
 *** Changes in GDB 8.2
 
 * The 'set disassembler-options' command now supports specifying options
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 5e2ab76d2bc..d4f03439f10 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2832,6 +2832,10 @@ Boolean signaling whether the inferior was created using `attach', or
 started by @value{GDBN} itself.
 @end defvar
 
+@defvar Inferior.progspace
+Program space associated to this inferrior, as a @code{gdb.Progspace}.
+@end defvar
+
 A @code{gdb.Inferior} object has the following methods:
 
 @defun Inferior.is_valid ()
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 8e6f148b17a..e770585184b 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -459,6 +459,23 @@ infpy_get_was_attached (PyObject *self, void *closure)
   Py_RETURN_FALSE;
 }
 
+/* Getter of gdb.Inferior.progspace.  */
+
+static PyObject *
+infpy_get_progspace (PyObject *self, void *closure)
+{
+  inferior_object *inf = (inferior_object *) self;
+
+  INFPY_REQUIRE_VALID (inf);
+
+  program_space *pspace = inf->inferior->pspace;
+  gdb_assert (pspace != nullptr);
+
+  PyObject *py_pspace = pspace_to_pspace_object (pspace);
+  Py_XINCREF (py_pspace);
+  return py_pspace;
+}
+
 static int
 build_inferior_list (struct inferior *inf, void *arg)
 {
@@ -963,6 +980,7 @@ static gdb_PyGetSetDef inferior_object_getset[] =
     NULL },
   { "was_attached", infpy_get_was_attached, NULL,
     "True if the inferior was created using 'attach'.", NULL },
+  { "progspace", infpy_get_progspace, NULL, "Program space of this inferior" },
   { NULL }
 };
 
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index 6f4a3146684..01307531412 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -54,6 +54,9 @@ gdb_test "python print ('result = %s' % i0.pid)" " = \[0-9\]+" "test Inferior.pi
 gdb_test "python print ('result = %s' % i0.was_attached)" " = False" "test Inferior.was_attached"
 gdb_test "python print (i0.threads ())" "\\(<gdb.InferiorThread object at 0x\[\[:xdigit:\]\]+>,\\)" "test Inferior.threads"
 
+gdb_test "python print (i0.progspace)" "<gdb.Progspace object at $hex>"
+gdb_test "python print (i0.progspace == gdb.progspaces()\[0\])" "True"
+
 # Test the number of inferior threads.
 
 gdb_breakpoint check_threads
@@ -253,6 +256,21 @@ with_test_prefix "is_valid" {
 
     gdb_test "python print (my_inferior_count)" "1" \
 	"test inferior-deleted event handler"
+	
+    # Test that other properties and methods handle the removed inferior
+    # correctly.
+    gdb_test "python print (inf_list\[1\].num)" \
+	"RuntimeError: Inferior no longer exists.*"
+    gdb_test "python print (inf_list\[1\].pid)" \
+	"RuntimeError: Inferior no longer exists.*"
+    gdb_test "python print (inf_list\[1\].was_attached)" \
+	"RuntimeError: Inferior no longer exists.*"
+    gdb_test "python print (inf_list\[1\].progspace)" \
+	"RuntimeError: Inferior no longer exists.*"
+    gdb_test "python print (inf_list\[1\].threads ())" \
+	"RuntimeError: Inferior no longer exists.*"
+    gdb_test "python print (inf_list\[1\].thread_from_thread_handle (1))" \
+	"RuntimeError: Inferior no longer exists.*"
 }
 
 # Test gdb.selected_inferior()
-- 
2.19.0

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

* Re: [PATCH 1/3] python: Add Inferior.progspace property
  2018-09-12 19:37 [PATCH 1/3] python: Add Inferior.progspace property Simon Marchi
  2018-09-12 19:36 ` [PATCH 3/3] python: Fix erroneous doc about gdb.objfiles() Simon Marchi
  2018-09-12 19:37 ` [PATCH 2/3] python: Add Progspace.objfiles method Simon Marchi
@ 2018-09-12 21:42 ` Tom Tromey
  2018-09-12 22:22   ` Simon Marchi
  2018-09-12 21:43 ` Tom Tromey
  2018-09-13  2:37 ` Eli Zaretskii
  4 siblings, 1 reply; 16+ messages in thread
From: Tom Tromey @ 2018-09-12 21:42 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> This patch adds a progspace property to the gdb.Inferior type, which
Simon> allows getting the gdb.Progspace object associated to that inferior.
Simon> In conjunction with the following patch, this will allow scripts iterate
Simon> on objfiles associated with a particular inferior.

I did this a long time ago, plus a bit more.
I think it would be good to compare approaches.

Old thread here:
https://sourceware.org/ml/gdb-patches/2014-06/msg00616.html

I rebaesd it - see the python/progspace-submit branch in my github.
I haven't re-tested it yet.

Simon> While modifying py-inferior.exp, I added some checks for the other
Simon> Inferior properties, when the Inferior is no longer valid.  This doesn't
Simon> seem tested at the moment.

This could go in immediately if you were so inclined.

Tom

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

* Re: [PATCH 1/3] python: Add Inferior.progspace property
  2018-09-12 19:37 [PATCH 1/3] python: Add Inferior.progspace property Simon Marchi
                   ` (2 preceding siblings ...)
  2018-09-12 21:42 ` [PATCH 1/3] python: Add Inferior.progspace property Tom Tromey
@ 2018-09-12 21:43 ` Tom Tromey
  2018-09-13  2:37 ` Eli Zaretskii
  4 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2018-09-12 21:43 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> +Program space associated to this inferrior, as a @code{gdb.Progspace}.

Forgot to mention that earlier I noticed a typo in "inferrior".

Tom

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

* Re: [PATCH 3/3] python: Fix erroneous doc about gdb.objfiles()
  2018-09-12 19:36 ` [PATCH 3/3] python: Fix erroneous doc about gdb.objfiles() Simon Marchi
@ 2018-09-12 21:46   ` Tom Tromey
  2018-09-13  2:37   ` Eli Zaretskii
  1 sibling, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2018-09-12 21:46 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> +Return a sequence of objfiles for the current program space (and by extension,
Simon> +for the selected inferior) @xref{Objfiles In Python} and
Simon> +@pxref{Progspaces In Python}.

The 'by extension' text seems mildly confusing to me.
Maybe it could say something like:

    Return a sequence of all the objfiles referenced by this program
    space.  @xref{Objfiles In Python}.  Note that lThis is the same as the
    objfiles for the selected inferior.

Or something else equally direct?

Tom

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

* Re: [PATCH 2/3] python: Add Progspace.objfiles method
  2018-09-12 19:37 ` [PATCH 2/3] python: Add Progspace.objfiles method Simon Marchi
@ 2018-09-12 22:01   ` Tom Tromey
  2018-09-12 22:58     ` Simon Marchi
  2018-09-13  2:38   ` Eli Zaretskii
  1 sibling, 1 reply; 16+ messages in thread
From: Tom Tromey @ 2018-09-12 22:01 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

>>>>> "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

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

* Re: [PATCH 1/3] python: Add Inferior.progspace property
  2018-09-12 21:42 ` [PATCH 1/3] python: Add Inferior.progspace property Tom Tromey
@ 2018-09-12 22:22   ` Simon Marchi
  2018-09-13  4:38     ` Tom Tromey
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Marchi @ 2018-09-12 22:22 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Simon Marchi, gdb-patches

On 2018-09-12 17:42, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> This patch adds a progspace property to the gdb.Inferior type, 
> which
> Simon> allows getting the gdb.Progspace object associated to that 
> inferior.
> Simon> In conjunction with the following patch, this will allow scripts 
> iterate
> Simon> on objfiles associated with a particular inferior.
> 
> I did this a long time ago, plus a bit more.
> I think it would be good to compare approaches.
> 
> Old thread here:
> https://sourceware.org/ml/gdb-patches/2014-06/msg00616.html
> 
> I rebaesd it - see the python/progspace-submit branch in my github.
> I haven't re-tested it yet.

Oh wow, that's much more complete than what I have.  I also spotted some 
other areas of improvement, but I know that if I try to bite too much in 
one go I never finish and submit what I have.  That's why I sent just 
these two by themselves.

If you intend on finishing and submitting that patch I'll drop mine.  
Otherwise, I'll go ahead and maybe submit yours chunk by chunk, adding 
tests in the process.

As far as the gdb.Inferior.progspace part is concerned, our two versions 
are essentially identical.

> Simon> While modifying py-inferior.exp, I added some checks for the 
> other
> Simon> Inferior properties, when the Inferior is no longer valid.  This 
> doesn't
> Simon> seem tested at the moment.
> 
> This could go in immediately if you were so inclined.

Ok, I'll push an obvious patch for that.

Simon

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

* Re: [PATCH 2/3] python: Add Progspace.objfiles method
  2018-09-12 22:01   ` Tom Tromey
@ 2018-09-12 22:58     ` Simon Marchi
  2018-09-13  4:50       ` Tom Tromey
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Marchi @ 2018-09-12 22:58 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Simon Marchi, gdb-patches

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

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

* Re: [PATCH 3/3] python: Fix erroneous doc about gdb.objfiles()
  2018-09-12 19:36 ` [PATCH 3/3] python: Fix erroneous doc about gdb.objfiles() Simon Marchi
  2018-09-12 21:46   ` Tom Tromey
@ 2018-09-13  2:37   ` Eli Zaretskii
  2018-09-13  2:42     ` Simon Marchi
  1 sibling, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2018-09-13  2:37 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: Simon Marchi <simon.marchi@ericsson.com>
> Date: Wed, 12 Sep 2018 15:36:17 -0400
> 
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index abda135e17d..a9de6bd815e 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -4104,8 +4104,9 @@ this function returns @code{None}.
>  
>  @findex gdb.objfiles
>  @defun gdb.objfiles ()
> -Return a sequence of all the objfiles current known to @value{GDBN}.
> -@xref{Objfiles In Python}.
> +Return a sequence of objfiles for the current program space (and by extension,
> +for the selected inferior) @xref{Objfiles In Python} and
> +@pxref{Progspaces In Python}.
>  @end defun

"By extension" confuses me.  And there should be a period before @xref
and a comma after its closing brace.

Thanks.

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

* Re: [PATCH 1/3] python: Add Inferior.progspace property
  2018-09-12 19:37 [PATCH 1/3] python: Add Inferior.progspace property Simon Marchi
                   ` (3 preceding siblings ...)
  2018-09-12 21:43 ` Tom Tromey
@ 2018-09-13  2:37 ` Eli Zaretskii
  4 siblings, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2018-09-13  2:37 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: Simon Marchi <simon.marchi@ericsson.com>
> Date: Wed, 12 Sep 2018 15:36:15 -0400
> 
> This patch adds a progspace property to the gdb.Inferior type, which
> allows getting the gdb.Progspace object associated to that inferior.
> In conjunction with the following patch, this will allow scripts iterate
> on objfiles associated with a particular inferior.
> 
> While modifying py-inferior.exp, I added some checks for the other
> Inferior properties, when the Inferior is no longer valid.  This doesn't
> seem tested at the moment.
> 
> gdb/ChangeLog:
> 
> 	* python/py-inferior.c (infpy_get_progspace): New function.
> 	(inferior_object_getset): Add progspace property.
> 	* NEWS: Mention the new property.
> 
> gdb/doc/ChangeLog:
> 
> 	* python.texi (Inferiors In Python): Document
> 	Inferior.progspace.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.python/py-inferior.exp: Add tests for Inferior.progspace
> 	and a few other Inferior properties when the Inferior is no
> 	longer valid.

OK for the documentation parts.

Thanks.

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

* Re: [PATCH 2/3] python: Add Progspace.objfiles method
  2018-09-12 19:37 ` [PATCH 2/3] python: Add Progspace.objfiles method Simon Marchi
  2018-09-12 22:01   ` Tom Tromey
@ 2018-09-13  2:38   ` Eli Zaretskii
  1 sibling, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2018-09-13  2:38 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

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

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

* Re: [PATCH 3/3] python: Fix erroneous doc about gdb.objfiles()
  2018-09-13  2:37   ` Eli Zaretskii
@ 2018-09-13  2:42     ` Simon Marchi
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Marchi @ 2018-09-13  2:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Simon Marchi, gdb-patches, tom

On 2018-09-12 22:36, Eli Zaretskii wrote:
>> From: Simon Marchi <simon.marchi@ericsson.com>
>> CC: Simon Marchi <simon.marchi@ericsson.com>
>> Date: Wed, 12 Sep 2018 15:36:17 -0400
>> 
>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
>> index abda135e17d..a9de6bd815e 100644
>> --- a/gdb/doc/python.texi
>> +++ b/gdb/doc/python.texi
>> @@ -4104,8 +4104,9 @@ this function returns @code{None}.
>> 
>>  @findex gdb.objfiles
>>  @defun gdb.objfiles ()
>> -Return a sequence of all the objfiles current known to @value{GDBN}.
>> -@xref{Objfiles In Python}.
>> +Return a sequence of objfiles for the current program space (and by 
>> extension,
>> +for the selected inferior) @xref{Objfiles In Python} and
>> +@pxref{Progspaces In Python}.
>>  @end defun
> 
> "By extension" confuses me.  And there should be a period before @xref
> and a comma after its closing brace.
> 
> Thanks.

Hi Eli,

This changed in the v2 I just sent 5 minutes ago :).

I stole the text from an old patch of Tom, which says

   This is identical to gdb.selected_inferior().progspace.objfiles() and 
is included for historical compatibility.

Which makes what I was trying to say here unnecessary.

Simon

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

* Re: [PATCH 1/3] python: Add Inferior.progspace property
  2018-09-12 22:22   ` Simon Marchi
@ 2018-09-13  4:38     ` Tom Tromey
  2018-09-13 22:16       ` Tom Tromey
  0 siblings, 1 reply; 16+ messages in thread
From: Tom Tromey @ 2018-09-13  4:38 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi, gdb-patches

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> If you intend on finishing and submitting that patch I'll drop mine.
Simon> Otherwise, I'll go ahead and maybe submit yours chunk by chunk, adding
Simon> tests in the process.

Ok, I cleaned it up tonight.  Let me know what you think.

I tried to address some of the review comments from years back as well.

However, I didn't remove Progspace.solib_name.  I think it's better for
the API to make sense in terms of the objects in gdb; and while I agree
that solib_name is probably useless, I don't actually know what people
use it for, and it seemed less good to fix up other instances of this
API badness but leave one alone.

I agree a better API can be done.  In fact it would be useful to me for
the SpiderMonkey unwinder.

Anyway let me know what you think.  Feel free to critique the solib_name
thing, I'm not sure my "logic" makes any sense at all!

Tom

commit 00b945a6d2f1af22b3860ce108ed7be2bed41325
Author: Tom Tromey <tom@tromey.com>
Date:   Thu Dec 26 19:50:05 2013 -0700

    Add more methods to gdb.Progspace
    
    There are a number of global functions in the gdb Python module which
    really should be methods on Progspace.  This patch adds new methods to
    Progspace and then redefines these globals in terms of these new
    methods.  It also adds an Inferior.progspace attribute so that the
    association between inferiors and progspaces is obvious; this lets us
    apply the same treatment to current_progspace.
    
    Built and regtested on x86-64 Fedora 28.
    
    gdb/ChangeLog
    2018-09-12  Tom Tromey  <tom@tromey.com>
    
            * python/lib/gdb/__init__.py (current_progspace, objfiles)
            (solib_name, block_for_pc, find_pc_line): New functions.
            (execute_unwinders): Update.
            * python/py-block.c (gdbpy_block_for_pc): Remove.
            * python/py-inferior.c (infpy_get_progspace): New function.
            (inferior_object_getset) <progspace>: Add.
            * python/py-progspace.c (PSPY_REQUIRE_VALID): New macro.
            (pspy_objfiles, pspy_solib_name, pspy_block_for_pc)
            (pspy_find_pc_line, pspy_is_valid): New functions.
            (pspace_object_methods): New global.
            (pspace_object_type): Reference pspace_object_methods.
            * python/python-internal.h (gdbpy_block_for_pc): Don't declare.
            * python/python.c: Don't include solib.h.
            (gdbpy_solib_name, gdbpy_find_pc_line)
            (gdbpy_get_current_progspace, gdbpy_objfiles): Remove.
            (GdbMethods) <current_progspace, objfiles, block_for_pc,
            solib_name, find_pc_line>: Remove entries.
    
    gdb/doc/ChangeLog
    2018-09-12  Tom Tromey  <tromey@redhat.com>
    
            * python.texi (Basic Python): Update docs for find_pc_line,
            solib_name.
            (Inferiors In Python): Document Inferior.progspace.
            (Progspaces In Python): Update docs for current_progspace.
            Document block_for_pc, find_pc_line, is_valid, objfiles,
            solib_name.
            (Objfiles In Python): Update docs for gdb.objfiles.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 454c45f9ad4..3810cd5df12 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,23 @@
+2018-09-12  Tom Tromey  <tom@tromey.com>
+
+	* python/lib/gdb/__init__.py (current_progspace, objfiles)
+	(solib_name, block_for_pc, find_pc_line): New functions.
+	(execute_unwinders): Update.
+	* python/py-block.c (gdbpy_block_for_pc): Remove.
+	* python/py-inferior.c (infpy_get_progspace): New function.
+	(inferior_object_getset) <progspace>: Add.
+	* python/py-progspace.c (PSPY_REQUIRE_VALID): New macro.
+	(pspy_objfiles, pspy_solib_name, pspy_block_for_pc)
+	(pspy_find_pc_line, pspy_is_valid): New functions.
+	(pspace_object_methods): New global.
+	(pspace_object_type): Reference pspace_object_methods.
+	* python/python-internal.h (gdbpy_block_for_pc): Don't declare.
+	* python/python.c: Don't include solib.h.
+	(gdbpy_solib_name, gdbpy_find_pc_line)
+	(gdbpy_get_current_progspace, gdbpy_objfiles): Remove.
+	(GdbMethods) <current_progspace, objfiles, block_for_pc,
+	solib_name, find_pc_line>: Remove entries.
+
 2018-09-12  John Baldwin  <jhb@FreeBSD.org>
 
 	* fbsd-nat.c (fbsd_nat_target::info_proc): Remove unused variable.
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index f04dca42835..da8cc8c6a41 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,13 @@
+2018-09-12  Tom Tromey  <tromey@redhat.com>
+
+	* python.texi (Basic Python): Update docs for find_pc_line,
+	solib_name.
+	(Inferiors In Python): Document Inferior.progspace.
+	(Progspaces In Python): Update docs for current_progspace.
+	Document block_for_pc, find_pc_line, is_valid, objfiles,
+	solib_name.
+	(Objfiles In Python): Update docs for gdb.objfiles.
+
 2018-09-12  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* gdb.texinfo (Skipping Over Functions and Files): Document
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 5e2ab76d2bc..c319808c356 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -324,7 +324,9 @@ Return the @code{gdb.Symtab_and_line} object corresponding to the
 @var{pc} value.  @xref{Symbol Tables In Python}.  If an invalid
 value of @var{pc} is passed as an argument, then the @code{symtab} and
 @code{line} attributes of the returned @code{gdb.Symtab_and_line} object
-will be @code{None} and 0 respectively.
+will be @code{None} and 0 respectively.  This is identical to
+@code{current_progspace().find_pc_line(pc)} and is included for
+historical compatibility.
 @end defun
 
 @findex gdb.post_event
@@ -444,7 +446,9 @@ never returned.
 @findex gdb.solib_name
 @defun gdb.solib_name (address)
 Return the name of the shared library holding the given @var{address}
-as a string, or @code{None}.
+as a string, or @code{None}.  This is identical to
+@code{current_progspace().solib_name(address)} and is included for
+historical compatibility.
 @end defun
 
 @findex gdb.decode_line 
@@ -2827,6 +2831,9 @@ Process ID of the inferior, as assigned by the underlying operating
 system.
 @end defvar
 
+@defvar Inferior.progspace
+The inferior's program space.  @xref{Progspaces in Python}.
+
 @defvar Inferior.was_attached
 Boolean signaling whether the inferior was created using `attach', or
 started by @value{GDBN} itself.
@@ -3990,8 +3997,10 @@ The following progspace-related functions are available in the
 
 @findex gdb.current_progspace
 @defun gdb.current_progspace ()
-This function returns the program space of the currently selected inferior.
-@xref{Inferiors and Programs}.
+This function returns the program space of the currently selected
+inferior.  @xref{Inferiors and Programs}.  This is identical to
+@code{selected_inferior().progspace} (@pxref{Inferiors In Python}) and
+is included for historical compatibility.
 @end defun
 
 @findex gdb.progspaces
@@ -4025,6 +4034,46 @@ The @code{frame_filters} attribute is a dictionary of frame filter
 objects.  @xref{Frame Filter API}, for more information.
 @end defvar
 
+A program space has the following methods:
+
+@findex Progspace.block_for_pc
+@defun Progspace.block_for_pc (pc)
+Return the innermost @code{gdb.Block} containing the given @var{pc}
+value.  If the block cannot be found for the @var{pc} value specified,
+the function will return @code{None}.
+@end defun
+
+@findex Progspace.find_pc_line
+@defun Progspace.find_pc_line (pc)
+Return the @code{gdb.Symtab_and_line} object corresponding to the
+@var{pc} value.  @xref{Symbol Tables In Python}.  If an invalid value
+of @var{pc} is passed as an argument, then the @code{symtab} and
+@code{line} attributes of the returned @code{gdb.Symtab_and_line}
+object will be @code{None} and 0 respectively.
+@end defun
+
+@findex Progspace.is_valid
+@defun Progspace.is_valid ()
+Returns @code{True} if the @code{gdb.Progspace} object is valid,
+@code{False} if not.  A @code{gdb.Progspace} object can become invalid
+if the program space file it refers to is not referenced by any
+inferior.  All other @code{gdb.Progspace} methods will throw an
+exception if it is invalid at the time the method is called.
+@end defun
+
+@findex Progspace.objfiles
+@defun Progspace.objfiles ()
+Return a sequence of all the objfiles referenced by this program
+space.  @xref{Objfiles In Python}.  The objfiles are not guaranteed to
+be returned in any particular order.
+@end defun
+
+@findex Progspace.solib_name
+@defun Progspace.solib_name (address)
+Return the name of the shared library holding the given @var{address}
+as a string, or @code{None}.
+@end defun
+
 One may add arbitrary attributes to @code{gdb.Progspace} objects
 in the usual Python way.
 This is useful if, for example, one needs to do some extra record keeping
@@ -4095,7 +4144,9 @@ this function returns @code{None}.
 @findex gdb.objfiles
 @defun gdb.objfiles ()
 Return a sequence of all the objfiles current known to @value{GDBN}.
-@xref{Objfiles In Python}.
+@xref{Objfiles In Python}.  This is identical to
+@code{gdb.current_progspace().objfiles()} (@pxref{Progspaces in
+Python}) and is included for historical compatibility.
 @end defun
 
 @findex gdb.lookup_objfile
@@ -4495,7 +4546,9 @@ module:
 @defun gdb.block_for_pc (pc)
 Return the innermost @code{gdb.Block} containing the given @var{pc}
 value.  If the block cannot be found for the @var{pc} value specified,
-the function will return @code{None}.
+the function will return @code{None}.  This is identical to
+@code{current_progspace().block_for_pc(pc)} and is included for
+historical compatibility.
 @end defun
 
 A @code{gdb.Block} object has the following methods:
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 7e7c299f3c7..c223f2bac3d 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -85,15 +85,14 @@ def execute_unwinders(pending_frame):
     Returns:
         gdb.UnwindInfo instance or None.
     """
-    for objfile in _gdb.objfiles():
+    for objfile in objfiles():
         for unwinder in objfile.frame_unwinders:
             if unwinder.enabled:
                 unwind_info = unwinder(pending_frame)
                 if unwind_info is not None:
                     return unwind_info
 
-    current_progspace = _gdb.current_progspace()
-    for unwinder in current_progspace.frame_unwinders:
+    for unwinder in current_progspace().frame_unwinders:
         if unwinder.enabled:
             unwind_info = unwinder(pending_frame)
             if unwind_info is not None:
@@ -163,3 +162,25 @@ def GdbSetPythonDirectory(dir):
     # attributes
     reload(__import__(__name__))
     auto_load_packages()
+
+def current_progspace():
+    "Return the current Progspace."
+    return selected_inferior().progspace
+
+def objfiles():
+    "Return a sequence of the current program space's objfiles."
+    return current_progspace().objfiles()
+
+def solib_name (addr):
+    """solib_name (Long) -> String.\n\
+Return the name of the shared library holding a given address, or None."""
+    return current_progspace().solib_name(addr)
+
+def block_for_pc(pc):
+    "Return the block containing the given pc value, or None."
+    return current_progspace().block_for_pc(pc)
+
+def find_pc_line(pc):
+    """find_pc_line (pc) -> Symtab_and_line.
+Return the gdb.Symtab_and_line object corresponding to the pc value."""
+    return current_progspace().find_pc_line(pc)
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index 70a0a76dc43..5fc33f0f0fb 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -366,44 +366,6 @@ blpy_iter_is_valid (PyObject *self, PyObject *args)
   Py_RETURN_TRUE;
 }
 
-/* Return the innermost lexical block containing the specified pc value,
-   or 0 if there is none.  */
-PyObject *
-gdbpy_block_for_pc (PyObject *self, PyObject *args)
-{
-  gdb_py_ulongest pc;
-  const struct block *block = NULL;
-  struct compunit_symtab *cust = NULL;
-
-  if (!PyArg_ParseTuple (args, GDB_PY_LLU_ARG, &pc))
-    return NULL;
-
-  TRY
-    {
-      cust = find_pc_compunit_symtab (pc);
-
-      if (cust != NULL && COMPUNIT_OBJFILE (cust) != NULL)
-	block = block_for_pc (pc);
-    }
-  CATCH (except, RETURN_MASK_ALL)
-    {
-      GDB_PY_HANDLE_EXCEPTION (except);
-    }
-  END_CATCH
-
-  if (cust == NULL || COMPUNIT_OBJFILE (cust) == NULL)
-    {
-      PyErr_SetString (PyExc_RuntimeError,
-		       _("Cannot locate object file for block."));
-      return NULL;
-    }
-
-  if (block)
-    return block_to_block_object (block, COMPUNIT_OBJFILE (cust));
-
-  Py_RETURN_NONE;
-}
-
 /* This function is called when an objfile is about to be freed.
    Invalidate the block as further actions on the block would result
    in bad data.  All access to obj->symbol should be gated by
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 1cf37296973..8c979fbdc69 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -448,6 +448,23 @@ infpy_get_pid (PyObject *self, void *closure)
   return PyLong_FromLong (inf->inferior->pid);
 }
 
+/* Implement the "progspace" method.  */
+
+static PyObject *
+infpy_get_progspace (PyObject *self, void *closure)
+{
+  inferior_object *inf = (inferior_object *) self;
+  PyObject *result;
+
+  INFPY_REQUIRE_VALID (inf);
+
+  result = pspace_to_pspace_object (inf->inferior->pspace);
+  if (result != NULL)
+    Py_INCREF (result);
+
+  return result;
+}
+
 static PyObject *
 infpy_get_was_attached (PyObject *self, void *closure)
 {
@@ -949,6 +966,8 @@ static gdb_PyGetSetDef inferior_object_getset[] =
   { "num", infpy_get_num, NULL, "ID of inferior, as assigned by GDB.", NULL },
   { "pid", infpy_get_pid, NULL, "PID of inferior, as assigned by the OS.",
     NULL },
+  { "progspace", infpy_get_progspace, NULL, "Progspace of inferior.",
+    NULL },
   { "was_attached", infpy_get_was_attached, NULL,
     "True if the inferior was created using 'attach'.", NULL },
   { NULL }
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 3eaa466666d..53376ed7691 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -25,6 +25,8 @@
 #include "language.h"
 #include "arch-utils.h"
 #include "py-ref.h"
+#include "solib.h"
+#include "block.h"
 
 typedef struct
 {
@@ -58,6 +60,17 @@ extern PyTypeObject pspace_object_type
 
 static const struct program_space_data *pspy_pspace_data_key;
 
+/* Require that PSPACE be a valid program space ID.  */
+#define PSPY_REQUIRE_VALID(Pspace)				\
+  do {								\
+    if (!Pspace->pspace)					\
+      {								\
+	PyErr_SetString (PyExc_RuntimeError,			\
+			 _("program space no longer exists."));	\
+	return NULL;						\
+      }								\
+  } while (0)
+
 \f
 
 /* An Objfile method which returns the objfile's file name, or None.  */
@@ -314,6 +327,155 @@ pspy_set_type_printers (PyObject *o, PyObject *value, void *ignore)
   return 0;
 }
 
+/* Return a sequence holding all the Objfiles.  */
+
+static PyObject *
+pspy_objfiles (PyObject *o, PyObject *args)
+{
+  pspace_object *self = (pspace_object *) o;
+
+  gdbpy_ref<> list (PyList_New (0));
+  if (list == NULL)
+    return NULL;
+
+  if (self->pspace != NULL)
+    {
+      struct objfile *objf;
+
+      ALL_PSPACE_OBJFILES (self->pspace, objf)
+	{
+	  PyObject *item = objfile_to_objfile_object (objf);
+
+	  if (!item || PyList_Append (list.get (), item) == -1)
+	    return NULL;
+	}
+    }
+
+  return list.release ();
+}
+
+/* Implementation of solib_name (Long) -> String.
+   Returns the name of the shared library holding a given address, or None.  */
+
+static PyObject *
+pspy_solib_name (PyObject *o, PyObject *args)
+{
+  char *soname;
+  PyObject *str_obj;
+  gdb_py_longest pc;
+  pspace_object *self = (pspace_object *) o;
+
+  PSPY_REQUIRE_VALID (self);
+
+  if (!PyArg_ParseTuple (args, GDB_PY_LLU_ARG, &pc))
+    return NULL;
+
+  soname = solib_name_from_address (self->pspace, pc);
+  if (soname)
+    str_obj = host_string_to_python_string (soname);
+  else
+    {
+      str_obj = Py_None;
+      Py_INCREF (Py_None);
+    }
+
+  return str_obj;
+}
+
+/* Return the innermost lexical block containing the specified pc value,
+   or 0 if there is none.  */
+static PyObject *
+pspy_block_for_pc (PyObject *o, PyObject *args)
+{
+  pspace_object *self = (pspace_object *) o;
+  gdb_py_ulongest pc;
+  const struct block *block = NULL;
+  struct compunit_symtab *cust = NULL;
+
+  PSPY_REQUIRE_VALID (self);
+
+  if (!PyArg_ParseTuple (args, GDB_PY_LLU_ARG, &pc))
+    return NULL;
+
+  TRY
+    {
+      scoped_restore_current_program_space saver;
+
+      set_current_program_space (self->pspace);
+      cust = find_pc_compunit_symtab (pc);
+
+      if (cust != NULL && COMPUNIT_OBJFILE (cust) != NULL)
+	block = block_for_pc (pc);
+    }
+  CATCH (except, RETURN_MASK_ALL)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+  END_CATCH
+
+  if (cust == NULL || COMPUNIT_OBJFILE (cust) == NULL)
+    {
+      PyErr_SetString (PyExc_RuntimeError,
+		       _("Cannot locate object file for block."));
+      return NULL;
+    }
+
+  if (block)
+    return block_to_block_object (block, COMPUNIT_OBJFILE (cust));
+
+  Py_RETURN_NONE;
+}
+
+/* Implementation of the find_pc_line function.
+   Returns the gdb.Symtab_and_line object corresponding to a PC value.  */
+
+static PyObject *
+pspy_find_pc_line (PyObject *o, PyObject *args)
+{
+  gdb_py_ulongest pc_llu;
+  PyObject *result = NULL; /* init for gcc -Wall */
+  pspace_object *self = (pspace_object *) o;
+
+  PSPY_REQUIRE_VALID (self);
+
+  if (!PyArg_ParseTuple (args, GDB_PY_LLU_ARG, &pc_llu))
+    return NULL;
+
+  TRY
+    {
+      struct symtab_and_line sal;
+      CORE_ADDR pc;
+      scoped_restore_current_program_space saver;
+
+      set_current_program_space (self->pspace);
+
+      pc = (CORE_ADDR) pc_llu;
+      sal = find_pc_line (pc, 0);
+      result = symtab_and_line_to_sal_object (sal);
+    }
+  CATCH (except, RETURN_MASK_ALL)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+  END_CATCH
+
+  return result;
+}
+
+/* Implementation of is_valid (self) -> Boolean.
+   Returns True if this program space still exists in GDB.  */
+
+static PyObject *
+pspy_is_valid (PyObject *o, PyObject *args)
+{
+  pspace_object *self = (pspace_object *) o;
+
+  if (self->pspace == NULL)
+    Py_RETURN_FALSE;
+
+  Py_RETURN_TRUE;
+}
+
 \f
 
 /* Clear the PSPACE pointer in a Pspace object and remove the reference.  */
@@ -397,6 +559,24 @@ static gdb_PyGetSetDef pspace_getset[] =
   { NULL }
 };
 
+static PyMethodDef pspace_object_methods[] =
+{
+  { "objfiles", pspy_objfiles, METH_NOARGS,
+    "Return a sequence of the program space's objfiles." },
+  { "solib_name", pspy_solib_name, METH_VARARGS,
+    "solib_name (Long) -> String.\n\
+Return the name of the shared library holding a given address, or None." },
+  { "block_for_pc", pspy_block_for_pc, METH_VARARGS,
+    "Return the block containing the given pc value, or None." },
+  { "find_pc_line", pspy_find_pc_line, METH_VARARGS,
+    "find_pc_line (pc) -> Symtab_and_line.\n\
+Return the gdb.Symtab_and_line object corresponding to the pc value." },
+  { "is_valid", pspy_is_valid, METH_NOARGS,
+    "is_valid () -> Boolean.\n\
+Return true if this program space is valid, false if not." },
+  { NULL }
+};
+
 PyTypeObject pspace_object_type =
 {
   PyVarObject_HEAD_INIT (NULL, 0)
@@ -426,7 +606,7 @@ PyTypeObject pspace_object_type =
   0,				  /* tp_weaklistoffset */
   0,				  /* tp_iter */
   0,				  /* tp_iternext */
-  0,				  /* tp_methods */
+  pspace_object_methods,	  /* tp_methods */
   0,				  /* tp_members */
   pspace_getset,		  /* tp_getset */
   0,				  /* tp_base */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 3874fdc5e79..1c526af1586 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -492,7 +492,6 @@ PyObject *gdbpy_current_recording (PyObject *self, PyObject *args);
 PyObject *gdbpy_stop_recording (PyObject *self, PyObject *args);
 PyObject *gdbpy_newest_frame (PyObject *self, PyObject *args);
 PyObject *gdbpy_selected_frame (PyObject *self, PyObject *args);
-PyObject *gdbpy_block_for_pc (PyObject *self, PyObject *args);
 PyObject *gdbpy_lookup_type (PyObject *self, PyObject *args, PyObject *kw);
 int gdbpy_is_field (PyObject *obj);
 PyObject *gdbpy_create_lazy_string_object (CORE_ADDR address, long length,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 6f798a0e461..1859331df01 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -90,7 +90,6 @@ const struct extension_language_defn extension_language_python =
 #include "cli/cli-decode.h"
 #include "charset.h"
 #include "top.h"
-#include "solib.h"
 #include "python-internal.h"
 #include "linespec.h"
 #include "source.h"
@@ -632,31 +631,6 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
   Py_RETURN_NONE;
 }
 
-/* Implementation of gdb.solib_name (Long) -> String.
-   Returns the name of the shared library holding a given address, or None.  */
-
-static PyObject *
-gdbpy_solib_name (PyObject *self, PyObject *args)
-{
-  char *soname;
-  PyObject *str_obj;
-  gdb_py_ulongest pc;
-
-  if (!PyArg_ParseTuple (args, GDB_PY_LLU_ARG, &pc))
-    return NULL;
-
-  soname = solib_name_from_address (current_program_space, pc);
-  if (soname)
-    str_obj = host_string_to_python_string (soname);
-  else
-    {
-      str_obj = Py_None;
-      Py_INCREF (Py_None);
-    }
-
-  return str_obj;
-}
-
 /* Implementation of Python rbreak command.  Take a REGEX and
    optionally a MINSYMS, THROTTLE and SYMTABS keyword and return a
    Python list that contains newly set breakpoints that match that
@@ -942,36 +916,6 @@ gdbpy_parse_and_eval (PyObject *self, PyObject *args)
   return value_to_value_object (result);
 }
 
-/* Implementation of gdb.find_pc_line function.
-   Returns the gdb.Symtab_and_line object corresponding to a PC value.  */
-
-static PyObject *
-gdbpy_find_pc_line (PyObject *self, PyObject *args)
-{
-  gdb_py_ulongest pc_llu;
-  PyObject *result = NULL; /* init for gcc -Wall */
-
-  if (!PyArg_ParseTuple (args, GDB_PY_LLU_ARG, &pc_llu))
-    return NULL;
-
-  TRY
-    {
-      struct symtab_and_line sal;
-      CORE_ADDR pc;
-
-      pc = (CORE_ADDR) pc_llu;
-      sal = find_pc_line (pc, 0);
-      result = symtab_and_line_to_sal_object (sal);
-    }
-  CATCH (except, RETURN_MASK_ALL)
-    {
-      GDB_PY_HANDLE_EXCEPTION (except);
-    }
-  END_CATCH
-
-  return result;
-}
-
 /* Implementation of gdb.invalidate_cached_frames.  */
 
 static PyObject *
@@ -1333,20 +1277,6 @@ gdbpy_print_stack (void)
 
 \f
 
-/* Return the current Progspace.
-   There always is one.  */
-
-static PyObject *
-gdbpy_get_current_progspace (PyObject *unused1, PyObject *unused2)
-{
-  PyObject *result;
-
-  result = pspace_to_pspace_object (current_program_space);
-  if (result)
-    Py_INCREF (result);
-  return result;
-}
-
 /* Return a sequence holding all the Progspaces.  */
 
 static PyObject *
@@ -1437,28 +1367,6 @@ gdbpy_get_current_objfile (PyObject *unused1, PyObject *unused2)
   return result;
 }
 
-/* Return a sequence holding all the Objfiles.  */
-
-static PyObject *
-gdbpy_objfiles (PyObject *unused1, PyObject *unused2)
-{
-  struct objfile *objf;
-
-  gdbpy_ref<> list (PyList_New (0));
-  if (list == NULL)
-    return NULL;
-
-  ALL_OBJFILES (objf)
-  {
-    PyObject *item = objfile_to_objfile_object (objf);
-
-    if (!item || PyList_Append (list.get (), item) == -1)
-      return NULL;
-  }
-
-  return list.release ();
-}
-
 /* Compute the list of active python type printers and store them in
    EXT_PRINTERS->py_type_printers.  The product of this function is used by
    gdbpy_apply_type_printers, and freed by gdbpy_free_type_printers.
@@ -2034,15 +1942,11 @@ set to True." },
   { "default_visualizer", gdbpy_default_visualizer, METH_VARARGS,
     "Find the default visualizer for a Value." },
 
-  { "current_progspace", gdbpy_get_current_progspace, METH_NOARGS,
-    "Return the current Progspace." },
   { "progspaces", gdbpy_progspaces, METH_NOARGS,
     "Return a sequence of all progspaces." },
 
   { "current_objfile", gdbpy_get_current_objfile, METH_NOARGS,
     "Return the current Objfile being loaded, or None." },
-  { "objfiles", gdbpy_objfiles, METH_NOARGS,
-    "Return a sequence of all loaded objfiles." },
 
   { "newest_frame", gdbpy_newest_frame, METH_NOARGS,
     "newest_frame () -> gdb.Frame.\n\
@@ -2088,11 +1992,6 @@ Look up the specified objfile.\n\
 If by_build_id is True, the objfile is looked up by using name\n\
 as its build id." },
 
-  { "block_for_pc", gdbpy_block_for_pc, METH_VARARGS,
-    "Return the block containing the given pc value, or None." },
-  { "solib_name", gdbpy_solib_name, METH_VARARGS,
-    "solib_name (Long) -> String.\n\
-Return the name of the shared library holding a given address, or None." },
   { "decode_line", gdbpy_decode_line, METH_VARARGS,
     "decode_line (String) -> Tuple.  Decode a string argument the way\n\
 that 'break' or 'edit' does.  Return a tuple containing two elements.\n\
@@ -2104,9 +2003,6 @@ gdb.Symtab_and_line objects (or None)."},
     "parse_and_eval (String) -> Value.\n\
 Parse String as an expression, evaluate it, and return the result as a Value."
   },
-  { "find_pc_line", gdbpy_find_pc_line, METH_VARARGS,
-    "find_pc_line (pc) -> Symtab_and_line.\n\
-Return the gdb.Symtab_and_line object corresponding to the pc value." },
 
   { "post_event", gdbpy_post_event, METH_VARARGS,
     "Post an event into gdb's event loop." },

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

* Re: [PATCH 2/3] python: Add Progspace.objfiles method
  2018-09-12 22:58     ` Simon Marchi
@ 2018-09-13  4:50       ` Tom Tromey
  0 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2018-09-13  4:50 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi, gdb-patches

>>>>> "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

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

* Re: [PATCH 1/3] python: Add Inferior.progspace property
  2018-09-13  4:38     ` Tom Tromey
@ 2018-09-13 22:16       ` Tom Tromey
  0 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2018-09-13 22:16 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Simon Marchi, Simon Marchi, gdb-patches

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
Simon> If you intend on finishing and submitting that patch I'll drop mine.
Simon> Otherwise, I'll go ahead and maybe submit yours chunk by chunk, adding
Simon> tests in the process.

Tom> Ok, I cleaned it up tonight.  Let me know what you think.

I've rebased on top of your patches now and I'll resubmit shortly.

Tom

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

end of thread, other threads:[~2018-09-13 22:16 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-12 19:37 [PATCH 1/3] python: Add Inferior.progspace property Simon Marchi
2018-09-12 19:36 ` [PATCH 3/3] python: Fix erroneous doc about gdb.objfiles() Simon Marchi
2018-09-12 21:46   ` Tom Tromey
2018-09-13  2:37   ` Eli Zaretskii
2018-09-13  2:42     ` Simon Marchi
2018-09-12 19:37 ` [PATCH 2/3] python: Add Progspace.objfiles method Simon Marchi
2018-09-12 22:01   ` Tom Tromey
2018-09-12 22:58     ` Simon Marchi
2018-09-13  4:50       ` Tom Tromey
2018-09-13  2:38   ` Eli Zaretskii
2018-09-12 21:42 ` [PATCH 1/3] python: Add Inferior.progspace property Tom Tromey
2018-09-12 22:22   ` Simon Marchi
2018-09-13  4:38     ` Tom Tromey
2018-09-13 22:16       ` Tom Tromey
2018-09-12 21:43 ` Tom Tromey
2018-09-13  2:37 ` 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).