Windows gdb+gdbserver: Move suspending thread to when returning event

The current code suspends a thread just before calling
GetThreadContext.  You can only call GetThreadContext if the thread is
suspended.  But, after WaitForDebugEvent, all threads are implicitly
suspended.  So I don't think we even needed to call SuspendThread
explictly at all before our GetThreadContext calls.

However, suspending threads when we're about to present a stop to gdb
simplifies adding non-stop support later.  This way, the windows
SuspendThread state corresponds to whether a thread is suspended or
resumed from the core's perspective.  Curiously, I noticed that Wine's
winedbg does something similar:
https://github.com/wine-mirror/wine/blob/234943344f7495d1e072338f0e06fa2d5cbf0aa1/programs/winedbg/gdbproxy.c#L651

This makes it much easier to reason about a thread's suspend state,
and simplifies adding non-stop mode later on.

Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ifd6889a8afc041fad33cd1c4500e38941da6781b
commit-id:c4d2c92e

diff --git a/gdb/aarch64-windows-nat.c b/gdb/aarch64-windows-nat.c
index d43ee6f13fe7603ab0fc5063cc769a52f72afd3c..871531bb93a4d8f2c2460cd4d5a8f25baf49b051 100644 (file)
--- a/gdb/aarch64-windows-nat.c
+++ b/gdb/aarch64-windows-nat.c
@@ -185,7 +185,6 @@ aarch64_windows_nat_target::fill_thread_context (windows_thread_info *th)
 
   if (context->ContextFlags == 0)
     {
-      th->suspend ();
       context->ContextFlags = WindowsContext<decltype(context)>::all;
       CHECK (get_thread_context (th->h, context));
     }

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 30b07221b623ee4dcc3e20676691da829d643c6b..468f451595d230b3ac377147969b823e4fa0eefd 100644 (file)
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -710,12 +710,6 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
 
        th->resume ();
       }
-    else
-      {
-       /* When single-stepping a specific thread, other threads must
-          be suspended.  */
-       th->suspend ();
-      }
 
   std::optional<unsigned> err;
   do_synchronously ([&] ()
@@ -1191,6 +1185,11 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
                  th->stopped_at_software_breakpoint = true;
                  th->pc_adjusted = false;
                }
+
+             /* All-stop, suspend all threads until they are
+                explicitly resumed.  */
+             for (auto &thr : windows_process.thread_list)
+               thr->suspend ();
            }
 
          return result;

diff --git a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c
index 5b0f7066fba29eae90c06c7ecfaca67634b5057b..baa6b969e99a41dd87f17f7fcbaa3dbaec434610 100644 (file)
--- a/gdb/x86-windows-nat.c
+++ b/gdb/x86-windows-nat.c
@@ -111,7 +111,6 @@ x86_windows_nat_target::fill_thread_context (windows_thread_info *th)
     {
       if (context->ContextFlags == 0)
        {
-         th->suspend ();
          context->ContextFlags = WindowsContext<decltype(context)>::all;
          CHECK (get_thread_context (th->h, context));
        }

diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index bf55183f705c41e96217b074560e02395435aadc..d78de17ff195e656c9ce2efa5c0670b8b29bf78e 100644 (file)
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -1188,6 +1188,11 @@ win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
            OUTMSG2 (("Child Stopped with signal = %d \n",
                      ourstatus->sig ()));
            maybe_adjust_pc ();
+
+           /* All-stop, suspend all threads until they are explicitly
+              resumed.  */
+           for_each_thread (suspend_one_thread);
+
            return debug_event_ptid (&windows_process.current_event);
          }
        default: