From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 04/10] gdb/python: add PENDING_FRAMEPY_REQUIRE_VALID macro in py-unwind.c
Date: Fri, 10 Mar 2023 14:55:21 +0000 [thread overview]
Message-ID: <9633a6228844ef210896ea73246525b00e131e8e.1678460067.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1678460067.git.aburgess@redhat.com>
This commit copies the pattern that is present in many other py-*.c
files: having a single macro to check that the Python object is still
valid.
This cleans up the code a little throughout the py-unwind.c file.
Some of the exception messages will change slightly with this commit,
though the type of the exceptions is still ValueError in all cases.
I started writing some tests for this change and immediately ran into
a problem: GDB would crash. It turns out that the PendingFrame
objects are not being marked as invalid!
In pyuw_sniffer where the pending frames are created, we make use of a
scoped_restore to invalidate the pending frame objects. However, this
only restores the pending_frame_object::frame_info field to its
previous value -- and it turns out we never actually give this field
an initial value, it's left undefined.
So, when the scoped_restore (called invalidate_frame) performs its
cleanup, it actually restores the frame_info field to an undefined
value. If this undefined value is not nullptr then any future
accesses to the PendingFrame object result in undefined behaviour and
most likely, a crash.
As part of this commit I now initialize the frame_info field, which
ensures all the new tests now pass.
---
gdb/python/py-unwind.c | 53 +++++++++++++-------------
gdb/testsuite/gdb.python/py-unwind.exp | 19 +++++++++
gdb/testsuite/gdb.python/py-unwind.py | 7 ++++
3 files changed, 53 insertions(+), 26 deletions(-)
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index ee776bf8dea..12b14616363 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -52,6 +52,17 @@ show_pyuw_debug (struct ui_file *file, int from_tty,
#define PYUW_SCOPED_DEBUG_ENTER_EXIT \
scoped_debug_enter_exit (pyuw_debug, "py-unwind")
+/* Require a valid pending frame. */
+#define PENDING_FRAMEPY_REQUIRE_VALID(pending_frame) \
+ do { \
+ if ((pending_frame)->frame_info == nullptr) \
+ { \
+ PyErr_SetString (PyExc_ValueError, \
+ _("gdb.PendingFrame is invalid.")); \
+ return nullptr; \
+ } \
+ } while (0)
+
struct pending_frame_object
{
PyObject_HEAD
@@ -215,21 +226,20 @@ unwind_infopy_str (PyObject *self)
}
/* Create UnwindInfo instance for given PendingFrame and frame ID.
- Sets Python error and returns NULL on error. */
+ Sets Python error and returns NULL on error.
+
+ The PYO_PENDING_FRAME object must be valid. */
static PyObject *
pyuw_create_unwind_info (PyObject *pyo_pending_frame,
struct frame_id frame_id)
{
+ gdb_assert (((pending_frame_object *) pyo_pending_frame)->frame_info
+ != nullptr);
+
unwind_info_object *unwind_info
- = PyObject_New (unwind_info_object, &unwind_info_object_type);
+ = PyObject_New (unwind_info_object, &unwind_info_object_type);
- if (((pending_frame_object *) pyo_pending_frame)->frame_info == NULL)
- {
- PyErr_SetString (PyExc_ValueError,
- "Attempting to use stale PendingFrame");
- return NULL;
- }
unwind_info->frame_id = frame_id;
Py_INCREF (pyo_pending_frame);
unwind_info->pending_frame = pyo_pending_frame;
@@ -365,15 +375,11 @@ static PyObject *
pending_framepy_read_register (PyObject *self, PyObject *args)
{
pending_frame_object *pending_frame = (pending_frame_object *) self;
+ PENDING_FRAMEPY_REQUIRE_VALID (pending_frame);
+
int regnum;
PyObject *pyo_reg_id;
- if (pending_frame->frame_info == NULL)
- {
- PyErr_SetString (PyExc_ValueError,
- "Attempting to read register from stale PendingFrame");
- return NULL;
- }
if (!PyArg_UnpackTuple (args, "read_register", 1, 1, &pyo_reg_id))
return NULL;
if (!gdbpy_parse_register_id (pending_frame->gdbarch, pyo_reg_id, ®num))
@@ -417,6 +423,8 @@ pending_framepy_create_unwind_info (PyObject *self, PyObject *args)
CORE_ADDR pc;
CORE_ADDR special;
+ PENDING_FRAMEPY_REQUIRE_VALID ((pending_frame_object *) self);
+
if (!PyArg_ParseTuple (args, "O:create_unwind_info", &pyo_frame_id))
return NULL;
if (!pyuw_object_attribute_to_pointer (pyo_frame_id, "sp", &sp))
@@ -451,12 +459,8 @@ pending_framepy_architecture (PyObject *self, PyObject *args)
{
pending_frame_object *pending_frame = (pending_frame_object *) self;
- if (pending_frame->frame_info == NULL)
- {
- PyErr_SetString (PyExc_ValueError,
- "Attempting to read register from stale PendingFrame");
- return NULL;
- }
+ PENDING_FRAMEPY_REQUIRE_VALID (pending_frame);
+
return gdbarch_to_arch_object (pending_frame->gdbarch);
}
@@ -467,12 +471,8 @@ pending_framepy_level (PyObject *self, PyObject *args)
{
pending_frame_object *pending_frame = (pending_frame_object *) self;
- if (pending_frame->frame_info == NULL)
- {
- PyErr_SetString (PyExc_ValueError,
- "Attempting to read stack level from stale PendingFrame");
- return NULL;
- }
+ PENDING_FRAMEPY_REQUIRE_VALID (pending_frame);
+
int level = frame_relative_level (pending_frame->frame_info);
return gdb_py_object_from_longest (level).release ();
}
@@ -538,6 +538,7 @@ pyuw_sniffer (const struct frame_unwind *self, frame_info_ptr this_frame,
return 0;
}
pfo->gdbarch = gdbarch;
+ pfo->frame_info = nullptr;
scoped_restore invalidate_frame = make_scoped_restore (&pfo->frame_info,
this_frame);
diff --git a/gdb/testsuite/gdb.python/py-unwind.exp b/gdb/testsuite/gdb.python/py-unwind.exp
index 337e5dc2504..3e214ee0f45 100644
--- a/gdb/testsuite/gdb.python/py-unwind.exp
+++ b/gdb/testsuite/gdb.python/py-unwind.exp
@@ -142,3 +142,22 @@ gdb_test "python obj = simple_unwinder(True)" \
[multi_line \
"TypeError: incorrect type for name: <class 'bool'>" \
"Error while executing Python code\\."]
+
+# Now register the simple_unwinder with a valid name, and use the
+# unwinder to capture a PendingFrame object.
+gdb_test_no_output "python obj = simple_unwinder(\"simple\")"
+gdb_test_no_output "python gdb.unwinder.register_unwinder(None, obj)"
+check_for_broken_backtrace "backtrace to capture a PendingFrame object"
+
+# Call methods on the captured gdb.PendingFrame and check we see the
+# expected error.
+gdb_test_no_output "python pf = captured_pending_frame"
+foreach cmd {"pf.read_register(\"pc\")" \
+ "pf.create_unwind_info(None)" \
+ "pf.architecture()" \
+ "pf.level()"} {
+ gdb_test "python $cmd" \
+ [multi_line \
+ "ValueError: gdb\\.PendingFrame is invalid\\." \
+ "Error while executing Python code\\."]
+}
diff --git a/gdb/testsuite/gdb.python/py-unwind.py b/gdb/testsuite/gdb.python/py-unwind.py
index edd2e30eb9b..b30e843e7e5 100644
--- a/gdb/testsuite/gdb.python/py-unwind.py
+++ b/gdb/testsuite/gdb.python/py-unwind.py
@@ -133,12 +133,19 @@ class TestUnwinder(Unwinder):
global_test_unwinder = TestUnwinder()
gdb.unwinder.register_unwinder(None, global_test_unwinder, True)
+# This is filled in by the simple_unwinder class.
+captured_pending_frame = None
+
class simple_unwinder(Unwinder):
def __init__(self, name):
super().__init__(name)
def __call__(self, pending_frame):
+ global captured_pending_frame
+
+ if captured_pending_frame is None:
+ captured_pending_frame = pending_frame
return None
--
2.25.4
next prev parent reply other threads:[~2023-03-10 14:55 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-10 14:55 [PATCH 00/10] Improvements & Cleanup For Python Unwinder API Andrew Burgess
2023-03-10 14:55 ` [PATCH 01/10] gdb/doc: spring clean the Python unwinders documentation Andrew Burgess
2023-03-10 15:24 ` Eli Zaretskii
2023-03-14 9:27 ` Andrew Burgess
2023-03-14 12:56 ` Eli Zaretskii
2023-03-16 14:30 ` Andrew Burgess
2023-03-10 14:55 ` [PATCH 02/10] gdb/python: make the gdb.unwinder.Unwinder class more robust Andrew Burgess
2023-03-10 15:32 ` Eli Zaretskii
2023-03-14 10:06 ` Andrew Burgess
2023-03-14 12:57 ` Eli Zaretskii
2023-03-31 2:15 ` Simon Marchi
2023-04-03 10:02 ` Andrew Burgess
2023-03-10 14:55 ` [PATCH 03/10] gdb/python: remove unneeded nullptr check in frapy_block Andrew Burgess
2023-03-10 14:55 ` Andrew Burgess [this message]
2023-03-10 14:55 ` [PATCH 05/10] gdb/python: add some additional methods to gdb.PendingFrame Andrew Burgess
2023-03-10 15:42 ` Eli Zaretskii
2023-03-14 10:18 ` Andrew Burgess
2023-03-14 12:59 ` Eli Zaretskii
2023-03-16 14:28 ` Andrew Burgess
2023-03-16 14:46 ` Eli Zaretskii
2023-03-16 17:26 ` Andrew Burgess
2023-03-16 19:54 ` Eli Zaretskii
2023-03-10 14:55 ` [PATCH 06/10] gdb/python: add __repr__ for PendingFrame and UnwindInfo Andrew Burgess
2023-03-10 14:55 ` [PATCH 07/10] gdb/python: remove Py_TPFLAGS_BASETYPE from gdb.UnwindInfo Andrew Burgess
2023-03-10 14:55 ` [PATCH 08/10] gdb: have value_as_address call unpack_pointer Andrew Burgess
2023-03-10 15:28 ` Tom Tromey
2023-03-10 22:08 ` Andrew Burgess
2023-03-10 14:55 ` [PATCH 09/10] gdb/python: Allow gdb.UnwindInfo to be created with non gdb.Value args Andrew Burgess
2023-03-10 15:34 ` Tom Tromey
2023-03-10 22:16 ` Andrew Burgess
2023-03-11 14:47 ` Tom Tromey
2023-03-10 15:38 ` Eli Zaretskii
2023-03-10 14:55 ` [PATCH 10/10] gdb/python: Add new gdb.unwinder.FrameId class Andrew Burgess
2023-03-10 15:36 ` Eli Zaretskii
2023-03-14 10:58 ` Andrew Burgess
2023-03-14 13:00 ` Eli Zaretskii
2023-03-29 16:27 ` [PATCH 00/10] Improvements & Cleanup For Python Unwinder API Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9633a6228844ef210896ea73246525b00e131e8e.1678460067.git.aburgess@redhat.com \
--to=aburgess@redhat.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).