From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id pi+/JW0z82WtFAcAWB0awg (envelope-from ) for ; Thu, 14 Mar 2024 13:27:09 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=ZqvtYPO1; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 8541A1E0BB; Thu, 14 Mar 2024 13:27:09 -0400 (EDT) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 668121E01D for ; Thu, 14 Mar 2024 13:27:07 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id F0F69385E036 for ; Thu, 14 Mar 2024 17:27:06 +0000 (GMT) Received: from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com [IPv6:2a00:1450:4864:20::12e]) by sourceware.org (Postfix) with ESMTPS id 2DF4D3858C60 for ; Thu, 14 Mar 2024 17:26:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2DF4D3858C60 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2DF4D3858C60 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::12e ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710437206; cv=none; b=miG7SvHr64Q42ZzO/wTfYEwCLmZ7c0NpKaw1+faP8279fvJZaCMGTTaoaKYoERZKBao12wJpeuFhT0p4o012C/D4JcVU46dnvnW0DemWobzowyzvG8HZmAwGspYKtN2oeaia5sTlOqzppImGG+CRVoZ43sh/vXFc/O7jqMoH51k= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710437206; c=relaxed/simple; bh=NPUgEdFAe6empQIHbGYg3w4wKY0eaS+Ho/DJ80mR6OA=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=dTQWBBIFEc5QjqkD3LH+K8CVHxJJctZT9oCEFYCRKwS/fnBci6HR0b+T8XkRTmz4xCEA64IXi3YVAvrRbbMLHkdMHDXFeMpOWAZtSgcslj98MXbU1JkRELwSXFN8kglKufecCVR1WpwaqN6a2RVydgGWcrNlJ6eYVmOnjGRzC9U= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x12e.google.com with SMTP id 2adb3069b0e04-513a6416058so1881905e87.1 for ; Thu, 14 Mar 2024 10:26:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710437199; x=1711041999; darn=sourceware.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=CB62a9CBE6MbX38zGw4lintZouu54Z1/i5mqb1/xxBU=; b=ZqvtYPO1HP3qnNCfXWjjt7aKsJifE7Vkh93py/koLUvp+60mDlbDcDZ8VoyGcMqqxW vw8abbcO99TpW5JA4NrBm78UdQCVSc2zmA+tukBAK1yQmpOkOPcydJcPzJdn7YLYpYR7 whas8ICxCBkCkIthMIHOk0Y/IuUJ5FqkQV9br7pMil5BUTBeyquqgOcuKP+36NVlp5ic 9i/k9Qr2d9zKa3OFpgxsGIfeD6wB0MUx5MvTl/aaWTq/Jyt6ze1AoxbtwTT7n/nO5316 L0cjpBs5OY2FBybyQh/0A2S//hb48XlDCLPsO19hADxN7qO30UVbTubo67ktGUCo/3ab Q4wg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710437199; x=1711041999; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=CB62a9CBE6MbX38zGw4lintZouu54Z1/i5mqb1/xxBU=; b=WD3SNRKkLEVCCzTa5n/N91fVeehlKEF8vwya/SDQHsG/IrkUfu+FoTUBr6OlCI9aZw Tdh3rfXc+7fbP+LTuiKuMXczN0xPyvvZnBR0ab+yA+mIWSG0nnhu8JfJNaNZvHQYiUah 9Xt4ESVBKNRckRuvS+gaPNl3U4HWokxo1+WdOTbd9Dk+5BfGuSb/EcBe7Ga+zYh95lxa koO7ifZJzDDW0RjQv0UMGapm+GFOQzeZmKPHqy22WN1njBJIJq6NtCuCbTz1fMGCA3Dl 1kPRk0jwUlqiDvkLnktyW0ltgHmXJt3XtvWYuq8vkacjEM6OdRfBbTFrzQnWByHA0Nft rHHw== X-Forwarded-Encrypted: i=1; AJvYcCWuipBwavZx/P+Fg3UuKfytcBslISqhgycaj+Tonhxq1kqyUjtOjz1XxFavL+lPudwuUAvnBA2UFFgV6DB/vF617ScrSDDk+0xG7g== X-Gm-Message-State: AOJu0YyluEMwO8etF4WzmzkHiGLa0vBg56DbuZFgjqiNaAAe3TZlnySA e+JGQCHTHL5NrSmzZv2ZoWaEqbJop3AteF83Vk9brhuFRq2M9WIGQxIldIyVoACJ2avq/slZbwE kyuBqUcXxpmwUcT7Lzgz4m84BwDA= X-Google-Smtp-Source: AGHT+IF6pn2QjRUHcRVVcyBVC8iTfamCngFfMJiH3Y+rz+1MHhH9i2vWrokq+vZndDhV+l2D1SlJzqMO0n7xdbOc2l8= X-Received: by 2002:a19:2d43:0:b0:512:f5e7:4ce9 with SMTP id t3-20020a192d43000000b00512f5e74ce9mr1539952lft.18.1710437199298; Thu, 14 Mar 2024 10:26:39 -0700 (PDT) MIME-Version: 1.0 References: <20240313204830.2521708-1-achronop@gmail.com> <50e7e7f5-024b-4ac8-be5c-948ad2f41e73@redhat.com> In-Reply-To: <50e7e7f5-024b-4ac8-be5c-948ad2f41e73@redhat.com> From: Alex Chronopoulos Date: Thu, 14 Mar 2024 18:26:27 +0100 Message-ID: Subject: Re: [PATCH] Change message when reaching end of reverse history. To: Guinevere Larsen Cc: "Metzger, Markus T" , "Eli Zaretskii (eliz@gnu.org)" , "gdb-patches@sourceware.org" Content-Type: multipart/alternative; boundary="000000000000acc79e0613a2314a" X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org --000000000000acc79e0613a2314a Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable > I think "reached beginning of ~" makes sense, but that wouldn't fix the > confusion about whether GDB can continue executing (and recording) on > its own. > > On the other hand, I worry just saying "switching to recording mode" > might seem too arbitrary for users, if they don't know the context of > the commit message. That's correct. The current issue in the forward case is that execution can continue. GDB will halt at the beginning of the reverse history because emulation cannot proceed. However, execution can resume in the 'normal' mode, and recording can also continue. I was attempting to inform the user that a different type of execution would be possible to continue debugging. Backward execution does not encounter the same issue because reaching the end of history signifies the end of the reverse execution. I am unsure how to convey all these points with fewer words unless we opt for less specificity. Please suggest something in the preferred direction. Thank you in advance, Alex On Thu, Mar 14, 2024 at 11:26=E2=80=AFAM Guinevere Larsen wrote: > On 14/03/2024 07:37, Metzger, Markus T wrote: > > Hello Alex, > > > > I like the idea, but I find the wording a bit verbose. Can we find > something shorter? > > E.g. "reached beginning of reverse-execution history" vs "reached end o= f > ~". > > Not sure if that's clear enough that we're recording again. We could > also omit > > the "reached end of ~" part and just say "switching to recording mode". > > I think "reached beginning of ~" makes sense, but that wouldn't fix the > confusion about whether GDB can continue executing (and recording) on > its own. > > On the other hand, I worry just saying "switching to recording mode" > might seem too arbitrary for users, if they don't know the context of > the commit message. > > I wonder if good documentation can offset the second issue, though... > > > > >> if (uiout->is_mi_like_p ()) > >> uiout->field_string ("reason", async_reason_lookup > >> (EXEC_ASYNC_NO_HISTORY)); > >> + else if (execution_direction =3D=3D EXEC_FORWARD) > >> + uiout->text ("\nNo more reverse-execution history for emulation. = " > >> + "Going forward will continue executing and recording.\n"= ); > >> else > >> - uiout->text ("\nNo more reverse-execution history.\n"); > >> + { > >> + gdb_assert (execution_direction =3D=3D EXEC_REVERSE); > >> + uiout->text ("\nNo more reverse-execution history.\n"); > >> + } > > We should probably give MI a similar indication, although that would > likely > > break existing MI consumers. Not sure how opt-in is handled in MI. > > I don't know either, but relatively easy change would be to add a new > field to the mi response to show direction when recording is enabled, so > mi consumers could decide how to handle a no-history stop. > > Or we could announce the change on GDB 15 release and hold off on > merging this after 15 branched? Not sure if this is the right process, > though. > > I had a suggestion after FOSDEM to have GDB indicate the direction of > execution for every command when recording is enabled, and adding that > to MI would be a step in that direction (again, leaving it up to MI > consumers on how to report it to users). what do you think? > > > > > regards, > > Markus. > > > >> -----Original Message----- > >> From: Alex Chronopoulos > >> Sent: Wednesday, March 13, 2024 9:49 PM > >> To: gdb-patches@sourceware.org > >> Cc: Alex Chronopoulos > >> Subject: [PATCH] Change message when reaching end of reverse history. > >> > >> In a record session, when we move backward, GDB switches from normal > >> execution to simulation. Moving forward again, the emulation continues > >> until the end of the reverse history. When the end is reached, the > >> execution stops, and a warning message is shown. This message has been > >> modified to indicate that the forward emulation has reached the end, b= ut > >> the execution can continue as normal, and the recording will also > continue. > >> > >> Before this patch, the warning message shown in that case was the same > as > >> in the reverse case. This meant that when the end of history was > reached in > >> either backward or forward emulation, the same message was displayed: > >> > >> "No more reverse-execution history." > >> > >> This message remains for backward emulation. However, in forward > >> emulation, > >> it has been modified to: > >> > >> "No more reverse-execution history for emulation. Going forward will > >> continue executing and recording." > >> > >> The reason for this change is that the initial message was deceiving, > >> making the user believe that forward debugging could not continue. > >> > >> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D31224 > >> --- > >> gdb/NEWS | 5 ++++ > >> gdb/infrun.c | 8 ++++- > >> gdb/testsuite/gdb.btrace/non-stop.exp | 30 ++++++++++++------- > >> gdb/testsuite/gdb.reverse/break-precsave.exp | 4 +-- > >> gdb/testsuite/gdb.reverse/break-reverse.exp | 2 +- > >> .../gdb.reverse/machinestate-precsave.exp | 2 +- > >> 6 files changed, 36 insertions(+), 15 deletions(-) > >> > >> diff --git a/gdb/NEWS b/gdb/NEWS > >> index 2638b3e0d9c..f2e85776a53 100644 > >> --- a/gdb/NEWS > >> +++ b/gdb/NEWS > >> @@ -13,6 +13,11 @@ > >> the background, resulting in faster startup. This can be controlle= d > >> using "maint set dwarf synchronous". > >> > >> +* In a record session, when a forward emulation reaches the end of th= e > >> reverse > >> + history, the warning message has been changed to indicate that the > end of > >> the > >> + history has been reached. It also specifies that the forward > execution can > >> + continue, and the recording will also continue. > >> + > >> * Changed commands > >> > >> disassemble > >> diff --git a/gdb/infrun.c b/gdb/infrun.c > >> index bbb98f6dcdb..e129eb0582f 100644 > >> --- a/gdb/infrun.c > >> +++ b/gdb/infrun.c > >> @@ -9244,8 +9244,14 @@ print_no_history_reason (struct ui_out *uiout) > >> { > >> if (uiout->is_mi_like_p ()) > >> uiout->field_string ("reason", async_reason_lookup > >> (EXEC_ASYNC_NO_HISTORY)); > >> + else if (execution_direction =3D=3D EXEC_FORWARD) > >> + uiout->text ("\nNo more reverse-execution history for emulation. = " > >> + "Going forward will continue executing and recording.\n"= ); > >> else > >> - uiout->text ("\nNo more reverse-execution history.\n"); > >> + { > >> + gdb_assert (execution_direction =3D=3D EXEC_REVERSE); > >> + uiout->text ("\nNo more reverse-execution history.\n"); > >> + } > >> } > >> > >> /* Print current location without a level number, if we have changed > >> diff --git a/gdb/testsuite/gdb.btrace/non-stop.exp > >> b/gdb/testsuite/gdb.btrace/non-stop.exp > >> index 62c940e4cd6..7ce3008c120 100644 > >> --- a/gdb/testsuite/gdb.btrace/non-stop.exp > >> +++ b/gdb/testsuite/gdb.btrace/non-stop.exp > >> @@ -79,7 +79,7 @@ proc gdb_cont_to_bp_line { line threads nthreads } { > >> $nthreads > >> } > >> > >> -proc gdb_cont_to_no_history { threads cmd nthreads } { > >> +proc gdb_cont_to_no_history_backward { threads cmd nthreads } { > >> gdb_cont_to $threads $cmd \ > >> [multi_line \ > >> "No more reverse-execution history\." \ > >> @@ -89,6 +89,16 @@ proc gdb_cont_to_no_history { threads cmd nthreads = } > >> { > >> $nthreads > >> } > >> > >> +proc gdb_cont_to_no_history_forward { threads cmd nthreads } { > >> + gdb_cont_to $threads $cmd \ > >> + [multi_line \ > >> + "No more reverse-execution history for emulation\. Going > forward > >> will continue executing and recording\." \ > >> + "\[^\\\r\\\n\]*" \ > >> + "\[^\\\r\\\n\]*" \ > >> + ] \ > >> + $nthreads > >> +} > >> + > >> # trace the code between the two breakpoints > >> with_test_prefix "prepare" { > >> gdb_cont_to_bp_line "$srcfile:$bp_1" all 2 > >> @@ -176,14 +186,14 @@ with_test_prefix "reverse-step" { > >> with_test_prefix "continue" { > >> with_test_prefix "thread 1" { > >> with_test_prefix "continue" { > >> - gdb_cont_to_no_history 1 "continue" 1 > >> + gdb_cont_to_no_history_forward 1 "continue" 1 > >> gdb_test "thread apply 1 info record" \ > >> ".*Recorded \[0-9\]+ instructions \[^\\\r\\\n\]*" > >> gdb_test "thread apply 2 info record" \ > >> ".*Replay in progress\. At instruction 5\." > >> } > >> with_test_prefix "reverse-continue" { > >> - gdb_cont_to_no_history 1 "reverse-continue" 1 > >> + gdb_cont_to_no_history_backward 1 "reverse-continue" 1 > >> gdb_test "thread apply 1 info record" \ > >> ".*Replay in progress\. At instruction 1\." > >> gdb_test "thread apply 2 info record" \ > >> @@ -193,14 +203,14 @@ with_test_prefix "continue" { > >> > >> with_test_prefix "thread 2" { > >> with_test_prefix "continue" { > >> - gdb_cont_to_no_history 2 "continue" 1 > >> + gdb_cont_to_no_history_forward 2 "continue" 1 > >> gdb_test "thread apply 1 info record" \ > >> ".*Replay in progress\. At instruction 1\." > >> gdb_test "thread apply 2 info record" \ > >> ".*Recorded \[0-9\]+ instructions \[^\\\r\\\n\]*" > >> } > >> with_test_prefix "reverse-continue" { > >> - gdb_cont_to_no_history 2 "reverse-continue" 1 > >> + gdb_cont_to_no_history_backward 2 "reverse-continue" 1 > >> gdb_test "thread apply 1 info record" \ > >> ".*Replay in progress\. At instruction 1\." > >> gdb_test "thread apply 2 info record" \ > >> @@ -215,8 +225,8 @@ with_test_prefix "no progress" { > >> gdb_test "thread apply 1 record goto end" ".*" > >> gdb_test "thread apply 2 record goto begin" ".*" > >> > >> - gdb_cont_to_no_history 1 "continue" 1 > >> - gdb_cont_to_no_history 1 "step" 1 > >> + gdb_cont_to_no_history_forward 1 "continue" 1 > >> + gdb_cont_to_no_history_forward 1 "step" 1 > >> gdb_test "thread apply 1 info record" \ > >> ".*Recorded \[0-9\]+ instructions \[^\\\r\\\n\]*" > >> gdb_test "thread apply 2 info record" \ > >> @@ -227,8 +237,8 @@ with_test_prefix "no progress" { > >> gdb_test "thread apply 1 record goto begin" ".*" > >> gdb_test "thread apply 2 record goto end" ".*" > >> > >> - gdb_cont_to_no_history 2 "continue" 1 > >> - gdb_cont_to_no_history 2 "step" 1 > >> + gdb_cont_to_no_history_forward 2 "continue" 1 > >> + gdb_cont_to_no_history_forward 2 "step" 1 > >> gdb_test "thread apply 1 info record" \ > >> ".*Replay in progress\. At instruction 1\." > >> gdb_test "thread apply 2 info record" \ > >> @@ -238,7 +248,7 @@ with_test_prefix "no progress" { > >> with_test_prefix "all" { > >> gdb_test "thread apply all record goto begin" ".*" > >> > >> - gdb_cont_to_no_history all "continue" 2 > >> + gdb_cont_to_no_history_forward all "continue" 2 > >> gdb_test "thread apply 1 info record" \ > >> ".*Recorded \[0-9\]+ instructions \[^\\\r\\\n\]*" > >> gdb_test "thread apply 2 info record" \ > >> diff --git a/gdb/testsuite/gdb.reverse/break-precsave.exp > >> b/gdb/testsuite/gdb.reverse/break-precsave.exp > >> index b9d94681247..6d3b95933d4 100644 > >> --- a/gdb/testsuite/gdb.reverse/break-precsave.exp > >> +++ b/gdb/testsuite/gdb.reverse/break-precsave.exp > >> @@ -73,7 +73,7 @@ proc precsave_tests {} { > >> -re ".*Breakpoint $decimal,.*$srcfile:$end_location.*$gdb_prompt = $" > >> { > >> pass "go to end of main forward" > >> } > >> - -re "No more reverse-execution history.* end of main .*$gdb_promp= t > >> $" { > >> + -re "No more reverse-execution history for emulation. Going forwa= rd > >> will continue executing and recording.* end of main .*$gdb_prompt $" { > >> pass "go to end of main forward" > >> } > >> } > >> @@ -103,7 +103,7 @@ proc precsave_tests {} { > >> -re ".*Breakpoint $decimal,.*$srcfile:$end_location.*$gdb_prompt = $" > >> { > >> pass "end of record log" > >> } > >> - -re "No more reverse-execution history.* end of main .*$gdb_promp= t > >> $" { > >> + -re "No more reverse-execution history for emulation. Going forwa= rd > >> will continue executing and recording.* end of main .*$gdb_prompt $" { > >> pass "end of record log" > >> } > >> } > >> diff --git a/gdb/testsuite/gdb.reverse/break-reverse.exp > >> b/gdb/testsuite/gdb.reverse/break-reverse.exp > >> index 1dd327ca654..bf74d6f7d07 100644 > >> --- a/gdb/testsuite/gdb.reverse/break-reverse.exp > >> +++ b/gdb/testsuite/gdb.reverse/break-reverse.exp > >> @@ -80,7 +80,7 @@ gdb_test_multiple "continue" "end of record log" { > >> -re ".*Breakpoint $decimal,.*$srcfile:$end_location.*$gdb_prompt > $" { > >> pass "end of record log" > >> } > >> - -re "No more reverse-execution history.* end of main .*$gdb_promp= t > $" { > >> + -re "No more reverse-execution history for emulation. Going > forward will > >> continue executing and recording.* end of main .*$gdb_prompt $" { > >> pass "end of record log" > >> } > >> } > >> diff --git a/gdb/testsuite/gdb.reverse/machinestate-precsave.exp > >> b/gdb/testsuite/gdb.reverse/machinestate-precsave.exp > >> index 19c5934bfdf..693c304bc18 100644 > >> --- a/gdb/testsuite/gdb.reverse/machinestate-precsave.exp > >> +++ b/gdb/testsuite/gdb.reverse/machinestate-precsave.exp > >> @@ -85,7 +85,7 @@ gdb_test_multiple "continue" "go to end of main > >> forward" { > >> -re ".*Breakpoint $decimal,.*$srcfile:$endmain.*$gdb_prompt $" { > >> pass "go to end of main forward" > >> } > >> - -re "No more reverse-execution history.* end main .*$gdb_prompt $= " > { > >> + -re "No more reverse-execution history for emulation. Going > forward will > >> continue executing and recording.* end main .*$gdb_prompt $" { > >> pass "go to end of main forward" > >> } > >> } > >> -- > >> 2.42.0 > > Intel Deutschland GmbH > > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > > Tel: +49 89 99 8853-0, www.intel.de > > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Sil= va > > Chairperson of the Supervisory Board: Nicole Lau > > Registered Office: Munich > > Commercial Register: Amtsgericht Muenchen HRB 186928 > > > > -- > Cheers, > Guinevere Larsen > She/Her/Hers > > --000000000000acc79e0613a2314a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
> I think "reached beginning of ~= " makes sense, but that wouldn't fix the
> confusion about w= hether GDB can continue executing (and recording) on
> its own.
>
> On the other hand, I worry just saying &q= uot;switching to recording mode"
> might seem too arbitrary for = users, if they don't know the context of
> the commit message.

