[binutils-gdb] gdb: fix missing print frame when stepping out of function

Tom de Vries via Gdb-cvs <[email protected]>
Newsgroups gmane.comp.gdb.cvs
Message-ID <[email protected]>
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3959bfb8a5238692cd910324383c947194a5cdd6

commit 3959bfb8a5238692cd910324383c947194a5cdd6
Author: Tom de Vries <[email protected]>
Date:   Tue Apr 14 22:36:09 2026 +0200

    gdb: fix missing print frame when stepping out of function
    
    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/[email protected]
    
    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.
    
    Test gdb.dwarf2/dw2-extend-inline-block.exp is extended with the
    additional test case described above.
    
    Tested on x86_64-linux.
    
    Co-Authored-By: Andrew Burgess <[email protected]>
    Approved-By: Andrew Burgess <[email protected]>
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33930
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33981

Diff:
---
 gdb/gdbthread.h                                    | 12 +++---
 gdb/infcmd.c                                       |  3 +-
 gdb/infrun.c                                       |  5 +--
 .../gdb.dwarf2/dw2-extend-inline-block.exp         | 45 ++++++++++++++++++++--
 4 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 224e6e4b657..c56c4ce4036 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -154,10 +154,10 @@ struct thread_control_state
   CORE_ADDR step_range_start = 0;	/* Inclusive */
   CORE_ADDR step_range_end = 0;		/* Exclusive */
 
-  /* Set m_step_start_function according to PC.  */
-  void set_step_start_function (CORE_ADDR pc)
+  /* Set m_step_start_function according to FRAME.  */
+  void set_step_start_function (const frame_info_ptr &frame)
   {
-    m_step_start_function = find_symbol_for_pc (pc);
+    m_step_start_function = get_frame_function (frame);
   }
 
   /* Reset m_step_start_function.  */
@@ -166,11 +166,11 @@ struct thread_control_state
     m_step_start_function = nullptr;
   }
 
-  /* Return true if the function symbol for PC matches
+  /* Return true if the function symbol of FRAME matches
      m_step_start_function.  */
-  bool in_step_start_function (CORE_ADDR pc)
+  bool in_step_start_function (const frame_info_ptr &frame)
   {
-    return m_step_start_function == find_symbol_for_pc (pc);
+    return m_step_start_function == get_frame_function (frame);
   }
 
   /* Return true if m_step_start_function is set.  */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index a1df59e09f2..c0d12be6e4a 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.set_step_start_function (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 becd68a21ad..dea2a2cbd48 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7871,8 +7871,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.in_step_start_function
-		    (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;
@@ -9359,7 +9358,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.in_step_start_function (tp->stop_pc ()))
+	  && tp->control.in_step_start_function (get_current_frame ()))
 	{
 	  symtab_and_line sal = find_frame_sal (get_selected_frame (nullptr));
 	  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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.