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

[Guinevere Larsen <blarsen@redhat.com>]

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