From patchwork Wed Oct 23 13:30:58 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 35245 Received: (qmail 119970 invoked by alias); 23 Oct 2019 13:31:03 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 119836 invoked by uid 89); 23 Oct 2019 13:31:03 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=locate, disruptive, sk:gdb.cur, examining X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 23 Oct 2019 13:31:01 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 11FFE2041F; Wed, 23 Oct 2019 09:31:00 -0400 (EDT) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id CD0692018E for ; Wed, 23 Oct 2019 09:30:58 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id A8B502192E for ; Wed, 23 Oct 2019 09:30:58 -0400 (EDT) X-Gerrit-PatchSet: 1 Date: Wed, 23 Oct 2019 09:30:58 -0400 From: "Andrew Burgess (Code Review)" To: gdb-patches@sourceware.org Message-ID: Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange Subject: [review] gdb/python: Return None from Progspace.block_for_pc on error X-Gerrit-Change-Id: I9cea8d2132902bcad0013d1fd39080dd5423cc57 X-Gerrit-Change-Number: 223 X-Gerrit-ChangeURL: X-Gerrit-Commit: f42e28c3c742694df402dad4c1606f1321ea438e References: Reply-To: andrew.burgess@embecosm.com, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3 Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/223 ...................................................................... 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 "", line 1, in 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 --- M gdb/ChangeLog M gdb/python/py-progspace.c M gdb/testsuite/ChangeLog M gdb/testsuite/gdb.python/py-progspace.exp 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 253c74f..91311d1 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2019-10-23 Andrew Burgess + + * python/py-progspace.c (pspy_block_for_pc): Return None for all + error paths. + 2019-10-23 Tom de Vries PR breakpoints/24687 diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c index 4483d03..bdb7072 100644 --- a/gdb/python/py-progspace.c +++ b/gdb/python/py-progspace.c @@ -397,11 +397,7 @@ } 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 9f826a6..3688f80 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2019-10-23 Andrew Burgess + + * gdb.python/py-progspace.exp: Add tests for the + Progspace.block_for_pc method. + 2019-10-21 Tom de Vries * gdb.base/infcall-nested-structs.c: Add diff --git a/gdb/testsuite/gdb.python/py-progspace.exp b/gdb/testsuite/gdb.python/py-progspace.exp index 5394382..d1bcb81 100644 --- a/gdb/testsuite/gdb.python/py-progspace.exp +++ b/gdb/testsuite/gdb.python/py-progspace.exp @@ -57,6 +57,20 @@ 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"