* [PATCH 01/34] Windows gdb: Dead code in windows_nat_target::do_initial_windows_stuff
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 14:39 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 02/34] Windows gdb: Eliminate global current_process.dr[8] global Pedro Alves
` (33 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
In windows_nat_target::do_initial_windows_stuff, there's no point in
setting windows_process.current_event.dwProcessId. It's a nop, given
the following memset.
Change-Id: I2fe460341b598ad293ea60d5f702b10cefc30711
---
gdb/windows-nat.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 70f955d9797..5578ae250a6 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1900,7 +1900,6 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching)
windows_process.cygwin_load_start = 0;
windows_process.cygwin_load_end = 0;
#endif
- windows_process.current_event.dwProcessId = pid;
memset (&windows_process.current_event, 0,
sizeof (windows_process.current_event));
inf = current_inferior ();
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 02/34] Windows gdb: Eliminate global current_process.dr[8] global
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
2024-05-07 23:42 ` [PATCH 01/34] Windows gdb: Dead code in windows_nat_target::do_initial_windows_stuff Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:02 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 03/34] Windows gdb+gdbserver: New find_thread, replaces thread_rec(DONT_INVALIDATE_CONTEXT) Pedro Alves
` (32 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
current_process.dr needs to be per-thread for non-stop. Actually, it
doesn't even need to exist at all. We have the x86_debug_reg_state
recording intent, and then the
cygwin_get_dr/cygwin_get_dr6/cygwin_get_dr7 functions are registered
as x86_dr_low_type vector functions, so they should return the current
value in the inferior's registers. See this comment in x86-dregs.c:
~~~
/* In non-stop/async, threads can be running while we change the
global dr_mirror (and friends). Say, we set a watchpoint, and
let threads resume. Now, say you delete the watchpoint, or
add/remove watchpoints such that dr_mirror changes while threads
are running. On targets that support non-stop,
inserting/deleting watchpoints updates the global dr_mirror only.
It does not update the real thread's debug registers; that's only
done prior to resume. Instead, if threads are running when the
mirror changes, a temporary and transparent stop on all threads
is forced so they can get their copy of the debug registers
updated on re-resume. Now, say, a thread hit a watchpoint before
having been updated with the new dr_mirror contents, and we
haven't yet handled the corresponding SIGTRAP. If we trusted
dr_mirror below, we'd mistake the real trapped address (from the
last time we had updated debug registers in the thread) with
whatever was currently in dr_mirror. So to fix this, dr_mirror
always represents intention, what we _want_ threads to have in
debug registers. To get at the address and cause of the trap, we
need to read the state the thread still has in its debug
registers.
In sum, always get the current debug register values the current
thread has, instead of trusting the global mirror. If the thread
was running when we last changed watchpoints, the mirror no
longer represents what was set in this thread's debug
registers. */
~~~
This patch makes the Windows native target follow that model as well.
I don't understand why would windows_nat_target::resume want to call
SetThreadContext itself. That duplicates things as it is currently
worrying about setting the debug registers as well. windows_continue
also does that, and windows_nat_target::resume always calls it. So
this patch simplifies windows_nat_target::resume too.
Change-Id: I2fe460341b598ad293ea60d5f702b10cefc30711
---
gdb/nat/windows-nat.h | 1 +
gdb/windows-nat.c | 151 ++++++++++++++++++-----------------------
gdbserver/win32-low.cc | 1 +
3 files changed, 68 insertions(+), 85 deletions(-)
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index f2b5d777016..3783f39051d 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -144,6 +144,7 @@ struct windows_process_info
{
/* The process handle */
HANDLE handle = 0;
+ DWORD process_id = 0;
DWORD main_thread_id = 0;
enum gdb_signal last_sig = GDB_SIGNAL_0;
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 5578ae250a6..0102511dc8d 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -97,8 +97,6 @@ struct windows_per_inferior : public windows_process_info
void handle_unload_dll () override;
bool handle_access_violation (const EXCEPTION_RECORD *rec) override;
- uintptr_t dr[8] {};
-
int windows_initialization_done = 0;
std::vector<std::unique_ptr<windows_thread_info>> thread_list;
@@ -730,36 +728,12 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r)
{
th->wow64_context.ContextFlags = CONTEXT_DEBUGGER_DR;
CHECK (Wow64GetThreadContext (th->h, &th->wow64_context));
- /* Copy dr values from that thread.
- But only if there were not modified since last stop.
- PR gdb/2388 */
- if (!th->debug_registers_changed)
- {
- windows_process.dr[0] = th->wow64_context.Dr0;
- windows_process.dr[1] = th->wow64_context.Dr1;
- windows_process.dr[2] = th->wow64_context.Dr2;
- windows_process.dr[3] = th->wow64_context.Dr3;
- windows_process.dr[6] = th->wow64_context.Dr6;
- windows_process.dr[7] = th->wow64_context.Dr7;
- }
}
else
#endif
{
th->context.ContextFlags = CONTEXT_DEBUGGER_DR;
CHECK (GetThreadContext (th->h, &th->context));
- /* Copy dr values from that thread.
- But only if there were not modified since last stop.
- PR gdb/2388 */
- if (!th->debug_registers_changed)
- {
- windows_process.dr[0] = th->context.Dr0;
- windows_process.dr[1] = th->context.Dr1;
- windows_process.dr[2] = th->context.Dr2;
- windows_process.dr[3] = th->context.Dr3;
- windows_process.dr[6] = th->context.Dr6;
- windows_process.dr[7] = th->context.Dr7;
- }
}
th->reload_context = false;
}
@@ -1283,18 +1257,21 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
for (auto &th : windows_process.thread_list)
if (id == -1 || id == (int) th->tid)
{
+ struct x86_debug_reg_state *state
+ = x86_debug_reg_state (windows_process.process_id);
+
#ifdef __x86_64__
if (windows_process.wow64_process)
{
if (th->debug_registers_changed)
{
th->wow64_context.ContextFlags |= CONTEXT_DEBUG_REGISTERS;
- th->wow64_context.Dr0 = windows_process.dr[0];
- th->wow64_context.Dr1 = windows_process.dr[1];
- th->wow64_context.Dr2 = windows_process.dr[2];
- th->wow64_context.Dr3 = windows_process.dr[3];
+ th->wow64_context.Dr0 = state->dr_mirror[0];
+ th->wow64_context.Dr1 = state->dr_mirror[1];
+ th->wow64_context.Dr2 = state->dr_mirror[2];
+ th->wow64_context.Dr3 = state->dr_mirror[3];
th->wow64_context.Dr6 = DR6_CLEAR_VALUE;
- th->wow64_context.Dr7 = windows_process.dr[7];
+ th->wow64_context.Dr7 = state->dr_control_mirror;
th->debug_registers_changed = false;
}
if (th->wow64_context.ContextFlags)
@@ -1319,12 +1296,12 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
if (th->debug_registers_changed)
{
th->context.ContextFlags |= CONTEXT_DEBUG_REGISTERS;
- th->context.Dr0 = windows_process.dr[0];
- th->context.Dr1 = windows_process.dr[1];
- th->context.Dr2 = windows_process.dr[2];
- th->context.Dr3 = windows_process.dr[3];
+ th->context.Dr0 = state->dr_mirror[0];
+ th->context.Dr1 = state->dr_mirror[1];
+ th->context.Dr2 = state->dr_mirror[2];
+ th->context.Dr3 = state->dr_mirror[3];
th->context.Dr6 = DR6_CLEAR_VALUE;
- th->context.Dr7 = windows_process.dr[7];
+ th->context.Dr7 = state->dr_control_mirror;
th->debug_registers_changed = false;
}
if (th->context.ContextFlags)
@@ -1463,22 +1440,6 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
fetch_registers (regcache, gdbarch_ps_regnum (gdbarch));
th->wow64_context.EFlags |= FLAG_TRACE_BIT;
}
-
- if (th->wow64_context.ContextFlags)
- {
- if (th->debug_registers_changed)
- {
- th->wow64_context.Dr0 = windows_process.dr[0];
- th->wow64_context.Dr1 = windows_process.dr[1];
- th->wow64_context.Dr2 = windows_process.dr[2];
- th->wow64_context.Dr3 = windows_process.dr[3];
- th->wow64_context.Dr6 = DR6_CLEAR_VALUE;
- th->wow64_context.Dr7 = windows_process.dr[7];
- th->debug_registers_changed = false;
- }
- CHECK (Wow64SetThreadContext (th->h, &th->wow64_context));
- th->wow64_context.ContextFlags = 0;
- }
}
else
#endif
@@ -1491,22 +1452,6 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
fetch_registers (regcache, gdbarch_ps_regnum (gdbarch));
th->context.EFlags |= FLAG_TRACE_BIT;
}
-
- if (th->context.ContextFlags)
- {
- if (th->debug_registers_changed)
- {
- th->context.Dr0 = windows_process.dr[0];
- th->context.Dr1 = windows_process.dr[1];
- th->context.Dr2 = windows_process.dr[2];
- th->context.Dr3 = windows_process.dr[3];
- th->context.Dr6 = DR6_CLEAR_VALUE;
- th->context.Dr7 = windows_process.dr[7];
- th->debug_registers_changed = false;
- }
- CHECK (SetThreadContext (th->h, &th->context));
- th->context.ContextFlags = 0;
- }
}
}
@@ -1887,19 +1832,15 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
void
windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching)
{
- int i;
struct inferior *inf;
windows_process.last_sig = GDB_SIGNAL_0;
windows_process.open_process_used = 0;
- for (i = 0;
- i < sizeof (windows_process.dr) / sizeof (windows_process.dr[0]);
- i++)
- windows_process.dr[i] = 0;
#ifdef __CYGWIN__
windows_process.cygwin_load_start = 0;
windows_process.cygwin_load_end = 0;
#endif
+ windows_process.process_id = pid;
memset (&windows_process.current_event, 0,
sizeof (windows_process.current_event));
inf = current_inferior ();
@@ -3321,7 +3262,6 @@ cygwin_set_dr (int i, CORE_ADDR addr)
{
if (i < 0 || i > 3)
internal_error (_("Invalid register %d in cygwin_set_dr.\n"), i);
- windows_process.dr[i] = addr;
for (auto &th : windows_process.thread_list)
th->debug_registers_changed = true;
@@ -3333,8 +3273,6 @@ cygwin_set_dr (int i, CORE_ADDR addr)
static void
cygwin_set_dr7 (unsigned long val)
{
- windows_process.dr[7] = (CORE_ADDR) val;
-
for (auto &th : windows_process.thread_list)
th->debug_registers_changed = true;
}
@@ -3344,26 +3282,69 @@ cygwin_set_dr7 (unsigned long val)
static CORE_ADDR
cygwin_get_dr (int i)
{
- return windows_process.dr[i];
+ windows_thread_info *th
+ = windows_process.thread_rec (inferior_ptid, DONT_INVALIDATE_CONTEXT);
+
+#ifdef __x86_64__
+ if (windows_process.wow64_process)
+ {
+ gdb_assert (th->wow64_context.ContextFlags != 0);
+ switch (i)
+ {
+ case 0:
+ return th->wow64_context.Dr0;
+ case 1:
+ return th->wow64_context.Dr1;
+ case 2:
+ return th->wow64_context.Dr2;
+ case 3:
+ return th->wow64_context.Dr3;
+ case 6:
+ return th->wow64_context.Dr6;
+ case 7:
+ return th->wow64_context.Dr7;
+ };
+ }
+ else
+#endif
+ {
+ gdb_assert (th->context.ContextFlags != 0);
+ switch (i)
+ {
+ case 0:
+ return th->context.Dr0;
+ case 1:
+ return th->context.Dr1;
+ case 2:
+ return th->context.Dr2;
+ case 3:
+ return th->context.Dr3;
+ case 6:
+ return th->context.Dr6;
+ case 7:
+ return th->context.Dr7;
+ };
+ }
+
+ gdb_assert_not_reached ("invalid x86 dr register number: %d", i);
}
-/* Get the value of the DR6 debug status register from the inferior.
- Here we just return the value stored in dr[6]
- by the last call to thread_rec for current_event.dwThreadId id. */
+/* Get the value of the DR6 debug status register from the
+ inferior. */
+
static unsigned long
cygwin_get_dr6 (void)
{
- return (unsigned long) windows_process.dr[6];
+ return cygwin_get_dr (6);
}
-/* Get the value of the DR7 debug status register from the inferior.
- Here we just return the value stored in dr[7] by the last call to
- thread_rec for current_event.dwThreadId id. */
+/* Get the value of the DR7 debug status register from the
+ inferior. */
static unsigned long
cygwin_get_dr7 (void)
{
- return (unsigned long) windows_process.dr[7];
+ return cygwin_get_dr (7);
}
/* Determine if the thread referenced by "ptid" is alive
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index f672e542d1b..cc314df8dbd 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -309,6 +309,7 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
windows_process.last_sig = GDB_SIGNAL_0;
windows_process.handle = proch;
+ windows_process.process_id = pid;
windows_process.main_thread_id = 0;
windows_process.soft_interrupt_requested = 0;
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 02/34] Windows gdb: Eliminate global current_process.dr[8] global
2024-05-07 23:42 ` [PATCH 02/34] Windows gdb: Eliminate global current_process.dr[8] global Pedro Alves
@ 2024-05-08 15:02 ` Tom Tromey
0 siblings, 0 replies; 64+ messages in thread
From: Tom Tromey @ 2024-05-08 15:02 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> This patch makes the Windows native target follow that model as well.
This looks good to me, though I am not super confident in my ability to
reason about it in the abstract.
Pedro> I don't understand why would windows_nat_target::resume want to call
Pedro> SetThreadContext itself. That duplicates things as it is currently
Pedro> worrying about setting the debug registers as well. windows_continue
Pedro> also does that, and windows_nat_target::resume always calls it. So
Pedro> this patch simplifies windows_nat_target::resume too.
FWIW when doing work on windows-nat, I found a number of little
oddities, but generally just erred on the side of preserving them, since
it wasn't always clear to me what the ramifications were. Your series
is a welcome and long-overdue cleanup.
Pedro> - /* Copy dr values from that thread.
Pedro> - But only if there were not modified since last stop.
Pedro> - PR gdb/2388 */
This was moved to https://sourceware.org/bugzilla/show_bug.cgi?id=9493
in the bugzilla migration. Anyway I'm wondering if you tried this
example. It didn't seem to come with a test case.
Tom
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 03/34] Windows gdb+gdbserver: New find_thread, replaces thread_rec(DONT_INVALIDATE_CONTEXT)
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
2024-05-07 23:42 ` [PATCH 01/34] Windows gdb: Dead code in windows_nat_target::do_initial_windows_stuff Pedro Alves
2024-05-07 23:42 ` [PATCH 02/34] Windows gdb: Eliminate global current_process.dr[8] global Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:03 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 04/34] Windows gdb: handle_output_debug_string return type Pedro Alves
` (31 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
The goal of the next few patches is to eliminate thread_rec
completely. This is the first patch in that effort.
thread_rec(DONT_INVALIDATE_CONTEXT) is really just a thread lookup
with no side effects, so this adds a find_thread function that lets
you do that.
Change-Id: Ie486badce00e234b10caa478b066c34537103e3f
---
gdb/nat/windows-nat.c | 5 ++-
gdb/nat/windows-nat.h | 7 ++--
gdb/windows-nat.c | 72 ++++++++++++++++++++----------------------
gdbserver/win32-low.cc | 30 +++++++++++-------
gdbserver/win32-low.h | 1 +
5 files changed, 60 insertions(+), 55 deletions(-)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index f9f6848861d..46fbc2bfecb 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -325,9 +325,8 @@ windows_process_info::handle_ms_vc_exception (const EXCEPTION_RECORD *rec)
if (named_thread_id == (DWORD) -1)
named_thread_id = current_event.dwThreadId;
- named_thread = thread_rec (ptid_t (current_event.dwProcessId,
- named_thread_id, 0),
- DONT_INVALIDATE_CONTEXT);
+ named_thread = find_thread (ptid_t (current_event.dwProcessId,
+ named_thread_id, 0));
if (named_thread != NULL)
{
int thread_name_len;
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 3783f39051d..9e8c87e81de 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -105,9 +105,6 @@ struct windows_thread_info
/* Possible values to pass to 'thread_rec'. */
enum thread_disposition_type
{
- /* Do not invalidate the thread's context, and do not suspend the
- thread. */
- DONT_INVALIDATE_CONTEXT,
/* Invalidate the context, but do not suspend the thread. */
DONT_SUSPEND,
/* Invalidate the context and suspend the thread. */
@@ -178,6 +175,10 @@ struct windows_process_info
bool ignore_first_breakpoint = false;
#endif
+ /* Find a thread record given a thread id.
+
+ This function must be supplied by the embedding application. */
+ virtual windows_thread_info *find_thread (ptid_t ptid) = 0;
/* Find a thread record given a thread id. THREAD_DISPOSITION
controls whether the thread is suspended, and whether the context
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 0102511dc8d..3a3dad5fb2e 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -90,6 +90,7 @@ struct windows_solib
struct windows_per_inferior : public windows_process_info
{
+ windows_thread_info *find_thread (ptid_t ptid) override;
windows_thread_info *thread_rec (ptid_t ptid,
thread_disposition_type disposition) override;
int handle_output_debug_string (struct target_waitstatus *ourstatus) override;
@@ -257,8 +258,7 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
bool stopped_by_sw_breakpoint () override
{
- windows_thread_info *th
- = windows_process.thread_rec (inferior_ptid, DONT_INVALIDATE_CONTEXT);
+ windows_thread_info *th = windows_process.find_thread (inferior_ptid);
return th->stopped_at_software_breakpoint;
}
@@ -515,34 +515,36 @@ windows_nat_target::wait_for_debug_event_main_thread (DEBUG_EVENT *event)
/* See nat/windows-nat.h. */
windows_thread_info *
-windows_per_inferior::thread_rec
- (ptid_t ptid, thread_disposition_type disposition)
+windows_per_inferior::find_thread (ptid_t ptid)
{
for (auto &th : thread_list)
if (th->tid == ptid.lwp ())
- {
- if (!th->suspended)
- {
- switch (disposition)
- {
- case DONT_INVALIDATE_CONTEXT:
- /* Nothing. */
- break;
- case INVALIDATE_CONTEXT:
- if (ptid.lwp () != current_event.dwThreadId)
- th->suspend ();
- th->reload_context = true;
- break;
- case DONT_SUSPEND:
- th->reload_context = true;
- th->suspended = -1;
- break;
- }
- }
- return th.get ();
- }
+ return th.get ();
+ return nullptr;
+}
- return NULL;
+windows_thread_info *
+windows_per_inferior::thread_rec
+ (ptid_t ptid, thread_disposition_type disposition)
+{
+ windows_thread_info *th = find_thread (ptid);
+
+ if (th != nullptr && !th->suspended)
+ {
+ switch (disposition)
+ {
+ case INVALIDATE_CONTEXT:
+ if (ptid.lwp () != current_event.dwThreadId)
+ th->suspend ();
+ th->reload_context = true;
+ break;
+ case DONT_SUSPEND:
+ th->reload_context = true;
+ th->suspended = -1;
+ break;
+ }
+ }
+ return th;
}
/* Add a thread to the thread list.
@@ -561,7 +563,7 @@ windows_nat_target::add_thread (ptid_t ptid, HANDLE h, void *tlb,
gdb_assert (ptid.lwp () != 0);
- if ((th = windows_process.thread_rec (ptid, DONT_INVALIDATE_CONTEXT)))
+ if ((th = windows_process.find_thread (ptid)))
return th;
CORE_ADDR base = (CORE_ADDR) (uintptr_t) tlb;
@@ -1149,7 +1151,7 @@ display_selectors (const char * args, int from_tty)
}
windows_thread_info *current_windows_thread
- = windows_process.thread_rec (inferior_ptid, DONT_INVALIDATE_CONTEXT);
+ = windows_process.find_thread (inferior_ptid);
if (!args)
{
@@ -1426,7 +1428,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
ptid.pid (), (unsigned) ptid.lwp (), step, sig);
/* Get context for currently selected thread. */
- th = windows_process.thread_rec (inferior_ptid, DONT_INVALIDATE_CONTEXT);
+ th = windows_process.find_thread (inferior_ptid);
if (th)
{
#ifdef __x86_64__
@@ -3119,7 +3121,7 @@ windows_nat_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
{
windows_thread_info *th;
- th = windows_process.thread_rec (ptid, DONT_INVALIDATE_CONTEXT);
+ th = windows_process.find_thread (ptid);
if (th == NULL)
return false;
@@ -3140,9 +3142,7 @@ windows_nat_target::get_ada_task_ptid (long lwp, ULONGEST thread)
const char *
windows_nat_target::thread_name (struct thread_info *thr)
{
- windows_thread_info *th
- = windows_process.thread_rec (thr->ptid,
- DONT_INVALIDATE_CONTEXT);
+ windows_thread_info *th = windows_process.find_thread (thr->ptid);
return th->thread_name ();
}
@@ -3282,8 +3282,7 @@ cygwin_set_dr7 (unsigned long val)
static CORE_ADDR
cygwin_get_dr (int i)
{
- windows_thread_info *th
- = windows_process.thread_rec (inferior_ptid, DONT_INVALIDATE_CONTEXT);
+ windows_thread_info *th = windows_process.find_thread (inferior_ptid);
#ifdef __x86_64__
if (windows_process.wow64_process)
@@ -3356,8 +3355,7 @@ windows_nat_target::thread_alive (ptid_t ptid)
{
gdb_assert (ptid.lwp () != 0);
- windows_thread_info *th
- = windows_process.thread_rec (ptid, DONT_INVALIDATE_CONTEXT);
+ windows_thread_info *th = windows_process.find_thread (ptid);
return WaitForSingleObject (th->h, 0) != WAIT_OBJECT_0;
}
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index cc314df8dbd..5a57eedd18b 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -141,6 +141,17 @@ win32_require_context (windows_thread_info *th)
/* See nat/windows-nat.h. */
+windows_thread_info *
+gdbserver_windows_process::find_thread (ptid_t ptid)
+{
+ thread_info *thread = find_thread_ptid (ptid);
+ if (thread == nullptr)
+ return nullptr;
+ return (windows_thread_info *) thread_target_data (thread);
+}
+
+/* See nat/windows-nat.h. */
+
windows_thread_info *
gdbserver_windows_process::thread_rec
(ptid_t ptid, thread_disposition_type disposition)
@@ -150,8 +161,7 @@ gdbserver_windows_process::thread_rec
return NULL;
windows_thread_info *th = (windows_thread_info *) thread_target_data (thread);
- if (disposition != DONT_INVALIDATE_CONTEXT)
- win32_require_context (th);
+ win32_require_context (th);
return th;
}
@@ -162,7 +172,7 @@ child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
windows_thread_info *th;
ptid_t ptid = ptid_t (pid, tid, 0);
- if ((th = windows_process.thread_rec (ptid, DONT_INVALIDATE_CONTEXT)))
+ if ((th = windows_process.find_thread (ptid)))
return th;
CORE_ADDR base = (CORE_ADDR) (uintptr_t) tlb;
@@ -822,7 +832,7 @@ win32_process_target::resume (thread_resume *resume_info, size_t n)
/* Get context for the currently selected thread. */
ptid = debug_event_ptid (&windows_process.current_event);
- th = windows_process.thread_rec (ptid, DONT_INVALIDATE_CONTEXT);
+ th = windows_process.find_thread (ptid);
if (th)
{
win32_prepare_to_resume (th);
@@ -974,8 +984,7 @@ maybe_adjust_pc ()
child_fetch_inferior_registers (regcache, -1);
windows_thread_info *th
- = windows_process.thread_rec (current_thread_ptid (),
- DONT_INVALIDATE_CONTEXT);
+ = windows_process.find_thread (current_thread_ptid ());
th->stopped_at_software_breakpoint = false;
if (windows_process.current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
@@ -1375,8 +1384,7 @@ win32_process_target::supports_get_tib_address ()
int
win32_process_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
{
- windows_thread_info *th;
- th = windows_process.thread_rec (ptid, DONT_INVALIDATE_CONTEXT);
+ windows_thread_info *th = windows_process.find_thread (ptid);
if (th == NULL)
return 0;
if (addr != NULL)
@@ -1397,8 +1405,7 @@ bool
win32_process_target::stopped_by_sw_breakpoint ()
{
windows_thread_info *th
- = windows_process.thread_rec (current_thread_ptid (),
- DONT_INVALIDATE_CONTEXT);
+ = windows_process.find_thread (current_thread_ptid ());
return th == nullptr ? false : th->stopped_at_software_breakpoint;
}
@@ -1424,8 +1431,7 @@ const char *
win32_process_target::thread_name (ptid_t thread)
{
windows_thread_info *th
- = windows_process.thread_rec (current_thread_ptid (),
- DONT_INVALIDATE_CONTEXT);
+ = windows_process.find_thread (current_thread_ptid ());
return th->thread_name ();
}
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index ff997df0a66..c725c182ac4 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -174,6 +174,7 @@ class win32_process_target : public process_stratum_target
struct gdbserver_windows_process : public windows_nat::windows_process_info
{
+ windows_nat::windows_thread_info *find_thread (ptid_t ptid) override;
windows_nat::windows_thread_info *thread_rec
(ptid_t ptid,
windows_nat::thread_disposition_type disposition) override;
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 04/34] Windows gdb: handle_output_debug_string return type
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (2 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 03/34] Windows gdb+gdbserver: New find_thread, replaces thread_rec(DONT_INVALIDATE_CONTEXT) Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 14:43 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 05/34] Windows gdb: Eliminate reload_context Pedro Alves
` (30 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
handle_output_debug_string returns a Windows thread id, so it should
return a DWORD instead of an int.
Change-Id: Icbd071a1a37de8a0fc8918bd13254a8d40311e32
---
gdb/nat/windows-nat.h | 2 +-
gdb/windows-nat.c | 16 ++++++++--------
gdbserver/win32-low.cc | 2 +-
gdbserver/win32-low.h | 2 +-
4 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 9e8c87e81de..7d336346a86 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -196,7 +196,7 @@ struct windows_process_info
a Cygwin signal. Otherwise just print the string as a warning.
This function must be supplied by the embedding application. */
- virtual int handle_output_debug_string (struct target_waitstatus *ourstatus) = 0;
+ virtual DWORD handle_output_debug_string (struct target_waitstatus *ourstatus) = 0;
/* Handle a DLL load event.
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3a3dad5fb2e..4ffbeaaf7e4 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -93,7 +93,7 @@ struct windows_per_inferior : public windows_process_info
windows_thread_info *find_thread (ptid_t ptid) override;
windows_thread_info *thread_rec (ptid_t ptid,
thread_disposition_type disposition) override;
- int handle_output_debug_string (struct target_waitstatus *ourstatus) override;
+ DWORD handle_output_debug_string (struct target_waitstatus *ourstatus) override;
void handle_load_dll (const char *dll_name, LPVOID base) override;
void handle_unload_dll () override;
bool handle_access_violation (const EXCEPTION_RECORD *rec) override;
@@ -996,11 +996,11 @@ signal_event_command (const char *args, int from_tty)
/* See nat/windows-nat.h. */
-int
+DWORD
windows_per_inferior::handle_output_debug_string
(struct target_waitstatus *ourstatus)
{
- int retval = 0;
+ DWORD thread_id = 0;
gdb::unique_xmalloc_ptr<char> s
= (target_read_string
@@ -1041,19 +1041,19 @@ windows_per_inferior::handle_output_debug_string
if (gotasig)
{
ourstatus->set_stopped (gotasig);
- retval = strtoul (p, &p, 0);
- if (!retval)
- retval = current_event.dwThreadId;
+ thread_id = strtoul (p, &p, 0);
+ if (thread_id == 0)
+ thread_id = current_event.dwThreadId;
else
x = (LPCVOID) (uintptr_t) strtoull (p, NULL, 0);
}
DEBUG_EVENTS ("gdb: cygwin signal %d, thread 0x%x, CONTEXT @ %p",
- gotasig, retval, x);
+ gotasig, thread_id, x);
}
#endif
- return retval;
+ return thread_id;
}
static int
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 5a57eedd18b..5d3ad38cdbb 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -642,7 +642,7 @@ win32_process_target::attach (unsigned long pid)
/* See nat/windows-nat.h. */
-int
+DWORD
gdbserver_windows_process::handle_output_debug_string
(struct target_waitstatus *ourstatus)
{
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index c725c182ac4..8162d975227 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -178,7 +178,7 @@ struct gdbserver_windows_process : public windows_nat::windows_process_info
windows_nat::windows_thread_info *thread_rec
(ptid_t ptid,
windows_nat::thread_disposition_type disposition) override;
- int handle_output_debug_string (struct target_waitstatus *ourstatus) override;
+ DWORD handle_output_debug_string (struct target_waitstatus *ourstatus) override;
void handle_load_dll (const char *dll_name, LPVOID base) override;
void handle_unload_dll () override;
bool handle_access_violation (const EXCEPTION_RECORD *rec) override;
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 05/34] Windows gdb: Eliminate reload_context
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (3 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 04/34] Windows gdb: handle_output_debug_string return type Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 14:45 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 06/34] Windows gdb+gdbserver: Eliminate thread_rec(INVALIDATE_CONTEXT) calls Pedro Alves
` (29 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
We don't need reload_context, because we can get the same information
out of th->context.ContextFlags. If ContextFlags is zero, then we
need to fetch the context out of the inferior thread. This is what
gdbserver does too.
Change-Id: Ied566037c81383414c46c77713bdd1aec6377b23
---
gdb/nat/windows-nat.h | 4 ----
gdb/windows-nat.c | 42 +++++++++++++++++++++++++-----------------
2 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 7d336346a86..41921aa653b 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -84,10 +84,6 @@ struct windows_thread_info
the thread. */
bool debug_registers_changed = false;
- /* Nonzero if CONTEXT is invalidated and must be re-read from the
- inferior thread. */
- bool reload_context = false;
-
/* True if this thread is currently stopped at a software
breakpoint. This is used to offset the PC when needed. */
bool stopped_at_software_breakpoint = false;
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 4ffbeaaf7e4..3888ab6a02c 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -98,6 +98,8 @@ struct windows_per_inferior : public windows_process_info
void handle_unload_dll () override;
bool handle_access_violation (const EXCEPTION_RECORD *rec) override;
+ void invalidate_context (windows_thread_info *th);
+
int windows_initialization_done = 0;
std::vector<std::unique_ptr<windows_thread_info>> thread_list;
@@ -523,6 +525,18 @@ windows_per_inferior::find_thread (ptid_t ptid)
return nullptr;
}
+
+void
+windows_per_inferior::invalidate_context (windows_thread_info *th)
+{
+#ifdef __x86_64__
+ if (windows_process.wow64_process)
+ th->wow64_context.ContextFlags = 0;
+ else
+#endif
+ th->context.ContextFlags = 0;
+}
+
windows_thread_info *
windows_per_inferior::thread_rec
(ptid_t ptid, thread_disposition_type disposition)
@@ -536,11 +550,11 @@ windows_per_inferior::thread_rec
case INVALIDATE_CONTEXT:
if (ptid.lwp () != current_event.dwThreadId)
th->suspend ();
- th->reload_context = true;
+ invalidate_context (th);
break;
case DONT_SUSPEND:
- th->reload_context = true;
th->suspended = -1;
+ invalidate_context (th);
break;
}
}
@@ -642,18 +656,13 @@ windows_nat_target::delete_thread (ptid_t ptid, DWORD exit_code,
and supplies its value to the given regcache.
This function assumes that R is non-negative. A failed assertion
- is raised if that is not true.
-
- This function assumes that TH->RELOAD_CONTEXT is not set, meaning
- that the windows_thread_info has an up-to-date context. A failed
- assertion is raised if that assumption is violated. */
+ is raised if that is not true. */
static void
windows_fetch_one_register (struct regcache *regcache,
windows_thread_info *th, int r)
{
gdb_assert (r >= 0);
- gdb_assert (!th->reload_context);
char *context_ptr = (char *) &th->context;
#ifdef __x86_64__
@@ -723,21 +732,23 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r)
if (th == NULL)
return;
- if (th->reload_context)
- {
#ifdef __x86_64__
- if (windows_process.wow64_process)
+ if (windows_process.wow64_process)
+ {
+ if (th->wow64_context.ContextFlags == 0)
{
th->wow64_context.ContextFlags = CONTEXT_DEBUGGER_DR;
CHECK (Wow64GetThreadContext (th->h, &th->wow64_context));
}
- else
+ }
+ else
#endif
+ {
+ if (th->context.ContextFlags == 0)
{
th->context.ContextFlags = CONTEXT_DEBUGGER_DR;
CHECK (GetThreadContext (th->h, &th->context));
}
- th->reload_context = false;
}
if (r < 0)
@@ -1533,10 +1544,7 @@ windows_nat_target::get_windows_debug_event
*ourstatus = stop->status;
ptid_t ptid (windows_process.current_event.dwProcessId, thread_id);
- windows_thread_info *th
- = windows_process.thread_rec (ptid, INVALIDATE_CONTEXT);
- th->reload_context = true;
-
+ windows_process.thread_rec (ptid, INVALIDATE_CONTEXT);
return ptid;
}
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 06/34] Windows gdb+gdbserver: Eliminate thread_rec(INVALIDATE_CONTEXT) calls
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (4 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 05/34] Windows gdb: Eliminate reload_context Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:08 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 07/34] Windows gdb+gdbserver: Eliminate DONT_SUSPEND Pedro Alves
` (28 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
Replace thread_rec(INVALIDATE_CONTEXT) calls with find_thread, and
invalidate_context / suspend calls in the spots that might need those.
I don't know why does the INVALIDATE_CONTEXT implementation in GDB
avoid suspending the event thread:
case INVALIDATE_CONTEXT:
if (ptid.lwp () != current_event.dwThreadId)
th->suspend ();
Checks for a global "current_event" get in the way of non-stop support
later in the series, as each thread will have its own "last debug
event". Regardless, it should be fine to suspend the event thread.
As a data point, the GDBserver implementation always suspends. So
this patch does not try to avoid suspending the event thread on the
native side either.
Change-Id: I8d2f0a749d23329956e62362a7007189902dddb5
---
gdb/nat/windows-nat.h | 2 --
gdb/windows-nat.c | 38 ++++++++++++++++++++++----------------
gdbserver/win32-low.cc | 8 ++++----
3 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 41921aa653b..0e2093ee06d 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -103,8 +103,6 @@ enum thread_disposition_type
{
/* Invalidate the context, but do not suspend the thread. */
DONT_SUSPEND,
- /* Invalidate the context and suspend the thread. */
- INVALIDATE_CONTEXT
};
/* A single pending stop. See "pending_stops" for more
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3888ab6a02c..f53a45f657f 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -547,11 +547,6 @@ windows_per_inferior::thread_rec
{
switch (disposition)
{
- case INVALIDATE_CONTEXT:
- if (ptid.lwp () != current_event.dwThreadId)
- th->suspend ();
- invalidate_context (th);
- break;
case DONT_SUSPEND:
th->suspended = -1;
invalidate_context (th);
@@ -724,8 +719,7 @@ windows_fetch_one_register (struct regcache *regcache,
void
windows_nat_target::fetch_registers (struct regcache *regcache, int r)
{
- windows_thread_info *th
- = windows_process.thread_rec (regcache->ptid (), INVALIDATE_CONTEXT);
+ windows_thread_info *th = windows_process.find_thread (regcache->ptid ());
/* Check if TH exists. Windows sometimes uses a non-existent
thread id in its events. */
@@ -737,6 +731,7 @@ 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));
}
@@ -746,6 +741,7 @@ 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));
}
@@ -770,11 +766,23 @@ windows_store_one_register (const struct regcache *regcache,
{
gdb_assert (r >= 0);
- char *context_ptr = (char *) &th->context;
+ DWORD context_flags;
+ char *context_ptr;
+
#ifdef __x86_64__
if (windows_process.wow64_process)
- context_ptr = (char *) &th->wow64_context;
+ {
+ context_flags = th->wow64_context.ContextFlags;
+ context_ptr = (char *) &th->wow64_context;
+ }
+ else
#endif
+ {
+ context_flags = th->context.ContextFlags;
+ context_ptr = (char *) &th->context;
+ }
+
+ gdb_assert (context_flags != 0);
struct gdbarch *gdbarch = regcache->arch ();
i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (gdbarch);
@@ -807,8 +815,7 @@ windows_store_one_register (const struct regcache *regcache,
void
windows_nat_target::store_registers (struct regcache *regcache, int r)
{
- windows_thread_info *th
- = windows_process.thread_rec (regcache->ptid (), INVALIDATE_CONTEXT);
+ windows_thread_info *th = windows_process.find_thread (regcache->ptid ());
/* Check if TH exists. Windows sometimes uses a non-existent
thread id in its events. */
@@ -1544,7 +1551,8 @@ windows_nat_target::get_windows_debug_event
*ourstatus = stop->status;
ptid_t ptid (windows_process.current_event.dwProcessId, thread_id);
- windows_process.thread_rec (ptid, INVALIDATE_CONTEXT);
+ windows_thread_info *th = windows_process.find_thread (ptid);
+ windows_process.invalidate_context (th);
return ptid;
}
@@ -1765,8 +1773,7 @@ windows_nat_target::get_windows_debug_event
&& windows_process.windows_initialization_done)
{
ptid_t ptid = ptid_t (current_event->dwProcessId, thread_id, 0);
- windows_thread_info *th
- = windows_process.thread_rec (ptid, INVALIDATE_CONTEXT);
+ windows_thread_info *th = windows_process.find_thread (ptid);
th->stopped_at_software_breakpoint = true;
th->pc_adjusted = false;
}
@@ -1804,8 +1811,7 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
if (ourstatus->kind () != TARGET_WAITKIND_EXITED
&& ourstatus->kind () != TARGET_WAITKIND_SIGNALLED)
{
- windows_thread_info *th
- = windows_process.thread_rec (result, INVALIDATE_CONTEXT);
+ windows_thread_info *th = windows_process.find_thread (result);
if (th != nullptr)
{
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 5d3ad38cdbb..b693da583b0 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -463,8 +463,8 @@ child_fetch_inferior_registers (struct regcache *regcache, int r)
{
int regno;
windows_thread_info *th
- = windows_process.thread_rec (current_thread_ptid (),
- INVALIDATE_CONTEXT);
+ = windows_process.find_thread (current_thread_ptid ());
+ win32_require_context (th);
if (r == -1 || r > NUM_REGS)
child_fetch_inferior_registers (regcache, NUM_REGS);
else
@@ -479,8 +479,8 @@ child_store_inferior_registers (struct regcache *regcache, int r)
{
int regno;
windows_thread_info *th
- = windows_process.thread_rec (current_thread_ptid (),
- INVALIDATE_CONTEXT);
+ = windows_process.find_thread (current_thread_ptid ());
+ win32_require_context (th);
if (r == -1 || r == 0 || r > NUM_REGS)
child_store_inferior_registers (regcache, NUM_REGS);
else
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 06/34] Windows gdb+gdbserver: Eliminate thread_rec(INVALIDATE_CONTEXT) calls
2024-05-07 23:42 ` [PATCH 06/34] Windows gdb+gdbserver: Eliminate thread_rec(INVALIDATE_CONTEXT) calls Pedro Alves
@ 2024-05-08 15:08 ` Tom Tromey
0 siblings, 0 replies; 64+ messages in thread
From: Tom Tromey @ 2024-05-08 15:08 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> Replace thread_rec(INVALIDATE_CONTEXT) calls with find_thread, and
Pedro> invalidate_context / suspend calls in the spots that might need those.
This looks good to me.
Approved-By: Tom Tromey <tom@tromey.com>
Pedro> I don't know why does the INVALIDATE_CONTEXT implementation in GDB
Pedro> avoid suspending the event thread:
Pedro> case INVALIDATE_CONTEXT:
Pedro> if (ptid.lwp () != current_event.dwThreadId)
Pedro> th-> suspend ();
Me neither. However, the suspend/resume code in gdb/windows-nat.c has
always seemed pretty suspect to me. Like, I think in all-stop, there
was probably normally no need for any of those suspend calls -- only
when scheduler-locking was enabled. At least, my understanding is that
on a stop, Windows is already suspending all threads.
Tom
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 07/34] Windows gdb+gdbserver: Eliminate DONT_SUSPEND
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (5 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 06/34] Windows gdb+gdbserver: Eliminate thread_rec(INVALIDATE_CONTEXT) calls Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:12 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 08/34] Windows gdb+gdbserver: Eliminate windows_process_info::thread_rec Pedro Alves
` (27 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
There's a single call to thread_rec(DONT_SUSPEND), in
windows_process_info::handle_exception.
In GDB, the windows-nat.c thread_rec implementation avoids actually
calling SuspendThread on the event thread by doing:
th->suspended = -1;
I am not exactly sure why, but it kind of looks like it is done as an
optimization, avoiding a SuspendThread call? It is probably done for
the same reason as the code touched in the previous patch avoided
suspending the event thread.
This however gets in the way of non-stop mode, which will really want
to SuspendThread the event thread for DBG_REPLY_LATER.
In gdbserver's thread_rec implementation DONT_SUSPEND is ignored, and
thread_rec actually always suspends, which really suggests that
SuspendThread on the event thread is really not a problem. I really
can't imagine why it would be.
DONT_SUSPEND invalidates the thread's context, but there is no need to
invalidate the context when we get an event for a thread, because we
invalidate it when we previously resumed the thread.
So, we can just remove the thread_rec call from
windows_process_info::handle_exception. That's what this patch does.
Change-Id: I0f328542bda6d8268814ca1ee4ae7a478098ecf2
---
gdb/nat/windows-nat.c | 4 ----
gdb/nat/windows-nat.h | 2 --
gdb/windows-nat.c | 11 -----------
3 files changed, 17 deletions(-)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 46fbc2bfecb..b5cfad6274b 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -366,10 +366,6 @@ windows_process_info::handle_exception (struct target_waitstatus *ourstatus,
memcpy (&siginfo_er, rec, sizeof siginfo_er);
- /* Record the context of the current thread. */
- thread_rec (ptid_t (current_event.dwProcessId, current_event.dwThreadId, 0),
- DONT_SUSPEND);
-
last_sig = GDB_SIGNAL_0;
switch (code)
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 0e2093ee06d..e18edc995b5 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -101,8 +101,6 @@ struct windows_thread_info
/* Possible values to pass to 'thread_rec'. */
enum thread_disposition_type
{
- /* Invalidate the context, but do not suspend the thread. */
- DONT_SUSPEND,
};
/* A single pending stop. See "pending_stops" for more
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index f53a45f657f..f5435d70ed3 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -542,17 +542,6 @@ windows_per_inferior::thread_rec
(ptid_t ptid, thread_disposition_type disposition)
{
windows_thread_info *th = find_thread (ptid);
-
- if (th != nullptr && !th->suspended)
- {
- switch (disposition)
- {
- case DONT_SUSPEND:
- th->suspended = -1;
- invalidate_context (th);
- break;
- }
- }
return th;
}
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 08/34] Windows gdb+gdbserver: Eliminate windows_process_info::thread_rec
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (6 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 07/34] Windows gdb+gdbserver: Eliminate DONT_SUSPEND Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:12 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 09/34] Windows gdb: Simplify windows_nat_target::wait Pedro Alves
` (26 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
After the previous patches, thread_rec is no longer called anywhere.
Delete it.
Change-Id: Ib14e5807fc427e1c3c4a393a9ea7b36b6047a2d7
---
gdb/nat/windows-nat.h | 13 -------------
gdb/windows-nat.c | 10 ----------
gdbserver/win32-low.cc | 15 ---------------
gdbserver/win32-low.h | 3 ---
4 files changed, 41 deletions(-)
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index e18edc995b5..fdbab0fc566 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -98,11 +98,6 @@ struct windows_thread_info
};
-/* Possible values to pass to 'thread_rec'. */
-enum thread_disposition_type
-{
-};
-
/* A single pending stop. See "pending_stops" for more
information. */
struct pending_stop
@@ -172,14 +167,6 @@ struct windows_process_info
This function must be supplied by the embedding application. */
virtual windows_thread_info *find_thread (ptid_t ptid) = 0;
- /* Find a thread record given a thread id. THREAD_DISPOSITION
- controls whether the thread is suspended, and whether the context
- is invalidated.
-
- This function must be supplied by the embedding application. */
- virtual windows_thread_info *thread_rec (ptid_t ptid,
- thread_disposition_type disposition) = 0;
-
/* Handle OUTPUT_DEBUG_STRING_EVENT from child process. Updates
OURSTATUS and returns the thread id if this represents a thread
change (this is specific to Cygwin), otherwise 0.
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index f5435d70ed3..9b1bedee250 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -91,8 +91,6 @@ struct windows_solib
struct windows_per_inferior : public windows_process_info
{
windows_thread_info *find_thread (ptid_t ptid) override;
- windows_thread_info *thread_rec (ptid_t ptid,
- thread_disposition_type disposition) override;
DWORD handle_output_debug_string (struct target_waitstatus *ourstatus) override;
void handle_load_dll (const char *dll_name, LPVOID base) override;
void handle_unload_dll () override;
@@ -537,14 +535,6 @@ windows_per_inferior::invalidate_context (windows_thread_info *th)
th->context.ContextFlags = 0;
}
-windows_thread_info *
-windows_per_inferior::thread_rec
- (ptid_t ptid, thread_disposition_type disposition)
-{
- windows_thread_info *th = find_thread (ptid);
- return th;
-}
-
/* Add a thread to the thread list.
PTID is the ptid of the thread to be added.
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index b693da583b0..0d67cd984e4 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -150,21 +150,6 @@ gdbserver_windows_process::find_thread (ptid_t ptid)
return (windows_thread_info *) thread_target_data (thread);
}
-/* See nat/windows-nat.h. */
-
-windows_thread_info *
-gdbserver_windows_process::thread_rec
- (ptid_t ptid, thread_disposition_type disposition)
-{
- thread_info *thread = find_thread_ptid (ptid);
- if (thread == NULL)
- return NULL;
-
- windows_thread_info *th = (windows_thread_info *) thread_target_data (thread);
- win32_require_context (th);
- return th;
-}
-
/* Add a thread to the thread list. */
static windows_thread_info *
child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index 8162d975227..7ee4de4b37f 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -175,9 +175,6 @@ class win32_process_target : public process_stratum_target
struct gdbserver_windows_process : public windows_nat::windows_process_info
{
windows_nat::windows_thread_info *find_thread (ptid_t ptid) override;
- windows_nat::windows_thread_info *thread_rec
- (ptid_t ptid,
- windows_nat::thread_disposition_type disposition) override;
DWORD handle_output_debug_string (struct target_waitstatus *ourstatus) override;
void handle_load_dll (const char *dll_name, LPVOID base) override;
void handle_unload_dll () override;
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 09/34] Windows gdb: Simplify windows_nat_target::wait
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (7 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 08/34] Windows gdb+gdbserver: Eliminate windows_process_info::thread_rec Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 10/34] Windows gdb+gdbserver: Move suspending thread to when returning event Pedro Alves
` (25 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
The logic in windows_nat_target::wait, where we decide what to do
depending on the result from get_windows_debug_event is harder to
grasp than it looks.
It is not easy to tell what should happen when in async mode
get_windows_debug_event returns that there's no event to process.
And then, if get_windows_debug_event returns null_ptid /
TARGET_WAITKIND_SPURIOUS, then we need to issue a ContinueDebugEvent.
There's also this comment in windows_nat_target::wait, which we're not
really implementing today:
~~~~
/* We loop when we get a non-standard exception rather than return
with a SPURIOUS because resume can try and step or modify things,
which needs a current_thread->h. But some of these exceptions mark
the birth or death of threads, which mean that the current thread
isn't necessarily what you think it is. */
~~~~
This patch changes things a bit so that the code is more obvious:
- look at the status kind, instead of ptid_t.
- add an explicit early return case for no-event.
- add an explicit case for TARGET_WAITKIND_SPURIOUS.
- with those, we no longer need to handle the case of find_thread not
finding a thread, so we can drop one indentation level.
Change-Id: I76c41762e1f893a7ff23465856ccf6a44af1f0e7
---
gdb/windows-nat.c | 41 +++++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 9b1bedee250..9730377d6fc 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1764,7 +1764,10 @@ windows_nat_target::get_windows_debug_event
}
if (thread_id == 0)
- return null_ptid;
+ {
+ ourstatus->set_ignore ();
+ return null_ptid;
+ }
return ptid_t (windows_process.current_event.dwProcessId, thread_id, 0);
}
@@ -1785,27 +1788,33 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
{
ptid_t result = get_windows_debug_event (pid, ourstatus, options);
- if (result != null_ptid)
+ if ((options & TARGET_WNOHANG) != 0
+ && ourstatus->kind () == TARGET_WAITKIND_IGNORE)
+ return result;
+
+ if (ourstatus->kind () == TARGET_WAITKIND_SPURIOUS)
+ {
+ CHECK (windows_continue (DBG_CONTINUE,
+ windows_process.desired_stop_thread_id, 0));
+ }
+ else if (ourstatus->kind () != TARGET_WAITKIND_IGNORE)
{
if (ourstatus->kind () != TARGET_WAITKIND_EXITED
- && ourstatus->kind () != TARGET_WAITKIND_SIGNALLED)
+ && ourstatus->kind () != TARGET_WAITKIND_SIGNALLED)
{
windows_thread_info *th = windows_process.find_thread (result);
- if (th != nullptr)
+ th->stopped_at_software_breakpoint = false;
+ if (windows_process.current_event.dwDebugEventCode
+ == EXCEPTION_DEBUG_EVENT
+ && ((windows_process.current_event.u.Exception.ExceptionRecord.ExceptionCode
+ == EXCEPTION_BREAKPOINT)
+ || (windows_process.current_event.u.Exception.ExceptionRecord.ExceptionCode
+ == STATUS_WX86_BREAKPOINT))
+ && windows_process.windows_initialization_done)
{
- th->stopped_at_software_breakpoint = false;
- if (windows_process.current_event.dwDebugEventCode
- == EXCEPTION_DEBUG_EVENT
- && ((windows_process.current_event.u.Exception.ExceptionRecord.ExceptionCode
- == EXCEPTION_BREAKPOINT)
- || (windows_process.current_event.u.Exception.ExceptionRecord.ExceptionCode
- == STATUS_WX86_BREAKPOINT))
- && windows_process.windows_initialization_done)
- {
- th->stopped_at_software_breakpoint = true;
- th->pc_adjusted = false;
- }
+ th->stopped_at_software_breakpoint = true;
+ th->pc_adjusted = false;
}
}
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 10/34] Windows gdb+gdbserver: Move suspending thread to when returning event
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (8 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 09/34] Windows gdb: Simplify windows_nat_target::wait Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 11/34] Windows gdb: Introduce continue_last_debug_event_main_thread Pedro Alves
` (24 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
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
---
gdb/windows-nat.c | 13 +++++--------
gdbserver/win32-low.cc | 5 +++++
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 9730377d6fc..aa531c4208a 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -710,7 +710,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));
}
@@ -720,7 +719,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));
}
@@ -1320,12 +1318,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 ([&] ()
@@ -1816,6 +1808,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/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 0d67cd984e4..8dd8f21330d 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -1226,6 +1226,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:
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 11/34] Windows gdb: Introduce continue_last_debug_event_main_thread
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (9 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 10/34] Windows gdb+gdbserver: Move suspending thread to when returning event Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 14:53 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 12/34] Windows gdb: Introduce windows_continue_flags Pedro Alves
` (23 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
We have code using do_synchronously to call continue_last_debug_event,
and later patches in the series would need to add the same code in few
more places. Factor it out to a continue_last_debug_event_main_thread
function so these other places in future patches can just call it.
Change-Id: I945e668d2b3daeb9de968219925a7b3c7c7ce9ed
---
gdb/windows-nat.c | 46 ++++++++++++++++++++++++++++++----------------
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index aa531c4208a..93ba7850c88 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -354,6 +354,12 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
needed. */
void wait_for_debug_event_main_thread (DEBUG_EVENT *event);
+ /* This continues the last debug event, dispatching to the worker
+ thread as needed. */
+ void continue_last_debug_event_main_thread (const char *context_str,
+ DWORD continue_status,
+ bool last_call = false);
+
/* Force the process_thread thread to return from WaitForDebugEvent.
PROCESS_ALIVE is set to false if the inferior process exits while
we're trying to break out the process_thread thread. This can
@@ -512,6 +518,27 @@ windows_nat_target::wait_for_debug_event_main_thread (DEBUG_EVENT *event)
m_continued = false;
}
+void
+windows_nat_target::continue_last_debug_event_main_thread
+ (const char *context_str, DWORD continue_status, bool last_call)
+{
+ std::optional<unsigned> err;
+ do_synchronously ([&] ()
+ {
+ if (!continue_last_debug_event (continue_status, debug_events))
+ err = (unsigned) GetLastError ();
+
+ /* On the last call, do not block waiting for an event that will
+ never come. */
+ return !last_call;
+ });
+ if (err.has_value ())
+ throw_winerror_with_name (_("ContinueDebugEvent failed"),
+ *err);
+
+ m_continued = !last_call;
+}
+
/* See nat/windows-nat.h. */
windows_thread_info *
@@ -1319,22 +1346,9 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
th->resume ();
}
- std::optional<unsigned> err;
- do_synchronously ([&] ()
- {
- if (!continue_last_debug_event (continue_status, debug_events))
- err = (unsigned) GetLastError ();
- /* On the last call, do not block waiting for an event that will
- never come. */
- return !last_call;
- });
-
- if (err.has_value ())
- throw_winerror_with_name (_("Failed to resume program execution"
- " - ContinueDebugEvent failed"),
- *err);
-
- m_continued = !last_call;
+ continue_last_debug_event_main_thread
+ (_("Failed to resume program execution"), continue_status,
+ last_call);
return TRUE;
}
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 11/34] Windows gdb: Introduce continue_last_debug_event_main_thread
2024-05-07 23:42 ` [PATCH 11/34] Windows gdb: Introduce continue_last_debug_event_main_thread Pedro Alves
@ 2024-05-08 14:53 ` Tom Tromey
0 siblings, 0 replies; 64+ messages in thread
From: Tom Tromey @ 2024-05-08 14:53 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> We have code using do_synchronously to call continue_last_debug_event,
Pedro> and later patches in the series would need to add the same code in few
Pedro> more places. Factor it out to a continue_last_debug_event_main_thread
Pedro> function so these other places in future patches can just call it.
This seems reasonable but I noticed:
Pedro> +void
Pedro> +windows_nat_target::continue_last_debug_event_main_thread
Pedro> + (const char *context_str, DWORD continue_status, bool last_call)
Pedro> +{
...
Pedro> + if (err.has_value ())
Pedro> + throw_winerror_with_name (_("ContinueDebugEvent failed"),
Pedro> + *err);
context_str isn't used in the body of the method.
Tom
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 12/34] Windows gdb: Introduce windows_continue_flags
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (10 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 11/34] Windows gdb: Introduce continue_last_debug_event_main_thread Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:16 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 13/34] Windows gdb: Factor code out of windows_nat_target::windows_continue Pedro Alves
` (22 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
windows_continue already has two boolean parameters:
(..., int killed, bool last_call = false)
A patch later in the series would need a third. Instead, convert
windows_continue to use an optional enum-flags parameter instead of
multiple booleans.
Change-Id: I17c4d8a12b662190f972c380f838cb3317bd2e1e
---
gdb/windows-nat.c | 48 ++++++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 93ba7850c88..3993820ddab 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -88,6 +88,22 @@ struct windows_solib
std::string name;
};
+/* Flags that can be passed to windows_continue. */
+
+enum windows_continue_flag
+ {
+ /* This means we have killed the inferior, so windows_continue
+ should ignore weird errors due to threads shutting down. */
+ WCONT_KILLED = 1,
+
+ /* This means we expect this windows_continue call to be the last
+ call to continue the inferior -- we are either mourning it or
+ detaching. */
+ WCONT_LAST_CALL = 2,
+ };
+
+DEF_ENUM_FLAGS_TYPE (windows_continue_flag, windows_continue_flags);
+
struct windows_per_inferior : public windows_process_info
{
windows_thread_info *find_thread (ptid_t ptid) override;
@@ -332,8 +348,8 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
void delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p);
DWORD fake_create_process ();
- BOOL windows_continue (DWORD continue_status, int id, int killed,
- bool last_call = false);
+ BOOL windows_continue (DWORD continue_status, int id,
+ windows_continue_flags cont_flags = 0);
/* Helper function to start process_thread. */
static DWORD WINAPI process_thread_starter (LPVOID self);
@@ -1258,14 +1274,12 @@ windows_per_inferior::handle_access_violation
}
/* Resume thread specified by ID, or all artificially suspended
- threads, if we are continuing execution. KILLED non-zero means we
- have killed the inferior, so we should ignore weird errors due to
- threads shutting down. LAST_CALL is true if we expect this to be
- the last call to continue the inferior -- we are either mourning it
- or detaching. */
+ threads, if we are continuing execution. See
+ windows_continue_flags' description for CONT_FLAGS. */
+
BOOL
windows_nat_target::windows_continue (DWORD continue_status, int id,
- int killed, bool last_call)
+ windows_continue_flags cont_flags)
{
windows_process.desired_stop_thread_id = id;
@@ -1308,7 +1322,7 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
BOOL status = Wow64SetThreadContext (th->h,
&th->wow64_context);
- if (!killed)
+ if ((cont_flags & WCONT_KILLED) == 0)
CHECK (status);
}
th->wow64_context.ContextFlags = 0;
@@ -1337,7 +1351,7 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
{
BOOL status = SetThreadContext (th->h, &th->context);
- if (!killed)
+ if ((cont_flags & WCONT_KILLED) == 0)
CHECK (status);
}
th->context.ContextFlags = 0;
@@ -1348,7 +1362,7 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
continue_last_debug_event_main_thread
(_("Failed to resume program execution"), continue_status,
- last_call);
+ cont_flags & WCONT_LAST_CALL);
return TRUE;
}
@@ -1464,9 +1478,9 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
Otherwise complain. */
if (resume_all)
- windows_continue (continue_status, -1, 0);
+ windows_continue (continue_status, -1);
else
- windows_continue (continue_status, ptid.lwp (), 0);
+ windows_continue (continue_status, ptid.lwp ());
}
/* Interrupt the inferior. */
@@ -2174,7 +2188,7 @@ windows_nat_target::detach (inferior *inf, int from_tty)
if (m_continued)
break_out_process_thread (process_alive);
- windows_continue (DBG_CONTINUE, -1, 0, true);
+ windows_continue (DBG_CONTINUE, -1, WCONT_LAST_CALL);
std::optional<unsigned> err;
if (process_alive)
@@ -2922,13 +2936,13 @@ windows_nat_target::create_inferior (const char *exec_file,
do_initial_windows_stuff (pi.dwProcessId, 0);
- /* windows_continue (DBG_CONTINUE, -1, 0); */
+ /* windows_continue (DBG_CONTINUE, -1); */
}
void
windows_nat_target::mourn_inferior ()
{
- (void) windows_continue (DBG_CONTINUE, -1, 0, true);
+ windows_continue (DBG_CONTINUE, -1, WCONT_LAST_CALL);
x86_cleanup_dregs();
if (windows_process.open_process_used)
{
@@ -2986,7 +3000,7 @@ windows_nat_target::kill ()
for (;;)
{
- if (!windows_continue (DBG_CONTINUE, -1, 1))
+ if (!windows_continue (DBG_CONTINUE, -1, WCONT_KILLED))
break;
wait_for_debug_event_main_thread (&windows_process.current_event);
if (windows_process.current_event.dwDebugEventCode
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 13/34] Windows gdb: Factor code out of windows_nat_target::windows_continue
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (11 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 12/34] Windows gdb: Introduce windows_continue_flags Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:18 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 14/34] Windows gdb: Pending stop and current_event Pedro Alves
` (21 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
This factors some code out of windows_nat_target::windows_continue
into a new windows_continue_one function. This will make the
following patch easier to read (as well as the resulting code itself).
Change-Id: I14a0386b1b8b03015e86273060af173b5130e375
---
gdb/windows-nat.c | 137 ++++++++++++++++++++++++----------------------
1 file changed, 72 insertions(+), 65 deletions(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3993820ddab..fb14d8eb55f 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -114,6 +114,9 @@ struct windows_per_inferior : public windows_process_info
void invalidate_context (windows_thread_info *th);
+ void continue_one_thread (windows_thread_info *th,
+ windows_continue_flags cont_flags);
+
int windows_initialization_done = 0;
std::vector<std::unique_ptr<windows_thread_info>> thread_list;
@@ -1273,6 +1276,74 @@ windows_per_inferior::handle_access_violation
return false;
}
+void
+windows_per_inferior::continue_one_thread (windows_thread_info *th,
+ windows_continue_flags cont_flags)
+{
+ struct x86_debug_reg_state *state = x86_debug_reg_state (process_id);
+
+#ifdef __x86_64__
+ if (wow64_process)
+ {
+ if (th->debug_registers_changed)
+ {
+ th->wow64_context.ContextFlags |= CONTEXT_DEBUG_REGISTERS;
+ th->wow64_context.Dr0 = state->dr_mirror[0];
+ th->wow64_context.Dr1 = state->dr_mirror[1];
+ th->wow64_context.Dr2 = state->dr_mirror[2];
+ th->wow64_context.Dr3 = state->dr_mirror[3];
+ th->wow64_context.Dr6 = DR6_CLEAR_VALUE;
+ th->wow64_context.Dr7 = state->dr_control_mirror;
+ th->debug_registers_changed = false;
+ }
+ if (th->wow64_context.ContextFlags)
+ {
+ DWORD ec = 0;
+
+ if (GetExitCodeThread (th->h, &ec)
+ && ec == STILL_ACTIVE)
+ {
+ BOOL status = Wow64SetThreadContext (th->h,
+ &th->wow64_context);
+
+ if ((cont_flags & WCONT_KILLED) == 0)
+ CHECK (status);
+ }
+ th->wow64_context.ContextFlags = 0;
+ }
+ }
+ else
+#endif
+ {
+ if (th->debug_registers_changed)
+ {
+ th->context.ContextFlags |= CONTEXT_DEBUG_REGISTERS;
+ th->context.Dr0 = state->dr_mirror[0];
+ th->context.Dr1 = state->dr_mirror[1];
+ th->context.Dr2 = state->dr_mirror[2];
+ th->context.Dr3 = state->dr_mirror[3];
+ th->context.Dr6 = DR6_CLEAR_VALUE;
+ th->context.Dr7 = state->dr_control_mirror;
+ th->debug_registers_changed = false;
+ }
+ if (th->context.ContextFlags)
+ {
+ DWORD ec = 0;
+
+ if (GetExitCodeThread (th->h, &ec)
+ && ec == STILL_ACTIVE)
+ {
+ BOOL status = SetThreadContext (th->h, &th->context);
+
+ if ((cont_flags & WCONT_KILLED) == 0)
+ CHECK (status);
+ }
+ th->context.ContextFlags = 0;
+ }
+ }
+ th->resume ();
+}
+
/* Resume thread specified by ID, or all artificially suspended
threads, if we are continuing execution. See
windows_continue_flags' description for CONT_FLAGS. */
@@ -1294,71 +1365,7 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
for (auto &th : windows_process.thread_list)
if (id == -1 || id == (int) th->tid)
- {
- struct x86_debug_reg_state *state
- = x86_debug_reg_state (windows_process.process_id);
-
-#ifdef __x86_64__
- if (windows_process.wow64_process)
- {
- if (th->debug_registers_changed)
- {
- th->wow64_context.ContextFlags |= CONTEXT_DEBUG_REGISTERS;
- th->wow64_context.Dr0 = state->dr_mirror[0];
- th->wow64_context.Dr1 = state->dr_mirror[1];
- th->wow64_context.Dr2 = state->dr_mirror[2];
- th->wow64_context.Dr3 = state->dr_mirror[3];
- th->wow64_context.Dr6 = DR6_CLEAR_VALUE;
- th->wow64_context.Dr7 = state->dr_control_mirror;
- th->debug_registers_changed = false;
- }
- if (th->wow64_context.ContextFlags)
- {
- DWORD ec = 0;
-
- if (GetExitCodeThread (th->h, &ec)
- && ec == STILL_ACTIVE)
- {
- BOOL status = Wow64SetThreadContext (th->h,
- &th->wow64_context);
-
- if ((cont_flags & WCONT_KILLED) == 0)
- CHECK (status);
- }
- th->wow64_context.ContextFlags = 0;
- }
- }
- else
-#endif
- {
- if (th->debug_registers_changed)
- {
- th->context.ContextFlags |= CONTEXT_DEBUG_REGISTERS;
- th->context.Dr0 = state->dr_mirror[0];
- th->context.Dr1 = state->dr_mirror[1];
- th->context.Dr2 = state->dr_mirror[2];
- th->context.Dr3 = state->dr_mirror[3];
- th->context.Dr6 = DR6_CLEAR_VALUE;
- th->context.Dr7 = state->dr_control_mirror;
- th->debug_registers_changed = false;
- }
- if (th->context.ContextFlags)
- {
- DWORD ec = 0;
-
- if (GetExitCodeThread (th->h, &ec)
- && ec == STILL_ACTIVE)
- {
- BOOL status = SetThreadContext (th->h, &th->context);
-
- if ((cont_flags & WCONT_KILLED) == 0)
- CHECK (status);
- }
- th->context.ContextFlags = 0;
- }
- }
- th->resume ();
- }
+ windows_process.continue_one_thread (th.get (), cont_flags);
continue_last_debug_event_main_thread
(_("Failed to resume program execution"), continue_status,
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 14/34] Windows gdb: Pending stop and current_event
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (12 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 13/34] Windows gdb: Factor code out of windows_nat_target::windows_continue Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:18 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 15/34] Windows gdb+gdbserver: Elim desired_stop_thread_id / rework pending_stops Pedro Alves
` (20 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
I noticed that windows_nat_target::get_windows_debug_event does not
copy the event recorded in pending stop to
windows_process.current_event. This seems like an oversight. The
equivalent code in gdbserver/win32-low.cc does copy it.
This change will become moot later in the series, but I figure its
still clearer to correct the buglet as preparatory patch.
Change-Id: Ic8935d854cf67a3a3c4edcbc1a1e8957b800d907
---
gdb/windows-nat.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index fb14d8eb55f..196f6a1f72d 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1555,6 +1555,7 @@ windows_nat_target::get_windows_debug_event
{
thread_id = stop->thread_id;
*ourstatus = stop->status;
+ windows_process.current_event = stop->event;
ptid_t ptid (windows_process.current_event.dwProcessId, thread_id);
windows_thread_info *th = windows_process.find_thread (ptid);
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 14/34] Windows gdb: Pending stop and current_event
2024-05-07 23:42 ` [PATCH 14/34] Windows gdb: Pending stop and current_event Pedro Alves
@ 2024-05-08 15:18 ` Tom Tromey
0 siblings, 0 replies; 64+ messages in thread
From: Tom Tromey @ 2024-05-08 15:18 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> I noticed that windows_nat_target::get_windows_debug_event does not
Pedro> copy the event recorded in pending stop to
Pedro> windows_process.current_event. This seems like an oversight. The
Pedro> equivalent code in gdbserver/win32-low.cc does copy it.
Pedro> This change will become moot later in the series, but I figure its
Pedro> still clearer to correct the buglet as preparatory patch.
Ok.
Approved-By: Tom Tromey <tom@tromey.com>
I wonder if this is observable in any way.
Tom
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 15/34] Windows gdb+gdbserver: Elim desired_stop_thread_id / rework pending_stops
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (13 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 14/34] Windows gdb: Pending stop and current_event Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 16/34] Windows gdb+gdbserver: Introduce get_last_debug_event_ptid Pedro Alves
` (19 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
windows_process.desired_stop_thread_id doesn't work for non-stop, as
in that case every thread will have its own independent
WaitForDebugEvent event.
Instead, detect whether we have been reported a stop that was not
supposed to be reported by simply checking whether the thread that is
reporting the event is suspended. This is now easilly possible since
each thread's suspend state is kept in sync with whether infrun wants
the thread executing or not.
windows_process.desired_stop_thread_id was also used as thread to pass
to windows_continue. However, we don't really need that either.
windows_continue is used to update the thread's registers, unsuspend
them, and then finally call ContinueDebugEvent. In most cases, we
only need the ContinueDebugEvent step, so we can convert the
windows_continue calls to continue_last_debug_event_main_thread calls.
The exception is when we see a thread creation event -- in that case,
we need to update the debug registers of the new thread. We can use
continue_one_thread for that.
Since the pending stop is now stored in windows_thread_info,
get_windows_debug_event needs to avoid reaching the bottom code if
there's no thread associated with the event anymore (i.e.,
EXIT_THREAD_DEBUG_EVENT / EXIT_PROCESS_DEBUG_EVENT).
I considered whether it would be possible to keep the pending_stop
handling code shared in gdb/nat/windows-nat.c, in this patch and
throughout the series, but I conclused that it isn't worth it, until
gdbserver is taught about async and non-stop as well.
The pending_stop struct will eventually be eliminated later down the
series.
Change-Id: Ib7c8e8d16edc0900b7c411976c5d70cf93931c1c
---
gdb/nat/windows-nat.c | 51 -------------------
gdb/nat/windows-nat.h | 71 ++++++++++----------------
gdb/windows-nat.c | 111 ++++++++++++++++++++++++-----------------
gdbserver/win32-low.cc | 53 ++++++++++++--------
4 files changed, 123 insertions(+), 163 deletions(-)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index b5cfad6274b..a01d7b3c0c0 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -666,57 +666,6 @@ windows_process_info::add_all_dlls ()
/* See nat/windows-nat.h. */
-bool
-windows_process_info::matching_pending_stop (bool debug_events)
-{
- /* If there are pending stops, and we might plausibly hit one of
- them, we don't want to actually continue the inferior -- we just
- want to report the stop. In this case, we just pretend to
- continue. See the comment by the definition of "pending_stops"
- for details on why this is needed. */
- for (const auto &item : pending_stops)
- {
- if (desired_stop_thread_id == -1
- || desired_stop_thread_id == item.thread_id)
- {
- DEBUG_EVENTS ("pending stop anticipated, desired=0x%x, item=0x%x",
- desired_stop_thread_id, item.thread_id);
- return true;
- }
- }
-
- return false;
-}
-
-/* See nat/windows-nat.h. */
-
-std::optional<pending_stop>
-windows_process_info::fetch_pending_stop (bool debug_events)
-{
- std::optional<pending_stop> result;
- for (auto iter = pending_stops.begin ();
- iter != pending_stops.end ();
- ++iter)
- {
- if (desired_stop_thread_id == -1
- || desired_stop_thread_id == iter->thread_id)
- {
- result = *iter;
- current_event = iter->event;
-
- DEBUG_EVENTS ("pending stop found in 0x%x (desired=0x%x)",
- iter->thread_id, desired_stop_thread_id);
-
- pending_stops.erase (iter);
- break;
- }
- }
-
- return result;
-}
-
-/* See nat/windows-nat.h. */
-
BOOL
continue_last_debug_event (DWORD continue_status, bool debug_events)
{
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index fdbab0fc566..c268a12fd8f 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -32,6 +32,21 @@
namespace windows_nat
{
+/* Info about a potential pending stop. Each thread holds one of
+ these. See "windows_thread_info::pending_stop" for more
+ information. */
+struct pending_stop
+{
+ /* The target waitstatus we computed. TARGET_WAITKIND_IGNORE if the
+ thread does not have a pending stop. */
+ target_waitstatus status;
+
+ /* The event. A few fields of this can be referenced after a stop,
+ and it seemed simplest to store the entire event. */
+ DEBUG_EVENT event;
+};
+
+
/* Thread information structure used to track extra information about
each thread. */
struct windows_thread_info
@@ -71,6 +86,18 @@ struct windows_thread_info
was not. */
int suspended = 0;
+/* Info about a potential pending stop.
+
+ Sometimes, Windows will report a stop on a thread that has been
+ ostensibly suspended. We believe what happens here is that two
+ threads hit a breakpoint simultaneously, and the Windows kernel
+ queues the stop events. However, this can result in the strange
+ effect of trying to single step thread A -- leaving all other
+ threads suspended -- and then seeing a stop in thread B. To handle
+ this scenario, we queue all such "pending" stops here, and then
+ process them once the step has completed. See PR gdb/22992. */
+ struct pending_stop pending_stop {};
+
/* The context of the thread, including any manipulations. */
union
{
@@ -97,22 +124,6 @@ struct windows_thread_info
gdb::unique_xmalloc_ptr<char> name;
};
-
-/* A single pending stop. See "pending_stops" for more
- information. */
-struct pending_stop
-{
- /* The thread id. */
- DWORD thread_id;
-
- /* The target waitstatus we computed. */
- target_waitstatus status;
-
- /* The event. A few fields of this can be referenced after a stop,
- and it seemed simplest to store the entire event. */
- DEBUG_EVENT event;
-};
-
enum handle_exception_result
{
HANDLE_EXCEPTION_UNHANDLED = 0,
@@ -136,22 +147,6 @@ struct windows_process_info
stop. */
DEBUG_EVENT current_event {};
- /* The ID of the thread for which we anticipate a stop event.
- Normally this is -1, meaning we'll accept an event in any
- thread. */
- DWORD desired_stop_thread_id = -1;
-
- /* A vector of pending stops. Sometimes, Windows will report a stop
- on a thread that has been ostensibly suspended. We believe what
- happens here is that two threads hit a breakpoint simultaneously,
- and the Windows kernel queues the stop events. However, this can
- result in the strange effect of trying to single step thread A --
- leaving all other threads suspended -- and then seeing a stop in
- thread B. To handle this scenario, we queue all such "pending"
- stops here, and then process them once the step has completed. See
- PR gdb/22992. */
- std::vector<pending_stop> pending_stops;
-
/* Contents of $_siginfo */
EXCEPTION_RECORD siginfo_er {};
@@ -217,18 +212,6 @@ struct windows_process_info
void add_all_dlls ();
- /* Return true if there is a pending stop matching
- desired_stop_thread_id. If DEBUG_EVENTS is true, logging will be
- enabled. */
-
- bool matching_pending_stop (bool debug_events);
-
- /* See if a pending stop matches DESIRED_STOP_THREAD_ID. If so,
- remove it from the list of pending stops, set 'current_event', and
- return it. Otherwise, return an empty optional. */
-
- std::optional<pending_stop> fetch_pending_stop (bool debug_events);
-
const char *pid_to_exec_file (int);
private:
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 196f6a1f72d..888f85b1862 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1352,15 +1352,22 @@ BOOL
windows_nat_target::windows_continue (DWORD continue_status, int id,
windows_continue_flags cont_flags)
{
- windows_process.desired_stop_thread_id = id;
-
- if (windows_process.matching_pending_stop (debug_events))
+ for (auto &th : windows_process.thread_list)
{
- /* There's no need to really continue, because there's already
- another event pending. However, we do need to inform the
- event loop of this. */
- serial_event_set (m_wait_event);
- return TRUE;
+ if ((id == -1 || id == (int) th->tid)
+ && !th->suspended
+ && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE)
+ {
+ DEBUG_EVENTS ("got matching pending stop event "
+ "for 0x%x, not resuming",
+ th->tid);
+
+ /* There's no need to really continue, because there's already
+ another event pending. However, we do need to inform the
+ event loop of this. */
+ serial_event_set (m_wait_event);
+ return TRUE;
+ }
}
for (auto &th : windows_process.thread_list)
@@ -1547,20 +1554,24 @@ windows_nat_target::get_windows_debug_event
DWORD thread_id = 0;
/* If there is a relevant pending stop, report it now. See the
- comment by the definition of "pending_stops" for details on why
- this is needed. */
- std::optional<pending_stop> stop
- = windows_process.fetch_pending_stop (debug_events);
- if (stop.has_value ())
+ comment by the definition of "windows_thread_info::pending_stop"
+ for details on why this is needed. */
+ for (auto &th : windows_process.thread_list)
{
- thread_id = stop->thread_id;
- *ourstatus = stop->status;
- windows_process.current_event = stop->event;
+ if (!th->suspended
+ && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE)
+ {
+ DEBUG_EVENTS ("reporting pending event for 0x%x", th->tid);
+
+ thread_id = th->tid;
+ *ourstatus = th->pending_stop.status;
+ th->pending_stop.status.set_ignore ();
+ windows_process.current_event = th->pending_stop.event;
- ptid_t ptid (windows_process.current_event.dwProcessId, thread_id);
- windows_thread_info *th = windows_process.find_thread (ptid);
- windows_process.invalidate_context (th);
- return ptid;
+ ptid_t ptid (windows_process.process_id, thread_id);
+ windows_process.invalidate_context (th.get ());
+ return ptid;
+ }
}
windows_process.last_sig = GDB_SIGNAL_0;
@@ -1602,12 +1613,18 @@ windows_nat_target::get_windows_debug_event
}
/* Record the existence of this thread. */
thread_id = current_event->dwThreadId;
- add_thread
- (ptid_t (current_event->dwProcessId, current_event->dwThreadId, 0),
- current_event->u.CreateThread.hThread,
- current_event->u.CreateThread.lpThreadLocalBase,
- false /* main_thread_p */);
+ {
+ windows_thread_info *th
+ = (add_thread
+ (ptid_t (current_event->dwProcessId, current_event->dwThreadId, 0),
+ current_event->u.CreateThread.hThread,
+ current_event->u.CreateThread.lpThreadLocalBase,
+ false /* main_thread_p */));
+
+ /* This updates debug registers if necessary. */
+ windows_process.continue_one_thread (th, 0);
+ }
break;
case EXIT_THREAD_DEBUG_EVENT:
@@ -1619,6 +1636,7 @@ windows_nat_target::get_windows_debug_event
current_event->dwThreadId, 0),
current_event->u.ExitThread.dwExitCode,
false /* main_thread_p */);
+ thread_id = 0;
break;
case CREATE_PROCESS_DEBUG_EVENT:
@@ -1669,8 +1687,7 @@ windows_nat_target::get_windows_debug_event
ourstatus->set_exited (exit_status);
else
ourstatus->set_signalled (gdb_signal_from_host (exit_signal));
-
- thread_id = current_event->dwThreadId;
+ return ptid_t (current_event->dwProcessId);
}
break;
@@ -1760,17 +1777,22 @@ windows_nat_target::get_windows_debug_event
if (!thread_id || windows_process.saw_create != 1)
{
- CHECK (windows_continue (continue_status,
- windows_process.desired_stop_thread_id, 0));
+ continue_last_debug_event_main_thread
+ (_("Failed to resume program execution"), continue_status);
+ ourstatus->set_ignore ();
+ return null_ptid;
}
- else if (windows_process.desired_stop_thread_id != -1
- && windows_process.desired_stop_thread_id != thread_id)
+
+ const ptid_t ptid = ptid_t (current_event->dwProcessId, thread_id, 0);
+ windows_thread_info *th = windows_process.find_thread (ptid);
+
+ if (th->suspended)
{
/* Pending stop. See the comment by the definition of
"pending_stops" for details on why this is needed. */
DEBUG_EVENTS ("get_windows_debug_event - "
- "unexpected stop in 0x%x (expecting 0x%x)",
- thread_id, windows_process.desired_stop_thread_id);
+ "unexpected stop in suspended thread 0x%x",
+ thread_id);
if (current_event->dwDebugEventCode == EXCEPTION_DEBUG_EVENT
&& ((current_event->u.Exception.ExceptionRecord.ExceptionCode
@@ -1779,24 +1801,19 @@ windows_nat_target::get_windows_debug_event
== STATUS_WX86_BREAKPOINT))
&& windows_process.windows_initialization_done)
{
- ptid_t ptid = ptid_t (current_event->dwProcessId, thread_id, 0);
- windows_thread_info *th = windows_process.find_thread (ptid);
th->stopped_at_software_breakpoint = true;
th->pc_adjusted = false;
}
- windows_process.pending_stops.push_back
- ({thread_id, *ourstatus, windows_process.current_event});
- thread_id = 0;
- CHECK (windows_continue (continue_status,
- windows_process.desired_stop_thread_id, 0));
- }
-
- if (thread_id == 0)
- {
+ th->pending_stop.status = *ourstatus;
+ th->pending_stop.event = *current_event;
ourstatus->set_ignore ();
+
+ continue_last_debug_event_main_thread
+ (_("Failed to resume program execution"), continue_status);
return null_ptid;
}
- return ptid_t (windows_process.current_event.dwProcessId, thread_id, 0);
+
+ return ptid;
}
/* Wait for interesting events to occur in the target process. */
@@ -1822,8 +1839,8 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
if (ourstatus->kind () == TARGET_WAITKIND_SPURIOUS)
{
- CHECK (windows_continue (DBG_CONTINUE,
- windows_process.desired_stop_thread_id, 0));
+ continue_last_debug_event_main_thread
+ (_("Failed to resume program execution"), DBG_CONTINUE);
}
else if (ourstatus->kind () != TARGET_WAITKIND_IGNORE)
{
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 8dd8f21330d..8f51dd67dc2 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -427,9 +427,14 @@ continue_one_thread (thread_info *thread, int thread_id)
static BOOL
child_continue (DWORD continue_status, int thread_id)
{
- windows_process.desired_stop_thread_id = thread_id;
- if (windows_process.matching_pending_stop (debug_threads))
- return TRUE;
+ for (thread_info *thread : all_threads)
+ {
+ auto *th = (windows_thread_info *) thread_target_data (thread);
+ if ((thread_id == -1 || thread_id == (int) th->tid)
+ && !th->suspended
+ && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE)
+ return TRUE;
+ }
/* The inferior will only continue after the ContinueDebugEvent
call. */
@@ -1012,15 +1017,20 @@ get_child_debug_event (DWORD *continue_status,
windows_process.attaching = 0;
{
- std::optional<pending_stop> stop
- = windows_process.fetch_pending_stop (debug_threads);
- if (stop.has_value ())
+ for (thread_info *thread : all_threads)
{
- *ourstatus = stop->status;
- windows_process.current_event = stop->event;
- ptid = debug_event_ptid (&windows_process.current_event);
- switch_to_thread (find_thread_ptid (ptid));
- return 1;
+ auto *th = (windows_thread_info *) thread_target_data (thread);
+
+ if (!th->suspended
+ && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE)
+ {
+ *ourstatus = th->pending_stop.status;
+ th->pending_stop.status.set_ignore ();
+ windows_process.current_event = th->pending_stop.event;
+ ptid = debug_event_ptid (&windows_process.current_event);
+ switch_to_thread (find_thread_ptid (ptid));
+ return 1;
+ }
}
/* Keep the wait time low enough for comfortable remote
@@ -1112,7 +1122,7 @@ get_child_debug_event (DWORD *continue_status,
else
ourstatus->set_signalled (gdb_signal_from_host (exit_signal));
}
- child_continue (DBG_CONTINUE, windows_process.desired_stop_thread_id);
+ continue_last_debug_event (DBG_CONTINUE, debug_threads);
break;
case LOAD_DLL_DEBUG_EVENT:
@@ -1169,17 +1179,19 @@ get_child_debug_event (DWORD *continue_status,
ptid = debug_event_ptid (&windows_process.current_event);
- if (windows_process.desired_stop_thread_id != -1
- && windows_process.desired_stop_thread_id != ptid.lwp ())
+ windows_thread_info *th = windows_process.find_thread (ptid);
+
+ if (th != nullptr && th->suspended)
{
/* Pending stop. See the comment by the definition of
- "pending_stops" for details on why this is needed. */
+ "windows_thread_info::pending_stop" for details on why this
+ is needed. */
OUTMSG2 (("get_windows_debug_event - "
- "unexpected stop in 0x%lx (expecting 0x%x)\n",
- ptid.lwp (), windows_process.desired_stop_thread_id));
+ "unexpected stop in suspended thread 0x%x\n",
+ th->tid));
maybe_adjust_pc ();
- windows_process.pending_stops.push_back
- ({(DWORD) ptid.lwp (), *ourstatus, *current_event});
+ th->pending_stop.status = *ourstatus;
+ th->pending_stop.event = *current_event;
ourstatus->set_spurious ();
}
else
@@ -1239,8 +1251,7 @@ win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
[[fallthrough]];
case TARGET_WAITKIND_SPURIOUS:
/* do nothing, just continue */
- child_continue (continue_status,
- windows_process.desired_stop_thread_id);
+ continue_last_debug_event (continue_status, debug_threads);
break;
}
}
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 16/34] Windows gdb+gdbserver: Introduce get_last_debug_event_ptid
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (14 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 15/34] Windows gdb+gdbserver: Elim desired_stop_thread_id / rework pending_stops Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 17/34] Windows gdb: Can't pass signal to thread other than last stopped thread Pedro Alves
` (18 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
This will be used in subsequent patches to avoid using
DBG_EXCEPTION_NOT_HANDLED on the wrong thread.
Change-Id: I32915623b5036fb902f9830ce2d6f0b1ccf1f5cf
---
gdb/nat/windows-nat.c | 8 ++++++++
gdb/nat/windows-nat.h | 5 +++++
2 files changed, 13 insertions(+)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index a01d7b3c0c0..7f8834facd4 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -666,6 +666,14 @@ windows_process_info::add_all_dlls ()
/* See nat/windows-nat.h. */
+ptid_t
+get_last_debug_event_ptid ()
+{
+ return ptid_t (last_wait_event.dwProcessId, last_wait_event.dwThreadId, 0);
+}
+
+/* See nat/windows-nat.h. */
+
BOOL
continue_last_debug_event (DWORD continue_status, bool debug_events)
{
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index c268a12fd8f..70912fd6210 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -253,6 +253,11 @@ struct windows_process_info
extern BOOL continue_last_debug_event (DWORD continue_status,
bool debug_events);
+/* Return the ptid_t of the thread that the last waited-for event was
+ for. */
+
+extern ptid_t get_last_debug_event_ptid ();
+
/* A simple wrapper for WaitForDebugEvent that also sets the internal
'last_wait_event' on success. */
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 17/34] Windows gdb: Can't pass signal to thread other than last stopped thread
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (15 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 16/34] Windows gdb+gdbserver: Introduce get_last_debug_event_ptid Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 18/34] Windows gdbserver: Fix scheduler-locking Pedro Alves
` (17 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
Passing a signal to a thread other than the one that last reported an
event would be later possible with DBG_REPLY_LATER and a backend
working in non-stop mode.
With an all-stop backend that isn't possible, so at least don't
incorrectly consider passing DBG_EXCEPTION_NOT_HANDLED if the thread
that we're going to call ContinueDebugEvent for is not the one that
the user issued "signal SIG" on.
Change-Id: I32915623b5036fb902f9830ce2d6f0b1ccf1f5cf
---
gdb/windows-nat.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 888f85b1862..a34e230f66e 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1421,10 +1421,21 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
if (sig != GDB_SIGNAL_0)
{
- if (windows_process.current_event.dwDebugEventCode
+ /* Note it is OK to call get_last_debug_event_ptid() from the
+ main thread here, because we know the process_thread thread
+ isn't waiting for an event at this point, so there's no data
+ race. */
+ if (inferior_ptid != get_last_debug_event_ptid ())
+ {
+ /* ContinueDebugEvent will be for a different thread. */
+ DEBUG_EXCEPT ("Cannot continue with signal %d here. "
+ "Not last-event thread", sig);
+ }
+ else if (windows_process.current_event.dwDebugEventCode
!= EXCEPTION_DEBUG_EVENT)
{
- DEBUG_EXCEPT ("Cannot continue with signal %d here.", sig);
+ DEBUG_EXCEPT ("Cannot continue with signal %d here. "
+ "Not stopped for EXCEPTION_DEBUG_EVENT", sig);
}
else if (sig == windows_process.last_sig)
continue_status = DBG_EXCEPTION_NOT_HANDLED;
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 18/34] Windows gdbserver: Fix scheduler-locking
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (16 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 17/34] Windows gdb: Can't pass signal to thread other than last stopped thread Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 19/34] Windows gdb: Enable "set scheduler-locking on" Pedro Alves
` (16 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
This rewrites win32_process_target::resume such that scheduler-locking
is implemented properly.
It also uses the new get_last_debug_event_ptid function to avoid
considering passing a signal to the wrong thread, like done for the
native side in a previous patch.
Note this code/comment being removed:
- /* Yes, we're ignoring resume_info[0].thread. It'd be tricky to make
- the Windows resume code do the right thing for thread switching. */
- tid = windows_process.current_event.dwThreadId;
This meant that scheduler-locking currently is broken badly unless you
stay in the thread that last reported an event. If you switch to a
different thread from the one that last reported an event and step,
you get a spurious SIGTRAP in the thread that last reported a stop,
not the one that you tried to step:
(gdb) t 1
[Switching to thread 1 (Thread 3908)]
#0 0x00007fffc768d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) set scheduler-locking on
(gdb) set disassemble-next-line
"on", "off" or "auto" expected.
(gdb) set disassemble-next-line on
(gdb) frame
#0 0x00007fffc768d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
=> 0x00007fffc768d6e4 <ntdll!ZwDelayExecution+20>: c3 ret
(gdb) si
Thread 3 received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 2660]
0x00007fffc4e4e92e in KERNELBASE!EncodeRemotePointer () from target:C:/Windows/System32/KernelBase.dll
=> 0x00007fffc4e4e92e <KERNELBASE!EncodeRemotePointer+8254>: eb 78 jmp 0x7fffc4e4e9a8 <KERNELBASE!EncodeRemotePointer+8376>
(gdb)
Note how we switched to thread 1, stepped, and GDBserver still stepped
thread 3... This is fixed by this patch. We now get:
(gdb) info threads
Id Target Id Frame
1 Thread 920 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
2 Thread 8528 0x00007ffe03730ad4 in ntdll!ZwWaitForWorkViaWorkerFactory () from target:C:/Windows/SYSTEM32/ntdll.dll
3 Thread 3128 0x00007ffe03730ad4 in ntdll!ZwWaitForWorkViaWorkerFactory () from target:C:/Windows/SYSTEM32/ntdll.dll
* 4 Thread 7164 0x00007ffe0102e929 in KERNELBASE!EncodeRemotePointer () from target:C:/Windows/System32/KernelBase.dll
5 Thread 8348 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
6 Thread 2064 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) t 1
[Switching to thread 1 (Thread 920)]
#0 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) set scheduler-locking on
(gdb) si
0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) si
0x00007ffe00f9b44e in SleepEx () from target:C:/Windows/System32/KernelBase.dll
(gdb) si
0x00007ffe00f9b453 in SleepEx () from target:C:/Windows/System32/KernelBase.dll
I.e., we kept stepping the right thread, thread 1.
Note we stopped again at 0x00007ffe0372d6e4 the first time (same PC
the thread already was at before the first stepi) because the thread
had been stopped at a syscall, so that's normal:
(gdb) disassemble
Dump of assembler code for function ntdll!ZwDelayExecution:
0x00007ffe0372d6d0 <+0>: mov %rcx,%r10
0x00007ffe0372d6d3 <+3>: mov $0x34,%eax
0x00007ffe0372d6d8 <+8>: testb $0x1,0x7ffe0308
0x00007ffe0372d6e0 <+16>: jne 0x7ffe0372d6e5 <ntdll!ZwDelayExecution+21>
0x00007ffe0372d6e2 <+18>: syscall
=> 0x00007ffe0372d6e4 <+20>: ret
Change-Id: I44f4fe4cb98592517569c6716b9d189f42db25a0
---
gdbserver/win32-low.cc | 160 +++++++++++++++++++++--------------------
1 file changed, 82 insertions(+), 78 deletions(-)
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 8f51dd67dc2..7d37068ab91 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -425,17 +425,8 @@ continue_one_thread (thread_info *thread, int thread_id)
}
static BOOL
-child_continue (DWORD continue_status, int thread_id)
+child_continue_for_kill (DWORD continue_status, int thread_id)
{
- for (thread_info *thread : all_threads)
- {
- auto *th = (windows_thread_info *) thread_target_data (thread);
- if ((thread_id == -1 || thread_id == (int) th->tid)
- && !th->suspended
- && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE)
- return TRUE;
- }
-
/* The inferior will only continue after the ContinueDebugEvent
call. */
for_each_thread ([&] (thread_info *thread)
@@ -701,7 +692,7 @@ win32_process_target::kill (process_info *process)
TerminateProcess (windows_process.handle, 0);
for (;;)
{
- if (!child_continue (DBG_CONTINUE, -1))
+ if (!child_continue_for_kill (DBG_CONTINUE, -1))
break;
if (!wait_for_debug_event (&windows_process.current_event, INFINITE))
break;
@@ -768,96 +759,109 @@ win32_process_target::thread_alive (ptid_t ptid)
return find_thread_ptid (ptid) != NULL;
}
-/* Resume the inferior process. RESUME_INFO describes how we want
- to resume. */
-void
-win32_process_target::resume (thread_resume *resume_info, size_t n)
+static void
+resume_one_thread (thread_info *thread, bool step, gdb_signal sig,
+ DWORD *continue_status)
{
- DWORD tid;
- enum gdb_signal sig;
- int step;
- windows_thread_info *th;
- DWORD continue_status = DBG_CONTINUE;
- ptid_t ptid;
-
- /* This handles the very limited set of resume packets that GDB can
- currently produce. */
-
- if (n == 1 && resume_info[0].thread == minus_one_ptid)
- tid = -1;
- else if (n > 1)
- tid = -1;
- else
- /* Yes, we're ignoring resume_info[0].thread. It'd be tricky to make
- the Windows resume code do the right thing for thread switching. */
- tid = windows_process.current_event.dwThreadId;
-
- if (resume_info[0].thread != minus_one_ptid)
- {
- sig = gdb_signal_from_host (resume_info[0].sig);
- step = resume_info[0].kind == resume_step;
- }
- else
- {
- sig = GDB_SIGNAL_0;
- step = 0;
- }
+ auto *th = (windows_thread_info *) thread_target_data (thread);
if (sig != GDB_SIGNAL_0)
{
- if (windows_process.current_event.dwDebugEventCode
- != EXCEPTION_DEBUG_EVENT)
+ /* Allow continuing with the same signal that interrupted us.
+ Otherwise complain. */
+ if (thread->id != get_last_debug_event_ptid ())
{
- OUTMSG (("Cannot continue with signal %s here.\n",
+ /* ContinueDebugEvent will be for a different thread. */
+ OUTMSG (("Cannot continue with signal %d here. "
+ "Not last-event thread", sig));
+ }
+ else if (windows_process.current_event.dwDebugEventCode
+ != EXCEPTION_DEBUG_EVENT)
+ {
+ OUTMSG (("Cannot continue with signal %s here. "
+ "Not stopped for EXCEPTION_DEBUG_EVENT.\n",
gdb_signal_to_string (sig)));
}
else if (sig == windows_process.last_sig)
- continue_status = DBG_EXCEPTION_NOT_HANDLED;
+ *continue_status = DBG_EXCEPTION_NOT_HANDLED;
else
OUTMSG (("Can only continue with received signal %s.\n",
gdb_signal_to_string (windows_process.last_sig)));
}
- windows_process.last_sig = GDB_SIGNAL_0;
+ win32_prepare_to_resume (th);
- /* Get context for the currently selected thread. */
- ptid = debug_event_ptid (&windows_process.current_event);
- th = windows_process.find_thread (ptid);
- if (th)
- {
- win32_prepare_to_resume (th);
-
- DWORD *context_flags;
+ DWORD *context_flags;
#ifdef __x86_64__
- if (windows_process.wow64_process)
- context_flags = &th->wow64_context.ContextFlags;
- else
+ if (windows_process.wow64_process)
+ context_flags = &th->wow64_context.ContextFlags;
+ else
#endif
- context_flags = &th->context.ContextFlags;
- if (*context_flags)
+ context_flags = &th->context.ContextFlags;
+ if (*context_flags)
+ {
+ /* Move register values from the inferior into the thread
+ context structure. */
+ regcache_invalidate ();
+
+ if (step)
{
- /* Move register values from the inferior into the thread
- context structure. */
- regcache_invalidate ();
+ if (the_low_target.single_step != NULL)
+ (*the_low_target.single_step) (th);
+ else
+ error ("Single stepping is not supported "
+ "in this configuration.\n");
+ }
+
+ win32_set_thread_context (th);
+ *context_flags = 0;
+ }
+
+ continue_one_thread (thread, th->tid);
+}
+
+/* Resume the inferior process. RESUME_INFO describes how we want
+ to resume. */
+void
+win32_process_target::resume (thread_resume *resume_info, size_t n)
+{
+ DWORD continue_status = DBG_CONTINUE;
+ bool any_pending = false;
- if (step)
+ for (thread_info *thread : all_threads)
+ {
+ auto *th = (windows_thread_info *) thread_target_data (thread);
+
+ for (int ndx = 0; ndx < n; ndx++)
+ {
+ thread_resume &r = resume_info[ndx];
+ ptid_t ptid = r.thread;
+ gdb_signal sig = gdb_signal_from_host (r.sig);
+ bool step = r.kind == resume_step;
+
+ if (ptid == minus_one_ptid || ptid == thread->id
+ /* Handle both 'pPID' and 'pPID.-1' as meaning 'all threads
+ of PID'. */
+ || (ptid.pid () == pid_of (thread)
+ && (ptid.is_pid () || ptid.lwp () == -1)))
{
- if (the_low_target.single_step != NULL)
- (*the_low_target.single_step) (th);
- else
- error ("Single stepping is not supported "
- "in this configuration.\n");
- }
+ /* Ignore (wildcard) resume requests for already-resumed
+ threads. */
+ if (!th->suspended)
+ continue;
- win32_set_thread_context (th);
- *context_flags = 0;
+ resume_one_thread (thread, step, sig, &continue_status);
+ break;
+ }
}
- }
- /* Allow continuing with the same signal that interrupted us.
- Otherwise complain. */
+ if (!th->suspended
+ && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE)
+ any_pending = true;
+ }
- child_continue (continue_status, tid);
+ if (!any_pending)
+ continue_last_debug_event (continue_status, debug_threads);
}
/* See nat/windows-nat.h. */
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 19/34] Windows gdb: Enable "set scheduler-locking on"
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (17 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 18/34] Windows gdbserver: Fix scheduler-locking Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:25 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 20/34] Windows gdbserver: Eliminate soft-interrupt mechanism Pedro Alves
` (15 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
Surprisingly (to me), enabling scheduler locking on Windows currently
fails:
(gdb)
set scheduler-locking on
Target 'native' cannot support this command.
The backend itself does support scheduler-locking. This patch
implements windows_nat_target::get_thread_control_capabilities so that
the core knows schedlocking works for this target.
Change-Id: Ie762d3768fd70e4ac398c8bcc03c3213bfa26a6a
---
gdb/windows-nat.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index a34e230f66e..5ebf53e2816 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -261,6 +261,9 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
void close () override;
+ thread_control_capabilities get_thread_control_capabilities () override
+ { return tc_schedlock; }
+
void attach (const char *, int) override;
bool attach_no_wait () override
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 19/34] Windows gdb: Enable "set scheduler-locking on"
2024-05-07 23:42 ` [PATCH 19/34] Windows gdb: Enable "set scheduler-locking on" Pedro Alves
@ 2024-05-08 15:25 ` Tom Tromey
0 siblings, 0 replies; 64+ messages in thread
From: Tom Tromey @ 2024-05-08 15:25 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> Surprisingly (to me), enabling scheduler locking on Windows currently
Pedro> fails:
Pedro> (gdb)
Pedro> set scheduler-locking on
Pedro> Target 'native' cannot support this command.
Pedro> The backend itself does support scheduler-locking. This patch
Pedro> implements windows_nat_target::get_thread_control_capabilities so that
Pedro> the core knows schedlocking works for this target.
Oops.
This is ok once the earlier scheduler-locking patch goes in.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 20/34] Windows gdbserver: Eliminate soft-interrupt mechanism
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (18 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 19/34] Windows gdb: Enable "set scheduler-locking on" Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:26 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 21/34] Windows gdb+gdbserver: Make current_event per-thread state Pedro Alves
` (14 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
I noticed that faked_breakpoint is write only. And then I hacked
win32_process_target::request_interrupt to force it to stop threads
using the soft_interrupt_requested mechanism (which suspends threads,
and then fakes a breakpoint event in the main thread), and saw that it
no longer works -- gdbserver crashes accessing a NULL current_thread,
because fake_breakpoint_event does not switch to a thread.
This code was originally added for Windows CE, as neither
GenerateConsoleCtrlEvent nor DebugBreakProcess worked there.
We nowadays require Windows XP or later, and XP has DebugBreakProcess.
The soft_interrupt_requested mechanism has other problems, like for
example faking the event in the main thread, even if that thread was
previously stopped, due to scheduler-locking.
A following patch will add a similar mechanism stopping all threads
with SuspendThread to native GDB, for non-stop mode, which doesn't
have these problems. It's different enough from this old code that I
think we should just rip the old code out, and reimplement it from
scratch (based on gdb's version) when we need it.
Change-Id: I89e98233a9c40c6dcba7c8e1dacee08603843fb1
---
gdbserver/win32-low.cc | 32 +-------------------------------
gdbserver/win32-low.h | 8 --------
2 files changed, 1 insertion(+), 39 deletions(-)
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 7d37068ab91..99a6150599e 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -307,8 +307,6 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
windows_process.process_id = pid;
windows_process.main_thread_id = 0;
- windows_process.soft_interrupt_requested = 0;
- windows_process.faked_breakpoint = 0;
windows_process.open_process_used = true;
memset (&windows_process.current_event, 0,
@@ -433,7 +431,6 @@ child_continue_for_kill (DWORD continue_status, int thread_id)
{
continue_one_thread (thread, thread_id);
});
- windows_process.faked_breakpoint = 0;
return continue_last_debug_event (continue_status, debug_threads);
}
@@ -941,23 +938,6 @@ suspend_one_thread (thread_info *thread)
th->suspend ();
}
-static void
-fake_breakpoint_event (void)
-{
- OUTMSG2(("fake_breakpoint_event\n"));
-
- windows_process.faked_breakpoint = 1;
-
- memset (&windows_process.current_event, 0,
- sizeof (windows_process.current_event));
- windows_process.current_event.dwThreadId = windows_process.main_thread_id;
- windows_process.current_event.dwDebugEventCode = EXCEPTION_DEBUG_EVENT;
- windows_process.current_event.u.Exception.ExceptionRecord.ExceptionCode
- = EXCEPTION_BREAKPOINT;
-
- for_each_thread (suspend_one_thread);
-}
-
/* See nat/windows-nat.h. */
bool
@@ -1012,13 +992,6 @@ get_child_debug_event (DWORD *continue_status,
DEBUG_EVENT *current_event = &windows_process.current_event;
- if (windows_process.soft_interrupt_requested)
- {
- windows_process.soft_interrupt_requested = 0;
- fake_breakpoint_event ();
- goto gotevent;
- }
-
windows_process.attaching = 0;
{
for (thread_info *thread : all_threads)
@@ -1058,8 +1031,6 @@ get_child_debug_event (DWORD *continue_status,
}
}
- gotevent:
-
switch (current_event->dwDebugEventCode)
{
case CREATE_THREAD_DEBUG_EVENT:
@@ -1313,8 +1284,7 @@ win32_process_target::request_interrupt ()
if (DebugBreakProcess (windows_process.handle))
return;
- /* Last resort, suspend all threads manually. */
- windows_process.soft_interrupt_requested = 1;
+ OUTMSG2 (("Could not interrupt.\n"));
}
bool
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index 7ee4de4b37f..4d26b87d73d 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -187,14 +187,6 @@ struct gdbserver_windows_process : public windows_nat::windows_process_info
debug event off the win32 API. */
struct target_waitstatus cached_status;
- /* Non zero if an interrupt request is to be satisfied by suspending
- all threads. */
- int soft_interrupt_requested = 0;
-
- /* Non zero if the inferior is stopped in a simulated breakpoint done
- by suspending all the threads. */
- int faked_breakpoint = 0;
-
/* True if current_process_handle needs to be closed. */
bool open_process_used = false;
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 21/34] Windows gdb+gdbserver: Make current_event per-thread state
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (19 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 20/34] Windows gdbserver: Eliminate soft-interrupt mechanism Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 22/34] Windows gdb+gdbserver: Make last_sig " Pedro Alves
` (13 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
With non-stop mode, each thread is controlled independently of the
others, and each thread has its own independent reason for its last
stop.
Thus, any thread-specific state that is currently per-process must be
converted to per-thread state.
This patch converts windows_process_info::current_event, moving it to
windows_thread_info instead, renamed to last_event.
Since each thread will have its own copy of its last Windows debug
event, we no longer need the same information stored in struct
pending_stop.
Since windows_process.current_event no longer exists, we need to pass
the current event as parameter to a number of methods.
This adjusts both native gdb and gdbserver.
Change-Id: Ice09a5d932c912210608d5af25e1898f823e3c99
---
gdb/nat/windows-nat.c | 13 ++--
gdb/nat/windows-nat.h | 24 ++++----
gdb/windows-nat.c | 136 ++++++++++++++++++++++-------------------
gdbserver/win32-low.cc | 70 ++++++++++-----------
gdbserver/win32-low.h | 5 +-
5 files changed, 129 insertions(+), 119 deletions(-)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 7f8834facd4..cabc61fb2d2 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -310,8 +310,10 @@ get_image_name (HANDLE h, void *address, int unicode)
/* See nat/windows-nat.h. */
bool
-windows_process_info::handle_ms_vc_exception (const EXCEPTION_RECORD *rec)
+windows_process_info::handle_ms_vc_exception (const DEBUG_EVENT ¤t_event)
{
+ const EXCEPTION_RECORD *rec = ¤t_event.u.Exception.ExceptionRecord;
+
if (rec->NumberParameters >= 3
&& (rec->ExceptionInformation[0] & 0xffffffff) == 0x1000)
{
@@ -352,7 +354,8 @@ windows_process_info::handle_ms_vc_exception (const EXCEPTION_RECORD *rec)
#define MS_VC_EXCEPTION 0x406d1388
handle_exception_result
-windows_process_info::handle_exception (struct target_waitstatus *ourstatus,
+windows_process_info::handle_exception (DEBUG_EVENT ¤t_event,
+ struct target_waitstatus *ourstatus,
bool debug_exceptions)
{
#define DEBUG_EXCEPTION_SIMPLE(x) if (debug_exceptions) \
@@ -480,7 +483,7 @@ windows_process_info::handle_exception (struct target_waitstatus *ourstatus,
break;
case MS_VC_EXCEPTION:
DEBUG_EXCEPTION_SIMPLE ("MS_VC_EXCEPTION");
- if (handle_ms_vc_exception (rec))
+ if (handle_ms_vc_exception (current_event))
{
ourstatus->set_stopped (GDB_SIGNAL_TRAP);
result = HANDLE_EXCEPTION_IGNORED;
@@ -634,11 +637,11 @@ windows_process_info::add_dll (LPVOID load_addr)
/* See nat/windows-nat.h. */
void
-windows_process_info::dll_loaded_event ()
+windows_process_info::dll_loaded_event (const DEBUG_EVENT ¤t_event)
{
gdb_assert (current_event.dwDebugEventCode == LOAD_DLL_DEBUG_EVENT);
- LOAD_DLL_DEBUG_INFO *event = ¤t_event.u.LoadDll;
+ const LOAD_DLL_DEBUG_INFO *event = ¤t_event.u.LoadDll;
const char *dll_name;
/* Try getting the DLL name via the lpImageName field of the event.
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 70912fd6210..73c828ec2c9 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -40,10 +40,6 @@ struct pending_stop
/* The target waitstatus we computed. TARGET_WAITKIND_IGNORE if the
thread does not have a pending stop. */
target_waitstatus status;
-
- /* The event. A few fields of this can be referenced after a stop,
- and it seemed simplest to store the entire event. */
- DEBUG_EVENT event;
};
@@ -98,6 +94,10 @@ struct windows_thread_info
process them once the step has completed. See PR gdb/22992. */
struct pending_stop pending_stop {};
+ /* The last Windows event returned by WaitForDebugEvent for this
+ thread. */
+ DEBUG_EVENT last_event {};
+
/* The context of the thread, including any manipulations. */
union
{
@@ -143,10 +143,6 @@ struct windows_process_info
DWORD main_thread_id = 0;
enum gdb_signal last_sig = GDB_SIGNAL_0;
- /* The current debug event from WaitForDebugEvent or from a pending
- stop. */
- DEBUG_EVENT current_event {};
-
/* Contents of $_siginfo */
EXCEPTION_RECORD siginfo_er {};
@@ -170,7 +166,8 @@ struct windows_process_info
a Cygwin signal. Otherwise just print the string as a warning.
This function must be supplied by the embedding application. */
- virtual DWORD handle_output_debug_string (struct target_waitstatus *ourstatus) = 0;
+ virtual DWORD handle_output_debug_string (const DEBUG_EVENT ¤t_event,
+ struct target_waitstatus *ourstatus) = 0;
/* Handle a DLL load event.
@@ -191,7 +188,7 @@ struct windows_process_info
This function must be supplied by the embedding application. */
- virtual void handle_unload_dll () = 0;
+ virtual void handle_unload_dll (const DEBUG_EVENT ¤t_event) = 0;
/* When EXCEPTION_ACCESS_VIOLATION is processed, we give the embedding
application a chance to change it to be considered "unhandled".
@@ -201,11 +198,12 @@ struct windows_process_info
virtual bool handle_access_violation (const EXCEPTION_RECORD *rec) = 0;
handle_exception_result handle_exception
- (struct target_waitstatus *ourstatus, bool debug_exceptions);
+ (DEBUG_EVENT ¤t_event,
+ struct target_waitstatus *ourstatus, bool debug_exceptions);
/* Call to indicate that a DLL was loaded. */
- void dll_loaded_event ();
+ void dll_loaded_event (const DEBUG_EVENT ¤t_event);
/* Iterate over all DLLs currently mapped by our inferior, and
add them to our list of solibs. */
@@ -222,7 +220,7 @@ struct windows_process_info
Return true if the exception was handled; return false otherwise. */
- bool handle_ms_vc_exception (const EXCEPTION_RECORD *rec);
+ bool handle_ms_vc_exception (const DEBUG_EVENT ¤t_event);
/* Iterate over all DLLs currently mapped by our inferior, looking for
a DLL which is loaded at LOAD_ADDR. If found, add the DLL to our
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 5ebf53e2816..1a01ae57e54 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -107,9 +107,10 @@ DEF_ENUM_FLAGS_TYPE (windows_continue_flag, windows_continue_flags);
struct windows_per_inferior : public windows_process_info
{
windows_thread_info *find_thread (ptid_t ptid) override;
- DWORD handle_output_debug_string (struct target_waitstatus *ourstatus) override;
+ DWORD handle_output_debug_string (const DEBUG_EVENT ¤t_event,
+ struct target_waitstatus *ourstatus) override;
void handle_load_dll (const char *dll_name, LPVOID base) override;
- void handle_unload_dll () override;
+ void handle_unload_dll (const DEBUG_EVENT ¤t_event) override;
bool handle_access_violation (const EXCEPTION_RECORD *rec) override;
void invalidate_context (windows_thread_info *th);
@@ -321,7 +322,8 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
const char *thread_name (struct thread_info *) override;
ptid_t get_windows_debug_event (int pid, struct target_waitstatus *ourstatus,
- target_wait_flags options);
+ target_wait_flags options,
+ DEBUG_EVENT *current_event);
void do_initial_windows_stuff (DWORD pid, bool attaching);
@@ -352,7 +354,7 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
windows_thread_info *add_thread (ptid_t ptid, HANDLE h, void *tlb,
bool main_thread_p);
void delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p);
- DWORD fake_create_process ();
+ DWORD fake_create_process (const DEBUG_EVENT ¤t_event);
BOOL windows_continue (DWORD continue_status, int id,
windows_continue_flags cont_flags = 0);
@@ -979,7 +981,7 @@ windows_per_inferior::handle_load_dll (const char *dll_name, LPVOID base)
/* See nat/windows-nat.h. */
void
-windows_per_inferior::handle_unload_dll ()
+windows_per_inferior::handle_unload_dll (const DEBUG_EVENT ¤t_event)
{
LPVOID lpBaseOfDll = current_event.u.UnloadDll.lpBaseOfDll;
@@ -1042,7 +1044,8 @@ signal_event_command (const char *args, int from_tty)
DWORD
windows_per_inferior::handle_output_debug_string
- (struct target_waitstatus *ourstatus)
+ (const DEBUG_EVENT ¤t_event,
+ struct target_waitstatus *ourstatus)
{
DWORD thread_id = 0;
@@ -1387,11 +1390,11 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
/* Called in pathological case where Windows fails to send a
CREATE_PROCESS_DEBUG_EVENT after an attach. */
DWORD
-windows_nat_target::fake_create_process ()
+windows_nat_target::fake_create_process (const DEBUG_EVENT ¤t_event)
{
windows_process.handle
= OpenProcess (PROCESS_ALL_ACCESS, FALSE,
- windows_process.current_event.dwProcessId);
+ current_event.dwProcessId);
if (windows_process.handle != NULL)
windows_process.open_process_used = 1;
else
@@ -1400,12 +1403,11 @@ windows_nat_target::fake_create_process ()
throw_winerror_with_name (_("OpenProcess call failed"), err);
/* We can not debug anything in that case. */
}
- add_thread (ptid_t (windows_process.current_event.dwProcessId,
- windows_process.current_event.dwThreadId, 0),
- windows_process.current_event.u.CreateThread.hThread,
- windows_process.current_event.u.CreateThread.lpThreadLocalBase,
+ add_thread (ptid_t (current_event.dwProcessId, current_event.dwThreadId, 0),
+ current_event.u.CreateThread.hThread,
+ current_event.u.CreateThread.lpThreadLocalBase,
true /* main_thread_p */);
- return windows_process.current_event.dwThreadId;
+ return current_event.dwThreadId;
}
void
@@ -1422,6 +1424,13 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
if (resume_all)
ptid = inferior_ptid;
+ DEBUG_EXEC ("pid=%d, tid=0x%x, step=%d, sig=%d",
+ ptid.pid (), (unsigned) ptid.lwp (), step, sig);
+
+ /* Get currently selected thread. */
+ th = windows_process.find_thread (inferior_ptid);
+ gdb_assert (th != nullptr);
+
if (sig != GDB_SIGNAL_0)
{
/* Note it is OK to call get_last_debug_event_ptid() from the
@@ -1434,8 +1443,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
DEBUG_EXCEPT ("Cannot continue with signal %d here. "
"Not last-event thread", sig);
}
- else if (windows_process.current_event.dwDebugEventCode
- != EXCEPTION_DEBUG_EVENT)
+ else if (th->last_event.dwDebugEventCode != EXCEPTION_DEBUG_EVENT)
{
DEBUG_EXCEPT ("Cannot continue with signal %d here. "
"Not stopped for EXCEPTION_DEBUG_EVENT", sig);
@@ -1452,7 +1460,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
for (const xlate_exception &x : xlate)
if (x.us == sig)
{
- current_event.u.Exception.ExceptionRecord.ExceptionCode
+ th->last_event.u.Exception.ExceptionRecord.ExceptionCode
= x.them;
continue_status = DBG_EXCEPTION_NOT_HANDLED;
break;
@@ -1469,36 +1477,28 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
windows_process.last_sig = GDB_SIGNAL_0;
- DEBUG_EXEC ("pid=%d, tid=0x%x, step=%d, sig=%d",
- ptid.pid (), (unsigned) ptid.lwp (), step, sig);
-
- /* Get context for currently selected thread. */
- th = windows_process.find_thread (inferior_ptid);
- if (th)
- {
#ifdef __x86_64__
- if (windows_process.wow64_process)
+ if (windows_process.wow64_process)
+ {
+ if (step)
{
- if (step)
- {
- /* Single step by setting t bit. */
- regcache *regcache = get_thread_regcache (inferior_thread ());
- struct gdbarch *gdbarch = regcache->arch ();
- fetch_registers (regcache, gdbarch_ps_regnum (gdbarch));
- th->wow64_context.EFlags |= FLAG_TRACE_BIT;
- }
+ /* Single step by setting t bit. */
+ regcache *regcache = get_thread_regcache (inferior_thread ());
+ struct gdbarch *gdbarch = regcache->arch ();
+ fetch_registers (regcache, gdbarch_ps_regnum (gdbarch));
+ th->wow64_context.EFlags |= FLAG_TRACE_BIT;
}
- else
+ }
+ else
#endif
+ {
+ if (step)
{
- if (step)
- {
- /* Single step by setting t bit. */
- regcache *regcache = get_thread_regcache (inferior_thread ());
- struct gdbarch *gdbarch = regcache->arch ();
- fetch_registers (regcache, gdbarch_ps_regnum (gdbarch));
- th->context.EFlags |= FLAG_TRACE_BIT;
- }
+ /* Single step by setting t bit. */
+ regcache *regcache = get_thread_regcache (inferior_thread ());
+ struct gdbarch *gdbarch = regcache->arch ();
+ fetch_registers (regcache, gdbarch_ps_regnum (gdbarch));
+ th->context.EFlags |= FLAG_TRACE_BIT;
}
}
@@ -1562,7 +1562,8 @@ windows_nat_target::pass_ctrlc ()
ptid_t
windows_nat_target::get_windows_debug_event
- (int pid, struct target_waitstatus *ourstatus, target_wait_flags options)
+ (int pid, struct target_waitstatus *ourstatus, target_wait_flags options,
+ DEBUG_EVENT *current_event)
{
DWORD continue_status, event_code;
DWORD thread_id = 0;
@@ -1580,7 +1581,7 @@ windows_nat_target::get_windows_debug_event
thread_id = th->tid;
*ourstatus = th->pending_stop.status;
th->pending_stop.status.set_ignore ();
- windows_process.current_event = th->pending_stop.event;
+ *current_event = th->last_event;
ptid_t ptid (windows_process.process_id, thread_id);
windows_process.invalidate_context (th.get ());
@@ -1589,7 +1590,6 @@ windows_nat_target::get_windows_debug_event
}
windows_process.last_sig = GDB_SIGNAL_0;
- DEBUG_EVENT *current_event = &windows_process.current_event;
if ((options & TARGET_WNOHANG) != 0 && !m_debug_event_pending)
{
@@ -1597,11 +1597,11 @@ windows_nat_target::get_windows_debug_event
return minus_one_ptid;
}
- wait_for_debug_event_main_thread (&windows_process.current_event);
+ wait_for_debug_event_main_thread (current_event);
continue_status = DBG_CONTINUE;
- event_code = windows_process.current_event.dwDebugEventCode;
+ event_code = current_event->dwDebugEventCode;
ourstatus->set_spurious ();
switch (event_code)
@@ -1619,7 +1619,7 @@ windows_nat_target::get_windows_debug_event
/* Kludge around a Windows bug where first event is a create
thread event. Caused when attached process does not have
a main thread. */
- thread_id = fake_create_process ();
+ thread_id = fake_create_process (*current_event);
if (thread_id)
windows_process.saw_create++;
}
@@ -1716,7 +1716,7 @@ windows_nat_target::get_windows_debug_event
break;
try
{
- windows_process.dll_loaded_event ();
+ windows_process.dll_loaded_event (*current_event);
}
catch (const gdb_exception &ex)
{
@@ -1736,7 +1736,7 @@ windows_nat_target::get_windows_debug_event
break;
try
{
- windows_process.handle_unload_dll ();
+ windows_process.handle_unload_dll (*current_event);
}
catch (const gdb_exception &ex)
{
@@ -1753,7 +1753,8 @@ windows_nat_target::get_windows_debug_event
"EXCEPTION_DEBUG_EVENT");
if (windows_process.saw_create != 1)
break;
- switch (windows_process.handle_exception (ourstatus, debug_exceptions))
+ switch (windows_process.handle_exception (*current_event,
+ ourstatus, debug_exceptions))
{
case HANDLE_EXCEPTION_UNHANDLED:
default:
@@ -1775,7 +1776,8 @@ windows_nat_target::get_windows_debug_event
"OUTPUT_DEBUG_STRING_EVENT");
if (windows_process.saw_create != 1)
break;
- thread_id = windows_process.handle_output_debug_string (ourstatus);
+ thread_id = windows_process.handle_output_debug_string (*current_event,
+ ourstatus);
break;
default:
@@ -1800,6 +1802,8 @@ windows_nat_target::get_windows_debug_event
const ptid_t ptid = ptid_t (current_event->dwProcessId, thread_id, 0);
windows_thread_info *th = windows_process.find_thread (ptid);
+ th->last_event = *current_event;
+
if (th->suspended)
{
/* Pending stop. See the comment by the definition of
@@ -1818,8 +1822,8 @@ windows_nat_target::get_windows_debug_event
th->stopped_at_software_breakpoint = true;
th->pc_adjusted = false;
}
+
th->pending_stop.status = *ourstatus;
- th->pending_stop.event = *current_event;
ourstatus->set_ignore ();
continue_last_debug_event_main_thread
@@ -1845,7 +1849,10 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
while (1)
{
- ptid_t result = get_windows_debug_event (pid, ourstatus, options);
+ DEBUG_EVENT current_event;
+
+ ptid_t result = get_windows_debug_event (pid, ourstatus, options,
+ ¤t_event);
if ((options & TARGET_WNOHANG) != 0
&& ourstatus->kind () == TARGET_WAITKIND_IGNORE)
@@ -1864,11 +1871,11 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
windows_thread_info *th = windows_process.find_thread (result);
th->stopped_at_software_breakpoint = false;
- if (windows_process.current_event.dwDebugEventCode
+ if (current_event.dwDebugEventCode
== EXCEPTION_DEBUG_EVENT
- && ((windows_process.current_event.u.Exception.ExceptionRecord.ExceptionCode
+ && ((current_event.u.Exception.ExceptionRecord.ExceptionCode
== EXCEPTION_BREAKPOINT)
- || (windows_process.current_event.u.Exception.ExceptionRecord.ExceptionCode
+ || (current_event.u.Exception.ExceptionRecord.ExceptionCode
== STATUS_WX86_BREAKPOINT))
&& windows_process.windows_initialization_done)
{
@@ -1909,8 +1916,6 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching)
windows_process.cygwin_load_end = 0;
#endif
windows_process.process_id = pid;
- memset (&windows_process.current_event, 0,
- sizeof (windows_process.current_event));
inf = current_inferior ();
if (!inf->target_is_pushed (this))
inf->push_target (this);
@@ -1957,7 +1962,10 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching)
&& status.kind () != TARGET_WAITKIND_SPURIOUS)
break;
- this->resume (minus_one_ptid, 0, GDB_SIGNAL_0);
+ /* Don't use windows_nat_target::resume here because that
+ assumes that inferior_ptid points at a valid thread, and we
+ haven't switched to any thread yet. */
+ windows_continue (DBG_CONTINUE, -1);
}
switch_to_thread (this->find_thread (last_ptid));
@@ -2233,7 +2241,7 @@ windows_nat_target::detach (inferior *inf, int from_tty)
if (process_alive)
do_synchronously ([&] ()
{
- if (!DebugActiveProcessStop (windows_process.current_event.dwProcessId))
+ if (!DebugActiveProcessStop (windows_process.process_id))
err = (unsigned) GetLastError ();
else
DebugSetProcessKillOnExit (FALSE);
@@ -2244,7 +2252,7 @@ windows_nat_target::detach (inferior *inf, int from_tty)
{
std::string msg
= string_printf (_("Can't detach process %u"),
- (unsigned) windows_process.current_event.dwProcessId);
+ windows_process.process_id);
throw_winerror_with_name (msg.c_str (), *err);
}
@@ -3041,9 +3049,9 @@ windows_nat_target::kill ()
{
if (!windows_continue (DBG_CONTINUE, -1, WCONT_KILLED))
break;
- wait_for_debug_event_main_thread (&windows_process.current_event);
- if (windows_process.current_event.dwDebugEventCode
- == EXIT_PROCESS_DEBUG_EVENT)
+ DEBUG_EVENT current_event;
+ wait_for_debug_event_main_thread (¤t_event);
+ if (current_event.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT)
break;
}
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 99a6150599e..5608fb902b1 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -309,9 +309,6 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
windows_process.open_process_used = true;
- memset (&windows_process.current_event, 0,
- sizeof (windows_process.current_event));
-
#ifdef __x86_64__
BOOL wow64;
if (!IsWow64Process (proch, &wow64))
@@ -622,7 +619,8 @@ win32_process_target::attach (unsigned long pid)
DWORD
gdbserver_windows_process::handle_output_debug_string
- (struct target_waitstatus *ourstatus)
+ (const DEBUG_EVENT ¤t_event,
+ struct target_waitstatus *ourstatus)
{
#define READ_BUFFER_LEN 1024
CORE_ADDR addr;
@@ -691,14 +689,13 @@ win32_process_target::kill (process_info *process)
{
if (!child_continue_for_kill (DBG_CONTINUE, -1))
break;
- if (!wait_for_debug_event (&windows_process.current_event, INFINITE))
+ DEBUG_EVENT current_event;
+ if (!wait_for_debug_event (¤t_event, INFINITE))
break;
- if (windows_process.current_event.dwDebugEventCode
- == EXIT_PROCESS_DEBUG_EVENT)
+ if (current_event.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT)
break;
- else if (windows_process.current_event.dwDebugEventCode
- == OUTPUT_DEBUG_STRING_EVENT)
- windows_process.handle_output_debug_string (nullptr);
+ else if (current_event.dwDebugEventCode == OUTPUT_DEBUG_STRING_EVENT)
+ windows_process.handle_output_debug_string (current_event, nullptr);
}
win32_clear_inferiors ();
@@ -772,8 +769,7 @@ resume_one_thread (thread_info *thread, bool step, gdb_signal sig,
OUTMSG (("Cannot continue with signal %d here. "
"Not last-event thread", sig));
}
- else if (windows_process.current_event.dwDebugEventCode
- != EXCEPTION_DEBUG_EVENT)
+ else if (th->last_event.dwDebugEventCode != EXCEPTION_DEBUG_EVENT)
{
OUTMSG (("Cannot continue with signal %s here. "
"Not stopped for EXCEPTION_DEBUG_EVENT.\n",
@@ -918,7 +914,7 @@ gdbserver_windows_process::handle_load_dll (const char *name, LPVOID base)
/* See nat/windows-nat.h. */
void
-gdbserver_windows_process::handle_unload_dll ()
+gdbserver_windows_process::handle_unload_dll (const DEBUG_EVENT ¤t_event)
{
CORE_ADDR load_addr =
(CORE_ADDR) (uintptr_t) current_event.u.UnloadDll.lpBaseOfDll;
@@ -952,7 +948,7 @@ gdbserver_windows_process::handle_access_violation
PC. */
static void
-maybe_adjust_pc ()
+maybe_adjust_pc (const DEBUG_EVENT ¤t_event)
{
struct regcache *regcache = get_thread_regcache (current_thread, 1);
child_fetch_inferior_registers (regcache, -1);
@@ -961,10 +957,10 @@ maybe_adjust_pc ()
= windows_process.find_thread (current_thread_ptid ());
th->stopped_at_software_breakpoint = false;
- if (windows_process.current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
- && ((windows_process.current_event.u.Exception.ExceptionRecord.ExceptionCode
+ if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
+ && ((current_event.u.Exception.ExceptionRecord.ExceptionCode
== EXCEPTION_BREAKPOINT)
- || (windows_process.current_event.u.Exception.ExceptionRecord.ExceptionCode
+ || (current_event.u.Exception.ExceptionRecord.ExceptionCode
== STATUS_WX86_BREAKPOINT))
&& windows_process.child_initialization_done)
{
@@ -979,7 +975,8 @@ maybe_adjust_pc ()
static int
get_child_debug_event (DWORD *continue_status,
- struct target_waitstatus *ourstatus)
+ struct target_waitstatus *ourstatus,
+ DEBUG_EVENT *current_event)
{
ptid_t ptid;
@@ -990,8 +987,6 @@ get_child_debug_event (DWORD *continue_status,
/* Check if GDB sent us an interrupt request. */
check_remote_input_interrupt_request ();
- DEBUG_EVENT *current_event = &windows_process.current_event;
-
windows_process.attaching = 0;
{
for (thread_info *thread : all_threads)
@@ -1003,8 +998,8 @@ get_child_debug_event (DWORD *continue_status,
{
*ourstatus = th->pending_stop.status;
th->pending_stop.status.set_ignore ();
- windows_process.current_event = th->pending_stop.event;
- ptid = debug_event_ptid (&windows_process.current_event);
+ *current_event = th->last_event;
+ ptid = debug_event_ptid (current_event);
switch_to_thread (find_thread_ptid (ptid));
return 1;
}
@@ -1013,7 +1008,7 @@ get_child_debug_event (DWORD *continue_status,
/* Keep the wait time low enough for comfortable remote
interruption, but high enough so gdbserver doesn't become a
bottleneck. */
- if (!wait_for_debug_event (&windows_process.current_event, 250))
+ if (!wait_for_debug_event (current_event, 250))
{
DWORD e = GetLastError();
@@ -1108,7 +1103,7 @@ get_child_debug_event (DWORD *continue_status,
CloseHandle (current_event->u.LoadDll.hFile);
if (! windows_process.child_initialization_done)
break;
- windows_process.dll_loaded_event ();
+ windows_process.dll_loaded_event (*current_event);
ourstatus->set_loaded ();
break;
@@ -1120,7 +1115,7 @@ get_child_debug_event (DWORD *continue_status,
(unsigned) current_event->dwThreadId));
if (! windows_process.child_initialization_done)
break;
- windows_process.handle_unload_dll ();
+ windows_process.handle_unload_dll (*current_event);
ourstatus->set_loaded ();
break;
@@ -1129,7 +1124,8 @@ get_child_debug_event (DWORD *continue_status,
"for pid=%u tid=%x\n",
(unsigned) current_event->dwProcessId,
(unsigned) current_event->dwThreadId));
- if (windows_process.handle_exception (ourstatus, debug_threads)
+ if (windows_process.handle_exception (*current_event,
+ ourstatus, debug_threads)
== HANDLE_EXCEPTION_UNHANDLED)
*continue_status = DBG_EXCEPTION_NOT_HANDLED;
break;
@@ -1140,7 +1136,7 @@ get_child_debug_event (DWORD *continue_status,
"for pid=%u tid=%x\n",
(unsigned) current_event->dwProcessId,
(unsigned) current_event->dwThreadId));
- windows_process.handle_output_debug_string (nullptr);
+ windows_process.handle_output_debug_string (*current_event, nullptr);
break;
default:
@@ -1152,10 +1148,12 @@ get_child_debug_event (DWORD *continue_status,
break;
}
- ptid = debug_event_ptid (&windows_process.current_event);
+ ptid = debug_event_ptid (current_event);
windows_thread_info *th = windows_process.find_thread (ptid);
+ th->last_event = *current_event;
+
if (th != nullptr && th->suspended)
{
/* Pending stop. See the comment by the definition of
@@ -1164,9 +1162,8 @@ get_child_debug_event (DWORD *continue_status,
OUTMSG2 (("get_windows_debug_event - "
"unexpected stop in suspended thread 0x%x\n",
th->tid));
- maybe_adjust_pc ();
+ maybe_adjust_pc (*current_event);
th->pending_stop.status = *ourstatus;
- th->pending_stop.event = *current_event;
ourstatus->set_spurious ();
}
else
@@ -1190,13 +1187,16 @@ win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
fails). Report it now. */
*ourstatus = windows_process.cached_status;
windows_process.cached_status.set_ignore ();
- return debug_event_ptid (&windows_process.current_event);
+ return ptid_t (windows_process.process_id,
+ windows_process.main_thread_id, 0);
}
while (1)
{
DWORD continue_status;
- if (!get_child_debug_event (&continue_status, ourstatus))
+ DEBUG_EVENT current_event;
+ if (!get_child_debug_event (&continue_status, ourstatus,
+ ¤t_event))
continue;
switch (ourstatus->kind ())
@@ -1205,20 +1205,20 @@ win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
OUTMSG2 (("Child exited with retcode = %x\n",
ourstatus->exit_status ()));
win32_clear_inferiors ();
- return ptid_t (windows_process.current_event.dwProcessId);
+ return ptid_t (windows_process.process_id);
case TARGET_WAITKIND_STOPPED:
case TARGET_WAITKIND_SIGNALLED:
case TARGET_WAITKIND_LOADED:
{
OUTMSG2 (("Child Stopped with signal = %d \n",
ourstatus->sig ()));
- maybe_adjust_pc ();
+ maybe_adjust_pc (current_event);
/* 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 (¤t_event);
}
default:
OUTMSG (("Ignoring unknown internal event, %d\n",
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index 4d26b87d73d..e99e47ea829 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -175,9 +175,10 @@ class win32_process_target : public process_stratum_target
struct gdbserver_windows_process : public windows_nat::windows_process_info
{
windows_nat::windows_thread_info *find_thread (ptid_t ptid) override;
- DWORD handle_output_debug_string (struct target_waitstatus *ourstatus) override;
+ DWORD handle_output_debug_string (const DEBUG_EVENT ¤t_event,
+ struct target_waitstatus *ourstatus) override;
void handle_load_dll (const char *dll_name, LPVOID base) override;
- void handle_unload_dll () override;
+ void handle_unload_dll (const DEBUG_EVENT ¤t_event) override;
bool handle_access_violation (const EXCEPTION_RECORD *rec) override;
int attaching = 0;
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 22/34] Windows gdb+gdbserver: Make last_sig per-thread state
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (20 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 21/34] Windows gdb+gdbserver: Make current_event per-thread state Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 23/34] Windows gdb+gdbserver: Make siginfo_er " Pedro Alves
` (12 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
With non-stop mode, each thread is controlled independently of the
others, and each thread has its own independent reason for its last
stop.
Thus, any thread-specific state that is currently per-process must be
converted to per-thread state.
This patch converts windows_process_info::last_sig to per-thread
state, moving it to windows_thread_info instead.
This adjusts both native gdb and gdbserver.
Change-Id: Ice09a5d932c912210608d5af25e1898f823e3c99
---
gdb/nat/windows-nat.c | 8 +++++---
gdb/nat/windows-nat.h | 5 ++++-
gdb/windows-nat.c | 10 +++-------
gdbserver/win32-low.cc | 7 +++----
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index cabc61fb2d2..57604312ccb 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -369,8 +369,6 @@ windows_process_info::handle_exception (DEBUG_EVENT ¤t_event,
memcpy (&siginfo_er, rec, sizeof siginfo_er);
- last_sig = GDB_SIGNAL_0;
-
switch (code)
{
case EXCEPTION_ACCESS_VIOLATION:
@@ -504,7 +502,11 @@ windows_process_info::handle_exception (DEBUG_EVENT ¤t_event,
}
if (ourstatus->kind () == TARGET_WAITKIND_STOPPED)
- last_sig = ourstatus->sig ();
+ {
+ ptid_t ptid (current_event.dwProcessId, current_event.dwThreadId, 0);
+ windows_thread_info *th = find_thread (ptid);
+ th->last_sig = ourstatus->sig ();
+ }
return result;
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 73c828ec2c9..aab76d66ec8 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -98,6 +98,10 @@ struct windows_thread_info
thread. */
DEBUG_EVENT last_event {};
+ /* The last signal reported for this thread, extracted out of
+ last_event. */
+ enum gdb_signal last_sig = GDB_SIGNAL_0;
+
/* The context of the thread, including any manipulations. */
union
{
@@ -141,7 +145,6 @@ struct windows_process_info
HANDLE handle = 0;
DWORD process_id = 0;
DWORD main_thread_id = 0;
- enum gdb_signal last_sig = GDB_SIGNAL_0;
/* Contents of $_siginfo */
EXCEPTION_RECORD siginfo_er {};
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 1a01ae57e54..dd59bd2d383 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1348,6 +1348,7 @@ windows_per_inferior::continue_one_thread (windows_thread_info *th,
}
}
th->resume ();
+ th->last_sig = GDB_SIGNAL_0;
}
/* Resume thread specified by ID, or all artificially suspended
@@ -1448,7 +1449,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
DEBUG_EXCEPT ("Cannot continue with signal %d here. "
"Not stopped for EXCEPTION_DEBUG_EVENT", sig);
}
- else if (sig == windows_process.last_sig)
+ else if (sig == th->last_sig)
continue_status = DBG_EXCEPTION_NOT_HANDLED;
else
#if 0
@@ -1472,11 +1473,9 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
}
#endif
DEBUG_EXCEPT ("Can only continue with received signal %d.",
- windows_process.last_sig);
+ th->last_sig);
}
- windows_process.last_sig = GDB_SIGNAL_0;
-
#ifdef __x86_64__
if (windows_process.wow64_process)
{
@@ -1589,8 +1588,6 @@ windows_nat_target::get_windows_debug_event
}
}
- windows_process.last_sig = GDB_SIGNAL_0;
-
if ((options & TARGET_WNOHANG) != 0 && !m_debug_event_pending)
{
ourstatus->set_ignore ();
@@ -1909,7 +1906,6 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching)
{
struct inferior *inf;
- windows_process.last_sig = GDB_SIGNAL_0;
windows_process.open_process_used = 0;
#ifdef __CYGWIN__
windows_process.cygwin_load_start = 0;
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 5608fb902b1..40575e36501 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -302,7 +302,6 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
{
struct process_info *proc;
- windows_process.last_sig = GDB_SIGNAL_0;
windows_process.handle = proch;
windows_process.process_id = pid;
windows_process.main_thread_id = 0;
@@ -415,6 +414,7 @@ continue_one_thread (thread_info *thread, int thread_id)
}
th->resume ();
+ th->last_sig = GDB_SIGNAL_0;
}
}
}
@@ -775,11 +775,11 @@ resume_one_thread (thread_info *thread, bool step, gdb_signal sig,
"Not stopped for EXCEPTION_DEBUG_EVENT.\n",
gdb_signal_to_string (sig)));
}
- else if (sig == windows_process.last_sig)
+ else if (sig == th->last_sig)
*continue_status = DBG_EXCEPTION_NOT_HANDLED;
else
OUTMSG (("Can only continue with received signal %s.\n",
- gdb_signal_to_string (windows_process.last_sig)));
+ gdb_signal_to_string (th->last_sig)));
}
win32_prepare_to_resume (th);
@@ -980,7 +980,6 @@ get_child_debug_event (DWORD *continue_status,
{
ptid_t ptid;
- windows_process.last_sig = GDB_SIGNAL_0;
ourstatus->set_spurious ();
*continue_status = DBG_CONTINUE;
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 23/34] Windows gdb+gdbserver: Make siginfo_er per-thread state
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (21 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 22/34] Windows gdb+gdbserver: Make last_sig " Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 24/34] Add backpointer from windows_thread_info to windows_process_info Pedro Alves
` (11 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
With non-stop mode support, each thread has its own "last event", and
so printing $_siginfo should print the siginfo of the selected thread.
Likewise, with all-stop and scheduler-locking.
This patch reworks the siginfo functions in gdb/windows-nat.c and
gdbserver/win32-low.cc to reuse the exception record already saved
within each thread's 'last_event' field.
Here's an example of what you'll see after the whole non-stop series:
(gdb) thread apply all p -pretty -- $_siginfo
Thread 3 (Thread 2612.0x1470):
$1 = {
ExceptionCode = DBG_CONTROL_C,
ExceptionFlags = 0,
ExceptionRecord = 0x0,
ExceptionAddress = 0x7ffd0583e929 <KERNELBASE!EncodeRemotePointer+8249>,
NumberParameters = 0,
{
ExceptionInformation = {0 <repeats 15 times>},
AccessViolationInformation = {
Type = READ_ACCESS_VIOLATION,
Address = 0x0
}
}
}
Thread 2 (Thread 2612.0x1704):
$2 = {
ExceptionCode = SINGLE_STEP,
ExceptionFlags = 0,
ExceptionRecord = 0x0,
ExceptionAddress = 0x7ffd080ad6e4 <ntdll!ZwDelayExecution+20>,
NumberParameters = 0,
{
ExceptionInformation = {0 <repeats 15 times>},
AccessViolationInformation = {
Type = READ_ACCESS_VIOLATION,
Address = 0x0
}
}
}
Thread 1 (Thread 2612.0x434):
$3 = {
ExceptionCode = BREAKPOINT,
ExceptionFlags = 0,
ExceptionRecord = 0x0,
ExceptionAddress = 0x7ff6f691174c <main+185>,
NumberParameters = 1,
{
ExceptionInformation = {0 <repeats 15 times>},
AccessViolationInformation = {
Type = READ_ACCESS_VIOLATION,
Address = 0x0
}
}
}
(gdb)
This was in non-stop mode, and the program originally had two threads.
Thread 1 stopped for a breakpoint, then thread 2 was manually
interrupted/paused and then single-stepped. And then I typed Ctrl-C
in the inferior's terminal, which made Windows inject thread 3 in the
inferior, and report a DBG_CONTROL_C exception for it.
Change-Id: I5d4f1b62f59e8aef3606642c6524df2362b0fb7d
---
gdb/nat/windows-nat.c | 2 --
gdb/nat/windows-nat.h | 3 ---
gdb/windows-nat.c | 33 ++++++++++++++++-----------------
gdbserver/win32-low.cc | 28 +++++++++++++++-------------
4 files changed, 31 insertions(+), 35 deletions(-)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 57604312ccb..d43d549cb0d 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -367,8 +367,6 @@ windows_process_info::handle_exception (DEBUG_EVENT ¤t_event,
DWORD code = rec->ExceptionCode;
handle_exception_result result = HANDLE_EXCEPTION_HANDLED;
- memcpy (&siginfo_er, rec, sizeof siginfo_er);
-
switch (code)
{
case EXCEPTION_ACCESS_VIOLATION:
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index aab76d66ec8..86ad8d02e3c 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -146,9 +146,6 @@ struct windows_process_info
DWORD process_id = 0;
DWORD main_thread_id = 0;
- /* Contents of $_siginfo */
- EXCEPTION_RECORD siginfo_er {};
-
#ifdef __x86_64__
/* The target is a WOW64 process */
bool wow64_process = false;
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index dd59bd2d383..510820d862d 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2992,7 +2992,6 @@ windows_nat_target::mourn_inferior ()
CHECK (CloseHandle (windows_process.handle));
windows_process.open_process_used = 0;
}
- windows_process.siginfo_er.ExceptionCode = 0;
inf_child_target::mourn_inferior ();
}
@@ -3109,8 +3108,15 @@ static enum target_xfer_status
windows_xfer_siginfo (gdb_byte *readbuf, ULONGEST offset, ULONGEST len,
ULONGEST *xfered_len)
{
- char *buf = (char *) &windows_process.siginfo_er;
- size_t bufsize = sizeof (windows_process.siginfo_er);
+ windows_thread_info *th = windows_process.find_thread (inferior_ptid);
+
+ if (th->last_event.dwDebugEventCode != EXCEPTION_DEBUG_EVENT)
+ return TARGET_XFER_E_IO;
+
+ EXCEPTION_RECORD &er = th->last_event.u.Exception.ExceptionRecord;
+
+ char *buf = (char *) &er;
+ size_t bufsize = sizeof (er);
#ifdef __x86_64__
EXCEPTION_RECORD32 er32;
@@ -3119,23 +3125,16 @@ windows_xfer_siginfo (gdb_byte *readbuf, ULONGEST offset, ULONGEST len,
buf = (char *) &er32;
bufsize = sizeof (er32);
- er32.ExceptionCode = windows_process.siginfo_er.ExceptionCode;
- er32.ExceptionFlags = windows_process.siginfo_er.ExceptionFlags;
- er32.ExceptionRecord
- = (uintptr_t) windows_process.siginfo_er.ExceptionRecord;
- er32.ExceptionAddress
- = (uintptr_t) windows_process.siginfo_er.ExceptionAddress;
- er32.NumberParameters = windows_process.siginfo_er.NumberParameters;
- int i;
- for (i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; i++)
- er32.ExceptionInformation[i]
- = windows_process.siginfo_er.ExceptionInformation[i];
+ er32.ExceptionCode = er.ExceptionCode;
+ er32.ExceptionFlags = er.ExceptionFlags;
+ er32.ExceptionRecord = (uintptr_t) er.ExceptionRecord;
+ er32.ExceptionAddress = (uintptr_t) er.ExceptionAddress;
+ er32.NumberParameters = er.NumberParameters;
+ for (int i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; i++)
+ er32.ExceptionInformation[i] = er.ExceptionInformation[i];
}
#endif
- if (windows_process.siginfo_er.ExceptionCode == 0)
- return TARGET_XFER_E_IO;
-
if (readbuf == nullptr)
return TARGET_XFER_E_IO;
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 40575e36501..55600910ffe 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -675,7 +675,6 @@ win32_clear_inferiors (void)
}
for_each_thread (delete_thread_info);
- windows_process.siginfo_er.ExceptionCode = 0;
clear_inferiors ();
}
@@ -1306,14 +1305,19 @@ win32_process_target::qxfer_siginfo (const char *annex,
unsigned const char *writebuf,
CORE_ADDR offset, int len)
{
- if (windows_process.siginfo_er.ExceptionCode == 0)
+ windows_thread_info *th
+ = windows_process.find_thread (current_thread_ptid ());
+
+ if (th->last_event.dwDebugEventCode != EXCEPTION_DEBUG_EVENT)
return -1;
if (readbuf == nullptr)
return -1;
- char *buf = (char *) &windows_process.siginfo_er;
- size_t bufsize = sizeof (windows_process.siginfo_er);
+ EXCEPTION_RECORD &er = th->last_event.u.Exception.ExceptionRecord;
+
+ char *buf = (char *) &er;
+ size_t bufsize = sizeof (er);
#ifdef __x86_64__
EXCEPTION_RECORD32 er32;
@@ -1322,17 +1326,15 @@ win32_process_target::qxfer_siginfo (const char *annex,
buf = (char *) &er32;
bufsize = sizeof (er32);
- er32.ExceptionCode = windows_process.siginfo_er.ExceptionCode;
- er32.ExceptionFlags = windows_process.siginfo_er.ExceptionFlags;
+ er32.ExceptionCode = er.ExceptionCode;
+ er32.ExceptionFlags = er.ExceptionFlags;
er32.ExceptionRecord
- = (uintptr_t) windows_process.siginfo_er.ExceptionRecord;
+ = (uintptr_t) er.ExceptionRecord;
er32.ExceptionAddress
- = (uintptr_t) windows_process.siginfo_er.ExceptionAddress;
- er32.NumberParameters = windows_process.siginfo_er.NumberParameters;
- int i;
- for (i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; i++)
- er32.ExceptionInformation[i]
- = windows_process.siginfo_er.ExceptionInformation[i];
+ = (uintptr_t) er.ExceptionAddress;
+ er32.NumberParameters = er.NumberParameters;
+ for (int i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; i++)
+ er32.ExceptionInformation[i] = er.ExceptionInformation[i];
}
#endif
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 24/34] Add backpointer from windows_thread_info to windows_process_info
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (22 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 23/34] Windows gdb+gdbserver: Make siginfo_er " Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:28 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 25/34] Windows gdb+gdbserver: Share $_siginfo reading code Pedro Alves
` (10 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
The next patch will move some duplicated code in gdb and gdbserver to
gdb/nat/windows-nat.c, where it would be convenient to get at the
Windows process info of a given Windows thread info, from within a
windows_thread_info method.
I first thought of passing down the windows_process_info pointer as
argument to the windows_thread_info method, but that looked a bit odd.
I think it looks better to just add a back pointer, so that's what
this patch does. The following patch will then add a use of it.
I suspect this will help moving more duplicated code to
gdb/nat/windows-nat.c in the future, too.
Change-Id: I47fc0d3323be5b6f6fcfe912b768051a41910666
---
gdb/nat/windows-nat.h | 10 ++++++++--
gdb/windows-nat.c | 2 +-
gdbserver/win32-low.cc | 2 +-
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 86ad8d02e3c..26b1eaea95a 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -42,13 +42,16 @@ struct pending_stop
target_waitstatus status;
};
+struct windows_process_info;
/* Thread information structure used to track extra information about
each thread. */
struct windows_thread_info
{
- windows_thread_info (DWORD tid_, HANDLE h_, CORE_ADDR tlb)
- : tid (tid_),
+ windows_thread_info (windows_process_info *proc_,
+ DWORD tid_, HANDLE h_, CORE_ADDR tlb)
+ : proc (proc_),
+ tid (tid_),
h (h_),
thread_local_base (tlb)
{
@@ -67,6 +70,9 @@ struct windows_thread_info
the next call. */
const char *thread_name ();
+ /* The process this thread belongs to. */
+ windows_process_info *proc;
+
/* The Win32 thread identifier. */
DWORD tid;
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 510820d862d..0e77614b4f4 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -612,7 +612,7 @@ windows_nat_target::add_thread (ptid_t ptid, HANDLE h, void *tlb,
if (windows_process.wow64_process)
base += 0x2000;
#endif
- th = new windows_thread_info (ptid.lwp (), h, base);
+ th = new windows_thread_info (&windows_process, ptid.lwp (), h, base);
windows_process.thread_list.emplace_back (th);
/* Add this new thread to the list of threads.
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 55600910ffe..985c6a0a0ed 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -167,7 +167,7 @@ child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
if (windows_process.wow64_process)
base += 2 * 4096; /* page size = 4096 */
#endif
- th = new windows_thread_info (tid, h, base);
+ th = new windows_thread_info (&windows_process, tid, h, base);
add_thread (ptid, th);
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 24/34] Add backpointer from windows_thread_info to windows_process_info
2024-05-07 23:42 ` [PATCH 24/34] Add backpointer from windows_thread_info to windows_process_info Pedro Alves
@ 2024-05-08 15:28 ` Tom Tromey
0 siblings, 0 replies; 64+ messages in thread
From: Tom Tromey @ 2024-05-08 15:28 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> The next patch will move some duplicated code in gdb and gdbserver to
Pedro> gdb/nat/windows-nat.c, where it would be convenient to get at the
Pedro> Windows process info of a given Windows thread info, from within a
Pedro> windows_thread_info method.
Pedro> I first thought of passing down the windows_process_info pointer as
Pedro> argument to the windows_thread_info method, but that looked a bit odd.
Pedro> I think it looks better to just add a back pointer, so that's what
Pedro> this patch does. The following patch will then add a use of it.
Pedro> I suspect this will help moving more duplicated code to
Pedro> gdb/nat/windows-nat.c in the future, too.
I generally dislike back-pointers but on the whole this seems fine to me.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 25/34] Windows gdb+gdbserver: Share $_siginfo reading code
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (23 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 24/34] Add backpointer from windows_thread_info to windows_process_info Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:29 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 26/34] Windows gdb+gdbserver: Eliminate struct pending_stop Pedro Alves
` (9 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
Both GDB and GDBserver have similar code to read the $_siginfo data.
This patch moves the bulk of it to gdb/nat/windows-nat.c so it can be
shared.
Change-Id: I47fc0d3323be5b6f6fcfe912b768051a41910666
---
gdb/nat/windows-nat.c | 49 ++++++++++++++++++++++++++++++++++++++++++
gdb/nat/windows-nat.h | 9 ++++++++
gdb/windows-nat.c | 40 +++-------------------------------
gdbserver/win32-low.cc | 42 ++++--------------------------------
4 files changed, 65 insertions(+), 75 deletions(-)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index d43d549cb0d..4fd717e6521 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -153,6 +153,55 @@ windows_thread_info::thread_name ()
return name.get ();
}
+/* Read Windows signal info. See nat/windows-nat.h. */
+
+bool
+windows_thread_info::xfer_siginfo (gdb_byte *readbuf,
+ ULONGEST offset, ULONGEST len,
+ ULONGEST *xfered_len)
+{
+ if (last_event.dwDebugEventCode != EXCEPTION_DEBUG_EVENT)
+ return false;
+
+ if (readbuf == nullptr)
+ return false;
+
+ EXCEPTION_RECORD &er = last_event.u.Exception.ExceptionRecord;
+
+ char *buf = (char *) &er;
+ size_t bufsize = sizeof (er);
+
+#ifdef __x86_64__
+ EXCEPTION_RECORD32 er32;
+ if (proc->wow64_process)
+ {
+ buf = (char *) &er32;
+ bufsize = sizeof (er32);
+
+ er32.ExceptionCode = er.ExceptionCode;
+ er32.ExceptionFlags = er.ExceptionFlags;
+ er32.ExceptionRecord
+ = (uintptr_t) er.ExceptionRecord;
+ er32.ExceptionAddress
+ = (uintptr_t) er.ExceptionAddress;
+ er32.NumberParameters = er.NumberParameters;
+ for (int i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; i++)
+ er32.ExceptionInformation[i] = er.ExceptionInformation[i];
+ }
+#endif
+
+ if (offset > bufsize)
+ return false;
+
+ if (offset + len > bufsize)
+ len = bufsize - offset;
+
+ memcpy (readbuf, buf + offset, len);
+ *xfered_len = len;
+
+ return true;
+}
+
/* Try to determine the executable filename.
EXE_NAME_RET is a pointer to a buffer whose size is EXE_NAME_MAX_LEN.
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 26b1eaea95a..af96811a2ba 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -70,6 +70,15 @@ struct windows_thread_info
the next call. */
const char *thread_name ();
+ /* Read LEN bytes of the thread's $_siginfo into READBUF, starting
+ at OFFSET. Store the number of actually-read bytes in
+ XFERED_LEN. Returns true on success, false on failure. Passing
+ READBUF as NULL indicates that the caller is trying to write to
+ $_siginfo, which is a failure case. */
+ bool xfer_siginfo (gdb_byte *readbuf,
+ ULONGEST offset, ULONGEST len,
+ ULONGEST *xfered_len);
+
/* The process this thread belongs to. */
windows_process_info *proc;
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 0e77614b4f4..cb35ff41771 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -3110,44 +3110,10 @@ windows_xfer_siginfo (gdb_byte *readbuf, ULONGEST offset, ULONGEST len,
{
windows_thread_info *th = windows_process.find_thread (inferior_ptid);
- if (th->last_event.dwDebugEventCode != EXCEPTION_DEBUG_EVENT)
- return TARGET_XFER_E_IO;
-
- EXCEPTION_RECORD &er = th->last_event.u.Exception.ExceptionRecord;
-
- char *buf = (char *) &er;
- size_t bufsize = sizeof (er);
-
-#ifdef __x86_64__
- EXCEPTION_RECORD32 er32;
- if (windows_process.wow64_process)
- {
- buf = (char *) &er32;
- bufsize = sizeof (er32);
-
- er32.ExceptionCode = er.ExceptionCode;
- er32.ExceptionFlags = er.ExceptionFlags;
- er32.ExceptionRecord = (uintptr_t) er.ExceptionRecord;
- er32.ExceptionAddress = (uintptr_t) er.ExceptionAddress;
- er32.NumberParameters = er.NumberParameters;
- for (int i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; i++)
- er32.ExceptionInformation[i] = er.ExceptionInformation[i];
- }
-#endif
-
- if (readbuf == nullptr)
- return TARGET_XFER_E_IO;
-
- if (offset > bufsize)
+ if (th->xfer_siginfo (readbuf, offset, len, xfered_len))
+ return TARGET_XFER_OK;
+ else
return TARGET_XFER_E_IO;
-
- if (offset + len > bufsize)
- len = bufsize - offset;
-
- memcpy (readbuf, buf + offset, len);
- *xfered_len = len;
-
- return TARGET_XFER_OK;
}
enum target_xfer_status
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 985c6a0a0ed..4707acc4e94 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -1308,45 +1308,11 @@ win32_process_target::qxfer_siginfo (const char *annex,
windows_thread_info *th
= windows_process.find_thread (current_thread_ptid ());
- if (th->last_event.dwDebugEventCode != EXCEPTION_DEBUG_EVENT)
- return -1;
-
- if (readbuf == nullptr)
- return -1;
-
- EXCEPTION_RECORD &er = th->last_event.u.Exception.ExceptionRecord;
-
- char *buf = (char *) &er;
- size_t bufsize = sizeof (er);
-
-#ifdef __x86_64__
- EXCEPTION_RECORD32 er32;
- if (windows_process.wow64_process)
- {
- buf = (char *) &er32;
- bufsize = sizeof (er32);
-
- er32.ExceptionCode = er.ExceptionCode;
- er32.ExceptionFlags = er.ExceptionFlags;
- er32.ExceptionRecord
- = (uintptr_t) er.ExceptionRecord;
- er32.ExceptionAddress
- = (uintptr_t) er.ExceptionAddress;
- er32.NumberParameters = er.NumberParameters;
- for (int i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; i++)
- er32.ExceptionInformation[i] = er.ExceptionInformation[i];
- }
-#endif
-
- if (offset > bufsize)
+ ULONGEST xfered_len;
+ if (th->xfer_siginfo (readbuf, offset, len, &xfered_len))
+ return xfered_len;
+ else
return -1;
-
- if (offset + len > bufsize)
- len = bufsize - offset;
-
- memcpy (readbuf, buf + offset, len);
-
- return len;
}
bool
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 26/34] Windows gdb+gdbserver: Eliminate struct pending_stop
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (24 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 25/34] Windows gdb+gdbserver: Share $_siginfo reading code Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 27/34] Windows gdb: Change serial_event management Pedro Alves
` (8 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
After the previous patches, struct pending_stop only contains one
field. So move that field into the windows_thread_info structure
directly, and eliminate struct pending_stop.
Change-Id: I7955884b3f378d8b39b908f6252d215f6568b367
---
gdb/nat/windows-nat.h | 17 +++++------------
gdb/windows-nat.c | 14 +++++++-------
gdbserver/win32-low.cc | 12 ++++++------
3 files changed, 18 insertions(+), 25 deletions(-)
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index af96811a2ba..6283ff0a4ee 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -32,16 +32,6 @@
namespace windows_nat
{
-/* Info about a potential pending stop. Each thread holds one of
- these. See "windows_thread_info::pending_stop" for more
- information. */
-struct pending_stop
-{
- /* The target waitstatus we computed. TARGET_WAITKIND_IGNORE if the
- thread does not have a pending stop. */
- target_waitstatus status;
-};
-
struct windows_process_info;
/* Thread information structure used to track extra information about
@@ -106,8 +96,11 @@ struct windows_thread_info
effect of trying to single step thread A -- leaving all other
threads suspended -- and then seeing a stop in thread B. To handle
this scenario, we queue all such "pending" stops here, and then
- process them once the step has completed. See PR gdb/22992. */
- struct pending_stop pending_stop {};
+ process them once the step has completed. See PR gdb/22992.
+
+ TARGET_WAITKIND_IGNORE if the thread does not have a pending
+ stop. */
+ target_waitstatus pending_status;
/* The last Windows event returned by WaitForDebugEvent for this
thread. */
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index cb35ff41771..7a5b57b24e1 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1363,7 +1363,7 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
{
if ((id == -1 || id == (int) th->tid)
&& !th->suspended
- && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE)
+ && th->pending_status.kind () != TARGET_WAITKIND_IGNORE)
{
DEBUG_EVENTS ("got matching pending stop event "
"for 0x%x, not resuming",
@@ -1568,18 +1568,18 @@ windows_nat_target::get_windows_debug_event
DWORD thread_id = 0;
/* If there is a relevant pending stop, report it now. See the
- comment by the definition of "windows_thread_info::pending_stop"
+ comment by the definition of "windows_thread_info::pending_status"
for details on why this is needed. */
for (auto &th : windows_process.thread_list)
{
if (!th->suspended
- && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE)
+ && th->pending_status.kind () != TARGET_WAITKIND_IGNORE)
{
DEBUG_EVENTS ("reporting pending event for 0x%x", th->tid);
thread_id = th->tid;
- *ourstatus = th->pending_stop.status;
- th->pending_stop.status.set_ignore ();
+ *ourstatus = th->pending_status;
+ th->pending_status.set_ignore ();
*current_event = th->last_event;
ptid_t ptid (windows_process.process_id, thread_id);
@@ -1804,7 +1804,7 @@ windows_nat_target::get_windows_debug_event
if (th->suspended)
{
/* Pending stop. See the comment by the definition of
- "pending_stops" for details on why this is needed. */
+ "pending_status" for details on why this is needed. */
DEBUG_EVENTS ("get_windows_debug_event - "
"unexpected stop in suspended thread 0x%x",
thread_id);
@@ -1820,7 +1820,7 @@ windows_nat_target::get_windows_debug_event
th->pc_adjusted = false;
}
- th->pending_stop.status = *ourstatus;
+ th->pending_status = *ourstatus;
ourstatus->set_ignore ();
continue_last_debug_event_main_thread
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 4707acc4e94..004bf94c83a 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -848,7 +848,7 @@ win32_process_target::resume (thread_resume *resume_info, size_t n)
}
if (!th->suspended
- && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE)
+ && th->pending_status.kind () != TARGET_WAITKIND_IGNORE)
any_pending = true;
}
@@ -992,10 +992,10 @@ get_child_debug_event (DWORD *continue_status,
auto *th = (windows_thread_info *) thread_target_data (thread);
if (!th->suspended
- && th->pending_stop.status.kind () != TARGET_WAITKIND_IGNORE)
+ && th->pending_status.kind () != TARGET_WAITKIND_IGNORE)
{
- *ourstatus = th->pending_stop.status;
- th->pending_stop.status.set_ignore ();
+ *ourstatus = th->pending_status;
+ th->pending_status.set_ignore ();
*current_event = th->last_event;
ptid = debug_event_ptid (current_event);
switch_to_thread (find_thread_ptid (ptid));
@@ -1155,13 +1155,13 @@ get_child_debug_event (DWORD *continue_status,
if (th != nullptr && th->suspended)
{
/* Pending stop. See the comment by the definition of
- "windows_thread_info::pending_stop" for details on why this
+ "windows_thread_info::pending_status" for details on why this
is needed. */
OUTMSG2 (("get_windows_debug_event - "
"unexpected stop in suspended thread 0x%x\n",
th->tid));
maybe_adjust_pc (*current_event);
- th->pending_stop.status = *ourstatus;
+ th->pending_status = *ourstatus;
ourstatus->set_spurious ();
}
else
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 27/34] Windows gdb: Change serial_event management
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (25 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 26/34] Windows gdb+gdbserver: Eliminate struct pending_stop Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 28/34] Windows gdb: cygwin_set_dr => windows_set_dr, etc Pedro Alves
` (7 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
windows_nat_target::windows_continue, when it finds a resumed thread
that has a pending event, does:
/* There's no need to really continue, because there's already
another event pending. However, we do need to inform the
event loop of this. */
serial_event_set (m_wait_event);
return TRUE;
If we have more than one pending event ready to be consumed, and,
windows_nat_target::wait returns without calling
windows_nat_target::windows_continue, which it will with the non-stop
support in the following patch, then we will miss waking up the event
loop.
This patch makes windows-nat.c manage the serial_event similarly to
how linux-nat.c does it. Clear it on entry to
windows_nat_target::wait, and set it if there may be more events to
process. With this, there's no need to set it from
windows_nat_target::wait_for_debug_event_main_thread, so the patch
also makes us not do it.
Change-Id: I44e1682721aa4866f1dbb052b3cfb4870fb13579
---
gdb/windows-nat.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 7a5b57b24e1..42d98badc4c 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -532,7 +532,6 @@ windows_nat_target::wait_for_debug_event_main_thread (DEBUG_EVENT *event)
{
*event = m_last_debug_event;
m_debug_event_pending = false;
- serial_event_clear (m_wait_event);
}
else
wait_for_debug_event (event, INFINITE);
@@ -1838,6 +1837,11 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
{
int pid = -1;
+ /* serial_event is a manual-reset event. Clear it first. We'll set
+ it again if we may need to wake up the event loop to get here
+ again. */
+ serial_event_clear (m_wait_event);
+
/* We loop when we get a non-standard exception rather than return
with a SPURIOUS because resume can try and step or modify things,
which needs a current_thread->h. But some of these exceptions mark
@@ -1886,6 +1890,10 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
thr->suspend ();
}
+ /* If something came out, assume there may be more. This is
+ needed because there may be pending events ready to
+ consume. */
+ serial_event_set (m_wait_event);
return result;
}
else
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 28/34] Windows gdb: cygwin_set_dr => windows_set_dr, etc.
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (26 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 27/34] Windows gdb: Change serial_event management Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 14:46 ` Tom Tromey
2024-05-07 23:42 ` [PATCH 29/34] windows_per_inferior::continue_one_thread, unify WoW64/non-WoW64 paths Pedro Alves
` (6 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
The Windows backend functions that manipulate the x86 debug registers
are called "cygwin_foo", which is outdated, because native MinGW gdb
also uses those functions, they are not Cygwin-specific. Rename them
to "windows_foo" to avoid confusion.
Change-Id: I46df3b44f5272adadf960da398342a3cbdb98533
---
gdb/windows-nat.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 42d98badc4c..6e72f99c3f5 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -214,11 +214,11 @@ enum
debug_prefixed_printf_cond (debug_exceptions, "windows except", fmt, \
## __VA_ARGS__)
-static void cygwin_set_dr (int i, CORE_ADDR addr);
-static void cygwin_set_dr7 (unsigned long val);
-static CORE_ADDR cygwin_get_dr (int i);
-static unsigned long cygwin_get_dr6 (void);
-static unsigned long cygwin_get_dr7 (void);
+static void windows_set_dr (int i, CORE_ADDR addr);
+static void windows_set_dr7 (unsigned long val);
+static CORE_ADDR windows_get_dr (int i);
+static unsigned long windows_get_dr6 (void);
+static unsigned long windows_get_dr7 (void);
/* User options. */
static bool new_console = false;
@@ -3194,11 +3194,11 @@ void _initialize_windows_nat ();
void
_initialize_windows_nat ()
{
- x86_dr_low.set_control = cygwin_set_dr7;
- x86_dr_low.set_addr = cygwin_set_dr;
- x86_dr_low.get_addr = cygwin_get_dr;
- x86_dr_low.get_status = cygwin_get_dr6;
- x86_dr_low.get_control = cygwin_get_dr7;
+ x86_dr_low.set_control = windows_set_dr7;
+ x86_dr_low.set_addr = windows_set_dr;
+ x86_dr_low.get_addr = windows_get_dr;
+ x86_dr_low.get_status = windows_get_dr6;
+ x86_dr_low.get_control = windows_get_dr7;
/* x86_dr_low.debug_register_length field is set by
calling x86_set_debug_register_length function
@@ -3301,10 +3301,10 @@ Use \"file\" or \"dll\" command to load executable/libraries directly."));
Here we just store the address in dr array, the registers will be
actually set up when windows_continue is called. */
static void
-cygwin_set_dr (int i, CORE_ADDR addr)
+windows_set_dr (int i, CORE_ADDR addr)
{
if (i < 0 || i > 3)
- internal_error (_("Invalid register %d in cygwin_set_dr.\n"), i);
+ internal_error (_("Invalid register %d in windows_set_dr.\n"), i);
for (auto &th : windows_process.thread_list)
th->debug_registers_changed = true;
@@ -3314,7 +3314,7 @@ cygwin_set_dr (int i, CORE_ADDR addr)
register. Here we just store the address in D_REGS, the watchpoint
will be actually set up in windows_wait. */
static void
-cygwin_set_dr7 (unsigned long val)
+windows_set_dr7 (unsigned long val)
{
for (auto &th : windows_process.thread_list)
th->debug_registers_changed = true;
@@ -3323,7 +3323,7 @@ cygwin_set_dr7 (unsigned long val)
/* Get the value of debug register I from the inferior. */
static CORE_ADDR
-cygwin_get_dr (int i)
+windows_get_dr (int i)
{
windows_thread_info *th = windows_process.find_thread (inferior_ptid);
@@ -3375,18 +3375,18 @@ cygwin_get_dr (int i)
inferior. */
static unsigned long
-cygwin_get_dr6 (void)
+windows_get_dr6 (void)
{
- return cygwin_get_dr (6);
+ return windows_get_dr (6);
}
/* Get the value of the DR7 debug status register from the
inferior. */
static unsigned long
-cygwin_get_dr7 (void)
+windows_get_dr7 (void)
{
- return cygwin_get_dr (7);
+ return windows_get_dr (7);
}
/* Determine if the thread referenced by "ptid" is alive
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 29/34] windows_per_inferior::continue_one_thread, unify WoW64/non-WoW64 paths
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (27 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 28/34] Windows gdb: cygwin_set_dr => windows_set_dr, etc Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 30/34] windows-nat.c: Avoid writing debug registers if watchpoint hit pending Pedro Alves
` (5 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
Consolidate the WoW64 & non-WoW64 paths in
windows_per_inferior::continue_one_thread to avoid code duplication.
The next patch will add more code to this function, and this
unification avoids writing that new code twice.
Change-Id: I794aadb412a3b8081212e4acf2af80d3edba7392
---
gdb/windows-nat.c | 68 ++++++++++++++++++++++-------------------------
1 file changed, 32 insertions(+), 36 deletions(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 6e72f99c3f5..5500d8b8c87 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1288,64 +1288,60 @@ windows_per_inferior::continue_one_thread (windows_thread_info *th,
struct x86_debug_reg_state *state = x86_debug_reg_state (process_id);
#ifdef __x86_64__
- if (wow64_process)
+ DWORD &context_flags_ref = (wow64_process
+ ? th->wow64_context.ContextFlags
+ : th->context.ContextFlags);
+#else
+ DWORD &context_flags_ref = th->context.ContextFlags;
+#endif
+
+ if (th->debug_registers_changed)
{
- if (th->debug_registers_changed)
+ context_flags_ref |= CONTEXT_DEBUG_REGISTERS;
+#ifdef __x86_64__
+ if (wow64_process)
{
- th->wow64_context.ContextFlags |= CONTEXT_DEBUG_REGISTERS;
th->wow64_context.Dr0 = state->dr_mirror[0];
th->wow64_context.Dr1 = state->dr_mirror[1];
th->wow64_context.Dr2 = state->dr_mirror[2];
th->wow64_context.Dr3 = state->dr_mirror[3];
th->wow64_context.Dr6 = DR6_CLEAR_VALUE;
th->wow64_context.Dr7 = state->dr_control_mirror;
- th->debug_registers_changed = false;
}
- if (th->wow64_context.ContextFlags)
- {
- DWORD ec = 0;
-
- if (GetExitCodeThread (th->h, &ec)
- && ec == STILL_ACTIVE)
- {
- BOOL status = Wow64SetThreadContext (th->h,
- &th->wow64_context);
-
- if ((cont_flags & WCONT_KILLED) == 0)
- CHECK (status);
- }
- th->wow64_context.ContextFlags = 0;
- }
- }
- else
+ else
#endif
- {
- if (th->debug_registers_changed)
{
- th->context.ContextFlags |= CONTEXT_DEBUG_REGISTERS;
th->context.Dr0 = state->dr_mirror[0];
th->context.Dr1 = state->dr_mirror[1];
th->context.Dr2 = state->dr_mirror[2];
th->context.Dr3 = state->dr_mirror[3];
th->context.Dr6 = DR6_CLEAR_VALUE;
th->context.Dr7 = state->dr_control_mirror;
- th->debug_registers_changed = false;
}
- if (th->context.ContextFlags)
- {
- DWORD ec = 0;
- if (GetExitCodeThread (th->h, &ec)
- && ec == STILL_ACTIVE)
- {
- BOOL status = SetThreadContext (th->h, &th->context);
+ th->debug_registers_changed = false;
+ }
+ if (context_flags_ref != 0)
+ {
+ DWORD ec = 0;
- if ((cont_flags & WCONT_KILLED) == 0)
- CHECK (status);
- }
- th->context.ContextFlags = 0;
+ if (GetExitCodeThread (th->h, &ec)
+ && ec == STILL_ACTIVE)
+ {
+ BOOL status;
+#ifdef __x86_64__
+ if (wow64_process)
+ status = Wow64SetThreadContext (th->h, &th->wow64_context);
+ else
+#endif
+ status = SetThreadContext (th->h, &th->context);
+
+ if ((cont_flags & WCONT_KILLED) == 0)
+ CHECK (status);
}
+ context_flags_ref = 0;
}
+
th->resume ();
th->last_sig = GDB_SIGNAL_0;
}
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 30/34] windows-nat.c: Avoid writing debug registers if watchpoint hit pending
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (28 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 29/34] windows_per_inferior::continue_one_thread, unify WoW64/non-WoW64 paths Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 31/34] Windows gdb+gdbserver: Check whether DBG_REPLY_LATER is available Pedro Alves
` (4 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
Several watchpoint-related testcases, such as
gdb.threads/watchthreads.exp for example, when tested with the backend
in non-stop mode, exposed an interesting detail of the Windows debug
API that wasn't considered before. The symptom observed is spurious
SIGTRAPs, like:
Thread 1 "watchthreads" received signal SIGTRAP, Trace/breakpoint trap.
0x00000001004010b1 in main () at .../src/gdb/testsuite/gdb.threads/watchthreads.c:48
48 args[i] = 1; usleep (1); /* Init value. */
After a good amount of staring at logs and headscratching, I realized
the problem:
#0 - It all starts in the fact that multiple threads can hit an event
at the same time. Say, a watchpoint for thread A, and a
breakpoint for thread B.
#1 - Say, WaitForDebugEvent reports the breakpoint hit for thread B
first, then GDB for some reason decides to update debug
registers, and continue. Updating debug registers means writing
the debug registers to _all_ threads, with SetThreadContext.
#2 - WaitForDebugEvent reports the watchpoint hit for thread A.
Watchpoint hits are reported as EXCEPTION_SINGLE_STEP.
#3 - windows-nat checks the Dr6 debug register to check if the step
was a watchpoint or hardware breakpoint stop, and finds that Dr6
is completely cleared. So windows-nat reports a plain SIGTRAP
(given EXCEPTION_SINGLE_STEP) to the core.
#4 - Thread A was not supposed to be stepping, so infrun reports the
SIGTRAP to the user as a random signal.
The strange part is #3 above. Why was Dr6 cleared?
Turns out what (at least in Windows 10 & 11), writing to _any_ debug
register has the side effect of clearing Dr6, even if you write the
same values the registers already had, back to the registers.
I confirmed it clearly by adding this hack to GDB:
if (th->context.ContextFlags == 0)
{
th->context.ContextFlags = CONTEXT_DEBUGGER_DR;
/* Get current values of debug registers. */
CHECK (GetThreadContext (th->h, &th->context));
DEBUG_EVENTS ("For 0x%x (once), Dr6=0x%llx", th->tid, th->context.Dr6);
/* Write debug registers back to thread, same values,
and re-read them. */
CHECK (SetThreadContext (th->h, &th->context));
CHECK (GetThreadContext (th->h, &th->context));
DEBUG_EVENTS ("For 0x%x (twice), Dr6=0x%llx", th->tid, th->context.Dr6);
}
Which showed Dr6=0 after the write + re-read:
[windows events] fill_thread_context: For 0x6a0 (once), Dr6=0xffff0ff1
[windows events] fill_thread_context: For 0x6a0 (twice), Dr6=0x0
This commit fixes the issue by detecting that a thread has a pending
watchpoint hit to report (Dr6 has interesting bits set), and if so,
avoid mofiying any debug register. Instead, let the pending
watchpoint hit be reported by WaitForDebugEvent. If infrun did want
to modify watchpoints, it will still be done when the thread is
eventually re-resumed after the pending watchpoint hit is reported.
(infrun knows how to gracefully handle the case of a watchpoint hit
for a watchpoint that has since been deleted.)
Change-Id: I21a3daa9e34eecfa054f0fea706e5ab40aabe70a
---
gdb/nat/windows-nat.h | 7 +++
gdb/windows-nat.c | 106 ++++++++++++++++++++++++++++++-----------
gdbserver/win32-low.cc | 8 ++++
gdbserver/win32-low.h | 2 +
4 files changed, 96 insertions(+), 27 deletions(-)
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 6283ff0a4ee..2efb54e1ce7 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -205,6 +205,13 @@ struct windows_process_info
virtual bool handle_access_violation (const EXCEPTION_RECORD *rec) = 0;
+ /* Fill in the thread's CONTEXT/WOW64_CONTEXT, if it wasn't filled
+ in yet.
+
+ This function must be supplied by the embedding application. */
+
+ virtual void fill_thread_context (windows_thread_info *th) = 0;
+
handle_exception_result handle_exception
(DEBUG_EVENT ¤t_event,
struct target_waitstatus *ourstatus, bool debug_exceptions);
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 5500d8b8c87..73a6237dfa7 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -114,6 +114,7 @@ struct windows_per_inferior : public windows_process_info
bool handle_access_violation (const EXCEPTION_RECORD *rec) override;
void invalidate_context (windows_thread_info *th);
+ void fill_thread_context (windows_thread_info *th) override;
void continue_one_thread (windows_thread_info *th,
windows_continue_flags cont_flags);
@@ -746,17 +747,10 @@ windows_fetch_one_register (struct regcache *regcache,
}
void
-windows_nat_target::fetch_registers (struct regcache *regcache, int r)
+windows_per_inferior::fill_thread_context (windows_thread_info *th)
{
- windows_thread_info *th = windows_process.find_thread (regcache->ptid ());
-
- /* Check if TH exists. Windows sometimes uses a non-existent
- thread id in its events. */
- if (th == NULL)
- return;
-
#ifdef __x86_64__
- if (windows_process.wow64_process)
+ if (wow64_process)
{
if (th->wow64_context.ContextFlags == 0)
{
@@ -773,6 +767,19 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r)
CHECK (GetThreadContext (th->h, &th->context));
}
}
+}
+
+void
+windows_nat_target::fetch_registers (struct regcache *regcache, int r)
+{
+ windows_thread_info *th = windows_process.find_thread (regcache->ptid ());
+
+ /* Check if TH exists. Windows sometimes uses a non-existent
+ thread id in its events. */
+ if (th == nullptr)
+ return;
+
+ windows_process.fill_thread_context (th);
if (r < 0)
for (r = 0; r < gdbarch_num_regs (regcache->arch()); r++)
@@ -1291,36 +1298,81 @@ windows_per_inferior::continue_one_thread (windows_thread_info *th,
DWORD &context_flags_ref = (wow64_process
? th->wow64_context.ContextFlags
: th->context.ContextFlags);
+ const DWORD64 dr6 = (wow64_process
+ ? th->wow64_context.Dr6
+ : th->context.Dr6);
#else
DWORD &context_flags_ref = th->context.ContextFlags;
+ const DWORD dr6 = th->context.Dr6;
#endif
if (th->debug_registers_changed)
{
- context_flags_ref |= CONTEXT_DEBUG_REGISTERS;
-#ifdef __x86_64__
- if (wow64_process)
+ windows_process.fill_thread_context (th);
+
+ gdb_assert ((context_flags_ref & CONTEXT_DEBUG_REGISTERS) != 0);
+
+ /* Check whether the thread has Dr6 set indicating a
+ watchpoint hit, and we haven't seen the watchpoint event
+ yet (reported as
+ EXCEPTION_SINGLE_STEP/STATUS_WX86_SINGLE_STEP). In that
+ case, don't change the debug registers. Changing debug
+ registers, even if to the same values, makes the kernel
+ clear Dr6. The result would be we would lose the
+ unreported watchpoint hit. */
+ if ((dr6 & ~DR6_CLEAR_VALUE) != 0)
{
- th->wow64_context.Dr0 = state->dr_mirror[0];
- th->wow64_context.Dr1 = state->dr_mirror[1];
- th->wow64_context.Dr2 = state->dr_mirror[2];
- th->wow64_context.Dr3 = state->dr_mirror[3];
- th->wow64_context.Dr6 = DR6_CLEAR_VALUE;
- th->wow64_context.Dr7 = state->dr_control_mirror;
+ if (th->last_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
+ && (th->last_event.u.Exception.ExceptionRecord.ExceptionCode
+ == EXCEPTION_SINGLE_STEP))
+ {
+ DEBUG_EVENTS ("0x%x already reported watchpoint", th->tid);
+ }
+ else
+ {
+ DEBUG_EVENTS ("0x%x last reported something else (0x%x)",
+ th->tid,
+ th->last_event.dwDebugEventCode);
+
+ /* Don't touch debug registers. Let the pending
+ watchpoint event be reported instead. We will
+ update the debug registers later when the thread
+ is re-resumed by the core after the watchpoint
+ event. */
+ context_flags_ref &= ~CONTEXT_DEBUG_REGISTERS;
+ }
}
else
-#endif
+ DEBUG_EVENTS ("0x%x has no dr6 set", th->tid);
+
+ if ((context_flags_ref & CONTEXT_DEBUG_REGISTERS) != 0)
{
- th->context.Dr0 = state->dr_mirror[0];
- th->context.Dr1 = state->dr_mirror[1];
- th->context.Dr2 = state->dr_mirror[2];
- th->context.Dr3 = state->dr_mirror[3];
- th->context.Dr6 = DR6_CLEAR_VALUE;
- th->context.Dr7 = state->dr_control_mirror;
- }
+ DEBUG_EVENTS ("0x%x changing dregs", th->tid);
+#ifdef __x86_64__
+ if (wow64_process)
+ {
+ th->wow64_context.Dr0 = state->dr_mirror[0];
+ th->wow64_context.Dr1 = state->dr_mirror[1];
+ th->wow64_context.Dr2 = state->dr_mirror[2];
+ th->wow64_context.Dr3 = state->dr_mirror[3];
+ th->wow64_context.Dr6 = DR6_CLEAR_VALUE;
+ th->wow64_context.Dr7 = state->dr_control_mirror;
+ }
+ else
+#endif
+ {
+ th->context.Dr0 = state->dr_mirror[0];
+ th->context.Dr1 = state->dr_mirror[1];
+ th->context.Dr2 = state->dr_mirror[2];
+ th->context.Dr3 = state->dr_mirror[3];
+ th->context.Dr6 = DR6_CLEAR_VALUE;
+ th->context.Dr7 = state->dr_control_mirror;
+ }
- th->debug_registers_changed = false;
+ th->debug_registers_changed = false;
+ }
}
+
if (context_flags_ref != 0)
{
DWORD ec = 0;
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 004bf94c83a..65b01dc97ac 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -141,6 +141,14 @@ win32_require_context (windows_thread_info *th)
/* See nat/windows-nat.h. */
+void
+gdbserver_windows_process::fill_thread_context (windows_thread_info *th)
+{
+ win32_require_context (th);
+}
+
+/* See nat/windows-nat.h. */
+
windows_thread_info *
gdbserver_windows_process::find_thread (ptid_t ptid)
{
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index e99e47ea829..ea2a9b4c5b6 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -181,6 +181,8 @@ struct gdbserver_windows_process : public windows_nat::windows_process_info
void handle_unload_dll (const DEBUG_EVENT ¤t_event) override;
bool handle_access_violation (const EXCEPTION_RECORD *rec) override;
+ void fill_thread_context (windows_nat::windows_thread_info *th) override;
+
int attaching = 0;
/* A status that hasn't been reported to the core yet, and so
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 31/34] Windows gdb+gdbserver: Check whether DBG_REPLY_LATER is available
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (29 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 30/34] windows-nat.c: Avoid writing debug registers if watchpoint hit pending Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 12:45 ` Eli Zaretskii
2024-05-07 23:42 ` [PATCH 32/34] Windows gdb: Add non-stop support Pedro Alves
` (3 subsequent siblings)
34 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
Per
<https://learn.microsoft.com/en-us/windows/win32/api/debugapi/nf-debugapi-continuedebugevent>,
DBG_REPLY_LATER is "Supported in Windows 10, version 1507 or above, ..."
On Linux, we check which ptrace options are supported by the running
kernel by forking gdb and then the parent gdb debugging the child gdb
with PTRACE_ME, and then trying to set the ptrace options.
Doing something like that on Windows would be more complicated,
because #1 - we can't just fork, we have to start some executable, and
the only executable we know we can start, probably, is gdb itself.
And that's a large program, so take time to be started. And then we'd
have to implement a WaitForDebugEvent loop to start up the process,
and then finally try ContinueDebugEvent(DBG_REPLY_LATER). It seems a
lot simpler to just check the Windows version. Unlike on Linux, we
don't have to worry about kernel feature backports.
Windows has a number of functions you can use to check the OS version,
like GetVersion/GetVersionEx, or the Version Helper functions like
IsWindows10OrGreater, VerifyVersionInfo, etc., however, as explained by
https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getversionexa
... and other pages, "Applications not manifested for Windows 8.1 or
Windows 10 will return the Windows 8 OS version value (6.2)."
"Manifested" here means that the application is linked with an xml
manifest as detailed at:
https://learn.microsoft.com/en-us/windows/win32/sysinfo/targeting-your-application-at-windows-8-1
I have actually tried doing that with windres, but I wasn't able to
make it work.
However, I found out that we can find the Windows major/minor/build in
the KUSER_SHARED_DATA structure, which defines the layout of a data
area that the kernel places at a pre-set address for sharing with
user-mode software:
https://www.geoffchappell.com/studies/windows/km/ntoskrnl/structs/kuser_shared_data/index.htm
The Windows major/minor/build version retrieved using that method
bypasses the manifest stuff, it actually gets you the real OS version
numbers. That is what this patch is using.
Change-Id: Ia27b981aeecaeef430ec90cebc5b3abdce00449d
---
gdb/nat/windows-nat.c | 60 +++++++++++++++++++++++++++++++++++++++++++
gdb/nat/windows-nat.h | 4 +++
2 files changed, 64 insertions(+)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 4fd717e6521..99bdadd80e6 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -939,6 +939,66 @@ disable_randomization_available ()
&& DeleteProcThreadAttributeList != nullptr);
}
+/* We can find the Windows major/minor/build in the KUSER_SHARED_DATA
+ structure, which defines the layout of a data area that the kernel
+ places at a pre-set address for sharing with user-mode software.
+
+ https://www.geoffchappell.com/studies/windows/km/ntoskrnl/structs/kuser_shared_data/index.htm
+
+ GetVersionEx or the Version Helper functions (IsWindows10OrGreater,
+ VerifyVersionInfo, etc.) don't work for Windows 10 and above,
+ because unless the binary includes a XML manifest declaring support
+ for such Windows versions, the functions always return "Windows
+ 8.2". KUSER_SHARED_DATA gives you the actual running OS info even
+ without the manifest. */
+#define KUSER_SHARED_DATA 0x7ffe0000
+
+/* The Windows major version number. */
+
+static inline ULONG
+windows_major_version ()
+{
+ return *(ULONG *) (KUSER_SHARED_DATA + 0x026c);
+}
+
+/* The Windows minor version number. */
+
+static inline ULONG
+windows_minor_version ()
+{
+ return *(ULONG *) (KUSER_SHARED_DATA + 0x0270);
+}
+
+/* The Windows build number. Windows build version info can be found
+ here:
+
+ https://www.gaijin.at/en/infos/windows-version-numbers
+ */
+
+static inline ULONG
+windows_build_number ()
+{
+ return *(ULONG *) (KUSER_SHARED_DATA + 0x0260);
+}
+
+/* See windows-nat.h. */
+
+bool
+dbg_reply_later_available ()
+{
+#if 0
+ debug_printf ("Windows Major Version: %d\n", windows_major_version ());
+ debug_printf ("Windows Minor Version: %d\n", windows_minor_version ());
+ debug_printf ("Windows Build Number: %d\n", windows_build_number ());
+#endif
+
+ /* Supported since Windows 10, Version 1507, which is reported as
+ build number 10240. */
+ return (windows_major_version () > 10
+ || (windows_major_version () == 10
+ && windows_build_number () >= 10240));
+}
+
/* See windows-nat.h. */
bool
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 2efb54e1ce7..256355f6ad0 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -415,6 +415,10 @@ extern DeleteProcThreadAttributeList_ftype *DeleteProcThreadAttributeList;
extern bool disable_randomization_available ();
+/* Return true if it's possible to use DBG_REPLY_LATER with
+ ContinueDebugEvent on this host. */
+extern bool dbg_reply_later_available ();
+
/* Load any functions which may not be available in ancient versions
of Windows. */
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 31/34] Windows gdb+gdbserver: Check whether DBG_REPLY_LATER is available
2024-05-07 23:42 ` [PATCH 31/34] Windows gdb+gdbserver: Check whether DBG_REPLY_LATER is available Pedro Alves
@ 2024-05-08 12:45 ` Eli Zaretskii
2024-05-08 21:33 ` [PATCH 31/34 v1.2] " Pedro Alves
0 siblings, 1 reply; 64+ messages in thread
From: Eli Zaretskii @ 2024-05-08 12:45 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> From: Pedro Alves <pedro@palves.net>
> Date: Wed, 8 May 2024 00:42:30 +0100
>
> Windows has a number of functions you can use to check the OS version,
> like GetVersion/GetVersionEx, or the Version Helper functions like
> IsWindows10OrGreater, VerifyVersionInfo, etc., however, as explained by
>
> https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getversionexa
>
> ... and other pages, "Applications not manifested for Windows 8.1 or
> Windows 10 will return the Windows 8 OS version value (6.2)."
>
> "Manifested" here means that the application is linked with an xml
> manifest as detailed at:
> https://learn.microsoft.com/en-us/windows/win32/sysinfo/targeting-your-application-at-windows-8-1
>
> I have actually tried doing that with windres, but I wasn't able to
> make it work.
It should be easy to do that by providing a separate manifest file as
part of the tarball, and installing it alongside gdb.exe as part of
"make install". Emacs does that, for example, and thus can make use
of the accurate Windows version where it needs that using
GetVersion/GetVersionEx. On Windows 11, for example, I get this in
Emacs:
M-: (w32-version) RET
=> (10 0 22631)
However, ...
> However, I found out that we can find the Windows major/minor/build in
> the KUSER_SHARED_DATA structure, which defines the layout of a data
> area that the kernel places at a pre-set address for sharing with
> user-mode software:
>
> https://www.geoffchappell.com/studies/windows/km/ntoskrnl/structs/kuser_shared_data/index.htm
>
> The Windows major/minor/build version retrieved using that method
> bypasses the manifest stuff, it actually gets you the real OS version
> numbers. That is what this patch is using.
...do we really need to do this via a version-check? Can't we instead
just call ContinueDebugEvent and if it fails, consider DBG_REPLY_LATER
unsupported? (If calling ContinueDebugEvent with that flag on older
versions of Windows causes an exception, we could use try/catch.) If
this works, it is a more reliable way to test, IMO and IME. I think
we should prefer that to poking kernel data structures.
WDYT?
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 31/34 v1.2] Windows gdb+gdbserver: Check whether DBG_REPLY_LATER is available
2024-05-08 12:45 ` Eli Zaretskii
@ 2024-05-08 21:33 ` Pedro Alves
2024-05-09 10:07 ` Hannes Domani
0 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-08 21:33 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 2024-05-08 13:45, Eli Zaretskii wrote:
>> However, I found out that we can find the Windows major/minor/build in
>> the KUSER_SHARED_DATA structure, which defines the layout of a data
>> area that the kernel places at a pre-set address for sharing with
>> user-mode software:
>>
>> https://www.geoffchappell.com/studies/windows/km/ntoskrnl/structs/kuser_shared_data/index.htm
>>
>> The Windows major/minor/build version retrieved using that method
>> bypasses the manifest stuff, it actually gets you the real OS version
>> numbers. That is what this patch is using.
>
> ...do we really need to do this via a version-check? Can't we instead
> just call ContinueDebugEvent and if it fails, consider DBG_REPLY_LATER
> unsupported? (If calling ContinueDebugEvent with that flag on older
> versions of Windows causes an exception, we could use try/catch.) If
> this works, it is a more reliable way to test, IMO and IME. I think
> we should prefer that to poking kernel data structures.
We need to know whether DBG_REPLY_LATER will work before starting the inferior.
And we can only call ContinueDebugEvent after starting some inferior, and
after the kernel returns an event for it via WaitForDebugEvent.
Hannes on IRC suggested using RtlGetVersion, which is exported by ntdll.dll.
I tried it, and it works nicely. Looking around the web, I see suggestions to
use that on stockoverflow, blogs, etc. I don't know why I didn't run into
that one before. Maybe because the MSFT page describing it says it is
"kernel-mode":
https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-rtlgetversion
In truth, you can call it in user mode just fine.
Here is a version of the patch using that function. Much simpler!
From 4331947c4728071b2d53011bf7d70bd0ab7c4e93 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 8 May 2024 21:14:26 +0100
Subject: [PATCH] Windows gdb+gdbserver: Check whether DBG_REPLY_LATER is
available
Per
<https://learn.microsoft.com/en-us/windows/win32/api/debugapi/nf-debugapi-continuedebugevent>,
DBG_REPLY_LATER is "Supported in Windows 10, version 1507 or above, ..."
We need to know whether DBG_REPLY_LATER is available before starting
any inferior.
On Linux, we check which ptrace options are supported by the running
kernel by forking gdb and then the parent gdb debugging the child gdb
with PTRACE_ME, and then trying to set the ptrace options.
Doing something like that on Windows would be more complicated,
because we can't just fork, we have to start some executable, and the
only executable we know we can start, probably, is gdb itself. And
that's a large program, so takes time to be started. And then we'd
have to implement a WaitForDebugEvent loop to start up the process,
and then finally try ContinueDebugEvent(DBG_REPLY_LATER).
It's a lot simpler to just check the Windows version. Unlike on
Linux, we don't have to worry about kernel feature backports. This
patch does that.
Change-Id: Ia27b981aeecaeef430ec90cebc5b3abdce00449d
---
gdb/nat/windows-nat.c | 56 +++++++++++++++++++++++++++++++++++++++++++
gdb/nat/windows-nat.h | 8 +++++++
2 files changed, 64 insertions(+)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 4fd717e6521..a7383276ec2 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -40,6 +40,7 @@ namespace windows_nat
ContinueDebugEvent. */
static DEBUG_EVENT last_wait_event;
+RtlGetVersion_ftype *RtlGetVersion;
AdjustTokenPrivileges_ftype *AdjustTokenPrivileges;
DebugActiveProcessStop_ftype *DebugActiveProcessStop;
DebugBreakProcess_ftype *DebugBreakProcess;
@@ -939,6 +940,57 @@ disable_randomization_available ()
&& DeleteProcThreadAttributeList != nullptr);
}
+/* Helper for dbg_reply_later_available. Does the actual work, while
+ dbg_reply_later_available handles caching. */
+
+static bool
+dbg_reply_later_available_1 ()
+{
+ /* Windows has a number of functions you can use to check the OS
+ version, like GetVersion/GetVersionEx, or the Version Helper
+ functions like IsWindows10OrGreater, VerifyVersionInfo, etc.
+ However, as explained by:
+
+ https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getversionexa
+
+ ... and other pages, "Applications not manifested for Windows 8.1
+ or Windows 10 will return the Windows 8 OS version value (6.2)."
+
+ RtlGetVersion is simpler because it bypasses the manifest
+ machinery. */
+
+ /* We require Windows XP, and RtlGetVersion exists since Windows
+ 2000. */
+ gdb_assert (RtlGetVersion != nullptr);
+
+ OSVERSIONINFOW version_info;
+ version_info.dwOSVersionInfoSize = sizeof (OSVERSIONINFOW);
+ /* This is documented to always succeed. */
+ gdb_assert (RtlGetVersion (&version_info) == 0);
+
+ debug_printf ("gdb: Windows version: major=%d, minor=%d, build=%d\n",
+ version_info.dwMajorVersion,
+ version_info.dwMinorVersion,
+ version_info.dwBuildNumber);
+
+ /* DBG_REPLY_LATER is supported since Windows 10, Version 1507,
+ which is reported as build number 10240. */
+ return (version_info.dwMajorVersion > 10
+ || (version_info.dwMajorVersion == 10
+ && version_info.dwBuildNumber >= 10240));
+}
+
+/* See windows-nat.h. */
+
+bool
+dbg_reply_later_available ()
+{
+ static int available = -1;
+ if (available == -1)
+ available = dbg_reply_later_available_1 ();
+ return available;
+}
+
/* See windows-nat.h. */
bool
@@ -950,6 +1002,10 @@ initialize_loadable ()
#define GPA(m, func) \
func = (func ## _ftype *) GetProcAddress (m, #func)
+ hm = LoadLibrary (TEXT ("ntdll.dll"));
+ if (hm)
+ GPA (hm, RtlGetVersion);
+
hm = LoadLibrary (TEXT ("kernel32.dll"));
if (hm)
{
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 2efb54e1ce7..cee710e07d4 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -295,6 +295,7 @@ extern BOOL create_process (const wchar_t *image, wchar_t *command_line,
PROCESS_INFORMATION *process_info);
#endif /* __CYGWIN__ */
+#define RtlGetVersion dyn_RtlGetVersion
#define AdjustTokenPrivileges dyn_AdjustTokenPrivileges
#define DebugActiveProcessStop dyn_DebugActiveProcessStop
#define DebugBreakProcess dyn_DebugBreakProcess
@@ -322,6 +323,9 @@ extern BOOL create_process (const wchar_t *image, wchar_t *command_line,
#define UpdateProcThreadAttribute dyn_UpdateProcThreadAttribute
#define DeleteProcThreadAttributeList dyn_DeleteProcThreadAttributeList
+typedef NTSTATUS NTAPI (RtlGetVersion_ftype) (PRTL_OSVERSIONINFOW);
+extern RtlGetVersion_ftype *RtlGetVersion;
+
typedef BOOL WINAPI (AdjustTokenPrivileges_ftype) (HANDLE, BOOL,
PTOKEN_PRIVILEGES,
DWORD, PTOKEN_PRIVILEGES,
@@ -415,6 +419,10 @@ extern DeleteProcThreadAttributeList_ftype *DeleteProcThreadAttributeList;
extern bool disable_randomization_available ();
+/* Return true if it's possible to use DBG_REPLY_LATER with
+ ContinueDebugEvent on this host. */
+extern bool dbg_reply_later_available ();
+
/* Load any functions which may not be available in ancient versions
of Windows. */
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 31/34 v1.2] Windows gdb+gdbserver: Check whether DBG_REPLY_LATER is available
2024-05-08 21:33 ` [PATCH 31/34 v1.2] " Pedro Alves
@ 2024-05-09 10:07 ` Hannes Domani
[not found] ` <86zfsz5kly.fsf@gnu.org>
0 siblings, 1 reply; 64+ messages in thread
From: Hannes Domani @ 2024-05-09 10:07 UTC (permalink / raw)
To: Eli Zaretskii, Pedro Alves; +Cc: gdb-patches
Am Mittwoch, 8. Mai 2024 um 23:34:25 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:
> On 2024-05-08 13:45, Eli Zaretskii wrote:
>
> >> However, I found out that we can find the Windows major/minor/build in
> >> the KUSER_SHARED_DATA structure, which defines the layout of a data
> >> area that the kernel places at a pre-set address for sharing with
> >> user-mode software:
> >>
> >> https://www.geoffchappell.com/studies/windows/km/ntoskrnl/structs/kuser_shared_data/index.htm
> >>
> >> The Windows major/minor/build version retrieved using that method
> >> bypasses the manifest stuff, it actually gets you the real OS version
> >> numbers. That is what this patch is using.
> >
> > ...do we really need to do this via a version-check? Can't we instead
> > just call ContinueDebugEvent and if it fails, consider DBG_REPLY_LATER
> > unsupported? (If calling ContinueDebugEvent with that flag on older
> > versions of Windows causes an exception, we could use try/catch.) If
> > this works, it is a more reliable way to test, IMO and IME. I think
> > we should prefer that to poking kernel data structures.
>
> We need to know whether DBG_REPLY_LATER will work before starting the inferior.
> And we can only call ContinueDebugEvent after starting some inferior, and
> after the kernel returns an event for it via WaitForDebugEvent.
Looks like just trying to call ContinueDebugEvent is possible after all.
#include <windows.h>
#include <stdio.h>
#ifndef DBG_REPLY_LATER
#define DBG_REPLY_LATER 0x40010001
#endif
int main()
{
if (!ContinueDebugEvent(0, 0, DBG_REPLY_LATER))
printf("error: 0x%lx\n", GetLastError());
return 0;
}
On Win10 this gives me error 0x6 (ERROR_INVALID_HANDLE), and on Win7 it
gives me error 0x57 (ERROR_INVALID_PARAMETER).
Hannes
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 32/34] Windows gdb: Add non-stop support
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (30 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 31/34] Windows gdb+gdbserver: Check whether DBG_REPLY_LATER is available Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 33/34] Windows gdb: Watchpoints while running (internal vs external stops) Pedro Alves
` (2 subsequent siblings)
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
This patch adds non-stop support to the native Windows target.
This is made possible by the ContinueDebugEvent DBG_REPLY_LATER flag:
https://learn.microsoft.com/en-us/windows/win32/api/debugapi/nf-debugapi-continuedebugevent
Supported in Windows 10, version 1507 or above, this flag causes
dwThreadId to replay the existing breaking event after the target
continues. By calling the SuspendThread API against dwThreadId, a
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
debugger can resume other threads in the process and later return to
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
the breaking.
^^^^^^^^^^^^
The patch adds a new comment section in gdb/windows-nat.c providing an
overall picture of how all-stop / non-stop work.
Without DBG_REPLY_LATER, if we SuspendThread the thread, and then
immediately ContinueDebugThread(DBG_CONTINUE) before getting back to
the prompt, we could still have non-stop mode working, however, then
users wouldn't have a chance to decide whether to pass the signal to
the inferior the next time they resume the program, as that is done by
passing DBG_EXCEPTION_NOT_HANDLED to ContinueDebugEvent, and that has
already been called.
The patch adds that DBG_REPLY_LATER handling, and also adds support
for target_stop, so the core can pause threads at its discretion.
This pausing does not use the same mechanisms used in
windows_nat_target::interrupt, as those inject a new thread in the
inferior. Instead, for each thread the core wants paused, it uses
SuspendThread, and enqueues a pending GDB_SIGNAL_0 stop on the thread.
Since DBG_REPLY_LATER only exists on Windows 10 and later, we only
enable non-stop mode on Windows 10 and later.
Since having the target backend work in non-stop mode adds features
compared to old all-stop mode (signal/exception passing/suppression is
truly per-thread), this patch also switches the backend to do
all-stop-on-top-of-non-stop, by having
windows_nat_target::always_non_stop_p return true if non-stop mode is
possible. To be clear, this just changes how the backend works in
coordination with infrun. The user-visible mode default mode is still
all-stop. The difference is that infrun is responsible for stopping
all threads when needed, instead of the backend (actually the kernel)
always doing that before reporting an event to infrun.
There is no displaced stepping support, but that's "just" a missed
optimization to be done later.
Cygwin signals handling was a major headache, but I managed to get it
working. See the "Cygwin signals" description section I added at the
top of windows-nat.c.
Change-Id: Id71aef461c43c244120635b5bedc638fe77c31fb
---
gdb/nat/windows-nat.c | 17 +-
gdb/nat/windows-nat.h | 56 ++-
gdb/windows-nat.c | 753 +++++++++++++++++++++++++++++++++++------
gdbserver/win32-low.cc | 10 +-
gdbserver/win32-low.h | 4 +-
5 files changed, 725 insertions(+), 115 deletions(-)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 99bdadd80e6..11251be07be 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -727,17 +727,20 @@ get_last_debug_event_ptid ()
/* See nat/windows-nat.h. */
BOOL
-continue_last_debug_event (DWORD continue_status, bool debug_events)
+continue_last_debug_event (DWORD cont_status, bool debug_events)
{
- DEBUG_EVENTS ("ContinueDebugEvent (cpid=%d, ctid=0x%x, %s)",
- (unsigned) last_wait_event.dwProcessId,
- (unsigned) last_wait_event.dwThreadId,
- continue_status == DBG_CONTINUE ?
- "DBG_CONTINUE" : "DBG_EXCEPTION_NOT_HANDLED");
+ DEBUG_EVENTS
+ ("ContinueDebugEvent (cpid=%d, ctid=0x%x, %s)",
+ (unsigned) last_wait_event.dwProcessId,
+ (unsigned) last_wait_event.dwThreadId,
+ cont_status == DBG_CONTINUE ? "DBG_CONTINUE" :
+ cont_status == DBG_EXCEPTION_NOT_HANDLED ? "DBG_EXCEPTION_NOT_HANDLED" :
+ cont_status == DBG_REPLY_LATER ? "DBG_REPLY_LATER" :
+ "DBG_???");
return ContinueDebugEvent (last_wait_event.dwProcessId,
last_wait_event.dwThreadId,
- continue_status);
+ cont_status);
}
/* See nat/windows-nat.h. */
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 256355f6ad0..c6d0c4e98dd 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -81,12 +81,55 @@ struct windows_thread_info
/* Thread Information Block address. */
CORE_ADDR thread_local_base;
+#ifdef __CYGWIN__
+ /* These two fields are used to handle Cygwin signals. When a
+ thread is signaled, the "sig" thread inside the Cygwin runtime
+ reports the fact to us via a special OutputDebugString message.
+ In order to make stepping into a signal handler work, we can only
+ resume the "sig" thread when we also resume the target signaled
+ thread. When we intercept a Cygwin signal, we set up a cross
+ link between the two threads using the two fields below, so we
+ can always identify one from the other. See the "Cygwin signals"
+ description in gdb/windows-nat.c for more. */
+
+ /* If this thread received a signal, then 'cygwin_sig_thread' points
+ to the "sig" thread within the Cygwin runtime. */
+ windows_thread_info *cygwin_sig_thread = nullptr;
+
+ /* If this thread is the Cygwin runtime's "sig" thread, then
+ 'signaled_thread' points at the thread that received a
+ signal. */
+ windows_thread_info *signaled_thread = nullptr;
+#endif
+
+ /* If the thread had its event postponed with DBG_REPLY_LATER, when
+ we later ResumeThread this thread, WaitForDebugEvent will
+ re-report the postponed event. This field holds the continue
+ status value to be automatically passed to ContinueDebugEvent
+ when we encounter this re-reported event. 0 if the thread has
+ not had its event postponed with DBG_REPLY_LATER. */
+ DWORD auto_cont = 0;
+
/* This keeps track of whether SuspendThread was called on this
thread. -1 means there was a failure or that the thread was
explicitly not suspended, 1 means it was called, and 0 means it
was not. */
int suspended = 0;
+ /* This flag indicates whether we are explicitly stopping this
+ thread in response to a target_stop request. This allows
+ distinguishing between threads that are explicitly stopped by the
+ debugger and threads that are stopped due to other reasons.
+
+ Typically, when we want to stop a thread, we suspend it, enqueue
+ a pending GDB_SIGNAL_0 stop status on the thread, and then set
+ this flag to true. However, if the thread has had its event
+ previously postponed with DBG_REPLY_LATER, it means that it
+ already has an event to report. In such case, we simply set the
+ 'stopping' flag without suspending the thread or enqueueing a
+ pending stop. See stop_one_thread. */
+ bool stopping = false;
+
/* Info about a potential pending stop.
Sometimes, Windows will report a stop on a thread that has been
@@ -167,15 +210,15 @@ struct windows_process_info
virtual windows_thread_info *find_thread (ptid_t ptid) = 0;
/* Handle OUTPUT_DEBUG_STRING_EVENT from child process. Updates
- OURSTATUS and returns the thread id if this represents a thread
- change (this is specific to Cygwin), otherwise 0.
+ OURSTATUS and returns true if this represents a Cygwin signal,
+ otherwise false.
Cygwin prepends its messages with a "cygwin:". Interpret this as
a Cygwin signal. Otherwise just print the string as a warning.
This function must be supplied by the embedding application. */
- virtual DWORD handle_output_debug_string (const DEBUG_EVENT ¤t_event,
- struct target_waitstatus *ourstatus) = 0;
+ virtual bool handle_output_debug_string (const DEBUG_EVENT ¤t_event,
+ struct target_waitstatus *ourstatus) = 0;
/* Handle a DLL load event.
@@ -415,6 +458,11 @@ extern DeleteProcThreadAttributeList_ftype *DeleteProcThreadAttributeList;
extern bool disable_randomization_available ();
+/* This is available starting with Windows 10. */
+#ifndef DBG_REPLY_LATER
+# define DBG_REPLY_LATER 0x40010001L
+#endif
+
/* Return true if it's possible to use DBG_REPLY_LATER with
ContinueDebugEvent on this host. */
extern bool dbg_reply_later_available ();
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 73a6237dfa7..a33ef868566 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -74,6 +74,162 @@
#include "ser-event.h"
#include "inf-loop.h"
+/* This comment documents high-level logic of this file.
+
+all-stop
+========
+
+In all-stop mode, there is only ever one Windows debug event in
+flight. When we receive an event from WaitForDebugEvent, the kernel
+has already implicitly suspended all the threads of the process. We
+report the breaking event to the core. When the core decides to
+resume the inferior, it calls windows_nat_target:resume, which
+triggers a ContinueDebugEvent call. This call makes all unsuspended
+threads schedulable again, and we go back to waiting for the next
+event in WaitForDebugEvent.
+
+non-stop
+========
+
+For non-stop mode, we utilize the DBG_REPLY_LATER flag in the
+ContinueDebugEvent function. According to Microsoft:
+
+ "This flag causes dwThreadId to replay the existing breaking event
+ after the target continues. By calling the SuspendThread API against
+ dwThreadId, a debugger can resume other threads in the process and
+ later return to the breaking."
+
+To enable non-stop mode, windows_nat_target::wait suspends the thread,
+calls 'ContinueForDebugEvent(..., DBG_REPLY_LATER)', and sets the
+process_thread thread to wait for the next event using
+WaitForDebugEvent, all before returning the original breaking event to
+the core.
+
+When the user/core finally decides to resume the inferior thread that
+reported the event, we unsuspend it using ResumeThread. Unlike in
+all-stop mode, we don't call ContinueDebugEvent then, as it has
+already been called when the event was first encountered. By making
+the inferior thread schedulable again, WaitForDebugEvent re-reports
+the same event (due to the earlier DBG_REPLY_LATER). In
+windows_nat_target::wait, we detect this delayed re-report and call
+ContinueDebugEvent on the thread, instructing the process_thread
+thread to continue waiting for the next event.
+
+During the initial thread resumption in windows_nat_target::resume, we
+recorded the dwContinueStatus argument to be passed to the last
+ContinueDebugEvent. See windows_thread_info::auto_cont for details.
+
+Note that with this setup, in non-stop mode, every stopped thread has
+its own independent last-reported Windows debug event. Therefore, we
+can decide on a per-thread basis whether to pass the thread's
+exception (DBG_EXCEPTION_NOT_HANDLED / DBG_CONTINUE) to the inferior.
+This per-thread decision is not possible in all-stop mode, where we
+only call ContinueDebugEvent for the thread that last reported a stop,
+at windows_nat_target::resume time.
+
+Cygwin signals
+==============
+
+The Cygwin runtime always spawns a "sig" thread, which is responsible
+for receiving signal delivery requests, and hijacking the signaled
+thread's execution to make it run the signal handler. This is all
+explained here:
+
+ https://sourceware.org/cgit/newlib-cygwin/tree/winsup/cygwin/DevDocs/how-signals-work.txt
+
+There's a custom debug api protocol between GDB and Cygwin to be able
+to intercept Cygwin signals before they're seen by the signaled
+thread, just like the debugger intercepts signals with ptrace on
+Linux. This Cygwin debugger protocol isn't well documented, though.
+Here's what happens: when the special "sig" thread in the Cygwin
+runtime is about to deliver a signal to the target thread, it calls
+OutputDebugString with a special message:
+
+ https://sourceware.org/cgit/newlib-cygwin/tree/winsup/cygwin/exceptions.cc?id=4becae7bd833e183c789821a477f25898ed0db1f#n1866
+
+OutputDebugString is a function that is part of the Windows debug API.
+It generates an OUTPUT_DEBUG_STRING_EVENT event out of
+WaitForDebugEvent in the debugger, which freezes the inferior, like
+any other event.
+
+GDB recognizes the special Cygwin signal marker string, and is able to
+report the intercepted Cygwin signal to the user.
+
+With the windows-nat backend in all-stop mode, if the user decides to
+single-step the signaled thread, GDB will set the trace flag in the
+signaled thread to force it to single-step, and then re-resume the
+program with ContinueDebugEvent. This resumes both the signaled
+thread, and the special "sig" thread. The special "sig" thread
+decides to make the signaled thread run the signal handler, so it
+suspends it with SuspendThread, does a read-modify-write operation
+with GetThreadContext/SetThreadContext, and then re-resumes it with
+ResumeThread. This is all done here:
+
+ https://sourceware.org/cgit/newlib-cygwin/tree/winsup/cygwin/exceptions.cc?id=4becae7bd833e183c789821a477f25898ed0db1f#n1011
+
+That resulting register context will still have its trace flag set, so
+the signaled thread ends up single-stepping the signal handler and
+reporting the trace stop to GDB, which reports the stop where the
+thread is now stopped, inside the signal handler.
+
+That is the intended behavior; stepping into a signal handler is a
+feature that works on other ports as well, including x86 GNU/Linux,
+for example. This is exercised by the gdb.base/sigstep.exp testcase.
+
+Now, making that work with the backend in non-stop mode (the default
+on Windows 10 and above) is tricker. In that case, when GDB sees the
+magic OUTPUT_DEBUG_STRING_EVENT event mentioned above, reported for
+the "sig" thread, GDB reports the signal stop for the target signaled
+thread to the user (leaving that thread stopped), but, unlike with an
+all-stop backend, in non-stop, only the evented/signaled thread should
+be stopped, so the backend would normally want to re-resume the Cygwin
+runtime's "sig" thread after handling the OUTPUT_DEBUG_STRING_EVENT
+event, like it does with any other event out of WaitForDebugEvent that
+is not reported to the core. If it did that (resume the "sig" thread)
+however, at that point, the signaled thread would be stopped,
+suspended with SuspendThread by GDB (while the user is inspecting it),
+but, unlike in all-stop, the "sig" thread would be set running free.
+The "sig" thread would reach the code that wants to redirect the
+signaled thread's execution to the signal handler (by hacking the
+registers context, as described above), but unlike in the all-stop
+case, the "sig" thread would notice that the signaled thread is
+suspended, and so would decide to defer the signal handler until a
+later time. It's the same code as described above for the all-stop
+case, except it would take the "then" branch:
+
+ https://sourceware.org/cgit/newlib-cygwin/tree/winsup/cygwin/exceptions.cc?id=4becae7bd833e183c789821a477f25898ed0db1f#n1019
+
+ // Just set pending if thread is already suspended
+ if (res)
+ {
+ tls->unlock ();
+ ResumeThread (hth);
+ goto out;
+ }
+
+The result would be that when the GDB user later finally decides to
+step the signaled thread, the signaled thread would just single step
+the mainline code, instead of stepping into the signal handler.
+
+To avoid this difference of behavior in non-stop mode compared to
+all-stop mode, we use a trick -- whenever we see that magic
+OUTPUT_DEBUG_STRING_EVENT event reported for the "sig" thread, we
+report a stop for the target signaled thread, _and_ leave the "sig"
+thread suspended as well, for as long as the target signaled thread is
+suspended. I.e., we don't let the "sig" thread run before the user
+decides what to do with the signaled thread's signal. Only when the
+user re-resumes the signaled thread, will we resume the "sig" thread
+as well. The trick is that all this is done here in the Windows
+backend, while providing the illusion to the core of GDB (and the
+user) that the "sig" thread is "running", for as long as the core
+wants the "sig" thread to be running.
+
+This isn't ideal, since this means that with user-visible non-stop,
+the inferior will only be able to process and report one signal at a
+time (as the "sig" thread is responsible for that), but that seems
+like an acceptible compromise, better than not being able to have the
+target work in non-stop by default on Cygwin. */
+
using namespace windows_nat;
/* Maintain a linked list of "so" information. */
@@ -100,6 +256,11 @@ enum windows_continue_flag
call to continue the inferior -- we are either mourning it or
detaching. */
WCONT_LAST_CALL = 2,
+
+ /* By default, windows_continue only calls ContinueDebugEvent in
+ all-stop mode. This flag indicates that windows_continue
+ should call ContinueDebugEvent even in non-stop mode. */
+ WCONT_CONTINUE_DEBUG_EVENT = 4,
};
DEF_ENUM_FLAGS_TYPE (windows_continue_flag, windows_continue_flags);
@@ -107,8 +268,8 @@ DEF_ENUM_FLAGS_TYPE (windows_continue_flag, windows_continue_flags);
struct windows_per_inferior : public windows_process_info
{
windows_thread_info *find_thread (ptid_t ptid) override;
- DWORD handle_output_debug_string (const DEBUG_EVENT ¤t_event,
- struct target_waitstatus *ourstatus) override;
+ bool handle_output_debug_string (const DEBUG_EVENT ¤t_event,
+ struct target_waitstatus *ourstatus) override;
void handle_load_dll (const char *dll_name, LPVOID base) override;
void handle_unload_dll (const DEBUG_EVENT ¤t_event) override;
bool handle_access_violation (const EXCEPTION_RECORD *rec) override;
@@ -269,7 +430,11 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
void attach (const char *, int) override;
bool attach_no_wait () override
- { return true; }
+ {
+ /* In non-stop, after attach, we leave all threads running, like
+ other targets. */
+ return !target_is_non_stop_p ();
+ }
void detach (inferior *, int) override;
@@ -312,8 +477,11 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
std::string pid_to_str (ptid_t) override;
void interrupt () override;
+ void stop (ptid_t) override;
void pass_ctrlc () override;
+ void thread_events (int enable) override;
+
const char *pid_to_exec_file (int pid) override;
ptid_t get_ada_task_ptid (long lwp, ULONGEST thread) override;
@@ -343,6 +511,9 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
return m_is_async;
}
+ bool supports_non_stop () override;
+ bool always_non_stop_p () override;
+
void async (bool enable) override;
int async_wait_fd () override
@@ -357,6 +528,8 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
void delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p);
DWORD fake_create_process (const DEBUG_EVENT ¤t_event);
+ void stop_one_thread (windows_thread_info *th);
+
BOOL windows_continue (DWORD continue_status, int id,
windows_continue_flags cont_flags = 0);
@@ -420,6 +593,9 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
already returned an event, and we need to ContinueDebugEvent
again to restart the inferior. */
bool m_continued = false;
+
+ /* Whether target_thread_events is in effect. */
+ int m_report_thread_events = 0;
};
static void
@@ -629,6 +805,13 @@ windows_nat_target::add_thread (ptid_t ptid, HANDLE h, void *tlb,
registers. */
th->debug_registers_changed = true;
+ /* Even if we're stopping the thread for some reason internal to
+ this module, from the perspective of infrun and the
+ user/frontend, this new thread is running until it next reports a
+ stop. */
+ set_running (this, ptid, true);
+ set_executing (this, ptid, true);
+
return th;
}
@@ -1048,12 +1231,17 @@ signal_event_command (const char *args, int from_tty)
/* See nat/windows-nat.h. */
-DWORD
+bool
windows_per_inferior::handle_output_debug_string
(const DEBUG_EVENT ¤t_event,
struct target_waitstatus *ourstatus)
{
- DWORD thread_id = 0;
+ windows_thread_info *event_thr
+ = windows_process.find_thread (ptid_t (current_event.dwProcessId,
+ current_event.dwThreadId));
+ if (event_thr->auto_cont)
+ internal_error ("OutputDebugString thread 0x%x has auto-cont set",
+ event_thr->tid);
gdb::unique_xmalloc_ptr<char> s
= (target_read_string
@@ -1090,15 +1278,37 @@ windows_per_inferior::handle_output_debug_string
int sig = strtol (s.get () + sizeof (_CYGWIN_SIGNAL_STRING) - 1, &p, 0);
gdb_signal gotasig = gdb_signal_from_host (sig);
LPCVOID x = 0;
+ DWORD thread_id = 0;
- if (gotasig)
+ if (gotasig != GDB_SIGNAL_0)
{
- ourstatus->set_stopped (gotasig);
thread_id = strtoul (p, &p, 0);
- if (thread_id == 0)
- thread_id = current_event.dwThreadId;
- else
- x = (LPCVOID) (uintptr_t) strtoull (p, NULL, 0);
+ if (thread_id != 0)
+ {
+ x = (LPCVOID) (uintptr_t) strtoull (p, NULL, 0);
+
+ ptid_t ptid (current_event.dwProcessId, thread_id, 0);
+ windows_thread_info *th = find_thread (ptid);
+
+ /* Suspend the signaled thread, and leave the signal as
+ a pending event. It will be picked up by
+ windows_nat_target::wait. */
+ th->suspend ();
+ th->stopping = true;
+ th->last_event = {};
+ th->pending_status.set_stopped (gotasig);
+
+ /* Link the "sig" thread and the signaled threads, so we
+ can keep the "sig" thread suspended until we resume
+ the signaled thread. See "Cygwin signals" at the
+ top. */
+ event_thr->signaled_thread = th;
+ th->cygwin_sig_thread = event_thr;
+
+ /* Leave the "sig" thread suspended. */
+ event_thr->suspend ();
+ return true;
+ }
}
DEBUG_EVENTS ("gdb: cygwin signal %d, thread 0x%x, CONTEXT @ %p",
@@ -1106,7 +1316,7 @@ windows_per_inferior::handle_output_debug_string
}
#endif
- return thread_id;
+ return false;
}
static int
@@ -1395,6 +1605,7 @@ windows_per_inferior::continue_one_thread (windows_thread_info *th,
}
th->resume ();
+ th->stopping = false;
th->last_sig = GDB_SIGNAL_0;
}
@@ -1406,31 +1617,77 @@ BOOL
windows_nat_target::windows_continue (DWORD continue_status, int id,
windows_continue_flags cont_flags)
{
- for (auto &th : windows_process.thread_list)
- {
- if ((id == -1 || id == (int) th->tid)
- && !th->suspended
- && th->pending_status.kind () != TARGET_WAITKIND_IGNORE)
- {
- DEBUG_EVENTS ("got matching pending stop event "
- "for 0x%x, not resuming",
- th->tid);
-
- /* There's no need to really continue, because there's already
- another event pending. However, we do need to inform the
- event loop of this. */
- serial_event_set (m_wait_event);
- return TRUE;
- }
- }
+ if ((cont_flags & (WCONT_LAST_CALL | WCONT_KILLED)) == 0)
+ for (auto &th : windows_process.thread_list)
+ {
+ if ((id == -1 || id == (int) th->tid)
+ && th->pending_status.kind () != TARGET_WAITKIND_IGNORE)
+ {
+ DEBUG_EVENTS ("got matching pending stop event "
+ "for 0x%x, not resuming",
+ th->tid);
+
+ /* There's no need to really continue, because there's already
+ another event pending. However, we do need to inform the
+ event loop of this. */
+ serial_event_set (m_wait_event);
+ return TRUE;
+ }
+ }
+ /* Resume any suspended thread whose ID matches "ID". Skip the
+ Cygwin "sig" thread in the main iteration, though. That one is
+ only resumed when the target signaled thread is resumed. See
+ "Cygwin signals" in the intro section. */
for (auto &th : windows_process.thread_list)
- if (id == -1 || id == (int) th->tid)
- windows_process.continue_one_thread (th.get (), cont_flags);
+ if (th->suspended
+#ifdef __CYGWIN__
+ && th->signaled_thread == nullptr
+#endif
+ && (id == -1 || id == (int) th->tid))
+ {
+ windows_process.continue_one_thread (th.get (), cont_flags);
- continue_last_debug_event_main_thread
- (_("Failed to resume program execution"), continue_status,
- cont_flags & WCONT_LAST_CALL);
+#ifdef __CYGWIN__
+ /* See if we're resuming a thread that caught a Cygwin signal.
+ If so, also resume the Cygwin runtime's "sig" thread. */
+ if (th->cygwin_sig_thread != nullptr)
+ {
+ DEBUG_EVENTS ("\"sig\" thread %d (0x%x) blocked by "
+ "just-resumed thread %d (0x%x)",
+ th->cygwin_sig_thread->tid,
+ th->cygwin_sig_thread->tid,
+ th->tid, th->tid);
+
+ inferior *inf = find_inferior_pid (this,
+ windows_process.process_id);
+ thread_info *sig_thr
+ = inf->find_thread (ptid_t (windows_process.process_id,
+ th->cygwin_sig_thread->tid));
+ if (sig_thr->executing ())
+ {
+ DEBUG_EVENTS ("\"sig\" thread %d (0x%x) meant to be executing, "
+ "continuing it now",
+ th->cygwin_sig_thread->tid,
+ th->cygwin_sig_thread->tid);
+ windows_process.continue_one_thread (th->cygwin_sig_thread,
+ cont_flags);
+ }
+ /* Break the chain. */
+ th->cygwin_sig_thread->signaled_thread = nullptr;
+ th->cygwin_sig_thread = nullptr;
+ }
+#endif
+ }
+
+ if (!target_is_non_stop_p ()
+ || (cont_flags & WCONT_CONTINUE_DEBUG_EVENT) != 0)
+ {
+ DEBUG_EVENTS ("windows_continue -> continue_last_debug_event");
+ continue_last_debug_event_main_thread
+ (_("Failed to resume program execution"), continue_status,
+ cont_flags & WCONT_LAST_CALL);
+ }
return TRUE;
}
@@ -1481,13 +1738,27 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
if (sig != GDB_SIGNAL_0)
{
+ /* Allow continuing with the same signal that interrupted us.
+ Otherwise complain. */
+
/* Note it is OK to call get_last_debug_event_ptid() from the
- main thread here, because we know the process_thread thread
- isn't waiting for an event at this point, so there's no data
- race. */
- if (inferior_ptid != get_last_debug_event_ptid ())
+ main thread here in all-stop, because we know the
+ process_thread thread is not waiting for an event at this
+ point, so there is no data race. We cannot call it in
+ non-stop mode, as the process_thread thread _is_ waiting for
+ events right now in that case. However, the restriction does
+ not exist in non-stop mode, so we don't even call it in that
+ mode. */
+ if (!target_is_non_stop_p ()
+ && inferior_ptid != get_last_debug_event_ptid ())
{
- /* ContinueDebugEvent will be for a different thread. */
+ /* In all-stop, ContinueDebugEvent will be for a different
+ thread. For non-stop, we've called ContinueDebugEvent
+ with DBG_REPLY_LATER for this thread, so we just set the
+ intended continue status in 'auto_cont', which is later
+ passed to ContinueDebugEvent in windows_nat_target::wait
+ after we resume the thread and we get the replied-later
+ (repeated) event out of WaitForDebugEvent. */
DEBUG_EXCEPT ("Cannot continue with signal %d here. "
"Not last-event thread", sig);
}
@@ -1523,33 +1794,40 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
th->last_sig);
}
+ /* If DBG_REPLY_LATER was used on the thread, we override the
+ continue status that will be passed to ContinueDebugEvent later
+ with the continue status we've just determined fulfils the
+ caller's resumption request. Note that DBG_REPLY_LATER is only
+ used in non-stop mode, and in that mode, windows_continue (called
+ below) does not call ContinueDebugEvent. */
+ if (th->auto_cont != 0)
+ th->auto_cont = continue_status;
+
+ /* Single step by setting t bit (trap flag). The trap flag is
+ automatically reset as soon as the single-step exception arrives,
+ however, it's possible to suspend/stop a thread before it
+ executes any instruction, leaving the trace flag set. If we
+ subsequently decide to continue such a thread instead of stepping
+ it, and we didn't clear the trap flag, the thread would step, and
+ we'd end up reporting a SIGTRAP to the core which the core
+ couldn't explain (because the thread wasn't supposed to be
+ stepping), and end up reporting a spurious SIGTRAP to the
+ user. */
+ regcache *regcache = get_thread_regcache (inferior_thread ());
+ fetch_registers (regcache, gdbarch_ps_regnum (regcache->arch ()));
+
+ DWORD *eflags;
#ifdef __x86_64__
if (windows_process.wow64_process)
- {
- if (step)
- {
- /* Single step by setting t bit. */
- regcache *regcache = get_thread_regcache (inferior_thread ());
- struct gdbarch *gdbarch = regcache->arch ();
- fetch_registers (regcache, gdbarch_ps_regnum (gdbarch));
- th->wow64_context.EFlags |= FLAG_TRACE_BIT;
- }
- }
+ eflags = &th->wow64_context.EFlags;
else
#endif
- {
- if (step)
- {
- /* Single step by setting t bit. */
- regcache *regcache = get_thread_regcache (inferior_thread ());
- struct gdbarch *gdbarch = regcache->arch ();
- fetch_registers (regcache, gdbarch_ps_regnum (gdbarch));
- th->context.EFlags |= FLAG_TRACE_BIT;
- }
- }
+ eflags = &th->context.EFlags;
- /* Allow continuing with the same signal that interrupted us.
- Otherwise complain. */
+ if (step)
+ *eflags |= FLAG_TRACE_BIT;
+ else
+ *eflags &= ~FLAG_TRACE_BIT;
if (resume_all)
windows_continue (continue_status, -1);
@@ -1598,12 +1876,88 @@ windows_nat_target::interrupt ()
"Press Ctrl-c in the program console."));
}
+/* Stop thread TH. This leaves a GDB_SIGNAL_0 pending in the thread,
+ which is later consumed by windows_nat_target::wait. */
+
+void
+windows_nat_target::stop_one_thread (windows_thread_info *th)
+{
+ ptid_t thr_ptid (windows_process.process_id, th->tid);
+
+ if (th->suspended == -1)
+ {
+ /* Already known to be stopped; and suspension failed, most
+ probably because the thread is exiting. Do nothing, and let
+ the thread exit event be reported. */
+ DEBUG_EVENTS ("already suspended %s: suspended=%d, stopping=%d",
+ thr_ptid.to_string ().c_str (),
+ th->suspended, th->stopping);
+ }
+#ifdef __CYGWIN__
+ else if (th->suspended
+ && th->signaled_thread != nullptr
+ && th->pending_status.kind () == TARGET_WAITKIND_IGNORE)
+ {
+ DEBUG_EVENTS ("explict stop for \"sig\" thread %s held for signal",
+ thr_ptid.to_string ().c_str ());
+
+ th->stopping = true;
+ th->pending_status.set_stopped (GDB_SIGNAL_0);
+ th->last_event = {};
+ serial_event_set (m_wait_event);
+ }
+#endif
+ else if (th->suspended)
+ {
+ /* Already known to be stopped; do nothing. */
+
+ DEBUG_EVENTS ("already suspended %s: suspended=%d, stopping=%d",
+ thr_ptid.to_string ().c_str (),
+ th->suspended, th->stopping);
+
+ th->stopping = true;
+ }
+ else
+ {
+ DEBUG_EVENTS ("stop request for %s", thr_ptid.to_string ().c_str ());
+
+ th->suspend ();
+ gdb_assert (th->suspended);
+
+ th->stopping = true;
+ th->pending_status.set_stopped (GDB_SIGNAL_0);
+ th->last_event = {};
+ serial_event_set (m_wait_event);
+ }
+}
+
+/* Implementation of target_ops::stop. */
+
+void
+windows_nat_target::stop (ptid_t ptid)
+{
+ for (auto &th : windows_process.thread_list)
+ {
+ ptid_t thr_ptid (windows_process.process_id, th->tid);
+ if (thr_ptid.matches (ptid))
+ stop_one_thread (th.get ());
+ }
+}
+
void
windows_nat_target::pass_ctrlc ()
{
interrupt ();
}
+/* Implementation of the target_ops::thread_events method. */
+
+void
+windows_nat_target::thread_events (int enable)
+{
+ m_report_thread_events = enable;
+}
+
/* Get the next event from the child. Returns the thread ptid. */
ptid_t
@@ -1619,7 +1973,7 @@ windows_nat_target::get_windows_debug_event
for details on why this is needed. */
for (auto &th : windows_process.thread_list)
{
- if (!th->suspended
+ if ((!th->suspended || th->stopping)
&& th->pending_status.kind () != TARGET_WAITKIND_IGNORE)
{
DEBUG_EVENTS ("reporting pending event for 0x%x", th->tid);
@@ -1630,7 +1984,6 @@ windows_nat_target::get_windows_debug_event
*current_event = th->last_event;
ptid_t ptid (windows_process.process_id, thread_id);
- windows_process.invalidate_context (th.get ());
return ptid;
}
}
@@ -1648,6 +2001,50 @@ windows_nat_target::get_windows_debug_event
event_code = current_event->dwDebugEventCode;
ourstatus->set_spurious ();
+ ptid_t result_ptid (current_event->dwProcessId,
+ current_event->dwThreadId, 0);
+ windows_thread_info *result_th = windows_process.find_thread (result_ptid);
+
+ /* If we previously used DBG_REPLY_LATER on this thread, and we're
+ seeing an event for it, it means we've already processed the
+ event, and then subsequently resumed the thread [1], intending to
+ pass AUTO_CONT to ContinueDebugEvent. Do that now, before the
+ switch table below, which may have side effects that don't make
+ sense for a delayed event.
+
+ [1] - with the caveat that sometimes Windows reports an event for
+ a suspended thread. Also handled below. */
+ if (result_th != nullptr && result_th->auto_cont != 0)
+ {
+ DEBUG_EVENTS ("auto-cont thread 0x%x", result_th->tid);
+
+ gdb_assert (dbg_reply_later_available ());
+
+ if (result_th->suspended)
+ {
+ /* Pending stop. See the comment by the definition of
+ "pending_status" for details on why this is needed. */
+ DEBUG_EVENTS ("unexpected auto-cont stop in suspended thread 0x%x",
+ result_th->tid);
+
+ /* Put the event back in the kernel queue. We haven't yet
+ decided which reply to use. */
+ continue_status = DBG_REPLY_LATER;
+ }
+ else
+ {
+ continue_status = result_th->auto_cont;
+ result_th->auto_cont = 0;
+ }
+
+ /* Go back to waiting for the next event. */
+ continue_last_debug_event_main_thread
+ (_("Failed to auto-continue thread"), continue_status);
+
+ ourstatus->set_ignore ();
+ return null_ptid;
+ }
+
switch (event_code)
{
case CREATE_THREAD_DEBUG_EVENT:
@@ -1680,21 +2077,35 @@ windows_nat_target::get_windows_debug_event
current_event->u.CreateThread.lpThreadLocalBase,
false /* main_thread_p */));
- /* This updates debug registers if necessary. */
- windows_process.continue_one_thread (th, 0);
+ /* Update the debug registers if we're not reporting the stop.
+ If we are (reporting the stop), the debug registers will be
+ updated when the thread is eventually re-resumed. */
+ if (m_report_thread_events)
+ ourstatus->set_thread_created ();
+ else
+ windows_process.continue_one_thread (th, 0);
}
break;
case EXIT_THREAD_DEBUG_EVENT:
- DEBUG_EVENTS ("kernel event for pid=%u tid=0x%x code=%s",
- (unsigned) current_event->dwProcessId,
- (unsigned) current_event->dwThreadId,
- "EXIT_THREAD_DEBUG_EVENT");
- delete_thread (ptid_t (current_event->dwProcessId,
- current_event->dwThreadId, 0),
- current_event->u.ExitThread.dwExitCode,
- false /* main_thread_p */);
- thread_id = 0;
+ {
+ DEBUG_EVENTS ("kernel event for pid=%u tid=0x%x code=%s",
+ (unsigned) current_event->dwProcessId,
+ (unsigned) current_event->dwThreadId,
+ "EXIT_THREAD_DEBUG_EVENT");
+ ptid_t thr_ptid (current_event->dwProcessId,
+ current_event->dwThreadId, 0);
+ if (m_report_thread_events)
+ {
+ ourstatus->set_thread_exited
+ (current_event->u.ExitThread.dwExitCode);
+ return thr_ptid;
+ }
+ delete_thread (thr_ptid,
+ current_event->u.ExitThread.dwExitCode,
+ false /* main_thread_p */);
+ thread_id = 0;
+ }
break;
case CREATE_PROCESS_DEBUG_EVENT:
@@ -1820,8 +2231,24 @@ windows_nat_target::get_windows_debug_event
"OUTPUT_DEBUG_STRING_EVENT");
if (windows_process.saw_create != 1)
break;
- thread_id = windows_process.handle_output_debug_string (*current_event,
- ourstatus);
+ if (windows_process.handle_output_debug_string (*current_event,
+ ourstatus))
+ {
+ /* We caught a Cygwin signal for a thread. That thread now
+ has a pending event, and the "sig" thread is
+ suspended. */
+ serial_event_set (m_wait_event);
+
+ /* In all-stop, return now to avoid reaching
+ ContinueDebugEvent further below. In all-stop, it's
+ always windows_nat_target::resume that does the
+ ContinueDebugEvent call. */
+ if (!target_is_non_stop_p ())
+ {
+ ourstatus->set_ignore ();
+ return null_ptid;
+ }
+ }
break;
default:
@@ -1856,22 +2283,35 @@ windows_nat_target::get_windows_debug_event
"unexpected stop in suspended thread 0x%x",
thread_id);
- if (current_event->dwDebugEventCode == EXCEPTION_DEBUG_EVENT
- && ((current_event->u.Exception.ExceptionRecord.ExceptionCode
- == EXCEPTION_BREAKPOINT)
- || (current_event->u.Exception.ExceptionRecord.ExceptionCode
- == STATUS_WX86_BREAKPOINT))
- && windows_process.windows_initialization_done)
+ if (dbg_reply_later_available ())
{
- th->stopped_at_software_breakpoint = true;
- th->pc_adjusted = false;
+ /* Thankfully, the Windows kernel doesn't immediately
+ re-report the unexpected event for a suspended thread
+ when we defer it with DBG_REPLY_LATER, otherwise this
+ would get us stuck in an infinite loop re-processing the
+ same unexpected event over and over. */
+ continue_status = DBG_REPLY_LATER;
}
+ else
+ {
+ if (current_event->dwDebugEventCode == EXCEPTION_DEBUG_EVENT
+ && ((current_event->u.Exception.ExceptionRecord.ExceptionCode
+ == EXCEPTION_BREAKPOINT)
+ || (current_event->u.Exception.ExceptionRecord.ExceptionCode
+ == STATUS_WX86_BREAKPOINT))
+ && windows_process.windows_initialization_done)
+ {
+ th->stopped_at_software_breakpoint = true;
+ th->pc_adjusted = false;
+ }
- th->pending_status = *ourstatus;
- ourstatus->set_ignore ();
+ th->pending_status = *ourstatus;
+ th->last_event = {};
+ }
continue_last_debug_event_main_thread
(_("Failed to resume program execution"), continue_status);
+ ourstatus->set_ignore ();
return null_ptid;
}
@@ -1902,6 +2342,14 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
ptid_t result = get_windows_debug_event (pid, ourstatus, options,
¤t_event);
+ /* True if this is a pending event that we injected ourselves,
+ instead of a real event out of WaitForDebugEvent. */
+ bool fake = current_event.dwDebugEventCode == 0;
+
+ DEBUG_EVENTS ("get_windows_debug_event returned [%s : %s, fake=%d]",
+ result.to_string ().c_str (),
+ ourstatus->to_string ().c_str(),
+ fake);
if ((options & TARGET_WNOHANG) != 0
&& ourstatus->kind () == TARGET_WAITKIND_IGNORE)
@@ -1932,10 +2380,46 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
th->pc_adjusted = false;
}
+ /* If non-stop, suspend the event thread, and continue
+ it with DBG_REPLY_LATER, so the other threads go back
+ to running as soon as possible. Don't do this if
+ stopping the thread, as in that case the thread was
+ already suspended, and also there's no real Windows
+ debug event to continue in that case. */
+ if (windows_process.windows_initialization_done
+ && target_is_non_stop_p ()
+ && !fake)
+ {
+ if (ourstatus->kind () == TARGET_WAITKIND_THREAD_EXITED)
+ {
+ /* If we failed to suspend the thread in
+ windows_nat_target::stop, then 'suspended'
+ will be -1 here. */
+ gdb_assert (th->suspended < 1);
+ delete_thread (result,
+ ourstatus->exit_status (),
+ false /* main_thread_p */);
+ continue_last_debug_event_main_thread
+ (_("Init: Failed to DBG_CONTINUE after thread exit"),
+ DBG_CONTINUE);
+ }
+ else
+ {
+ th->suspend ();
+ th->auto_cont = DBG_CONTINUE;
+ continue_last_debug_event_main_thread
+ (_("Init: Failed to defer event with DBG_REPLY_LATER"),
+ DBG_REPLY_LATER);
+ }
+ }
+
/* All-stop, suspend all threads until they are
explicitly resumed. */
- for (auto &thr : windows_process.thread_list)
- thr->suspend ();
+ if (!target_is_non_stop_p ())
+ for (auto &thr : windows_process.thread_list)
+ thr->suspend ();
+
+ th->stopping = false;
}
/* If something came out, assume there may be more. This is
@@ -2017,7 +2501,7 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching)
/* Don't use windows_nat_target::resume here because that
assumes that inferior_ptid points at a valid thread, and we
haven't switched to any thread yet. */
- windows_continue (DBG_CONTINUE, -1);
+ windows_continue (DBG_CONTINUE, -1, WCONT_CONTINUE_DEBUG_EVENT);
}
switch_to_thread (this->find_thread (last_ptid));
@@ -2162,7 +2646,29 @@ windows_nat_target::attach (const char *args, int from_tty)
#endif
do_initial_windows_stuff (pid, 1);
- target_terminal::ours ();
+
+ if (target_is_non_stop_p ())
+ {
+ /* Leave all threads running. */
+
+ continue_last_debug_event_main_thread
+ (_("Failed to DBG_CONTINUE after attach"),
+ DBG_CONTINUE);
+
+ /* The thread that reports the initial breakpoint, and thus ends
+ up as selected thread here, was injected by Windows into the
+ program for the attach, and it exits as soon as we resume it.
+ Switch to the first thread in the inferior, otherwise the
+ user will be left with an exited thread selected. */
+ switch_to_thread (first_thread_of_inferior (current_inferior ()));
+ }
+ else
+ {
+ set_running (this, minus_one_ptid, false);
+ set_executing (this, minus_one_ptid, false);
+
+ target_terminal::ours ();
+ }
}
void
@@ -2280,10 +2786,11 @@ windows_nat_target::detach (inferior *inf, int from_tty)
bool process_alive = true;
/* The process_thread helper thread will be blocked in
- WaitForDebugEvent waiting for events if we've resumed the target
- before we get here, e.g., with "attach&" or "c&". We need to
- unblock it so that we can have it call DebugActiveProcessStop
- below, in the do_synchronously block. */
+ WaitForDebugEvent waiting for events if we're in non-stop mode,
+ or if in all-stop and we've resumed the target before we get
+ here, e.g., with "attach&" or "c&". We need to unblock it so
+ that we can have it call DebugActiveProcessStop below, in the
+ do_synchronously block. */
if (m_continued)
break_out_process_thread (process_alive);
@@ -3035,13 +3542,33 @@ windows_nat_target::create_inferior (const char *exec_file,
do_initial_windows_stuff (pi.dwProcessId, 0);
- /* windows_continue (DBG_CONTINUE, -1); */
+ /* There is one thread in the process now, and inferior_ptid points
+ to it. Present it as stopped to the core. */
+ windows_thread_info *th = windows_process.find_thread (inferior_ptid);
+
+ th->suspend ();
+ set_running (this, inferior_ptid, false);
+ set_executing (this, inferior_ptid, false);
+
+ if (target_is_non_stop_p ())
+ {
+ /* In non-stop mode, we always immediately use DBG_REPLY_LATER
+ on threads as soon as they report an event. However, during
+ the initial startup, windows_nat_target::wait does not do
+ this, so we need to handle it here for the initial
+ thread. */
+ th->auto_cont = DBG_CONTINUE;
+ continue_last_debug_event_main_thread
+ (_("Failed to defer event with DBG_REPLY_LATER"),
+ DBG_REPLY_LATER);
+ }
}
void
windows_nat_target::mourn_inferior ()
{
- windows_continue (DBG_CONTINUE, -1, WCONT_LAST_CALL);
+ windows_continue (DBG_CONTINUE, -1,
+ WCONT_LAST_CALL | WCONT_CONTINUE_DEBUG_EVENT);
x86_cleanup_dregs();
if (windows_process.open_process_used)
{
@@ -3096,14 +3623,28 @@ windows_nat_target::kill ()
{
CHECK (TerminateProcess (windows_process.handle, 0));
+ /* In non-stop mode, windows_continue does not call
+ ContinueDebugEvent by default. This behavior is appropriate for
+ the first call to windows_continue because any thread that is
+ stopped has already been ContinueDebugEvent'ed with
+ DBG_REPLY_LATER. However, after the first
+ wait_for_debug_event_main_thread call in the loop, this will no
+ longer be true.
+
+ In all-stop mode, the WCONT_CONTINUE_DEBUG_EVENT flag has no
+ effect, so writing the code in this way ensures that the code is
+ the same for both modes. */
+ windows_continue_flags flags = WCONT_KILLED;
+
for (;;)
{
- if (!windows_continue (DBG_CONTINUE, -1, WCONT_KILLED))
+ if (!windows_continue (DBG_CONTINUE, -1, flags))
break;
DEBUG_EVENT current_event;
wait_for_debug_event_main_thread (¤t_event);
if (current_event.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT)
break;
+ flags |= WCONT_CONTINUE_DEBUG_EVENT;
}
target_mourn_inferior (inferior_ptid); /* Or just windows_mourn_inferior? */
@@ -3237,6 +3778,24 @@ windows_nat_target::thread_name (struct thread_info *thr)
return th->thread_name ();
}
+/* Implementation of the target_ops::supports_non_stop method. */
+
+bool
+windows_nat_target::supports_non_stop ()
+{
+ /* Non-stop support requires DBG_REPLY_LATER, which only exists on
+ Windows 10 and later. */
+ return dbg_reply_later_available ();
+}
+
+/* Implementation of the target_ops::always_non_stop_p method. */
+
+bool
+windows_nat_target::always_non_stop_p ()
+{
+ /* If we can do non-stop, prefer it. */
+ return supports_non_stop ();
+}
void _initialize_windows_nat ();
void
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 65b01dc97ac..b15d8d0c16d 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -625,7 +625,7 @@ win32_process_target::attach (unsigned long pid)
/* See nat/windows-nat.h. */
-DWORD
+bool
gdbserver_windows_process::handle_output_debug_string
(const DEBUG_EVENT ¤t_event,
struct target_waitstatus *ourstatus)
@@ -636,7 +636,7 @@ gdbserver_windows_process::handle_output_debug_string
DWORD nbytes = current_event.u.DebugString.nDebugStringLength;
if (nbytes == 0)
- return 0;
+ return false;
if (nbytes > READ_BUFFER_LEN)
nbytes = READ_BUFFER_LEN;
@@ -655,7 +655,7 @@ gdbserver_windows_process::handle_output_debug_string
else
{
if (read_inferior_memory (addr, (unsigned char *) s, nbytes) != 0)
- return 0;
+ return false;
}
if (!startswith (s, "cYg"))
@@ -663,14 +663,14 @@ gdbserver_windows_process::handle_output_debug_string
if (!server_waiting)
{
OUTMSG2(("%s", s));
- return 0;
+ return false;
}
monitor_output (s);
}
#undef READ_BUFFER_LEN
- return 0;
+ return false;
}
static void
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index ea2a9b4c5b6..2c1b3b9335c 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -175,8 +175,8 @@ class win32_process_target : public process_stratum_target
struct gdbserver_windows_process : public windows_nat::windows_process_info
{
windows_nat::windows_thread_info *find_thread (ptid_t ptid) override;
- DWORD handle_output_debug_string (const DEBUG_EVENT ¤t_event,
- struct target_waitstatus *ourstatus) override;
+ bool handle_output_debug_string (const DEBUG_EVENT ¤t_event,
+ struct target_waitstatus *ourstatus) override;
void handle_load_dll (const char *dll_name, LPVOID base) override;
void handle_unload_dll (const DEBUG_EVENT ¤t_event) override;
bool handle_access_violation (const EXCEPTION_RECORD *rec) override;
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 33/34] Windows gdb: Watchpoints while running (internal vs external stops)
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (31 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 32/34] Windows gdb: Add non-stop support Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-07 23:42 ` [PATCH 34/34] Mention Windows non-stop support in NEWS Pedro Alves
2024-05-08 15:40 ` [PATCH 00/34] Windows non-stop mode Tom Tromey
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
Teach the Windows target to temporarily pause all threads when we
change the debug registers for a watchpoint. Implements the same
logic as Linux uses:
~~~
/* (...) if threads are running when the
mirror changes, a temporary and transparent stop on all threads
is forced so they can get their copy of the debug registers
updated on re-resume. (...) */
~~~
On Linux, we send each thread a SIGSTOP to step them. On Windows,
SuspendThread itself doesn't cause any asynchronous debug event to be
reported. However, we've implemented windows_nat_target::stop such
that it uses SuspendThread, and then queues a pending GDB_SIGNAL_0
stop on the thread. That results in a user-visible stop, while here
we want a non-user-visible stop. So what we do is re-use that
windows_nat_target::stop stopping mechanism, but add an external vs
internal stopping kind distinction. An internal stop results in
windows_nat_target::wait immediately re-resuming the thread.
Note we don't make the debug registers poking code SuspendThread ->
write debug registers -> ContinueThread itself, because SuspendThread
is actually asynchronous and may take a bit to stop the thread (a
following GetThreadContext blocks until the thread is actually
suspended), and, there will be several debug register writes when a
watchpoint is set, because we have to set all of DR0, DR1, DR2, DR3,
and DR7. Defering the actualy writes to ::wait avoids a bunch of
SuspendThread/ResumeThread sequences, so in principle should be
faster.
Change-Id: I39c2492c7aac06d23ef8f287f4afe3747b7bc53f
---
gdb/nat/windows-nat.h | 27 ++++++++--
gdb/windows-nat.c | 116 ++++++++++++++++++++++++++++++++++--------
2 files changed, 119 insertions(+), 24 deletions(-)
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index c6d0c4e98dd..8399ed7b3b7 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -34,6 +34,24 @@ namespace windows_nat
struct windows_process_info;
+/* The reason for explicitly stopping a thread. Note the enumerators
+ are ordered such that when comparing two stopping_kind's numerical
+ value, the highest should prevail. */
+enum stopping_kind
+ {
+ /* Not really stopping the thread. */
+ SK_NOT_STOPPING = 0,
+
+ /* We're stopping the thread for internal reasons, the stop should
+ not be reported as an event to the core. */
+ SK_INTERNAL = 1,
+
+ /* We're stopping the thread for external reasons, meaning, the
+ core/user asked us to stop the thread, so we must report a stop
+ event to the core. */
+ SK_EXTERNAL = 2,
+ };
+
/* Thread information structure used to track extra information about
each thread. */
struct windows_thread_info
@@ -117,9 +135,10 @@ struct windows_thread_info
int suspended = 0;
/* This flag indicates whether we are explicitly stopping this
- thread in response to a target_stop request. This allows
- distinguishing between threads that are explicitly stopped by the
- debugger and threads that are stopped due to other reasons.
+ thread in response to a target_stop request or for
+ backend-internal reasons. This allows distinguishing between
+ threads that are explicitly stopped by the debugger and threads
+ that are stopped due to other reasons.
Typically, when we want to stop a thread, we suspend it, enqueue
a pending GDB_SIGNAL_0 stop status on the thread, and then set
@@ -128,7 +147,7 @@ struct windows_thread_info
already has an event to report. In such case, we simply set the
'stopping' flag without suspending the thread or enqueueing a
pending stop. See stop_one_thread. */
- bool stopping = false;
+ stopping_kind stopping = SK_NOT_STOPPING;
/* Info about a potential pending stop.
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index a33ef868566..cb1d030d12b 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -261,6 +261,10 @@ enum windows_continue_flag
all-stop mode. This flag indicates that windows_continue
should call ContinueDebugEvent even in non-stop mode. */
WCONT_CONTINUE_DEBUG_EVENT = 4,
+
+ /* Skip calling ContinueDebugEvent even in all-stop mode. This is
+ the default in non-stop mode. */
+ WCONT_DONT_CONTINUE_DEBUG_EVENT = 8,
};
DEF_ENUM_FLAGS_TYPE (windows_continue_flag, windows_continue_flags);
@@ -521,6 +525,8 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
return serial_event_fd (m_wait_event);
}
+ void debug_registers_changed_all_threads ();
+
private:
windows_thread_info *add_thread (ptid_t ptid, HANDLE h, void *tlb,
@@ -528,7 +534,8 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
void delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p);
DWORD fake_create_process (const DEBUG_EVENT ¤t_event);
- void stop_one_thread (windows_thread_info *th);
+ void stop_one_thread (windows_thread_info *th,
+ enum stopping_kind stopping_kind);
BOOL windows_continue (DWORD continue_status, int id,
windows_continue_flags cont_flags = 0);
@@ -1294,7 +1301,7 @@ windows_per_inferior::handle_output_debug_string
a pending event. It will be picked up by
windows_nat_target::wait. */
th->suspend ();
- th->stopping = true;
+ th->stopping = SK_EXTERNAL;
th->last_event = {};
th->pending_status.set_stopped (gotasig);
@@ -1605,7 +1612,7 @@ windows_per_inferior::continue_one_thread (windows_thread_info *th,
}
th->resume ();
- th->stopping = false;
+ th->stopping = SK_NOT_STOPPING;
th->last_sig = GDB_SIGNAL_0;
}
@@ -1680,8 +1687,19 @@ windows_nat_target::windows_continue (DWORD continue_status, int id,
#endif
}
- if (!target_is_non_stop_p ()
- || (cont_flags & WCONT_CONTINUE_DEBUG_EVENT) != 0)
+ /* WCONT_DONT_CONTINUE_DEBUG_EVENT and WCONT_CONTINUE_DEBUG_EVENT
+ can't both be enabled at the same time. */
+ gdb_assert ((cont_flags & WCONT_DONT_CONTINUE_DEBUG_EVENT) == 0
+ || (cont_flags & WCONT_CONTINUE_DEBUG_EVENT) == 0);
+
+ bool continue_debug_event;
+ if ((cont_flags & WCONT_CONTINUE_DEBUG_EVENT) != 0)
+ continue_debug_event = true;
+ else if ((cont_flags & WCONT_DONT_CONTINUE_DEBUG_EVENT) != 0)
+ continue_debug_event = false;
+ else
+ continue_debug_event = !target_is_non_stop_p ();
+ if (continue_debug_event)
{
DEBUG_EVENTS ("windows_continue -> continue_last_debug_event");
continue_last_debug_event_main_thread
@@ -1876,11 +1894,13 @@ windows_nat_target::interrupt ()
"Press Ctrl-c in the program console."));
}
-/* Stop thread TH. This leaves a GDB_SIGNAL_0 pending in the thread,
- which is later consumed by windows_nat_target::wait. */
+/* 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. */
void
-windows_nat_target::stop_one_thread (windows_thread_info *th)
+windows_nat_target::stop_one_thread (windows_thread_info *th,
+ enum stopping_kind stopping_kind)
{
ptid_t thr_ptid (windows_process.process_id, th->tid);
@@ -1896,12 +1916,18 @@ windows_nat_target::stop_one_thread (windows_thread_info *th)
#ifdef __CYGWIN__
else if (th->suspended
&& th->signaled_thread != nullptr
- && th->pending_status.kind () == TARGET_WAITKIND_IGNORE)
+ && th->pending_status.kind () == TARGET_WAITKIND_IGNORE
+ /* If doing an internal stop to update debug registers,
+ then just leave the "sig" thread suspended. Otherwise
+ windows_nat_target::wait would incorrectly break the
+ signaled_thread lock when it later processes the pending
+ stop and calls windows_continue on this thread. */
+ && stopping_kind == SK_EXTERNAL)
{
DEBUG_EVENTS ("explict stop for \"sig\" thread %s held for signal",
thr_ptid.to_string ().c_str ());
- th->stopping = true;
+ th->stopping = stopping_kind;
th->pending_status.set_stopped (GDB_SIGNAL_0);
th->last_event = {};
serial_event_set (m_wait_event);
@@ -1915,7 +1941,9 @@ windows_nat_target::stop_one_thread (windows_thread_info *th)
thr_ptid.to_string ().c_str (),
th->suspended, th->stopping);
- th->stopping = true;
+ /* Upgrade stopping. */
+ if (stopping_kind > th->stopping)
+ th->stopping = stopping_kind;
}
else
{
@@ -1924,9 +1952,13 @@ windows_nat_target::stop_one_thread (windows_thread_info *th)
th->suspend ();
gdb_assert (th->suspended);
- th->stopping = true;
- th->pending_status.set_stopped (GDB_SIGNAL_0);
- th->last_event = {};
+ if (stopping_kind > th->stopping)
+ {
+ th->stopping = stopping_kind;
+ th->pending_status.set_stopped (GDB_SIGNAL_0);
+ th->last_event = {};
+ }
+
serial_event_set (m_wait_event);
}
}
@@ -1940,7 +1972,7 @@ windows_nat_target::stop (ptid_t ptid)
{
ptid_t thr_ptid (windows_process.process_id, th->tid);
if (thr_ptid.matches (ptid))
- stop_one_thread (th.get ());
+ stop_one_thread (th.get (), SK_EXTERNAL);
}
}
@@ -2367,6 +2399,17 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
{
windows_thread_info *th = windows_process.find_thread (result);
+ /* If this thread was temporarily stopped just so we
+ could update its debug registers on the next
+ resumption, do it now. */
+ if (th->stopping == SK_INTERNAL)
+ {
+ gdb_assert (fake);
+ windows_continue (DBG_CONTINUE, th->tid,
+ WCONT_DONT_CONTINUE_DEBUG_EVENT);
+ continue;
+ }
+
th->stopped_at_software_breakpoint = false;
if (current_event.dwDebugEventCode
== EXCEPTION_DEBUG_EVENT
@@ -2419,7 +2462,7 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
for (auto &thr : windows_process.thread_list)
thr->suspend ();
- th->stopping = false;
+ th->stopping = SK_NOT_STOPPING;
}
/* If something came out, assume there may be more. This is
@@ -3902,6 +3945,41 @@ Use \"file\" or \"dll\" command to load executable/libraries directly."));
}
}
+/* For each thread, set the debug_registers_changed flag, and
+ temporarily stop it so we can update its debug registers. */
+
+void
+windows_nat_target::debug_registers_changed_all_threads ()
+{
+ for (auto &th : windows_process.thread_list)
+ {
+ th->debug_registers_changed = true;
+
+ /* Note we don't SuspendThread => update debug regs =>
+ ResumeThread, because SuspendThread is actually asynchronous
+ (and GetThreadContext blocks until the thread really
+ suspends), and doing that for all threads may take a bit.
+ Also, the core does one call per DR register update, so that
+ would result in a lot of suspend-resumes. So instead, we
+ suspend the thread if it wasn't already suspended, and queue
+ a pending stop to be handled by windows_nat_target::wait.
+ This means we only stop each thread once, and, we don't block
+ waiting for each individual thread stop. */
+ stop_one_thread (th.get (), SK_INTERNAL);
+ }
+}
+
+/* Trampoline helper to get at the
+ windows_nat_target::debug_registers_changed_all_threads method in
+ the native target. */
+
+static void
+debug_registers_changed_all_threads ()
+{
+ auto *win_tgt = static_cast<windows_nat_target *> (get_native_target ());
+ win_tgt->debug_registers_changed_all_threads ();
+}
+
/* Hardware watchpoint support, adapted from go32-nat.c code. */
/* Pass the address ADDR to the inferior in the I'th debug register.
@@ -3913,8 +3991,7 @@ windows_set_dr (int i, CORE_ADDR addr)
if (i < 0 || i > 3)
internal_error (_("Invalid register %d in windows_set_dr.\n"), i);
- for (auto &th : windows_process.thread_list)
- th->debug_registers_changed = true;
+ debug_registers_changed_all_threads ();
}
/* Pass the value VAL to the inferior in the DR7 debug control
@@ -3923,8 +4000,7 @@ windows_set_dr (int i, CORE_ADDR addr)
static void
windows_set_dr7 (unsigned long val)
{
- for (auto &th : windows_process.thread_list)
- th->debug_registers_changed = true;
+ debug_registers_changed_all_threads ();
}
/* Get the value of debug register I from the inferior. */
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH 34/34] Mention Windows non-stop support in NEWS
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (32 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 33/34] Windows gdb: Watchpoints while running (internal vs external stops) Pedro Alves
@ 2024-05-07 23:42 ` Pedro Alves
2024-05-08 15:40 ` [PATCH 00/34] Windows non-stop mode Tom Tromey
34 siblings, 0 replies; 64+ messages in thread
From: Pedro Alves @ 2024-05-07 23:42 UTC (permalink / raw)
To: gdb-patches
Add a note to gdb/NEWS mentioning Windows non-stop support.
Change-Id: Id0e28525c06e57120c47b07f978581d1627d70f3
---
gdb/NEWS | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gdb/NEWS b/gdb/NEWS
index 050f6f96e62..b867cb27d64 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -9,6 +9,9 @@
* Building GDB and GDBserver now requires a C++17 compiler.
For example, GCC 9 or later.
+* The Windows native target now supports non-stop mode. This feature
+ requires Windows 10 or later.
+
* GDB index now contains information about the main function. This speeds up
startup when it is being used for some large binaries.
--
2.43.2
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 00/34] Windows non-stop mode
2024-05-07 23:41 [PATCH 00/34] Windows non-stop mode Pedro Alves
` (33 preceding siblings ...)
2024-05-07 23:42 ` [PATCH 34/34] Mention Windows non-stop support in NEWS Pedro Alves
@ 2024-05-08 15:40 ` Tom Tromey
2024-05-15 17:35 ` Tom Tromey
34 siblings, 1 reply; 64+ messages in thread
From: Tom Tromey @ 2024-05-08 15:40 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> This series adds non-stop mode support to the Windows native backend,
Pedro> on Windows 10 and above. Earlier Windows version lack the necessary
Pedro> feature, so those keep working in all-stop mode, only.
Pedro> After the series, the Windows target backend defaults to working in
Pedro> non-stop mode (as in, "maint set target-non-stop"), even if
Pedro> user-visible mode is all-stop ("set non-stop off"). This is the same
Pedro> as the Linux backend.
Thanks for doing this. This implements a pretty big subset of my
windows-nat wish-list.
I reviewed the easy subset of the patches. I'll do some more later.
I'd suggest not landing the more major changes until after GDB 15.
It seems pretty close to the release to add a big change like this.
Pedro> I've been testing this on Cygwin native with the GDB testsuite as I've
Pedro> been developing this. Running the testsuite on Cygwin is a pain, and
Pedro> many testcases run into cascading timeouts still, and some even hang
Pedro> the test run forever until you kill them manually.
At some point I'll apply the patches and run them through the AdaCore
test suite.
Tom
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 00/34] Windows non-stop mode
2024-05-08 15:40 ` [PATCH 00/34] Windows non-stop mode Tom Tromey
@ 2024-05-15 17:35 ` Tom Tromey
2024-05-15 20:39 ` Pedro Alves
0 siblings, 1 reply; 64+ messages in thread
From: Tom Tromey @ 2024-05-15 17:35 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches
Tom> At some point I'll apply the patches and run them through the AdaCore
Tom> test suite.
I did this today and I found a few failures.
Now, one thing to note is that I did this by merging your branch into
the AdaCore branch; and AdaCore carries a few local changes.
In particular AdaCore still has the "random thread switch" change that I
submitted a long time ago -- and one of the problems seems to be related
to that. Without really debugging I don't know if that's a problem in
the series or with the local changes.
Anyway, there are some other problems as well. For instance:
(gdb) start
Temporary breakpoint 1 at 0x140001aab: file p.adb, line 4.
Starting program: C:\Users\itmgr\sandbox\x86_64-windows64\gdb_version-head_test\tmp\tes
t\gdb-TS-f539jxg_\J225-024__attach_detach_task\p.exe
Thread 1 hit Temporary breakpoint 1, p () at p.adb:4
4 Barrier : Integer := 0;
PASSED:J225-024__attach_detach_task:start
(gdb) detach
Detaching from program: C:\Users\itmgr\sandbox\x86_64-windows64\gdb_version-head_test\t
mp\test\gdb-TS-f539jxg_\J225-024__attach_detach_task\p.exe, process 4272
[Inferior 1 (process 4272) detached]
PASSED:J225-024__attach_detach_task:detach
(gdb) attach 4272
Can't attach to process 4272 (error 87): The parameter is incorrect.
Here the test detaches and then re-attaches -- but attach fails.
Another test runs gdb without a symbol file and then does an attach.
The expected output (happens with gdb head) is:
(gdb) attach 2328
Attaching to process 2328
Reading symbols from C:\Users\itmgr\sandbox\x86_64-windows64\gdb_version-head_test\tmp\
test\gdb-TS-bgt06sfm\N203-009__attach_no_exe\foo_n203_009.EXE...
However with the branch, gdb doesn't try to read the symbols and instead
mentions some system dll:
(gdb) attach 1276
Attaching to process 1276
0x00007ff8b9b50274 in ntdll!ZwDelayExecution ()
from C:\Windows\SYSTEM32\ntdll.dll
There was also a timeout when detaching. The inferior being run is
fairly ordinary -- nothing special, just a spot to set a breakpoint:
(gdb) tbreak break_me
Temporary breakpoint 1 at 0x140001ca4: file pck.adb, line 37.
PASSED:CA30-017__detach:tbreak break_me
(gdb) run
Starting program: C:\Users\itmgr\sandbox\x86_64-windows64\gdb_version-head_test\tmp\test\gdb-TS-f539jxg_\CA30-017__detach\foo.exe
Thread 1 hit Temporary breakpoint 1, pck.break_me () at pck.adb:37
37 null;
PASSED:CA30-017__detach:runto 'break_me'
(gdb) detach
TIMEOUT:CA30-017__detach:detach (timeout)
[killing GDB (pid = N/A (already dead?))]
FWIW I ran the tests before and after the series, on the same machine,
so there shouldn't be any machine configuration issues or anything like
that.
hope this helps,
Tom
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 00/34] Windows non-stop mode
2024-05-15 17:35 ` Tom Tromey
@ 2024-05-15 20:39 ` Pedro Alves
2024-05-16 15:53 ` Tom Tromey
0 siblings, 1 reply; 64+ messages in thread
From: Pedro Alves @ 2024-05-15 20:39 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Thank you very much! I will try to reproduce the issues and see if I can fix them.
To understand whether non-stop or all-stop modes were being used in
the tests used below -- was this with a Windows >= 10 machine or older Windows?
Pedro Alves
On 2024-05-15 18:35, Tom Tromey wrote:
> Tom> At some point I'll apply the patches and run them through the AdaCore
> Tom> test suite.
>
> I did this today and I found a few failures.
>
> Now, one thing to note is that I did this by merging your branch into
> the AdaCore branch; and AdaCore carries a few local changes.
>
> In particular AdaCore still has the "random thread switch" change that I
> submitted a long time ago -- and one of the problems seems to be related
> to that. Without really debugging I don't know if that's a problem in
> the series or with the local changes.
>
> Anyway, there are some other problems as well. For instance:
>
> (gdb) start
> Temporary breakpoint 1 at 0x140001aab: file p.adb, line 4.
> Starting program: C:\Users\itmgr\sandbox\x86_64-windows64\gdb_version-head_test\tmp\tes
> t\gdb-TS-f539jxg_\J225-024__attach_detach_task\p.exe
>
> Thread 1 hit Temporary breakpoint 1, p () at p.adb:4
> 4 Barrier : Integer := 0;
> PASSED:J225-024__attach_detach_task:start
> (gdb) detach
> Detaching from program: C:\Users\itmgr\sandbox\x86_64-windows64\gdb_version-head_test\t
> mp\test\gdb-TS-f539jxg_\J225-024__attach_detach_task\p.exe, process 4272
> [Inferior 1 (process 4272) detached]
> PASSED:J225-024__attach_detach_task:detach
> (gdb) attach 4272
> Can't attach to process 4272 (error 87): The parameter is incorrect.
>
> Here the test detaches and then re-attaches -- but attach fails.
>
>
> Another test runs gdb without a symbol file and then does an attach.
> The expected output (happens with gdb head) is:
>
> (gdb) attach 2328
> Attaching to process 2328
> Reading symbols from C:\Users\itmgr\sandbox\x86_64-windows64\gdb_version-head_test\tmp\
> test\gdb-TS-bgt06sfm\N203-009__attach_no_exe\foo_n203_009.EXE...
>
> However with the branch, gdb doesn't try to read the symbols and instead
> mentions some system dll:
>
> (gdb) attach 1276
> Attaching to process 1276
> 0x00007ff8b9b50274 in ntdll!ZwDelayExecution ()
> from C:\Windows\SYSTEM32\ntdll.dll
>
>
> There was also a timeout when detaching. The inferior being run is
> fairly ordinary -- nothing special, just a spot to set a breakpoint:
>
> (gdb) tbreak break_me
> Temporary breakpoint 1 at 0x140001ca4: file pck.adb, line 37.
> PASSED:CA30-017__detach:tbreak break_me
> (gdb) run
> Starting program: C:\Users\itmgr\sandbox\x86_64-windows64\gdb_version-head_test\tmp\test\gdb-TS-f539jxg_\CA30-017__detach\foo.exe
>
> Thread 1 hit Temporary breakpoint 1, pck.break_me () at pck.adb:37
> 37 null;
> PASSED:CA30-017__detach:runto 'break_me'
> (gdb) detach
> TIMEOUT:CA30-017__detach:detach (timeout)
> [killing GDB (pid = N/A (already dead?))]
>
>
>
> FWIW I ran the tests before and after the series, on the same machine,
> so there shouldn't be any machine configuration issues or anything like
> that.
>
> hope this helps,
> Tom
>
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH 00/34] Windows non-stop mode
2024-05-15 20:39 ` Pedro Alves
@ 2024-05-16 15:53 ` Tom Tromey
0 siblings, 0 replies; 64+ messages in thread
From: Tom Tromey @ 2024-05-16 15:53 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> Thank you very much! I will try to reproduce the issues and see if I can fix them.
Pedro> To understand whether non-stop or all-stop modes were being used in
Pedro> the tests used below -- was this with a Windows >= 10 machine or older Windows?
I think it is a "Windows 2019" machine, which searching tells me is
based on Windows 10. I'm not sure I have access to anything older.
Tom
^ permalink raw reply [flat|nested] 64+ messages in thread