gdb/python: Return None from Progspace.block_for_pc on error

The documentation for Progspace.block_for_pc says:

  Return the innermost gdb.Block containing the given pc value. If the
  block cannot be found for the pc value specified, the function will
  return None.

However, the implementation actually throws an error for invalid
addresses, like this:

    (gdb) python print gdb.current_progspace ().block_for_pc (1)
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    RuntimeError: Cannot locate object file for block.
    Error while executing Python code.
    (gdb)

This has been the behaviour since the command was first added (when
the documentation was still as above) in this commit:

    commit f3e9a8177c41893858fce2bdf339dbe90b3a4ef5
    Date:   Wed Feb 24 21:18:28 2010 +0000

Since that commit the code in question has moved around, but the
important parts are largely unchanged.  The function in question is
now in py-progspace.c:pspy_block_for_pc.

Examining the code shows that the real state is more complex than just
the function throws an error instead of returning None, instead the
real situation is:

  1. If we can't find a compilation unit for the $pc value then we
  throw an error, but

  2. If we can find a compilation unit, but can't find a block within
  the compilation unit for the $pc then return None.

I suspect for most users of the Python API this distinction is
irrelevant, and I propose that we standardise on one single failure
mechanism.

Given the function can currently return None in some cases, and is
documented to return None on error, I propose we make that the case
for all error paths, which is what this patch does.

As the Progspace.block_for_pc method is currently untested, I've added
some basic tests including for a call with an invalid $pc.

This is potentially an API breaking change, though an undocumented
part of the API.  Also, users should have been checking and handling a
None return value anyway, so my hope is that this shouldn't be too
disruptive.

gdb/ChangeLog:

	* python/py-progspace.c (pspy_block_for_pc): Return None for all
	error paths.

gdb/testsuite/ChangeLog:

	* gdb.python/py-progspace.exp: Add tests for the
	Progspace.block_for_pc method.

Change-Id: I9cea8d2132902bcad0013d1fd39080dd5423cc57

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 09adb916c0aca0426940bccaf2a30ed7fe6893cf..887c7fb68ae59314fd55b002dc3036e90ace97d9 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-10-24  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+       * python/py-progspace.c (pspy_block_for_pc): Return None for all
+       error paths.
+
 2019-10-23  Tom Tromey  <tom@tromey.com>
 
        * arc-tdep.c: Remove ".." from include.

diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 4483d036ace5206e311742c8c46242db864347ee..bdb70723e562f0c358fc4393c4ca37052b9064cb 100644 (file)
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -397,11 +397,7 @@ pspy_block_for_pc (PyObject *o, PyObject *args)
     }
 
   if (cust == NULL || COMPUNIT_OBJFILE (cust) == NULL)
-    {
-      PyErr_SetString (PyExc_RuntimeError,
-                      _("Cannot locate object file for block."));
-      return NULL;
-    }
+    Py_RETURN_NONE;
 
   if (block)
     return block_to_block_object (block, COMPUNIT_OBJFILE (cust));

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0d1fed93e69633278312b9216374a8d80927a550..12ef1fe4954cba9f865091c6f1cd1b0547881851 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-10-24  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+       * gdb.python/py-progspace.exp: Add tests for the
+       Progspace.block_for_pc method.
+
 2019-10-23  Tom Tromey  <tom@tromey.com>
 
        * configure: Rebuild.

diff --git a/gdb/testsuite/gdb.python/py-progspace.exp b/gdb/testsuite/gdb.python/py-progspace.exp
index 539438246745c1506745c0d1739c59c214282e8b..d1bcb81bb47c31c9305797807aea6b3fd7b4a90b 100644 (file)
--- a/gdb/testsuite/gdb.python/py-progspace.exp
+++ b/gdb/testsuite/gdb.python/py-progspace.exp
@@ -57,6 +57,20 @@ if {![runto_main]} {
     return
 }
 
+# Check we can get a block for the current $pc.
+set pc_val [get_integer_valueof "\$pc" 0]
+gdb_py_test_silent_cmd "python blk = gdb.current_progspace ().block_for_pc (${pc_val})" \
+    "get block for the current \$pc" 1
+gdb_test "python print blk.start <= ${pc_val}" "True" \
+    "block start is before \$pc"
+gdb_test "python print blk.end >= ${pc_val}" "True" \
+    "block end is after \$pc"
+
+# Check what happens when we ask for a block of an invalid address.
+if ![is_address_zero_readable] {
+    gdb_test "python print gdb.current_progspace ().block_for_pc (0)" "None"
+}
+
 # With a single inferior, progspace.objfiles () and gdb.objfiles () should
 # be identical.
 gdb_test "python print (progspace.objfiles () == gdb.objfiles ())" "True"