[v2,1/2] Handle stepping out of function into loop, case 1
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
Commit Message
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.
After submitting v1, the Linaro CI reported a FAIL in the test-case
gdb.dwarf2/dw2-insn-loop-nop.exp on aarch64-linux.
The problem there is that the "stepped into subroutine" case in
process_event_stop_test doesn't get triggered, because the
"stepping inside range" gets triggered before. This didn't happen on
x86_64, but with aarch64 foo is frameless and consequently the stepping range
spans the entire function foo, including the first instruction.
Fix this by making the stepping range empty.
Tested on x86_64-linux and aarch64-linux.
PR gdb/32000
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32000
---
gdb/infrun.c | 29 +++++-
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, 320 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
base-commit: f83ca9c7cca4ab7a2d7352104a37932474320410
@@ -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,25 @@ 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,
+ and
+ - make the stepping range empty to prevent stepping back into the
+ stepping range. */
+ ecs->event_thread->control.step_stack_frame_id
+ = get_stack_frame_id (frame);
+ ecs->event_thread->control.step_range_start
+ = ecs->event_thread->control.step_range_end;
+ keep_going (ecs);
+ return;
+ }
+ }
if (execution_direction == EXEC_REVERSE
&& *curr_frame_id != original_frame_id
new file mode 100644
@@ -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"
new file mode 100644
@@ -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
+ }
+}
new file mode 100644
@@ -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. */
+}
new file mode 100644
@@ -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
+ }
+}
new file mode 100644
@@ -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
+ }
+ }
+}