[PATCH 3/3] Fix GDB reverse execution behavior

[Carl Love <cel@linux.ibm.com>]

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.
+	#   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
+}
-- 
2.41.0

---------------------------------