From 296d3d2e156c8fe96c0250d5b59a008e7054946e Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Mon, 4 Oct 2021 14:38:23 -0400 Subject: [PATCH] Fix watchpoints with multiple threads on Windows A recent internal change pointed out that watchpoints were not working on Windows when the inferior was multi-threaded. This happened because the debug registers were only updated for certain threads -- in particular, those that were being resumed and that were not marked as suspended. In the case of single-stepping, the need to update the debug registers in other threads could also be "forgotten". This patch changes windows-nat.c to mark all threads needing a debug register update. This brings the code closer to what gdbserver does (though, unfortunately, it still seems more complicated than needed). --- gdb/windows-nat.c | 71 +++++++++++++---------------------------------- 1 file changed, 20 insertions(+), 51 deletions(-) diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index a26d6eac816..f3201295f31 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -123,8 +123,6 @@ enum | CONTEXT_EXTENDED_REGISTERS static uintptr_t dr[8]; -static int debug_registers_changed; -static int debug_registers_used; static int windows_initialization_done; #define DR6_CLEAR_VALUE 0xffff0ff0 @@ -386,40 +384,10 @@ windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p) else add_thread (&the_windows_nat_target, ptid); - /* Set the debug registers for the new thread if they are used. */ - if (debug_registers_used) - { -#ifdef __x86_64__ - if (wow64_process) - { - /* Only change the value of the debug registers. */ - th->wow64_context.ContextFlags = CONTEXT_DEBUG_REGISTERS; - CHECK (Wow64GetThreadContext (th->h, &th->wow64_context)); - th->wow64_context.Dr0 = dr[0]; - th->wow64_context.Dr1 = dr[1]; - th->wow64_context.Dr2 = dr[2]; - th->wow64_context.Dr3 = dr[3]; - th->wow64_context.Dr6 = DR6_CLEAR_VALUE; - th->wow64_context.Dr7 = dr[7]; - CHECK (Wow64SetThreadContext (th->h, &th->wow64_context)); - th->wow64_context.ContextFlags = 0; - } - else -#endif - { - /* Only change the value of the debug registers. */ - th->context.ContextFlags = CONTEXT_DEBUG_REGISTERS; - CHECK (GetThreadContext (th->h, &th->context)); - th->context.Dr0 = dr[0]; - th->context.Dr1 = dr[1]; - th->context.Dr2 = dr[2]; - th->context.Dr3 = dr[3]; - th->context.Dr6 = DR6_CLEAR_VALUE; - th->context.Dr7 = dr[7]; - CHECK (SetThreadContext (th->h, &th->context)); - th->context.ContextFlags = 0; - } - } + /* It's simplest to always set this and update the debug + registers. */ + th->debug_registers_changed = true; + return th; } @@ -593,7 +561,7 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r) /* Copy dr values from that thread. But only if there were not modified since last stop. PR gdb/2388 */ - if (!debug_registers_changed) + if (!th->debug_registers_changed) { dr[0] = th->wow64_context.Dr0; dr[1] = th->wow64_context.Dr1; @@ -611,7 +579,7 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r) /* Copy dr values from that thread. But only if there were not modified since last stop. PR gdb/2388 */ - if (!debug_registers_changed) + if (!th->debug_registers_changed) { dr[0] = th->context.Dr0; dr[1] = th->context.Dr1; @@ -1193,12 +1161,10 @@ windows_continue (DWORD continue_status, int id, int killed) for (windows_thread_info *th : thread_list) if (id == -1 || id == (int) th->tid) { - if (!th->suspended) - continue; #ifdef __x86_64__ if (wow64_process) { - if (debug_registers_changed) + if (th->debug_registers_changed) { th->wow64_context.ContextFlags |= CONTEXT_DEBUG_REGISTERS; th->wow64_context.Dr0 = dr[0]; @@ -1207,6 +1173,7 @@ windows_continue (DWORD continue_status, int id, int killed) th->wow64_context.Dr3 = dr[3]; th->wow64_context.Dr6 = DR6_CLEAR_VALUE; th->wow64_context.Dr7 = dr[7]; + th->debug_registers_changed = false; } if (th->wow64_context.ContextFlags) { @@ -1227,7 +1194,7 @@ windows_continue (DWORD continue_status, int id, int killed) else #endif { - if (debug_registers_changed) + if (th->debug_registers_changed) { th->context.ContextFlags |= CONTEXT_DEBUG_REGISTERS; th->context.Dr0 = dr[0]; @@ -1236,6 +1203,7 @@ windows_continue (DWORD continue_status, int id, int killed) th->context.Dr3 = dr[3]; th->context.Dr6 = DR6_CLEAR_VALUE; th->context.Dr7 = dr[7]; + th->debug_registers_changed = false; } if (th->context.ContextFlags) { @@ -1268,7 +1236,6 @@ windows_continue (DWORD continue_status, int id, int killed) " (ContinueDebugEvent failed, error %u)"), (unsigned int) GetLastError ()); - debug_registers_changed = 0; return res; } @@ -1365,7 +1332,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig) if (th->wow64_context.ContextFlags) { - if (debug_registers_changed) + if (th->debug_registers_changed) { th->wow64_context.Dr0 = dr[0]; th->wow64_context.Dr1 = dr[1]; @@ -1373,6 +1340,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig) th->wow64_context.Dr3 = dr[3]; th->wow64_context.Dr6 = DR6_CLEAR_VALUE; th->wow64_context.Dr7 = dr[7]; + th->debug_registers_changed = false; } CHECK (Wow64SetThreadContext (th->h, &th->wow64_context)); th->wow64_context.ContextFlags = 0; @@ -1392,7 +1360,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig) if (th->context.ContextFlags) { - if (debug_registers_changed) + if (th->debug_registers_changed) { th->context.Dr0 = dr[0]; th->context.Dr1 = dr[1]; @@ -1400,6 +1368,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig) th->context.Dr3 = dr[3]; th->context.Dr6 = DR6_CLEAR_VALUE; th->context.Dr7 = dr[7]; + th->debug_registers_changed = false; } CHECK (SetThreadContext (th->h, &th->context)); th->context.ContextFlags = 0; @@ -1798,8 +1767,6 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching) last_sig = GDB_SIGNAL_0; open_process_used = 0; - debug_registers_changed = 0; - debug_registers_used = 0; for (i = 0; i < sizeof (dr) / sizeof (dr[0]); i++) dr[i] = 0; #ifdef __CYGWIN__ @@ -3203,8 +3170,9 @@ cygwin_set_dr (int i, CORE_ADDR addr) internal_error (__FILE__, __LINE__, _("Invalid register %d in cygwin_set_dr.\n"), i); dr[i] = addr; - debug_registers_changed = 1; - debug_registers_used = 1; + + for (windows_thread_info *th : thread_list) + th->debug_registers_changed = true; } /* Pass the value VAL to the inferior in the DR7 debug control @@ -3214,8 +3182,9 @@ static void cygwin_set_dr7 (unsigned long val) { dr[7] = (CORE_ADDR) val; - debug_registers_changed = 1; - debug_registers_used = 1; + + for (windows_thread_info *th : thread_list) + th->debug_registers_changed = true; } /* Get the value of debug register I from the inferior. */ -- 2.30.2