[PATCH] [gdb] Handle next stepping out of function
Guinevere Larsen
blarsen@redhat.com
Wed Sep 4 12:16:57 GMT 2024
On 8/19/24 12:24 PM, Tom de Vries wrote:
> 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 debuginfo:
> ...
> $ g++ -g test.c
> ...
>
> Using next to step out of function f also steps out of the loop:
> ...
> $ gdb -q a.out -ex start -ex step
> 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))
> f (w=@0x7fffffffdc7c: 0) at test.c:5
> 5 bool result = ++count != 42;
> (gdb) next
> 6 return result;
> (gdb) next
> 7 }
> (gdb) next
> main () at test.c:14
> 14 return n;
> (gdb)
> ...
>
> [ In contrast, if we use step to step out of function f, we step back into
> function f at line 5. ]
>
> 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.
>
> We don't stop there, and since next sets
> ecs->event_thread->control.step_over_calls to STEP_OVER_ALL, we step over the
> call to f as well, and so on, until we exit the loop.
Wait, you have me confused here. Why do we not stop on 0x4004fb? it is a
different source code line from the one where the "next" (or step) was
emitted, so GDB should be stopping there.
Related to this bug, but not what you reported, I think the behavior of
step in this example is also wrong, and - what's worse - very confusing
for the user. Look at this log from the test:
Breakpoint 1, main () at dw2-insn-loop.c:31^M
31|_____ while (foo ()) /* main while loop. */^M
(gdb) step^M
foo () at dw2-insn-loop.c:23^M
23|_____ return --loop; /* foo return. */^M
(gdb) PASS: gdb.dwarf2/dw2-insn-loop.exp: step into foo
step^M
foo () at dw2-insn-loop.c:23^M
23|_____ return --loop; /* foo return. */^M
(gdb) PASS: gdb.dwarf2/dw2-insn-loop.exp: step back into foo using step
next^M
foo () at dw2-insn-loop.c:23^M
23|_____ return --loop; /* foo return. */^M
(gdb) PASS: gdb.dwarf2/dw2-insn-loop.exp: step back into foo using next
We step into foo, which only has the "return --loop;" line. Then you
issue a "step" and you're back at the same line. As an end user, and
possibly someone not too familiar with the inferior code, this just
looks like gdb just didn't do anything, a "next" command would be my
next guess to get the inferior unstuck, and it still looks stuck. This
would very much look like a rough GDB bug to me, if I was an end user.
In my opinion, both the "step" and "next" command should stop at
0x4004fb, the "while (foo ())" line, since IMO if you leave the
innermost frame, you are in the next source line and gdb should stop
either way.
>
> The semantics of next are:
> ...
> Continue to the next source line in the current (innermost) stack frame. This
> is similar to step, but function calls that appear within the line of code are
> executed without stopping. Execution stops when control reaches a different
> line of code at the original stack level that was executing when you gave the
> next command.
> ....
Also, I think saying "next source line" in this semantics text is
misleading. In a regular loop, using next can land you in a previous
line. The command that is guaranteed to land you on the _next_ line is
"until". So, based on that and my personal understanding/expectations of
"next", I would reword this semantics text to say "continue to a
different source line".
>
> We could argue that after leaving the current line, line 7, there's no
> obligation anymore to skip any function calls.
>
> Fix this by degrading the next into a step for this scenario.
>
> Tested on x86_64-linux and aarch64-linux.
>
> PR gdb/32000
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32000
> ---
> gdb/infrun.c | 17 +++-
> gdb/testsuite/gdb.dwarf2/dw2-insn-loop.c | 36 +++++++
> gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp | 109 +++++++++++++++++++++
> 3 files changed, 159 insertions(+), 3 deletions(-)
> create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-insn-loop.c
> create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 05e81a08e03..fc14babb182 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8180,9 +8180,20 @@ process_event_stop_test (struct execution_control_state *ecs)
> }
>
> 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))
> + bool different_line
> + = (ecs->event_thread->current_line != stop_pc_sal.line
> + || ecs->event_thread->current_symtab != stop_pc_sal.symtab);
> +
> + if (different_line && execution_direction == EXEC_FORWARD
> + && ecs->event_thread->control.step_over_calls == STEP_OVER_ALL)
> + {
> + /* Stepped out of line, degrade next to step. */
> + ecs->event_thread->control.step_over_calls
> + = STEP_OVER_UNDEBUGGABLE;
> + }
> +
> + if (different_line
> + && ecs->event_thread->stop_pc () == stop_pc_sal.pc)
> {
> /* We are at a different line. */
>
> 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..8e0f60d107f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop.c
> @@ -0,0 +1,36 @@
> +/*
> + 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");
> +
> + while (foo ()) /* main while loop. */
> + ;
> +
> + 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..f2b5f449709
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-insn-loop.exp
> @@ -0,0 +1,109 @@
> +# 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 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 "main while loop."]
> +set main_return_line [gdb_get_line_number "main return."]
> +
> +set asm_file [standard_output_file $srcfile2]
> +
> +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
> + }
> + }
> +}
> +
> +if { [prepare_for_testing "failed to prepare" $testfile \
> + [list $srcfile $asm_file] {nodebug}] } {
> + return
> +}
> +
> +if {![runto_main]} {
> + return
> +}
> +
> +set re [string_to_regexp "/* foo return. */"]
> +
> +gdb_test step $re \
> + "step into foo"
> +
> +gdb_test step $re \
> + "step back into foo using step"
> +
> +# Regression test for PR32000. This should not step out of the loop.
> +gdb_test next $re \
> + "step back into foo using next"
>
> base-commit: d71c16aec2a5d9a34791563a48b7347b02718b0e
--
Cheers,
Guinevere Larsen
She/Her/Hers
More information about the Gdb-patches
mailing list