[PATCHv2,11/13] gdb/remote: avoid SIGINT after calling remote_target::stop
Commit Message
Currently, if the remote target is not running in non-stop mode, then,
when GDB calls remote_target::stop, we end up sending an interrupt
packet \x03 to the remote target.
If the user interrupts the inferior from the GDB prompt (e.g. by
typing Ctrl-c), then GDB calls remote_target::interrupt, which also
ends up sending the interrupt packet.
The problem here is that both of these mechanisms end up sending the
interrupt packet, which means, when the target stops with a SIGINT,
and this is reported back to GDB, we have no choice but to report this
to the user as a SIGINT stop event.
Now maybe this is the correct thing to do, after all the target has
been stopped with SIGINT. However, this leads to an unfortunate
change in behaviour.
When running in non-stop mode, and remote_target::stop is called, the
target will be stopped with a vCont packet, and this stop is then
reported back to GDB as GDB_SIGNAL_0, this will cause GDB to print a
message like:
Program stopped.
Or:
Thread NN "binary name" stopped.
In contrast, when non-stop mode is off, we get messages like:
Program received SIGINT, Segmentation fault.
Or:
Thread NN "binary name" received SIGINT, Segmentation fault.
In this commit I propose a mechanism where we can track that a stop
has been requested for a particular thread through
remote_target::stop, then, when the stop arrives, we can convert the
SIGINT to a GDB_SIGNAL_0. With this done GDB will now display the
"stopped" based messages rather than the "received SIGINT" messages.
Two of the tests added in the previous commit exposed this issue. In
the previous commit the tests looked for either of the above
patterns. In this commit I've updated these tests to only look for
the "stopped" based messages.
---
gdb/remote.c | 17 +++++++++++++++++
gdb/testsuite/gdb.base/infcall-timeout.exp | 9 +--------
.../infcall-from-bp-cond-timeout.exp | 9 +--------
3 files changed, 19 insertions(+), 16 deletions(-)
Comments
On Wednesday, January 18, 2023 5:18 PM, Andrew Burgess wrote:
> Currently, if the remote target is not running in non-stop mode, then,
> when GDB calls remote_target::stop, we end up sending an interrupt
> packet \x03 to the remote target.
>
> If the user interrupts the inferior from the GDB prompt (e.g. by
> typing Ctrl-c), then GDB calls remote_target::interrupt, which also
> ends up sending the interrupt packet.
>
> The problem here is that both of these mechanisms end up sending the
> interrupt packet, which means, when the target stops with a SIGINT,
> and this is reported back to GDB, we have no choice but to report this
> to the user as a SIGINT stop event.
>
> Now maybe this is the correct thing to do, after all the target has
> been stopped with SIGINT. However, this leads to an unfortunate
> change in behaviour.
>
> When running in non-stop mode, and remote_target::stop is called, the
> target will be stopped with a vCont packet, and this stop is then
> reported back to GDB as GDB_SIGNAL_0, this will cause GDB to print a
> message like:
>
> Program stopped.
>
> Or:
>
> Thread NN "binary name" stopped.
>
> In contrast, when non-stop mode is off, we get messages like:
>
> Program received SIGINT, Segmentation fault.
>
> Or:
>
> Thread NN "binary name" received SIGINT, Segmentation fault.
>
> In this commit I propose a mechanism where we can track that a stop
> has been requested for a particular thread through
> remote_target::stop, then, when the stop arrives, we can convert the
> SIGINT to a GDB_SIGNAL_0. With this done GDB will now display the
> "stopped" based messages rather than the "received SIGINT" messages.
>
> Two of the tests added in the previous commit exposed this issue. In
> the previous commit the tests looked for either of the above
> patterns. In this commit I've updated these tests to only look for
> the "stopped" based messages.
> ---
> gdb/remote.c | 17 +++++++++++++++++
> gdb/testsuite/gdb.base/infcall-timeout.exp | 9 +--------
> .../infcall-from-bp-cond-timeout.exp | 9 +--------
> 3 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 218bca30d04..61781a24820 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -1139,6 +1139,10 @@ struct remote_thread_info : public private_thread_info
> std::string name;
> int core = -1;
>
> + /* Only used when not in non-stop mode. Set to true when a stop is
> + requested for the thread. */
> + bool stop_requested = false;
The thread_info struct already has a `stop_requested` field. Why can't we use it?
> /* Thread handle, perhaps a pthread_t or thread_t value, stored as a
> sequence of bytes. */
> gdb::byte_vector thread_handle;
> @@ -7114,6 +7118,12 @@ remote_target::stop (ptid_t ptid)
> /* We don't currently have a way to transparently pause the
> remote target in all-stop mode. Interrupt it instead. */
> remote_interrupt_as ();
> +
> + /* Record that this thread's stop is a result of GDB asking for the
> + stop, rather than the user asking for an interrupt. We can use
> + this information to adjust the waitstatus when it arrives. */
> + remote_thread_info *remote_thr = get_remote_thread_info (this, ptid);
> + remote_thr->stop_requested = true;
> }
> }
>
> @@ -8097,9 +8107,16 @@ remote_target::process_stop_reply (struct stop_reply *stop_reply,
> /* If the target works in non-stop mode, a stop-reply indicates that
> only this thread stopped. */
> remote_thr->set_not_resumed ();
> + gdb_assert (!remote_thr->stop_requested);
> }
> else
> {
> + if (status->kind () == TARGET_WAITKIND_STOPPED
> + && status->sig () == GDB_SIGNAL_INT
> + && remote_thr->stop_requested)
> + status->set_stopped (GDB_SIGNAL_0);
> + remote_thr->stop_requested = false;
> +
> /* If the target works in all-stop mode, a stop-reply indicates that
> all the target's threads stopped. */
> for (thread_info *tp : all_non_exited_threads (this))
> diff --git a/gdb/testsuite/gdb.base/infcall-timeout.exp b/gdb/testsuite/gdb.base/infcall-
> timeout.exp
> index a5b0111ed04..bd6b2bfac3e 100644
> --- a/gdb/testsuite/gdb.base/infcall-timeout.exp
> +++ b/gdb/testsuite/gdb.base/infcall-timeout.exp
> @@ -45,16 +45,9 @@ proc_with_prefix run_test { target_async target_non_stop } {
>
> gdb_test_no_output "set direct-call-timeout 5"
>
> - # When non-stop mode is off we get slightly different output from GDB.
> - if { [gdb_is_remote_or_extended_remote_target] && $target_non_stop == "off" } {
> - set stopped_line_pattern "Program received signal SIGINT, Interrupt\\."
> - } else {
> - set stopped_line_pattern "Program stopped\\."
> - }
> -
> gdb_test "print function_that_never_returns ()" \
> [multi_line \
> - $stopped_line_pattern \
> + "Program stopped\\." \
> ".*" \
> "The program being debugged timed out while in a function called from GDB\\." \
> "GDB remains in the frame where the timeout occurred\\." \
> diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
> b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
> index 3341ff33f19..9ba38e6896a 100644
> --- a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
> +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp
> @@ -92,16 +92,9 @@ proc run_test { target_async target_non_stop other_thread_bp } {
> "get number for segfault breakpoint"]
> }
>
> - # When non-stop mode is off we get slightly different output from GDB.
> - if { [gdb_is_remote_or_extended_remote_target] && $target_non_stop == "off" } {
> - set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" received signal
> SIGINT, Interrupt\\."
> - } else {
> - set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\."
> - }
> -
> gdb_test "continue" \
> [multi_line \
> - $stopped_line_pattern \
> + "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\." \
> ".*" \
> "Error in testing condition for breakpoint ${bp_num}:" \
> "The program being debugged timed out while in a function called from GDB\\." \
> --
> 2.25.4
Regards
-Baris
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
@@ -1139,6 +1139,10 @@ struct remote_thread_info : public private_thread_info
std::string name;
int core = -1;
+ /* Only used when not in non-stop mode. Set to true when a stop is
+ requested for the thread. */
+ bool stop_requested = false;
+
/* Thread handle, perhaps a pthread_t or thread_t value, stored as a
sequence of bytes. */
gdb::byte_vector thread_handle;
@@ -7114,6 +7118,12 @@ remote_target::stop (ptid_t ptid)
/* We don't currently have a way to transparently pause the
remote target in all-stop mode. Interrupt it instead. */
remote_interrupt_as ();
+
+ /* Record that this thread's stop is a result of GDB asking for the
+ stop, rather than the user asking for an interrupt. We can use
+ this information to adjust the waitstatus when it arrives. */
+ remote_thread_info *remote_thr = get_remote_thread_info (this, ptid);
+ remote_thr->stop_requested = true;
}
}
@@ -8097,9 +8107,16 @@ remote_target::process_stop_reply (struct stop_reply *stop_reply,
/* If the target works in non-stop mode, a stop-reply indicates that
only this thread stopped. */
remote_thr->set_not_resumed ();
+ gdb_assert (!remote_thr->stop_requested);
}
else
{
+ if (status->kind () == TARGET_WAITKIND_STOPPED
+ && status->sig () == GDB_SIGNAL_INT
+ && remote_thr->stop_requested)
+ status->set_stopped (GDB_SIGNAL_0);
+ remote_thr->stop_requested = false;
+
/* If the target works in all-stop mode, a stop-reply indicates that
all the target's threads stopped. */
for (thread_info *tp : all_non_exited_threads (this))
@@ -45,16 +45,9 @@ proc_with_prefix run_test { target_async target_non_stop } {
gdb_test_no_output "set direct-call-timeout 5"
- # When non-stop mode is off we get slightly different output from GDB.
- if { [gdb_is_remote_or_extended_remote_target] && $target_non_stop == "off" } {
- set stopped_line_pattern "Program received signal SIGINT, Interrupt\\."
- } else {
- set stopped_line_pattern "Program stopped\\."
- }
-
gdb_test "print function_that_never_returns ()" \
[multi_line \
- $stopped_line_pattern \
+ "Program stopped\\." \
".*" \
"The program being debugged timed out while in a function called from GDB\\." \
"GDB remains in the frame where the timeout occurred\\." \
@@ -92,16 +92,9 @@ proc run_test { target_async target_non_stop other_thread_bp } {
"get number for segfault breakpoint"]
}
- # When non-stop mode is off we get slightly different output from GDB.
- if { [gdb_is_remote_or_extended_remote_target] && $target_non_stop == "off" } {
- set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" received signal SIGINT, Interrupt\\."
- } else {
- set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\."
- }
-
gdb_test "continue" \
[multi_line \
- $stopped_line_pattern \
+ "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\." \
".*" \
"Error in testing condition for breakpoint ${bp_num}:" \
"The program being debugged timed out while in a function called from GDB\\." \