[PATCH v3] Move x86 debug registers and related code into x86-windows-nat.c

[Hannes Domani <ssbssa@yahoo.de>]

---
Changes in v2:
  - changed new windows_nat_target functions from public to protected
  - better comments for these functions
Changes in v3:
  - made check() function non-static, removed duplicate in x86-windows-nat.c
---
 gdb/windows-nat.c     | 151 ++---------------------------------
 gdb/windows-nat.h     |  24 +++++-
 gdb/x86-windows-nat.c | 178 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 208 insertions(+), 145 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 9bcbc740f86..88c821d885d 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -62,7 +62,6 @@
 
 #include "windows-tdep.h"
 #include "windows-nat.h"
-#include "x86-nat.h"
 #include "complaints.h"
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/pathstuff.h"
@@ -85,25 +84,12 @@ windows_per_inferior *windows_process;
 #   define STARTUPINFO STARTUPINFOW
 #endif
 
-/* If we're not using the old Cygwin header file set, define the
-   following which never should have been in the generic Win32 API
-   headers in the first place since they were our own invention...  */
-#ifndef _GNU_H_WINDOWS_H
-enum
-  {
-    FLAG_TRACE_BIT = 0x100,
-  };
-#endif
-
-#define DR6_CLEAR_VALUE 0xffff0ff0
-
 /* The string sent by cygwin when it processes a signal.
    FIXME: This should be in a cygwin include file.  */
 #ifndef _CYGWIN_SIGNAL_STRING
 #define _CYGWIN_SIGNAL_STRING "cYgSiGw00f"
 #endif
 
-#define CHECK(x)	check (x, __FILE__,__LINE__)
 #define DEBUG_EXEC(fmt, ...) \
   debug_prefixed_printf_cond (debug_exec, "windows exec", fmt, ## __VA_ARGS__)
 #define DEBUG_EVENTS(fmt, ...) \
@@ -116,12 +102,6 @@ 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);
-
 /* User options.  */
 static bool new_console = false;
 #ifdef __CYGWIN__
@@ -158,7 +138,7 @@ static const struct xlate_exception xlate[] =
 
 #endif /* 0 */
 
-static void
+void
 check (BOOL ok, const char *file, int line)
 {
   if (!ok)
@@ -494,23 +474,7 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r)
 
   if (th->reload_context)
     {
-      windows_process->with_context (th, [&] (auto *context)
-	{
-	  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)
-	    {
-	      windows_process->dr[0] = context->Dr0;
-	      windows_process->dr[1] = context->Dr1;
-	      windows_process->dr[2] = context->Dr2;
-	      windows_process->dr[3] = context->Dr3;
-	      windows_process->dr[6] = context->Dr6;
-	      windows_process->dr[7] = context->Dr7;
-	    }
-	});
+      fill_thread_context (th);
 
       th->reload_context = false;
     }
@@ -1009,35 +973,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)
       {
-	windows_process->with_context (th.get (), [&] (auto *context)
-	  {
-	    if (th->debug_registers_changed)
-	      {
-		context->ContextFlags
-		  |= WindowsContext<decltype(context)>::debug;
-		context->Dr0 = windows_process->dr[0];
-		context->Dr1 = windows_process->dr[1];
-		context->Dr2 = windows_process->dr[2];
-		context->Dr3 = windows_process->dr[3];
-		context->Dr6 = DR6_CLEAR_VALUE;
-		context->Dr7 = windows_process->dr[7];
-		th->debug_registers_changed = false;
-	      }
-	    if (context->ContextFlags)
-	      {
-		DWORD ec = 0;
-
-		if (GetExitCodeThread (th->h, &ec)
-		    && ec == STILL_ACTIVE)
-		  {
-		    BOOL status = set_thread_context (th->h, context);
-
-		    if (!killed)
-		      CHECK (status);
-		  }
-		context->ContextFlags = 0;
-	      }
-	  });
+	thread_context_continue (th.get (), killed);
 
 	th->resume ();
       }
@@ -1153,10 +1089,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
       regcache *regcache = get_thread_regcache (inferior_thread ());
       struct gdbarch *gdbarch = regcache->arch ();
       fetch_registers (regcache, gdbarch_ps_regnum (gdbarch));
