From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id XVp2NQXR8mWqqAYAWB0awg (envelope-from ) for ; Thu, 14 Mar 2024 06:27:17 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=YCob10sB; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id C6B431E0C0; Thu, 14 Mar 2024 06:27:17 -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 9A4D11E030 for ; Thu, 14 Mar 2024 06:27:15 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E0C8D3857809 for ; Thu, 14 Mar 2024 10:27:14 +0000 (GMT) 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 8B9AB3857C62 for ; Thu, 14 Mar 2024 10:26:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8B9AB3857C62 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 8B9AB3857C62 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=1710412009; cv=none; b=yCI/3lyblU319bkwvrXQnumWMcRAovANXOQwaxkpBUVF6KRqcdB0cQ9/hOcFWJjAGZrOLNjYRu1A+13esUKLlE+GRy0pnOSPOjz/v3t5nYl3jzOVX8oLq2ugmilPNH7WaTpaQnvusvxL2L/Er9+QIPr7q8esH1oofcgho8slqI4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710412009; c=relaxed/simple; bh=guSrm8Z5gfYLzmukoWVQ1zt0FVT5lZHwTJvz0v5rgJU=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=fFMvdnqD9xKMj8SySYXH407s/AV8ZXw/BY2aKHMVV1+94mtUvHdDExHPfl8FSM26dcGe5B49hz/m8T6fcyIRaZRGEfHLegOVPLzxHIWOWcNsG+39pCPVkw+fUtH/OYHHA0/0Lq3hcQ+VZ8qUkeWLqFPUJLJcm9L0D1mcN8p7fqA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710412004; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=cdv67i2ozlBH5M54YalG7xExxhdMj0nASM6vd9NrTkI=; b=YCob10sB6tAlcjRZXI9vQzIx1CzUtzYFI+JlJ+XvFrxJFn4nDYLpTxhXuepLpr0P1tRlNG UerJA2aOvVdiV1NXvM7J8NZcUn2TGMWi/fQ7RlyHseOuBy8nrd7/Lf5blXoSRNoUR6wsmf xI+JIKEmjkUNAdNG8t5sIjKmlsQBsl8= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-372-aASR9-oYPKOiElLZ-BgVxg-1; Thu, 14 Mar 2024 06:26:41 -0400 X-MC-Unique: aASR9-oYPKOiElLZ-BgVxg-1 Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-42ed6809154so26554401cf.0 for ; Thu, 14 Mar 2024 03:26:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710412000; x=1711016800; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=cdv67i2ozlBH5M54YalG7xExxhdMj0nASM6vd9NrTkI=; b=L0CITxj8o8GuuCb6rdTZr7jGR2syuzfzU5Buw+ohXzd892OGxprG4LEgsLefG8ByQC yxiOCsJb9HhVTR8Fif83EhZ7Qnhzd4wvd6WayZyvcXArm75jq1kr9FUGLkkQrSK2uiVs WZElH4eIJN447G3DgLVa/xpfP+Exu6QJ+f6efq8fIeWbX2sTn6RbvoZRNi/Ewdh3VSDx I/moSD8LxVHml26LvkFKbrCazmR9gzsDJlWtj7KhbzWsY0+Jx5Js6mQd0zoK+fihk6Pm AsHmhzOqFmRLU/TauYs3hdXP0mB5DtdZBnGj1t1mYvRNh2gPvtLYzUAdnTmkGwNS+Prf 4/YQ== X-Forwarded-Encrypted: i=1; AJvYcCUz2PshDWWVAlD2H4GtQgxssBT/PfBbgRLt8iWVYmQPo+Ser9jA56aIkQlVE0J7sBsQzfm8tYM5kUxqjTBqYsXihbinNNCOItqPSA== X-Gm-Message-State: AOJu0YxirIee8ElN3YcLinknRMaUmQnZvXL62WaaGrAzBl5PRH6aREUz 7jW2+HollajDKFm95M3lQX7WllChkIcAgM6ENoYBfhQrI4mwtqflzCMjq2DBRPOrnTSncvtPwk5 UyU2ZoQZtC45OvKJNLH0AWRnxfoA0x2rv77YcYh5Dc0Xahtp99mZRWHbloy4= X-Received: by 2002:a05:622a:1309:b0:42e:f5f8:e970 with SMTP id v9-20020a05622a130900b0042ef5f8e970mr2360782qtk.12.1710412000558; Thu, 14 Mar 2024 03:26:40 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEZBxohxIZvjhsqHIyW5zrfthb5RUt854iaPsVWFPh594aDB8Af2PpWSnagUtjMwEt797i3rQ== X-Received: by 2002:a05:622a:1309:b0:42e:f5f8:e970 with SMTP id v9-20020a05622a130900b0042ef5f8e970mr2360754qtk.12.1710412000119; Thu, 14 Mar 2024 03:26:40 -0700 (PDT) Received: from [10.202.149.221] (nat-pool-brq-u.redhat.com. [213.175.37.12]) by smtp.gmail.com with ESMTPSA id j26-20020ac84c9a000000b0042edba08089sm617613qtv.55.2024.03.14.03.26.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 14 Mar 2024 03:26:39 -0700 (PDT) Message-ID: <50e7e7f5-024b-4ac8-be5c-948ad2f41e73@redhat.com> Date: Thu, 14 Mar 2024 11:26:38 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] Change message when reaching end of reverse history. To: "Metzger, Markus T" , Alex Chronopoulos Cc: "Eli Zaretskii (eliz@gnu.org)" , "gdb-patches@sourceware.org" References: <20240313204830.2521708-1-achronop@gmail.com> From: Guinevere Larsen In-Reply-To: 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=-12.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, 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.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 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