That's correct. The current issue in the forward case= is that execution can
continue. GDB will halt at the beginning of the r= everse history because
emulation cannot proceed. However, execution can = resume in the 'normal' mode,
and recording can also continue. I = was attempting to inform the user that a
different type of execution wou= ld be possible to continue debugging. Backward
execution does not encoun= ter the same issue because reaching the end of history
signifies the end= of the reverse execution. I am unsure how to convey all these
points wi= th fewer words unless we opt for less specificity. Please suggest
someth= ing in the preferred direction.

Thank you in advance,
Alex

On Thu, Mar 14, 2024 at 11:26=E2=80=AFAM Guinevere L= arsen <blarsen@redhat.com> = wrote:
On 14/03/= 2024 07:37, Metzger, Markus T wrote:
> Hello Alex,
>
> I like the idea, but I find the wording a bit verbose.=C2=A0 Can we fi= nd something shorter?
> E.g. "reached beginning of reverse-execution history" vs &qu= ot;reached end of ~".
> Not sure if that's clear enough that we're recording again.=C2= =A0 We could also omit
> the "reached end of ~" part and just say "switching to = recording mode".

I think "reached beginning of ~" makes sense, but that wouldn'= ;t fix the
confusion about whether GDB can continue executing (and recording) on
its own.

