Re: [PATCH] Fix test next-reverse-bkpt-over-sr.exp

[Bruno Larsen <blarsen@redhat.com>]

On 27/09/2022 18:14, Carl Love via Gdb-patches wrote:
> GDB maintainers:
>
> The next-reverse-bkpt-over-sr.exp test sets a breakpoint on *callee.
> The intention is the test is setting a breakpoint on the entry point of
> the function.  However on PowerPC, the statement sets the breakpoint on
> the global entry point.  The test uses the local entry point to the
> function callee and thus the breakpoint on the global entry point is
> never hit resulting in the test failing.
>
> This patch changes the break instruction to callee to properly set the
> breakpoint.
>
> The patch has been tested on both Power10 and X86-64 with no regression
> errors.
>
> Please let me know if this patch is acceptable for mainline.  Thanks.
>
>                      Carl Love
>
Hi Carl,

I don't think this change is acceptable. Looking at the comment at the 
top of next-reverse-bkpt-over-sr.exp, we see the following:

# reverse-next over a function call sets a step-resume breakpoint at
# callee's entry point, runs to it, and then does an extra single-step
# to get at the callee's caller.  Test that a user breakpoint set at
# the same location as the step-resume breakpoint isn't ignored.

Based on this, the test seems to expect that a user has placed the 
breakpoint at the exact same instruction as where GDB will place the 
step-resume breakpoint.

By getting GDB to print where it is placing the step-resume breakpoint 
(using the patch at the end of this email), and running the test 
manually to see where breakpoints are located when using "break callee" 
and "break *callee", I get the following output:

(gdb) rn
setting step_resume_breakpoint at 0x401136
55         callee();    /* NEXT OVER THIS CALL */
(gdb) b callee
Breakpoint 2 at 0x40113a: file 
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/step-reverse.c, 
line 26.
(gdb) b *callee
Breakpoint 3 at 0x401136: file 
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/step-reverse.c, 
line 25.

As you can see, setting a breakpoint at callee does not create a user 
breakpoint at the same instruction as the step-resume breakpoint in 
x86_64 processors, rendering this test useless.

I encourage you to try the same patch I used to get the location for the 
step-resume breakpoint on powerpc architectures, and if they turn out to 
be the same, maybe you can make an architecture check at the top of this 
test, changing the location of the breakpoint based on it.

Cheers,
Bruno

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 1957e8020dd..1699f4a0c6c 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7130,6 +7130,7 @@ process_event_stop_test (struct 
execution_control_state *ecs)
                 {
                   /* Normal function call return (static or dynamic).  */
                   symtab_and_line sr_sal;
+                 gdb_printf ("setting step_resume_breakpoint at 
0x%lx\n", ecs->stop_func_start);
                   sr_sal.pc = ecs->stop_func_start;
                   sr_sal.pspace = get_frame_program_space (frame);
                   insert_step_resume_breakpoint_at_sal (gdbarch,