[v3] Change message when reaching end of reverse history.
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
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(-)
Comments
> 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>
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.
>> 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
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
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.
@@ -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
@@ -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
@@ -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" \
@@ -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"
}
}
@@ -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"
}
}
@@ -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"
}
}