Re: [PATCH] gdb/reverse: Fix stepping over recursive functions

[Bruno Larsen <blarsen@redhat.com>]

On 7/7/22 15:48, Pedro Alves wrote:
> I also don't understand why do we need all these different frame checks as above.  Breakpoints
> can have a frame associated, so that when a breakpoint hits, the breakpoint.c stuff checks
> whether the hit happened in the right frame, and if not, the breakpoint is ignored / set to
> be auto-stepped-over, and BPSTAT_WHAT_STEP_RESUME isn't reached.
> 
> The actual problem seems to me that we create the step-resume breakpoint in
> question without giving it a frame.  If I do that instead of your fix, like so:
> 
> diff --git c/gdb/infrun.c w/gdb/infrun.c
> index 02c98b50c8c..1360b8da70e 100644
> --- c/gdb/infrun.c
> +++ w/gdb/infrun.c
> @@ -7118,8 +7118,8 @@ process_event_stop_test (struct execution_control_state *ecs)
>                    symtab_and_line sr_sal;
>                    sr_sal.pc = ecs->stop_func_start;
>                    sr_sal.pspace = get_frame_program_space (frame);
> -                 insert_step_resume_breakpoint_at_sal (gdbarch,
> -                                                       sr_sal, null_frame_id);
> +                 insert_step_resume_breakpoint_at_sal
> +                   (gdbarch, sr_sal, get_stack_frame_id (frame));
>                  }
>              }
>            else
> 
> and then run your updated testcase, it passes.  I also see no regressions in gdb.reverse/ tests.
> 
> WDYT?  Are you seeing something that would break with this approach?

Hi Pedro

I have spent a while attempting to use your suggestion, but I did find something that breaks with this patch.
gdb.reverse/solib-precsave.exp shows 10 new failures. Here's a snippet before your change:

record restore /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.reverse/solib-precsave/solib.precsave
Warning: 'record restore', an alias for the command 'record full restore', is deprecated.
Use 'record full restore'.
[New LWP 22073]
Core was generated by `/home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.rever'.
Program terminated with signal SIGTRAP, Trace/breakpoint trap.
#0  main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:27
27        char* cptr = "String 1";
Restored records from core file /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.reverse/solib-precsave/solib.precsave.
#0  main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:27
27        char* cptr = "String 1";
(gdb) PASS: gdb.reverse/solib-precsave.exp: reload core file
until 46
main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:46
46        return 0;                     /* end part one */
(gdb) PASS: gdb.reverse/solib-precsave.exp: run until end part one
reverse-step
44        shr1 ("message 3\n");         /* shr1 three */
(gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step third shr1
reverse-step
43        shr1 ("message 2\n");         /* shr1 two */
(gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step second shr1
reverse-step
42        shr1 ("message 1\n");         /* shr1 one */
(gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step first shr1
reverse-step
40        b[0] = 6;   b[1] = 9;         /* generic statement, end part two */
(gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step generic
until 46
main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:46
46        return 0;                     /* end part one */
(gdb) PASS: gdb.reverse/solib-precsave.exp: forward to end part one
reverse-next
44        shr1 ("message 3\n");         /* shr1 three */
(gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-next third shr1


and after your change:

record restore /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.reverse/solib-precsave/solib.precsave
Warning: 'record restore', an alias for the command 'record full restore', is deprecated.
Use 'record full restore'.
[New LWP 21721]
Core was generated by `/home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.rever'.
Program terminated with signal SIGTRAP, Trace/breakpoint trap.
#0  main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:27
27        char* cptr = "String 1";
Restored records from core file /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.reverse/solib-precsave/solib.precsave.
#0  main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:27
27        char* cptr = "String 1";
(gdb) PASS: gdb.reverse/solib-precsave.exp: reload core file
until 46
main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:46
46        return 0;                     /* end part one */
(gdb) PASS: gdb.reverse/solib-precsave.exp: run until end part one
reverse-step
44        shr1 ("message 3\n");         /* shr1 three */
(gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step third shr1
reverse-step
43        shr1 ("message 2\n");         /* shr1 two */
(gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step second shr1
reverse-step
42        shr1 ("message 1\n");         /* shr1 one */
(gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step first shr1
reverse-step
40        b[0] = 6;   b[1] = 9;         /* generic statement, end part two */
(gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step generic
until 46
main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:46
46        return 0;                     /* end part one */
(gdb) PASS: gdb.reverse/solib-precsave.exp: forward to end part one
reverse-next

No more reverse-execution history.
main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:27
27        char* cptr = "String 1";
(gdb) FAIL: gdb.reverse/solib-precsave.exp: reverse-next third shr1

And a few more reverse-next fails after this. I haven't made much progress in debugging this issue because it seems that the whole record restore
has somewhat bit-rotted, as trying to restore a recording from another GDB session results in internal errors, so I wonder if this test is still
relevant, or if we can add these failures as "FIXME" since the whole feature is having issues. DO you have any thoughts on this?

Cheers!
Bruno Larsen