Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] Change message when reaching end of reverse history.
@ 2024-03-13 20:48 Alex Chronopoulos
  2024-03-14  6:37 ` Metzger, Markus T
  0 siblings, 1 reply; 16+ messages in thread
From: Alex Chronopoulos @ 2024-03-13 20:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Alex Chronopoulos

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] Change message when reaching end of reverse history.
  2024-03-13 20:48 [PATCH] Change message when reaching end of reverse history Alex Chronopoulos
@ 2024-03-14  6:37 ` Metzger, Markus T
  2024-03-14 10:26   ` Guinevere Larsen
  2024-03-14 14:53   ` Tom Tromey
  0 siblings, 2 replies; 16+ messages in thread
From: Metzger, Markus T @ 2024-03-14  6:37 UTC (permalink / raw)
  To: Alex Chronopoulos; +Cc: Eli Zaretskii (eliz@gnu.org), gdb-patches

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".

>   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.

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 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 <http://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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change message when reaching end of reverse history.
  2024-03-14  6:37 ` Metzger, Markus T
@ 2024-03-14 10:26   ` Guinevere Larsen
  2024-03-14 17:26     ` Alex Chronopoulos
  2024-03-14 14:53   ` Tom Tromey
  1 sibling, 1 reply; 16+ messages in thread
From: Guinevere Larsen @ 2024-03-14 10:26 UTC (permalink / raw)
  To: Metzger, Markus T, Alex Chronopoulos
  Cc: Eli Zaretskii (eliz@gnu.org), gdb-patches

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 <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 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 <http://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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change message when reaching end of reverse history.
  2024-03-14  6:37 ` Metzger, Markus T
  2024-03-14 10:26   ` Guinevere Larsen
@ 2024-03-14 14:53   ` Tom Tromey
  2024-03-14 16:13     ` Metzger, Markus T
  1 sibling, 1 reply; 16+ messages in thread
From: Tom Tromey @ 2024-03-14 14:53 UTC (permalink / raw)
  To: Metzger, Markus T
  Cc: Alex Chronopoulos, Eli Zaretskii (eliz@gnu.org), gdb-patches

>>>>> Metzger, Markus T <markus.t.metzger@intel.com> writes:

> 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 thought perhaps this change was just about being more clear to users
-- which isn't really a concern for MI consumers.  So my first reaction
is to wonder if this really is needed for MI.

Anyway in MI there are a few ways to change things without breaking
consumers.  One way is to add a new field to the output.  Another way is
opt-in behavior -- a new command to enable the new behavior, or simply
enabling it in the "next" MI version.

Tom

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] Change message when reaching end of reverse history.
  2024-03-14 14:53   ` Tom Tromey
@ 2024-03-14 16:13     ` Metzger, Markus T
  0 siblings, 0 replies; 16+ messages in thread
From: Metzger, Markus T @ 2024-03-14 16:13 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Alex Chronopoulos, Eli Zaretskii (eliz@gnu.org), gdb-patches

>> 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 thought perhaps this change was just about being more clear to users
>-- which isn't really a concern for MI consumers.  So my first reaction
>is to wonder if this really is needed for MI.

If MI consumers have some other way of distinguishing the two cases I agree
that we do not need to enable them.  If they do not, I think we should enable
them to give a clearer indication to their users like this patch does to CLI users.

Regards,
Markus.
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 Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change message when reaching end of reverse history.
  2024-03-14 10:26   ` Guinevere Larsen
@ 2024-03-14 17:26     ` Alex Chronopoulos
  2024-04-03 17:54       ` Guinevere Larsen
  0 siblings, 1 reply; 16+ messages in thread
From: Alex Chronopoulos @ 2024-03-14 17:26 UTC (permalink / raw)
  To: Guinevere Larsen
  Cc: Metzger, Markus T, Eli Zaretskii (eliz@gnu.org), gdb-patches

[-- Attachment #1: Type: text/plain, Size: 15033 bytes --]

> 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 AM Guinevere Larsen <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.  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 <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 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 <http://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
>
>

[-- Attachment #2: Type: text/html, Size: 19787 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change message when reaching end of reverse history.
  2024-03-14 17:26     ` Alex Chronopoulos
@ 2024-04-03 17:54       ` Guinevere Larsen
  2024-04-03 21:15         ` Alex Chronopoulos
  0 siblings, 1 reply; 16+ messages in thread
