uefi-ref/binutils-gdb
1
0
Fork 0
mirror of https://sourceware.org/git/binutils-gdb.git synced 2026-08-27 00:26:02 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

gdb/python: fix use of frame_info_ptr within pending_frame_object

Browse source 
The previous commit added a type trait which identifies types that
should not be used within Python objects, that is, types that are not
trivially default constructible.  As a result of this, it was
discovered that pending_frame_object includes a frame_info_ptr field.

The problem with frame_info_ptr is that its constructor registers the
new frame_info_ptr with the global frame_list.  It is by this
registration that invalidation of frame_info_ptr objects is performed.

As Python is written in C, C++ constructors are not called, so when a
pending_frame_object is created the constructor for the nested
frame_info_ptr field is never run, and the frame_info_ptr is never
registered with the global frame_list.  As a result the frame_info_ptr
will never be invalidated if the frame cache is flushed, this can then
lead to problems where we make use of the 'frame_info *' within the
frame_info_ptr, even though it is no longer valid.

In this commit I change the frame_info_ptr within pending_frame_object
to a 'frame_info_ptr *' and allocate the frame_info_ptr object on the
heap, releasing the object, and resetting the point to NULL, when we
are done with it.  As the pending_frame_object only needs to remain
valid for the duration of frame_unwind_python::sniff, the 'new' and
'delete' both performed within the function.

We can now check that a pending_frame_object is valid by checking if
the 'frame_info_ptr *' is NULL or not.  As the frame_info_ptr is
created in a valid state, and the point is set back to NULL when we
are done with it, we no longer need to compare the frame_info_ptr
object itself against NULL.

The remaining changes in this patch are to dereference the
'frame_info_ptr *' in places where we need the actual object.  In some
cases I need to move the dereference later within a function, after a
validity check, in order to avoid dereferencing a NULL pointer.

Finally, I can add the static_assert that guarantees that
pending_frame_object is now safe for allocation by Python.

I discovered this bug while looking at PR gdb/32120.  That bug is
about a user's custom frame unwinder that triggers a flush of the
frame cache during the sniffer phase (the
RemoteTargetConnection.send_packet call switches thread, which
triggers the frame cache flush).  While looking at that bug I noticed
that the frame_info_ptr within the pending_frame_object wasn't being
reset when the frame cache was flushed.  Fixing this does not resolve
the user's issue, but I thought it was still worth tagging this commit
with the bug link.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32120

Approved-By: Tom Tromey <tom@tromey.com>
This commit is contained in:
Andrew Burgess 2026-05-14 10:42:59 +01:00
parent 60cd57ff6e
commit e7fce05fcf
1 changed files with 33 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

53
gdb/python/py-unwind.c
View file

