[PATCH 1/2] [gdb] Handle stepping out of function into loop, case 1

Tom de Vries tdevries@suse.de
Thu Sep 5 15:06:39 GMT 2024


Consider test-case test.c:
...
     1	int count{0};
     2
     3	bool f(int& w)
     4	{
     5	  bool result = ++count != 42;
     6	  return result;
     7	}
     8
     9	int main()
    10	{
    11	  int n;
    12	  while (f(n))
    13	    ;
    14	  return n;
    15	}
...
compiled with gcc 7.5.0 and -g:
...
$ g++ -g test.c
...

Using step to step out of function f steps back into function f:
...
$ gdb -q a.out -ex start
Reading symbols from a.out...
Temporary breakpoint 1 at 0x4004fb: file test.c, line 12.
Starting program: a.out

Temporary breakpoint 1, main () at test.c:12
12	  while (f(n))
(gdb) step
f (w=@0x7fffffffdc7c: 0) at test.c:5
5	  bool result = ++count != 42;
(gdb) step
6	  return result;
(gdb) step
7	}
(gdb) step
f (w=@0x7fffffffdc7c: 0) at test.c:5
5	  bool result = ++count != 42;
(gdb)
...

So, why didn't we step out of function f to the line with the while loop,
line 12?

To understand what happens, the line table for main is:
...
CU: test.c:
File name                        Line number    Starting address    View    Stmt
test.c                                    10            0x4004f3               x
test.c                                    12            0x4004fb               x
test.c                                    14            0x40050d               x
test.c                                    15            0x400510               x
test.c                                     -            0x400512
...

That info allows us to annotate main like so:
...
00000000004004f3 <main>:
line 10:
  4004f3:       55                      push   %rbp
  4004f4:       48 89 e5                mov    %rsp,%rbp
  4004f7:       48 83 ec 10             sub    $0x10,%rsp
line 12:
  4004fb:       48 8d 45 fc             lea    -0x4(%rbp),%rax
  4004ff:       48 89 c7                mov    %rax,%rdi
  400502:       e8 c0 ff ff ff          call   4004c7 <_Z1fRi>
  400507:       84 c0                   test   %al,%al
  400509:       74 02                   je     40050d <main+0x1a>
  40050b:       eb ee                   jmp    4004fb <main+0x8>
14:
  40050d:       8b 45 fc                mov    -0x4(%rbp),%eax
15:
  400510:       c9                      leave
  400511:       c3                      ret
...

When stepping out of function f, ptrace single-stepping stops at pcs 0x400507,
0x400509, 0x40050b and 0x4004fb where it re-enters the loop.

The address 0x4004fb is at the start of a line entry with is_stmt flag set, so
we could expect a stop there for that reason.

But gdb doesn't stop there, because it's stepping in range:
...
  [infrun] handle_signal_stop: stop_pc=0x4004fb
  [infrun] process_event_stop_test: stepping inside range [0x4004fb-0x40050d]
...

The stepping range was set earlier at 0x400507:
...
  [infrun] handle_signal_stop: stop_pc=0x400507
  [infrun] process_event_stop_test: updated step range, \
    start = 0x4004fb, end = 0x40050d, may_range_step = 1
  [infrun] set_step_info: symtab = test.c, line = 12, \
    step_frame_id={stack=0x7fffffffdcb0,code=0x00000000004004f3,!special}, \
    step_stack_frame_id={stack=0x7fffffffdcb0,code=0x00000000004004f3,!special}