From: Guinevere Larsen @ 2024-04-03 17:54 UTC (permalink / raw)
  To: Alex Chronopoulos
  Cc: Metzger, Markus T, Eli Zaretskii (eliz@gnu.org), gdb-patches

[-- Attachment #1: Type: text/plain, Size: 17478 bytes --]

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 <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. 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 <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
>     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 <http://www.intel.de>
>     <http://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
>

[-- Attachment #2: Type: text/html, Size: 25685 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change message when reaching end of reverse history.
  2024-04-03 17:54       ` Guinevere Larsen
@ 2024-04-03 21:15         ` Alex Chronopoulos
  2024-04-04  6:22           ` Metzger, Markus T
  0 siblings, 1 reply; 16+ messages in thread
From: Alex Chronopoulos @ 2024-04-03 21:15 UTC (permalink / raw)
  To: Guinevere Larsen
  Cc: Metzger, Markus T, Eli Zaretskii (eliz@gnu.org), gdb-patches

[-- Attachment #1: Type: text/plain, Size: 16581 bytes --]

Thank you, Guinevere.

My understanding is that you are referring to the forward case. Does it
mean we keep the backward case as is?

"No more history" sounds good to me. If that were the case, would the
suggested forward message become: "No more history, recording will
continue"?



On Wed, Apr 3, 2024 at 7:54 PM Guinevere Larsen <blarsen@redhat.com> wrote:

> 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 <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.  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 <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 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 <http://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
>>
>>

[-- Attachment #2: Type: text/html, Size: 27322 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] Change message when reaching end of reverse history.
  2024-04-03 21:15         ` Alex Chronopoulos
@ 2024-04-04  6:22           ` Metzger, Markus T
  2024-04-04  7:00             ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Metzger, Markus T @ 2024-04-04  6:22 UTC (permalink / raw)
  To: Eli Zaretskii (eliz@gnu.org)
  Cc: gdb-patches, Alex Chronopoulos, Guinevere Larsen

[-- Attachment #1: Type: text/plain, Size: 16534 bytes --]

Hello,

I see no reason for changing the existing portion.  The message is short enough:

    No more reverse-execution history.  Recording will continue.

Eli, are you OK with that change?

Regards,
Markus.

From: Alex Chronopoulos <achronop@gmail.com>
Sent: Wednesday, April 3, 2024 11:15 PM
To: Guinevere Larsen <blarsen@redhat.com>
Cc: Metzger, Markus T <markus.t.metzger@intel.com>; Eli Zaretskii (eliz@gnu.org) <eliz@gnu.org>; gdb-patches@sourceware.org
Subject: Re: [PATCH] Change message when reaching end of reverse history.

Thank you, Guinevere.

My understanding is that you are referring to the forward case. Does it mean we keep the backward case as is?

"No more history" sounds good to me. If that were the case, would the suggested forward message become: "No more history, recording will continue"?



On Wed, Apr 3, 2024 at 7:54 PM Guinevere Larsen <blarsen@redhat.com<mailto:blarsen@redhat.com>> wrote:
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 <blarsen@redhat.com<mailto: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.  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 <achronop@gmail.com<mailto:achronop@gmail.com>>
>> Sent: Wednesday, March 13, 2024 9:49 PM
>> To: gdb-patches@sourceware.org<mailto:gdb-patches@sourceware.org>
>> Cc: Alex Chronopoulos <achronop@gmail.com<mailto:achronop@gmail.com>>
>> 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<http://www.intel.de> <http://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
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 Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[-- Attachment #2: Type: text/html, Size: 27543 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change message when reaching end of reverse history.
  2024-04-04  6:22           ` Metzger, Markus T
@ 2024-04-04  7:00             ` Eli Zaretskii
  2024-04-04 12:16               ` Guinevere Larsen
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2024-04-04  7:00 UTC (permalink / raw)
  To: Metzger, Markus T; +Cc: gdb-patches, achronop, blarsen

> From: "Metzger, Markus T" <markus.t.metzger@intel.com>
> CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>, "Alex
>  Chronopoulos" <achronop@gmail.com>, Guinevere Larsen <blarsen@redhat.com>
> Date: Thu, 4 Apr 2024 06:22:14 +0000
> 
> I see no reason for changing the existing portion.  The message is short enough:
> 
>     No more reverse-execution history.  Recording will continue.
> 
> Eli, are you OK with that change?

I don't think I ever chimed into this discussion, and I'm not sure I
even understand the issue well enough.  But just on the face of it:
doesn't a bare "Recording will continue" fail to say anything about
the execution, which AFAIU also switches to some other mode in this
case?

If the above makes no sense, would someone please explain in more
detail what was the original issue, and I will try to come up with a
better suggestion.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change message when reaching end of reverse history.
  2024-04-04  7:00             ` Eli Zaretskii
@ 2024-04-04 12:16               ` Guinevere Larsen
  2024-04-04 12:39                 ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Guinevere Larsen @ 2024-04-04 12:16 UTC (permalink / raw)
  To: Eli Zaretskii, Metzger, Markus T; +Cc: gdb-patches, achronop

On 4/4/24 04:00, Eli Zaretskii wrote:
>> From: "Metzger, Markus T" <markus.t.metzger@intel.com>
>> CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>, "Alex
>>   Chronopoulos" <achronop@gmail.com>, Guinevere Larsen <blarsen@redhat.com>
>> Date: Thu, 4 Apr 2024 06:22:14 +0000
>>
>> I see no reason for changing the existing portion.  The message is short enough:

I didn't either, but after my email I found a group of people internal 
to redhat to ask about this (they are developers but not GDB users, so 
as fresh a set of eyes as we can ask for) and someone read 'no more 
reverse-execution history' as if GDB had somehow lost track of it or it 
had been corrupted. The alternative wording suggested was:

     end of history. Recording will continue.

>>
>>      No more reverse-execution history.  Recording will continue.
>>
>> Eli, are you OK with that change?
> I don't think I ever chimed into this discussion, and I'm not sure I
> even understand the issue well enough.  But just on the face of it:
> doesn't a bare "Recording will continue" fail to say anything about
> the execution, which AFAIU also switches to some other mode in this
> case?

 From a regular user's viewpoint, the execution never switched states, 
so explicitly saying it will change back will confuse the user more than 
help. If GDB said, for instance, "changing to replay mode" when the user 
first steps backwards, then it would make sense to specify that we're 
back at recording mode, but since we don't do that (and I want to 
minimize behavior change), I think we should not explicitly say behavior 
was changed.

>
> If the above makes no sense, would someone please explain in more
> detail what was the original issue, and I will try to come up with a
> better suggestion.
>
The original issue is that when a user is in replay mode going forward, 
and they hit the message "no more reverse-execution history", some 
interpret that message as 'you can no longer execute forward' instead of 
the intended 'if you continue executing, we will do new things and 
record them, instead of just replaying what was already done'.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change message when reaching end of reverse history.
  2024-04-04 12:16               ` Guinevere Larsen
@ 2024-04-04 12:39                 ` Eli Zaretskii
  2024-04-04 17:26                   ` Guinevere Larsen
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2024-04-04 12:39 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: markus.t.metzger, gdb-patches, achronop

> Date: Thu, 4 Apr 2024 09:16:46 -0300
> Cc: gdb-patches@sourceware.org, achronop@gmail.com
> From: Guinevere Larsen <blarsen@redhat.com>
> 
> > If the above makes no sense, would someone please explain in more
> > detail what was the original issue, and I will try to come up with a
> > better suggestion.
> >
> The original issue is that when a user is in replay mode going forward, 
> and they hit the message "no more reverse-execution history", some 
> interpret that message as 'you can no longer execute forward' instead of 
> the intended 'if you continue executing, we will do new things and 
> record them, instead of just replaying what was already done'.

Then how about this instead:

  End of recorded history; following steps will be added to history.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change message when reaching end of reverse history.
  2024-04-04 12:39                 ` Eli Zaretskii
@ 2024-04-04 17:26                   ` Guinevere Larsen
  2024-04-04 19:55                     ` Alex Chronopoulos
  0 siblings, 1 reply; 16+ messages in thread
From: Guinevere Larsen @ 2024-04-04 17:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: markus.t.metzger, gdb-patches, achronop

On 4/4/24 09:39, Eli Zaretskii wrote:
>> Date: Thu, 4 Apr 2024 09:16:46 -0300
>> Cc: gdb-patches@sourceware.org, achronop@gmail.com
>> From: Guinevere Larsen <blarsen@redhat.com>
>>
>>> If the above makes no sense, would someone please explain in more
>>> detail what was the original issue, and I will try to come up with a
>>> better suggestion.
>>>
>> The original issue is that when a user is in replay mode going forward,
>> and they hit the message "no more reverse-execution history", some
>> interpret that message as 'you can no longer execute forward' instead of
>> the intended 'if you continue executing, we will do new things and
>> record them, instead of just replaying what was already done'.
> Then how about this instead:
>
>    End of recorded history; following steps will be added to history.
>
Sounds good to me. Markus, Alex, do you agree?

-- 
Cheers,
Guinevere Larsen
She/Her/Hers


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change message when reaching end of reverse history.
  2024-04-04 17:26                   ` Guinevere Larsen
@ 2024-04-04 19:55                     ` Alex Chronopoulos
  2024-04-05  5:18                       ` Metzger, Markus T
  0 siblings, 1 reply; 16+ messages in thread
From: Alex Chronopoulos @ 2024-04-04 19:55 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: Eli Zaretskii, Metzger, Markus T, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]

Absolutely, thank you both.

My understanding is that the backward message remains the same. If not let
me know. Otherwise will update the patch.

On Thu, Apr 4, 2024, 19:26 Guinevere Larsen <blarsen@redhat.com> wrote:

> On 4/4/24 09:39, Eli Zaretskii wrote:
> >> Date: Thu, 4 Apr 2024 09:16:46 -0300
> >> Cc: gdb-patches@sourceware.org, achronop@gmail.com
> >> From: Guinevere Larsen <blarsen@redhat.com>
> >>
> >>> If the above makes no sense, would someone please explain in more
> >>> detail what was the original issue, and I will try to come up with a
> >>> better suggestion.
> >>>
> >> The original issue is that when a user is in replay mode going forward,
> >> and they hit the message "no more reverse-execution history", some
> >> interpret that message as 'you can no longer execute forward' instead of
> >> the intended 'if you continue executing, we will do new things and
> >> record them, instead of just replaying what was already done'.
> > Then how about this instead:
> >
> >    End of recorded history; following steps will be added to history.
> >
> Sounds good to me. Markus, Alex, do you agree?
>
> --
> Cheers,
> Guinevere Larsen
> She/Her/Hers
>
>

[-- Attachment #2: Type: text/html, Size: 1921 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] Change message when reaching end of reverse history.
  2024-04-04 19:55                     ` Alex Chronopoulos
@ 2024-04-05  5:18                       ` Metzger, Markus T
  2024-04-05 15:43                         ` Guinevere Larsen
  0 siblings, 1 reply; 16+ messages in thread
From: Metzger, Markus T @ 2024-04-05  5:18 UTC (permalink / raw)
  To: Alex Chronopoulos, Guinevere Larsen; +Cc: Eli Zaretskii, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2112 bytes --]

