[PINGv4] [PATCH] gdb/reverse: Fix stepping over recursive functions

[Bruno Larsen <blarsen@redhat.com>]

ping!

Cheers!
Bruno Larsen

On 6/22/22 15:49, Bruno Larsen wrote:
> ping!
> 
> Cheers!
> Bruno Larsen
> 
> On 6/15/22 16:21, Bruno Larsen wrote:
>> ping!
>>
>> Cheers!
>> Bruno Larsen
>>
>> On 6/8/22 08:17, Bruno Larsen wrote:
>>> Ping!
>>>
>>> Cheers!
>>> Bruno Larsen
>>>
>>> On 5/25/22 15:02, Bruno Larsen wrote:
>>>> Currently, when using GDB to do reverse debugging, if we try to use the
>>>> command "reverse next" to skip a recursive function, instead of skipping
>>>> all of the recursive calls and stopping in the previous line, we stop at
>>>> the second to last recursive call, and need to manually step backwards
>>>> until we leave the first call.  This is well documented in PR gdb/16678.
>>>>
>>>> This bug happens because when GDB notices that a reverse step has
>>>> entered into a function, GDB will add a step_resume_breakpoint at the
>>>> start of the function, then single step out of the prologue once that
>>>> breakpoint is hit.  Recursion poses a problem because that breakpoint will
>>>> be hit many times before GDB should actually stop the inferior.  To fix
>>>> this issue, when the step_resume_breakpoint is hit (and GDB is executing
>>>> backwards), we analyze if the caller of the frame is the original frame
>>>> where we started the "reverse next", and if it is, GDB will stop the
>>>> inferior, otherwise GDB will just ignore the breakpoint.
>>>>
>>>> This commit also changes gdb.reverse/step-reverse.exp to contain a
>>>> recursive function and attempt to both, skip it altogether, and to skip
>>>> the second call from inside the first call, as this setup broke a
>>>> previous version of the patch.
>>>> ---
>>>>   gdb/infrun.c                               | 24 +++++++++
>>>>   gdb/testsuite/gdb.reverse/step-reverse.c   | 16 +++++-
>>>>   gdb/testsuite/gdb.reverse/step-reverse.exp | 59 ++++++++++++++++++++--
>>>>   3 files changed, 95 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>>>> index 02c98b50c8c..95fb3227aa3 100644
>>>> --- a/gdb/infrun.c
>>>> +++ b/gdb/infrun.c
>>>> @@ -6761,6 +6761,30 @@ process_event_stop_test (struct execution_control_state *ecs)
>>>>       case BPSTAT_WHAT_STEP_RESUME:
>>>>         infrun_debug_printf ("BPSTAT_WHAT_STEP_RESUME");
>>>> +      /* If we are executing backwards, we are doing a "next" and the
>>>> +     current frame is not the same as where we started, the
>>>> +     step_resume_breakpoint we have just hit has been added to the start
>>>> +     of a function so we can skip the whle function.  However, if we are
>>>> +     skipping a recursive call, we only want to act as if we hit the
>>>> +     breakpoint only when the caller of the current frame is the original
>>>> +     frame we were single stepping from.  */
>>>> +      if (execution_direction == EXEC_REVERSE
>>>> +      && ecs->event_thread->control.step_over_calls == STEP_OVER_ALL
>>>> +      && !frame_id_eq (get_stack_frame_id (get_current_frame ()),
>>>> +               ecs->event_thread->control.step_stack_frame_id))
>>>> +    {
>>>> +      infrun_debug_printf ("We are not done with recursing yet");
>>>> +      /* If the caller's ID is not the same as the starting frame, we
>>>> +         can ignore this breakpoint.  */
>>>> +      if (!frame_id_eq (frame_unwind_caller_id (get_current_frame ()),
>>>> +                ecs->event_thread->control.step_stack_frame_id))
>>>> +        {
>>>> +          keep_going (ecs);
>>>> +          return;
>>>> +        }
>>>> +    } else
>>>> +        infrun_debug_printf ("are we not recursing?");
>>>> +
>>>>         delete_step_resume_breakpoint (ecs->event_thread);
>>>>         if (ecs->event_thread->control.proceed_to_finish
>>>>         && execution_direction == EXEC_REVERSE)
>>>> diff --git a/gdb/testsuite/gdb.reverse/step-reverse.c b/gdb/testsuite/gdb.reverse/step-reverse.c
>>>> index aea2a98541d..a390ac2580c 100644
>>>> --- a/gdb/testsuite/gdb.reverse/step-reverse.c
>>>> +++ b/gdb/testsuite/gdb.reverse/step-reverse.c
>>>> @@ -26,6 +26,17 @@ int callee() {        /* ENTER CALLEE */
>>>>     return myglob++;    /* ARRIVED IN CALLEE */
>>>>   }            /* RETURN FROM CALLEE */
>>>> +/* We need to make this function take more than a single instruction
>>>> +   to run, otherwise it could hide PR gdb/16678, as reverse execution can
>>>> +   step over a single-instruction function.  */
>>>> +int recursive_callee (int val){
>>>> +    if (val == 0) return 0;
>>>> +    val /= 2;
>>>> +    if (val>1)
>>>> +    val++;
>>>> +    return recursive_callee (val);    /* RECURSIVE CALL */
>>>> +} /* EXIT RECURSIVE FUNCTION */
>>>> +
>>>>   /* A structure which, we hope, will need to be passed using memcpy.  */
>>>>   struct rhomboidal {
>>>>     int rather_large[100];
>>>> @@ -51,6 +62,9 @@ int main () {
>>>>      y = y + 4;
>>>>      z = z + 5;    /* STEP TEST 2 */
>>>> +   /* Test that next goes over recursive calls too */
>>>> +   recursive_callee (32); /* NEXT OVER THIS RECURSION */
>>>> +
>>>>      /* Test that "next" goes over a call */
>>>>      callee();    /* NEXT OVER THIS CALL */
>>>> @@ -60,7 +74,7 @@ int main () {
>>>>      /* Test "stepi" */
>>>>      a[5] = a[3] - a[4]; /* FINISH TEST */
>>>>      callee();    /* STEPI TEST */
>>>> -
>>>> +
>>>>      /* Test "nexti" */
>>>>      callee();    /* NEXTI TEST */
>>>> diff --git a/gdb/testsuite/gdb.reverse/step-reverse.exp b/gdb/testsuite/gdb.reverse/step-reverse.exp
>>>> index 997b62604d5..4f56b4785ca 100644
>>>> --- a/gdb/testsuite/gdb.reverse/step-reverse.exp
>>>> +++ b/gdb/testsuite/gdb.reverse/step-reverse.exp
>>>> @@ -47,9 +47,11 @@ gdb_test "step" ".*STEP TEST 1.*" "step test 1"
>>>>   gdb_test "next 2" ".*NEXT TEST 2.*" "next test 2"
>>>>   gdb_test "step 3" ".*STEP TEST 2.*" "step test 2"
>>>> +# next through a recursive function call
>>>> +gdb_test "next 2" "NEXT OVER THIS CALL.*" "next over recursion"
>>>> +
>>>>   # step over call
>>>> -gdb_test "step" ".*NEXT OVER THIS CALL.*" "step up to call"
>>>>   gdb_test "next" ".*STEP INTO THIS CALL.*" "next over call"
>>>>   # step into call
>>>> @@ -118,7 +120,7 @@ gdb_test_multiple "stepi" "$test_message" {
>>>>   set test_message "stepi back from function call"
>>>>   gdb_test_multiple "stepi" "$test_message" {
>>>> -    -re "NEXTI TEST.*$gdb_prompt $" {
>>>> +    -re -wrap "NEXTI TEST.*" {
>>>>       pass "$test_message"
>>>>       }
>>>>       -re "ARRIVED IN CALLEE.*$gdb_prompt $" {
>>>> @@ -143,7 +145,6 @@ gdb_test_multiple "stepi" "$test_message" {
>>>>   ###
>>>>   # Set reverse execution direction
>>>> -
>>>>   gdb_test_no_output "set exec-dir reverse" "set reverse execution"
>>>>   # stepi backward thru return and into a function
>>>> @@ -247,6 +248,58 @@ gdb_test_multiple "step" "$test_message" {
>>>>   gdb_test "next" ".*NEXT OVER THIS CALL.*" "reverse next over call"
>>>> +# Dont reverse the execution direction yet, as we will need another
>>>> +# forward step after this test
>>>> +
>>>> +set step_out 0
>>>> +gdb_test_multiple "next" "reverse next over recursion" {
>>>> +    -re -wrap ".*NEXT OVER THIS RECURSION.*" {
>>>> +    pass "reverse next over recursion"
>>>> +    }
>>>> +    -re -wrap ".*RECURSIVE CALL.*" {
>>>> +    fail "reverse next over recursion"
>>>> +    set step_out 1
>>>> +    }
>>>> +}
>>>> +if { "$step_out" == 1 } {
>>>> +    gdb_test_multiple "next" "stepping out of recursion" {
>>>> +    -re -wrap "NEXT OVER THIS RECURSION.*" {
>>>> +        set step_out 0
>>>> +    }
>>>> +    -re -wrap ".*" {
>>>> +        send_gdb "reverse-next\n"
>>>> +        exp_continue
>>>> +    }
>>>> +    }
>>>> +}
>>>> +
>>>> +# Step forward over recursion again so we can test stepping over calls
>>>> +# inside the recursion itself.
>>>> +gdb_test_no_output "set exec-dir forward" "forward again to test recursion"
>>>> +gdb_test "next" "NEXT OVER THIS CALL.*" "reverse next over recursion again"
>>>> +gdb_test_no_output "set exec-dir reverse" "reverse again to test recursion"
>>>> +
>>>> +gdb_test "step" ".*EXIT RECURSIVE FUNCTION.*" "enter recursive function"
>>>> +set step_pass 1
>>>> +gdb_test_multiple "next" "step over recursion inside the recursion" {
>>>> +    -re -wrap ".*EXIT RECURSIVE FUNCTION.*" {
>>>> +    set step_pass 0
>>>> +    send_gdb "next\n"
>>>> +    exp_continue
>>>> +    }
>>>> +    -re -wrap ".*NEXT OVER THIS RECURSION.*" {
>>>> +    if {$step_pass} {
>>>> +        pass "step over recursion inside the recursion"
>>>> +    } else {
>>>> +        fail "step over recursion inside the recursion"
>>>> +    }
>>>> +    }
>>>> +    -re -wrap ".*" {
>>>> +    send_gdb "next\n"
>>>> +    exp_continue
>>>> +    }
>>>> +}
>>>> +
>>>>   # step/next backward with count
>>>>   gdb_test "step 3" ".*REVERSE STEP TEST 1.*" "reverse step test 1"