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:
234943344f/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.

Change-Id: Ifd6889a8afc041fad33cd1c4500e38941da6781b
This commit is contained in:
Pedro Alves
2023-05-09 10:27:04 +01:00
parent 87e727c0c6
commit 552616cbb7
2 changed files with 10 additions and 8 deletions

View File

@@ -710,7 +710,6 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r)
{ {
if (th->wow64_context.ContextFlags == 0) if (th->wow64_context.ContextFlags == 0)
{ {
th->suspend ();
th->wow64_context.ContextFlags = CONTEXT_DEBUGGER_DR; th->wow64_context.ContextFlags = CONTEXT_DEBUGGER_DR;
CHECK (Wow64GetThreadContext (th->h, &th->wow64_context)); CHECK (Wow64GetThreadContext (th->h, &th->wow64_context));
} }
@@ -720,7 +719,6 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r)
{ {
if (th->context.ContextFlags == 0) if (th->context.ContextFlags == 0)
{ {
th->suspend ();
th->context.ContextFlags = CONTEXT_DEBUGGER_DR; th->context.ContextFlags = CONTEXT_DEBUGGER_DR;
CHECK (GetThreadContext (th->h, &th->context)); CHECK (GetThreadContext (th->h, &th->context));
} }
@@ -1320,12 +1318,6 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
} }
th->resume (); th->resume ();
} }
else
{
/* When single-stepping a specific thread, other threads must
be suspended. */
th->suspend ();
}
std::optional<unsigned> err; std::optional<unsigned> err;
do_synchronously ([&] () do_synchronously ([&] ()
@@ -1816,6 +1808,11 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
th->stopped_at_software_breakpoint = true; th->stopped_at_software_breakpoint = true;
th->pc_adjusted = false; 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; return result;

View File

@@ -1226,6 +1226,11 @@ win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
OUTMSG2 (("Child Stopped with signal = %d \n", OUTMSG2 (("Child Stopped with signal = %d \n",
ourstatus->sig ())); ourstatus->sig ()));
maybe_adjust_pc (); 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); return debug_event_ptid (&windows_process.current_event);
} }
default: default: