Fix test next-reverse-bkpt-over-sr.exp

Message ID e0f721cd4a34b242d7046d7823be04aeb54efdd7.camel@us.ibm.com
State New
Headers
Series Fix test next-reverse-bkpt-over-sr.exp |

Commit Message

Carl Love Sept. 27, 2022, 4:14 p.m. UTC
  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


---------------------------------------------------------------
Fix test next-reverse-bkpt-over-sr.exp

The test suses a gdb_test to set a breakpoint at *callee.  The message
says "set breakpoint at callee's entry".

The statement b *callee will set a breakpoint on the first instruction of
the function.  On some architectres, that may be the function entry point.
The PowerPC ABI specifies both local and global entry points.  The statement
b *callee sets the breakpoint t the global entry point and b callee sets
the breakpoint at the beginning of the fucntion, i.e. after the local
entry point and prolog.  The local entry point is generally used.  The
global entry point is only used if the TOC needs to be stup.

  int callee() {          /* ENTER CALLEE */
      100007ac:   02 10 40 3c     lis     r2,4098     <- global entry point
      100007b0:   00 7f 42 38     addi    r2,r2,32512
      100007b4:   f8 ff e1 fb     std     r31,-8(r1)  <- local entry point
      100007b8:   d1 ff 21 f8     stdu    r1,-48(r1)  <- prolog
      100007bc:   78 0b 3f 7c     mr      r31,r1      <- prolog
    return myglob++;	/* ARRIVED IN CALLEE */
      100007c0:	00 00 00 60 	nop                       <- location b callee
      100007c4:	40 81 22 39 	addi    r9,r2,-32448
      100007c8:	00 00 29 81 	lwz     r9,0(r9)
      100007cc:	01 00 09 39 	addi    r8,r9,1
       ...

The test should break on callee, not *callee, to get the desired behaviour
on PowerPC as well as other architecturs like X86-64.

This patch has been tested on Poser PC and X86-64 without any regression
failures.
---
 gdb/testsuite/gdb.reverse/next-reverse-bkpt-over-sr.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Bruno Larsen Sept. 28, 2022, 7:35 a.m. UTC | #1
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,
  

Patch

diff --git a/gdb/testsuite/gdb.reverse/next-reverse-bkpt-over-sr.exp b/gdb/testsuite/gdb.reverse/next-reverse-bkpt-over-sr.exp
index 6ef56d30e7b..4e163c5c2f8 100644
--- a/gdb/testsuite/gdb.reverse/next-reverse-bkpt-over-sr.exp
+++ b/gdb/testsuite/gdb.reverse/next-reverse-bkpt-over-sr.exp
@@ -45,7 +45,7 @@  if [supports_process_record] {
 set lineno [gdb_get_line_number "STEP INTO THIS CALL"]
 gdb_test "advance $lineno" ".*STEP INTO THIS CALL.*" "get past callee call"
 
-gdb_test "b \*callee" "" "set breakpoint at callee's entry"
+gdb_test "b callee" "" "set breakpoint at callee's entry"
 
 set bpnum [get_integer_valueof "\$bpnum" 0]
 gdb_test "reverse-next" \