[PATCHv3,11/13] gdb/remote: avoid SIGINT after calling remote_target::stop

Message ID 97a59b0b300f2510c40237c129d26e302fe702da.1675185990.git.aburgess@redhat.com
State New
Headers
Series Infcalls from B/P conditions in multi-threaded inferiors |

Commit Message

Andrew Burgess Jan. 31, 2023, 5:27 p.m. UTC
  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 comparing non-stop vs all-stop mode.

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 making use of thread_info::stop_requested
within remote.c to know if the stop was triggered by GDB (and the
SIGINT should be hidden) or if the stop was a user interrupt, and the
SIGINT should be printed.

In remote_target::process_stop_reply if the inferior stopped with
SIGINT and the thread_info::stop_requested flag is set, then we change
the stop signal to GDB_SIGNAL_0.

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.

This commit is the reason why the previous commit took care to set the
thread_info::stop_requested flag in infcall.c.
---
 gdb/remote.c                                           | 10 ++++++++++
 gdb/testsuite/gdb.base/infcall-timeout.exp             |  9 +--------
 .../gdb.threads/infcall-from-bp-cond-timeout.exp       |  9 +--------
 3 files changed, 12 insertions(+), 16 deletions(-)
  

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index ba7a7520cb4..604d75b9689 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -8275,6 +8275,16 @@  remote_target::process_stop_reply (struct stop_reply *stop_reply,
 	}
       else
 	{
+	  /* If this stop was actually requested by GDB then we can hide
+	     the SIGINT from the user.  */
+	  if (status->kind () == TARGET_WAITKIND_STOPPED
+	      && status->sig () == GDB_SIGNAL_INT)
+	    {
+	      thread_info *thr = find_thread_ptid (this, ptid);
+	      if (thr->stop_requested)
+		status->set_stopped (GDB_SIGNAL_0);
+	    }
+
 	  /* 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\\." \