-      windows_process->with_context (th, [&] (auto *context)
-	{
-	  context->EFlags |= FLAG_TRACE_BIT;
-	});
+      thread_context_step (th);
     }
 
   /* Allow continuing with the same signal that interrupted us.
@@ -1536,15 +1469,12 @@ 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;
 
+  initialize_windows_arch ();
+
   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;
@@ -1890,7 +1820,7 @@ windows_nat_target::detach (inferior *inf, int from_tty)
 
   target_announce_detach (from_tty);
 
-  x86_cleanup_dregs ();
+  cleanup_windows_arch ();
   switch_to_no_thread ();
   detach_inferior (inf);
 
@@ -2622,7 +2552,7 @@ void
 windows_nat_target::mourn_inferior ()
 {
   (void) windows_continue (DBG_CONTINUE, -1, 0, true);
-  x86_cleanup_dregs();
+  cleanup_windows_arch ();
   if (windows_process->open_process_used)
     {
       CHECK (CloseHandle (windows_process->handle));
@@ -2857,16 +2787,6 @@ windows_nat_target::thread_name (struct thread_info *thr)
 
 INIT_GDB_FILE (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.debug_register_length field is set by
-     calling x86_set_debug_register_length function
-     in processor windows specific native file.  */
-
 #ifdef __CYGWIN__
   cygwin_internal (CW_SET_DOS_FILE_WARNING, 0);
 #endif
@@ -2956,61 +2876,6 @@ Use \"%ps\" or \"%ps\" command to load executable/libraries directly."),
     }
 }
 
