From: Pedro Alves Date: Mon, 8 May 2023 20:36:28 +0000 (+0100) Subject: Windows gdb+gdbserver: Eliminate thread_rec(INVALIDATE_CONTEXT) calls X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0ab4174c4ccc745a125102eeed7c47e77b6fe438;p=thirdparty%2Fbinutils-gdb.git Windows gdb+gdbserver: Eliminate thread_rec(INVALIDATE_CONTEXT) calls 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. Approved-By: Tom Tromey Change-Id: I8d2f0a749d23329956e62362a7007189902dddb5 --- diff --git a/gdb/aarch64-windows-nat.c b/gdb/aarch64-windows-nat.c index c375bbf2ddd..d43ee6f13fe 100644 --- a/gdb/aarch64-windows-nat.c +++ b/gdb/aarch64-windows-nat.c @@ -185,6 +185,7 @@ aarch64_windows_nat_target::fill_thread_context (windows_thread_info *th) if (context->ContextFlags == 0) { + th->suspend (); context->ContextFlags = WindowsContext::all; CHECK (get_thread_context (th->h, context)); } @@ -285,6 +286,7 @@ aarch64_windows_nat_target::store_one_register (const struct regcache *regcache, windows_thread_info *th, int r) { gdb_assert (r >= 0); + gdb_assert (th->context.ContextFlags != 0); char *context_ptr = (char *) &th->context; diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h index 24ef6cc3f9c..1a564fbf11b 100644 --- a/gdb/nat/windows-nat.h +++ b/gdb/nat/windows-nat.h @@ -109,8 +109,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 e8df5e053e6..ca9a197d64e 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -284,11 +284,6 @@ windows_per_inferior::thread_rec (ptid_t ptid, { switch (disposition) { - case INVALIDATE_CONTEXT: - if (ptid.lwp () != current_event.dwThreadId) - th->suspend (); - invalidate_thread_context (th); - break; case DONT_SUSPEND: th->suspended = -1; invalidate_thread_context (th); @@ -392,8 +387,7 @@ windows_nat_target::delete_thread (ptid_t ptid, DWORD exit_code, 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. */ @@ -415,8 +409,7 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r) 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. */ @@ -933,7 +926,9 @@ 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); + if (th != nullptr) + windows_process->invalidate_thread_context (th); return ptid; } @@ -1151,8 +1146,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; } @@ -1190,8 +1184,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/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c index d76b89b186a..5b0f7066fba 100644 --- a/gdb/x86-windows-nat.c +++ b/gdb/x86-windows-nat.c @@ -111,6 +111,7 @@ x86_windows_nat_target::fill_thread_context (windows_thread_info *th) { if (context->ContextFlags == 0) { + th->suspend (); context->ContextFlags = WindowsContext::all; CHECK (get_thread_context (th->h, context)); } @@ -254,6 +255,7 @@ x86_windows_nat_target::store_one_register (const struct regcache *regcache, char *context_ptr = x86_windows_process.with_context (th, [] (auto *context) { + gdb_assert (context->ContextFlags != 0); return (char *) context; }); diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc index d6d7acf599d..180fabe0df9 100644 --- a/gdbserver/win32-low.cc +++ b/gdbserver/win32-low.cc @@ -432,9 +432,8 @@ static void child_fetch_inferior_registers (struct regcache *regcache, int r) { int regno; - windows_thread_info *th - = windows_process.thread_rec (current_thread->id, - INVALIDATE_CONTEXT); + windows_thread_info *th = windows_process.find_thread (current_thread->id); + win32_require_context (th); if (r == -1 || r > NUM_REGS) child_fetch_inferior_registers (regcache, NUM_REGS); else @@ -448,9 +447,8 @@ static void child_store_inferior_registers (struct regcache *regcache, int r) { int regno; - windows_thread_info *th - = windows_process.thread_rec (current_thread->id, - INVALIDATE_CONTEXT); + windows_thread_info *th = windows_process.find_thread (current_thread->id); + win32_require_context (th); if (r == -1 || r == 0 || r > NUM_REGS) child_store_inferior_registers (regcache, NUM_REGS); else