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

Bruno Larsen via Gdb-patches
2023-01-17T12:36:19Z

On 16/01/2023 17:37, Carl Love wrote:
> GDB maintainers:
>
> Version 2: Addressed various comments from Bruno Larsen.
>
> This patch fixes the issues with the reverse-finish command on X86.
> The reverse-finish command now correctly stops at the first instruction
> in the source code line of the caller. It now only requires a single
> reverse-step or reverse-next instruction to get back to the previous
> source code line.
>
> It also adds a new testcase, gdb.reverse/finish-reverse-next.exp, and
> updates several existing testcases.
>
> Version 2 patch changes have been re-verified on PowerPC and X86 with
> no regressions.
>
> Please let me know if you have any comments on the patch. Thanks.
>
> Carl
>
> --------------------------------------------------------
> X86: reverse-finish fix
>
> PR record/29927 - reverse-finish requires two reverse next instructions to
> reach previous source line
>
> Currently on X86, when executing the finish command in reverse, gdb does a
> single step from the first instruction in the callee to get back to the
> caller. GDB stops on the last instruction in the source code line where
> the call was made. When stopped at the last instruction of the source code
> line, a reverse next or step command will stop at the first instruction
> of the same source code line thus requiring two step/next commands to
> reach the previous source code line. It should only require one step/next
> command to reach the previous source code line.
>
> By contrast, a reverse next or step command from the first line in a
> function stops at the first instruction in the source code line where the
> call was made.
>
> This patch fixes the reverse finish command so it will stop at the first
> instruction of the source line where the function call was made. The
> behavior on X86 for the reverse-finish command now matches doing a
> reverse-next from the beginning of the function.
>
> The proceed_to_finish flag in struct thread_control_state is no longer
> used. This patch removes the declaration, initialization and setting of
> the flag.
>
> This patch requires a number of regression tests to be updated. Test
> gdb.mi/mi-reverse.exp no longer needs to execute two steps to get to the
> previous line. The gdb output for tests gdb.reverse/until-precsave.exp
> and gdb.reverse/until-reverse.exp changed slightly. The expected result in
> tests gdb.reverse/amd64-failcall-reverse.exp and
> gdb.reverse/singlejmp-reverse.exp are updated to the correct expected
> result.
>
> This patch adds a new test gdb.reverse/finish-reverse-next.exp to test the
> reverse-finish command when returning from the entry point and from the
> body of the function.
>
> The step_until proceedure in test gdb.reverse/step-indirect-call-thunk.exp
> was moved to lib/gdb.exp and renamed cmd_until.
>
> The patch has been tested on X86 and PowerPC to verify no additional
> regression failures occured.
>
> Bug:https://sourceware.org/bugzilla/show_bug.cgi?id=29927
> ---

Changes look good now and I also dont see any regressions.

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

--
Cheers,
Bruno