[binutils-gdb] gdb/python: fix use of frame_info_ptr within pending_frame_object

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e7fce05fcf47c487b4f996f08a68f5818d1d41ca

commit e7fce05fcf47c487b4f996f08a68f5818d1d41ca
Author: Andrew Burgess <[email protected]>
Date:   Thu May 14 10:42:59 2026 +0100

    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 <[email protected]>

Diff:
---
 gdb/python/py-unwind.c | 53 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 20 deletions(-)

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<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