[PATCH v3] Change message when reaching end of reverse history.

[PATCH v3] Change message when reaching end of reverse history.

[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:

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

The reason for this change is that the initial message was deceiving, for
the forward case, 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..c0f037c11a2 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 ("\nEnd of recorded history; following steps will be added to "
+		 "history.\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..a57cfec62be 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 \
+	     "End of recorded history; following steps will be added to history\." \
+	     "\[^\\\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..dc7666e2f1a 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 -wrap "End of recorded history; following steps.* end of main .*" {
 	    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 -wrap "End of recorded history; following steps.* end of main .*" {
 	    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..28edddf3517 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 -wrap "End of recorded history; following steps will.* end of main .*" {
 	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..3d004cf3fee 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 -wrap "End of recorded history; following steps will be.* end main .*" {
 	pass "go to end of main forward"
     }
 }
-- 
2.42.0

Re: [PATCH v3] Change message when reaching end of reverse history.

[Eli Zaretskii]

> From: Alex Chronopoulos <achronop@gmail.com>
> Cc: Alex Chronopoulos <achronop@gmail.com>
> Date: Sun, 14 Apr 2024 21:36:53 +0200
> 
> 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:
> 
> "End of recorded history; following steps will be added to history."
> 
> The reason for this change is that the initial message was deceiving, for
> the forward case, 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(-)

Thanks, the NEWS part is approved.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Re: [PATCH v3] Change message when reaching end of reverse history.

[Guinevere Larsen]

On 4/14/24 16:36, Alex Chronopoulos wrote:
> 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:
>
> "End of recorded history; following steps will be added to history."
>
> The reason for this change is that the initial message was deceiving, for
> the forward case, making the user believe that forward debugging could not
> continue.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31224

Sorry about the delay in getting this review. I'm fine with this. 
Reviewed-By: Guinevere Larsen <blarsen@redhat.com>

Hopefully Markus gives his OK soon.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

> ---
>   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..c0f037c11a2 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 ("\nEnd of recorded history; following steps will be added to "
> +		 "history.\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..a57cfec62be 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 \
> +	     "End of recorded history; following steps will be added to history\." \
> +	     "\[^\\\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..dc7666e2f1a 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 -wrap "End of recorded history; following steps.* end of main .*" {
>   	    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 -wrap "End of recorded history; following steps.* end of main .*" {
>   	    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..28edddf3517 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 -wrap "End of recorded history; following steps will.* end of main .*" {
>   	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..3d004cf3fee 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 -wrap "End of recorded history; following steps will be.* end main .*" {
>   	pass "go to end of main forward"
>       }
>   }

RE: [PATCH v3] Change message when reaching end of reverse history.

[Metzger, Markus T]

>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31224
>
>Sorry about the delay in getting this review. I'm fine with this.
>Reviewed-By: Guinevere Larsen <blarsen@redhat.com>
>
>Hopefully Markus gives his OK soon.

I already OK'ed v2.

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

Re: [PATCH v3] Change message when reaching end of reverse history.

[Guinevere Larsen]

On 4/23/24 11:03, Metzger, Markus T wrote:
>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31224
>> Sorry about the delay in getting this review. I'm fine with this.
>> Reviewed-By: Guinevere Larsen <blarsen@redhat.com>
>>
>> Hopefully Markus gives his OK soon.
> I already OK'ed v2.

Oh, ok. I missed that in the review.

Alex, feel free to change my review tag to an approval tag. Once 
copyright goes through, we can work on getting you write-after-approval 
access to push this!

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

Re: [PATCH v3] Change message when reaching end of reverse history.

[Pedro Alves]

Sorry for not chiming in earlier...

On 2024-04-14 20:36, Alex Chronopoulos wrote:
> 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:
> 
> "End of recorded history; following steps will be added to history."
> 

IMO, "steps" here is confusing.  It's ambiguous with stepping.  Like as if
you're saying that the following "step" commands will be added to history.
"But what about if I continue??"

It also isn't true that "following steps will be added to history.".  If
the user does "reverse-continue" for example, they won't, they're
already there...

The following tweak would be more accurate and not have that "step" confusion, IMO:

 "End of recorded history; following forward execution will be added to history."

and it's still under 80 chars.

Except it fells a bit awkward, for not stating that we're stopping before
talking about following execution.  This would be clearer to me:

  Reached end of recorded history; stopping.
  Following forward execution will be added to history.


Also, with the patch, we have these two messages, for the forward case:

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

and for the reverse case:

 No more reverse-execution history.

I read the v1/v2 discussions, and I have to say that I don't understand how
the potential user confusion that led to changing the "No more reverse-execution"
wording in the forward case doesn't apply to the reverse case...  I think
we should be consistent.  

With my suggestion above, we could have:

forward:

  Reached end of recorded history; stopping.
  Following forward execution will be added to history.

backward:

  Reached end of recorded history; stopping.

or, backward:

  Reached end of recorded history; stopping.
  Backward execution from here not possible.

Re: [PATCH v3] Change message when reaching end of reverse history.

[Alex Chronopoulos]

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

Thank you, Pedro. No worries, it's still early enough :)

I like your suggestions and would happily follow them. I believe they are
clear and leave fewer questions for the user.

I also prefer the extended version for the backward case. However, I don't
want to make the final call. I'll wait for others to comment, and I'll
update the patch when we have the final version.


On Tue, Apr 23, 2024 at 10:36 PM Pedro Alves <pedro@palves.net> wrote:

> Sorry for not chiming in earlier...
>
> On 2024-04-14 20:36, Alex Chronopoulos wrote:
> > 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:
> >
> > "End of recorded history; following steps will be added to history."
> >
>
> IMO, "steps" here is confusing.  It's ambiguous with stepping.  Like as if
> you're saying that the following "step" commands will be added to history.
> "But what about if I continue??"
>
> It also isn't true that "following steps will be added to history.".  If
> the user does "reverse-continue" for example, they won't, they're
> already there...
>
> The following tweak would be more accurate and not have that "step"
> confusion, IMO:
>
>  "End of recorded history; following forward execution will be added to
> history."
>
> and it's still under 80 chars.
>
> Except it fells a bit awkward, for not stating that we're stopping before
> talking about following execution.  This would be clearer to me:
>
>   Reached end of recorded history; stopping.
>   Following forward execution will be added to history.
>
>
> Also, with the patch, we have these two messages, for the forward case:
>
>  End of recorded history; following steps will be added to history.
>
> and for the reverse case:
>
>  No more reverse-execution history.
>
> I read the v1/v2 discussions, and I have to say that I don't understand how
> the potential user confusion that led to changing the "No more
> reverse-execution"
> wording in the forward case doesn't apply to the reverse case...  I think
> we should be consistent.
>
> With my suggestion above, we could have:
>
> forward:
>
>   Reached end of recorded history; stopping.
>   Following forward execution will be added to history.
>
> backward:
>
>   Reached end of recorded history; stopping.
>
> or, backward:
>
>   Reached end of recorded history; stopping.
>   Backward execution from here not possible.
>
>

RE: [PATCH v3] Change message when reaching end of reverse history.

[Metzger, Markus T]

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

I’d prefer compact, one-line (error) messages.  A longer explanation can go into the manual or in the help text.  E.g.

    Stopping replaying at end of execution history

and

    Stopping at beginning of execution history

Markus.

From: Alex Chronopoulos <achronop@gmail.com>
Sent: Friday, May 3, 2024 7:17 PM
To: gdb-patches@sourceware.org
Cc: Pedro Alves <pedro@palves.net>; Metzger, Markus T <markus.t.metzger@intel.com>; Guinevere Larsen <blarsen@redhat.com>
Subject: Re: [PATCH v3] Change message when reaching end of reverse history.


Thank you, Pedro. No worries, it's still early enough :)

I like your suggestions and would happily follow them. I believe they are clear and leave fewer questions for the user.

I also prefer the extended version for the backward case. However, I don't want to make the final call. I'll wait for others to comment, and I'll update the patch when we have the final version.


On Tue, Apr 23, 2024 at 10:36 PM Pedro Alves <pedro@palves.net<mailto:pedro@palves.net>> wrote:
Sorry for not chiming in earlier...

On 2024-04-14 20:36, Alex Chronopoulos wrote:
> 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:
>
> "End of recorded history; following steps will be added to history."
>

IMO, "steps" here is confusing.  It's ambiguous with stepping.  Like as if
you're saying that the following "step" commands will be added to history.
"But what about if I continue??"

It also isn't true that "following steps will be added to history.".  If
the user does "reverse-continue" for example, they won't, they're
already there...

The following tweak would be more accurate and not have that "step" confusion, IMO:

 "End of recorded history; following forward execution will be added to history."

and it's still under 80 chars.

Except it fells a bit awkward, for not stating that we're stopping before
talking about following execution.  This would be clearer to me:

  Reached end of recorded history; stopping.
  Following forward execution will be added to history.


Also, with the patch, we have these two messages, for the forward case:

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

and for the reverse case:

 No more reverse-execution history.

I read the v1/v2 discussions, and I have to say that I don't understand how
the potential user confusion that led to changing the "No more reverse-execution"
wording in the forward case doesn't apply to the reverse case...  I think
we should be consistent.

With my suggestion above, we could have:

forward:

  Reached end of recorded history; stopping.
  Following forward execution will be added to history.

backward:

  Reached end of recorded history; stopping.

or, backward:

  Reached end of recorded history; stopping.
  Backward execution from here not possible.
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

Re: [PATCH v3] Change message when reaching end of reverse history.

[Guinevere Larsen]

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

On 5/6/24 02:19, Metzger, Markus T wrote:
>
> I’d prefer compact, one-line (error) messages.  A longer explanation 
> can go into the manual or in the help text.  E.g.
>
>     Stopping replaying at end of execution history
>
This simple message brings us all the way back around to the original 
problem. It's not obvious to a new user that they can keep going and 
things will continue to be recorded.

If you really want something very short, a "check 'help record'" hint 
and some text explaining that forward execution can always be resumed 
(if using record full or record btrace), and if you're executing further 
new forward execution is added to history, that's ok too.

I don't think the manual is enough, if nothing else, because new users 
are unlikely to find and read the manual in my experience.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

> and
>
>     Stopping at beginning of execution history
>
> Markus.
>
> *From:* Alex Chronopoulos <achronop@gmail.com>
> *Sent:* Friday, May 3, 2024 7:17 PM
> *To:* gdb-patches@sourceware.org
> *Cc:* Pedro Alves <pedro@palves.net>; Metzger, Markus T 
> <markus.t.metzger@intel.com>; Guinevere Larsen <blarsen@redhat.com>
> *Subject:* Re: [PATCH v3] Change message when reaching end of reverse 
> history.
>
> Thank you, Pedro. No worries, it's still early enough :)
>
> I like your suggestions and would happily follow them. I believe they 
> are clear and leave fewer questions for the user.
>
> I also prefer the extended version for the backward case. However, I 
> don't want to make the final call. I'll wait for others to comment, 
> and I'll update the patch when we have the final version.
>
> On Tue, Apr 23, 2024 at 10:36 PM Pedro Alves <pedro@palves.net> wrote:
>
>     Sorry for not chiming in earlier...
>
>     On 2024-04-14 20:36, Alex Chronopoulos wrote:
>     > 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:
>     >
>     > "End of recorded history; following steps will be added to history."
>     >
>
>     IMO, "steps" here is confusing.  It's ambiguous with stepping. 
>     Like as if
>     you're saying that the following "step" commands will be added to
>     history.
>     "But what about if I continue??"
>
>     It also isn't true that "following steps will be added to
>     history.".  If
>     the user does "reverse-continue" for example, they won't, they're
>     already there...
>
>     The following tweak would be more accurate and not have that
>     "step" confusion, IMO:
>
>      "End of recorded history; following forward execution will be
>     added to history."
>
>     and it's still under 80 chars.
>
>     Except it fells a bit awkward, for not stating that we're stopping
>     before
>     talking about following execution.  This would be clearer to me:
>
>       Reached end of recorded history; stopping.
>       Following forward execution will be added to history.
>
>
>     Also, with the patch, we have these two messages, for the forward
>     case:
>
>      End of recorded history; following steps will be added to history.
>
>     and for the reverse case:
>
>      No more reverse-execution history.
>
>     I read the v1/v2 discussions, and I have to say that I don't
>     understand how
>     the potential user confusion that led to changing the "No more
>     reverse-execution"
>     wording in the forward case doesn't apply to the reverse case... 
>     I think
>     we should be consistent.
>
>     With my suggestion above, we could have:
>
>     forward:
>
>       Reached end of recorded history; stopping.
>       Following forward execution will be added to history.
>
>     backward:
>
>       Reached end of recorded history; stopping.
>
>     or, backward:
>
>       Reached end of recorded history; stopping.
>       Backward execution from here not possible.
>
> 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
>