PowerPC: fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp

Message ID 15864a6b87b25c93e99a28149f23138267735f2a.camel@us.ibm.com
State New
Headers
Series PowerPC: fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp |

Commit Message

Carl Love Feb. 10, 2023, 8:55 p.m. UTC
  GDB maintainers:

I have reworked the PowerPC patch from the previous series to fix the
behavior of the reverse-finish command on PowerPC.  The patch does not
change the behavior of the reverse-finish command on other
architectures.  The patch simply make the command on PowerPC behave the
same as other architectures, specifically X86.

The issue with the reverse-finish command on PowerPC is the reverse-
finish command doesn't stop at the function call.  The issue is PowerPC
uses two entry points.  PowerPC calls the two entry points the local
entry point (LEP) and the global entry point (GEP).  The LEP is
normally used when calling a function.  The GEP is used when the table
of contents (TOC) needs to be setup before continuing execution at the
LEP.  GDB is not handling the two entry points correctly.  On systems
that don't use two entry points the LEP and the GEP are the same.

A new test case is added to verify the reverse-finish command works
properly.  It checks five cases for the reverse-finish command,
executing the command when stopped on the alternate entry point, the
normal entry point, between the entry points, the body of the function
when it is called via the normal entry point and finally the body of
the function when called via the alternate entry point.

The patch and new test have been run on Power 10, Intel X86 pre
generation 5 and an Intel X86 system with a generation 5 processor. 
The 5th generation X86 system is needed to run the btrace/tailcall.exp
and btrace/tailcall-only.exp tests that were an issue with the previous
patch series which was withdrawn from consideration.

Please let me know if the patch is acceptable.  Thanks. 

                Carl 

----------------------------------------------------------
PowerPC: fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp

PPC64 multiple entry points.  When executing in reverse the function
finish_backward sets the break point at the alternate entry point, known
as the global entry point (GEP) in PowerPC.  However if the forward
execution enters via the normal entry point, known as the local entry
point (LEP) on PowerPC, reverse execution never sees the break point at the
alternate entry point of the function.  Reverse execution continues until
the next break point is encountered thus stopping at the wrong place.

This patch adds a new address to struct execution_control_state to hold the
address of the alternate entry point (GEP).  The finish_backwards function
is updated, if the stopping point is between the normal entry point (LEP)
and the end of the function, a breakpoint is set at the normal entry point.
If the stopping point is between the entry points, a breakpoint is set to
the alternate entry point.

The patch fixes the behavior of the reverse-finish command on PowerPC to
match the behavior of the command on other platforms, specifically X86.
The patch does not change the behavior of the command on X86.

A new test is added to verify the reverse-finish command on PowerPC
correctly stops at the instruction where the function call is made.  A new
test, finish-reverse-next.exp, is added to verify the correct functionality
of the reverse-finish command.  The reverse-finish comand is expected to
stop on the instruction that jumps to the function.  Procedure
proc_step_until from test gdb.reverse/step-indirect-call-thunk.exp is moved
to lib/gdb.exp and renamed repeat_cmd_until.

The patch fixes 11 regression errors in test gdb.reverse/finish-precsave.exp
and 11 regression errors in test gdb.reverse/finish-reverse.exp.

The patch has been tested on Power 10 and X86 processor with no new
regression failures.
---
 gdb/infcmd.c                                  |  47 ++--
 gdb/infrun.c                                  |  26 ++
 .../gdb.reverse/finish-reverse-next.c         |  91 +++++++
 .../gdb.reverse/finish-reverse-next.exp       | 224 ++++++++++++++++++
 .../gdb.reverse/step-indirect-call-thunk.exp  |  33 ---
 gdb/testsuite/lib/gdb.exp                     |  33 +++
 6 files changed, 404 insertions(+), 50 deletions(-)
 create mode 100644 gdb/testsuite/gdb.reverse/finish-reverse-next.c
 create mode 100644 gdb/testsuite/gdb.reverse/finish-reverse-next.exp
  

Comments

Ulrich Weigand Feb. 17, 2023, 12:24 p.m. UTC | #1
Carl Love <cel@us.ibm.com> wrote:

This looks generally OK to me, except for two minor issues
in the code base, and one problem in the test suite:


>diff --git a/gdb/infcmd.c b/gdb/infcmd.c
>index 77206fcbfe8..a65cc700fc9 100644
>--- a/gdb/infcmd.c
>+++ b/gdb/infcmd.c
>@@ -1728,28 +1728,41 @@ finish_backward (struct finish_command_fsm *sm)
>      no way that a function up the stack can have a return address
>      that's equal to its entry point.  */
> 
>-  if (sal.pc != pc)
>-    {
>-      frame_info_ptr frame = get_selected_frame (nullptr);
>-      struct gdbarch *gdbarch = get_frame_arch (frame);
>+  CORE_ADDR alt_entry_point = sal.pc;
>+  CORE_ADDR entry_point = alt_entry_point;
>+  frame_info_ptr frame = get_selected_frame (nullptr);
>+  struct gdbarch *gdbarch = get_frame_arch (frame);
> 
>-      /* Set a step-resume at the function's entry point.  Once that's
>-	 hit, we'll do one more step backwards.  */
>-      symtab_and_line sr_sal;
>-      sr_sal.pc = sal.pc;
>-      sr_sal.pspace = get_frame_program_space (frame);
>-      insert_step_resume_breakpoint_at_sal (gdbarch,
>-					    sr_sal, null_frame_id);
>+  if (gdbarch_skip_entrypoint_p (gdbarch))
>+    /* Some architectures, like PowerPC use local and global entry points.
>+       There is only one Entry Point (GEP = LEP) for other architectures.
>+       The GEP is an alternate entry point.  The LEP is the normal entry point.
>+       The value of entry_point was initialized to the alternate entry point
>+       (GEP).  It will be adjusted to the normal entry point if the function
>+       has two entry points.  */
>+    entry_point = gdbarch_skip_entrypoint (gdbarch, sal.pc);
>+
>+  if  ((pc >= alt_entry_point) && (pc <= entry_point))
>+    /* We are either at one of the entry points or between the entry points.
>+       If we are not at the alt_entry point, go back to the alt_entry_point
>+       If we at the normal entry point step back one instruction, when we
>+       stop we will determine if we entered via the entry point or the
>+       alternate entry point.  If we are at the alternate entry point,
>+       single step back to the function call.  */
>+    tp->control.step_range_start = tp->control.step_range_end = 1;
> 
>-      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
>-    }
>   else
>     {
>-      /* We're almost there -- we just need to back up by one more
>-	 single-step.  */
>-      tp->control.step_range_start = tp->control.step_range_end = 1;
>-      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
>+      /* We are in the body of the function.  Set a breakpoint to backup to
>+	 the normal entry point.  */
>+      symtab_and_line sr_sal;
>+      sr_sal.pc = entry_point;
>+      sr_sal.pspace = get_frame_program_space (frame);
>+      insert_step_resume_breakpoint_at_sal (gdbarch, sr_sal,
>+					    null_frame_id);
>     }
>+
>+  proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
> }

This change looks much larger than it actually is, because you've
swapped the order of the if vs. else blocks (the orignal code sets
the breakpoint in the if path and the single-step range in the
else path, while you've swapped this).  Can you swap this back to
shorten the diff?

>+  if (execution_direction == EXEC_REVERSE
>+      && ecs->event_thread->control.proceed_to_finish)
>+    {
>+      /* We are executing the reverse-finish command.  */
>+      if (ecs->event_thread->stop_pc () >= ecs->stop_func_alt_start
>+	  && ecs->event_thread->stop_pc () < ecs->stop_func_start
>+	  && ecs->stop_func_alt_start != ecs->stop_func_start)

This third condition seems redundant: if stop_pc is >= func_alt_start
*and* < func_start, then func_alt_start cannot be equal func_start.


>diff --git a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
>index 94292d5eb9b..dc5cf097511 100644
>--- a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
>+++ b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
>@@ -36,39 +36,6 @@ if { ![runto_main] } {
>     return -1
> }
> 
>-# Do repeated stepping COMMANDs in order to reach TARGET from CURRENT
>-#
>-#  COMMAND is a stepping command
>-#  CURRENT is a string matching the current location
>-#  TARGET  is a string matching the target location
>-#  TEST    is the test name
>-#
>-# The function issues repeated COMMANDs as long as the location matches
>-# CURRENT up to a maximum of 100 steps.
>-#
>-# TEST passes if the resulting location matches TARGET and fails
>-# otherwise.
>-#
>-proc step_until { command current target test } {
>-    global gdb_prompt
>-
>-    set count 0
>-    gdb_test_multiple "$command" "$test" {
>-        -re "$current.*$gdb_prompt $" {
>-            incr count
>-            if { $count < 100 } {
>-                send_gdb "$command\n"
>-                exp_continue
>-            } else {
>-                fail "$test"
>-            }
>-        }
>-        -re "$target.*$gdb_prompt $" {
>-            pass "$test"
>-        }
>-    }
>-}
>-
> gdb_test_no_output "record"
> gdb_test "next" ".*" "record trace"


How is it OK to just remove this procedure?  This is still used
in the rest of the step-indirect-call-thunk.exp test case ...

I get why you may want to reduce duplication, but then you'd
have to update current users of "step_until" as well.

Also, talking about duplication, should the (already separate)
gdb_step_until command in lib/gdb.exp now just be a variant
of the new repeat_cmd_until ?


Bye,
Ulrich
  
Carl Love Feb. 20, 2023, 4:34 p.m. UTC | #2
On Fri, 2023-02-17 at 12:24 +0000, Ulrich Weigand wrote:
> Carl Love <cel@us.ibm.com> wrote:
> 
> This looks generally OK to me, except for two minor issues
> in the code base, and one problem in the test suite:
> 
> 
> > diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> > index 77206fcbfe8..a65cc700fc9 100644
> > --- a/gdb/infcmd.c
> > +++ b/gdb/infcmd.c
> > @@ -1728,28 +1728,41 @@ finish_backward (struct finish_command_fsm
> > *sm)
> >      no way that a function up the stack can have a return address
> >      that's equal to its entry point.  */
> > 
> > -  if (sal.pc != pc)
> > -    {
> > -      frame_info_ptr frame = get_selected_frame (nullptr);
> > -      struct gdbarch *gdbarch = get_frame_arch (frame);
> > +  CORE_ADDR alt_entry_point = sal.pc;
> > +  CORE_ADDR entry_point = alt_entry_point;
> > +  frame_info_ptr frame = get_selected_frame (nullptr);
> > +  struct gdbarch *gdbarch = get_frame_arch (frame);
> > 
> > -      /* Set a step-resume at the function's entry point.  Once
> > that's
> > -	 hit, we'll do one more step backwards.  */
> > -      symtab_and_line sr_sal;
> > -      sr_sal.pc = sal.pc;
> > -      sr_sal.pspace = get_frame_program_space (frame);
> > -      insert_step_resume_breakpoint_at_sal (gdbarch,
> > -					    sr_sal, null_frame_id);
> > +  if (gdbarch_skip_entrypoint_p (gdbarch))
> > +    /* Some architectures, like PowerPC use local and global entry
> > points.
> > +       There is only one Entry Point (GEP = LEP) for other
> > architectures.
> > +       The GEP is an alternate entry point.  The LEP is the normal
> > entry point.
> > +       The value of entry_point was initialized to the alternate
> > entry point
> > +       (GEP).  It will be adjusted to the normal entry point if
> > the function
> > +       has two entry points.  */
> > +    entry_point = gdbarch_skip_entrypoint (gdbarch, sal.pc);
> > +
> > +  if  ((pc >= alt_entry_point) && (pc <= entry_point))
> > +    /* We are either at one of the entry points or between the
> > entry points.
> > +       If we are not at the alt_entry point, go back to the
> > alt_entry_point
> > +       If we at the normal entry point step back one instruction,
> > when we
> > +       stop we will determine if we entered via the entry point or
> > the
> > +       alternate entry point.  If we are at the alternate entry
> > point,
> > +       single step back to the function call.  */
> > +    tp->control.step_range_start = tp->control.step_range_end = 1;
> > 
> > -      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
> > -    }
> >   else
> >     {
> > -      /* We're almost there -- we just need to back up by one more
> > -	 single-step.  */
> > -      tp->control.step_range_start = tp->control.step_range_end =
> > 1;
> > -      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
> > +      /* We are in the body of the function.  Set a breakpoint to
> > backup to
> > +	 the normal entry point.  */
> > +      symtab_and_line sr_sal;
> > +      sr_sal.pc = entry_point;
> > +      sr_sal.pspace = get_frame_program_space (frame);
> > +      insert_step_resume_breakpoint_at_sal (gdbarch, sr_sal,
> > +					    null_frame_id);
> >     }
> > +
> > +  proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
> > }
> 
> This change looks much larger than it actually is, because you've
> swapped the order of the if vs. else blocks (the orignal code sets
> the breakpoint in the if path and the single-step range in the
> else path, while you've swapped this).  Can you swap this back to
> shorten the diff?

OK, swapped them.

> 
> > +  if (execution_direction == EXEC_REVERSE
> > +      && ecs->event_thread->control.proceed_to_finish)
> > +    {
> > +      /* We are executing the reverse-finish command.  */
> > +      if (ecs->event_thread->stop_pc () >= ecs-
> > >stop_func_alt_start
> > +	  && ecs->event_thread->stop_pc () < ecs->stop_func_start
> > +	  && ecs->stop_func_alt_start != ecs->stop_func_start)
> 
> This third condition seems redundant: if stop_pc is >= func_alt_start
> *and* < func_start, then func_alt_start cannot be equal func_start.

Yes, on Powerpc the third condition is redundant.  However, on other
platforms, specifically X86, the third condition is always false which
ensures the if statement is false.  This is important on the tests
gdb.btrace/tailcall-only.exp and gdb.btrace/tailcall.exp which only run
on X86.  The tests call the up command which will satisfy conditions 1
and 2 and thus do an "extra" step command resulting in a test failure. 
The third condition ensures that we never execute an additional
reverse-step unless the platform supports multiple entry points and we
are at the alternate entry point.  I can see where this is not obvious
at first glance.  I added a comment to the if statement explaining the
need for the apparent redundancy.

Also, the if statement is nested inside of the if reverse if statement.
I combined the two if statements into a single if statement.

> > diff --git a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp 
> > b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
> > index 94292d5eb9b..dc5cf097511 100644
> > --- a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
> > +++ b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
> > @@ -36,39 +36,6 @@ if { ![runto_main] } {
> >     return -1
> > }
> > 
> > -# Do repeated stepping COMMANDs in order to reach TARGET from
> > CURRENT
> > -#
> > -#  COMMAND is a stepping command
> > -#  CURRENT is a string matching the current location
> > -#  TARGET  is a string matching the target location
> > -#  TEST    is the test name
> > -#
> > -# The function issues repeated COMMANDs as long as the location
> > matches
> > -# CURRENT up to a maximum of 100 steps.
> > -#
> > -# TEST passes if the resulting location matches TARGET and fails
> > -# otherwise.
> > -#
> > -proc step_until { command current target test } {
> > -    global gdb_prompt
> > -
> > -    set count 0
> > -    gdb_test_multiple "$command" "$test" {
> > -        -re "$current.*$gdb_prompt $" {
> > -            incr count
> > -            if { $count < 100 } {
> > -                send_gdb "$command\n"
> > -                exp_continue
> > -            } else {
> > -                fail "$test"
> > -            }
> > -        }
> > -        -re "$target.*$gdb_prompt $" {
> > -            pass "$test"
> > -        }
> > -    }
> > -}
> > -
> > gdb_test_no_output "record"
> > gdb_test "next" ".*" "record trace"
> 
> How is it OK to just remove this procedure?  This is still used
> in the rest of the step-indirect-call-thunk.exp test case ...

Yes, looks like my fix up of the call to step_until in step-indirect-
call-thunk.exp got lost somewhere along the line.  I remember making
the change.

I went back to see why the regression testing didn't catch the error. 
On PowerPC, the test doesn't run as the compile options "-mindirect-
branch=thunk" and "-mfunction-return=thunk" are not supported on
PowerPC, so the test fails anyways.  It looks like the test didn't run
on my X86 either due to the regression test failing before getting to
the gdb tests.  Looks like I need to run the regression test in
subdirectory gdb to make sure all of the gdb tests get run.  

I have run the updated patch, on X86, manually and in the regression
test to make sure there are no regressions with this test.  I checked
out the base and regression runs on X86 to make sure the regression
tests are all be run.

> 
> I get why you may want to reduce duplication, but then you'd
> have to update current users of "step_until" as well.

Yes, those updates got lost rebasing the patch at some point.  Fixed.

> 
> Also, talking about duplication, should the (already separate)
> gdb_step_until command in lib/gdb.exp now just be a variant
> of the new repeat_cmd_until ?

OK, I extended the new repeat_cmd_until proceedure to take the
additional input max_steps.  I created a new version of gdb_step_until
that calls repeat_cmd_until.  I added an optional argument, current, to
gdb_step_until proceedure for the current line.  By default, current is
set to "\}" which is needed by the call to repeat_cmd_until proceedure.
The new gdb_step_until proceedure works in the existing regression
tests without changing the gdb_step_until call in the existing tests. 

Thanks for reviewing the patch.  I will post an updated patch shortly.

                         Carl
  
Guinevere Larsen Feb. 20, 2023, 4:48 p.m. UTC | #3
On 20/02/2023 17:34, Carl Love wrote:
>>> +  if (execution_direction == EXEC_REVERSE
>>> +      && ecs->event_thread->control.proceed_to_finish)
>>> +    {
>>> +      /* We are executing the reverse-finish command.  */
>>> +      if (ecs->event_thread->stop_pc () >= ecs-
>>>> stop_func_alt_start
>>> +	  && ecs->event_thread->stop_pc () < ecs->stop_func_start
>>> +	  && ecs->stop_func_alt_start != ecs->stop_func_start)
>> This third condition seems redundant: if stop_pc is >= func_alt_start
>> *and*  < func_start, then func_alt_start cannot be equal func_start.
> Yes, on Powerpc the third condition is redundant.  However, on other
> platforms, specifically X86, the third condition is always false which
> ensures the if statement is false.  This is important on the tests
> gdb.btrace/tailcall-only.exp and gdb.btrace/tailcall.exp which only run
> on X86.  The tests call the up command which will satisfy conditions 1
> and 2 and thus do an "extra" step command resulting in a test failure.
> The third condition ensures that we never execute an additional
> reverse-step unless the platform supports multiple entry points and we
> are at the alternate entry point.  I can see where this is not obvious
> at first glance.  I added a comment to the if statement explaining the
> need for the apparent redundancy.
But it is always redundant, even on x86. If the stop_func_start is A, 
there is no way the stop_pc can be simultaneously larger or equal to A, 
and strictly smaller than A. The only way the third condition would make 
a difference is if the second condition was `ecs->event_thread->stop_pc 
() <= ecs->stop_func_start`
  
Carl Love Feb. 20, 2023, 8:24 p.m. UTC | #4
Ulrich, Bruno, GDB maintainers:

Per the comments from Ulrich, I have updated the patch to address the
comments about the source code and the testcase. I updated the new
library procedure so it can be called from gdb_step_until.  So
gdb_step_until is now just a variant of the new repeat_cmd_until
proceedure.

The redundant if test has been removed.  It was added to fix a
regression testing failure I saw on the gdb.btrace/tailcall.exp on X86.
I went back and redid the testing, rebuilding everything from scratch. 
I am not able to reproduce the test failure without the redundant
check.  Not sure why I initially saw the regression failure at this
point?  Perhaps I hadn't re-enabled the
/proc/sys/kernel/perf_event_paranoid value?  It must be 2 or lower for
the test to run.  Otherwise the test fails.  Anyway, the regression
tests on X86 run without the redundant check are passing on my laptop. 

I resolved the issues with the testing on X86 to make sure it isn't
missing failures when testing gdb.reverse/step-indirect-call-thunk.exp. 
As mentioned in the previous email, the test does not run on PowerPC
since the gcc command line options "-mindirect-branch=thunk" and "-
mfunction-return=thunk" are not supported on PowerPC.  I disabled the
test on PowerPC in the updated patch.

Retested on X86 generation 5 and PowerPC with no regressions found. 

Hopefully this version of the patch is acceptable. 

                          Carl 

  -----------------------------------------------------

PowerPC: fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp

PPC64 multiple entry points.  When executing in reverse the function
finish_backward sets the break point at the alternate entry point, known
as the global entry point (GEP) in PowerPC.  However if the forward
execution enters via the normal entry point, known as the local entry
point (LEP) on PowerPC, reverse execution never sees the break point at the
alternate entry point of the function.  Reverse execution continues until
the next break point is encountered thus stopping at the wrong place.

This patch adds a new address to struct execution_control_state to hold the
address of the alternate entry point (GEP).  The finish_backwards function
is updated, if the stopping point is between the normal entry point (LEP)
and the end of the function, a breakpoint is set at the normal entry point.
If the stopping point is between the entry points, a breakpoint is set to
the alternate entry point.

The patch fixes the behavior of the reverse-finish command on PowerPC to
match the behavior of the command on other platforms, specifically X86.
The patch does not change the behavior of the command on X86.

A new test is added to verify the reverse-finish command on PowerPC
correctly stops at the instruction where the function call is made.  A new
test, finish-reverse-next.exp, is added to verify the correct functionality
of the reverse-finish command.  The reverse-finish comand is expected to
stop on the instruction that jumps to the function.  Procedure
proc_step_until from test gdb.reverse/step-indirect-call-thunk.exp is moved
to lib/gdb.exp and renamed repeat_cmd_until.

The patch fixes 11 regression errors in test gdb.reverse/finish-precsave.exp
and 11 regression errors in test gdb.reverse/finish-reverse.exp.

The patch has been tested on Power 10 and X86 processor with no new
regression failures.
---
 gdb/infcmd.c                                  |  47 ++--
 gdb/infrun.c                                  |  24 ++
 .../gdb.reverse/finish-reverse-next.c         |  91 +++++++
 .../gdb.reverse/finish-reverse-next.exp       | 224 ++++++++++++++++++
 .../gdb.reverse/step-indirect-call-thunk.exp  |  55 ++---
 gdb/testsuite/lib/gdb.exp                     |  49 +++-
 6 files changed, 421 insertions(+), 69 deletions(-)
 create mode 100644 gdb/testsuite/gdb.reverse/finish-reverse-next.c
 create mode 100644 gdb/testsuite/gdb.reverse/finish-reverse-next.exp

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 77206fcbfe8..0fa5719d38b 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1728,28 +1728,41 @@ finish_backward (struct finish_command_fsm *sm)
      no way that a function up the stack can have a return address
      that's equal to its entry point.  */
 
-  if (sal.pc != pc)
-    {
-      frame_info_ptr frame = get_selected_frame (nullptr);
-      struct gdbarch *gdbarch = get_frame_arch (frame);
+  CORE_ADDR alt_entry_point = sal.pc;
+  CORE_ADDR entry_point = alt_entry_point;
+  frame_info_ptr frame = get_selected_frame (nullptr);
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+
+  if (gdbarch_skip_entrypoint_p (gdbarch))
+    /* Some architectures, like PowerPC use local and global entry points.
+       There is only one Entry Point (GEP = LEP) for other architectures.
+       The GEP is an alternate entry point.  The LEP is the normal entry point.
+       The value of entry_point was initialized to the alternate entry point
+       (GEP).  It will be adjusted to the normal entry point if the function
+       has two entry points.  */
+    entry_point = gdbarch_skip_entrypoint (gdbarch, sal.pc);
 
-      /* Set a step-resume at the function's entry point.  Once that's
-	 hit, we'll do one more step backwards.  */
+  if  ((pc < alt_entry_point) || (pc > entry_point))
+    {
+      /* We are in the body of the function.  Set a breakpoint to backup to
+	 the normal entry point.  */
       symtab_and_line sr_sal;
-      sr_sal.pc = sal.pc;
+      sr_sal.pc = entry_point;
       sr_sal.pspace = get_frame_program_space (frame);
-      insert_step_resume_breakpoint_at_sal (gdbarch,
-					    sr_sal, null_frame_id);
-
-      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
+      insert_step_resume_breakpoint_at_sal (gdbarch, sr_sal,
+					    null_frame_id);
     }
+
   else
-    {
-      /* We're almost there -- we just need to back up by one more
-	 single-step.  */
-      tp->control.step_range_start = tp->control.step_range_end = 1;
-      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
-    }
+    /* We are either at one of the entry points or between the entry points.
+       If we are not at the alt_entry point, go back to the alt_entry_point
+       If we at the normal entry point step back one instruction, when we
+       stop we will determine if we entered via the entry point or the
+       alternate entry point.  If we are at the alternate entry point,
+       single step back to the function call.  */
+    tp->control.step_range_start = tp->control.step_range_end = 1;
+
+  proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
 }
 
 /* finish_forward -- helper function for finish_command.  FRAME is the
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 87ab73c47a4..987dbd16ea4 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1868,6 +1868,7 @@ struct execution_control_state
 
   struct target_waitstatus ws;
   int stop_func_filled_in = 0;
+  CORE_ADDR stop_func_alt_start = 0;
   CORE_ADDR stop_func_start = 0;
   CORE_ADDR stop_func_end = 0;
   const char *stop_func_name = nullptr;
@@ -4680,6 +4681,11 @@ fill_in_stop_func (struct gdbarch *gdbarch,
 	  ecs->stop_func_start
 	    += gdbarch_deprecated_function_start_offset (gdbarch);
 
+	  /* PowerPC functions have a Local Entry Point (LEP) and a Global
+	     Entry Point (GEP).  There is only one Entry Point (GEP = LEP) for
+	     other architectures.  */
+	  ecs->stop_func_alt_start = ecs->stop_func_start;
+
 	  if (gdbarch_skip_entrypoint_p (gdbarch))
 	    ecs->stop_func_start
 	      = gdbarch_skip_entrypoint (gdbarch, ecs->stop_func_start);
@@ -7269,6 +7275,24 @@ process_event_stop_test (struct execution_control_state *ecs)
 	}
     }
 
+  if (execution_direction == EXEC_REVERSE
+      && ecs->event_thread->control.proceed_to_finish
+      && ecs->event_thread->stop_pc () >= ecs->stop_func_alt_start
+      && ecs->event_thread->stop_pc () < ecs->stop_func_start)
+    {
+      /* We are executing the reverse-finish command.
+	 If the system supports multiple entry points and we are finishing a
+	 function in reverse.   If we are between the entry points singe-step
+	 back to the alternate entry point.  If we are at the alternate entry
+	 point -- just   need to back up by one more single-step, which
+	 should take us back to the function call.  */
+      ecs->event_thread->control.step_range_start
+	= ecs->event_thread->control.step_range_end = 1;
+      keep_going (ecs);
+      return;
+
+    }
+
   if (ecs->event_thread->control.step_range_end == 1)
     {
       /* It is stepi or nexti.  We always want to stop stepping after
diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-next.c b/gdb/testsuite/gdb.reverse/finish-reverse-next.c
new file mode 100644
index 00000000000..e95ee8e33a6
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/finish-reverse-next.c
@@ -0,0 +1,91 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012-2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* The reverse finish command should return from a function and stop on
+   the first instruction of the source line where the function call is made.
+   Specifically, the behavior should match doing a reverse next from the
+   first instruction in the function.  GDB should only require one reverse
+   step or next statement to reach the previous source code line.
+
+   This test verifies the fix for gdb bugzilla:
+
+   https://sourceware.org/bugzilla/show_bug.cgi?id=29927
+
+   PowerPC supports two entry points to a function.  The normal entry point
+   is called the local entry point (LEP).  The alternate entry point is called
+   the global entry point (GEP).  The GEP is only used if the table of
+   contents (TOC) value stored in register r2 needs to be setup prior to
+   execution starting at the LEP.  A function call via a function pointer
+   will entry via the GEP.  A normal function call will enter via the LEP.
+
+   This test has been expanded to include tests to verify the reverse-finish
+   command works properly if the function is called via the GEP.  The original
+   test only verified the reverse-finish command for a normal call that used
+   the LEP.  */
+
+int
+function2 (int a, int b)
+{
+  int ret = 0;
+  ret = ret + a + b;
+  return ret;
+}
+
+int
+function1 (int a, int b)   // FUNCTION1
+{
+  int ret = 0;
+  int (*funp) (int, int) = &function2;
+  /* The assembly code for this function when compiled for PowerPC is as
+     follows:
+
+     0000000010000758 <function1>:
+     10000758:	02 10 40 3c 	lis     r2,4098        <- GEP
+     1000075c:	00 7f 42 38 	addi    r2,r2,32512
+     10000760:	a6 02 08 7c 	mflr    r0             <- LEP
+     10000764:	10 00 01 f8 	std     r0,16(r1)
+     ....
+
+     When the function is called on PowerPC with function1 (a, b) the call
+     enters at the Local Entry Point (LEP).  When the function is called via
+     a function pointer, the Global Entry Point (GEP) for function1 is used.
+     The GEP sets up register 2 before reaching the LEP.
+  */
+  ret = funp (a + 1, b + 2);
+  return ret;
+}
+
+int
+main(int argc, char* argv[])
+{
+  int a, b;
+  int (*funp) (int, int) = &function1;
+
+  /* Call function via Local Entry Point (LEP).  */
+
+  a = 1;
+  b = 5;
+
+  function1 (a, b);   // CALL VIA LEP
+
+  /* Call function via Global Entry Point (GEP).  */
+  a = 10;
+  b = 50;
+
+  funp (a, b);        // CALL VIA GEP
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-next.exp b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
new file mode 100644
index 00000000000..1f53b649a7d
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
@@ -0,0 +1,224 @@
+# Copyright 2008-2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# This file is part of the GDB testsuite.  It tests reverse stepping.
+# Lots of code borrowed from "step-test.exp".
+
+# The reverse finish command should return from a function and stop on
+# the first instruction of the source line where the function call is made.
+# Specifically, the behavior should match doing a reverse next from the
+# first instruction in the function.  GDB should only take one reverse step
+# or next statement to reach the previous source code line.
+
+# This testcase verifies the reverse-finish command stops at the first
+# instruction in the source code line where the function was called.  There
+# are two scenarios that must be checked:
+#   1) gdb is at the entry point instruction for the function
+#   2) gdb is in the body of the function.
+
+# This test verifies the fix for gdb bugzilla:
+#   https://sourceware.org/bugzilla/show_bug.cgi?id=29927
+
+# PowerPC supports two entry points to a function.  The normal entry point
+# is called the local entry point (LEP).  The alternate entry point is called
+# the global entry point (GEP).  A function call via a function pointer
+# will entry via the GEP.  A normal function call will enter via the LEP.
+#
+# This test has been expanded to include tests to verify the reverse-finish
+# command works properly if the function is called via the GEP.  The original
+# test only verified the reverse-finish command for a normal call that used
+# the LEP.
+
+if ![supports_reverse] {
+    return
+}
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+runto_main
+set target_remote [gdb_is_target_remote]
+
+if [supports_process_record] {
+    # Activate process record/replay.
+    gdb_test_no_output "record" "turn on process record for test1"
+}
+
+
+### TEST 1: reverse finish from the entry point instruction (LEP) in
+### function1 when called using the normal entry point (LEP).
+
+# Set breakpoint at call to function1 in main.
+set bp_LEP_test [gdb_get_line_number "CALL VIA LEP" $srcfile]
+gdb_breakpoint $srcfile:$bp_LEP_test temporary
+
+# Continue to break point at function1 call in main.
+gdb_continue_to_breakpoint \
+    "stopped at function1 entry point instruction to stepi into function" \
+    ".*$srcfile:$bp_LEP_test\r\n.*"
+
+# stepi until we see "{" indicating we entered function1
+repeat_cmd_until "stepi" "CALL VIA LEP" "{" "stepi into function1 call" "100"
+
+# The reverse-finish command should stop on the function call instruction
+# which is the last instruction in the source code line.  A reverse-next
+# instruction should then stop at the first instruction in the same source
+# code line.  Another revers-next instruction stops at the previous source
+# code line.
+gdb_test "reverse-finish" ".*function1 \\(a, b\\);   // CALL VIA LEP.*" \
+    "reverse-finish function1 LEP call from LEP "
+gdb_test "reverse-next" ".*function1 \\(a, b\\);   // CALL VIA LEP" \
+    "reverse next 1 LEP entry point function call from LEP"
+gdb_test "reverse-next" ".*b = 5;.*" "reverse next 2, at b = 5, call from LEP"
+
+
+gdb_test "reverse-continue" ".*" "setup for test 2"
+
+# Turn off record to clear logs and turn on again
+gdb_test "record stop"  "Process record is stopped.*" \
+    "turn off process record for test1"
+gdb_test_no_output "record" "turn on process record for test2"
+
+
+### TEST 2: reverse finish from the body of function1.
+
+# Set breakpoint at call to function1 in main.
+gdb_breakpoint $srcfile:$bp_LEP_test temporary
+
+# Continue to break point at function1 call in main.
+gdb_continue_to_breakpoint \
+    "at function1 entry point instruction to step to body of function" \
+    ".*$srcfile:$bp_LEP_test\r\n.*"
+
+# do a step instruction to get to the body of the function
+gdb_test "step" ".*int ret = 0;.*" "step test 1"
+
+# The reverse-finish command should stop on the function call instruction
+# which is the last instruction in the source code line.  A reverse-next
+# instruction should then stop at the first instruction in the same source
+# code line.  Another revers-next instruction stops at the previous source
+# code line.
+gdb_test "reverse-finish" ".*function1 \\(a, b\\);   // CALL VIA LEP.*" \
+    "reverse-finish function1 LEP call from function body"
+gdb_test "reverse-next" ".*function1 \\(a, b\\);   // CALL VIA LEP.*" \
+    "reverse next 1 LEP from function body"
+gdb_test "reverse-next" ".*b = 5;.*" \
+    "reverse next 2 at b = 5, from function body"
+
+gdb_test "reverse-continue" ".*" "setup for test 3"
+
+# Turn off record to clear logs and turn on again
+gdb_test "record stop"  "Process record is stopped.*" \
+    "turn off process record for test2"
+gdb_test_no_output "record" "turn on process record for test3"
+
+
+### TEST 3: reverse finish from the alternate entry point instruction (GEP) in
+### function1 when called using the alternate entry point (GEP).
+
+# Set breakpoint at call to funp in main.
+set bp_GEP_test  [gdb_get_line_number "CALL VIA GEP" $srcfile]
+gdb_breakpoint $srcfile:$bp_GEP_test temporary
+
+# Continue to break point at funp call in main.
+gdb_continue_to_breakpoint \
+    "stopped at function1 entry point instruction to stepi into funp" \
+    ".*$srcfile:$bp_GEP_test\r\n.*"
+
+# stepi until we see "{" indicating we entered function.
+repeat_cmd_until "stepi" "CALL VIA GEP" "{" "stepi into funp call"
+
+# The reverse-finish command should stop on the function call instruction
+# which is the last instruction in the source code line.  A reverse-next
+# instruction should then stop at the first instruction in the same source
+# code line.  Another revers-next instruction stops at the previous source
+# code line.
+gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
+    "function1 GEP call call from GEP"
+gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
+    "reverse next 1 GEP entry point function call from GEP"
+gdb_test "reverse-next" ".*b = 50;.*" "reverse next 2 at b = 50, call from GEP"
+
+gdb_test "reverse-continue" ".*" "setup for test 4"
+
+# Turn off record to clear logs and turn on again
+gdb_test "record stop"  "Process record is stopped.*" \
+    "turn off process record for test3"
+gdb_test_no_output "record" "turn on process record for test4"
+
+### TEST 4: reverse finish from between the GEP and LEP in
+### function1 when called using the alternate entry point (GEP).
+
+# Set breakpoint at call to funp in main.
+set bp_GEP_test  [gdb_get_line_number "CALL VIA GEP" $srcfile]
+gdb_breakpoint $srcfile:$bp_GEP_test temporary
+
+# Continue to break point at funp call in main.
+gdb_continue_to_breakpoint \
+    "stopped at function1 entry point instruction to stepi into funp again" \
+    ".*$srcfile:$bp_GEP_test\r\n.*"
+
+# stepi until we see "{" indicating we entered function.
+repeat_cmd_until "stepi" "CALL VIA GEP" "{" "stepi into funp call again"
+
+# do one more stepi so we are between the GEP and LEP.
+gdb_test "stepi" "{" "stepi to between GEP and LEP"
+
+# The reverse-finish command should stop on the function call instruction
+# which is the last instruction in the source code line.  A reverse-next
+# instruction should then stop at the first instruction in the same source
+# code line.  Another revers-next instruction stops at the previous source
+# code line.
+gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
+    "function1 GEP call call from GEP again"
+gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
+    "reverse next 1 GEP entry point function call from GEP again"
+gdb_test "reverse-next" ".*b = 50;.*" \
+    "reverse next 2 at b = 50, call from GEP again"
+
+gdb_test "reverse-continue" ".*" "setup for test 5"
+
+# Turn off record to clear logs and turn on again
+gdb_test "record stop"  "Process record is stopped.*" \
+    "turn off process record for test4"
+gdb_test_no_output "record" "turn on process record for test5"
+
+
+### TEST 5: reverse finish from the body of function 1 when calling using the
+### alternate entrypoint (GEP).
+gdb_breakpoint $srcfile:$bp_GEP_test temporary
+
+# Continue to break point at funp call.
+gdb_continue_to_breakpoint \
+    "at function1 entry point instruction to step to body of funp call" \
+    ".*$srcfile:$bp_GEP_test\r\n.*"
+
+# Step into body of funp, called via GEP.
+gdb_test "step" ".*int ret = 0;.*" "step test 2"
+
+# The reverse-finish command should stop on the function call instruction
+# which is the last instruction in the source code line.  A reverse-next
+# instruction should then stop at the first instruction in the same source
+# code line.  Another revers-next instruction stops at the previous source
+# code line.
+gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
+    "reverse-finish function1 GEP call, from function body  "
+gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
+    "reverse next 1 GEP call from function body"
+gdb_test "reverse-next" ".*b = 50;.*" \
+    "reverse next 2 at b = 50 from function body"
diff --git a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
index 94292d5eb9b..61fb4974b8e 100644
--- a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
+++ b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
@@ -17,6 +17,12 @@ require supports_reverse
 
 standard_testfile
 
+if { [istarget "powerpc*-*-linux*"] } {
+    # GCC for PowerPC on linux does not support the -mindirect-branch and
+    # -mfunction-return command line options.
+    return 0
+}
+
 set cflags {}
 lappend cflags debug
 lappend cflags additional_flags=-mindirect-branch=thunk
@@ -36,39 +42,6 @@ if { ![runto_main] } {
     return -1
 }
 
-# Do repeated stepping COMMANDs in order to reach TARGET from CURRENT
-#
-#  COMMAND is a stepping command
-#  CURRENT is a string matching the current location
-#  TARGET  is a string matching the target location
-#  TEST    is the test name
-#
-# The function issues repeated COMMANDs as long as the location matches
-# CURRENT up to a maximum of 100 steps.
-#
-# TEST passes if the resulting location matches TARGET and fails
-# otherwise.
-#
-proc step_until { command current target test } {
-    global gdb_prompt
-
-    set count 0
-    gdb_test_multiple "$command" "$test" {
-        -re "$current.*$gdb_prompt $" {
-            incr count
-            if { $count < 100 } {
-                send_gdb "$command\n"
-                exp_continue
-            } else {
-                fail "$test"
-            }
-        }
-        -re "$target.*$gdb_prompt $" {
-            pass "$test"
-        }
-    }
-}
-
 gdb_test_no_output "record"
 gdb_test "next" ".*" "record trace"
 
@@ -88,20 +61,20 @@ gdb_test "reverse-next" "apply\.2.*" \
     "reverse-step through thunks and over inc"
 
 # We can use instruction stepping to step into thunks.
-step_until "stepi" "apply\.2" "indirect_thunk" "stepi into call thunk"
-step_until "stepi" "indirect_thunk" "inc" \
+repeat_cmd_until "stepi" "apply\.2" "indirect_thunk" "stepi into call thunk"
+repeat_cmd_until "stepi" "indirect_thunk" "inc" \
     "stepi out of call thunk into inc"
 set alphanum_re "\[a-zA-Z0-9\]"
 set pic_thunk_re  "__$alphanum_re*\\.get_pc_thunk\\.$alphanum_re* \\(\\)"
-step_until "stepi" "(inc|$pic_thunk_re)" "return_thunk" "stepi into return thunk"
-step_until "stepi" "return_thunk" "apply" \
+repeat_cmd_until "stepi" "(inc|$pic_thunk_re)" "return_thunk" "stepi into return thunk"
+repeat_cmd_until "stepi" "return_thunk" "apply" \
     "stepi out of return thunk back into apply"
 
-step_until "reverse-stepi" "apply" "return_thunk" \
+repeat_cmd_until "reverse-stepi" "apply" "return_thunk" \
     "reverse-stepi into return thunk"
-step_until "reverse-stepi" "return_thunk" "inc" \
+repeat_cmd_until "reverse-stepi" "return_thunk" "inc" \
     "reverse-stepi out of return thunk into inc"
-step_until "reverse-stepi" "(inc|$pic_thunk_re)" "indirect_thunk" \
+repeat_cmd_until "reverse-stepi" "(inc|$pic_thunk_re)" "indirect_thunk" \
     "reverse-stepi into call thunk"
-step_until "reverse-stepi" "indirect_thunk" "apply" \
+repeat_cmd_until "reverse-stepi" "indirect_thunk" "apply" \
     "reverse-stepi out of call thunk into apply"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index faa0ac05a9a..b10555fe5fb 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -9267,31 +9267,58 @@ gdb_caching_proc arm_cc_for_target {
 
 # Step until the pattern REGEXP is found.  Step at most
 # MAX_STEPS times, but stop stepping once REGEXP is found.
-#
+# START matches current location
 # If REGEXP is found then a single pass is emitted, otherwise, after
 # MAX_STEPS steps, a single fail is emitted.
 #
 # TEST_NAME is the name used in the pass/fail calls.
 
-proc gdb_step_until { regexp {test_name ""} {max_steps 10} } {
+proc gdb_step_until { regexp {test_name ""} {current ""} \
+			  { max_steps 10 } } {
+    if { $current == "" } {
+	set current "\}"
+    }
+    if { $test_name == "" } {
+	set test_name "stepping until regexp"
+    }
+
+    repeat_cmd_until "step" $current  $regexp  $test_name "10"
+}
+
+# Do repeated stepping COMMANDs in order to reach TARGET from CURRENT
+#
+#  COMMAND is a stepping command
+#  CURRENT is a string matching the current location
+#  TARGET  is a string matching the target location
+#  TEST    is the test name
+#  MAX_STEPS is number of steps attempted before fail is emitted
+#
+# The function issues repeated COMMANDs as long as the location matches
+# CURRENT up to a maximum of 100 steps.
+#
+# TEST passes if the resulting location matches TARGET and fails
+# otherwise.
+#
+proc repeat_cmd_until { command current target test_name {max_steps 100} } {
+    global gdb_prompt
     if { $test_name == "" } {
 	set test_name "stepping until regexp"
     }
 
     set count 0
-    gdb_test_multiple "step" "$test_name" {
-	-re "$regexp\r\n$::gdb_prompt $" {
-	    pass $test_name
-	}
-	-re ".*$::gdb_prompt $" {
-	    if {$count < $max_steps} {
-		incr count
-		send_gdb "step\n"
+    gdb_test_multiple "$command" "$test_name" {
+	-re "$current.*$gdb_prompt $" {
+	    incr count
+	    if { $count < $max_steps } {
+		send_gdb "$command\n"
 		exp_continue
 	    } else {
-		fail $test_name
+		fail "$test_name"
 	    }
 	}
+	-re "$target.*$gdb_prompt $" {
+	    pass "$test_name"
+	}
     }
 }
  
Carl Love Feb. 27, 2023, 4:09 p.m. UTC | #5
Ulrich, Bruno, Tom, Pedro, GDB maintainers:

Wondering if anyone has had a chance to review and test this patch? 
Would really like to make sure some can test it on an Intel Generation
5 or newer processor to make sure there are no issues with the tests
gdb.btrace/rn-dl-bind.exp and gdb.btrace/tailcall.exp: reverse-next.

Thanks for the time and effort looking at this patch.

               Carl

-------------------------------------------------

On Mon, 2023-02-20 at 12:24 -0800, Carl Love wrote:
> Ulrich, Bruno, GDB maintainers:
> 
> Per the comments from Ulrich, I have updated the patch to address the
> comments about the source code and the testcase. I updated the new
> library procedure so it can be called from gdb_step_until.  So
> gdb_step_until is now just a variant of the new repeat_cmd_until
> proceedure.
> 
> The redundant if test has been removed.  It was added to fix a
> regression testing failure I saw on the gdb.btrace/tailcall.exp on
> X86.
> I went back and redid the testing, rebuilding everything from
> scratch. 
> I am not able to reproduce the test failure without the redundant
> check.  Not sure why I initially saw the regression failure at this
> point?  Perhaps I hadn't re-enabled the
> /proc/sys/kernel/perf_event_paranoid value?  It must be 2 or lower
> for
> the test to run.  Otherwise the test fails.  Anyway, the regression
> tests on X86 run without the redundant check are passing on my
> laptop. 
> 
> I resolved the issues with the testing on X86 to make sure it isn't
> missing failures when testing gdb.reverse/step-indirect-call-
> thunk.exp. 
> As mentioned in the previous email, the test does not run on PowerPC
> since the gcc command line options "-mindirect-branch=thunk" and "-
> mfunction-return=thunk" are not supported on PowerPC.  I disabled the
> test on PowerPC in the updated patch.
> 
> Retested on X86 generation 5 and PowerPC with no regressions found. 
> 
> Hopefully this version of the patch is acceptable. 
> 
>                           Carl 
> 
>   -----------------------------------------------------
> 
> PowerPC: fix for gdb.reverse/finish-precsave.exp and
> gdb.reverse/finish-reverse.exp
> 
> PPC64 multiple entry points.  When executing in reverse the function
> finish_backward sets the break point at the alternate entry point,
> known
> as the global entry point (GEP) in PowerPC.  However if the forward
> execution enters via the normal entry point, known as the local entry
> point (LEP) on PowerPC, reverse execution never sees the break point
> at the
> alternate entry point of the function.  Reverse execution continues
> until
> the next break point is encountered thus stopping at the wrong place.
> 
> This patch adds a new address to struct execution_control_state to
> hold the
> address of the alternate entry point (GEP).  The finish_backwards
> function
> is updated, if the stopping point is between the normal entry point
> (LEP)
> and the end of the function, a breakpoint is set at the normal entry
> point.
> If the stopping point is between the entry points, a breakpoint is
> set to
> the alternate entry point.
> 
> The patch fixes the behavior of the reverse-finish command on PowerPC
> to
> match the behavior of the command on other platforms, specifically
> X86.
> The patch does not change the behavior of the command on X86.
> 
> A new test is added to verify the reverse-finish command on PowerPC
> correctly stops at the instruction where the function call is
> made.  A new
> test, finish-reverse-next.exp, is added to verify the correct
> functionality
> of the reverse-finish command.  The reverse-finish comand is expected
> to
> stop on the instruction that jumps to the function.  Procedure
> proc_step_until from test gdb.reverse/step-indirect-call-thunk.exp is
> moved
> to lib/gdb.exp and renamed repeat_cmd_until.
> 
> The patch fixes 11 regression errors in test gdb.reverse/finish-
> precsave.exp
> and 11 regression errors in test gdb.reverse/finish-reverse.exp.
> 
> The patch has been tested on Power 10 and X86 processor with no new
> regression failures.
> ---
>  gdb/infcmd.c                                  |  47 ++--
>  gdb/infrun.c                                  |  24 ++
>  .../gdb.reverse/finish-reverse-next.c         |  91 +++++++
>  .../gdb.reverse/finish-reverse-next.exp       | 224
> ++++++++++++++++++
>  .../gdb.reverse/step-indirect-call-thunk.exp  |  55 ++---
>  gdb/testsuite/lib/gdb.exp                     |  49 +++-
>  6 files changed, 421 insertions(+), 69 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.reverse/finish-reverse-next.c
>  create mode 100644 gdb/testsuite/gdb.reverse/finish-reverse-next.exp
> 
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 77206fcbfe8..0fa5719d38b 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1728,28 +1728,41 @@ finish_backward (struct finish_command_fsm
> *sm)
>       no way that a function up the stack can have a return address
>       that's equal to its entry point.  */
> 
> -  if (sal.pc != pc)
> -    {
> -      frame_info_ptr frame = get_selected_frame (nullptr);
> -      struct gdbarch *gdbarch = get_frame_arch (frame);
> +  CORE_ADDR alt_entry_point = sal.pc;
> +  CORE_ADDR entry_point = alt_entry_point;
> +  frame_info_ptr frame = get_selected_frame (nullptr);
> +  struct gdbarch *gdbarch = get_frame_arch (frame);
> +
> +  if (gdbarch_skip_entrypoint_p (gdbarch))
> +    /* Some architectures, like PowerPC use local and global entry
> points.
> +       There is only one Entry Point (GEP = LEP) for other
> architectures.
> +       The GEP is an alternate entry point.  The LEP is the normal
> entry point.
> +       The value of entry_point was initialized to the alternate
> entry point
> +       (GEP).  It will be adjusted to the normal entry point if the
> function
> +       has two entry points.  */
> +    entry_point = gdbarch_skip_entrypoint (gdbarch, sal.pc);
> 
> -      /* Set a step-resume at the function's entry point.  Once
> that's
> -	 hit, we'll do one more step backwards.  */
> +  if  ((pc < alt_entry_point) || (pc > entry_point))
> +    {
> +      /* We are in the body of the function.  Set a breakpoint to
> backup to
> +	 the normal entry point.  */
>        symtab_and_line sr_sal;
> -      sr_sal.pc = sal.pc;
> +      sr_sal.pc = entry_point;
>        sr_sal.pspace = get_frame_program_space (frame);
> -      insert_step_resume_breakpoint_at_sal (gdbarch,
> -					    sr_sal, null_frame_id);
> -
> -      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
> +      insert_step_resume_breakpoint_at_sal (gdbarch, sr_sal,
> +					    null_frame_id);
>      }
> +
>    else
> -    {
> -      /* We're almost there -- we just need to back up by one more
> -	 single-step.  */
> -      tp->control.step_range_start = tp->control.step_range_end = 1;
> -      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
> -    }
> +    /* We are either at one of the entry points or between the entry
> points.
> +       If we are not at the alt_entry point, go back to the
> alt_entry_point
> +       If we at the normal entry point step back one instruction,
> when we
> +       stop we will determine if we entered via the entry point or
> the
> +       alternate entry point.  If we are at the alternate entry
> point,
> +       single step back to the function call.  */
> +    tp->control.step_range_start = tp->control.step_range_end = 1;
> +
> +  proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
>  }
> 
>  /* finish_forward -- helper function for finish_command.  FRAME is
> the
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 87ab73c47a4..987dbd16ea4 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -1868,6 +1868,7 @@ struct execution_control_state
> 
>    struct target_waitstatus ws;
>    int stop_func_filled_in = 0;
> +  CORE_ADDR stop_func_alt_start = 0;
>    CORE_ADDR stop_func_start = 0;
>    CORE_ADDR stop_func_end = 0;
>    const char *stop_func_name = nullptr;
> @@ -4680,6 +4681,11 @@ fill_in_stop_func (struct gdbarch *gdbarch,
>  	  ecs->stop_func_start
>  	    += gdbarch_deprecated_function_start_offset (gdbarch);
> 
> +	  /* PowerPC functions have a Local Entry Point (LEP) and a
> Global
> +	     Entry Point (GEP).  There is only one Entry Point (GEP =
> LEP) for
> +	     other architectures.  */
> +	  ecs->stop_func_alt_start = ecs->stop_func_start;
> +
>  	  if (gdbarch_skip_entrypoint_p (gdbarch))
>  	    ecs->stop_func_start
>  	      = gdbarch_skip_entrypoint (gdbarch, ecs-
> >stop_func_start);
> @@ -7269,6 +7275,24 @@ process_event_stop_test (struct
> execution_control_state *ecs)
>  	}
>      }
> 
> +  if (execution_direction == EXEC_REVERSE
> +      && ecs->event_thread->control.proceed_to_finish
> +      && ecs->event_thread->stop_pc () >= ecs->stop_func_alt_start
> +      && ecs->event_thread->stop_pc () < ecs->stop_func_start)
> +    {
> +      /* We are executing the reverse-finish command.
> +	 If the system supports multiple entry points and we are
> finishing a
> +	 function in reverse.   If we are between the entry points
> singe-step
> +	 back to the alternate entry point.  If we are at the alternate
> entry
> +	 point -- just   need to back up by one more single-step, which
> +	 should take us back to the function call.  */
> +      ecs->event_thread->control.step_range_start
> +	= ecs->event_thread->control.step_range_end = 1;
> +      keep_going (ecs);
> +      return;
> +
> +    }
> +
>    if (ecs->event_thread->control.step_range_end == 1)
>      {
>        /* It is stepi or nexti.  We always want to stop stepping
> after
> diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-next.c
> b/gdb/testsuite/gdb.reverse/finish-reverse-next.c
> new file mode 100644
> index 00000000000..e95ee8e33a6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/finish-reverse-next.c
> @@ -0,0 +1,91 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2012-2023 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or
> modify
> +   it under the terms of the GNU General Public License as published
> by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <
> http://www.gnu.org/licenses/>.  */
> +
> +/* The reverse finish command should return from a function and stop
> on
> +   the first instruction of the source line where the function call
> is made.
> +   Specifically, the behavior should match doing a reverse next from
> the
> +   first instruction in the function.  GDB should only require one
> reverse
> +   step or next statement to reach the previous source code line.
> +
> +   This test verifies the fix for gdb bugzilla:
> +
> +   https://sourceware.org/bugzilla/show_bug.cgi?id=29927
> +
> +   PowerPC supports two entry points to a function.  The normal
> entry point
> +   is called the local entry point (LEP).  The alternate entry point
> is called
> +   the global entry point (GEP).  The GEP is only used if the table
> of
> +   contents (TOC) value stored in register r2 needs to be setup
> prior to
> +   execution starting at the LEP.  A function call via a function
> pointer
> +   will entry via the GEP.  A normal function call will enter via
> the LEP.
> +
> +   This test has been expanded to include tests to verify the
> reverse-finish
> +   command works properly if the function is called via the
> GEP.  The original
> +   test only verified the reverse-finish command for a normal call
> that used
> +   the LEP.  */
> +
> +int
> +function2 (int a, int b)
> +{
> +  int ret = 0;
> +  ret = ret + a + b;
> +  return ret;
> +}
> +
> +int
> +function1 (int a, int b)   // FUNCTION1
> +{
> +  int ret = 0;
> +  int (*funp) (int, int) = &function2;
> +  /* The assembly code for this function when compiled for PowerPC
> is as
> +     follows:
> +
> +     0000000010000758 <function1>:
> +     10000758:	02 10 40 3c 	lis     r2,4098        <- GEP
> +     1000075c:	00 7f 42 38 	addi    r2,r2,32512
> +     10000760:	a6 02 08 7c 	mflr    r0             <- LEP
> +     10000764:	10 00 01 f8 	std     r0,16(r1)
> +     ....
> +
> +     When the function is called on PowerPC with function1 (a, b)
> the call
> +     enters at the Local Entry Point (LEP).  When the function is
> called via
> +     a function pointer, the Global Entry Point (GEP) for function1
> is used.
> +     The GEP sets up register 2 before reaching the LEP.
> +  */
> +  ret = funp (a + 1, b + 2);
> +  return ret;
> +}
> +
> +int
> +main(int argc, char* argv[])
> +{
> +  int a, b;
> +  int (*funp) (int, int) = &function1;
> +
> +  /* Call function via Local Entry Point (LEP).  */
> +
> +  a = 1;
> +  b = 5;
> +
> +  function1 (a, b);   // CALL VIA LEP
> +
> +  /* Call function via Global Entry Point (GEP).  */
> +  a = 10;
> +  b = 50;
> +
> +  funp (a, b);        // CALL VIA GEP
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
> b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
> new file mode 100644
> index 00000000000..1f53b649a7d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
> @@ -0,0 +1,224 @@
> +# Copyright 2008-2023 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or
> modify
> +# it under the terms of the GNU General Public License as published
> by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <
> http://www.gnu.org/licenses/>.  */
> +
> +# This file is part of the GDB testsuite.  It tests reverse
> stepping.
> +# Lots of code borrowed from "step-test.exp".
> +
> +# The reverse finish command should return from a function and stop
> on
> +# the first instruction of the source line where the function call
> is made.
> +# Specifically, the behavior should match doing a reverse next from
> the
> +# first instruction in the function.  GDB should only take one
> reverse step
> +# or next statement to reach the previous source code line.
> +
> +# This testcase verifies the reverse-finish command stops at the
> first
> +# instruction in the source code line where the function was
> called.  There
> +# are two scenarios that must be checked:
> +#   1) gdb is at the entry point instruction for the function
> +#   2) gdb is in the body of the function.
> +
> +# This test verifies the fix for gdb bugzilla:
> +#   https://sourceware.org/bugzilla/show_bug.cgi?id=29927
> +
> +# PowerPC supports two entry points to a function.  The normal entry
> point
> +# is called the local entry point (LEP).  The alternate entry point
> is called
> +# the global entry point (GEP).  A function call via a function
> pointer
> +# will entry via the GEP.  A normal function call will enter via the
> LEP.
> +#
> +# This test has been expanded to include tests to verify the
> reverse-finish
> +# command works properly if the function is called via the GEP.  The
> original
> +# test only verified the reverse-finish command for a normal call
> that used
> +# the LEP.
> +
> +if ![supports_reverse] {
> +    return
> +}
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] }
> {
> +    return -1
> +}
> +
> +runto_main
> +set target_remote [gdb_is_target_remote]
> +
> +if [supports_process_record] {
> +    # Activate process record/replay.
> +    gdb_test_no_output "record" "turn on process record for test1"
> +}
> +
> +
> +### TEST 1: reverse finish from the entry point instruction (LEP) in
> +### function1 when called using the normal entry point (LEP).
> +
> +# Set breakpoint at call to function1 in main.
> +set bp_LEP_test [gdb_get_line_number "CALL VIA LEP" $srcfile]
> +gdb_breakpoint $srcfile:$bp_LEP_test temporary
> +
> +# Continue to break point at function1 call in main.
> +gdb_continue_to_breakpoint \
> +    "stopped at function1 entry point instruction to stepi into
> function" \
> +    ".*$srcfile:$bp_LEP_test\r\n.*"
> +
> +# stepi until we see "{" indicating we entered function1
> +repeat_cmd_until "stepi" "CALL VIA LEP" "{" "stepi into function1
> call" "100"
> +
> +# The reverse-finish command should stop on the function call
> instruction
> +# which is the last instruction in the source code line.  A reverse-
> next
> +# instruction should then stop at the first instruction in the same
> source
> +# code line.  Another revers-next instruction stops at the previous
> source
> +# code line.
> +gdb_test "reverse-finish" ".*function1 \\(a, b\\);   // CALL VIA
> LEP.*" \
> +    "reverse-finish function1 LEP call from LEP "
> +gdb_test "reverse-next" ".*function1 \\(a, b\\);   // CALL VIA LEP"
> \
> +    "reverse next 1 LEP entry point function call from LEP"
> +gdb_test "reverse-next" ".*b = 5;.*" "reverse next 2, at b = 5, call
> from LEP"
> +
> +
> +gdb_test "reverse-continue" ".*" "setup for test 2"
> +
> +# Turn off record to clear logs and turn on again
> +gdb_test "record stop"  "Process record is stopped.*" \
> +    "turn off process record for test1"
> +gdb_test_no_output "record" "turn on process record for test2"
> +
> +
> +### TEST 2: reverse finish from the body of function1.
> +
> +# Set breakpoint at call to function1 in main.
> +gdb_breakpoint $srcfile:$bp_LEP_test temporary
> +
> +# Continue to break point at function1 call in main.
> +gdb_continue_to_breakpoint \
> +    "at function1 entry point instruction to step to body of
> function" \
> +    ".*$srcfile:$bp_LEP_test\r\n.*"
> +
> +# do a step instruction to get to the body of the function
> +gdb_test "step" ".*int ret = 0;.*" "step test 1"
> +
> +# The reverse-finish command should stop on the function call
> instruction
> +# which is the last instruction in the source code line.  A reverse-
> next
> +# instruction should then stop at the first instruction in the same
> source
> +# code line.  Another revers-next instruction stops at the previous
> source
> +# code line.
> +gdb_test "reverse-finish" ".*function1 \\(a, b\\);   // CALL VIA
> LEP.*" \
> +    "reverse-finish function1 LEP call from function body"
> +gdb_test "reverse-next" ".*function1 \\(a, b\\);   // CALL VIA
> LEP.*" \
> +    "reverse next 1 LEP from function body"
> +gdb_test "reverse-next" ".*b = 5;.*" \
> +    "reverse next 2 at b = 5, from function body"
> +
> +gdb_test "reverse-continue" ".*" "setup for test 3"
> +
> +# Turn off record to clear logs and turn on again
> +gdb_test "record stop"  "Process record is stopped.*" \
> +    "turn off process record for test2"
> +gdb_test_no_output "record" "turn on process record for test3"
> +
> +
> +### TEST 3: reverse finish from the alternate entry point
> instruction (GEP) in
> +### function1 when called using the alternate entry point (GEP).
> +
> +# Set breakpoint at call to funp in main.
> +set bp_GEP_test  [gdb_get_line_number "CALL VIA GEP" $srcfile]
> +gdb_breakpoint $srcfile:$bp_GEP_test temporary
> +
> +# Continue to break point at funp call in main.
> +gdb_continue_to_breakpoint \
> +    "stopped at function1 entry point instruction to stepi into
> funp" \
> +    ".*$srcfile:$bp_GEP_test\r\n.*"
> +
> +# stepi until we see "{" indicating we entered function.
> +repeat_cmd_until "stepi" "CALL VIA GEP" "{" "stepi into funp call"
> +
> +# The reverse-finish command should stop on the function call
> instruction
> +# which is the last instruction in the source code line.  A reverse-
> next
> +# instruction should then stop at the first instruction in the same
> source
> +# code line.  Another revers-next instruction stops at the previous
> source
> +# code line.
> +gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
> +    "function1 GEP call call from GEP"
> +gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
> +    "reverse next 1 GEP entry point function call from GEP"
> +gdb_test "reverse-next" ".*b = 50;.*" "reverse next 2 at b = 50,
> call from GEP"
> +
> +gdb_test "reverse-continue" ".*" "setup for test 4"
> +
> +# Turn off record to clear logs and turn on again
> +gdb_test "record stop"  "Process record is stopped.*" \
> +    "turn off process record for test3"
> +gdb_test_no_output "record" "turn on process record for test4"
> +
> +### TEST 4: reverse finish from between the GEP and LEP in
> +### function1 when called using the alternate entry point (GEP).
> +
> +# Set breakpoint at call to funp in main.
> +set bp_GEP_test  [gdb_get_line_number "CALL VIA GEP" $srcfile]
> +gdb_breakpoint $srcfile:$bp_GEP_test temporary
> +
> +# Continue to break point at funp call in main.
> +gdb_continue_to_breakpoint \
> +    "stopped at function1 entry point instruction to stepi into funp
> again" \
> +    ".*$srcfile:$bp_GEP_test\r\n.*"
> +
> +# stepi until we see "{" indicating we entered function.
> +repeat_cmd_until "stepi" "CALL VIA GEP" "{" "stepi into funp call
> again"
> +
> +# do one more stepi so we are between the GEP and LEP.
> +gdb_test "stepi" "{" "stepi to between GEP and LEP"
> +
> +# The reverse-finish command should stop on the function call
> instruction
> +# which is the last instruction in the source code line.  A reverse-
> next
> +# instruction should then stop at the first instruction in the same
> source
> +# code line.  Another revers-next instruction stops at the previous
> source
> +# code line.
> +gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
> +    "function1 GEP call call from GEP again"
> +gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
> +    "reverse next 1 GEP entry point function call from GEP again"
> +gdb_test "reverse-next" ".*b = 50;.*" \
> +    "reverse next 2 at b = 50, call from GEP again"
> +
> +gdb_test "reverse-continue" ".*" "setup for test 5"
> +
> +# Turn off record to clear logs and turn on again
> +gdb_test "record stop"  "Process record is stopped.*" \
> +    "turn off process record for test4"
> +gdb_test_no_output "record" "turn on process record for test5"
> +
> +
> +### TEST 5: reverse finish from the body of function 1 when calling
> using the
> +### alternate entrypoint (GEP).
> +gdb_breakpoint $srcfile:$bp_GEP_test temporary
> +
> +# Continue to break point at funp call.
> +gdb_continue_to_breakpoint \
> +    "at function1 entry point instruction to step to body of funp
> call" \
> +    ".*$srcfile:$bp_GEP_test\r\n.*"
> +
> +# Step into body of funp, called via GEP.
> +gdb_test "step" ".*int ret = 0;.*" "step test 2"
> +
> +# The reverse-finish command should stop on the function call
> instruction
> +# which is the last instruction in the source code line.  A reverse-
> next
> +# instruction should then stop at the first instruction in the same
> source
> +# code line.  Another revers-next instruction stops at the previous
> source
> +# code line.
> +gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
> +    "reverse-finish function1 GEP call, from function body  "
> +gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
> +    "reverse next 1 GEP call from function body"
> +gdb_test "reverse-next" ".*b = 50;.*" \
> +    "reverse next 2 at b = 50 from function body"
> diff --git a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
> b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
> index 94292d5eb9b..61fb4974b8e 100644
> --- a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
> +++ b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
> @@ -17,6 +17,12 @@ require supports_reverse
> 
>  standard_testfile
> 
> +if { [istarget "powerpc*-*-linux*"] } {
> +    # GCC for PowerPC on linux does not support the -mindirect-
> branch and
> +    # -mfunction-return command line options.
> +    return 0
> +}
> +
>  set cflags {}
>  lappend cflags debug
>  lappend cflags additional_flags=-mindirect-branch=thunk
> @@ -36,39 +42,6 @@ if { ![runto_main] } {
>      return -1
>  }
> 
> -# Do repeated stepping COMMANDs in order to reach TARGET from
> CURRENT
> -#
> -#  COMMAND is a stepping command
> -#  CURRENT is a string matching the current location
> -#  TARGET  is a string matching the target location
> -#  TEST    is the test name
> -#
> -# The function issues repeated COMMANDs as long as the location
> matches
> -# CURRENT up to a maximum of 100 steps.
> -#
> -# TEST passes if the resulting location matches TARGET and fails
> -# otherwise.
> -#
> -proc step_until { command current target test } {
> -    global gdb_prompt
> -
> -    set count 0
> -    gdb_test_multiple "$command" "$test" {
> -        -re "$current.*$gdb_prompt $" {
> -            incr count
> -            if { $count < 100 } {
> -                send_gdb "$command\n"
> -                exp_continue
> -            } else {
> -                fail "$test"
> -            }
> -        }
> -        -re "$target.*$gdb_prompt $" {
> -            pass "$test"
> -        }
> -    }
> -}
> -
>  gdb_test_no_output "record"
>  gdb_test "next" ".*" "record trace"
> 
> @@ -88,20 +61,20 @@ gdb_test "reverse-next" "apply\.2.*" \
>      "reverse-step through thunks and over inc"
> 
>  # We can use instruction stepping to step into thunks.
> -step_until "stepi" "apply\.2" "indirect_thunk" "stepi into call
> thunk"
> -step_until "stepi" "indirect_thunk" "inc" \
> +repeat_cmd_until "stepi" "apply\.2" "indirect_thunk" "stepi into
> call thunk"
> +repeat_cmd_until "stepi" "indirect_thunk" "inc" \
>      "stepi out of call thunk into inc"
>  set alphanum_re "\[a-zA-Z0-9\]"
>  set pic_thunk_re  "__$alphanum_re*\\.get_pc_thunk\\.$alphanum_re*
> \\(\\)"
> -step_until "stepi" "(inc|$pic_thunk_re)" "return_thunk" "stepi into
> return thunk"
> -step_until "stepi" "return_thunk" "apply" \
> +repeat_cmd_until "stepi" "(inc|$pic_thunk_re)" "return_thunk" "stepi
> into return thunk"
> +repeat_cmd_until "stepi" "return_thunk" "apply" \
>      "stepi out of return thunk back into apply"
> 
> -step_until "reverse-stepi" "apply" "return_thunk" \
> +repeat_cmd_until "reverse-stepi" "apply" "return_thunk" \
>      "reverse-stepi into return thunk"
> -step_until "reverse-stepi" "return_thunk" "inc" \
> +repeat_cmd_until "reverse-stepi" "return_thunk" "inc" \
>      "reverse-stepi out of return thunk into inc"
> -step_until "reverse-stepi" "(inc|$pic_thunk_re)" "indirect_thunk" \
> +repeat_cmd_until "reverse-stepi" "(inc|$pic_thunk_re)"
> "indirect_thunk" \
>      "reverse-stepi into call thunk"
> -step_until "reverse-stepi" "indirect_thunk" "apply" \
> +repeat_cmd_until "reverse-stepi" "indirect_thunk" "apply" \
>      "reverse-stepi out of call thunk into apply"
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index faa0ac05a9a..b10555fe5fb 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -9267,31 +9267,58 @@ gdb_caching_proc arm_cc_for_target {
> 
>  # Step until the pattern REGEXP is found.  Step at most
>  # MAX_STEPS times, but stop stepping once REGEXP is found.
> -#
> +# START matches current location
>  # If REGEXP is found then a single pass is emitted, otherwise, after
>  # MAX_STEPS steps, a single fail is emitted.
>  #
>  # TEST_NAME is the name used in the pass/fail calls.
> 
> -proc gdb_step_until { regexp {test_name ""} {max_steps 10} } {
> +proc gdb_step_until { regexp {test_name ""} {current ""} \
> +			  { max_steps 10 } } {
> +    if { $current == "" } {
> +	set current "\}"
> +    }
> +    if { $test_name == "" } {
> +	set test_name "stepping until regexp"
> +    }
> +
> +    repeat_cmd_until "step" $current  $regexp  $test_name "10"
> +}
> +
> +# Do repeated stepping COMMANDs in order to reach TARGET from
> CURRENT
> +#
> +#  COMMAND is a stepping command
> +#  CURRENT is a string matching the current location
> +#  TARGET  is a string matching the target location
> +#  TEST    is the test name
> +#  MAX_STEPS is number of steps attempted before fail is emitted
> +#
> +# The function issues repeated COMMANDs as long as the location
> matches
> +# CURRENT up to a maximum of 100 steps.
> +#
> +# TEST passes if the resulting location matches TARGET and fails
> +# otherwise.
> +#
> +proc repeat_cmd_until { command current target test_name {max_steps
> 100} } {
> +    global gdb_prompt
>      if { $test_name == "" } {
>  	set test_name "stepping until regexp"
>      }
> 
>      set count 0
> -    gdb_test_multiple "step" "$test_name" {
> -	-re "$regexp\r\n$::gdb_prompt $" {
> -	    pass $test_name
> -	}
> -	-re ".*$::gdb_prompt $" {
> -	    if {$count < $max_steps} {
> -		incr count
> -		send_gdb "step\n"
> +    gdb_test_multiple "$command" "$test_name" {
> +	-re "$current.*$gdb_prompt $" {
> +	    incr count
> +	    if { $count < $max_steps } {
> +		send_gdb "$command\n"
>  		exp_continue
>  	    } else {
> -		fail $test_name
> +		fail "$test_name"
>  	    }
>  	}
> +	-re "$target.*$gdb_prompt $" {
> +	    pass "$test_name"
> +	}
>      }
>  }
>
  
Guinevere Larsen Feb. 28, 2023, 1:39 p.m. UTC | #6
On 20/02/2023 21:24, Carl Love wrote:
> Ulrich, Bruno, GDB maintainers:
>
> Per the comments from Ulrich, I have updated the patch to address the
> comments about the source code and the testcase. I updated the new
> library procedure so it can be called from gdb_step_until.  So
> gdb_step_until is now just a variant of the new repeat_cmd_until
> proceedure.
>
> The redundant if test has been removed.  It was added to fix a
> regression testing failure I saw on the gdb.btrace/tailcall.exp on X86.
> I went back and redid the testing, rebuilding everything from scratch.
> I am not able to reproduce the test failure without the redundant
> check.  Not sure why I initially saw the regression failure at this
> point?  Perhaps I hadn't re-enabled the
> /proc/sys/kernel/perf_event_paranoid value?  It must be 2 or lower for
> the test to run.  Otherwise the test fails.  Anyway, the regression
> tests on X86 run without the redundant check are passing on my laptop.
>
> I resolved the issues with the testing on X86 to make sure it isn't
> missing failures when testing gdb.reverse/step-indirect-call-thunk.exp.
> As mentioned in the previous email, the test does not run on PowerPC
> since the gcc command line options "-mindirect-branch=thunk" and "-
> mfunction-return=thunk" are not supported on PowerPC.  I disabled the
> test on PowerPC in the updated patch.
>
> Retested on X86 generation 5 and PowerPC with no regressions found.
>
> Hopefully this version of the patch is acceptable.
>
>                            Carl

Hi Carl,

Sorry about the delay. I've tested the series and see no regressions. In 
particular, the tests you called out all passed on my machine. I have a 
minor wording nit in one comment, but regardless, you can add to the tag:

Reviewed-By: Bruno Larsen <blarsen@redhat.com>

>    -----------------------------------------------------
>
> PowerPC: fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp
>
> PPC64 multiple entry points.  When executing in reverse the function
> finish_backward sets the break point at the alternate entry point, known
> as the global entry point (GEP) in PowerPC.  However if the forward
> execution enters via the normal entry point, known as the local entry
> point (LEP) on PowerPC, reverse execution never sees the break point at the
> alternate entry point of the function.  Reverse execution continues until
> the next break point is encountered thus stopping at the wrong place.
>
> This patch adds a new address to struct execution_control_state to hold the
> address of the alternate entry point (GEP).  The finish_backwards function
> is updated, if the stopping point is between the normal entry point (LEP)
> and the end of the function, a breakpoint is set at the normal entry point.
> If the stopping point is between the entry points, a breakpoint is set to
> the alternate entry point.
>
> The patch fixes the behavior of the reverse-finish command on PowerPC to
> match the behavior of the command on other platforms, specifically X86.
> The patch does not change the behavior of the command on X86.
>
> A new test is added to verify the reverse-finish command on PowerPC
> correctly stops at the instruction where the function call is made.  A new
> test, finish-reverse-next.exp, is added to verify the correct functionality
> of the reverse-finish command.  The reverse-finish comand is expected to
> stop on the instruction that jumps to the function.  Procedure
> proc_step_until from test gdb.reverse/step-indirect-call-thunk.exp is moved
> to lib/gdb.exp and renamed repeat_cmd_until.
>
> The patch fixes 11 regression errors in test gdb.reverse/finish-precsave.exp
> and 11 regression errors in test gdb.reverse/finish-reverse.exp.
>
> The patch has been tested on Power 10 and X86 processor with no new
> regression failures.
> ---
>   gdb/infcmd.c                                  |  47 ++--
>   gdb/infrun.c                                  |  24 ++
>   .../gdb.reverse/finish-reverse-next.c         |  91 +++++++
>   .../gdb.reverse/finish-reverse-next.exp       | 224 ++++++++++++++++++
>   .../gdb.reverse/step-indirect-call-thunk.exp  |  55 ++---
>   gdb/testsuite/lib/gdb.exp                     |  49 +++-
>   6 files changed, 421 insertions(+), 69 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.reverse/finish-reverse-next.c
>   create mode 100644 gdb/testsuite/gdb.reverse/finish-reverse-next.exp
>
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 77206fcbfe8..0fa5719d38b 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1728,28 +1728,41 @@ finish_backward (struct finish_command_fsm *sm)
>        no way that a function up the stack can have a return address
>        that's equal to its entry point.  */
>   
> -  if (sal.pc != pc)
> -    {
> -      frame_info_ptr frame = get_selected_frame (nullptr);
> -      struct gdbarch *gdbarch = get_frame_arch (frame);
> +  CORE_ADDR alt_entry_point = sal.pc;
> +  CORE_ADDR entry_point = alt_entry_point;
> +  frame_info_ptr frame = get_selected_frame (nullptr);
> +  struct gdbarch *gdbarch = get_frame_arch (frame);
> +
> +  if (gdbarch_skip_entrypoint_p (gdbarch))
> +    /* Some architectures, like PowerPC use local and global entry points.
> +       There is only one Entry Point (GEP = LEP) for other architectures.
> +       The GEP is an alternate entry point.  The LEP is the normal entry point.
> +       The value of entry_point was initialized to the alternate entry point
> +       (GEP).  It will be adjusted to the normal entry point if the function
> +       has two entry points.  */
> +    entry_point = gdbarch_skip_entrypoint (gdbarch, sal.pc);
>   
> -      /* Set a step-resume at the function's entry point.  Once that's
> -	 hit, we'll do one more step backwards.  */
> +  if  ((pc < alt_entry_point) || (pc > entry_point))
> +    {
> +      /* We are in the body of the function.  Set a breakpoint to backup to

I would change backup for "return", or separate it to "back up", 
otherwise I personally think of data backups.
  
Carl Love Feb. 28, 2023, 4:19 p.m. UTC | #7
Bruno:

Thanks for testing and reviewing the patch

On Tue, 2023-02-28 at 14:39 +0100, Bruno Larsen wrote:
> Hi Carl,
> 
> Sorry about the delay. I've tested the series and see no regressions.
> In 
> particular, the tests you called out all passed on my machine. I have
> a 
> minor wording nit in one comment, but regardless, you can add to the
> tag:
> 
> Reviewed-By: Bruno Larsen <blarsen@redhat.com>
> 
> >     -----------------------------------------------------
> > 
> > 
<snip>
> > -      /* Set a step-resume at the function's entry point.  Once
> > that's
> > -      hit, we'll do one more step backwards.  */
> > +  if  ((pc < alt_entry_point) || (pc > entry_point))
> > +    {
> > +      /* We are in the body of the function.  Set a breakpoint to
> > backup to
> 
> I would change backup for "return", or separate it to "back up", 
> otherwise I personally think of data backups.


OK, I changed the line as follows in my copy of the patch

    entry_point = gdbarch_skip_entrypoint (gdbarch, sal.pc);
 
   if  ((pc < alt_entry_point) || (pc > entry_point))
     {
-      /* We are in the body of the function.  Set a breakpoint to backup to
+      /* We are in the body of the function.  Set a breakpoint to go back to
 	 the normal entry point.  */
       symtab_and_line sr_sal;

I think that addresses your concern.  It is not worth resending the
patch at this point for this change but will include it in any future
posts or commits.  Thanks for the feedback.

                             Carl
  
Tom de Vries March 1, 2023, 1:43 p.m. UTC | #8
On 2/20/23 21:24, Carl Love wrote:
> +if { [istarget "powerpc*-*-linux*"] } {
> +    # GCC for PowerPC on linux does not support the -mindirect-branch and
> +    # -mfunction-return command line options.
> +    return 0
> +}
> +

This bit is no longer necessary.

I've committed 2ef339e38f5 ("[gdb/testsuite] Require istarget x86* in 
gdb.reverse/step-indirect-call-thunk.exp") which copied your fix from 
commit 43127ae5714 ("Fix gdb.base/step-indirect-call-thunk.exp").

Thanks,
- Tom
  
Tom de Vries March 1, 2023, 2:03 p.m. UTC | #9
On 2/20/23 21:24, Carl Love wrote:
>   # Step until the pattern REGEXP is found.  Step at most
>   # MAX_STEPS times, but stop stepping once REGEXP is found.
> -#
> +# START matches current location
>   # If REGEXP is found then a single pass is emitted, otherwise, after
>   # MAX_STEPS steps, a single fail is emitted.
>   #
>   # TEST_NAME is the name used in the pass/fail calls.
>   
> -proc gdb_step_until { regexp {test_name ""} {max_steps 10} } {
> +proc gdb_step_until { regexp {test_name ""} {current ""} \
> +			  { max_steps 10 } } {

I assume you meant CURRENT instead of START.

Thanks,
- Tom
  
Tom de Vries March 1, 2023, 2:34 p.m. UTC | #10
On 2/20/23 21:24, Carl Love wrote:
>   .../gdb.reverse/step-indirect-call-thunk.exp  |  55 ++---
>   gdb/testsuite/lib/gdb.exp                     |  49 +++-

I think the changes in these two files are not really related to 
$subject.  The only connection is that the newly introduced test-case 
gdb.reverse/finish-reverse-next.exp uses repeat_cmd_until.

I propose to commit attached patch separately.

Carl, can you review the commit message?

Thanks,
- Tom
  
Carl Love March 1, 2023, 4:26 p.m. UTC | #11
Tom:

On Wed, 2023-03-01 at 14:43 +0100, Tom de Vries wrote:
> On 2/20/23 21:24, Carl Love wrote:
> > +if { [istarget "powerpc*-*-linux*"] } {
> > +    # GCC for PowerPC on linux does not support the -mindirect-
> > branch and
> > +    # -mfunction-return command line options.
> > +    return 0
> > +}
> > +
> 
> This bit is no longer necessary.
> 
> I've committed 2ef339e38f5 ("[gdb/testsuite] Require istarget x86*
> in 
> gdb.reverse/step-indirect-call-thunk.exp") which copied your fix
> from 
> commit 43127ae5714 ("Fix gdb.base/step-indirect-call-thunk.exp").

I update my source tree, without my patch applied, I verified the test
is not running on PowerPC.  I will drop the above change from my patch.

Thanks.

                  Carl
  
Carl Love March 1, 2023, 4:43 p.m. UTC | #12
Tom:

On Wed, 2023-03-01 at 15:03 +0100, Tom de Vries wrote:
> On 2/20/23 21:24, Carl Love wrote:
> >   # Step until the pattern REGEXP is found.  Step at most
> >   # MAX_STEPS times, but stop stepping once REGEXP is found.
> > -#
> > +# START matches current location
> >   # If REGEXP is found then a single pass is emitted, otherwise,
> > after
> >   # MAX_STEPS steps, a single fail is emitted.
> >   #
> >   # TEST_NAME is the name used in the pass/fail calls.
> >   
> > -proc gdb_step_until { regexp {test_name ""} {max_steps 10} } {
> > +proc gdb_step_until { regexp {test_name ""} {current ""} \
> > +			  { max_steps 10 } } {
> 
> I assume you meant CURRENT instead of START.


Yes, thanks for catching that.  Fixed.

                  Carl
  
Carl Love March 1, 2023, 8:39 p.m. UTC | #13
Tom:

On Wed, 2023-03-01 at 15:34 +0100, Tom de Vries wrote:
> On 2/20/23 21:24, Carl Love wrote:
> >   .../gdb.reverse/step-indirect-call-thunk.exp  |  55 ++---
> >   gdb/testsuite/lib/gdb.exp                     |  49 +++-
> 
> I think the changes in these two files are not really related to 
> $subject.  The only connection is that the newly introduced test-
> case 
> gdb.reverse/finish-reverse-next.exp uses repeat_cmd_until.
> 
> I propose to commit attached patch separately.

OK, I split the patch.  The first patch does the changes to the above
two files.  The second patch is the PowerPC fix that uses the new
library proceedure.

> 
> Carl, can you review the commit message?\

I reworked the commit message to make it flow better and remove the
comments about the movement of the proceedure.

I will post the two patch series.

                            Carl
  

Patch

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 77206fcbfe8..a65cc700fc9 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1728,28 +1728,41 @@  finish_backward (struct finish_command_fsm *sm)
      no way that a function up the stack can have a return address
      that's equal to its entry point.  */
 
-  if (sal.pc != pc)
-    {
-      frame_info_ptr frame = get_selected_frame (nullptr);
-      struct gdbarch *gdbarch = get_frame_arch (frame);
+  CORE_ADDR alt_entry_point = sal.pc;
+  CORE_ADDR entry_point = alt_entry_point;
+  frame_info_ptr frame = get_selected_frame (nullptr);
+  struct gdbarch *gdbarch = get_frame_arch (frame);
 
-      /* Set a step-resume at the function's entry point.  Once that's
-	 hit, we'll do one more step backwards.  */
-      symtab_and_line sr_sal;
-      sr_sal.pc = sal.pc;
-      sr_sal.pspace = get_frame_program_space (frame);
-      insert_step_resume_breakpoint_at_sal (gdbarch,
-					    sr_sal, null_frame_id);
+  if (gdbarch_skip_entrypoint_p (gdbarch))
+    /* Some architectures, like PowerPC use local and global entry points.
+       There is only one Entry Point (GEP = LEP) for other architectures.
+       The GEP is an alternate entry point.  The LEP is the normal entry point.
+       The value of entry_point was initialized to the alternate entry point
+       (GEP).  It will be adjusted to the normal entry point if the function
+       has two entry points.  */
+    entry_point = gdbarch_skip_entrypoint (gdbarch, sal.pc);
+
+  if  ((pc >= alt_entry_point) && (pc <= entry_point))
+    /* We are either at one of the entry points or between the entry points.
+       If we are not at the alt_entry point, go back to the alt_entry_point
+       If we at the normal entry point step back one instruction, when we
+       stop we will determine if we entered via the entry point or the
+       alternate entry point.  If we are at the alternate entry point,
+       single step back to the function call.  */
+    tp->control.step_range_start = tp->control.step_range_end = 1;
 
-      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
-    }
   else
     {
-      /* We're almost there -- we just need to back up by one more
-	 single-step.  */
-      tp->control.step_range_start = tp->control.step_range_end = 1;
-      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
+      /* We are in the body of the function.  Set a breakpoint to backup to
+	 the normal entry point.  */
+      symtab_and_line sr_sal;
+      sr_sal.pc = entry_point;
+      sr_sal.pspace = get_frame_program_space (frame);
+      insert_step_resume_breakpoint_at_sal (gdbarch, sr_sal,
+					    null_frame_id);
     }
+
+  proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
 }
 
 /* finish_forward -- helper function for finish_command.  FRAME is the
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 87ab73c47a4..f0bd13fc219 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1868,6 +1868,7 @@  struct execution_control_state
 
   struct target_waitstatus ws;
   int stop_func_filled_in = 0;
+  CORE_ADDR stop_func_alt_start = 0;
   CORE_ADDR stop_func_start = 0;
   CORE_ADDR stop_func_end = 0;
   const char *stop_func_name = nullptr;
@@ -4680,6 +4681,11 @@  fill_in_stop_func (struct gdbarch *gdbarch,
 	  ecs->stop_func_start
 	    += gdbarch_deprecated_function_start_offset (gdbarch);
 
+	  /* PowerPC functions have a Local Entry Point (LEP) and a Global
+	     Entry Point (GEP).  There is only one Entry Point (GEP = LEP) for
+	     other architectures.  */
+	  ecs->stop_func_alt_start = ecs->stop_func_start;
+
 	  if (gdbarch_skip_entrypoint_p (gdbarch))
 	    ecs->stop_func_start
 	      = gdbarch_skip_entrypoint (gdbarch, ecs->stop_func_start);
@@ -7269,6 +7275,26 @@  process_event_stop_test (struct execution_control_state *ecs)
 	}
     }
 
+  if (execution_direction == EXEC_REVERSE
+      && ecs->event_thread->control.proceed_to_finish)
+    {
+      /* We are executing the reverse-finish command.  */
+      if (ecs->event_thread->stop_pc () >= ecs->stop_func_alt_start
+	  && ecs->event_thread->stop_pc () < ecs->stop_func_start
+	  && ecs->stop_func_alt_start != ecs->stop_func_start)
+	/* If the system supports multiple entry points and we are finishing a
+	   function in reverse.   If we are between the entry points singe-step
+	   back to the alternate entry point.  If we are at the alternate entry
+	   point -- just   need to back up by one more single-step, which
+	   should take us back to the function call.  */
+	{
+	  ecs->event_thread->control.step_range_start
+	    = ecs->event_thread->control.step_range_end = 1;
+	  keep_going (ecs);
+	  return;
+	}
+    }
+
   if (ecs->event_thread->control.step_range_end == 1)
     {
       /* It is stepi or nexti.  We always want to stop stepping after
diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-next.c b/gdb/testsuite/gdb.reverse/finish-reverse-next.c
new file mode 100644
index 00000000000..e95ee8e33a6
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/finish-reverse-next.c
@@ -0,0 +1,91 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012-2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* The reverse finish command should return from a function and stop on
+   the first instruction of the source line where the function call is made.
+   Specifically, the behavior should match doing a reverse next from the
+   first instruction in the function.  GDB should only require one reverse
+   step or next statement to reach the previous source code line.
+
+   This test verifies the fix for gdb bugzilla:
+
+   https://sourceware.org/bugzilla/show_bug.cgi?id=29927
+
+   PowerPC supports two entry points to a function.  The normal entry point
+   is called the local entry point (LEP).  The alternate entry point is called
+   the global entry point (GEP).  The GEP is only used if the table of
+   contents (TOC) value stored in register r2 needs to be setup prior to
+   execution starting at the LEP.  A function call via a function pointer
+   will entry via the GEP.  A normal function call will enter via the LEP.
+
+   This test has been expanded to include tests to verify the reverse-finish
+   command works properly if the function is called via the GEP.  The original
+   test only verified the reverse-finish command for a normal call that used
+   the LEP.  */
+
+int
+function2 (int a, int b)
+{
+  int ret = 0;
+  ret = ret + a + b;
+  return ret;
+}
+
+int
+function1 (int a, int b)   // FUNCTION1
+{
+  int ret = 0;
+  int (*funp) (int, int) = &function2;
+  /* The assembly code for this function when compiled for PowerPC is as
+     follows:
+
+     0000000010000758 <function1>:
+     10000758:	02 10 40 3c 	lis     r2,4098        <- GEP
+     1000075c:	00 7f 42 38 	addi    r2,r2,32512
+     10000760:	a6 02 08 7c 	mflr    r0             <- LEP
+     10000764:	10 00 01 f8 	std     r0,16(r1)
+     ....
+
+     When the function is called on PowerPC with function1 (a, b) the call
+     enters at the Local Entry Point (LEP).  When the function is called via
+     a function pointer, the Global Entry Point (GEP) for function1 is used.
+     The GEP sets up register 2 before reaching the LEP.
+  */
+  ret = funp (a + 1, b + 2);
+  return ret;
+}
+
+int
+main(int argc, char* argv[])
+{
+  int a, b;
+  int (*funp) (int, int) = &function1;
+
+  /* Call function via Local Entry Point (LEP).  */
+
+  a = 1;
+  b = 5;
+
+  function1 (a, b);   // CALL VIA LEP
+
+  /* Call function via Global Entry Point (GEP).  */
+  a = 10;
+  b = 50;
+
+  funp (a, b);        // CALL VIA GEP
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-next.exp b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
new file mode 100644
index 00000000000..6e08530c970
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp
@@ -0,0 +1,224 @@ 
+# Copyright 2008-2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# This file is part of the GDB testsuite.  It tests reverse stepping.
+# Lots of code borrowed from "step-test.exp".
+
+# The reverse finish command should return from a function and stop on
+# the first instruction of the source line where the function call is made.
+# Specifically, the behavior should match doing a reverse next from the
+# first instruction in the function.  GDB should only take one reverse step
+# or next statement to reach the previous source code line.
+
+# This testcase verifies the reverse-finish command stops at the first
+# instruction in the source code line where the function was called.  There
+# are two scenarios that must be checked:
+#   1) gdb is at the entry point instruction for the function
+#   2) gdb is in the body of the function.
+
+# This test verifies the fix for gdb bugzilla:
+#   https://sourceware.org/bugzilla/show_bug.cgi?id=29927
+
+# PowerPC supports two entry points to a function.  The normal entry point
+# is called the local entry point (LEP).  The alternate entry point is called
+# the global entry point (GEP).  A function call via a function pointer
+# will entry via the GEP.  A normal function call will enter via the LEP.
+#
+# This test has been expanded to include tests to verify the reverse-finish
+# command works properly if the function is called via the GEP.  The original
+# test only verified the reverse-finish command for a normal call that used
+# the LEP.
+
+if ![supports_reverse] {
+    return
+}
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+runto_main
+set target_remote [gdb_is_target_remote]
+
+if [supports_process_record] {
+    # Activate process record/replay.
+    gdb_test_no_output "record" "turn on process record for test1"
+}
+
+
+### TEST 1: reverse finish from the entry point instruction (LEP) in
+### function1 when called using the normal entry point (LEP).
+
+# Set breakpoint at call to function1 in main.
+set bp_LEP_test [gdb_get_line_number "CALL VIA LEP" $srcfile]
+gdb_breakpoint $srcfile:$bp_LEP_test temporary
+
+# Continue to break point at function1 call in main.
+gdb_continue_to_breakpoint \
+    "stopped at function1 entry point instruction to stepi into function" \
+    ".*$srcfile:$bp_LEP_test\r\n.*"
+
+# stepi until we see "{" indicating we entered function1
+repeat_cmd_until "stepi" "CALL VIA LEP" "{" "stepi into function1 call"
+
+# The reverse-finish command should stop on the function call instruction
+# which is the last instruction in the source code line.  A reverse-next
+# instruction should then stop at the first instruction in the same source
+# code line.  Another revers-next instruction stops at the previous source
+# code line.
+gdb_test "reverse-finish" ".*function1 \\(a, b\\);   // CALL VIA LEP.*" \
+    "reverse-finish function1 LEP call from LEP "
+gdb_test "reverse-next" ".*function1 \\(a, b\\);   // CALL VIA LEP" \
+    "reverse next 1 LEP entry point function call from LEP"
+gdb_test "reverse-next" ".*b = 5;.*" "reverse next 2, at b = 5, call from LEP"
+
+
+gdb_test "reverse-continue" ".*" "setup for test 2"
+
+# Turn off record to clear logs and turn on again
+gdb_test "record stop"  "Process record is stopped.*" \
+    "turn off process record for test1"
+gdb_test_no_output "record" "turn on process record for test2"
+
+
+### TEST 2: reverse finish from the body of function1.
+
+# Set breakpoint at call to function1 in main.
+gdb_breakpoint $srcfile:$bp_LEP_test temporary
+
+# Continue to break point at function1 call in main.
+gdb_continue_to_breakpoint \
+    "at function1 entry point instruction to step to body of function" \
+    ".*$srcfile:$bp_LEP_test\r\n.*"
+
+# do a step instruction to get to the body of the function
+gdb_test "step" ".*int ret = 0;.*" "step test 1"
+
+# The reverse-finish command should stop on the function call instruction
+# which is the last instruction in the source code line.  A reverse-next
+# instruction should then stop at the first instruction in the same source
+# code line.  Another revers-next instruction stops at the previous source
+# code line.
+gdb_test "reverse-finish" ".*function1 \\(a, b\\);   // CALL VIA LEP.*" \
+    "reverse-finish function1 LEP call from function body"
+gdb_test "reverse-next" ".*function1 \\(a, b\\);   // CALL VIA LEP.*" \
+    "reverse next 1 LEP from function body"
+gdb_test "reverse-next" ".*b = 5;.*" \
+    "reverse next 2 at b = 5, from function body"
+
+gdb_test "reverse-continue" ".*" "setup for test 3"
+
+# Turn off record to clear logs and turn on again
+gdb_test "record stop"  "Process record is stopped.*" \
+    "turn off process record for test2"
+gdb_test_no_output "record" "turn on process record for test3"
+
+
+### TEST 3: reverse finish from the alternate entry point instruction (GEP) in
+### function1 when called using the alternate entry point (GEP).
+
+# Set breakpoint at call to funp in main.
+set bp_GEP_test  [gdb_get_line_number "CALL VIA GEP" $srcfile]
+gdb_breakpoint $srcfile:$bp_GEP_test temporary
+
+# Continue to break point at funp call in main.
+gdb_continue_to_breakpoint \
+    "stopped at function1 entry point instruction to stepi into funp" \
+    ".*$srcfile:$bp_GEP_test\r\n.*"
+
+# stepi until we see "{" indicating we entered function.
+repeat_cmd_until "stepi" "CALL VIA GEP" "{" "stepi into funp call"
+
+# The reverse-finish command should stop on the function call instruction
+# which is the last instruction in the source code line.  A reverse-next
+# instruction should then stop at the first instruction in the same source
+# code line.  Another revers-next instruction stops at the previous source
+# code line.
+gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
+    "function1 GEP call call from GEP"
+gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
+    "reverse next 1 GEP entry point function call from GEP"
+gdb_test "reverse-next" ".*b = 50;.*" "reverse next 2 at b = 50, call from GEP"
+
+gdb_test "reverse-continue" ".*" "setup for test 4"
+
+# Turn off record to clear logs and turn on again
+gdb_test "record stop"  "Process record is stopped.*" \
+    "turn off process record for test3"
+gdb_test_no_output "record" "turn on process record for test4"
+
+### TEST 4: reverse finish from between the GEP and LEP in
+### function1 when called using the alternate entry point (GEP).
+
+# Set breakpoint at call to funp in main.
+set bp_GEP_test  [gdb_get_line_number "CALL VIA GEP" $srcfile]
+gdb_breakpoint $srcfile:$bp_GEP_test temporary
+
+# Continue to break point at funp call in main.
+gdb_continue_to_breakpoint \
+    "stopped at function1 entry point instruction to stepi into funp again" \
+    ".*$srcfile:$bp_GEP_test\r\n.*"
+
+# stepi until we see "{" indicating we entered function.
+repeat_cmd_until "stepi" "CALL VIA GEP" "{" "stepi into funp call again"
+
+# do one more stepi so we are between the GEP and LEP.
+gdb_test "stepi" "{" "stepi to between GEP and LEP"
+
+# The reverse-finish command should stop on the function call instruction
+# which is the last instruction in the source code line.  A reverse-next
+# instruction should then stop at the first instruction in the same source
+# code line.  Another revers-next instruction stops at the previous source
+# code line.
+gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
+    "function1 GEP call call from GEP again"
+gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
+    "reverse next 1 GEP entry point function call from GEP again"
+gdb_test "reverse-next" ".*b = 50;.*" \
+    "reverse next 2 at b = 50, call from GEP again"
+
+gdb_test "reverse-continue" ".*" "setup for test 5"
+
+# Turn off record to clear logs and turn on again
+gdb_test "record stop"  "Process record is stopped.*" \
+    "turn off process record for test4"
+gdb_test_no_output "record" "turn on process record for test5"
+
+
+### TEST 5: reverse finish from the body of function 1 when calling using the
+### alternate entrypoint (GEP).
+gdb_breakpoint $srcfile:$bp_GEP_test temporary
+
+# Continue to break point at funp call.
+gdb_continue_to_breakpoint \
+    "at function1 entry point instruction to step to body of funp call" \
+    ".*$srcfile:$bp_GEP_test\r\n.*"
+
+# Step into body of funp, called via GEP.
+gdb_test "step" ".*int ret = 0;.*" "step test 2"
+
+# The reverse-finish command should stop on the function call instruction
+# which is the last instruction in the source code line.  A reverse-next
+# instruction should then stop at the first instruction in the same source
+# code line.  Another revers-next instruction stops at the previous source
+# code line.
+gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \
+    "reverse-finish function1 GEP call, from function body  "
+gdb_test "reverse-next" ".*funp \\(a, b\\);.*" \
+    "reverse next 1 GEP call from function body"
+gdb_test "reverse-next" ".*b = 50;.*" \
+    "reverse next 2 at b = 50 from function body"
diff --git a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
index 94292d5eb9b..dc5cf097511 100644
--- a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
+++ b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
@@ -36,39 +36,6 @@  if { ![runto_main] } {
     return -1
 }
 
-# Do repeated stepping COMMANDs in order to reach TARGET from CURRENT
-#
-#  COMMAND is a stepping command
-#  CURRENT is a string matching the current location
-#  TARGET  is a string matching the target location
-#  TEST    is the test name
-#
-# The function issues repeated COMMANDs as long as the location matches
-# CURRENT up to a maximum of 100 steps.
-#
-# TEST passes if the resulting location matches TARGET and fails
-# otherwise.
-#
-proc step_until { command current target test } {
-    global gdb_prompt
-
-    set count 0
-    gdb_test_multiple "$command" "$test" {
-        -re "$current.*$gdb_prompt $" {
-            incr count
-            if { $count < 100 } {
-                send_gdb "$command\n"
-                exp_continue
-            } else {
-                fail "$test"
-            }
-        }
-        -re "$target.*$gdb_prompt $" {
-            pass "$test"
-        }
-    }
-}
-
 gdb_test_no_output "record"
 gdb_test "next" ".*" "record trace"
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index faa0ac05a9a..c156982a674 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -9295,6 +9295,39 @@  proc gdb_step_until { regexp {test_name ""} {max_steps 10} } {
     }
 }
 
+# Do repeated stepping COMMANDs in order to reach TARGET from CURRENT
+#
+#  COMMAND is a stepping command
+#  CURRENT is a string matching the current location
+#  TARGET  is a string matching the target location
+#  TEST    is the test name
+#
+# The function issues repeated COMMANDs as long as the location matches
+# CURRENT up to a maximum of 100 steps.
+#
+# TEST passes if the resulting location matches TARGET and fails
+# otherwise.
+#
+proc repeat_cmd_until { command current target test } {
+    global gdb_prompt
+
+    set count 0
+    gdb_test_multiple "$command" "$test" {
+	-re "$current.*$gdb_prompt $" {
+	    incr count
+	    if { $count < 100 } {
+		send_gdb "$command\n"
+		exp_continue
+	    } else {
+		fail "$test"
+	    }
+	}
+	-re "$target.*$gdb_prompt $" {
+	    pass "$test"
+	}
+    }
+}
+
 # Check if the compiler emits epilogue information associated
 # with the closing brace or with the last statement line.
 #