Re: [PATCH 1/2] gdb: use get_current_frame consistently in print_stop_location

Tom de Vries
2026-04-09T06:42:35Z

[-- Attachment #1: Type: text/plain, Size: 2598 bytes --]

On 4/5/26 12:12 PM, Andrew Burgess wrote:
> In print_stop_location, in the PRINT_UNKNOWN case we currently use a
> strange mix of get_current_frame and get_selected_frame. This works
> fine because at the point print_stop_location is called the selected
> frame will always be the current frame, but calling these two
> different functions is confusing, at least for me.
>
> As we are stopping, and deciding whether to print information about
> the frame, it makes sense, I think, to make the choice based on the
> current frame, and so let's call get_current_frame once, and then use
> that result throughout the decision making process.
>
> There should be no user visible changes after this commit.

Hi Andrew,

thanks for looking into this, for me using these two different functions
is also confusing.

While reviewing this patch I wondered why this line at the end of the
function still uses get_selected_frame:
...
print_stack_frame (get_selected_frame (nullptr), 0, source_flag, 1);
...
so I tried to use get_current_frame () there as well, and ran into
trouble in these ada test-cases:
...
1 gdb.ada/catch_assert_if.exp:
6 gdb.ada/catch_ex.exp:
6 gdb.ada/excep_handle.exp:
1 gdb.ada/mi_catch_assert.exp:
6 gdb.ada/mi_catch_ex.exp:
3 gdb.ada/mi_catch_ex_hand.exp:
1 gdb.ada/mi_ex_cond.exp:
...

I investigated this, and found that bpstat_print may change the selected
frame, specifically ada_catchpoint::print_it which calls
ada_find_printable_frame. As it happens, ada_catchpoint::print_it
returns PRINT_SRC_AND_LOC, so it doesn't interact with the PRINT_UNKNOWN
case.

But if I consider the scenario where ada_catchpoint::print_it would
return PRINT_UNKNOWN, it seems more logical to me to use the selected
frame there as well.

That is, if we're stepping in some function foo, and run into an ada
catchpoint, and ada_catchpoint::print_it selects the frame in foo, and
ada_catchpoint::print_it returns PRINT_UNKNOWN, you want to use the
selected frame in the PRINT_UNKNOWN case to select SRC_LINE, which is
appropriate because as far as the user can see, we haven't left foo.

So I'm proposing a change to this patch (attached below, doesn't apply
to the first but to the second patch) that:
- introduces a variable print_frame
- assigns get_selected_frame (nullptr) to print_frame
- adds a comment explaining how print_frame relates to the stop frame
- uses print_frame everywhere in the function

I tested the series in combination with the attached patch on
x86_64-linux, and found no regressions.

Thanks,
- Tom

[-- Attachment #2: tmp.patch --]
[-- Type: text/x-patch, Size: 1782 bytes --]

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 344f13df4fa..687390f24dd 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -9349,18 +9349,22 @@ print_stop_location (const target_waitstatus &ws)
struct thread_info *tp = inferior_thread ();

bpstat_ret = bpstat_print (tp->control.stop_bpstat, ws.kind ());
+ /* Function bpstat_print selects the frame to print. Typically, that is the
+ stop frame, in other words get_current_frame (). But bpstat_print may
+ select a different frame, see for instance ada_catchpoint::print_it. */
+ frame_info_ptr print_frame = get_selected_frame (nullptr);
+
switch (bpstat_ret)
{
case PRINT_UNKNOWN:
/* FIXME: cagney/2002-12-01: Given that a frame ID does (or
should) carry around the function and does (or should) use
that when doing a frame comparison. */
- if (frame_info_ptr frame = get_current_frame ();
- tp->control.stop_step
- && (tp->control.step_frame_id == get_frame_id (frame))
- && tp->control.in_step_start_function (frame))
+ if (tp->control.stop_step
+ && (tp->control.step_frame_id == get_frame_id (print_frame))
+ && tp->control.in_step_start_function (print_frame))
{
- symtab_and_line sal = find_frame_sal (frame);
+ symtab_and_line sal = find_frame_sal (print_frame);
if (sal.symtab != tp->current_symtab)
{
/* Finished step in same frame but into different file, print
@@ -9403,7 +9407,7 @@ print_stop_location (const target_waitstatus &ws)
LOCATION: Print only location
SRC_AND_LOC: Print location and source line. */
if (do_frame_printing)
- print_stack_frame (get_selected_frame (nullptr), 0, source_flag, 1);
+ print_stack_frame (print_frame, 0, source_flag, 1);
}

/* See `print_stop_event` in infrun.h. */