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

Carl Love via Gdb-patches
2023-01-20T20:05:20Z

On Thu, 2023-01-19 at 15:57 -0800, Carl Love wrote:
> Bruno, Tom:
>
>
> On Thu, 2023-01-19 at 08:56 -0800, Carl Love wrote:
>
<snip>
>
> Without the patch, when you were in the main program at "answer += 1"
> and did a reverse-next, gdb would go back thru the call to function
> bar
> and thru the call to function foo stopping in main. That doesn't
> seem
> right to me as you have stepped back thru function foo and bar.
>
> With the patch the reverse-next only step back thru function bar and
> stops in function foo at the call to bar, i.e. doesn't continue to
> step
> back thru function bar.
>
> The following is the tailcall source code with comments showing where
> the reverse-step and reverse-next stop with and without the X86 patch
> applied. Hopefully the code below is a little easier to follow.
> Initially, the reverse-step and reverse-next tests are executed from
> the line "answer += 1;" in function main.
>
>
> static __attribute__ ((noinline)) int
> bar (void)
> {
> return 42;
> } <- reverse-step stops here (with no patch
> and with X86 patch)
>
> static __attribute__ ((noinline)) int
> foo (void)
> {
> return bar (); <- reverse-next stops here (patch X86 applied)
> }
>
> int
> main (void)
> {
> int answer;
>
> answer = foo (); <- reverse-next tops here (no patch applied,
> stepped
> back thru both the bar and foo functions)
> answer += 1; <- GDB is here, now issue the reverse-step
> and reverse-next commands
>
> return answer;
> }
>
> As a result of the change in the reverse-next instruction, the
> expected
> test results for the tailcall test need to be updated to reflect the
> fact that gdb now stops in function foo not main.
>
> --- a/gdb/testsuite/gdb.btrace/tailcall.exp
> +++ b/gdb/testsuite/gdb.btrace/tailcall.exp
> @@ -102,9 +102,9 @@ gdb_test "reverse-step" "\[^\r\n\]*main \\(\\) at
> tailcall.c
> :37\r\n.*" \
> "reverse-step.2"
> gdb_test "next" "\[^\r\n\]*38.*" \
> "next.1"
> -gdb_test "reverse-next" "\[^\r\n\]*main \\(\\) at
> tailcall.c:37\r\n.*" \
> +gdb_test "reverse-next" "\[^\r\n\]*foo \\(\\) at
> tailcall.c:29\r\n.*" \
> "reverse-next.1"
> -gdb_test "step" "\[^\r\n\]*foo \\(\\) at tailcall.c:29\r\n.*" \
> +gdb_test "step" "\[^\r\n\]*bar \\(\\) at tailcall.c:24\r\n.*" \
> "step.1"
> gdb_test "finish" "\[^\r\n\]*main \\(\\) at tailcall.c:38\r\n.*" \
> "finish.2"
>
> With this change, the tailcall.exp test now passes on my X86 laptop.
> The PowerPC do not change since the test is not supported on PowerPC.
>
> I will post an updated version of the X86 patch with the fixes to the
> tailcall test. It will be version 3. There are no changes to the
> PowerPC patch.
>
> The gdb.btrace/rn-dl-bind.exp test passes with and without my
> patches.
> I still can not get this test to fail on my system.

I have been thinking about this and have decided the above change is
not what we should do. Basically if we forward step at answer = foo();
we stop at answer += 1;. Which is the expected behavior for next. So,
is we are at answer += 1; and do a reverse step it should take us to
answer = foo(). It would be the opposite of the forward next.

I think I have identified the single line in the function call
set_step_info() which caused the regression in gdb.btrace/tailcall.exp.
I have so far run a few experiments and it looks like not doing the
statement:

tp->control.step_stack_frame_id = get_stack_frame_id (frame);

eliminates the regression in tailcall.exp.

So far. I need to do some more testing but it looks like there is a
couple of regressions on my X86 laptop. It looks like the expected
behavior in both of these tests is for the reverse step over a function
stops at a function call within the function not stepping all the way
back to the call of the function. Basically, the expected behavior of
these reverse tests is not consistent across tests.

I am not seeing any PowerPC regression failures with the above change.
My IBM x86 machine tests are still running.

Anyway, just wanted to give people a heads up that I am pursuing
another fix and that I now think the above fix is not the best
direction to go.

Carl