@ -68,13 +68,17 @@ show_pyuw_debug (struct ui_file *file, int from_tty,
struct pending_frame_object : public PyObject
{
/* Frame we are unwinding. */
frame_info_ptr frame_info;
/* Frame we are unwinding. We cannot place a frame_info_ptr
directly within this struct as it is not trivially default
constructable. */
frame_info_ptr *frame_info;
/* Its architecture, passed by the sniffer caller. */
struct gdbarch *gdbarch;
};
static_assert (gdb::is_python_allocatable_v<pending_frame_object>);
/* Saved registers array item. */
struct saved_reg
@ -245,9 +249,8 @@ unwind_infopy_repr (PyObject *self)
unwind_info_object *unwind_info = (unwind_info_object *) self;
pending_frame_object *pending_frame
= (pending_frame_object *) (unwind_info->pending_frame);
frame_info_ptr frame = pending_frame->frame_info;
if (frame == nullptr)
if (pending_frame->frame_info == nullptr)
return PyUnicode_FromFormat ("<%s for an invalid frame>",
gdbpy_py_obj_tp_name (self));
@ -263,6 +266,7 @@ unwind_infopy_repr (PyObject *self)
saved_reg_names = (saved_reg_names + ", ") + name;
}
const frame_info_ptr &frame (*pending_frame->frame_info);
return PyUnicode_FromFormat ("<%s frame #%d, saved_regs=(%s)>",
gdbpy_py_obj_tp_name (self),
frame_relative_level (frame),
@ -331,7 +335,7 @@ unwind_infopy_add_saved_register (PyObject *self, PyObject *args, PyObject *kw)
if (regnum >= gdbarch_num_cooked_regs (pending_frame->gdbarch))
{
struct value *user_reg_value
= value_of_user_reg (regnum, pending_frame->frame_info);
= value_of_user_reg (regnum, *pending_frame->frame_info);
if (user_reg_value->lval () == lval_register)
regnum = user_reg_value->regnum ();
if (regnum >= gdbarch_num_cooked_regs (pending_frame->gdbarch))
@ -414,14 +418,15 @@ unwind_infopy_dealloc (PyObject *self)
static PyObject *
pending_framepy_str (PyObject *self)
{
frame_info_ptr frame = ((pending_frame_object *) self)->frame_info;
pending_frame_object *pending_frame = (pending_frame_object *) self;
const char *sp_str = NULL;
const char *pc_str = NULL;
if (frame == NULL)
if (pending_frame->frame_info == nullptr)
return PyUnicode_FromString ("Stale PendingFrame instance");
try
{
const frame_info_ptr &frame (*pending_frame->frame_info);
sp_str = core_addr_to_string_nz (get_frame_sp (frame));
pc_str = core_addr_to_string_nz (get_frame_pc (frame));
}
@ -439,14 +444,15 @@ static PyObject *
pending_framepy_repr (PyObject *self)
{
pending_frame_object *pending_frame = (pending_frame_object *) self;
frame_info_ptr frame = pending_frame->frame_info;
if (frame == nullptr)
if (pending_frame->frame_info == nullptr)
return gdb_py_invalid_object_repr (self);
const char *sp_str = nullptr;
const char *pc_str = nullptr;
const frame_info_ptr &frame (*pending_frame->frame_info);
try
{
sp_str = core_addr_to_string_nz (get_frame_sp (frame));
@ -493,7 +499,7 @@ pending_framepy_read_register (PyObject *self, PyObject *args, PyObject *kw)
get_frame_register_value() was used here, which did not
handle the user register case. */
value *val = value_of_register
(regnum, get_next_frame_sentinel_okay (pending_frame->frame_info));
(regnum, get_next_frame_sentinel_okay (*pending_frame->frame_info));
if (val == NULL)
PyErr_Format (PyExc_ValueError,
"Cannot read register %d from frame.",
@ -520,6 +526,10 @@ pending_framepy_is_valid (PyObject *self, PyObject *args)
if (pending_frame->frame_info == nullptr)
return py_false ().release ();
/* The frame_info field should never point at an uninitialized
object. */
gdb_assert (*pending_frame->frame_info != nullptr);
return py_true ().release ();
}
@ -538,7 +548,7 @@ pending_framepy_name (PyObject *self, PyObject *args)
try
{
enum language lang;
frame_info_ptr frame = pending_frame->frame_info;
const frame_info_ptr &frame = *pending_frame->frame_info;
name = find_frame_funname (frame, &lang, nullptr);
}
@ -568,7 +578,7 @@ pending_framepy_pc (PyObject *self, PyObject *args)
try
{
pc = get_frame_pc (pending_frame->frame_info);
pc = get_frame_pc (*pending_frame->frame_info);
}
catch (const gdb_exception &except)
{
@ -590,7 +600,7 @@ pending_framepy_language (PyObject *self, PyObject *args)
try
{
frame_info_ptr fi = pending_frame->frame_info;
const frame_info_ptr &fi (*pending_frame->frame_info);
enum language lang = get_frame_language (fi);
const language_defn *lang_def = language_def (lang);
@ -615,7 +625,7 @@ pending_framepy_find_sal (PyObject *self, PyObject *args)
try
{
frame_info_ptr frame = pending_frame->frame_info;
const frame_info_ptr &frame (*pending_frame->frame_info);
symtab_and_line sal = find_frame_sal (frame);
return symtab_and_line_to_sal_object (sal).release ();
@ -636,7 +646,7 @@ pending_framepy_block (PyObject *self, PyObject *args)
PENDING_FRAMEPY_REQUIRE_VALID (pending_frame);
frame_info_ptr frame = pending_frame->frame_info;
const frame_info_ptr &frame (*pending_frame->frame_info);
const struct block *block = nullptr, *fn_block;
try
@ -682,7 +692,7 @@ pending_framepy_function (PyObject *self, PyObject *args)
try
{
enum language funlang;
frame_info_ptr frame = pending_frame->frame_info;
const frame_info_ptr &frame (*pending_frame->frame_info);
gdb::unique_xmalloc_ptr<char> funname
= find_frame_funname (frame, &funlang, &sym);
@ -774,7 +784,7 @@ pending_framepy_level (PyObject *self, PyObject *args)
PENDING_FRAMEPY_REQUIRE_VALID (pending_frame);
int level = frame_relative_level (pending_frame->frame_info);
int level = frame_relative_level (*pending_frame->frame_info);
return gdb_py_object_from_longest (level).release ();
}
@ -863,9 +873,12 @@ frame_unwind_python::sniff (const 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);
pfo->frame_info = new frame_info_ptr (this_frame);
SCOPE_EXIT
{
delete pfo->frame_info;
pfo->frame_info = nullptr;
};
/* Run unwinders. */
if (gdb_python_module == NULL