[PING] Re: [PATCH] PowerPC, fix gdb.reverse/finish-reverse-bkpt.exp and gdb.reverse/next-reverse-bkpt-over-sr.exp

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

Bruno:

When you have a chance, could you take a look at the new version of the
test and see if you agree this does not break the test like my initial
fix did.  Thanks.

                      Carl Love

On Mon, 2022-11-14 at 13:05 -0800, Carl Love wrote:
> Bruno, GDB maintainers:
> 
> The first attempt to fix test next-reverse-bkpt-over-sr.exp basically
> breaks the test per the comments from Bruno in the thread with the
> subject [PATCH] Fix test next-reverse-bkpt-over-sr.exp.
> 
> The following is a second, new approach to fixing the issue.
> 
> PowerPC uses a Global Entry Point (GEP) and a Local Entry Point
> (LEP). 
> The GEP is located on the first instruction of the function.  It sets
> up register r2 before the LEP.
> 
>   <callee>:
>    lis     r2,4098        <- GEP
>    addi    r2,r2,32512
>    mflr    r0             <- LEP
>    std     r0,16(r1)
>    ...
> 
> The issue is the gdb command break *callee sets the breakpoint on the
> first instruction in the function.  The call to function callee
> enters
> the function at the LEP.  Thus gdb never "sees" the breakpoint at the
> GEP resulting in the test failing on PowerPC.  If the function is
> called using a function pointer, then the linker will setup the call
> to
> use the GEP rather instead of the LEP and the test then works as
> expected on PowerPC.  
> 
> A new function pointer call to callee afollowed by a normal call to
> callee is added for test next-reverse-bkpt-over-sr.exp at the end of
> the main so as to not change the test behavior of step-reverse.exp. 
> Note, the source file step-reverse.c is used by both test next-
> reverse-
> bkpt-over-sr.exp and test step-reverse.exp.  Test next-reverse-bkpt-
> over-sr.exp starts at the new call to callee at the end of the main
> program. The rest of the test runs without changes.  By entering via
> the GEP on PowerPC, the breakpoint is seen and the test passes as
> expected.  
> 
> Note, on non PowerPC systems the GEP and LEP are the same.  Calling
> function callee normally or with a function pointer has the same
> behavior on non-PowerPC systems.  This was specifically verified on
> Intel X86-64.
> 
> Test gdb.reverse/finish-reverse-bkpt.exp also fails on PowerPC for
> the
> same reasons as discussed above.  Again, this test is modified by
> adding a function pointer call to void_func at the end of main.  The
> source code is used for both test finish-reverse-bkpt.exp and finish-
> reverse.exp so the a breakpoint is added to finish-reverse-bkpt.exp
> to
> stop before the function pointer call to void_func where the test
> then
> starts.
> 
> The addition of the function pointer declarations in the two source
> files require an additional next statement to tests step-precsave.exp
> and step-reverse.exp to keep the keep things lined up.
> 
> Hopefully, this approach to fixing the failing tests on PowerPC is
> acceptable without changing the behavior on non-PowerPC platforms.
> 
> The patch has been tested on both PowerPC and X86-64 with no
> regressions.
> 
> Please let me know if this patch is acceptable.  Thanks.
> 
>                           Carl Love
> 
> --------------------------------
> PowerPC, fix gdb.reverse/finish-reverse-bkpt.exp and
> gdb.reverse/next-reverse-bkpt-over-sr.exp
> 
> The tests set a break point with the command break *func.  This sets
> a
> breakpoint on the first instruction of the function.  PowerPC uses
> Global Entry Points (GEP) and Local Entry Points (LEP).  The first
> instruction in the function is the GEP.  The GEP sets up register
> r2 before reaching the LEP.  When the function is called with func()
> the
> function is entered via the LEP and the test fails because GDB does
> not
> see the breakpoint on the GEP.  However, if the function is called
> via a
> function pointer, execution begins at the GEP as the test expects.
> 
> The tests were modified to call the function with a function pointer
> so
> the test will work correctly on both PowerPC with a GEP and LEP as
> well as
> on other systems.  The GEP is the same as the LEP on non PowerPC
> systems.
> 
> This patch fixes two PowerPC test failures in each of the tests
> gdb.reverse/finish-reverse-bkpt.exp and
> gdb.reverse/next-reverse-bkpt-over-sr.exp.
> 
> Patch tested on PowerPC and Intel X86-64 with no regressions.
> ---
>  .../gdb.reverse/finish-reverse-bkpt.exp       | 30
> +++++++++++++++++++
>  gdb/testsuite/gdb.reverse/finish-reverse.c    |  4 +++
>  .../gdb.reverse/next-reverse-bkpt-over-sr.exp | 29 ++++++++++++++++-
> -
>  gdb/testsuite/gdb.reverse/step-precsave.exp   |  1 +
>  gdb/testsuite/gdb.reverse/step-reverse.c      |  9 ++++++
>  gdb/testsuite/gdb.reverse/step-reverse.exp    |  1 +
>  6 files changed, 71 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-bkpt.exp
> b/gdb/testsuite/gdb.reverse/finish-reverse-bkpt.exp
> index 2a204748d98..94bcf41dc67 100644
> --- a/gdb/testsuite/gdb.reverse/finish-reverse-bkpt.exp
> +++ b/gdb/testsuite/gdb.reverse/finish-reverse-bkpt.exp
> @@ -19,6 +19,27 @@
>  # the functions entry would be ignored.  Make sure the bug doesn't
>  # reappear.
> 
> +# The test sets a breakpoint with the command break *void_func to
> set a
> +# breakpoint on the first instruction of the function.  The issue is
> on
> +# PowerPC it uses Global Entry Points (GEP) and Local Entry Points
> (LEP).
> +# The GEP is the first instruction in the function.  It sets up
> register
> +# r2 and then reaches the LEP.
> +#
> +#   <void_func>:
> +#  	lis     r2,4098        <- GEP
> +#   	addi    r2,r2,32512
> +#   	mflr    r0             <- LEP
> +#   	std     r0,16(r1)
> +#        ....
> +
> +#
> +# The command break *void_func sets the breakpoint on the
> GEP.  Calling
> +# the function with void_func() will enter the function via the
> LEP.  So,
> +# this test needs to use a function pointer to call void_func() so
> the
> +# function will be entered via the GEP to work as designed on
> PowerPC in
> +# addition to non-PowerPC systems.  On non-PowerPC systems, the GEP
> and LEP
> +# are the same.
> +
>  if ![supports_reverse] {
>      return
>  }
> @@ -38,6 +59,15 @@ if [supports_process_record] {
>      gdb_test_no_output "record" "turn on process record"
>  }
> 
> +# Move to the function pointer call to void_func so we will use the
> GEP
> +# to enter void_func and break.
> +set breakloc [gdb_get_line_number "FUNCTION PTR" "$srcfile"]
> +gdb_test "break $breakloc" \
> +    "Breakpoint $decimal at .*$srcfile, line $breakloc\." \
> +    "set breakpoint on funp"
> +gdb_continue_to_breakpoint "funp call" ".*$srcfile:$breakloc.*"
> +
> +# Start the test
>  set breakloc [gdb_get_line_number "VOID FUNC" "$srcfile"]
>  gdb_test "tbreak void_func" \
>      "Temporary breakpoint $decimal at .*$srcfile, line $breakloc\."
> \
> diff --git a/gdb/testsuite/gdb.reverse/finish-reverse.c
> b/gdb/testsuite/gdb.reverse/finish-reverse.c
> index 316d6f6aa7e..676d960ca9c 100644
> --- a/gdb/testsuite/gdb.reverse/finish-reverse.c
> +++ b/gdb/testsuite/gdb.reverse/finish-reverse.c
> @@ -89,6 +89,7 @@ int main (int argc, char **argv)
>    float float_resultval;
>    double double_resultval;
>    int i;
> +  void (*funp) (void) = void_func;
> 
>    /* A "test load" that will insure that the function really
> returns 
>       a ${type} (as opposed to just a truncated or part of a
> ${type}).  */
> @@ -123,6 +124,9 @@ int main (int argc, char **argv)
>    testval.double_testval = 3.14159265358979323846; /*
> float_checkpoint */
>    double_resultval    = double_func ();		
>    main_test = 1;				/* double_checkpoint */
> +
> +  /* This call is used with finish-reverse-bkpt.exp.  */
> +  funp();                              /* FUNCTION PTR call to
> void_func */
>    return 0;
>  } /* end of main */
> 
> 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..d78c6ac490e 100644
> --- a/gdb/testsuite/gdb.reverse/next-reverse-bkpt-over-sr.exp
> +++ b/gdb/testsuite/gdb.reverse/next-reverse-bkpt-over-sr.exp
> @@ -22,6 +22,25 @@
>  # 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.
>  #
> +# The test sets a breakpoint with the command break *callee to set a
> +# breakpoint on the first instruction of the function.  The issue is
> on
> +# PowerPC it uses Global Entry Points (GEP) and Local Entry Points
> (LEP).
> +# The GEP is the first instruction in the function.  It sets up
> register
> +# r2 and then reaches the LEP.
> +#
> +#  <callee>:
> +#   lis     r2,4098        <- GEP
> +#   addi    r2,r2,32512
> +#   mflr    r0             <- LEP
> +#   std     r0,16(r1)
> +
> +#
> +# The command break *callee sets the breakpoint on the GEP.  Calling
> +# the function with callee() will enter the function via the
> LEP.  So,
> +# this test needs to use a function pointer to call callee() so the
> +# function will be entered via the GEP to work as designed on
> PowerPC in
> +# addition to non-PowerPC systems.  On non-PowerPC systems, the GEP
> and LEP
> +# are the same.
> 
>  if ![supports_reverse] {
>      return
> @@ -42,8 +61,12 @@ if [supports_process_record] {
>      gdb_test_no_output "record" "turn on process record"
>  }
> 
> -set lineno [gdb_get_line_number "STEP INTO THIS CALL"]
> -gdb_test "advance $lineno" ".*STEP INTO THIS CALL.*" "get past
> callee call"
> +# Move to the function pointer call to the callee call after the
> function
> +# pointer call to callee to begin the test.  The function pointer
> call to
> +# callee will use the Global Entry Point on Power.
> +set lineno [gdb_get_line_number "STEP INTO CALL AFTER FUNP CALL"]
> +gdb_test "advance $lineno" ".*STEP INTO CALL AFTER FUNP CALL.*" \
> +    "get past callee call"
> 
>  gdb_test "b \*callee" "" "set breakpoint at callee's entry"
> 
> @@ -53,5 +76,5 @@ gdb_test "reverse-next" \
>      "reverse-next over call trips user breakpoint at function entry"
> 
>  gdb_test "up" \
> -    ".*NEXT OVER THIS CALL.*" \
> +    ".*FUNCTION PTR CALL TO CALLEE.*" \
>      "stopped at the right callee call"
> diff --git a/gdb/testsuite/gdb.reverse/step-precsave.exp
> b/gdb/testsuite/gdb.reverse/step-precsave.exp
> index 3279b6ce879..602dd7e6976 100644
> --- a/gdb/testsuite/gdb.reverse/step-precsave.exp
> +++ b/gdb/testsuite/gdb.reverse/step-precsave.exp
> @@ -76,6 +76,7 @@ gdb_test "record restore $precsave" \
> 
>  # plain vanilla step/next (no count)
> 
> +gdb_test "next" ".*BREAK AT MAIN.*" "next past funp declaration"
>  gdb_test "next" ".*NEXT TEST 1.*" "next test 1"
>  gdb_test "step" ".*STEP TEST 1.*" "step test 1"
> 
> diff --git a/gdb/testsuite/gdb.reverse/step-reverse.c
> b/gdb/testsuite/gdb.reverse/step-reverse.c
> index 809c7d16dc9..984fd336510 100644
> --- a/gdb/testsuite/gdb.reverse/step-reverse.c
> +++ b/gdb/testsuite/gdb.reverse/step-reverse.c
> @@ -54,6 +54,7 @@ large_struct_by_value (struct rhomboidal r)
>  int main () {
>     int w,x,y,z;
>     int a[10], b[10];
> +   int (*funp) (void) = callee;
> 
>     /* Test "next" and "step" */
>     w = 0;	/* BREAK AT MAIN */
> @@ -90,6 +91,14 @@ int main () {
>       large_struct_by_value (r);  /* step-test.exp: large struct by
> value */
>     }
> 
> +   /* Test next-reverse-bkpt-over-sr.exp needs to call function
> callee using
> +      a function pointer to work correctly on PowerPC.  See comments
> in
> +      next-reverse-bkpt-over-sr.exp.  */
> +   funp();       /* FUNCTION PTR CALL TO CALLEE */
> +
> +   /* Test that "step" doesn't */
> +   callee();	/* STEP INTO CALL AFTER FUNP CALL */
> +
>     exit (0); /* end of main */
>  }
> 
> diff --git a/gdb/testsuite/gdb.reverse/step-reverse.exp
> b/gdb/testsuite/gdb.reverse/step-reverse.exp
> index d2975cffb5c..bce137a97ad 100644
> --- a/gdb/testsuite/gdb.reverse/step-reverse.exp
> +++ b/gdb/testsuite/gdb.reverse/step-reverse.exp
> @@ -40,6 +40,7 @@ if [supports_process_record] {
> 
>  # plain vanilla step/next (no count)
> 
> +gdb_test "next" ".*BREAK AT MAIN.*" "next past funp declaration"
>  gdb_test "next" ".*NEXT TEST 1.*" "next test 1"
>  gdb_test "step" ".*STEP TEST 1.*" "step test 1"
>