Windows gdb: all-stop, interrupt with "stopped" instead of SIGTRAP

Message ID 20260505122434.1444507-1-pedro@palves.net
State New
Headers
Series Windows gdb: all-stop, interrupt with "stopped" instead of SIGTRAP |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm fail Patch failed to apply

Commit Message

Pedro Alves May 5, 2026, 12:24 p.m. UTC
  Currently, "interrupt" uses DebugBreakProcess (or equivalent), which
injects a new thread in the inferior that executes an int3 instruction
(on x86), which raises a SIGTRAP.  With the target backend in non-stop
mode, its easy to avoid all that and make interrupt in all-stop just
suspend a pre-existing thread and report "stopped", like "set non-stop
on" does (via target_stop).

E.g., before:

 (gdb) c&
 Continuing.
 ...
 (gdb) info threads
   Id   Target Id          Frame
 * 1    Thread 1180.0x1374 (running)
   2    Thread 1180.0xc3c  (running)
   3    Thread 1180.0x6ec  (running)
   5    Thread 1180.0x1378 (running)
 (gdb)
 ...
 [New Thread 1180.0xc70]
 [Thread 1180.0x1378 exited with code 0]
 [Thread 1180.0xc70 exited with code 0]
 [New Thread 1180.0xdc0]
 ...
 interrupt
 (gdb) [New Thread 1180.0x1010]

 Thread 8 received signal SIGTRAP, Trace/breakpoint trap.
 [Switching to Thread 1180.0x1010]
 0x00007ffa57490b71 in ntdll!DbgBreakPoint () from C:\Windows\SYSTEM32\ntdll.dll
 info threads
   Id   Target Id          Frame
   1    Thread 1180.0x1374 0x00007ffa5748d6e4 in ntdll!ZwDelayExecution () from C:\Windows\SYSTEM32\ntdll.dll
   2    Thread 1180.0xc3c  0x00007ffa57490ad4 in ntdll!ZwWaitForWorkViaWorkerFactory () from C:\Windows\SYSTEM32\ntdll.dll
   3    Thread 1180.0x6ec  0x00007ffa5748d6e4 in ntdll!ZwDelayExecution () from C:\Windows\SYSTEM32\ntdll.dll
   7    Thread 1180.0xdc0  0x00007ffa5748d6e4 in ntdll!ZwDelayExecution () from C:\Windows\SYSTEM32\ntdll.dll
 * 8    Thread 1180.0x1010 0x00007ffa57490b71 in ntdll!DbgBreakPoint () from C:\Windows\SYSTEM32\ntdll.dll
 (gdb)

After:

 (gdb) info threads
   Id   Target Id          Frame
 * 1    Thread 5912.0x394  (running)
   2    Thread 5912.0x608  (running)
   3    Thread 5912.0x1704 (running)
   10   Thread 5912.0x870  (running)
 (gdb)
 ...
 [Thread 5912.0x870 exited with code 0]
 ...
 (gdb) interrupt
 (gdb)
 Thread 1 stopped.
 0x00007ffa5748d6e4 in ntdll!ZwDelayExecution () from C:\Windows\SYSTEM32\ntdll.dll
 info threads
   Id   Target Id          Frame
 * 1    Thread 5912.0x394  0x00007ffa5748d6e4 in ntdll!ZwDelayExecution () from C:\Windows\SYSTEM32\ntdll.dll
   2    Thread 5912.0x608  0x00007ffa57490ad4 in ntdll!ZwWaitForWorkViaWorkerFactory () from C:\Windows\SYSTEM32\ntdll.dll
   3    Thread 5912.0x1704 0x00007ffa5748d6e4 in ntdll!ZwDelayExecution () from C:\Windows\SYSTEM32\ntdll.dll
   8    Thread 5912.0x1200 0x00007ffa5748d6e4 in ntdll!ZwDelayExecution () from C:\Windows\SYSTEM32\ntdll.dll
 (gdb)

Change-Id: I569fc69392ce9a070a2ebe1003388b7386412b14
commit-id: 8f6b76d2
---
 gdb/windows-nat.c | 68 +++++++++++++++++++++++++++++++++++++++--------
 gdb/windows-nat.h |  5 +++-
 2 files changed, 61 insertions(+), 12 deletions(-)


