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 04DAD3858C53 for ; Mon, 23 May 2022 20:03:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 04DAD3858C53 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-533-j365oU2oPx6NPtUGqPHiFA-1; Mon, 23 May 2022 16:03:43 -0400 X-MC-Unique: j365oU2oPx6NPtUGqPHiFA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id EF39429ABA35 for ; Mon, 23 May 2022 20:03:42 +0000 (UTC) Received: from [10.97.116.35] (ovpn-116-35.gru2.redhat.com [10.97.116.35]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 861D1112131B for ; Mon, 23 May 2022 20:03:42 +0000 (UTC) Message-ID: Date: Mon, 23 May 2022 17:03:40 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH] gdb/reverse: Fix stepping over recursive functions To: "gdb-patches@sourceware.org" References: <20220523134653.20330-1-blarsen@redhat.com> From: Bruno Larsen In-Reply-To: <20220523134653.20330-1-blarsen@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 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: 7bit X-Spam-Status: No, score=-14.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, 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: Mon, 23 May 2022 20:03:48 -0000 Please disregard. This patch was sent without proper testing, and failed to fix the whole problem. A better fix should be coming later this week. Sorry for the noise :( Cheers! Bruno Larsen On 5/23/22 10:46, 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. > --- > gdb/infrun.c | 22 ++++++++++++++++++ > gdb/testsuite/gdb.reverse/step-reverse.c | 13 +++++++++++ > gdb/testsuite/gdb.reverse/step-reverse.exp | 26 +++++++++++++++++++++- > 3 files changed, 60 insertions(+), 1 deletion(-) > > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 02c98b50c8c..ee42f7dfa34 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -6233,6 +6233,28 @@ handle_signal_stop (struct execution_control_state *ecs) > infrun_debug_printf ("[%s] hit its single-step breakpoint", > ecs->ptid.to_string ().c_str ()); > } > + > + /* 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)) > + { > + /* 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; > + } > + } > } > delete_just_stopped_threads_single_step_breakpoints (); > > diff --git a/gdb/testsuite/gdb.reverse/step-reverse.c b/gdb/testsuite/gdb.reverse/step-reverse.c > index aea2a98541d..958d4b297b0 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 (){ > + if (myglob == 0) return 0; > + myglob /= 2; > + if (myglob>1) > + myglob++; > + return recursive_callee (); /* REVERSE NEXT FAIL */ > +} > + > /* A structure which, we hope, will need to be passed using memcpy. */ > struct rhomboidal { > int rather_large[100]; > @@ -61,6 +72,8 @@ int main () { > a[5] = a[3] - a[4]; /* FINISH TEST */ > callee(); /* STEPI TEST */ > > + recursive_callee (); /* NEXT OVER THIS RECURSION */ > + > /* 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..4238675431e 100644 > --- a/gdb/testsuite/gdb.reverse/step-reverse.exp > +++ b/gdb/testsuite/gdb.reverse/step-reverse.exp > @@ -118,7 +118,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 "NEXT OVER THIS RECURSION.*" { > pass "$test_message" > } > -re "ARRIVED IN CALLEE.*$gdb_prompt $" { > @@ -138,6 +138,9 @@ gdb_test_multiple "stepi" "$test_message" { > } > } > > +# next through a recursive function call > +gdb_test "next" "NEXTI TEST.*" "next over recursion" > + > ### > ### > ### > @@ -145,6 +148,27 @@ gdb_test_multiple "stepi" "$test_message" { > # Set reverse execution direction > > gdb_test_no_output "set exec-dir reverse" "set reverse execution" > +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 ".*REVERSE NEXT FAIL.*" { > + 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 "next\n" > + exp_continue > + } > + } > +} > > # stepi backward thru return and into a function >