* [RFC v2 1/3] [gdb/symtab] Add find_symbol_for_pc_maybe_inline
2026-03-31 13:23 [RFC v2 0/3] [gdb] Fix missing print frame when stepping out of function Tom de Vries
@ 2026-03-31 13:23 ` Tom de Vries
2026-03-31 13:23 ` [RFC v2 2/3] [gdb] Add thread_control_state::step_start_function methods Tom de Vries
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Tom de Vries @ 2026-03-31 13:23 UTC (permalink / raw)
To: gdb-patches
We have function find_symbol_for_pc:
...
find_symbol_for_pc (CORE_ADDR pc)
{
return find_symbol_for_pc_sect (pc, find_pc_mapped_section (pc));
}
...
which uses some standard way of getting the section for the given pc.
Add a similar function find_symbol_for_pc_maybe_inline that calls
find_symbol_for_pc_sect_maybe_inline, and use it in jump_command.
Tested on x86_64-linux.
---
gdb/blockframe.c | 8 ++++++++
gdb/infcmd.c | 3 +--
gdb/symtab.h | 6 ++++++
3 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index 59efa391604..ce28d99c7e4 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -152,6 +152,14 @@ find_symbol_for_pc (CORE_ADDR pc)
/* See symtab.h. */
+struct symbol *
+find_symbol_for_pc_maybe_inline (CORE_ADDR pc)
+{
+ return find_symbol_for_pc_sect_maybe_inline (pc, find_pc_mapped_section (pc));
+}
+
+/* See symtab.h. */
+
struct symbol *
find_symbol_for_pc_sect_maybe_inline (CORE_ADDR pc, struct obj_section *section)
{
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 9fbee5aaba7..2e1a4d592c1 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1249,8 +1249,7 @@ jump_command (const char *arg, int from_tty)
/* See if we are trying to jump to another function. */
fn = get_frame_function (get_current_frame ());
- sfn = find_symbol_for_pc_sect_maybe_inline (sal.pc,
- find_pc_mapped_section (sal.pc));
+ sfn = find_symbol_for_pc_maybe_inline (sal.pc);
if (fn != nullptr && sfn != fn)
{
if (!query (_("Line %ps is not in `%ps'. Jump anyway? "),
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 8f0cc728410..132058a0065 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2201,6 +2201,12 @@ extern struct type *lookup_enum (const char *, const struct block *);
extern struct symbol *find_symbol_for_pc (CORE_ADDR);
+/* Lookup the function symbol corresponding to the address. The return value
+ will be the closest enclosing function, which might be an inline
+ function. */
+
+extern struct symbol *find_symbol_for_pc_maybe_inline (CORE_ADDR pc);
+
/* lookup the function corresponding to the address and section. The
return value will not be an inlined function; the containing
function will be returned instead. */
--
2.51.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [RFC v2 2/3] [gdb] Add thread_control_state::step_start_function methods
2026-03-31 13:23 [RFC v2 0/3] [gdb] Fix missing print frame when stepping out of function Tom de Vries
2026-03-31 13:23 ` [RFC v2 1/3] [gdb/symtab] Add find_symbol_for_pc_maybe_inline Tom de Vries
@ 2026-03-31 13:23 ` Tom de Vries
2026-03-31 13:23 ` [RFC v2 3/3] [gdb] Fix missing print frame when stepping out of function Tom de Vries
2026-04-05 10:12 ` [PATCH 0/2] " Andrew Burgess
3 siblings, 0 replies; 14+ messages in thread
From: Tom de Vries @ 2026-03-31 13:23 UTC (permalink / raw)
To: gdb-patches
Factor out all uses of thread_control_state::step_start_function into
methods, and make the field private.
Also add a shorthand thread_info::in_step_start_function, to avoid long lines:
...
-ecs->event_thread->control.in_step_start_function (ecs->event_thread->stop_pc ())
+ecs->event_thread->in_step_start_function ()
...
Tested on x86_64-linux.
---
gdb/gdbthread.h | 41 +++++++++++++++++++++++++++++++++++++++--
gdb/infcmd.c | 2 +-
gdb/infrun.c | 12 +++++-------
3 files changed, 45 insertions(+), 10 deletions(-)
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 68d7aebbbf7..c0a828409a4 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -126,8 +126,35 @@ struct thread_control_state
CORE_ADDR step_range_start = 0; /* Inclusive */
CORE_ADDR step_range_end = 0; /* Exclusive */
- /* Function the thread was in as of last it started stepping. */
- struct symbol *step_start_function = nullptr;
+ /* Set step_start_function according to PC. */
+ void set_step_start_function (CORE_ADDR pc)
+ {
+ m_step_start_function = find_symbol_for_pc (pc);
+ }
+
+ /* Reset step_start_function. */
+ void reset_step_start_function ()
+ {
+ m_step_start_function = nullptr;
+ }
+
+ /* Return true if PC is in step_start_function. */
+ bool in_step_start_function (CORE_ADDR pc)
+ {
+ return m_step_start_function == find_symbol_for_pc (pc);
+ }
+
+ /* Return true if step_start_function is set. */
+ bool step_start_function_p ()
+ {
+ return m_step_start_function != nullptr;
+ }
+
+ /* Return step_start_function. */
+ struct symbol *step_start_function ()
+ {
+ return m_step_start_function;
+ }
/* If GDB issues a target step request, and this is nonzero, the
target should single-step this thread once, and then continue
@@ -176,6 +203,10 @@ struct thread_control_state
/* True if the thread is evaluating a BP condition. */
bool in_cond_eval = false;
+
+private:
+ /* Function the thread was in as of last it started stepping. */
+ struct symbol *m_step_start_function = nullptr;
};
/* Inferior thread specific part of `struct infcall_suspend_state'. */
@@ -348,6 +379,12 @@ class thread_info : public intrusive_list_node<thread_info>,
See `struct thread_control_state'. */
thread_control_state control;
+ /* Return true if stopped in step_start_function. */
+ bool in_step_start_function ()
+ {
+ return stop_pc_p () && control.in_step_start_function (stop_pc ());
+ }
+
/* Save M_SUSPEND to SUSPEND. */
void save_suspend_to (thread_suspend_state &suspend) const
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 2e1a4d592c1..91fde405d7a 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -899,7 +899,7 @@ set_step_frame (thread_info *tp)
set_step_info (tp, frame, sal);
CORE_ADDR pc = get_frame_pc (frame);
- tp->control.step_start_function = find_symbol_for_pc (pc);
+ tp->control.set_step_start_function (pc);
}
/* Step until outside of current statement. */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9864b5bbdec..d0378d9f7bc 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3086,7 +3086,7 @@ clear_proceed_status_thread (struct thread_info *tp)
tp->control.step_frame_id = null_frame_id;
tp->control.step_stack_frame_id = null_frame_id;
tp->control.step_over_calls = STEP_OVER_UNDEBUGGABLE;
- tp->control.step_start_function = nullptr;
+ tp->control.reset_step_start_function ();
tp->stop_requested = false;
tp->control.stop_step = 0;
@@ -7738,9 +7738,9 @@ process_event_stop_test (struct execution_control_state *ecs)
if (execution_direction != EXEC_REVERSE
&& ecs->event_thread->control.step_over_calls == STEP_OVER_UNDEBUGGABLE
&& in_solib_dynsym_resolve_code (ecs->event_thread->stop_pc ())
- && (ecs->event_thread->control.step_start_function == nullptr
+ && (!ecs->event_thread->control.step_start_function_p ()
|| !in_solib_dynsym_resolve_code (
- ecs->event_thread->control.step_start_function->value_block ()
+ ecs->event_thread->control.step_start_function ()->value_block ()
->entry_pc ())))
{
CORE_ADDR pc_after_resolver =
@@ -7864,8 +7864,7 @@ process_event_stop_test (struct execution_control_state *ecs)
== ecs->event_thread->control.step_stack_frame_id)
&& ((ecs->event_thread->control.step_stack_frame_id
!= outer_frame_id)
- || (ecs->event_thread->control.step_start_function
- != find_symbol_for_pc (ecs->event_thread->stop_pc ())))))
+ || !ecs->event_thread->in_step_start_function ())))
{
CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
CORE_ADDR real_stop_pc;
@@ -9352,8 +9351,7 @@ print_stop_location (const target_waitstatus &ws)
if (tp->control.stop_step
&& (tp->control.step_frame_id
== get_frame_id (get_current_frame ()))
- && (tp->control.step_start_function
- == find_symbol_for_pc (tp->stop_pc ())))
+ && (tp->in_step_start_function ()))
{
symtab_and_line sal = find_frame_sal (get_selected_frame (nullptr));
if (sal.symtab != tp->current_symtab)
--
2.51.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [RFC v2 3/3] [gdb] Fix missing print frame when stepping out of function
2026-03-31 13:23 [RFC v2 0/3] [gdb] Fix missing print frame when stepping out of function Tom de Vries
2026-03-31 13:23 ` [RFC v2 1/3] [gdb/symtab] Add find_symbol_for_pc_maybe_inline Tom de Vries
2026-03-31 13:23 ` [RFC v2 2/3] [gdb] Add thread_control_state::step_start_function methods Tom de Vries
@ 2026-03-31 13:23 ` Tom de Vries
2026-04-05 10:12 ` [PATCH 0/2] " Andrew Burgess
3 siblings, 0 replies; 14+ messages in thread
From: Tom de Vries @ 2026-03-31 13:23 UTC (permalink / raw)
To: gdb-patches
[ This is an RFC. The patch fixes one problem, but introduces another
problem. The point of the RFC is the analysis of the second problem. ]
Consider test-case gdb.dwarf2/dw2-extend-inline-block.exp, specifically the
"contiguous block" prefix part, which can be stepped through like this:
...
$ gdb -q outputs/gdb.dwarf2/dw2-extend-inline-block/dw2-extend-inline-block-5
Reading symbols from dw2-extend-inline-block-5...
(gdb) start
...
Temporary breakpoint 1, main () at dw2-extend-inline-block.c:35
35 /* main:2 */
(gdb) s
36 /* main:3 */ foo (); /* foo call line */
(gdb)
foo () at dw2-extend-inline-block.c:26
26 /* foo:1 */
(gdb) s
27 /* foo:2 */
(gdb) s
28 /* foo:3 */
(gdb) s
main () at dw2-extend-inline-block.c:37
37 /* main:4 */
(gdb)
...
If we slightly modify the line program:
...
DW_LNE_set_address main_4
+ DW_LNS_advance_line 1
DW_LNS_copy
DW_LNE_set_address main_5
- DW_LNS_advance_line 1
DW_LNS_negate_stmt
DW_LNS_copy
...
we change the 36/0x401165 entry into a 37/0x401165 entry:
...
File name Line number Starting address View Stmt
dw2-extend-inline-block.c 34 0x401116 x
dw2-extend-inline-block.c 35 0x401129 x
dw2-extend-inline-block.c 26 0x401138 x
dw2-extend-inline-block.c 27 0x401147 x
dw2-extend-inline-block.c 28 0x401156 x
dw2-extend-inline-block.c 36 0x401156
-dw2-extend-inline-block.c 36 0x401165
+dw2-extend-inline-block.c 37 0x401165
dw2-extend-inline-block.c 37 0x401174 x
dw2-extend-inline-block.c 38 0x401183 x
dw2-extend-inline-block.c 39 0x401192 x
dw2-extend-inline-block.c 40 0x4011a1 x
dw2-extend-inline-block.c - 0x4011b7
...
As it happens, the fix to extend truncated inlined function blocks doesn't
work in this case. This is PR33930.
We can work around this by making sure that the inlined function block isn't
truncated in the first place:
...
- DW_AT_high_pc main_3 addr
+ DW_AT_high_pc main_4 addr
...
But then we still run into PR33981: the problem that gdb doesn't notify us
when stepping out of foo:
...
(gdb) step^M
28 /* foo:3 */^M
(gdb) step^M
37 /* main:4 */^M
(gdb)
...
What happens is that the slightly different line program triggers a different
stepping path, which includes a case of "stepped to a different frame, but
it's not the start of a statement", which refreshes the stepping info and
consequently updates tp->control.step_frame_id to the frame id of main.
So by the time we're stopped at line 37, and are trying to figure out what to
print in print_stop_location, this condition evaluates to true:
...
if (tp->control.stop_step
&& (tp->control.step_frame_id
== get_frame_id (get_current_frame ()))
&& (tp->control.step_start_function
== find_symbol_for_pc (tp->stop_pc ())))
...
and we get:
...
/* Finished step in same frame and same file, just print source
line. */
source_flag = SRC_LINE;
...
It's good to realize here that because foo is inlined into main,
tp->control.step_start_function is not foo but main, so consequently the
step_start_function check (which checks if we are still in the same function)
also passes, even though we actually stepped from foo into main.
Fix this by using find_symbol_for_pc_maybe_inline instead of find_symbol_for_pc
in set_step_start_function and in_step_start_function.
Doing so however regresses gdb.opt/inline-cmds.exp.
To understand that regression, let's first investigate the behavior without
this patch.
We step from function main (lines 64/66) into function func1 (35):
...
$ gdb -q outputs/gdb.opt/inline-cmds/inline-cmds -ex "tbreak 64" -ex run
...
Temporary breakpoint 1, main () at inline-cmds.c:64
64 y = 8; /* set mi break here */
(gdb) step
66 result = func1 ();
(gdb) step
func1 () at inline-cmds.c:35
35 bar ();
(gdb)
...
This seems trivial, but it's not.
If we list the effects of the two steps in terms of line number and pc, we get:
- 64/0x401124
- step
- 66/0x40112e
- step
- 35/0x40112e
We're initially stopped at pc 0x401124, and after the first step stopped at pc
0x40112e.
Effectively we've entered the inlined function func1 at this point, both
according to the line number info (where 0x40112e maps to line 35):
...
Line number Starting address View Stmt
60 0x401116 x
63 0x40111a x
64 0x401124 x
35 0x40112e x
36 0x401133 x
...
and to the debug info:
...
<2><395>: Abbrev Number: 1 (DW_TAG_inlined_subroutine)
<396> DW_AT_abstract_origin: <0x4b7>
<39a> DW_AT_low_pc : 0x40112e
<3a2> DW_AT_high_pc : 0x14
<3aa> DW_AT_call_file : 1
<3aa> DW_AT_call_line : 66
<3ab> DW_AT_call_column : 12
<1><4b7>: Abbrev Number: 5 (DW_TAG_subprogram)
<4b8> DW_AT_external : 1
<4b8> DW_AT_name : func1
<4bc> DW_AT_decl_file : 1
<4bc> DW_AT_decl_line : 33
<4bd> DW_AT_decl_column : 17
<4bd> DW_AT_prototyped : 1
<4bd> DW_AT_type : <0x30b>
<4c1> DW_AT_inline : 3 (declared as inline and inlined)
...
However, we're going to pretend that we're still in main at the call site at
line 66, and require another step to land at line 35.
This works as follows.
When stopping at pc 0x40112e, find_frame_sal is called, where this code:
...
if (frame_inlined_callees (frame) > 0)
{
const symbol *sym;
/* If the current frame has some inlined callees, and we have a next
frame, then that frame must be an inlined frame. In this case
this frame's sal is the "call site" of the next frame's inlined
function, which can not be inferred from get_frame_pc. */
next_frame = get_next_frame (frame);
if (next_frame)
sym = get_frame_function (next_frame);
else
sym = inline_skipped_symbol (inferior_thread ());
...
selects the line number for the call site, 66.
Then we're going to decide what to print in print_stop_location, and decide
that since the step started in main, and stopped in main, there no need to
print anything else than the line, and so we get:
...
(gdb) step
66 result = func1 ();
(gdb)
...
When executing the second step, prepare_one_step is called, which hits this
clause:
...
/* Step at an inlined function behaves like "down". */
if (!sm->skip_subroutines
&& inline_skipped_frames (tp))
{
...
/* Pretend that we've ran. */
...
and consequently the pc stays the same.
Instead, step_into_inline_frames make a change in the "inline frame state" of
the thread, so the next time find_frame_sal is called, the
"frame_inlined_callees (frame) > 0" condition no longer triggers, and line 35
is selected.
Then when deciding what to print in print_stop_location, the comparison of the
frame ids fails, so SRC_AND_LOC is selected and we get:
...
(gdb) step
func1 () at inline-cmds.c:35
35 bar ();
(gdb)
...
Now with this patch, instead we get:
...
$ gdb -q outputs/gdb.opt/inline-cmds/inline-cmds -ex "tbreak 64" -ex run
...
Temporary breakpoint 1, main () at inline-cmds.c:64
64 y = 8; /* set mi break here */
(gdb) s
main () at inline-cmds.c:66
66 result = func1 ();
(gdb) s
func1 () at inline-cmds.c:35
35 bar ();
(gdb)
...
The problem is that when stepping from line 64 to 66, we seem to be entering
main, while that's not the case, because we were already in main.
So how does that happen?
This time, when deciding what to print in print_stop_location, we observe
that we started the step in main, but are stopped in func1 (without the patch,
we would have observed that we're stopped in main):
...
(gdb) p m_step_start_function.m_name
$12 = 0x33cc351 "main"
(gdb) p find_symbol_for_pc_maybe_inline (pc).m_name
$13 = 0x33cc2e1 "func1"
(gdb)
...
so we get SRC_AND_LOC.
Then when doing the actual printing, the infrastructure does look at the
"inline frame state" and concludes that we're actually in main, and so we get:
...
(gdb) s
main () at inline-cmds.c:66
66 result = func1 ();
(gdb)
...
In short, AFAIU the problem is that our proposed fix doesn't take the
"inline frame state" into account.
I hope that doing so will be easy, but I thought it would be a good idea to
record the current state using an RFC.
Tested on x86_64-linux.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33981
---
gdb/gdbthread.h | 4 +-
.../gdb.dwarf2/dw2-extend-inline-block.exp | 45 +++++++++++++++++--
2 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index c0a828409a4..6b2d0ba0801 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -129,7 +129,7 @@ struct thread_control_state
/* Set step_start_function according to PC. */
void set_step_start_function (CORE_ADDR pc)
{
- m_step_start_function = find_symbol_for_pc (pc);
+ m_step_start_function = find_symbol_for_pc_maybe_inline (pc);
}
/* Reset step_start_function. */
@@ -141,7 +141,7 @@ struct thread_control_state
/* Return true if PC is in step_start_function. */
bool in_step_start_function (CORE_ADDR pc)
{
- return m_step_start_function == find_symbol_for_pc (pc);
+ return m_step_start_function == find_symbol_for_pc_maybe_inline (pc);
}
/* Return true if step_start_function is set. */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-extend-inline-block.exp b/gdb/testsuite/gdb.dwarf2/dw2-extend-inline-block.exp
index 9e4798b53f3..3f92c1965d8 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-extend-inline-block.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-extend-inline-block.exp
@@ -52,8 +52,11 @@ get_func_info main
# Create DWARF for the test. In this case, inline function 'foo' is created
# with a contiguous address range that needs extending.
-proc build_dwarf_for_contiguous_block { asm_file } {
+proc build_dwarf_for_contiguous_block { asm_file {range_correct 0} {variant 0}} {
Dwarf::assemble $asm_file {
+ upvar range_correct range_correct
+ upvar variant variant
+
declare_labels lines_table inline_func
cu { } {
@@ -83,7 +86,11 @@ proc build_dwarf_for_contiguous_block { asm_file } {
DW_AT_call_file 1 data1
DW_AT_call_line $::foo_call_line data1
DW_AT_low_pc main_1 addr
- DW_AT_high_pc main_3 addr
+ if {$range_correct} {
+ DW_AT_high_pc main_4 addr
+ } else {
+ DW_AT_high_pc main_3 addr
+ }
}
}
}
@@ -120,10 +127,15 @@ proc build_dwarf_for_contiguous_block { asm_file } {
DW_LNS_copy
DW_LNE_set_address main_4
+ if {$variant == 1} {
+ DW_LNS_advance_line 1
+ }
DW_LNS_copy
DW_LNE_set_address main_5
- DW_LNS_advance_line 1
+ if {$variant == 0} {
+ DW_LNS_advance_line 1
+ }
DW_LNS_negate_stmt
DW_LNS_copy
@@ -146,6 +158,22 @@ proc build_dwarf_for_contiguous_block { asm_file } {
}
}
+# Like build_dwarf_for_contiguous_block, but use a slightly different line
+# info by setting variant == 1.
+# Use range_correct 1, so we're not testing the fix for PR33930.
+
+proc build_dwarf_for_contiguous_block_2 { asm_file } {
+ return [build_dwarf_for_contiguous_block $asm_file 1 1]
+}
+
+# Like build_dwarf_for_contiguous_block, but use a slightly different line
+# info by setting variant == 1.
+# Use range_correct 0, so we're testing the fix for PR33930.
+
+proc build_dwarf_for_contiguous_block_3 { asm_file } {
+ return [build_dwarf_for_contiguous_block $asm_file 0 1]
+}
+
# Assuming GDB is stopped at the entry $pc for 'foo', use 'maint info
# blocks' to check the block for 'foo' is correct. This function checks
# 'foo' created by 'build_dwarf_for_contiguous_block'.
@@ -555,6 +583,12 @@ set test_list \
[list "contiguous block" \
build_dwarf_for_contiguous_block \
check_contiguous_block] \
+ [list "contiguous block 2" \
+ build_dwarf_for_contiguous_block_2 \
+ check_contiguous_block] \
+ [list "contiguous block 3" \
+ build_dwarf_for_contiguous_block_3 \
+ check_contiguous_block] \
]
# Run all the tests.
@@ -566,6 +600,11 @@ foreach test_spec $test_list {
set build_dwarf_func [lindex $test_spec 1]
set check_block_func [lindex $test_spec 2]
+ if {$build_dwarf_func == "build_dwarf_for_contiguous_block_3"} {
+ # Work around PR33930.
+ continue
+ }
+
with_test_prefix $prefix {
set asm_file [standard_output_file ${testfile}-${suffix}.S]
$build_dwarf_func $asm_file
--
2.51.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 0/2] Fix missing print frame when stepping out of function
2026-03-31 13:23 [RFC v2 0/3] [gdb] Fix missing print frame when stepping out of function Tom de Vries
` (2 preceding siblings ...)
2026-03-31 13:23 ` [RFC v2 3/3] [gdb] Fix missing print frame when stepping out of function Tom de Vries
@ 2026-04-05 10:12 ` Andrew Burgess
2026-04-05 10:12 ` [PATCH 1/2] gdb: use get_current_frame consistently in print_stop_location Andrew Burgess
` (2 more replies)
3 siblings, 3 replies; 14+ messages in thread
From: Andrew Burgess @ 2026-04-05 10:12 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess, Tom de Vries
Hi Tom,
I looked into the regression you saw with gdb.opt/inline-cmds.exp, and
read your notes. As you predicted, the issue is that we need to
better take into account the inline frame state when setting up, and
checking during, the step/next process.
Luckily, we already have a mechanism in GDB to do this,
get_frame_function. This returns the function symbol for a frame,
taking into account which frames are inlined but being skipped.
Assuming a call stack like: 'non-inline function -> inline function'
the original code was always returning the non-inline function. My
initial proposal was always returning the inline function, and the
reality is neither of these is always correct.
The get_frame_function returns the function that represents the given
frame, which is exactly what we want.
In fact, I did consider the idea that we should move away from
tracking via the function symbol, and instead just hold a separate
frame-id for the "original frame when stepping started". I think this
would probably work just fine, but currently, if the frame-id changes,
but the function symbol stays the same then GDB would work a
particular way, switching to using a frame-id would change this. I
doubt this is actually a real concern, but I didn't want to change too
much in one go, so I'm sticking with function symbol tracking.
The gdb.opt/inline-cmds.exp regression you ran into was only visible
from part of the test that runs in MI mode, but the bug itself would
manifest in CLI and MI mode, we just didn't spot the bug in CLI mode.
So I extended the test to reveal the bug in CLI mode too. I expect
that the bug likely was present in other tests too, but the test
patterns are too lax, and so didn't trigger for the regression, which
is a bit of a shame, but I don't have the time right now to track down
tests that _should_ have failed and fix them, at least we have one
test that we know checks this stuff now.
Your new additions to gdb.dwarf2/dw2-extend-inline-block.exp all pass,
and I've done a full local test run and don't see any other
regressions.
Your original RFC patch #1 was no longer needed for this series, but
you might want to repost that separately as it did have one use
outside this series, so maybe it's worth having anyway?
I folded RFC patch #2 into what is the second patch in this series. I
ended up changing the API anyway, so it seemed to make sense to do all
the changes in a single patch.
Take a look through, and let me know what you think.
Thanks,
Andrew
---
Andrew Burgess (1):
gdb: use get_current_frame consistently in print_stop_location
Tom de Vries (1):
gdb: fix missing print frame when stepping out of function
gdb/gdbthread.h | 36 ++++++++++++++-
gdb/infcmd.c | 3 +-
gdb/infrun.c | 20 ++++-----
.../gdb.dwarf2/dw2-extend-inline-block.exp | 45 +++++++++++++++++--
gdb/testsuite/gdb.opt/inline-cmds.exp | 33 +++++++++-----
5 files changed, 108 insertions(+), 29 deletions(-)
base-commit: 9d3cf9efd51ebae3f45bb49e3544cb7eeb63a138
--
2.25.4
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/2] gdb: use get_current_frame consistently in print_stop_location
2026-04-05 10:12 ` [PATCH 0/2] " Andrew Burgess
@ 2026-04-05 10:12 ` Andrew Burgess
2026-04-09 6:42 ` Tom de Vries
2026-04-05 10:12 ` [PATCH 2/2] gdb: fix missing print frame when stepping out of function Andrew Burgess
2026-04-10 10:48 ` [PATCH 0/2] Fix " Tom de Vries
2 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2026-04-05 10:12 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess, Tom de Vries
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.
---
gdb/infrun.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6ca2a505299..aa1c3553131 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -9362,13 +9362,13 @@ print_stop_location (const target_waitstatus &ws)
/* 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 (tp->control.stop_step
- && (tp->control.step_frame_id
- == get_frame_id (get_current_frame ()))
+ if (frame_info_ptr frame = get_current_frame ();
+ tp->control.stop_step
+ && (tp->control.step_frame_id == get_frame_id (frame))
&& (tp->control.step_start_function
== find_symbol_for_pc (tp->stop_pc ())))
{
- symtab_and_line sal = find_frame_sal (get_selected_frame (nullptr));
+ symtab_and_line sal = find_frame_sal (frame);
if (sal.symtab != tp->current_symtab)
{
/* Finished step in same frame but into different file, print
--
2.25.4
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/2] gdb: use get_current_frame consistently in print_stop_location
2026-04-05 10:12 ` [PATCH 1/2] gdb: use get_current_frame consistently in print_stop_location Andrew Burgess
@ 2026-04-09 6:42 ` Tom de Vries
2026-04-09 8:54 ` Andrew Burgess
0 siblings, 1 reply; 14+ messages in thread
From: Tom de Vries @ 2026-04-09 6:42 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
[-- 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. */
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/2] gdb: use get_current_frame consistently in print_stop_location
2026-04-09 6:42 ` Tom de Vries
@ 2026-04-09 8:54 ` Andrew Burgess
2026-04-09 13:43 ` Tom de Vries
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2026-04-09 8:54 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
Tom de Vries <tdevries@suse.de> writes:
> 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 for the great analysis, and explanation, I wasn't aware of this
aspect of bpstat_print. Given this new information, I think your patch
is the right solution.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Thanks,
> - Tom
> 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. */
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/2] gdb: use get_current_frame consistently in print_stop_location
2026-04-09 8:54 ` Andrew Burgess
@ 2026-04-09 13:43 ` Tom de Vries
2026-04-10 8:57 ` Andrew Burgess
0 siblings, 1 reply; 14+ messages in thread
From: Tom de Vries @ 2026-04-09 13:43 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 4/9/26 10:54 AM, Andrew Burgess wrote:
>> 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 for the great analysis, and explanation, I wasn't aware of this
> aspect of bpstat_print. Given this new information, I think your patch
> is the right solution.
>
> Approved-By: Andrew Burgess<aburgess@redhat.com>
Hi Andrew,
my initial idea here was that the patch I posted could be trivially
merged with the first patch, but upon attempting this I ended up
re-editing the series. I'm currently testing this, and will submit if
that goes ok.
The content of the patch series should be what you posted plus
aforementioned patch, the changes I made are merely refactoring.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] gdb: use get_current_frame consistently in print_stop_location
2026-04-09 13:43 ` Tom de Vries
@ 2026-04-10 8:57 ` Andrew Burgess
2026-04-10 10:17 ` Tom de Vries
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2026-04-10 8:57 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
Tom de Vries <tdevries@suse.de> writes:
> On 4/9/26 10:54 AM, Andrew Burgess wrote:
>>> 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 for the great analysis, and explanation, I wasn't aware of this
>> aspect of bpstat_print. Given this new information, I think your patch
>> is the right solution.
>>
>> Approved-By: Andrew Burgess<aburgess@redhat.com>
>
> Hi Andrew,
>
> my initial idea here was that the patch I posted could be trivially
> merged with the first patch, but upon attempting this I ended up
> re-editing the series. I'm currently testing this, and will submit if
> that goes ok.
>
> The content of the patch series should be what you posted plus
> aforementioned patch, the changes I made are merely refactoring.
The patch you posted applies on top of my patch #1, but I think it would
be best if you just merged those two patches together. As you point
out, my patch moves in the wrong direction. You're patch basically
reverts mine and does something different, which is the right thing to
do.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] gdb: use get_current_frame consistently in print_stop_location
2026-04-10 8:57 ` Andrew Burgess
@ 2026-04-10 10:17 ` Tom de Vries
0 siblings, 0 replies; 14+ messages in thread
From: Tom de Vries @ 2026-04-10 10:17 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 4/10/26 10:57 AM, Andrew Burgess wrote:
> Tom de Vries <tdevries@suse.de> writes:
>
>> On 4/9/26 10:54 AM, Andrew Burgess wrote:
>>>> 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 for the great analysis, and explanation, I wasn't aware of this
>>> aspect of bpstat_print. Given this new information, I think your patch
>>> is the right solution.
>>>
>>> Approved-By: Andrew Burgess<aburgess@redhat.com>
>>
>> Hi Andrew,
>>
>> my initial idea here was that the patch I posted could be trivially
>> merged with the first patch, but upon attempting this I ended up
>> re-editing the series. I'm currently testing this, and will submit if
>> that goes ok.
>>
>> The content of the patch series should be what you posted plus
>> aforementioned patch, the changes I made are merely refactoring.
>
> The patch you posted applies on top of my patch #1, but I think it would
> be best if you just merged those two patches together. As you point
> out, my patch moves in the wrong direction. You're patch basically
> reverts mine and does something different, which is the right thing to
> do.
The problem with doing so is that it makes the first patch nonsensical,
in the sense that it results in using the selected frame in one part of
the condition, and the stop pc (corresponding to the current frame or
stop frame) in another part of the condition. So I ended up moving the
patch to the end of the series.
Submitted here (
https://sourceware.org/pipermail/gdb-patches/2026-April/226422.html ).
Thanks,
- Tom
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] gdb: fix missing print frame when stepping out of function
2026-04-05 10:12 ` [PATCH 0/2] " Andrew Burgess
2026-04-05 10:12 ` [PATCH 1/2] gdb: use get_current_frame consistently in print_stop_location Andrew Burgess
@ 2026-04-05 10:12 ` Andrew Burgess
2026-04-10 10:29 ` Tom de Vries
2026-04-10 10:48 ` [PATCH 0/2] Fix " Tom de Vries
2 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2026-04-05 10:12 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom de Vries, Andrew Burgess
From: Tom de Vries <tdevries@suse.de>
Consider test-case gdb.dwarf2/dw2-extend-inline-block.exp,
specifically the "contiguous block" prefix part, which can be stepped
through like this:
...
$ gdb -q outputs/gdb.dwarf2/dw2-extend-inline-block/dw2-extend-inline-block-5
Reading symbols from dw2-extend-inline-block-5...
(gdb) start
... snip ...
Temporary breakpoint 1, main () at dw2-extend-inline-block.c:35
35 /* main:2 */
(gdb) s
36 /* main:3 */ foo (); /* foo call line */
(gdb)
foo () at dw2-extend-inline-block.c:26
26 /* foo:1 */
(gdb) s
27 /* foo:2 */
(gdb) s
28 /* foo:3 */
(gdb) s
main () at dw2-extend-inline-block.c:37
37 /* main:4 */
(gdb)
...
If we slightly modify the line program:
...
DW_LNE_set_address main_4
+ DW_LNS_advance_line 1
DW_LNS_copy
DW_LNE_set_address main_5
- DW_LNS_advance_line 1
DW_LNS_negate_stmt
DW_LNS_copy
...
we change the 36/0x401165 entry into a 37/0x401165 entry:
...
File name Line number Starting address View Stmt
dw2-extend-inline-block.c 34 0x401116 x
dw2-extend-inline-block.c 35 0x401129 x
dw2-extend-inline-block.c 26 0x401138 x
dw2-extend-inline-block.c 27 0x401147 x
dw2-extend-inline-block.c 28 0x401156 x
dw2-extend-inline-block.c 36 0x401156
-dw2-extend-inline-block.c 36 0x401165
+dw2-extend-inline-block.c 37 0x401165
dw2-extend-inline-block.c 37 0x401174 x
dw2-extend-inline-block.c 38 0x401183 x
dw2-extend-inline-block.c 39 0x401192 x
dw2-extend-inline-block.c 40 0x4011a1 x
dw2-extend-inline-block.c - 0x4011b7
...
As it happens, the fix to extend truncated inlined function blocks
doesn't work in this case. This is PR gdb/33930.
We can work around this by making sure that the inlined function block
isn't truncated in the first place:
...
- DW_AT_high_pc main_3 addr
+ DW_AT_high_pc main_4 addr
...
But then we still run into PR gdb/33981: the problem that gdb doesn't
notify us when stepping out of foo:
...
(gdb) step^M
28 /* foo:3 */^M
(gdb) step^M
37 /* main:4 */^M
(gdb)
...
What happens is that the slightly different line program triggers a
different stepping path, which includes a case of "stepped to a
different frame, but it's not the start of a statement", which
refreshes the stepping info and consequently updates
tp->control.step_frame_id to the frame id of main.
So by the time we're stopped at line 37, and are trying to figure out
what to print in print_stop_location, this condition evaluates to
true:
...
if (tp->control.stop_step
&& (tp->control.step_frame_id
== get_frame_id (get_current_frame ()))
&& (tp->control.step_start_function
== find_symbol_for_pc (tp->stop_pc ())))
...
and we get:
...
/* Finished step in same frame and same file, just print source
line. */
source_flag = SRC_LINE;
...
It's good to realize here that because foo is inlined into main,
tp->control.step_start_function is not foo but main, so consequently
the step_start_function check (which checks if we are still in the
same function) also passes, even though we actually stepped from foo
into main.
The problem is the use of find_symbol_for_pc, this function
deliberately skips over inline frames and returns the symbol for the
innermost non-inline frame.
It might be tempting to think that we should switch to use
find_symbol_for_pc_sect_maybe_inline, which will return the symbol for
an inline frame, but this also has problems, specifically, attempting
this caused a regression in gdb.opt/inline-cmds.exp. The previous
version of this patch, which showed the regression can be found here:
https://inbox.sourceware.org/gdb-patches/20260331132342.1050954-1-tdevries@suse.de
At a given $pc the inferior might be reported as being within an
inline frame, or it might be reported as being in the containing
non-inline frame. When the user performs a 'step' the
step_start_function needs to be set based on the function symbol of
the frame the inferior is actually reported in. And we have a
function that gives us this information, get_frame_function. So
instead of looking up the step_start_function based on the $pc value,
set it based on the frame.
Management of step_start_function has been moved into member functions
within thread_control_state, and step_start_function is made private.
Test gdb.dwarf2/dw2-extend-inline-block.exp is extended with the
additional test case described above.
Test gdb.opt/inline-cmds.exp is extended to better expose the
regression that was mentioned above when using
find_symbol_for_pc_sect_maybe_inline. Previously the regression was
only exposed during part of the test that ran in MI mode. I've
extended the test so the regression is also revealed in CLI mode.
Tested on x86_64-linux.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33930
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33981
---
gdb/gdbthread.h | 36 ++++++++++++++-
gdb/infcmd.c | 3 +-
gdb/infrun.c | 12 +++--
.../gdb.dwarf2/dw2-extend-inline-block.exp | 45 +++++++++++++++++--
gdb/testsuite/gdb.opt/inline-cmds.exp | 33 +++++++++-----
5 files changed, 104 insertions(+), 25 deletions(-)
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 68d7aebbbf7..d221af89744 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -126,8 +126,36 @@ struct thread_control_state
CORE_ADDR step_range_start = 0; /* Inclusive */
CORE_ADDR step_range_end = 0; /* Exclusive */
- /* Function the thread was in as of last it started stepping. */
- struct symbol *step_start_function = nullptr;
+ /* Set m_step_start_function according to FRAME. */
+ void set_step_start_function (const frame_info_ptr &frame)
+ {
+ m_step_start_function = get_frame_function (frame);
+ }
+
+ /* Reset step_start_function. */
+ void reset_step_start_function ()
+ {
+ m_step_start_function = nullptr;
+ }
+
+ /* Return true if the function symbol of FRAME matches
+ m_step_start_function. */
+ bool in_step_start_function (const frame_info_ptr &frame) const
+ {
+ return m_step_start_function == get_frame_function (frame);
+ }
+
+ /* Return true if step_start_function is set. */
+ bool step_start_function_p () const
+ {
+ return m_step_start_function != nullptr;
+ }
+
+ /* Return step_start_function. */
+ const struct symbol *step_start_function () const
+ {
+ return m_step_start_function;
+ }
/* If GDB issues a target step request, and this is nonzero, the
target should single-step this thread once, and then continue
@@ -176,6 +204,10 @@ struct thread_control_state
/* True if the thread is evaluating a BP condition. */
bool in_cond_eval = false;
+
+private:
+ /* Function the thread was in when it last started stepping. */
+ const struct symbol *m_step_start_function = nullptr;
};
/* Inferior thread specific part of `struct infcall_suspend_state'. */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 98b45f884b1..17691633def 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -898,8 +898,7 @@ set_step_frame (thread_info *tp)
symtab_and_line sal = find_frame_sal (frame);
set_step_info (tp, frame, sal);
- CORE_ADDR pc = get_frame_pc (frame);
- tp->control.step_start_function = find_symbol_for_pc (pc);
+ tp->control.set_step_start_function (frame);
}
/* Step until outside of current statement. */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index aa1c3553131..0af8c67d554 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3099,7 +3099,7 @@ clear_proceed_status_thread (struct thread_info *tp)
tp->control.step_frame_id = null_frame_id;
tp->control.step_stack_frame_id = null_frame_id;
tp->control.step_over_calls = STEP_OVER_UNDEBUGGABLE;
- tp->control.step_start_function = nullptr;
+ tp->control.reset_step_start_function ();
tp->stop_requested = false;
tp->control.stop_step = 0;
@@ -7751,9 +7751,9 @@ process_event_stop_test (struct execution_control_state *ecs)
if (execution_direction != EXEC_REVERSE
&& ecs->event_thread->control.step_over_calls == STEP_OVER_UNDEBUGGABLE
&& in_solib_dynsym_resolve_code (ecs->event_thread->stop_pc ())
- && (ecs->event_thread->control.step_start_function == nullptr
+ && (!ecs->event_thread->control.step_start_function_p ()
|| !in_solib_dynsym_resolve_code (
- ecs->event_thread->control.step_start_function->value_block ()
+ ecs->event_thread->control.step_start_function ()->value_block ()
->entry_pc ())))
{
CORE_ADDR pc_after_resolver =
@@ -7877,8 +7877,7 @@ process_event_stop_test (struct execution_control_state *ecs)
== ecs->event_thread->control.step_stack_frame_id)
&& ((ecs->event_thread->control.step_stack_frame_id
!= outer_frame_id)
- || (ecs->event_thread->control.step_start_function
- != find_symbol_for_pc (ecs->event_thread->stop_pc ())))))
+ || !ecs->event_thread->control.in_step_start_function (frame))))
{
CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
CORE_ADDR real_stop_pc;
@@ -9365,8 +9364,7 @@ print_stop_location (const target_waitstatus &ws)
if (frame_info_ptr frame = get_current_frame ();
tp->control.stop_step
&& (tp->control.step_frame_id == get_frame_id (frame))
- && (tp->control.step_start_function
- == find_symbol_for_pc (tp->stop_pc ())))
+ && tp->control.in_step_start_function (frame))
{
symtab_and_line sal = find_frame_sal (frame);
if (sal.symtab != tp->current_symtab)
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-extend-inline-block.exp b/gdb/testsuite/gdb.dwarf2/dw2-extend-inline-block.exp
index 9e4798b53f3..481dfbdd134 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-extend-inline-block.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-extend-inline-block.exp
@@ -52,8 +52,11 @@ get_func_info main
# Create DWARF for the test. In this case, inline function 'foo' is created
# with a contiguous address range that needs extending.
-proc build_dwarf_for_contiguous_block { asm_file } {
+proc build_dwarf_for_contiguous_block { asm_file {range_correct 0} {variant 0} } {
Dwarf::assemble $asm_file {
+ upvar range_correct range_correct
+ upvar variant variant
+
declare_labels lines_table inline_func
cu { } {
@@ -83,7 +86,11 @@ proc build_dwarf_for_contiguous_block { asm_file } {
DW_AT_call_file 1 data1
DW_AT_call_line $::foo_call_line data1
DW_AT_low_pc main_1 addr
- DW_AT_high_pc main_3 addr
+ if {$range_correct} {
+ DW_AT_high_pc main_4 addr
+ } else {
+ DW_AT_high_pc main_3 addr
+ }
}
}
}
@@ -120,10 +127,15 @@ proc build_dwarf_for_contiguous_block { asm_file } {
DW_LNS_copy
DW_LNE_set_address main_4
+ if {$variant == 1} {
+ DW_LNS_advance_line 1
+ }
DW_LNS_copy
DW_LNE_set_address main_5
- DW_LNS_advance_line 1
+ if {$variant == 0} {
+ DW_LNS_advance_line 1
+ }
DW_LNS_negate_stmt
DW_LNS_copy
@@ -146,6 +158,22 @@ proc build_dwarf_for_contiguous_block { asm_file } {
}
}
+# Like build_dwarf_for_contiguous_block, but use a slightly different line
+# info by setting variant == 1.
+# Use range_correct 1, so we're not testing the fix for PR33930.
+
+proc build_dwarf_for_contiguous_block_2 { asm_file } {
+ return [build_dwarf_for_contiguous_block $asm_file 1 1]
+}
+
+# Like build_dwarf_for_contiguous_block, but use a slightly different line
+# info by setting variant == 1.
+# Use range_correct 0, so we're testing the fix for PR33930.
+
+proc build_dwarf_for_contiguous_block_3 { asm_file } {
+ return [build_dwarf_for_contiguous_block $asm_file 0 1]
+}
+
# Assuming GDB is stopped at the entry $pc for 'foo', use 'maint info
# blocks' to check the block for 'foo' is correct. This function checks
# 'foo' created by 'build_dwarf_for_contiguous_block'.
@@ -555,6 +583,12 @@ set test_list \
[list "contiguous block" \
build_dwarf_for_contiguous_block \
check_contiguous_block] \
+ [list "contiguous block 2" \
+ build_dwarf_for_contiguous_block_2 \
+ check_contiguous_block] \
+ [list "contiguous block 3" \
+ build_dwarf_for_contiguous_block_3 \
+ check_contiguous_block] \
]
# Run all the tests.
@@ -566,6 +600,11 @@ foreach test_spec $test_list {
set build_dwarf_func [lindex $test_spec 1]
set check_block_func [lindex $test_spec 2]
+ if {$build_dwarf_func == "build_dwarf_for_contiguous_block_3"} {
+ # Work around PR gdb/33930.
+ continue
+ }
+
with_test_prefix $prefix {
set asm_file [standard_output_file ${testfile}-${suffix}.S]
$build_dwarf_func $asm_file
diff --git a/gdb/testsuite/gdb.opt/inline-cmds.exp b/gdb/testsuite/gdb.opt/inline-cmds.exp
index 12db0709fe6..9bfc32f58b3 100644
--- a/gdb/testsuite/gdb.opt/inline-cmds.exp
+++ b/gdb/testsuite/gdb.opt/inline-cmds.exp
@@ -176,24 +176,35 @@ if { $bt_test == 0 } {
# "stop" at the call sites before entering them.
runto_main
-set msg "step into func1"
-set saw_call_site 0
-gdb_test_multiple "list" $msg {
- -re "($first|$opt).*$gdb_prompt $" {
+set saw_call_site false
+set saw_main_frame false
+# Don't send the command using gdb_test_multiple, as it will be
+# injected into the regexp that start with '^' and use '-wrap', and we
+# send a different command using 'send_gdb' within the gdb_test_multiple.
+send_gdb "list\r"
+gdb_test_multiple "" "step into func1" {
+ -re "^(list|step)\r\n" {
+ exp_continue
+ }
+ -re -wrap "^\[^\r\n\]*($first|$opt)\[^\r\n\]*" {
send_gdb "step\r"
exp_continue
}
- -re "result = func1.*$gdb_prompt $" {
- set saw_call_site 1
+ -re -wrap "^\[^\r\n\]*y = 8\[^\r\n\]*" {
+ send_gdb "step\r"
+ exp_continue
+ }
+ -re "^main \\(\\) at \[^\r\n\]+\r\n" {
+ set saw_main_frame true
+ exp_continue
+ }
+ -re -wrap "^\[^\r\n\]*result = func1 \\(\\);" {
+ set saw_call_site true
send_gdb "step\r"
exp_continue
}
-re "func1 \\\(\\\) at .*\r\n$decimal.*bar \\\(\\\);\r\n$gdb_prompt $" {
- if { $saw_call_site } {
- pass $msg
- } else {
- fail $msg
- }
+ gdb_assert { $saw_call_site && !$saw_main_frame } $gdb_test_name
}
}
--
2.25.4
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 0/2] Fix missing print frame when stepping out of function
2026-04-05 10:12 ` [PATCH 0/2] " Andrew Burgess
2026-04-05 10:12 ` [PATCH 1/2] gdb: use get_current_frame consistently in print_stop_location Andrew Burgess
2026-04-05 10:12 ` [PATCH 2/2] gdb: fix missing print frame when stepping out of function Andrew Burgess
@ 2026-04-10 10:48 ` Tom de Vries
2 siblings, 0 replies; 14+ messages in thread
From: Tom de Vries @ 2026-04-10 10:48 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 4/5/26 12:12 PM, Andrew Burgess wrote:
> Hi Tom,
>
> I looked into the regression you saw with gdb.opt/inline-cmds.exp, and
> read your notes.
Hi Andrew,
a massive thanks for helping out with this.
> As you predicted, the issue is that we need to
> better take into account the inline frame state when setting up, and
> checking during, the step/next process.
>
> Luckily, we already have a mechanism in GDB to do this,
> get_frame_function. This returns the function symbol for a frame,
> taking into account which frames are inlined but being skipped.
>
> Assuming a call stack like: 'non-inline function -> inline function'
> the original code was always returning the non-inline function. My
> initial proposal was always returning the inline function, and the
> reality is neither of these is always correct.
>
> The get_frame_function returns the function that represents the given
> frame, which is exactly what we want.
>
> In fact, I did consider the idea that we should move away from
> tracking via the function symbol, and instead just hold a separate
> frame-id for the "original frame when stepping started".
Yes, I actually had already started working on that approach when I saw
this series.
> I think this
> would probably work just fine, but currently, if the frame-id changes,
> but the function symbol stays the same then GDB would work a
> particular way, switching to using a frame-id would change this. I
> doubt this is actually a real concern, but I didn't want to change too
> much in one go, so I'm sticking with function symbol tracking.
I see.
> The gdb.opt/inline-cmds.exp regression you ran into was only visible
> from part of the test that runs in MI mode, but the bug itself would
> manifest in CLI and MI mode, we just didn't spot the bug in CLI mode.
> So I extended the test to reveal the bug in CLI mode too.
That's a great idea, thanks.
> I expect
> that the bug likely was present in other tests too, but the test
> patterns are too lax, and so didn't trigger for the regression, which
> is a bit of a shame, but I don't have the time right now to track down
> tests that _should_ have failed and fix them, at least we have one
> test that we know checks this stuff now.
I didn't think of that, good point.
> Your new additions to gdb.dwarf2/dw2-extend-inline-block.exp all pass,
> and I've done a full local test run and don't see any other
> regressions.
>
> Your original RFC patch #1 was no longer needed for this series, but
> you might want to repost that separately as it did have one use
> outside this series, so maybe it's worth having anyway?
>
OK, I'll do that.
> I folded RFC patch #2 into what is the second patch in this series. I
> ended up changing the API anyway, so it seemed to make sense to do all
> the changes in a single patch.
>
I ended up reverting that in v4, I think the fix is more readable with
infrastructure changes factored out.
> Take a look through, and let me know what you think.
I think v4 is ok to commit.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 14+ messages in thread