On the other hand, I worry just saying "switching to recording mode&qu= ot;
might seem too arbitrary for users, if they don't know the context of <= br> the commit message.

I wonder if good documentation can offset the second issue, though...

>
>>=C2=A0 =C2=A0 if (uiout->is_mi_like_p ())
>>=C2=A0 =C2=A0 =C2=A0 uiout->field_string ("reason", as= ync_reason_lookup
>> (EXEC_ASYNC_NO_HISTORY));
>> +=C2=A0 else if (execution_direction =3D=3D EXEC_FORWARD)
>> +=C2=A0 =C2=A0 uiout->text ("\nNo more reverse-execution h= istory for emulation. "
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"Going forwa= rd will continue executing and recording.\n");
>>=C2=A0 =C2=A0 else
>> -=C2=A0 =C2=A0 uiout->text ("\nNo more reverse-execution h= istory.\n");
>> +=C2=A0 =C2=A0 {
>> +=C2=A0 =C2=A0 =C2=A0 gdb_assert (execution_direction =3D=3D EXEC_= REVERSE);
>> +=C2=A0 =C2=A0 =C2=A0 uiout->text ("\nNo more reverse-exec= ution history.\n");
>> +=C2=A0 =C2=A0 }
> We should probably give MI a similar indication, although that would l= ikely
> break existing MI consumers.=C2=A0 Not sure how opt-in is handled in M= I.

