Skip to content

Commit

Permalink
Windows gdb/gdbserver: Move suspending thread to when returning event
Browse files Browse the repository at this point in the history
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.

Change-Id: Ifd6889a8afc041fad33cd1c4500e38941da6781b
  • Loading branch information
palves committed Jun 2, 2023
1 parent 03e4b61 commit 96509b0
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 8 deletions.
13 changes: 5 additions & 8 deletions gdb/windows-nat.c
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,6 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r)
{
if (th->wow64_context.ContextFlags == 0)
{
th->suspend ();
th->wow64_context.ContextFlags = CONTEXT_DEBUGGER_DR;
CHECK (Wow64GetThreadContext (th->h, &th->wow64_context));
}
Expand All @@ -718,7 +717,6 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r)
{
if (th->context.ContextFlags == 0)
{
th->suspend ();
th->context.ContextFlags = CONTEXT_DEBUGGER_DR;
CHECK (GetThreadContext (th->h, &th->context));
}
Expand Down Expand Up @@ -1298,12 +1296,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 ();
}

gdb::optional<unsigned> err;
do_synchronously ([&] ()
Expand Down Expand Up @@ -1793,6 +1785,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;
Expand Down
5 changes: 5 additions & 0 deletions gdbserver/win32-low.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,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:
Expand Down

0 comments on commit 96509b0

Please sign in to comment.