base-commit: 297fe552edd546e4fe9f47c7a0765a55ce084f0c
  

Comments

Tom Tromey May 5, 2026, 2:17 p.m. UTC | #1
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> Currently, "interrupt" uses DebugBreakProcess (or equivalent), which
Pedro> injects a new thread in the inferior that executes an int3 instruction
Pedro> (on x86), which raises a SIGTRAP.  With the target backend in non-stop
Pedro> mode, its easy to avoid all that and make interrupt in all-stop just
Pedro> suspend a pre-existing thread and report "stopped", like "set non-stop
Pedro> on" does (via target_stop).

Looks reasonable to me, thanks.
Approved-By: Tom Tromey <tom@tromey.com>

Pedro> +void
Pedro> +windows_nat_target::stop (ptid_t ptid)
Pedro> +{
Pedro> +  stop_interrupt (ptid, false);
Pedro> +}

I guess this didn't land:

https://sourceware.org/pipermail/gdb-patches/2026-April/226456.html

but OTOH I think your implementation is more precise.

thanks,
Tom
  
Pedro Alves May 7, 2026, 6:44 p.m. UTC | #2
On 2026-05-05 15:17, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> Currently, "interrupt" uses DebugBreakProcess (or equivalent), which
> Pedro> injects a new thread in the inferior that executes an int3 instruction
> Pedro> (on x86), which raises a SIGTRAP.  With the target backend in non-stop
> Pedro> mode, its easy to avoid all that and make interrupt in all-stop just
> Pedro> suspend a pre-existing thread and report "stopped", like "set non-stop
> Pedro> on" does (via target_stop).
> 
> Looks reasonable to me, thanks.
> Approved-By: Tom Tromey <tom@tromey.com>
> 

Thanks, merged.

> Pedro> +void
> Pedro> +windows_nat_target::stop (ptid_t ptid)
> Pedro> +{
> Pedro> +  stop_interrupt (ptid, false);
> Pedro> +}
> 
> I guess this didn't land:
> 
> https://sourceware.org/pipermail/gdb-patches/2026-April/226456.html
> 

Oh, I hadn't seen that.  (If it isn't obvious, I haven't been able to keep up with the
list in the past year or so.  I'm hoping that will change soon-ish.)

I had been sitting on this patch until non-stop was in.  Apparently for exactly 3 years! :-P

 commit 88ae0d8325ced6d62781f1838e2bfa6195866929
 Author:     Pedro Alves <pedro@palves.net>
 AuthorDate: Fri May 5 15:51:31 2023 +0100
 Commit:     Pedro Alves <pedro@palves.net>
 CommitDate: Thu May 7 19:32:35 2026 +0100

> but OTOH I think your implementation is more precise.

Yeah, that one ignores the ptid argument completely, and would just make target_stop inject
the magic ctrl-c thread, which is not what the semantics of target_stop should be.

Note "windows_nat_target::stop(ptid_t ptid)" already existed, it was added by the
non-stop support.  This patch is refactoring it to be usable from the interrupt() context.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index a9647e90bb8..b9d32b6c0c4 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1177,6 +1177,30 @@  void
 windows_nat_target::interrupt ()
 {
   DEBUG_EVENTS ("interrupt");
+
+  if (target_is_non_stop_p ())
+    {
+      /* Since we have finer-grained control and can suspend threads,
+	 we can report a "stopped" event for an existing thread,
+	 instead of force-injecting a new thread that reports SIGTRAP
+	 with DebugBreakProcess.
+
+	 Stop one thread, any thread.  */
+      stop_interrupt (minus_one_ptid, true);
+      return;
+    }
+
+  if (!dbg_break_process ())
+    warning (_("Could not interrupt program.  "
+	       "Press Ctrl-c in the program console."));
+}
+
+/* Stop the process with DebugBreakProcess or equivalent.  Return true
+   on success, false otherwise.  */
+
+bool
+windows_nat_target::dbg_break_process ()
+{
 #ifdef __x86_64__
   if (windows_process->wow64_process)
     {
@@ -1200,23 +1224,25 @@  windows_nat_target::interrupt ()
 	  if (thread)
 	    {
 	      CloseHandle (thread);
-	      return;
+	      return true;
 	    }
 	}
     }
   else
 #endif
     if (DebugBreakProcess (windows_process->handle))