I don't know either, but relatively easy change would be to add a new <= br> field to the mi response to show direction when recording is enabled, so mi consumers could decide how to handle a no-history stop.

Or we could announce the change on GDB 15 release and hold off on
merging this after 15 branched? Not sure if this is the right process,
though.

I had a suggestion after FOSDEM to have GDB indicate the direction of
execution for every command when recording is enabled, and adding that
to MI would be a step in that direction (again, leaving it up to MI
consumers on how to report it to users). what do you think?

>
> regards,
> Markus.
>
>> -----Original Message-----
>> From: Alex Chronopoulos <achronop@gmail.com>
>> Sent: Wednesday, March 13, 2024 9:49 PM
>> To: gdb-patches@sourceware.org
>> Cc: Alex Chronopoulos <achronop@gmail.com>
>> Subject: [PATCH] Change message when reaching end of reverse histo= ry.
>>
>> In a record session, when we move backward, GDB switches from norm= al
>> execution to simulation. Moving forward again, the emulation conti= nues
>> until the end of the reverse history. When the end is reached, the=
>> execution stops, and a warning message is shown. This message has = been
>> modified to indicate that the forward emulation has reached the en= d, but
>> the execution can continue as normal, and the recording will also = continue.
>>
>> Before this patch, the warning message shown in that case was the = same as
>> in the reverse case. This meant that when the end of history was r= eached in
>> either backward or forward emulation, the same message was display= ed:
>>
>> "No more reverse-execution history."
>>
>> This message remains for backward emulation. However, in forward >> emulation,
>> it has been modified to:
>>
>> "No more reverse-execution history for emulation. Going forwa= rd will
>> continue executing and recording."
>>
>> The reason for this change is that the initial message was deceivi= ng,
>> making the user believe that forward debugging could not continue.=
>>
>> Bug: https://sourceware.org/bugzilla= /show_bug.cgi?id=3D31224
>> ---
>> gdb/NEWS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 |=C2=A0 5 ++++
>> gdb/infrun.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 = 8 ++++-
>> gdb/testsuite/gdb.btrace/non-stop.exp=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0| 30 ++++++++++++-------
>> gdb/testsuite/gdb.reverse/break-precsave.exp=C2=A0 |=C2=A0 4 +-- >> gdb/testsuite/gdb.reverse/break-reverse.exp=C2=A0 =C2=A0|=C2=A0 2 = +-
>> .../gdb.reverse/machinestate-precsave.exp=C2=A0 =C2=A0 =C2=A0|=C2= =A0 2 +-
>> 6 files changed, 36 insertions(+), 15 deletions(-)
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index 2638b3e0d9c..f2e85776a53 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -13,6 +13,11 @@
>>=C2=A0 =C2=A0 the background, resulting in faster startup.=C2=A0 Th= is can be controlled
>>=C2=A0 =C2=A0 using "maint set dwarf synchronous".
>>
>> +* In a record session, when a forward emulation reaches the end o= f the
>> reverse
>> +=C2=A0 history, the warning message has been changed to indicate = that the end of
>> the
>> +=C2=A0 history has been reached.=C2=A0 It also specifies that the= forward execution can
>> +=C2=A0 continue, and the recording will also continue.
>> +
>> * Changed commands
>>
>> disassemble
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index bbb98f6dcdb..e129eb0582f 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -9244,8 +9244,14 @@ print_no_history_reason (struct ui_out *uio= ut)
>> {
>>=C2=A0 =C2=A0 if (uiout->is_mi_like_p ())
>>=C2=A0 =C2=A0 =C2=A0 uiout->field_string ("reason", as= ync_reason_lookup
>> (EXEC_ASYNC_NO_HISTORY));
>> +=C2=A0 else if (execution_direction =3D=3D EXEC_FORWARD)
>> +=C2=A0 =C2=A0 uiout->text ("\nNo more reverse-execution h= istory for emulation. "
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"Going forwa= rd will continue executing and recording.\n");
>>=C2=A0 =C2=A0 else
>> -=C2=A0 =C2=A0 uiout->text ("\nNo more reverse-execution h= istory.\n");
>> +=C2=A0 =C2=A0 {
>> +=C2=A0 =C2=A0 =C2=A0 gdb_assert (execution_direction =3D=3D EXEC_= REVERSE);
>> +=C2=A0 =C2=A0 =C2=A0 uiout->text ("\nNo more reverse-exec= ution history.\n");
>> +=C2=A0 =C2=A0 }
>> }
>>
>> /* Print current location without a level number, if we have chang= ed
>> diff --git a/gdb/testsuite/gdb.btrace/non-stop.exp
>> b/gdb/testsuite/gdb.btrace/non-stop.exp
>> index 62c940e4cd6..7ce3008c120 100644
>> --- a/gdb/testsuite/gdb.btrace/non-stop.exp
>> +++ b/gdb/testsuite/gdb.btrace/non-stop.exp
>> @@ -79,7 +79,7 @@ proc gdb_cont_to_bp_line { line threads nthreads= } {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 $nthreads
>> }
>>
>> -proc gdb_cont_to_no_history { threads cmd nthreads } {
>> +proc gdb_cont_to_no_history_backward { threads cmd nthreads } { >>=C2=A0 =C2=A0 =C2=A0 gdb_cont_to $threads $cmd \
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 [multi_line \
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"No mor= e reverse-execution history\." \
>> @@ -89,6 +89,16 @@ proc gdb_cont_to_no_history { threads cmd nthre= ads }
>> {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 $nthreads
>> }
>>
>> +proc gdb_cont_to_no_history_forward { threads cmd nthreads } { >> +=C2=A0 =C2=A0 gdb_cont_to $threads $cmd \
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 [multi_line \
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"No more rev= erse-execution history for emulation\. Going forward
>> will continue executing and recording\." \
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"\[^\\\r\\\n= \]*" \
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"\[^\\\r\\\n= \]*" \
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ] \
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 $nthreads
>> +}
>> +
>> # trace the code between the two breakpoints
>> with_test_prefix "prepare" {
>>=C2=A0 =C2=A0 =C2=A0 gdb_cont_to_bp_line "$srcfile:$bp_1"= all 2
>> @@ -176,14 +186,14 @@ with_test_prefix "reverse-step" {<= br> >> with_test_prefix "continue" {
>>=C2=A0 =C2=A0 =C2=A0 with_test_prefix "thread 1" {
>>=C2=A0 =C2=A0 =C2=A0 with_test_prefix "continue" {
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history 1 "contin= ue" 1
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history_forward 1 &quo= t;continue" 1
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 1 in= fo record" \
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ".*Recorded \= [0-9\]+ instructions \[^\\\r\\\n\]*"
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 2 in= fo record" \
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ".*Replay in = progress\.=C2=A0 At instruction 5\."
>>=C2=A0 =C2=A0 =C2=A0 }
>>=C2=A0 =C2=A0 =C2=A0 with_test_prefix "reverse-continue" = {
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history 1 "revers= e-continue" 1
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history_backward 1 &qu= ot;reverse-continue" 1
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 1 in= fo record" \
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ".*Replay in = progress\.=C2=A0 At instruction 1\."
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 2 in= fo record" \
>> @@ -193,14 +203,14 @@ with_test_prefix "continue" {
>>
>>=C2=A0 =C2=A0 =C2=A0 with_test_prefix "thread 2" {
>>=C2=A0 =C2=A0 =C2=A0 with_test_prefix "continue" {
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history 2 "contin= ue" 1
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history_forward 2 &quo= t;continue" 1
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 1 in= fo record" \
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ".*Replay in = progress\.=C2=A0 At instruction 1\."
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 2 in= fo record" \
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ".*Recorded \= [0-9\]+ instructions \[^\\\r\\\n\]*"
>>=C2=A0 =C2=A0 =C2=A0 }
>>=C2=A0 =C2=A0 =C2=A0 with_test_prefix "reverse-continue" = {
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history 2 "revers= e-continue" 1
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history_backward 2 &qu= ot;reverse-continue" 1
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 1 in= fo record" \
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ".*Replay in = progress\.=C2=A0 At instruction 1\."
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 2 in= fo record" \
>> @@ -215,8 +225,8 @@ with_test_prefix "no progress" {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 1 re= cord goto end" ".*"
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 2 re= cord goto begin" ".*"
>>
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history 1 "contin= ue" 1
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history 1 "step&q= uot; 1
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history_forward 1 &quo= t;continue" 1
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history_forward 1 &quo= t;step" 1
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 1 in= fo record" \
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ".*Recorded \= [0-9\]+ instructions \[^\\\r\\\n\]*"
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 2 in= fo record" \
>> @@ -227,8 +237,8 @@ with_test_prefix "no progress" {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 1 re= cord goto begin" ".*"
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 2 re= cord goto end" ".*"
>>
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history 2 "contin= ue" 1
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history 2 "step&q= uot; 1
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history_forward 2 &quo= t;continue" 1
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history_forward 2 &quo= t;step" 1
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 1 in= fo record" \
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ".*Replay in = progress\.=C2=A0 At instruction 1\."
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 2 in= fo record" \
>> @@ -238,7 +248,7 @@ with_test_prefix "no progress" {
>>=C2=A0 =C2=A0 =C2=A0 with_test_prefix "all" {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply all = record goto begin" ".*"
>>
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history all "cont= inue" 2
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_cont_to_no_history_forward all &q= uot;continue" 2
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 1 in= fo record" \
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ".*Recorded \= [0-9\]+ instructions \[^\\\r\\\n\]*"
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_test "thread apply 2 in= fo record" \
>> diff --git a/gdb/testsuite/gdb.reverse/break-precsave.exp
>> b/gdb/testsuite/gdb.reverse/break-precsave.exp
>> index b9d94681247..6d3b95933d4 100644
>> --- a/gdb/testsuite/gdb.reverse/break-precsave.exp
>> +++ b/gdb/testsuite/gdb.reverse/break-precsave.exp
>> @@ -73,7 +73,7 @@ proc precsave_tests {} {
>>=C2=A0 =C2=A0 =C2=A0 -re ".*Breakpoint $decimal,.*$srcfile:$en= d_location.*$gdb_prompt $"
>> {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pass "go to end of main for= ward"
>>=C2=A0 =C2=A0 =C2=A0 }
>> -=C2=A0 =C2=A0 -re "No more reverse-execution history.* end o= f main .*$gdb_prompt
>> $" {
>> +=C2=A0 =C2=A0 -re "No more reverse-execution history for emu= lation. Going forward
>> will continue executing and recording.* end of main .*$gdb_prompt = $" {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pass "go to end of main for= ward"
>>=C2=A0 =C2=A0 =C2=A0 }
>>=C2=A0 =C2=A0 =C2=A0 }
>> @@ -103,7 +103,7 @@ proc precsave_tests {} {
>>=C2=A0 =C2=A0 =C2=A0 -re ".*Breakpoint $decimal,.*$srcfile:$en= d_location.*$gdb_prompt $"
>> {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pass "end of record log&quo= t;
>>=C2=A0 =C2=A0 =C2=A0 }
>> -=C2=A0 =C2=A0 -re "No more reverse-execution history.* end o= f main .*$gdb_prompt
>> $" {
>> +=C2=A0 =C2=A0 -re "No more reverse-execution history for emu= lation. Going forward
>> will continue executing and recording.* end of main .*$gdb_prompt = $" {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pass "end of record log&quo= t;
>>=C2=A0 =C2=A0 =C2=A0 }
>>=C2=A0 =C2=A0 =C2=A0 }
>> diff --git a/gdb/testsuite/gdb.reverse/break-reverse.exp
>> b/gdb/testsuite/gdb.reverse/break-reverse.exp
>> index 1dd327ca654..bf74d6f7d07 100644
>> --- a/gdb/testsuite/gdb.reverse/break-reverse.exp
>> +++ b/gdb/testsuite/gdb.reverse/break-reverse.exp
>> @@ -80,7 +80,7 @@ gdb_test_multiple "continue" "end= of record log" {
>>=C2=A0 =C2=A0 =C2=A0 -re ".*Breakpoint $decimal,.*$srcfile:$en= d_location.*$gdb_prompt $" {
>>=C2=A0 =C2=A0 =C2=A0 pass "end of record log"
>>=C2=A0 =C2=A0 =C2=A0 }
>> -=C2=A0 =C2=A0 -re "No more reverse-execution history.* end o= f main .*$gdb_prompt $" {
>> +=C2=A0 =C2=A0 -re "No more reverse-execution history for emu= lation. Going forward will
>> continue executing and recording.* end of main .*$gdb_prompt $&quo= t; {
>>=C2=A0 =C2=A0 =C2=A0 pass "end of record log"
>>=C2=A0 =C2=A0 =C2=A0 }
>> }
>> diff --git a/gdb/testsuite/gdb.reverse/machinestate-precsave.exp >> b/gdb/testsuite/gdb.reverse/machinestate-precsave.exp
>> index 19c5934bfdf..693c304bc18 100644
>> --- a/gdb/testsuite/gdb.reverse/machinestate-precsave.exp
>> +++ b/gdb/testsuite/gdb.reverse/machinestate-precsave.exp
>> @@ -85,7 +85,7 @@ gdb_test_multiple "continue" "go = to end of main
>> forward" {
>>=C2=A0 =C2=A0 =C2=A0 -re ".*Breakpoint $decimal,.*$srcfile:$en= dmain.*$gdb_prompt $"=C2=A0 {
>>=C2=A0 =C2=A0 =C2=A0 pass "go to end of main forward"
>>=C2=A0 =C2=A0 =C2=A0 }
>> -=C2=A0 =C2=A0 -re "No more reverse-execution history.* end m= ain .*$gdb_prompt $" {
>> +=C2=A0 =C2=A0 -re "No more reverse-execution history for emu= lation. Going forward will
>> continue executing and recording.* end main .*$gdb_prompt $" = {
>>=C2=A0 =C2=A0 =C2=A0 pass "go to end of main forward"
>>=C2=A0 =C2=A0 =C2=A0 }
>> }
>> --
>> 2.42.0
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Si= lva
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
>

--
Cheers,
Guinevere Larsen
She/Her/Hers

--000000000000acc79e0613a2314a--