Re: [PATCH 3/3] Fix GDB reverse execution behavior

Luis Machado
2023-11-29T11:46:50Z

On 11/22/23 23:33, Carl Love wrote:
> GDB maintainers:
>
> The following patch changes the behavior of GDB when executing the next
> command in reverse over a line that contains multiple function calls on
> the same source code line. Currently, GDB stops in the middle of the
> line not at the beginning of the line. This patch causes GDB to stop
> at the beginning of the line. The incorrect behavior was pointed out
> by Pedro Alves in
> https://sourceware.org/pipermail/gdb-patches/2023-January/196110.html
>
> A new testcase, func-map-to-same-line.exp, is added to verify the
> behavior of GDB.
>
> The fix requires updating two testcases, gdb/testsuite/gdb.mi/mi-
> reverse.exp and gdb.reverse/finish-reverse-next.exp. The testcases
> have to issue two reverse next instructions to reach the previous
> source code line. With this patch, they only require a single reverse
> next instruction to reach the previous line.
>
> The patch has been tested on PowerPC and X86-64 with no regression
> errors.
>
> Please let me know if the patch is acceptable for mainline. Thanks.
>
> Carl
> ----------------------------------------------------
>
> Fix GDB reverse-step and reverse-next command behavior
>
> Currently GDB when executing in reverse over multiple statements in a single
> line of source code, GDB stops in the middle of the line. Thus requiring
> multiple commands to reach the previous line. GDB should stop at the first
> instruction of the line, not in the middle of the line.
>
> The following description of the incorrect behavior was taken from an
> earlier message by Pedro Alves <pedro@palves.net>:
>
> https://sourceware.org/pipermail/gdb-patches/2023-January/196110.html
>
> ---------------------------------
>
> The source line looks like:
>
> func1 (); func2 ();
>
> in the test case:
>
> (gdb) list 1
> 1 void func1 ()
> 2 {
> 3 }
> 4
> 5 void func2 ()
> 6 {
> 7 }
> 8
> 9 int main ()
> 10 {
> 11 func1 (); func2 ();
> 12 }
>
> compiled with:
>
> $ gcc reverse.c -o reverse -g3 -O0
> $ gcc -v
> ...
> gcc version 11.3.0 (Ubuntu 11.3.0-1ubuntu1~22.04)
>
> Now let's debug it with target record, using current gdb git master
> (f3d8ae90b236),
>
> $ gdb ~/reverse
> GNU gdb (GDB) 14.0.50.20230124-git
> ...
> Reading symbols from /home/pedro/reverse...
> (gdb) start
> Temporary breakpoint 1 at 0x1147: file reverse.c, line 11.
> Starting program: /home/pedro/reverse
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>
> Temporary breakpoint 1, main () at reverse.c:11
> 11 func1 (); func2 ();
> (gdb) record
>
> (gdb) disassemble /s
> Dump of assembler code for function main:
> reverse.c:
> 10 {
> 0x000055555555513f <+0>: endbr64
> 0x0000555555555143 <+4>: push %rbp
> 0x0000555555555144 <+5>: mov %rsp,%rbp
>
> 11 func1 (); func2 ();
> => 0x0000555555555147 <+8>: mov $0x0,%eax
> 0x000055555555514c <+13>: call 0x555555555129 <func1>
> 0x0000555555555151 <+18>: mov $0x0,%eax
> 0x0000555555555156 <+23>: call 0x555555555134 <func2>
> 0x000055555555515b <+28>: mov $0x0,%eax
>
> 12 }
> 0x0000555555555160 <+33>: pop %rbp
> 0x0000555555555161 <+34>: ret
> End of assembler dump.
>
> (gdb) n
> 12 }
>
> So far so good, a "next" stepped over the whole of line 11 and stopped at
> line 12.
>
> Let's confirm where we are now:
>
> (gdb) disassemble /s
> Dump of assembler code for function main:
> reverse.c:
> 10 {
> 0x000055555555513f <+0>: endbr64
> 0x0000555555555143 <+4>: push %rbp
> 0x0000555555555144 <+5>: mov %rsp,%rbp
>
> 11 func1 (); func2 ();
> 0x0000555555555147 <+8>: mov $0x0,%eax
> 0x000055555555514c <+13>: call 0x555555555129 <func1>
> 0x0000555555555151 <+18>: mov $0x0,%eax
> 0x0000555555555156 <+23>: call 0x555555555134 <func2>
> 0x000055555555515b <+28>: mov $0x0,%eax
>
> 12 }
> => 0x0000555555555160 <+33>: pop %rbp
> 0x0000555555555161 <+34>: ret
> End of assembler dump.
>
> Good, we're at the first instruction of line 12.
>
> Now let's undo the "next", with "reverse-next":
>
> (gdb) reverse-next
> 11 func1 (); func2 ();
>
> Seemingly stopped at line 11. Let's see exactly where:
>
> (gdb) disassemble /s
> Dump of assembler code for function main:
> reverse.c:
> 10 {
> 0x000055555555513f <+0>: endbr64
> 0x0000555555555143 <+4>: push %rbp
> 0x0000555555555144 <+5>: mov %rsp,%rbp
>
> 11 func1 (); func2 ();
> 0x0000555555555147 <+8>: mov $0x0,%eax
> 0x000055555555514c <+13>: call 0x555555555129 <func1>
> => 0x0000555555555151 <+18>: mov $0x0,%eax
> 0x0000555555555156 <+23>: call 0x555555555134 <func2>
> 0x000055555555515b <+28>: mov $0x0,%eax
>
> 12 }
> 0x0000555555555160 <+33>: pop %rbp
> 0x0000555555555161 <+34>: ret
> End of assembler dump.
> (gdb)
>
> And lo, we stopped in the middle of line 11! That is a bug, we should have
> stepped back all the way to the beginning of the line. The "reverse-next"
> should have fully undone the prior "next" command.
>
> --------------------
>
> This patch fixes the incorrect GDB behavior by ensuring that GDB stops at
> the first instruction in the line.
>
> The test case gdb.reverse/func-map-to-same-line.exp is added to testsuite
> to verify this fix when the line table information is and is not available.
> ---
> gdb/infcmd.c | 13 ++
> gdb/testsuite/gdb.mi/mi-reverse.exp | 5 +-
> .../gdb.reverse/finish-reverse-next.exp | 40 ++---
> .../gdb.reverse/func-map-to-same-line.c | 37 +++++
> .../gdb.reverse/func-map-to-same-line.exp | 140 ++++++++++++++++++
> 5 files changed, 201 insertions(+), 34 deletions(-)
> create mode 100644 gdb/testsuite/gdb.reverse/func-map-to-same-line.c
> create mode 100644 gdb/testsuite/gdb.reverse/func-map-to-same-line.exp
>
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index cf8cd527955..1415fe55163 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -975,6 +975,19 @@ prepare_one_step (thread_info *tp, struct step_command_fsm *sm)
> &tp->control.step_range_start,
> &tp->control.step_range_end);
>
> + if (execution_direction == EXEC_REVERSE)
> + {
> + symtab_and_line sal = find_pc_line (pc, 0);
> + symtab_and_line sal_start
> + = find_pc_line (tp->control.step_range_start, 0);
> +
> + if (sal.line == sal_start.line)
> + /* Executing in reverse, the step_range_start address is in
> + the same line. We want to stop in the previous line so
> + move step_range_start before the current line. */
> + tp->control.step_range_start--;
> + }
> +
> /* There's a problem in gcc (PR gcc/98780) that causes missing line
> table entries, which results in a too large stepping range.
> Use inlined_subroutine info to make the range more narrow. */
> diff --git a/gdb/testsuite/gdb.mi/mi-reverse.exp b/gdb/testsuite/gdb.mi/mi-reverse.exp
> index baa53a495d7..0630b8b6c7f 100644
> --- a/gdb/testsuite/gdb.mi/mi-reverse.exp
> +++ b/gdb/testsuite/gdb.mi/mi-reverse.exp
> @@ -96,11 +96,8 @@ proc test_controlled_execution_reverse {} {
> "reverse finish from callme"
>
> # Test exec-reverse-next
> - # It takes two steps to get back to the previous line,
> - # as the first step moves us to the start of the current line,
> - # and the one after that moves back to the previous line.
>
> - mi_execute_to "exec-next --reverse 2" \
> + mi_execute_to "exec-next --reverse" \
> "end-stepping-range" "main" "" \
> "basics.c" $line_main_hello "" \
> "reverse next to get over the call to do_nothing"
> diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-next.exp b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
> index 1f53b649a7d..6488f66b107 100644
> --- a/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
> +++ b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
> @@ -76,14 +76,10 @@ gdb_continue_to_breakpoint \
> repeat_cmd_until "stepi" "CALL VIA LEP" "{" "stepi into function1 call" "100"
>
> # The reverse-finish command should stop on the function call instruction
> -# which is the last instruction in the source code line. A reverse-next
> -# instruction should then stop at the first instruction in the same source
> -# code line. Another revers-next instruction stops at the previous source
> -# code line.
> +# which is the last instruction in the source code line. Another reverse-next
> +# instruction stops at the previous source code line.
> gdb_test "reverse-finish" ".*function1 \\(a, b\\); // CALL VIA LEP.*" \
> "reverse-finish function1 LEP call from LEP "
> -gdb_test "reverse-next" ".*function1 \\(a, b\\); // CALL VIA LEP" \
> - "reverse next 1 LEP entry point function call from LEP"
> gdb_test "reverse-next" ".*b = 5;.*" "reverse next 2, at b = 5, call from LEP"
>
>
> @@ -109,14 +105,10 @@ gdb_continue_to_breakpoint \
> gdb_test "step" ".*int ret = 0;.*" "step test 1"
>
> # The reverse-finish command should stop on the function call instruction
> -# which is the last instruction in the source code line. A reverse-next
> -# instruction should then stop at the first instruction in the same source
> -# code line. Another revers-next instruction stops at the previous source
> -# code line.
> +# which is the last instruction in the source code line. Another reverse-next
> +# instruction stops at the previous source code line.
> gdb_test "reverse-finish" ".*function1 \\(a, b\\); // CALL VIA LEP.*" \
> "reverse-finish function1 LEP call from function body"
> -gdb_test "reverse-next" ".*function1 \\(a, b\\); // CALL VIA LEP.*" \
> - "reverse next 1 LEP from function body"
> gdb_test "reverse-next" ".*b = 5;.*" \
> "reverse next 2 at b = 5, from function body"
>
> @@ -144,14 +136,10 @@ gdb_continue_to_breakpoint \
> repeat_cmd_until "stepi" "CALL VIA GEP" "{" "stepi into funp call"
>
> # The reverse-finish command should stop on the function call instruction
> -# which is the last instruction in the source code line. A reverse-next
> -# instruction should then stop at the first instruction in the same source
> -# code line. Another revers-next instruction stops at the previous source
> -# code line.
> +# which is the last instruction in the source code line. Another reverse-next
> +# instruction stops at the previous source code line.
> gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
> "function1 GEP call call from GEP"
> -gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
> - "reverse next 1 GEP entry point function call from GEP"
> gdb_test "reverse-next" ".*b = 50;.*" "reverse next 2 at b = 50, call from GEP"
>
> gdb_test "reverse-continue" ".*" "setup for test 4"
> @@ -180,14 +168,10 @@ repeat_cmd_until "stepi" "CALL VIA GEP" "{" "stepi into funp call again"
> gdb_test "stepi" "{" "stepi to between GEP and LEP"
>
> # The reverse-finish command should stop on the function call instruction
> -# which is the last instruction in the source code line. A reverse-next
> -# instruction should then stop at the first instruction in the same source
> -# code line. Another revers-next instruction stops at the previous source
> -# code line.
> +# which is the last instruction in the source code line. Another reverse-next
> +# instruction stops at the previous source code line.
> gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
> "function1 GEP call call from GEP again"
> -gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
> - "reverse next 1 GEP entry point function call from GEP again"
> gdb_test "reverse-next" ".*b = 50;.*" \
> "reverse next 2 at b = 50, call from GEP again"
>
> @@ -212,13 +196,9 @@ gdb_continue_to_breakpoint \
> gdb_test "step" ".*int ret = 0;.*" "step test 2"
>
> # The reverse-finish command should stop on the function call instruction
> -# which is the last instruction in the source code line. A reverse-next
> -# instruction should then stop at the first instruction in the same source
> -# code line. Another revers-next instruction stops at the previous source
> -# code line.
> +# which is the last instruction in the source code line. Another reverse-next
> +# instruction stops at the previous source code line.
> gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
> "reverse-finish function1 GEP call, from function body "
> -gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
> - "reverse next 1 GEP call from function body"
> gdb_test "reverse-next" ".*b = 50;.*" \
> "reverse next 2 at b = 50 from function body"
> diff --git a/gdb/testsuite/gdb.reverse/func-map-to-same-line.c b/gdb/testsuite/gdb.reverse/func-map-to-same-line.c
> new file mode 100644
> index 00000000000..17fe17af267
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line.c
> @@ -0,0 +1,37 @@
> +/* Copyright 2023 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/>.
> +
> + This test is used to test the reverse-step and reverse-next instruction
> + execution for a source line that contains multiple function calls. */
> +
> +void
> +func1 (void)
> +{
> +} /* END FUNC1 */
> +
> +void
> +func2 (void)
> +{
> +} /* END FUNC2 */
> +
> +int
> +main (void)
> +{
> + int a, b;
> + a = 1;
> + b = 2;
> + func1 (); func2 ();
> + a = a + b; /* START REVERSE TEST */
> +}
> diff --git a/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp b/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp
> new file mode 100644
> index 00000000000..1b23233b43e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp
> @@ -0,0 +1,140 @@
> +# Copyright 2023 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/>. */
> +
> +# This file is part of the GDB testsuite. It tests reverse stepping.
> +# Lots of code borrowed from "step-test.exp".
> +
> +# This test checks to make sure there is no regression failures for
> +# the reverse-next command when stepping back over two functions in
> +# the same line.
> +
> +require supports_reverse
> +
> +# This test uses the gcc no-column-info command which was added in gcc 7.1.
> +
> +proc run_tests {} {
> + global testfile
> +
> + clean_restart ${testfile}
> +
> + if { ![runto_main] } {
> + return
> + }
> +
> + with_test_prefix "next-test" {
> + gdb_test_no_output "record" "turn on process record"
> +
> + # This regression test verifies the reverse-step and reverse-next
> + # commands work properly when executing backwards thru a source line
> + # containing two function calls on the same source line, i.e.
> + # func1 (); func2 ();. This test is compiled so the dwarf info
> + # does not contain the line table information.
> +
> + # Test 1, reverse-next command
> + # Set breakpoint at the line after the function calls.
> + set bp_start_reverse_test [gdb_get_line_number "START REVERSE TEST"]
> +
> + gdb_breakpoint $bp_start_reverse_test temporary
> +
> + # Continue to break point for reverse-next test.
> + # Command definition: reverse-next [count]
> + # Run backward to the beginning of the previous line executed in
> + # the current (innermost) stack frame. If the line contains
> + # function calls,they will be “un-executed” without stopping.

space after comma.

> + # Starting from the first line of a function, reverse-next will
> + # take you back to the caller of that function, before the
> + # function was called, just as the normal next command would take
> + # you from the last line of a function back to its return to its
> + # caller 2.
> +
> + gdb_continue_to_breakpoint \
> + "stopped at command reverse-next test start location" \
> + ".*$bp_start_reverse_test\r\n.*"
> +
> + # The reverse-next should step all the way back to the beginning of
> + # the line, i.e. at the beginning of the func1 call.
> + gdb_test "reverse-next" ".*func1 \\(\\); func2 \\(\\);.*" \
> + " reverse-next to line with two functions"
> +
> + # We should be stopped at the first instruction of the line. A
> + # reverse-step should step back and stop at the beginning of the
> + # previous line b = 2, i.e. not in func1 ().
> + gdb_test "reverse-stepi" ".*b = 2;.*" \
> + "reverse-stepi to previous line b = 2"
> + }
> +
> + # Setup for test 2
> + clean_restart ${testfile}
> +
> + if { ![runto_main] } {
> + return
> + }
> +
> + with_test_prefix "step-test" {
> + gdb_test_no_output "record" "turn on process record"
> +
> + # Test 2, reverse-step command
> + # Set breakpoint at the line after the function calls.
> + gdb_breakpoint $bp_start_reverse_test temporary
> +
> + # Continue to the start of the reverse-step test.
> + # Command definition: reverse-step [count]
> + # Run the program backward until control reaches the start of a
> + # different source line; then stop it, and return control to gdb.
> + # Like the step command, reverse-step will only stop at the
> + # beginning of a source line. It “un-executes” the previously
> + # executed source line. If the previous source line included calls
> + # to debuggable functions, reverse-step will step (backward) into
> + # the called function, stopping at the beginning of the last
> + # statement in the called function (typically a return statement).
> + # Also, as with the step command, if non-debuggable functions are
> + # called, reverse-step will run thru them backward without
> + # stopping.
> +
> + gdb_continue_to_breakpoint \
> + "stopped at command reverse-step test start location" \
> + ".*$bp_start_reverse_test\r\n.*"
> +
> + # The first reverse step should take us call of func2 ().
> + gdb_test "reverse-step" ".*END FUNC2.*" \
> + "reverse-step into func2 "
> +
> + # The second reverse step should take us into func1 ().
> + gdb_test "reverse-step" ".*END FUNC1.*" \
> + "reverse-step into func1 "
> +
> + # The third reverse step should take us call of func1 ().
> + gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \
> + "reverse-step to line func1(); func2(), at call for func1 "
> + # We should be stopped at the first instruction of the line. A
> + # reversee stepi should take us to b = 2 ().
> + gdb_test "reverse-stepi" ".*b = 2;.*" \
> + "reverse-stepi to line b = 2 "
> + }
> +}
> +
> +standard_testfile .c
> +
> +# test with and without gcc column info enabled
> +foreach_with_prefix column_info_flag {column-info no-column-info} {
> + set options [list debug $column_info_flag]
> +
> + if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> + $options]} {
> + return -1
> + }
> +
> + run_tests
> +}

I haven't followed the discussions around this bug, but it looks like something that was
already there, and not a regression I suppose.

In any case, I've tested this on aarch64-linux and arm-linux. Works as expected and the
code looks OK to me.

Tested-By: Luis Machado <luis.machado@arm.com>
Reviewed-By: Luis Machado <luis.machado@arm.com>