[Bug gdb/29962] New: FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call

[Bug gdb/29962] New: FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29962

            Bug ID: 29962
           Summary: FAIL: gdb.reverse/step-reverse.exp: reverse step into
                    fn call
           Product: gdb
           Version: unknown
            Status: NEW
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

[ Likewise for gdb.reverse/step-precsave.exp, which uses the same source file.
]

On ubuntu 20.04.5 x86_64 (using gcc 9.4.0), I run into:
...
(gdb) PASS: gdb.reverse/step-reverse.exp: simple reverse stepi
step^M
78         a[5] = a[3] - a[4]; /* FINISH TEST */^M
(gdb) FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call
step^M
...

The basic problem here is that the forward execution goes like:
...
$ ./build/gdb/gdb -q -batch
./build/gdb/testsuite/outputs/gdb.reverse/step-reverse/step-reverse -ex "b 78"
-ex run -ex "set trace-commands on" -ex stepi -ex "stepi" -ex "stepi" -ex
"stepi" -ex "stepi"
Breakpoint 1 at 0x123b: file
/home/vries/gdb/src/gdb/testsuite/gdb.reverse/step-reverse.c, line 78.

Breakpoint 1, main () at
/home/vries/gdb/src/gdb/testsuite/gdb.reverse/step-reverse.c:78
78         a[5] = a[3] - a[4]; /* FINISH TEST */
+stepi
78         a[5] = a[3] - a[4]; /* FINISH TEST */
+stepi
78         a[5] = a[3] - a[4]; /* FINISH TEST */
+stepi
0x0000555555555243      78         a[5] = a[3] - a[4]; /* FINISH TEST */
+stepi
78         a[5] = a[3] - a[4]; /* FINISH TEST */
+stepi
79         callee();    /* STEPI TEST */
$ 
...
because we have:
...
INDEX  LINE   ADDRESS            IS-STMT PROLOGUE-END 
26     75     0x0000555555555231 Y                    
27     78     0x000055555555523b Y                    
28     78     0x000055555555523e Y                    
29     78     0x0000555555555241 Y                    
30     78     0x0000555555555245 Y                    
31     79     0x0000555555555248 Y                    
...
and when doing the backward execution, we stop at 0x0000555555555245 rather
stepi-ing to 0x000055555555523b as is the intention.

This simple test-case patch fixes it:
...
diff --git a/gdb/testsuite/gdb.reverse/step-reverse.exp
b/gdb/testsuite/gdb.reverse/step-reverse.exp
index 27e4b175274..8ed4f6d0269 100644
--- a/gdb/testsuite/gdb.reverse/step-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/step-reverse.exp
@@ -72,6 +72,8 @@ gdb_test_multiple "finish" "$test_message" {
     }
 }

+set pc_finish_test [get_valueof /x {$pc} ""]
+
 # stepi over flat code (no calls)

 set test_message "simple stepi"
@@ -216,7 +218,13 @@ gdb_test_multiple "stepi" "$test_message" {
        exp_continue
     }
     -re "$stepi_location.* FINISH TEST.*$gdb_prompt $" {
-       pass "$test_message"
+       set pc [get_valueof /x {$pc} ""]
+       if { $pc != $pc_finish_test } {
+           send_gdb "stepi\n"
+           exp_continue
+       } else {
+           pass "$test_message"
+       }
     }
     -re "STEP INTO THIS CALL.*$gdb_prompt $" {
        fail "$test_message (too far)"
...
(though it introduces a duplicate for the get_valueof).

OTOH, this can be fixed by:
...
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 851852404eb..0e6ee8fe04d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -20205,11 +20205,6 @@ dwarf_record_line_p (struct dwarf2_cu *cu,
     return 1;
   if (line != last_line)
     return 1;
-  /* Same line for the same file that we've seen already.
-     As a last check, for pr 17276, only record the line if the line
-     has never had a non-zero discriminator.  */
-  if (!line_has_non_zero_discriminator)
-    return 1;
   return 0;
 } 
...
which gets us:
...
25     75     0x0000555555555231 Y                    
26     78     0x000055555555523b Y                    
27     79     0x0000555555555248 Y                    
...
and:
...
Breakpoint 1, main () at step-reverse.c:78
78         a[5] = a[3] - a[4]; /* FINISH TEST */
+stepi
0x000055555555523e      78         a[5] = a[3] - a[4]; /* FINISH TEST */
+stepi
0x0000555555555241      78         a[5] = a[3] - a[4]; /* FINISH TEST */
+stepi
0x0000555555555243      78         a[5] = a[3] - a[4]; /* FINISH TEST */
+stepi
0x0000555555555245      78         a[5] = a[3] - a[4]; /* FINISH TEST */
+stepi
79         callee();    /* STEPI TEST */
...

So in that sense, it's similar to PR25911.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29962] FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29962

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Hmm, I'm starting to wonder if I mis-analyzed this.

Looking at:
...
stepi^M
78         a[5] = a[3] - a[4]; /* FINISH TEST */^M
(gdb) PASS: gdb.reverse/step-reverse.exp: simple reverse stepi
step^M
78         a[5] = a[3] - a[4]; /* FINISH TEST */^M
(gdb) FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call
...
we see a step from line 78 to line ... 78.  That looks like the problem.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29962] FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29962

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
The decision to stop stepping happens here in this code in
process_event_stop_test:
...
     /* When stepping backward, stop at beginning of line range                 
         (unless it's the function entry point, in which case                   
         keep going back to the call point).  */
      CORE_ADDR stop_pc = ecs->event_thread->stop_pc ();
      if (stop_pc == ecs->event_thread->control.step_range_start
          && stop_pc != ecs->stop_func_start
          && execution_direction == EXEC_REVERSE)
        end_stepping_range (ecs);
      else
        keep_going (ecs);

      return;
    }

  /* We stepped out of the stepping range.  */