...
and in process_event_stop_test there's a comment that describes what happens:
...
  /* We aren't done stepping.

     Optimize by setting the stepping range to the line.
     (We might not be in the original line, but if we entered a
     new line in mid-statement, we continue stepping.  This makes
     things like for(;;) statements work better.)
...

The comment does not specify explicitly what better means, so I don't manage to
deduce whether the observed behaviour is an intended consequence of this code.

Regardless, change gdb to stop at line 12, by detecting the situation:
- stepped to a different line
- did not step to the first instruction of a line entry
and handling the situation by this basic fix:
- calling keep_going, and
- returning and thereby avoiding refreshing step info and updating stepping
  range.

That works for test-case gdb.dwarf2/dw2-insn-loop.exp, which reflects the debug
info as generated with gcc 7.5.

However, with gcc 13.3, we get a nop at the start of the loop:
...
  4004fa:       90                      nop
  4004fb:       48 8d 45 fc             lea    -0x4(%rbp),%rax
  4004ff:       48 89 c7                mov    %rax,%rdi
  400502:       e8 bf ff ff ff          call   4004c6 <_Z1fRi>
  400507:       84 c0                   test   %al,%al
  400509:       75 f0                   jne    4004fb <main+0x9>
  40050b:       8b 45 fc                mov    -0x4(%rbp),%eax
...
and this line info:
...
File name                        Line number    Starting address    View    Stmt
test.c                                    12            0x4004fa               x
test.c                                    12            0x4004fb               x
test.c                                    14            0x40050b               x
...
which gdb boils down to:
...
INDEX  LINE   REL-ADDRESS        UNREL-ADDRESS      IS-STMT
8      12     0x00000000004004fa 0x00000000004004fa Y
9      14     0x000000000040050b 0x000000000040050b Y
...

Also in this case using step to step out of function f steps back into
function f:
...
(gdb) step
7	}
(gdb) step
f (w=@0x7fffffffdc7c: 0) at test.c:5
5	  bool result = ++count != 42;
(gdb)
...
but since the loop doesn't revisit the nop, we cannot expect stopping at line
12.

However, with the basic fix the behaviour regresses to:
...
(gdb) step
7	}
(gdb)
4	{
(gdb) s
5	  bool result = ++count != 42;
(gdb)
...

Fix this by extending the basic fix to update the stepping frame.

Tested on x86_64-linux.

PR gdb/32000
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32000
---
 gdb/infrun.c                                  | 24 ++++-
 gdb/testsuite/gdb.dwarf2/dw2-insn-loop-nop.c  | 19 ++++
 .../gdb.dwarf2/dw2-insn-loop-nop.exp          | 67 ++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-insn-loop.c      | 43 +++++++++
 gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp    | 89 +++++++++++++++++++
 .../gdb.dwarf2/dw2-insn-loop.exp.tcl          | 77 ++++++++++++++++
 6 files changed, 315 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-insn-loop-nop.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-insn-loop-nop.exp
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-insn-loop.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp.tcl

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4ca15450afe..ec9becae2f1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8179,12 +8179,14 @@ process_event_stop_test (struct execution_control_state *ecs)
       return;
     }
 
+  bool different_line
+    = (ecs->event_thread->current_line != stop_pc_sal.line
+       || ecs->event_thread->current_symtab != stop_pc_sal.symtab);
+
   bool refresh_step_info = true;
-  if ((ecs->event_thread->stop_pc () == stop_pc_sal.pc)
-      && (ecs->event_thread->current_line != stop_pc_sal.line
-	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))
+  if (different_line && ecs->event_thread->stop_pc () == stop_pc_sal.pc)
     {
-      /* We are at a different line.  */
+      /* We are at a different line, at the start of a line entry.  */
 
       if (stop_pc_sal.is_stmt)
 	{
@@ -8245,6 +8247,20 @@ process_event_stop_test (struct execution_control_state *ecs)
 			       "it's not the start of a statement");
 	}
     }
