[binutils-gdb] Windows gdb: Eliminate global current_process.dr[8] global

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8de9be5a80d1ff5920e5c515571668b7cfb78ea3

commit 8de9be5a80d1ff5920e5c515571668b7cfb78ea3
Author: Pedro Alves <[email protected]>
Date:   Tue May 2 20:42:35 2023 +0100

    Windows gdb: Eliminate global current_process.dr[8] global
    
    current_process.dr needs to be per-thread for non-stop.  Actually, it
    doesn't even need to exist at all.  We have 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.
    
    Tromey pointed out that gdb/2388 mentioned in the code being removed
    was moved to https://sourceware.org/bugzilla/show_bug.cgi?id=9493 in
    the bugzilla migration.  I tried the reproducer mentioned there, and
    it still works correctly.
    
    Approved-By: Tom Tromey <[email protected]>
    Change-Id: Id762d0faa7d5e788402f2ff5adad5352447a7526
    commit-id:8a975ed0

Diff:
---
 gdb/nat/windows-nat.h  |  1 +
 gdb/windows-nat.c      |  1 +
 gdb/x86-windows-nat.c  | 73 +++++++++++++++++++++++++++-----------------------
 gdbserver/win32-low.cc |  1 +
 4 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 5411c73ff97..f4b3669df1b 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -150,6 +150,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 507a25199c8..e25ae81f054 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1235,6 +1235,7 @@ 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->process_id = pid;
   memset (&windows_process->current_event, 0,
 	  sizeof (windows_process->current_event));
   inf = current_inferior ();
diff --git a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c
index f8830809694..c7be192a894 100644
--- a/gdb/x86-windows-nat.c
+++ b/gdb/x86-windows-nat.c
@@ -44,8 +44,6 @@ enum
 
 struct x86_windows_per_inferior : public windows_per_inferior
 {
-  uintptr_t dr[8] {};
-
   /* The function to use in order to determine whether a register is
      a segment register or not.  */
   segment_register_p_ftype *segment_register_p = nullptr;
@@ -77,8 +75,6 @@ static x86_windows_per_inferior x86_windows_process;
 void
 x86_windows_nat_target::initialize_windows_arch (bool attaching)
 {
-  memset (x86_windows_process.dr, 0, sizeof (x86_windows_process.dr));
-
 #ifdef __x86_64__
   x86_windows_process.ignore_first_breakpoint
     = !attaching && x86_windows_process.wow64_process;
@@ -113,19 +109,6 @@ x86_windows_nat_target::fill_thread_context (windows_thread_info *th)
     {
       context->ContextFlags = WindowsContext<decltype(context)>::all;
       CHECK (get_thread_context (th->h, 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)
-	{
-	  x86_windows_process.dr[0] = context->Dr0;
-	  x86_windows_process.dr[1] = context->Dr1;
-	  x86_windows_process.dr[2] = context->Dr2;
-	  x86_windows_process.dr[3] = context->Dr3;
-	  x86_windows_process.dr[6] = context->Dr6;
-	  x86_windows_process.dr[7] = context->Dr7;
-	}
     });
 }
 
@@ -137,15 +120,18 @@ x86_windows_nat_target::thread_context_continue (windows_thread_info *th,
 {
   x86_windows_process.with_context (th, [&] (auto *context)
     {
+      struct x86_debug_reg_state *state
+	= x86_debug_reg_state (windows_process->process_id);
+
       if (th->debug_registers_changed)
 	{
 	  context->ContextFlags |= WindowsContext<decltype(context)>::debug;
-	  context->Dr0 = x86_windows_process.dr[0];
-	  context->Dr1 = x86_windows_process.dr[1];
-	  context->Dr2 = x86_windows_process.dr[2];
-	  context->Dr3 = x86_windows_process.dr[3];
+	  context->Dr0 = state->dr_mirror[0];
+	  context->Dr1 = state->dr_mirror[1];
+	  context->Dr2 = state->dr_mirror[2];
+	  context->Dr3 = state->dr_mirror[3];
 	  context->Dr6 = DR6_CLEAR_VALUE;
-	  context->Dr7 = x86_windows_process.dr[7];
+	  context->Dr7 = state->dr_control_mirror;
 	  th->debug_registers_changed = false;
 	}
 
@@ -301,7 +287,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);
-  x86_windows_process.dr[i] = addr;
 
   for (auto &th : x86_windows_process.thread_list)
     th->debug_registers_changed = true;
@@ -313,8 +298,6 @@ cygwin_set_dr (int i, CORE_ADDR addr)
 static void
 cygwin_set_dr7 (unsigned long val)
 {
-  x86_windows_process.dr[7] = (CORE_ADDR) val;
-
   for (auto &th : x86_windows_process.thread_list)
     th->debug_registers_changed = true;
 }
@@ -324,26 +307,48 @@ cygwin_set_dr7 (unsigned long val)
 static CORE_ADDR
 cygwin_get_dr (int i)
 {
-  return x86_windows_process.dr[i];
+  windows_thread_info *th
+    = windows_process->thread_rec (inferior_ptid, DONT_INVALIDATE_CONTEXT);
+
+  return windows_process->with_context (th, [&] (auto *context) -> CORE_ADDR
+    {
+      gdb_assert (context->ContextFlags != 0);
+      switch (i)
+	{
+	case 0:
+	  return context->Dr0;
+	case 1:
+	  return context->Dr1;
+	case 2:
+	  return context->Dr2;
+	case 3:
+	  return context->Dr3;
+	case 6:
+	  return context->Dr6;
+	case 7:
+	  return 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) x86_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) x86_windows_process.dr[7];
+  return cygwin_get_dr (7);
 }
 
 static int
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index b05f6af6b56..ddc5e5475ec 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -291,6 +291,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;