...

AFAICT, there just isn't enough information at this point to decide to stop
stepping.

This code is part of an early-out that tries to do:
...
 /* If stepping through a line, keep going if still within it.                  
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29962] FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29962

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
As mentioned before, this failure shows up with gcc 9.4.0.

It doesn't show up with gcc 12.2.1, because of a difference in generated code.

In the 12.2.1 case we have 4 insns:
...
  401208:       8b 55 cc                mov    -0x34(%rbp),%edx
  40120b:       8b 45 d0                mov    -0x30(%rbp),%eax
  40120e:       29 c2                   sub    %eax,%edx
  401210:       89 55 d4                mov    %edx,-0x2c(%rbp)
...
each with their line number entry:
... 
step-reverse.c         78            0x401208               x
step-reverse.c         78            0x40120b               x
step-reverse.c         78            0x40120e               x
step-reverse.c         78            0x401210               x
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29962] FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29962

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #3)
> As mentioned before, this failure shows up with gcc 9.4.0.
> 
> It doesn't show up with gcc 12.2.1, because of a difference in generated
> code.
> 
> In the 12.2.1 case we have 4 insns:
> ...
>   401208:       8b 55 cc                mov    -0x34(%rbp),%edx
>   40120b:       8b 45 d0                mov    -0x30(%rbp),%eax
>   40120e:       29 c2                   sub    %eax,%edx
>   401210:       89 55 d4                mov    %edx,-0x2c(%rbp)
> ...
> each with their line number entry:
> ... 
> step-reverse.c         78            0x401208               x
> step-reverse.c         78            0x40120b               x
> step-reverse.c         78            0x40120e               x
> step-reverse.c         78            0x401210               x
> ...

In the gcc 9.4.0 case, we have 5 insns:
...
  401228:       8b 55 cc                mov    -0x34(%rbp),%edx
  40122b:       8b 45 d0                mov    -0x30(%rbp),%eax
  40122e:       29 c2                   sub    %eax,%edx
  401230:       89 d0                   mov    %edx,%eax
  401232:       89 45 d4                mov    %eax,-0x2c(%rbp)
...
but still 4 line number entries:
...
step-reverse.c           78            0x401228               x
step-reverse.c           78            0x40122b               x
step-reverse.c           78            0x40122e               x
step-reverse.c           78            0x401232               x
...
Note that the insn at 0x401230 doesn't have it's own line number entry.

So, we are at line 79:
...
(gdb) stepi^M
79         callee();    /* STEPI TEST */^M
...
and do a stepi, which lands us, as expected at insn 0x401232:
...
stepi^M
78         a[5] = a[3] - a[4]; /* FINISH TEST */^M
(gdb) PASS: gdb.reverse/step-reverse.exp: simple reverse stepi
print /x $pc^M
$2 = 0x401232^M
(gdb) PASS: gdb.reverse/step-reverse.exp: print /x $pc
...

Then we do a step, which lands us, unexpectedly, at insn 0x40122e:
...
step^M
78         a[5] = a[3] - a[4]; /* FINISH TEST */^M
(gdb) FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call
print /x $pc^M
$3 = 0x40122e^M
(gdb) PASS: gdb.reverse/step-reverse.exp: print /x $pc
...

It's stepping over the 0x401230 insn without line number entry that triggers
the problem.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29962] FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29962

--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 14575
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14575&action=edit
log file including debug infrun output (gcc 9.4.0 case)

$ gdb -q -batch outputs/gdb.reverse/step-reverse/step-reverse -ex "set
trace-commands on" -ex start -ex record -ex "break 79" -ex continue -ex
reverse-stepi -ex "set debug infrun 1" -ex reverse-step 2>&1 | tee log.txt

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29962] FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29962

--- Comment #6 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 14576
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14576&action=edit
log file including debug infrun output (gcc 12.2.1 case)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29962] FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29962

--- Comment #7 from Tom de Vries <vries at gcc dot gnu.org> ---
This passes gdb.reverse/*.exp:
...
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 181d961d80d..ba0b10de965 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -6900,11 +6900,12 @@ process_event_stop_test (struct execution_control_state
*ecs)
       if (stop_pc == ecs->event_thread->control.step_range_start
          && stop_pc != ecs->stop_func_start
          && execution_direction == EXEC_REVERSE)
-       end_stepping_range (ecs);
+       ;
       else
-       keep_going (ecs);
-
-      return;
+       {
+         keep_going (ecs);
+         return;
+       }
     }

   /* We stepped out of the stepping range.  */
@@ -7401,6 +7402,8 @@ process_event_stop_test (struct execution_control_state
*ecs)
                               "it's not the start of a statement");
        }
     }
+  else if (execution_direction == EXEC_REVERSE)
+    refresh_step_info = false;

   /* We aren't done stepping.

...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29962] FAIL: gdb.reverse/step-reverse.exp: reverse step into fn call

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29962

--- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 14588
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14588&action=edit
Tentative patch

-- 
You are receiving this mail because:
You are on the CC list for the bug.