Reverse-next bug test case

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

Pedro, Ulrich, Tom, Bruno:

I put together a test case that demonstrates the error in the reverse-
next command based on Pedro's example from our earlier discussion.  I
thought it would be best to make sure we all agree what the expected
behavior of the test should be before attempting to fix the issue.  :-)

The first test scenario in the test is the example that Pedo gave.  The
test expects that the reverse-next will stop at the beginning of the
line containing the two function calls.  Then a reverse-step command
should then stop at the previous source code line.  The test currently
fails the same on X86 and PowerPC, as expected, because the reverse-
next in correctly stops between func1 and func2.  The following
reverse-step stops at the end of func1 not at the previous source code
line as it should.  

I added a second test to do a reverse-step all the way back thru the
source line with the two function calls on it to make sure we agree on
the expected results for that scenario as well.  The second test passes
on X86 and PowerPC.

The test case, explanation from Pedro's test, i.e. my first test case.

The patch for the new test case is attached at the end.

Thanks for the help reviewing this test.

                  Carl 

On Tue, 2023-01-24 at 14:08 +0000, Pedro Alves wrote:

<snip>
 
> 
> Wait.  That right there sounds bogus.  The source line looks like:
> 
>    func1 ();  func2 ();
> 
> so stepping backwards over that line should always stop at the first
> instruction of the line, not in the middle.  Let's simplify this.
> 
> Here's the full source code of my example:
> 
> (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),
> without your patch:
> 
>  $ 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.  Here's the same thing without the
> distracting
> disassemble commands:
> 
>  (gdb) b 11
>  Breakpoint 1 at 0x1147: file reverse.c, line 11.
>  (gdb) r
>  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".
> 
>  Breakpoint 1, main () at reverse.c:11
>  11        func1 (); func2 ();
>  (gdb) p $pc
>  $1 = (void (*)()) 0x555555555147 <main+8>
>  (gdb) record 
>  (gdb) next
>  12      }
>  (gdb) reverse-next
>  11        func1 (); func2 ();
>  (gdb) p $pc
>  $2 = (void (*)()) 0x555555555151 <main+18>
>  (gdb) 
> 
> 
> This:
> 
>  next -> reverse-next -> next -> reverse-next
> 
> ... should leave you at the same instruction.  But it doesn't in this
> example!
> 

-------------------------------------------------------
Patch for the test case based on Pedro's test

New pedro test

---
 gdb/testsuite/gdb.reverse/pedro_test.c   |  18 ++++
 gdb/testsuite/gdb.reverse/pedro_test.exp | 118 +++++++++++++++++++++++
 2 files changed, 136 insertions(+)
 create mode 100644 gdb/testsuite/gdb.reverse/pedro_test.c
 create mode 100644 gdb/testsuite/gdb.reverse/pedro_test.exp

diff --git a/gdb/testsuite/gdb.reverse/pedro_test.c b/gdb/testsuite/gdb.reverse/pedro_test.c
new file mode 100644
index 00000000000..e3183356ff6
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/pedro_test.c
@@ -0,0 +1,18 @@
+void
+func1 ()
+{
+}
+
+void
+func2 ()
+{
+}
+
+int main ()
+{
+  int a, b;
+  a = 1;
+  b = 2;
+  func1 (); func2 ();
+  a = a + b;     // START REVERSE TEST
+}
diff --git a/gdb/testsuite/gdb.reverse/pedro_test.exp b/gdb/testsuite/gdb.reverse/pedro_test.exp
new file mode 100644
index 00000000000..d315e993d5f
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/pedro_test.exp
@@ -0,0 +1,118 @@
+# Copyright 2008-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.
+
+if ![supports_reverse] {
+    return
+}
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+runto_main
+set target_remote [gdb_is_target_remote]
+
+if [supports_process_record] {
+    # Activate process record/replay.
+    gdb_test_no_output "record" "turn on process record for test1"
+}
+
+# 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" $srcfile]
+gdb_breakpoint $srcfile:$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" \
+    ".*$srcfile:$bp_start_reverse_test\r\n.*"
+
+# The reverse-next should step all the way back to the begining of the line,
+# i.e. at the begining of the func1 call.
+gdb_test "reverse-next" ".*func1 \\(\\); func2 \\(\\);.*" \
+    "reverse-next to line with two functions"
+
+# A reverse-step should step back and stop at the begining
+# of the previous line b = 2, i.e. not in func1 ().
+gdb_test "reverse-step" ".*b = 2;.*" \
+    "reverse-step to previous line b = 2"
+
+
+# Setup for test 2
+# Go back to the start of the function
+gdb_test "reverse-continue" "a = 1;" "At start of main, setup for test 2"
+
+# Turn off record to clear logs and turn on again
+gdb_test "record stop"  "Process record is stopped.*" \
+    "turn off process record for test1"
+gdb_test_no_output "record" "turn on process record for test2"
+
+
+# Test 2, reverse-step command
+# Set breakpoint at the line after the function calls.
+gdb_breakpoint $srcfile:$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" \
+    ".*$srcfile:$bp_start_reverse_test\r\n.*"
+
+# The first reverse step should take us call of func2 ().
+gdb_test "reverse-step" ".*}.*" \
+    "reverse-step into func2 "
+
+# The second reverse step should take us call of func2 ().
+gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \
+    "reverse-step to line func1(); func2(), at call for func2 "
+
+# The third reverse step should take us into func1 ().
+gdb_test "reverse-step" ".*}.*" \
+    "reverse-step into func1 "
+
+# The fourth reverse step should take us call of func1 ().
+gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \
+    "reverse-step to line func1(); func2(), at call for func1 "
+
+# The fifth reverse step should take us to b = 2 ().
+gdb_test "reverse-step" ".*b = 2;.*" \
+    "reverse-step to line b = 2 "
-- 
2.37.2