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 <tom@tromey.com>

diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index eef926c6dfa3ae8dae8fb24cb60549348b33a166..7ec87367d0a45c3dfd309a6b2144b606d5eca663 100644 (file)
--- 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