-      return;
-  warning (_("Could not interrupt program.  "
-	     "Press Ctrl-c in the program console."));
+      return true;
+
+  return false;
 }
 
 /* Stop thread TH, for STOPPING_KIND reason.  This leaves a
    GDB_SIGNAL_0 pending in the thread, which is later consumed by
-   windows_nat_target::wait.  */
+   windows_nat_target::wait.  Return true if TH gets suspended and now
+   has a new stop event to report; false if TH was already suspended
+   and has no new stop event.  */
 
-void
+bool
 windows_nat_target::stop_one_thread (windows_thread_info *th,
 				     enum stopping_kind stopping_kind)
 {
@@ -1230,6 +1256,7 @@  windows_nat_target::stop_one_thread (windows_thread_info *th,
       DEBUG_EVENTS ("already suspended %s: suspended=%d, stopping=%d",
 		    thr_ptid.to_string ().c_str (),
 		    th->suspended, th->stopping);
+      return false;
     }
 #ifdef __CYGWIN__
   else if (th->suspended
@@ -1249,6 +1276,7 @@  windows_nat_target::stop_one_thread (windows_thread_info *th,
       th->pending_status.set_stopped (GDB_SIGNAL_0);
       th->last_event = {};
       serial_event_set (m_wait_event);
+      return true;
     }
 #endif
   else if (th->suspended)
@@ -1262,6 +1290,7 @@  windows_nat_target::stop_one_thread (windows_thread_info *th,
       /* Upgrade stopping.  */
       if (stopping_kind > th->stopping)
 	th->stopping = stopping_kind;
+      return false;
     }
   else
     {
@@ -1278,7 +1307,7 @@  windows_nat_target::stop_one_thread (windows_thread_info *th,
 			thr_ptid.to_string ().c_str ());
 	  if (stopping_kind > th->stopping)
 	    th->stopping = stopping_kind;
-	  return;
+	  return false;
 	}
 
       gdb_assert (th->suspended == 1);
@@ -1291,21 +1320,38 @@  windows_nat_target::stop_one_thread (windows_thread_info *th,
 	}
 
       serial_event_set (m_wait_event);
+      return true;
     }
 }
 
-/* Implementation of target_ops::stop.  */
+/* Helper for windows_nat_target::stop and
+   windows_nat_target::interrupt.  Stops PTID.  If STOP_ON_FIRST_MATCH
+   is true, returns immediately as soon as one thread is stopped.  */
 
 void
-windows_nat_target::stop (ptid_t ptid)
+windows_nat_target::stop_interrupt (ptid_t ptid, bool stop_on_first_match)
 {
   for (thread_info &thr : all_non_exited_threads (this))
     {
-      if (thr.ptid.matches (ptid))
-	stop_one_thread (as_windows_thread_info (&thr), SK_EXTERNAL);
+      if (!thr.ptid.matches (ptid))
+	continue;
+
+      if (stop_one_thread (as_windows_thread_info (&thr), SK_EXTERNAL))
+	{
+	  if (stop_on_first_match)
+	    return;
+	}
     }
 }
 
+/* Implementation of target_ops::stop.  */
+
+void
+windows_nat_target::stop (ptid_t ptid)
+{
+  stop_interrupt (ptid, false);
+}
+
 void
 windows_nat_target::pass_ctrlc ()
 {
diff --git a/gdb/windows-nat.h b/gdb/windows-nat.h
index 1f7ecb07e4f..8e6e79a8160 100644
--- a/gdb/windows-nat.h
+++ b/gdb/windows-nat.h
@@ -312,9 +312,12 @@  struct windows_nat_target : public inf_child_target
   void delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p);
   DWORD fake_create_process (const DEBUG_EVENT &current_event);
 
-  void stop_one_thread (windows_thread_info *th,
+  bool stop_one_thread (windows_thread_info *th,
 			enum windows_nat::stopping_kind stopping_kind);
 
+  void stop_interrupt (ptid_t ptid, bool stop_on_first_match);
+  bool dbg_break_process ();
+
   DWORD continue_status_for_event_detaching
     (const DEBUG_EVENT &event, size_t *reply_later_events_left = nullptr);