From: Bruno Larsen <blarsen@redhat.com>
To: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: [PINGv5] [PATCH] gdb/reverse: Fix stepping over recursive functions
Date: Wed, 6 Jul 2022 08:47:16 -0300 [thread overview]
Message-ID: <5e9b3cdd-bb87-8e99-ef79-748ad01f0029@redhat.com> (raw)
In-Reply-To: <85d7b4e2-761e-aa32-b0a5-319b682666f3@redhat.com>
ping!
Cheers!
Bruno Larsen
On 6/29/22 15:46, Bruno Larsen wrote:
> 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"
next prev parent reply other threads:[~2022-07-06 11:47 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-25 18:02 Bruno Larsen
2022-06-08 11:17 ` [PING] " Bruno Larsen
2022-06-15 19:21 ` [PINGv2] " Bruno Larsen
2022-06-22 18:49 ` [PINGv3] " Bruno Larsen
2022-06-29 18:46 ` [PINGv4] " Bruno Larsen
2022-07-06 11:47 ` Bruno Larsen [this message]
2022-07-07 18:48 ` Pedro Alves
2022-07-08 17:16 ` Bruno Larsen
2022-07-22 15:31 ` Bruno Larsen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5e9b3cdd-bb87-8e99-ef79-748ad01f0029@redhat.com \
--to=blarsen@redhat.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).