gdb/python: add PENDING_FRAMEPY_REQUIRE_VALID macro in py-unwind.c

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.

Reviewed-By: Tom Tromey <tom@tromey.com>
This commit is contained in:
Andrew Burgess 2023-03-09 10:58:54 +00:00
parent 3194ca90fe
commit 44d9b0a174
3 changed files with 54 additions and 27 deletions

View file

@ -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)
{
unwind_info_object *unwind_info
= PyObject_New (unwind_info_object, &unwind_info_object_type);
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);
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, &regnum))
@ -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);