[PING] Re: [PATCH 2/2 ] PowerPC: fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp

Carl Love via Gdb-patches
2023-03-08T16:19:46Z

GCC developers:

Ping. Just wondering if someone could review this patch for
me. Thanks.

Carl

On Wed, 2023-03-01 at 12:59 -0800, Carl Love wrote:
> Tom, Ulrich, Bruno, Pedro, GDB maintainers:
>
> This patch fixes the reverse-finish command on PowerPC. The command
> now works the same as on other architectures, specifically
> X86. There
> are no functional changes for other architectures. The patch
> includes
> a new testcase to verify the reverse-finish command works correctly
> with the multiple entry points supported by PowerPC.
>
>
> Patch tested on PowerPC and 5th generation X86 with no regression
> failures.
>
> Carl
>
> --------------------------------------------------------
> PowerPC: fix for gdb.reverse/finish-precsave.exp and
> gdb.reverse/finish-reverse.exp
>
> PPC64 multiple entry points, a normal entry point and an alternate
> entry
> point. The alternate entry point is to setup the Table of Contents
> (TOC)
> register before continuing at the normal entry point. When the TOC
> is
> already valid, the normal entry point is used, this is typically the
> case.
> The alternate entry point is typically referred to as the global
> entry
> point (GEP) in IBM. The normal entry point is typically referred to
> as
> the local entry point (LEP).
>
> When GDB is executing the finish command in reverse, the function
> finish_backward currently sets the break point at the alternate entry
> point.
> This issue is if the function, when executing in the forward
> direction, entered
> the function via the normal entry point, execution in the reverse
> direction
> will never sees the break point at the alternate entry point. In
> this case,
> the 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 at
> the alternate entry point. This ensures that GDB will always stop at
> the
> normal entry point. If the function did enter via the alternate
> entry point,
> GDB will detect that and continue to execute backwards in the
> function until
> the alternate entry point is reached.
>
> 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.
>
> 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
> ++++++++++++++++++
> 4 files changed, 369 insertions(+), 17 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 c369b795757..81c617448af 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 go
> back 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 ab77300f1ff..ca2fc02898a 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -1938,6 +1938,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;
> @@ -4822,6 +4823,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);
> @@ -7411,6 +7417,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"