[PATCH] [gdb] Handle next stepping out of function
Tom de Vries
tdevries@suse.de
Thu Sep 5 15:36:25 GMT 2024
On 9/4/24 14:16, Guinevere Larsen wrote:
> 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.
Hi Gwen,
thanks again for the review.
I've finally managed to write a patch to address this, and I've
resubmitted as a series with that one as first patch (
https://sourceware.org/pipermail/gdb-patches/2024-September/211570.html ).
Hopefully the detailed explanation in that patch answers your question.
> 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.
>
Agreed, it looks funny, but I suppose hitting the same breakpoint over
an over again with a continue might also look stuck, so I think it's a
more general problem.
The new patch mentioned above makes both next and step stop at the while
command, but for the case described above, with gcc 7.5. With gcc 13.3,
code and line info are different, and things are harder to handle.
>> 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".
>
FWIW, I suppose next could also be read as next-in-execution, which is
not the same as next-in-file.
I agree step/next docs could be improved, but I don't feel qualified to
give that a go.
Thanks,
- Tom
>> 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
>
>
More information about the Gdb-patches
mailing list