* [PATCH 2/2] gdb/python: remove frame_object::frame_id_is_next as unnecessary
2026-06-02 9:54 [PATCH 0/2] Cleanup in Python frame_object Andrew Burgess
2026-06-02 9:54 ` [PATCH 1/2] gdb/testsuite: don't overwrite test executable in py-frame.exp Andrew Burgess
@ 2026-06-02 9:54 ` Andrew Burgess
2026-07-08 12:51 ` [PATCH 0/2] Cleanup in Python frame_object Andrew Burgess
2 siblings, 0 replies; 4+ messages in thread
From: Andrew Burgess @ 2026-06-02 9:54 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
Remove the frame_object::frame_id_is_next field. This has been part
of how Python handles frames since this code was first added in commit
f8f6f20b6e37e6d219940fcad58b1f66124d11c1 back in 2009. The motivation
for this field can be found in a couple of comments, there's this one
on frame_id_is_next:
/* Marks that the FRAME_ID member actually holds the ID of the frame next
to this, and not this frames' ID itself. This is a hack to permit Python
frame objects which represent invalid frames (i.e., the last frame_info
in a corrupt stack). The problem arises from the fact that this code
relies on FRAME_ID to uniquely identify a frame, which is not always true
for the last "frame" in a corrupt stack (it can have a null ID, or the same
ID as the previous frame). Whenever get_prev_frame returns NULL, we
record the frame_id of the next frame and set FRAME_ID_IS_NEXT to 1. */
And this one in frame_info_to_frame_object:
/* Try to get the previous frame, to determine if this is the last frame
in a corrupt stack. If so, we need to store the frame_id of the next
frame and not of this one (which is possibly invalid). */
This field is dealing with a problem that was present in older
versions of GDB where not every stack frame had a valid frame-id, if
the stack was corrupted in some way then the last frame might have an
invalid frame-id. However, that is no longer the case. With current
GDB there is a promise that every frame has a valid frame-id. I don't
have a single commit to point to where this became the reality, but it
is my understanding of current GDB.
As an example, the frame-id of every frame (except #0) is computed as
the frame is created, any frames with a duplicate frame-id, or any
errors during computation of the frame-id, and the new frame is
discarded.
Additionally, our frame_info_ptr::reinflate mechanism relies on unique
and valid frame-ids.
The problem with the existing code is that the last frame in a
corrupted stack will hold the frame-id of the next frame, that is, the
more inner frame. This means we have two frames holding the same
frame-id, and the only difference is the frame_id_is_next flag.
However, the frapy_str function, which prints a string representation
of the frame, doesn't take frame_id_is_next into account, so printing
the last two frames in a corrupted stack, will print the same
frame-id.
We could fix this in frapy_str and also frapy_repr by checking the
frame_id_is_next flag and then fetching the previous frame-id, but
this would still assume that the previous frame has a valid-id, so we
might as well just drop the frame_id_is_next flag and make everything
simpler.
There's a new test which exposes the incorrectly printed frame-id
problem.
While testing I needed to "fix" the results for two existing tests.
In frame_info_to_frame_object, in order to figure out if we should use
the next frame, we called get_prev_frame. This would cause GDB to
always unwind 1 extra level of the stack.
What this means is that, if there is an error, or some diagnostic
output, when unwinding frame #1, then this will show up when trying to
access frame #0 via the Python API as frame_info_to_frame_object on
frame #0 would call get_prev_frame, which would then unwind frame #1.
After this commit this is no longer the case. We only see
output (either errors, or diagnostic output) associated with frame #1
when we actually try to unwind to frame #1. The two tests that needed
updating were expecting output associated with frame #1 while printing
frame #0. This is now fixed and the output for frame #1 occurs later
on. I think this is an improvement.
---
gdb/python/py-frame.c | 33 +---
gdb/testsuite/gdb.python/py-frame.exp | 33 ++++
gdb/testsuite/gdb.python/py-frame.py | 187 ++++++++++++++++++
.../gdb.python/py-pending-frame-level.exp | 2 +-
gdb/testsuite/gdb.python/py-unwind.exp | 4 +-
5 files changed, 226 insertions(+), 33 deletions(-)
create mode 100644 gdb/testsuite/gdb.python/py-frame.py
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 0a1a0dffbf9..367c08917cd 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -32,16 +32,6 @@ struct frame_object : public PyObject
{
struct frame_id frame_id;
struct gdbarch *gdbarch;
-
- /* Marks that the FRAME_ID member actually holds the ID of the frame next
- to this, and not this frames' ID itself. This is a hack to permit Python
- frame objects which represent invalid frames (i.e., the last frame_info
- in a corrupt stack). The problem arises from the fact that this code
- relies on FRAME_ID to uniquely identify a frame, which is not always true
- for the last "frame" in a corrupt stack (it can have a null ID, or the same
- ID as the previous frame). Whenever get_prev_frame returns NULL, we
- record the frame_id of the next frame and set FRAME_ID_IS_NEXT to 1. */
- int frame_id_is_next;
};
static_assert (gdb::is_python_allocatable_v<frame_object>);
@@ -69,9 +59,6 @@ frame_object_to_frame_info (PyObject *obj)
if (frame == NULL)
return NULL;
- if (frame_obj->frame_id_is_next)
- frame = get_prev_frame (frame);
-
return frame;
}
@@ -371,22 +358,7 @@ frame_info_to_frame_object (const frame_info_ptr &frame)
try
{
-
- /* Try to get the previous frame, to determine if this is the last frame
- in a corrupt stack. If so, we need to store the frame_id of the next
- frame and not of this one (which is possibly invalid). */
- if (get_prev_frame (frame) == NULL
- && get_frame_unwind_stop_reason (frame) != UNWIND_NO_REASON
- && get_next_frame (frame) != NULL)
- {
- frame_obj->frame_id = get_frame_id (get_next_frame (frame));
- frame_obj->frame_id_is_next = 1;
- }
- else
- {
- frame_obj->frame_id = get_frame_id (frame);
- frame_obj->frame_id_is_next = 0;
- }
+ frame_obj->frame_id = get_frame_id (frame);
frame_obj->gdbarch = get_frame_arch (frame);
}
catch (const gdb_exception &except)
@@ -730,8 +702,7 @@ frapy_richcompare (PyObject *self, PyObject *other, int op)
frame_object *self_frame = (frame_object *) self;
frame_object *other_frame = (frame_object *) other;
- if (self_frame->frame_id_is_next == other_frame->frame_id_is_next
- && self_frame->frame_id == other_frame->frame_id)
+ if (self_frame->frame_id == other_frame->frame_id)
result = Py_EQ;
else
result = Py_NE;
diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp
index 9c425f6aa98..2bac1b6f5b9 100644
--- a/gdb/testsuite/gdb.python/py-frame.exp
+++ b/gdb/testsuite/gdb.python/py-frame.exp
@@ -203,6 +203,39 @@ gdb_test "python print(gdb.selected_frame().read_register(bad_object))" \
".*Invalid type for register.*" \
"test Frame.read_register with bad_type object"
+# Load the Python script, this includes a frame unwinder which can be
+# used to "corrupt" the backtrace by introducing a cycle.
+set remote_python_file [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
+gdb_test_no_output "source ${remote_python_file}" "load python file"
+
+# Check that the frame-id as read by Python is correct for each frame
+# in the backtrace. When STOP_AT_LEVEL is None then the stack is not
+# corrupted. For other STOP_AT_LEVEL values a cycle is introduced
+# into the stack after the numbered level. E.g. for STOP_AT_LEVEL 0,
+# frame #1 will appear to have the same frame-id as #0, which
+# terminates the unwind.
+foreach_with_prefix stop_at_level { None 0 1 2 } {
+ gdb_test_no_output "python global_unwinder.stop_at_level = $stop_at_level"
+
+ if { $stop_at_level eq "None" } {
+ set frame_limit 3
+ } else {
+ set frame_limit [expr {$stop_at_level + 1}]
+ }
+
+ for { set i 0 } { $i < $frame_limit } { incr i } {
+ gdb_test "frame $i" ".*" "select frame $i"
+
+ set expected_output [capture_command_output "maint print frame-id" \
+ [string_to_regexp "frame-id for frame #$i: "]]
+ set python_output [capture_command_output \
+ "python print(gdb.selected_frame())" ""]
+
+ gdb_assert { $python_output eq $expected_output } \
+ "frame-id for frame $i"
+ }
+}
+
# Compile again without debug info.
if { [prepare_for_testing "failed to prepare" ${testfile}-nodebug ${srcfile} {}] } {
return
diff --git a/gdb/testsuite/gdb.python/py-frame.py b/gdb/testsuite/gdb.python/py-frame.py
new file mode 100644
index 00000000000..320f977fb2b
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-frame.py
@@ -0,0 +1,187 @@
+# Copyright (C) 2026 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# A class which is a GDB frame unwinder. This frame unwinder can be
+# used to corrupt the backtrace by tricking GDB into believing that
+# there is a repeated frame-id in the backtrace.
+#
+# Sourcing this script creates a global called 'global_unwinder' and
+# registers a global unwinder called 'stop-at-level'. The unwinder
+# needs to be primed with the frame-ids copied from the current stack,
+# this can be done by enabling the unwinder using the GDB command:
+#
+# (gdb) enable unwinder global stop-at-level
+#
+# You then need to set the level at which to corrupt the stack, e.g.:
+#
+# (gdb) python global_unwinder.stop_at_level = 3
+#
+# This will allow the frames #0, #1, #2, and #3 to print correctly,
+# but frame #4 will appear to be a repeat of frame #3, so GDB will
+# terminate the backtrace.
+#
+# As the unwinder relies on cached frame-ids, then resuming the
+# inferior for which the frame-ids were cached will cause the unwinder
+# to auto-disable itself, after which you'll need to re-enable the
+# next time the inferior stops.
+#
+# Changing the 'global_unwinder.stop_at_level' will flush the frame
+# cache, so you can test breaking the stack at different levels.
+
+
+import re
+import gdb
+from gdb.unwinder import Unwinder, FrameId
+
+
+class corrupt_stack_unwinder(Unwinder):
+ def __init__(self):
+ # Is this unwinder enabled or not? Accessed via the 'enabled'
+ # property.
+ self._enabled = True
+
+ # List of FrameId instances, one for each stack frame. This list is
+ # populated when this file is sourced into GDB.
+ self._frame_ids = []
+
+ # The inferior for which we cached self._frame_ids. The
+ # unwinder will only work within this inferior.
+ self._inferior = None
+
+ # At what stack level should we break the unwind?
+ self._stop_at_level = None
+
+ Unwinder.__init__(self, "stop-at-level")
+
+ if gdb.selected_inferior().pid > 0:
+ self._update_frame_id_cache()
+ else:
+ self._enabled = False
+
+ gdb.events.cont.connect(lambda ev: self._continued_event_handler(ev))
+
+ # Assume that the current inferior has a stack. Collect a FrameId
+ # object for each level of the current stack.
+ def _update_frame_id_cache(self):
+ self._frame_ids = []
+ self._inferior = gdb.selected_inferior()
+
+ frame = gdb.newest_frame()
+ while frame is not None:
+ # Get the frame-id in a verbose text form.
+ output = gdb.execute(
+ "maint print frame-id %d" % frame.level(), to_string=True
+ )
+
+ # Parse the frame-id in OUTPUT, find the stack and code addresses.
+ match = re.search(r"stack=(0x[0-9a-fA-F]+).*?code=(0x[0-9a-fA-F]+)", output)
+ if not match:
+ raise gdb.GdbError("Could not parse frame-id for frame #%d" % frame.level())
+
+ # Create the FrameId object.
+ sp_addr = int(match.group(1), 16)
+ pc_addr = int(match.group(2), 16)
+ self._frame_ids.append(FrameId(sp_addr, pc_addr))
+
+ frame = frame.older()
+
+
+ # Called when an inferior continues. If this is the inferior for
+ # which this unwinder collected the stack, then discard the
+ # collected FrameId objects and disable the unwinder.
+ def _continued_event_handler(self, event):
+ if gdb.selected_inferior() == self._inferior:
+ self._enabled = False
+ self._frame_ids = []
+ self._inferior = None
+
+ # To aid debugging, print the captured FrameId instances.
+ def print_frame_ids(self):
+ for level, fid in enumerate(self._frame_ids):
+ print(
+ "frame-id for frame #%s: {stack=0x%x,code=0x%x}"
+ % (level, fid.sp, fid.pc)
+ )
+
+ def __call__(self, pending_frame):
+ if self.stop_at_level is None or pending_frame.level() != self.stop_at_level:
+ return None
+
+ if gdb.selected_inferior() != self._inferior:
+ return None
+
+ if len(self._frame_ids) <= self.stop_at_level:
+ raise gdb.GdbError("not enough parsed frame-ids")
+
+ # Set the frame-id for this frame to its actual, expected
+ # frame-id, which we captured in the FRAME_IDS list.
+ unwinder = pending_frame.create_unwind_info(self._frame_ids[self.stop_at_level])
+
+ # Provide the register values for the caller frame, that is,
+ # the frame at 'STOP_AT_LEVEL + 1'.
+ #
+ # We forward all of the register values unchanged from this
+ # frame.
+ #
+ # What this means is that, as far as GDB is concerned, the
+ # caller frame will appear to be identical to this frame. Of
+ # particular importance, we send $pc and $sp unchanged to the
+ # caller frame.
+ #
+ # Because the caller frame has the same $pc and $sp as this
+ # frame, GDB will compute the same frame-id for the caller
+ # frame as we just supplied for this frame (above). This
+ # creates the artificial frame cycle which is the whole point
+ # of this test.
+ #
+ # NOTE: Forwarding all registers unchanged like this to the
+ # caller frame is not how you'd normally write a frame
+ # unwinder. Some registers might indeed be unmodified between
+ # frames, but we'd usually expect the $sp and/or the $pc to
+ # change. This test is deliberately doing something weird in
+ # order to force a cycle, and so test GDB.
+ for reg in pending_frame.architecture().registers("general"):
+ val = pending_frame.read_register(reg)
+ # Having unavailable registers leads to a fall back to the standard
+ # unwinders. Don't add unavailable registers to avoid this.
+ if str(val) == "<unavailable>":
+ continue
+ unwinder.add_saved_register(reg, val)
+ return unwinder
+
+ @property
+ def enabled(self):
+ return self._enabled
+
+ @enabled.setter
+ def enabled(self, val):
+ if val:
+ self._enabled = False
+ self._update_frame_id_cache()
+
+ self._enabled = val
+
+ @property
+ def stop_at_level(self):
+ return self._stop_at_level
+
+ @stop_at_level.setter
+ def stop_at_level(self, val):
+ self._stop_at_level = val
+ gdb.invalidate_cached_frames()
+
+
+global_unwinder = corrupt_stack_unwinder()
+gdb.unwinder.register_unwinder(None, global_unwinder, True)
diff --git a/gdb/testsuite/gdb.python/py-pending-frame-level.exp b/gdb/testsuite/gdb.python/py-pending-frame-level.exp
index 0e10b8065d1..102b1c2d740 100644
--- a/gdb/testsuite/gdb.python/py-pending-frame-level.exp
+++ b/gdb/testsuite/gdb.python/py-pending-frame-level.exp
@@ -51,8 +51,8 @@ gdb_test_no_output "source ${pyfile}"\
# unwinder mixed in.
gdb_test_sequence "bt" "Backtrace with extra Python output" {
"Func f0, Level 0"
- "Func f1, Level 1"
"\\r\\n#0 \[^\r\n\]* f0 \\(\\) at "
+ "Func f1, Level 1"
"\\r\\n#1 \[^\r\n\]* f1 \\(\\) at "
"Func f2, Level 2"
"\\r\\n#2 \[^\r\n\]* f2 \\(\\) at "
diff --git a/gdb/testsuite/gdb.python/py-unwind.exp b/gdb/testsuite/gdb.python/py-unwind.exp
index af864dceedf..7cdc1e8052c 100644
--- a/gdb/testsuite/gdb.python/py-unwind.exp
+++ b/gdb/testsuite/gdb.python/py-unwind.exp
@@ -249,7 +249,9 @@ with_test_prefix "bad object unwinder" {
gdb_test_no_output "python obj = bad_object_unwinder(\"bad-object\")"
gdb_test_no_output "python gdb.unwinder.register_unwinder(None, obj, replace=True)"
gdb_test "backtrace" \
- "Python Exception <class 'gdb.error'>: an Unwinder should return gdb.UnwindInfo, not bad_object_unwinder.+Blah\\.\r\n.*"
+ [multi_line \
+ "#0 corrupt_frame_inner \\(\\) at \[^\r\n\]+" \
+ "an Unwinder should return gdb.UnwindInfo, not bad_object_unwinder.+Blah\\."]
}
# Gather information about every frame.
--
2.25.4
^ permalink raw reply [flat|nested] 4+ messages in thread