-/* Hardware watchpoint support, adapted from go32-nat.c code.  */
-
-/* Pass the address ADDR to the inferior in the I'th debug register.
-   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)
-{
-  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;
-}
-
-/* Pass the value VAL to the inferior in the DR7 debug control
-   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_process->dr[7] = (CORE_ADDR) val;
-
-  for (auto &th : windows_process->thread_list)
-    th->debug_registers_changed = true;
-}
-
-/* Get the value of debug register I from the inferior.  */
-
-static CORE_ADDR
-cygwin_get_dr (int i)
-{
-  return windows_process->dr[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.  */
-static unsigned long
-cygwin_get_dr6 (void)
-{
-  return (unsigned long) windows_process->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.  */
-
-static unsigned long
-cygwin_get_dr7 (void)
-{
-  return (unsigned long) windows_process->dr[7];
-}
-
 /* Determine if the thread referenced by "ptid" is alive
    by "polling" it.  If WaitForSingleObject returns WAIT_OBJECT_0
    it means that the thread has died.  Otherwise it is assumed to be alive.  */
diff --git a/gdb/windows-nat.h b/gdb/windows-nat.h
index 03c358fac54..f67ca3ad193 100644
--- a/gdb/windows-nat.h
+++ b/gdb/windows-nat.h
@@ -54,8 +54,6 @@ struct windows_per_inferior : public windows_nat::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;
@@ -183,6 +181,23 @@ struct windows_nat_target : public inf_child_target
     return serial_event_fd (m_wait_event);
   }
 
+protected:
+
+  /* Initialize arch-specific data for a new inferior (debug registers,
+     register mappings).  */
+  virtual void initialize_windows_arch () = 0;
+  /* Cleanup arch-specific data after inferior exit.  */
+  virtual void cleanup_windows_arch () = 0;
+
+  /* Reload the thread context.  */
+  virtual void fill_thread_context (windows_thread_info *th) = 0;
+
+  /* Prepare the thread context for continuing.  */
+  virtual void thread_context_continue (windows_thread_info *th,
+					int killed) = 0;
+  /* Set the stepping bit in the thread context.  */
+  virtual void thread_context_step (windows_thread_info *th) = 0;
+
 private:
 
   windows_thread_info *add_thread (ptid_t ptid, HANDLE h, void *tlb,
@@ -249,6 +264,11 @@ struct windows_nat_target : public inf_child_target
   bool m_continued = false;
 };
 
+/* Check if Windows API call succeeds, and otherwise print error code
+   and description.  */
+void check (BOOL ok, const char *file, int line);
+#define CHECK(x)	check (x, __FILE__,__LINE__)
+
 /* The current process.  */
 extern windows_per_inferior *windows_process;
 
diff --git a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c
index d1f78a01255..58a9d3d8866 100644
--- a/gdb/x86-windows-nat.c
+++ b/gdb/x86-windows-nat.c
@@ -18,22 +18,200 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "windows-nat.h"
+#include "regcache.h"
+#include "gdbarch.h"
 
 #include "x86-nat.h"
 
+using namespace windows_nat;
+
+/* If we're not using the old Cygwin header file set, define the
+   following which never should have been in the generic Win32 API
+   headers in the first place since they were our own invention...  */
+#ifndef _GNU_H_WINDOWS_H
+enum
+  {
+    FLAG_TRACE_BIT = 0x100,
+  };
+#endif
+
+#define DR6_CLEAR_VALUE 0xffff0ff0
+
 struct x86_windows_per_inferior : public windows_per_inferior
 {
+  uintptr_t dr[8] {};
 };
 
 struct x86_windows_nat_target final : public x86_nat_target<windows_nat_target>
 {
+  void initialize_windows_arch () override;
+  void cleanup_windows_arch () override;
+
+  void fill_thread_context (windows_thread_info *th) override;
+
+  void thread_context_continue (windows_thread_info *th, int killed) override;
+  void thread_context_step (windows_thread_info *th) override;
 };
 
 /* The current process.  */
 static x86_windows_per_inferior x86_windows_process;
 
+/* See windows-nat.h.  */
+
+void
+x86_windows_nat_target::initialize_windows_arch ()
+{
+  memset (x86_windows_process.dr, 0, sizeof (x86_windows_process.dr));
+}
+
+/* See windows-nat.h.  */
+
+void
+x86_windows_nat_target::cleanup_windows_arch ()
+{
+  x86_cleanup_dregs ();
+}
+
+/* See windows-nat.h.  */
+
+void
+x86_windows_nat_target::fill_thread_context (windows_thread_info *th)
+{
+  x86_windows_process.with_context (th, [&] (auto *context)
+    {
+      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;
+	}
+    });
+}
+
+/* See windows-nat.h.  */
+
+void
+x86_windows_nat_target::thread_context_continue (windows_thread_info *th,
+						 int killed)
+{
+  x86_windows_process.with_context (th, [&] (auto *context)
+    {
+      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->Dr6 = DR6_CLEAR_VALUE;
+	  context->Dr7 = x86_windows_process.dr[7];
+	  th->debug_registers_changed = false;
+	}
+
+      if (context->ContextFlags)
+	{
+	  DWORD ec = 0;
+
+	  if (GetExitCodeThread (th->h, &ec)
+	      && ec == STILL_ACTIVE)
+	    {
+	      BOOL status = set_thread_context (th->h, context);
+
+	      if (!killed)
+		CHECK (status);
+	    }
+	  context->ContextFlags = 0;
+	}
+    });
+}
+
+/* See windows-nat.h.  */
+
+void
+x86_windows_nat_target::thread_context_step (windows_thread_info *th)
+{
+  x86_windows_process.with_context (th, [&] (auto *context)
+    {
+      context->EFlags |= FLAG_TRACE_BIT;
+    });
+}
+
+/* Hardware watchpoint support, adapted from go32-nat.c code.  */
+
+/* Pass the address ADDR to the inferior in the I'th debug register.
+   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)
+{
+  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;
+}
+
+/* Pass the value VAL to the inferior in the DR7 debug control
+   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)
+{
+  x86_windows_process.dr[7] = (CORE_ADDR) val;
+
+  for (auto &th : x86_windows_process.thread_list)
+    th->debug_registers_changed = true;
+}
+
+/* Get the value of debug register I from the inferior.  */
+
+static CORE_ADDR
+cygwin_get_dr (int i)
+{
+  return x86_windows_process.dr[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.  */
+static unsigned long
+cygwin_get_dr6 (void)
+{
+  return (unsigned long) x86_windows_process.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.  */
+
+static unsigned long
+cygwin_get_dr7 (void)
+{
+  return (unsigned long) x86_windows_process.dr[7];
+}
+
 INIT_GDB_FILE (x86_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.debug_register_length field is set by
+     calling x86_set_debug_register_length function
+     in processor windows specific native file.  */
+
   /* The target is not a global specifically to avoid a C++ "static
      initializer fiasco" situation.  */
   add_inf_child_target (new x86_windows_nat_target);
-- 
2.52.0