From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 9B9C03858404 for ; Wed, 6 Jul 2022 11:47:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9B9C03858404 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-517-R79CPnoaM1yXToEPjpnyWw-1; Wed, 06 Jul 2022 07:47:19 -0400 X-MC-Unique: R79CPnoaM1yXToEPjpnyWw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id B9787811E87 for ; Wed, 6 Jul 2022 11:47:18 +0000 (UTC) Received: from [10.97.116.44] (ovpn-116-44.gru2.redhat.com [10.97.116.44]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5178340CF8E7 for ; Wed, 6 Jul 2022 11:47:18 +0000 (UTC) Message-ID: <5e9b3cdd-bb87-8e99-ef79-748ad01f0029@redhat.com> Date: Wed, 6 Jul 2022 08:47:16 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: [PINGv5] [PATCH] gdb/reverse: Fix stepping over recursive functions From: Bruno Larsen To: "gdb-patches@sourceware.org" References: <20220525180247.29731-1-blarsen@redhat.com> <94a25d4e-5ca0-9411-8941-2c3be1d3fa4c@redhat.com> <85d7b4e2-761e-aa32-b0a5-319b682666f3@redhat.com> In-Reply-To: <85d7b4e2-761e-aa32-b0a5-319b682666f3@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jul 2022 11:47:22 -0000 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"