From: Andrew Burgess Date: Thu, 14 May 2026 09:42:59 +0000 (+0100) Subject: gdb/python: fix use of frame_info_ptr within pending_frame_object X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e7fce05fcf47c487b4f996f08a68f5818d1d41ca;p=thirdparty%2Fbinutils-gdb.git gdb/python: fix use of frame_info_ptr within pending_frame_object 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 --- diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c index eef926c6dfa..7ec87367d0a 100644 --- a/gdb/python/py-unwind.c +++ b/gdb/python/py-unwind.c @@ -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); + /* 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 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