Looks good to me, too.  Please make sure to update tests that check for the old string.

That leaves the question whether we need an indication for MI, too.

Regards,
Markus.

From: Alex Chronopoulos <achronop@gmail.com>
Sent: Thursday, April 4, 2024 9:55 PM
To: Guinevere Larsen <blarsen@redhat.com>
Cc: Eli Zaretskii <eliz@gnu.org>; Metzger, Markus T <markus.t.metzger@intel.com>; gdb-patches@sourceware.org
Subject: Re: [PATCH] Change message when reaching end of reverse history.

Absolutely, thank you both.

My understanding is that the backward message remains the same. If not let me know. Otherwise will update the patch.

On Thu, Apr 4, 2024, 19:26 Guinevere Larsen <blarsen@redhat.com<mailto:blarsen@redhat.com>> wrote:
On 4/4/24 09:39, Eli Zaretskii wrote:
>> Date: Thu, 4 Apr 2024 09:16:46 -0300
>> Cc: gdb-patches@sourceware.org<mailto:gdb-patches@sourceware.org>, achronop@gmail.com<mailto:achronop@gmail.com>
>> From: Guinevere Larsen <blarsen@redhat.com<mailto:blarsen@redhat.com>>
>>
>>> If the above makes no sense, would someone please explain in more
>>> detail what was the original issue, and I will try to come up with a
>>> better suggestion.
>>>
>> The original issue is that when a user is in replay mode going forward,
>> and they hit the message "no more reverse-execution history", some
>> interpret that message as 'you can no longer execute forward' instead of
>> the intended 'if you continue executing, we will do new things and
>> record them, instead of just replaying what was already done'.
> Then how about this instead:
>
>    End of recorded history; following steps will be added to history.
>
Sounds good to me. Markus, Alex, do you agree?

