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.133.124]) by sourceware.org (Postfix) with ESMTPS id 828283847725 for ; Wed, 3 Apr 2024 17:54:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 828283847725 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 828283847725 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712166860; cv=none; b=M3KxTaqqeRIgEYcC5mkYasr1wb0U+/9hVxNBjuWtCR3/ciAaMsnODQefr0KTKo8IHWicFfgvuAcvAhzooVWmV3xxRSBSF2UgDu4Zbpn0tEFHXMC3r4Ijv8YbsZy+WYHQJlQ5QxbOSNiq5Mxk12mQuhlhI6fHIww5REup73Rtexo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712166860; c=relaxed/simple; bh=KavnJ20OaQBdxL9gLhUViJHlf3wGi7D9sFkYvU2GHe4=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=qErQlWYeaapxy4Jggb3hSTN7cT9IPcwAI9DHfI7i2wSuRNXKgxLBws0q+Uwdy4f0hNDPFtctB55WfNA99JKHaZSphaSFVC7VIsUjXnr+30welumeIbBQw0coda181eEKdtu99Ui8/MeoBZ34H/x+HO7DeC9Oqy4Lg8qE1wQDzN4= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1712166856; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=PD4bR4zNLYzdTgdiAkJVgT+EoPihhSefmduxLdeIZdc=; b=cqfShLxSV7Vi47Jfb0Sqw6urXCnIfejVNFDVt073X8kkQC1PTdbHIjoL10BgkkbIFj5tUZ 942etxbhs2+yZ8wbEclf7sUxTBCKdaJe/QEbmIgIUcRyrLd9fhSkm7/mxBGQ1yWjaIISUq 3WBPiLzrdABz/YPrRshcDHzl6XDL5I4= Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-22-yGn83ZJZM-KPqnizkEcz5g-1; Wed, 03 Apr 2024 13:54:14 -0400 X-MC-Unique: yGn83ZJZM-KPqnizkEcz5g-1 Received: by mail-qv1-f69.google.com with SMTP id 6a1803df08f44-6992977e697so611776d6.1 for ; Wed, 03 Apr 2024 10:54:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712166854; x=1712771654; h=in-reply-to:from:content-language:references:cc:to:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=PD4bR4zNLYzdTgdiAkJVgT+EoPihhSefmduxLdeIZdc=; b=NpwO1kMcDsdFvYN8IF3ShqO5c2KKyr2fDEMGKH/zYytTKggjZ+64JhNF+1jciq/PVV uT0tDI/fb+2oUPkFFEjvBrn9sm3UEUOQuWncjcdy6GwFGghD566K+8VRdaxoLR6z0MGO 4HARujYTVdl4fB5XDufHILFALFP0kGFO0eJT01Lu8UopkHsn6ZO2n4iICncDUCWhw/HC nDbht/HLgBGrYivIkoETrUeEvudaRoNVxAP15nUTiHsS1mkPzxs1YX4nQ1F3zxtCDnS1 DK3sv6eMCifRpXER1l/LdT0Bb5CTI5Rk8AYU4zqZKDxzW7HqQN8xRkoYonU9iOR8Cf3H sBbQ== X-Forwarded-Encrypted: i=1; AJvYcCVjv41f1SgpIZh/vOKVJcHHkX0mU/HPsChKVLYh0v46Lu0VgADkfP7KTNAiuVcskUVrGFCN/lUOseuPqGTEA6ROVvcQKFzPtgt93A== X-Gm-Message-State: AOJu0YwQkPuXs2u1fOZxx84Udxf5QcDOOmFLZVMyt17Sjt+r/KTZ0yww fbRuIPvtjRmDPUd+d7vsM5zkXysCiZbI5DtTpCygDxtW8WVUtkbbwP4ymjpwG/6Im8/ELFs+9Of tzJ8n/xbjh+YRM7cMHSb3mWZKKRConfI7UQDV+kmmg3zCt/gQrYD/LGYVeHc= X-Received: by 2002:a05:6214:da5:b0:699:29e1:560 with SMTP id h5-20020a0562140da500b0069929e10560mr59156qvh.48.1712166853676; Wed, 03 Apr 2024 10:54:13 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFpT9Glm2JF1iKY0tUdO8J+ezVhafZdd6zKUIDwagQoCJ7+9xEWY7BcS5TUF/QjlFC2ERfVDA== X-Received: by 2002:a05:6214:da5:b0:699:29e1:560 with SMTP id h5-20020a0562140da500b0069929e10560mr59139qvh.48.1712166853107; Wed, 03 Apr 2024 10:54:13 -0700 (PDT) Received: from ?IPV6:2804:14d:8084:92c5::1001? ([2804:14d:8084:92c5::1001]) by smtp.gmail.com with ESMTPSA id fn2-20020ad45d62000000b00698fe4f91edsm4282300qvb.79.2024.04.03.10.54.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 03 Apr 2024 10:54:12 -0700 (PDT) Message-ID: Date: Wed, 3 Apr 2024 14:54:08 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] Change message when reaching end of reverse history. To: Alex Chronopoulos Cc: "Metzger, Markus T" , "Eli Zaretskii (eliz@gnu.org)" , "gdb-patches@sourceware.org" References: <20240313204830.2521708-1-achronop@gmail.com> <50e7e7f5-024b-4ac8-be5c-948ad2f41e73@redhat.com> From: Guinevere Larsen In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="------------8Q5WWI2kBFmO7f42pLlNXF0m" Content-Language: en-US X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This is a multi-part message in MIME format. --------------8Q5WWI2kBFmO7f42pLlNXF0m Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 3/14/24 14:26, Alex Chronopoulos wrote: > > 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. Markus, Alex, Sorry about the last 2 weeks of silence, I had 2 weeks of vacation I had to take. Back to the complicated wording, I asked a couple (non GDB developer) friends to get end users perspectives. One said that "switching to recording mode" was very confusing, since they thought GDB was already in recording mode. After some brainstorming, we came up with "recording will continue" as a way to express that the program can still execute forward We can continue using the old "No more reverse-execution history." to explain why record more, or we can shorten it to "no more history", "no more recorded history", or some other variation. What do you think? > > Thank you in advance, > Alex > > On Thu, Mar 14, 2024 at 11:26 AM 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 of ~". > > 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 == 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 == 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, 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 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=31224 > >> --- > >> 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 > controlled > >>    using "maint set dwarf synchronous". > >> > >> +* In a record session, when a forward emulation reaches the > end of the > >> 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 == 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 == 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_prompt > >> $" { > >> +    -re "No more reverse-execution history for emulation. > Going forward > >> 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_prompt > >> $" { > >> +    -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/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_prompt $" { > >> +    -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 Silva > > Chairperson of the Supervisory Board: Nicole Lau > > Registered Office: Munich > > Commercial Register: Amtsgericht Muenchen HRB 186928 > > > > -- > Cheers, > Guinevere Larsen > She/Her/Hers > --------------8Q5WWI2kBFmO7f42pLlNXF0m--