+  else if (different_line)
+    {
+      /* We are at a different line, but not at the start of a line entry.  */
+
+      if (execution_direction == EXEC_FORWARD)
+	{
+	  /* Keep going until we are at the start of a line entry, but update
+	     the stepping frame to notice stepping into a function.  */
+	  ecs->event_thread->control.step_stack_frame_id
+	    = get_stack_frame_id (frame);
+	  keep_going (ecs);
+	  return;
+	}
+    }
 
   if (execution_direction == EXEC_REVERSE
 	  && *curr_frame_id != original_frame_id
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-insn-loop-nop.c b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop-nop.c
new file mode 100644
index 00000000000..a1aed6739e9
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop-nop.c
@@ -0,0 +1,19 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2024 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/>.  */
+
+#define NOP 1
+#include "dw2-insn-loop.c"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-insn-loop-nop.exp b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop-nop.exp
new file mode 100644
index 00000000000..4c9f63647d5
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop-nop.exp
@@ -0,0 +1,67 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2024 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/>.
+
+load_lib dwarf.exp
+
+require dwarf2_support
+
+standard_testfile .c .S dw2-insn-loop.c
+
+get_func_info main
+get_func_info foo
+
+set asm_file [standard_output_file $srcfile2]
+
+save_vars { srcfile } {
+    set srcfile $srcfile3
+    source $srcdir/$subdir/dw2-insn-loop.exp.tcl
+}
+
+if { [prepare_for_testing "failed to prepare" $testfile \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return
+}
+
+if { ![runto_main temporary] } {
+    return
+}
+
+set re_foo [string_to_regexp "/* foo return.  */"]
+
+set re_loop_start [string_to_regexp "/* loop start.  */"]
+
+set re_main_return [string_to_regexp "/* main return.  */"]
+
+set in_foo 0
+gdb_test_multiple step "step into foo" {
+    -re -wrap $re_foo {
+	set in_foo 1
+	pass $gdb_test_name
+    }
+}
+
+if { ! $in_foo } {
+    return
+}
+
+set in_foo 0
+gdb_test_multiple step "step out of foo using step" {
+    -re -wrap $re_foo {
+	set in_foo 1
+	pass $gdb_test_name
+    }
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-insn-loop.c b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop.c
new file mode 100644
index 00000000000..3eb0207d353
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop.c
@@ -0,0 +1,43 @@
+/*
+   Copyright 2024 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/>.  */
+
+static int loop = 5;
+
+static int
+foo (void)
+{ /* foo start.  */
+  asm ("foo_label: .globl foo_label");
+  return --loop; /* foo return.  */
+}
+
+int
+main (void)
+{ /* main start.  */
+  asm ("main_label: .globl main_label");
+
+#ifdef NOP
+  volatile int i = 0;
+#endif
+
+  int res;
+ loop_label:
+  res = foo (); /* loop start.  */
+  if (res)
+    goto loop_label;
+
+  asm ("main_label_2: .globl main_label_2");
+  return 0; /* main return.  */
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp
new file mode 100644
index 00000000000..e9e9ae1086c
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp
@@ -0,0 +1,89 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2024 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/>.
+
+load_lib dwarf.exp
+
+require dwarf2_support
+
+standard_testfile .c .S
+
+get_func_info main
+get_func_info foo
+
+set asm_file [standard_output_file $srcfile2]
+
+source $srcdir/$subdir/dw2-insn-loop.exp.tcl
+
+if { [prepare_for_testing "failed to prepare" $testfile \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return
+}
+
+if { ![runto_main temporary] } {
+    return
+}
+
+set re_foo [string_to_regexp "/* foo return.  */"]
+
+set re_loop_start [string_to_regexp "/* loop start.  */"]
+
+set re_main_return [string_to_regexp "/* main return.  */"]
+
+set in_foo 0
+gdb_test_multiple step "step into foo" {
+    -re -wrap $re_foo {
+	set in_foo 1
+	pass $gdb_test_name
+    }
+}
+
+if { ! $in_foo } {
+    return
+}
+
+set in_loop 0
+set in_foo 0
+gdb_test_multiple step "step out of foo using step" {
+    -re -wrap $re_loop_start {
+	set in_loop 1
+	pass $gdb_test_name
+    }
+    -re -wrap $re_foo {
+	set in_foo 1
+	# Behaviour in gdb 15 and earlier.
+	fail $gdb_test_name
+    }
+}
+
+if { ! $in_foo && ! $in_loop } {
+    return
+}
+
+if { $in_loop } {
+    gdb_test step $re_foo \
+	"step into foo, again"
+}
+
+gdb_test_multiple next "step out of foo using next" {
+    -re -wrap $re_loop_start {
+	pass $gdb_test_name
+    }
+    -re -wrap $re_main_return {
+	# Behaviour in gdb 15 and earlier.
+	fail $gdb_test_name
+    }
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp.tcl b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp.tcl
new file mode 100644
index 00000000000..88f0603f9cd
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp.tcl
@@ -0,0 +1,77 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2024 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/>.
+
+set foo_start_line [gdb_get_line_number "foo start."]
+set foo_return_line [gdb_get_line_number "foo return."]
+
+set main_start_line [gdb_get_line_number "main start."]
+set main_loop_line [gdb_get_line_number "loop start."]
+set main_return_line [gdb_get_line_number "main return."]
+
+Dwarf::assemble $asm_file {
+    declare_labels lines_unit
+
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_name $::srcfile}
+	    {DW_AT_stmt_list $lines_unit DW_FORM_sec_offset}
+	} {
+	    DW_TAG_subprogram {
+		{ DW_AT_name foo }
+		{ DW_AT_low_pc $::foo_start DW_FORM_addr }
+		{ DW_AT_high_pc $::foo_end DW_FORM_addr }
+	    }
+
+	    DW_TAG_subprogram {
+		{ DW_AT_name main }
+		{ DW_AT_low_pc $::main_start DW_FORM_addr }
+		{ DW_AT_high_pc $::main_end DW_FORM_addr }
+	    }
+	}
+    }
+
+    lines {} lines_unit {
+	file_name $::srcfile 1
+	program {
+	    DW_LNE_set_address $::foo_start
+	    line $::foo_start_line
+	    DW_LNS_copy
+
+	    DW_LNE_set_address foo_label
+	    line $::foo_return_line
+	    DW_LNS_copy
+
+	    DW_LNE_set_address $::foo_end
+	    DW_LNE_end_sequence
+
+	    DW_LNE_set_address $::main_start
+	    line $::main_start_line
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label
+	    line $::main_loop_line
+	    DW_LNS_copy
+
+	    DW_LNE_set_address main_label_2
+	    line $::main_return_line
+	    DW_LNS_copy
+
+	    DW_LNE_set_address $::main_end
+	    DW_LNE_end_sequence
+	}
+    }
+}

base-commit: 897a57a12626b8f41490bf62bac212b0e4a7566b
-- 
2.35.3



More information about the Gdb-patches mailing list