--
Cheers,
Guinevere Larsen
She/Her/Hers
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 Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[-- Attachment #2: Type: text/html, Size: 4950 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change message when reaching end of reverse history.
  2024-04-05  5:18                       ` Metzger, Markus T
@ 2024-04-05 15:43                         ` Guinevere Larsen
  0 siblings, 0 replies; 16+ messages in thread
From: Guinevere Larsen @ 2024-04-05 15:43 UTC (permalink / raw)
  To: Metzger, Markus T, Alex Chronopoulos; +Cc: Eli Zaretskii, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2560 bytes --]

I think it would be good to have an indication for MI too, but I don't 
think it has to be tied to the acceptance of this patch due to the 
difference in how MI reports (only saying 'reason: no-history'). Once 
this patch is in, I'll open a bug where we can discuss how to do it.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

On 4/5/24 02:18, Metzger, Markus T wrote:
>
> Looks good to me, too.  Please make sure to update tests that check 
> for the old string.
>
> That leaves the question whether we need an indication for MI, too.
>
> Regards,
>
> Markus.
>
> *From:* Alex Chronopoulos <achronop@gmail.com>
> *Sent:* Thursday, April 4, 2024 9:55 PM
> *To:* Guinevere Larsen <blarsen@redhat.com>
> *Cc:* Eli Zaretskii <eliz@gnu.org>; Metzger, Markus T 
> <markus.t.metzger@intel.com>; gdb-patches@sourceware.org
> *Subject:* Re: [PATCH] Change message when reaching end of reverse 
> history.
>
> Absolutely, thank you both.
>
> My understanding is that the backward message remains the same. If not 
> let me know. Otherwise will update the patch.
>
> On Thu, Apr 4, 2024, 19:26 Guinevere Larsen <blarsen@redhat.com> wrote:
>
>     On 4/4/24 09:39, Eli Zaretskii wrote:
>     >> Date: Thu, 4 Apr 2024 09:16:46 -0300
>     >> Cc: gdb-patches@sourceware.org, achronop@gmail.com
>     >> From: Guinevere Larsen <blarsen@redhat.com>
>     >>
>     >>> If the above makes no sense, would someone please explain in more
>     >>> detail what was the original issue, and I will try to come up
>     with a
>     >>> better suggestion.
>     >>>
>     >> The original issue is that when a user is in replay mode going
>     forward,
>     >> and they hit the message "no more reverse-execution history", some
>     >> interpret that message as 'you can no longer execute forward'
>     instead of
>     >> the intended 'if you continue executing, we will do new things and
>     >> record them, instead of just replaying what was already done'.
>     > Then how about this instead:
>     >
>     >    End of recorded history; following steps will be added to
>     history.
>     >
>     Sounds good to me. Markus, Alex, do you agree?
>
>     -- 
>     Cheers,
>     Guinevere Larsen
>     She/Her/Hers
>
> 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 Silva
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
>

[-- Attachment #2: Type: text/html, Size: 7063 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-04-05 15:44 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-13 20:48 [PATCH] Change message when reaching end of reverse history Alex Chronopoulos
2024-03-14  6:37 ` Metzger, Markus T
2024-03-14 10:26   ` Guinevere Larsen
2024-03-14 17:26     ` Alex Chronopoulos
2024-04-03 17:54       ` Guinevere Larsen
2024-04-03 21:15         ` Alex Chronopoulos
2024-04-04  6:22           ` Metzger, Markus T
2024-04-04  7:00             ` Eli Zaretskii
2024-04-04 12:16               ` Guinevere Larsen
2024-04-04 12:39                 ` Eli Zaretskii
2024-04-04 17:26                   ` Guinevere Larsen
2024-04-04 19:55                     ` Alex Chronopoulos
2024-04-05  5:18                       ` Metzger, Markus T
2024-04-05 15:43                         ` Guinevere Larsen
2024-03-14 14:53   ` Tom Tromey
2024-03-14 16:13     ` Metzger, Markus T

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox