[PATCH v2] Implement debugging of WOW64 processes in gdbserver

[PATCH v2] Implement debugging of WOW64 processes in gdbserver

[Hannes Domani]

gdbserver/ChangeLog:

2020-04-27  Hannes Domani  <ssbssa@yahoo.de>

	* configure.srv <x86_64-*-mingw*, x86_64-*-cygwin*> (srv_tgtobj):
	Add arch/i386.o.
	* win32-arm-low.cc (arm_num_regs): New function.
	(struct win32_target_ops): Use arm_num_regs.
	* win32-i386-low.cc (win32_get_current_dr): Adapt for WOW64
	processes.
	(i386_get_thread_context): Likewise.
	(i386_prepare_to_resume): Likewise.
	(i386_thread_added): Likewise.
	(i386_single_step): Likewise.
	(i386_fetch_inferior_register): Likewise.
	(i386_store_inferior_register): Likewise.
	(i386_arch_setup): Likewise.
	(i386_win32_num_regs): New function.
	(struct win32_target_ops): Use i386_win32_num_regs.
	* win32-low.cc (win32_get_thread_context): Adapt for WOW64
	processes.
	(win32_require_context): Likewise.
	(child_add_thread): Likewise.
	(do_initial_child_stuff): Likewise.
	(continue_one_thread): Likewise.
	(win32_process_target::resume): Likewise.
	(load_psapi): Likewise.
	(win32_add_all_dlls): Likewise.
	(maybe_adjust_pc): Likewise.
	(win32_process_target::qxfer_siginfo): Likewise.
	(initialize_low): Likewise.
	* win32-low.h (struct win32_target_ops): Change num_regs to
	callback function.
---
v2:
- Stop with error() if any of the WOW64 functions is not available when
  debugging WOW64 processes.
---
 gdbserver/configure.srv     |   4 +-
 gdbserver/win32-arm-low.cc  |  10 +-
 gdbserver/win32-i386-low.cc | 171 +++++++++++++++++++++-----
 gdbserver/win32-low.cc      | 236 ++++++++++++++++++++++++++++++++----
 gdbserver/win32-low.h       |  10 +-
 5 files changed, 369 insertions(+), 62 deletions(-)

diff --git a/gdbserver/configure.srv b/gdbserver/configure.srv
index 7acf229fbe..9a027e44af 100644
--- a/gdbserver/configure.srv
+++ b/gdbserver/configure.srv
@@ -397,14 +397,14 @@ case "${gdbserver_host}" in
 			srv_tgtobj="x86-low.o nat/x86-dregs.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
-			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
+			srv_tgtobj="${srv_tgtobj} arch/amd64.o arch/i386.o"
 			srv_mingw=yes
 			;;
   x86_64-*-cygwin*)	srv_regobj=""
 			srv_tgtobj="x86-low.o nat/x86-dregs.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
-			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
+			srv_tgtobj="${srv_tgtobj} arch/amd64.o arch/i386.o"
 			;;
 
   xtensa*-*-linux*)	srv_regobj=reg-xtensa.o
diff --git a/gdbserver/win32-arm-low.cc b/gdbserver/win32-arm-low.cc
index 238ee4b05b..aacf2cdf8c 100644
--- a/gdbserver/win32-arm-low.cc
+++ b/gdbserver/win32-arm-low.cc
@@ -111,6 +111,14 @@ arm_arch_setup (void)
   win32_tdesc = tdesc_arm;
 }
 
+/* Implement win32_target_ops "num_regs" method.  */
+
+static int
+arm_num_regs (void)
+{
+  return sizeof (mappings) / sizeof (mappings[0]),
+}
+
 /* Correct in either endianness.  We do not support Thumb yet.  */
 static const unsigned long arm_wince_breakpoint = 0xe6000010;
 #define arm_wince_breakpoint_len 4
@@ -138,7 +146,7 @@ arm_win32_set_pc (struct regcache *regcache, CORE_ADDR pc)
 
 struct win32_target_ops the_low_target = {
   arm_arch_setup,
-  sizeof (mappings) / sizeof (mappings[0]),
+  arm_num_regs,
   NULL, /* initial_stuff */
   arm_get_thread_context,
   NULL, /* prepare_to_resume */
diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
index 48893af33b..389ec49284 100644
--- a/gdbserver/win32-i386-low.cc
+++ b/gdbserver/win32-i386-low.cc
@@ -80,18 +80,40 @@ win32_get_current_dr (int dr)
 
   win32_require_context (th);
 
+#ifdef __x86_64__
+#define RET_DR(DR)				\
+  case DR:					\
+    return th->wow64_context.Dr ## DR
+
+  if (wow64_process)
+    {
+      switch (dr)
+	{
+	  RET_DR (0);
+	  RET_DR (1);
+	  RET_DR (2);
+	  RET_DR (3);
+	  RET_DR (6);
+	  RET_DR (7);
+	}
+    }
+  else
+#undef RET_DR
+#endif
 #define RET_DR(DR)				\
   case DR:					\
     return th->context.Dr ## DR
 
-  switch (dr)
     {
-      RET_DR (0);
-      RET_DR (1);
-      RET_DR (2);
-      RET_DR (3);
-      RET_DR (6);
-      RET_DR (7);
+      switch (dr)
+	{
+	  RET_DR (0);
+	  RET_DR (1);
+	  RET_DR (2);
+	  RET_DR (3);
+	  RET_DR (6);
+	  RET_DR (7);
+	}
     }
 
 #undef RET_DR
@@ -219,12 +241,27 @@ i386_get_thread_context (windows_thread_info *th)
   static DWORD extended_registers = CONTEXT_EXTENDED_REGISTERS;
 
  again:
-  th->context.ContextFlags = (CONTEXT_FULL
-			      | CONTEXT_FLOATING_POINT
-			      | CONTEXT_DEBUG_REGISTERS
-			      | extended_registers);
+#ifdef __x86_64__
+  if (wow64_process)
+    th->wow64_context.ContextFlags = (CONTEXT_FULL
+				      | CONTEXT_FLOATING_POINT
+				      | CONTEXT_DEBUG_REGISTERS
+				      | extended_registers);
+  else
+#endif
+    th->context.ContextFlags = (CONTEXT_FULL
+				| CONTEXT_FLOATING_POINT
+				| CONTEXT_DEBUG_REGISTERS
+				| extended_registers);
 
-  if (!GetThreadContext (th->h, &th->context))
+  BOOL ret;
+#ifdef __x86_64__
+  if (wow64_process)
+    ret = win32_Wow64GetThreadContext (th->h, &th->wow64_context);
+  else
+#endif
+    ret = GetThreadContext (th->h, &th->context);
+  if (!ret)
     {
       DWORD e = GetLastError ();
 
@@ -247,13 +284,28 @@ i386_prepare_to_resume (windows_thread_info *th)
 
       win32_require_context (th);
 
-      th->context.Dr0 = dr->dr_mirror[0];
-      th->context.Dr1 = dr->dr_mirror[1];
-      th->context.Dr2 = dr->dr_mirror[2];
-      th->context.Dr3 = dr->dr_mirror[3];
-      /* th->context.Dr6 = dr->dr_status_mirror;
-	 FIXME: should we set dr6 also ?? */
-      th->context.Dr7 = dr->dr_control_mirror;
+#ifdef __x86_64__
+      if (wow64_process)
+	{
+	  th->wow64_context.Dr0 = dr->dr_mirror[0];
+	  th->wow64_context.Dr1 = dr->dr_mirror[1];
+	  th->wow64_context.Dr2 = dr->dr_mirror[2];
+	  th->wow64_context.Dr3 = dr->dr_mirror[3];
+	  /* th->wow64_context.Dr6 = dr->dr_status_mirror;
+	     FIXME: should we set dr6 also ?? */
+	  th->wow64_context.Dr7 = dr->dr_control_mirror;
+	}
+      else
+#endif
+	{
+	  th->context.Dr0 = dr->dr_mirror[0];
+	  th->context.Dr1 = dr->dr_mirror[1];
+	  th->context.Dr2 = dr->dr_mirror[2];
+	  th->context.Dr3 = dr->dr_mirror[3];
+	  /* th->context.Dr6 = dr->dr_status_mirror;
+	     FIXME: should we set dr6 also ?? */
+	  th->context.Dr7 = dr->dr_control_mirror;
+	}
 
       th->debug_registers_changed = false;
     }
@@ -268,11 +320,14 @@ i386_thread_added (windows_thread_info *th)
 static void
 i386_single_step (windows_thread_info *th)
 {
-  th->context.EFlags |= FLAG_TRACE_BIT;
+#ifdef __x86_64__
+  if (wow64_process)
+    th->wow64_context.EFlags |= FLAG_TRACE_BIT;
+  else
+#endif
+    th->context.EFlags |= FLAG_TRACE_BIT;
 }
 
-#ifndef __x86_64__
-
 /* An array of offset mappings into a Win32 Context structure.
    This is a one-to-one mapping which is indexed by gdb's register
    numbers.  It retrieves an offset into the context structure where
@@ -280,8 +335,12 @@ i386_single_step (windows_thread_info *th)
    An offset value of -1 indicates that Win32 does not provide this
    register in it's CONTEXT structure.  In this case regptr will return
    a pointer into a dummy register.  */
+#ifdef __x86_64__
+#define context_offset(x) (offsetof (WOW64_CONTEXT, x))
+#else
 #define context_offset(x) ((int)&(((CONTEXT *)NULL)->x))
-static const int mappings[] = {
+#endif
+static const int i386_mappings[] = {
   context_offset (Eax),
   context_offset (Ecx),
   context_offset (Edx),
@@ -328,10 +387,10 @@ static const int mappings[] = {
 };
 #undef context_offset
 
-#else /* __x86_64__ */
+#ifdef __x86_64__
 
 #define context_offset(x) (offsetof (CONTEXT, x))
-static const int mappings[] =
+static const int amd64_mappings[] =
 {
   context_offset (Rax),
   context_offset (Rbx),
@@ -402,7 +461,21 @@ static void
 i386_fetch_inferior_register (struct regcache *regcache,
 			      windows_thread_info *th, int r)
 {
-  char *context_offset = (char *) &th->context + mappings[r];
+  const int *mappings;
+#ifdef __x86_64__
+  if (!wow64_process)
+    mappings = amd64_mappings;
+  else
+#endif
+    mappings = i386_mappings;
+
+  char *context_offset;
+#ifdef __x86_64__
+  if (wow64_process)
+    context_offset = (char *) &th->wow64_context + mappings[r];
+  else
+#endif
+    context_offset = (char *) &th->context + mappings[r];
 
   long l;
   if (r == FCS_REGNUM)
@@ -424,7 +497,22 @@ static void
 i386_store_inferior_register (struct regcache *regcache,
 			      windows_thread_info *th, int r)
 {
-  char *context_offset = (char *) &th->context + mappings[r];
+  const int *mappings;
+#ifdef __x86_64__
+  if (!wow64_process)
+    mappings = amd64_mappings;
+  else
+#endif
+    mappings = i386_mappings;
+
+  char *context_offset;
+#ifdef __x86_64__
+  if (wow64_process)
+    context_offset = (char *) &th->wow64_context + mappings[r];
+  else
+#endif
+    context_offset = (char *) &th->context + mappings[r];
+
   collect_register (regcache, r, context_offset);
 }
 
@@ -439,15 +527,32 @@ i386_arch_setup (void)
 #ifdef __x86_64__
   tdesc = amd64_create_target_description (X86_XSTATE_SSE_MASK, false,
 					   false, false);
-  const char **expedite_regs = amd64_expedite_regs;
-#else
+  init_target_desc (tdesc, amd64_expedite_regs);
+  win32_tdesc = tdesc;
+#endif
+
   tdesc = i386_create_target_description (X86_XSTATE_SSE_MASK, false, false);
-  const char **expedite_regs = i386_expedite_regs;
+  init_target_desc (tdesc, i386_expedite_regs);
+#ifdef __x86_64__
+  wow64_win32_tdesc = tdesc;
+#else
+  win32_tdesc = tdesc;
 #endif
+}
 
-  init_target_desc (tdesc, expedite_regs);
+/* Implement win32_target_ops "num_regs" method.  */
 
-  win32_tdesc = tdesc;
+static int
+i386_win32_num_regs (void)
+{
+  int num_regs;
+#ifdef __x86_64__
+  if (!wow64_process)
+    num_regs = sizeof (amd64_mappings) / sizeof (amd64_mappings[0]);
+  else
+#endif
+    num_regs = sizeof (i386_mappings) / sizeof (i386_mappings[0]);
+  return num_regs;
 }
 
 /* Implement win32_target_ops "get_pc" method.  */
@@ -496,7 +601,7 @@ i386_win32_set_pc (struct regcache *regcache, CORE_ADDR pc)
 
 struct win32_target_ops the_low_target = {
   i386_arch_setup,
-  sizeof (mappings) / sizeof (mappings[0]),
+  i386_win32_num_regs,
   i386_initial_stuff,
   i386_get_thread_context,
   i386_prepare_to_resume,
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 5a6f0df39f..d3fc31914d 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -88,15 +88,32 @@ static int soft_interrupt_requested = 0;
    by suspending all the threads.  */
 static int faked_breakpoint = 0;
 
+#ifdef __x86_64__
+bool wow64_process = false;
+#endif
+
 const struct target_desc *win32_tdesc;
+#ifdef __x86_64__
+const struct target_desc *wow64_win32_tdesc;
+#endif
 
-#define NUM_REGS (the_low_target.num_regs)
+#define NUM_REGS (the_low_target.num_regs ())
 
 typedef BOOL (WINAPI *winapi_DebugActiveProcessStop) (DWORD dwProcessId);
 typedef BOOL (WINAPI *winapi_DebugSetProcessKillOnExit) (BOOL KillOnExit);
 typedef BOOL (WINAPI *winapi_DebugBreakProcess) (HANDLE);
 typedef BOOL (WINAPI *winapi_GenerateConsoleCtrlEvent) (DWORD, DWORD);
 
+#ifdef __x86_64__
+typedef DWORD (WINAPI *winapi_Wow64SuspendThread) (HANDLE);
+typedef BOOL (WINAPI *winapi_Wow64SetThreadContext) (HANDLE,
+						     const WOW64_CONTEXT *);
+
+static winapi_Wow64SuspendThread win32_Wow64SuspendThread;
+winapi_Wow64GetThreadContext win32_Wow64GetThreadContext;
+static winapi_Wow64SetThreadContext win32_Wow64SetThreadContext;
+#endif
+
 #ifndef _WIN32_WCE
 static void win32_add_all_dlls (void);
 #endif
@@ -121,7 +138,12 @@ debug_event_ptid (DEBUG_EVENT *event)
 static void
 win32_get_thread_context (windows_thread_info *th)
 {
-  memset (&th->context, 0, sizeof (CONTEXT));
+#ifdef __x86_64__
+  if (wow64_process)
+    memset (&th->wow64_context, 0, sizeof (WOW64_CONTEXT));
+  else
+#endif
+    memset (&th->context, 0, sizeof (CONTEXT));
   (*the_low_target.get_thread_context) (th);
 #ifdef _WIN32_WCE
   memcpy (&th->base_context, &th->context, sizeof (CONTEXT));
@@ -146,7 +168,14 @@ win32_set_thread_context (windows_thread_info *th)
      it between stopping and resuming.  */
   if (memcmp (&th->context, &th->base_context, sizeof (CONTEXT)) != 0)
 #endif
-    SetThreadContext (th->h, &th->context);
+    {
+#ifdef __x86_64__
+      if (wow64_process)
+	win32_Wow64SetThreadContext (th->h, &th->wow64_context);
+      else
+#endif
+	SetThreadContext (th->h, &th->context);
+    }
 }
 
 /* Set the thread context of the thread associated with TH.  */
@@ -163,7 +192,14 @@ win32_prepare_to_resume (windows_thread_info *th)
 void
 win32_require_context (windows_thread_info *th)
 {
-  if (th->context.ContextFlags == 0)
+  DWORD context_flags;
+#ifdef __x86_64__
+  if (wow64_process)
+    context_flags = th->wow64_context.ContextFlags;
+  else
+#endif
+    context_flags = th->context.ContextFlags;
+  if (context_flags == 0)
     {
       th->suspend ();
       win32_get_thread_context (th);
@@ -195,7 +231,14 @@ child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
   if ((th = thread_rec (ptid, DONT_INVALIDATE_CONTEXT)))
     return th;
 
-  th = new windows_thread_info (tid, h, (CORE_ADDR) (uintptr_t) tlb);
+  CORE_ADDR base = (CORE_ADDR) (uintptr_t) tlb;
+#ifdef __x86_64__
+  /* For WOW64 processes, this is actually the pointer to the 64bit TIB,
+     and the 32bit TIB is exactly 2 pages after it.  */
+  if (wow64_process)
+    base += 0x2000;
+#endif
+  th = new windows_thread_info (tid, h, base);
 
   add_thread (ptid, th);
 
@@ -345,8 +388,27 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
 
   memset (&current_event, 0, sizeof (current_event));
 
+#ifdef __x86_64__
+  BOOL wow64;
+  if (IsWow64Process (proch, &wow64))
+    wow64_process = wow64;
+
+  if (wow64_process
+      && (win32_Wow64SuspendThread == nullptr
+	  || win32_Wow64GetThreadContext == nullptr
+	  || win32_Wow64SetThreadContext == nullptr))
+    error ("WOW64 debugging is not supported on this system.\n");
+
+  ignore_first_breakpoint = !attached && wow64_process;
+#endif
+
   proc = add_process (pid, attached);
-  proc->tdesc = win32_tdesc;
+#ifdef __x86_64__
+  if (wow64_process)
+    proc->tdesc = wow64_win32_tdesc;
+  else
+#endif
+    proc->tdesc = win32_tdesc;
   child_init_thread_list ();
   child_initialization_done = 0;
 
@@ -416,10 +478,17 @@ continue_one_thread (thread_info *thread, int thread_id)
 
       if (th->suspended)
 	{
-	  if (th->context.ContextFlags)
+	  DWORD *context_flags;
+#ifdef __x86_64__
+	  if (wow64_process)
+	    context_flags = &th->wow64_context.ContextFlags;
+	  else
+#endif
+	    context_flags = &th->context.ContextFlags;
+	  if (*context_flags)
 	    {
 	      win32_set_thread_context (th);
-	      th->context.ContextFlags = 0;
+	      *context_flags = 0;
 	    }
 
 	  th->resume ();
@@ -943,7 +1012,14 @@ win32_process_target::resume (thread_resume *resume_info, size_t n)
     {
       win32_prepare_to_resume (th);
 
-      if (th->context.ContextFlags)
+      DWORD *context_flags;
+#ifdef __x86_64__
+      if (wow64_process)
+	context_flags = &th->wow64_context.ContextFlags;
+      else
+#endif
+	context_flags = &th->context.ContextFlags;
+      if (*context_flags)
 	{
 	  /* Move register values from the inferior into the thread
 	     context structure.  */
@@ -959,7 +1035,7 @@ win32_process_target::resume (thread_resume *resume_info, size_t n)
 	    }
 
 	  win32_set_thread_context (th);
-	  th->context.ContextFlags = 0;
+	  *context_flags = 0;
 	}
     }
 
@@ -1032,12 +1108,19 @@ win32_add_one_solib (const char *name, CORE_ADDR load_addr)
 
 typedef BOOL (WINAPI *winapi_EnumProcessModules) (HANDLE, HMODULE *,
 						  DWORD, LPDWORD);
+#ifdef __x86_64__
+typedef BOOL (WINAPI *winapi_EnumProcessModulesEx) (HANDLE, HMODULE *, DWORD,
+						    LPDWORD, DWORD);
+#endif
 typedef BOOL (WINAPI *winapi_GetModuleInformation) (HANDLE, HMODULE,
 						    LPMODULEINFO, DWORD);
 typedef DWORD (WINAPI *winapi_GetModuleFileNameExA) (HANDLE, HMODULE,
 						     LPSTR, DWORD);
 
 static winapi_EnumProcessModules win32_EnumProcessModules;
+#ifdef __x86_64__
+static winapi_EnumProcessModulesEx win32_EnumProcessModulesEx;
+#endif
 static winapi_GetModuleInformation win32_GetModuleInformation;
 static winapi_GetModuleFileNameExA win32_GetModuleFileNameExA;
 
@@ -1055,12 +1138,21 @@ load_psapi (void)
 	return FALSE;
       win32_EnumProcessModules =
 	      GETPROCADDRESS (dll, EnumProcessModules);
+#ifdef __x86_64__
+      win32_EnumProcessModulesEx =
+	      GETPROCADDRESS (dll, EnumProcessModulesEx);
+#endif
       win32_GetModuleInformation =
 	      GETPROCADDRESS (dll, GetModuleInformation);
       win32_GetModuleFileNameExA =
 	      GETPROCADDRESS (dll, GetModuleFileNameExA);
     }
 
+#ifdef __x86_64__
+  if (wow64_process && win32_EnumProcessModulesEx == nullptr)
+    return FALSE;
+#endif
+
   return (win32_EnumProcessModules != NULL
 	  && win32_GetModuleInformation != NULL
 	  && win32_GetModuleFileNameExA != NULL);
@@ -1084,10 +1176,19 @@ win32_add_all_dlls (void)
     return;
 
   cbNeeded = 0;
-  ok = (*win32_EnumProcessModules) (current_process_handle,
-				    DllHandle,
-				    sizeof (HMODULE),
-				    &cbNeeded);
+#ifdef __x86_64__
+  if (wow64_process)
+    ok = (*win32_EnumProcessModulesEx) (current_process_handle,
+					DllHandle,
+					sizeof (HMODULE),
+					&cbNeeded,
+					LIST_MODULES_32BIT);
+  else
+#endif
+    ok = (*win32_EnumProcessModules) (current_process_handle,
+				      DllHandle,
+				      sizeof (HMODULE),
+				      &cbNeeded);
 
   if (!ok || !cbNeeded)
     return;
@@ -1096,13 +1197,53 @@ win32_add_all_dlls (void)
   if (!DllHandle)
     return;
 
-  ok = (*win32_EnumProcessModules) (current_process_handle,
-				    DllHandle,
-				    cbNeeded,
-				    &cbNeeded);
+#ifdef __x86_64__
+  if (wow64_process)
+    ok = (*win32_EnumProcessModulesEx) (current_process_handle,
+					DllHandle,
+					cbNeeded,
+					&cbNeeded,
+					LIST_MODULES_32BIT);
+  else
+#endif
+    ok = (*win32_EnumProcessModules) (current_process_handle,
+				      DllHandle,
+				      cbNeeded,
+				      &cbNeeded);
   if (!ok)
     return;
 
+  char system_dir[MAX_PATH];
+  char syswow_dir[MAX_PATH];
+  size_t system_dir_len = 0;
+  bool convert_syswow_dir = false;
+#ifdef __x86_64__
+  if (wow64_process)
+#endif
+    {
+      /* This fails on 32bit Windows because it has no SysWOW64 directory,
+	 and in this case a path conversion isn't necessary.  */
+      UINT len = GetSystemWow64DirectoryA (syswow_dir, sizeof (syswow_dir));
+      if (len > 0)
+	{
+	  /* Check that we have passed a large enough buffer.  */
+	  gdb_assert (len < sizeof (syswow_dir));
+
+	  len = GetSystemDirectoryA (system_dir, sizeof (system_dir));
+	  /* Error check.  */
+	  gdb_assert (len != 0);
+	  /* Check that we have passed a large enough buffer.  */
+	  gdb_assert (len < sizeof (system_dir));
+
+	  strcat (system_dir, "\\");
+	  strcat (syswow_dir, "\\");
+	  system_dir_len = strlen (system_dir);
+
+	  convert_syswow_dir = true;
+	}
+
+    }
+
   for (i = 1; i < ((size_t) cbNeeded / sizeof (HMODULE)); i++)
     {
       MODULEINFO mi;
@@ -1118,7 +1259,22 @@ win32_add_all_dlls (void)
 					 dll_name,
 					 MAX_PATH) == 0)
 	continue;
-      win32_add_one_solib (dll_name, (CORE_ADDR) (uintptr_t) mi.lpBaseOfDll);
+
+      const char *name = dll_name;
+      /* Convert the DLL path of 32bit processes returned by
+	 GetModuleFileNameEx from the 64bit system directory to the
+	 32bit syswow64 directory if necessary.  */
+      std::string syswow_dll_path;
+      if (convert_syswow_dir
+	  && strncasecmp (dll_name, system_dir, system_dir_len) == 0
+	  && strchr (dll_name + system_dir_len, '\\') == nullptr)
+	{
+	  syswow_dll_path = syswow_dir;
+	  syswow_dll_path += dll_name + system_dir_len;
+	  name = syswow_dll_path.c_str();
+	}
+
+      win32_add_one_solib (name, (CORE_ADDR) (uintptr_t) mi.lpBaseOfDll);
     }
 }
 #endif
@@ -1221,8 +1377,10 @@ maybe_adjust_pc ()
   th->stopped_at_software_breakpoint = false;
 
   if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
-      && (current_event.u.Exception.ExceptionRecord.ExceptionCode
-	  == EXCEPTION_BREAKPOINT)
+      && ((current_event.u.Exception.ExceptionRecord.ExceptionCode
+	   == EXCEPTION_BREAKPOINT)
+	  || (current_event.u.Exception.ExceptionRecord.ExceptionCode
+	      == STATUS_WX86_BREAKPOINT))
       && child_initialization_done)
     {
       th->stopped_at_software_breakpoint = true;
@@ -1684,13 +1842,34 @@ win32_process_target::qxfer_siginfo (const char *annex,
   if (readbuf == nullptr)
     return -1;
 
-  if (offset > sizeof (siginfo_er))
+  char *buf = (char *) &siginfo_er;
+  size_t bufsize = sizeof (siginfo_er);
+
+#ifdef __x86_64__
+  EXCEPTION_RECORD32 er32;
+  if (wow64_process)
+    {
+      buf = (char *) &er32;
+      bufsize = sizeof (er32);
+
+      er32.ExceptionCode = siginfo_er.ExceptionCode;
+      er32.ExceptionFlags = siginfo_er.ExceptionFlags;
+      er32.ExceptionRecord = (uintptr_t) siginfo_er.ExceptionRecord;
+      er32.ExceptionAddress = (uintptr_t) siginfo_er.ExceptionAddress;
+      er32.NumberParameters = siginfo_er.NumberParameters;
+      int i;
+      for (i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; i++)
+	er32.ExceptionInformation[i] = siginfo_er.ExceptionInformation[i];
+    }
+#endif
+
+  if (offset > bufsize)
     return -1;
 
-  if (offset + len > sizeof (siginfo_er))
-    len = sizeof (siginfo_er) - offset;
+  if (offset + len > bufsize)
+    len = bufsize - offset;
 
-  memcpy (readbuf, (char *) &siginfo_er + offset, len);
+  memcpy (readbuf, buf + offset, len);
 
   return len;
 }
@@ -1760,4 +1939,11 @@ initialize_low (void)
 {
   set_target_ops (&the_win32_target);
   the_low_target.arch_setup ();
+
+#ifdef __x86_64__
+  HMODULE dll = GetModuleHandle (_T("KERNEL32.DLL"));
+  win32_Wow64SuspendThread = GETPROCADDRESS (dll, Wow64SuspendThread);
+  win32_Wow64GetThreadContext = GETPROCADDRESS (dll, Wow64GetThreadContext);
+  win32_Wow64SetThreadContext = GETPROCADDRESS (dll, Wow64SetThreadContext);
+#endif
 }
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index b3fa392dd3..a023eb1f83 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -27,6 +27,14 @@ struct target_desc;
 /* The inferior's target description.  This is a global because the
    Windows ports support neither bi-arch nor multi-process.  */
 extern const struct target_desc *win32_tdesc;
+#ifdef __x86_64__
+extern const struct target_desc *wow64_win32_tdesc;
+
+extern bool wow64_process;
+
+typedef BOOL (WINAPI *winapi_Wow64GetThreadContext) (HANDLE, PWOW64_CONTEXT);
+extern winapi_Wow64GetThreadContext win32_Wow64GetThreadContext;
+#endif
 
 struct win32_target_ops
 {
@@ -34,7 +42,7 @@ struct win32_target_ops
   void (*arch_setup) (void);
 
   /* The number of target registers.  */
-  int num_regs;
+  int (*num_regs) (void);
 
   /* Perform initializations on startup.  */
   void (*initial_stuff) (void);
-- 
2.26.2

Re: [PATCH v2] Implement debugging of WOW64 processes in gdbserver

[Simon Marchi]

On 2020-04-27 9:34 a.m., Hannes Domani via Gdb-patches wrote:
> @@ -496,7 +601,7 @@ i386_win32_set_pc (struct regcache *regcache, CORE_ADDR pc)
>  
>  struct win32_target_ops the_low_target = {
>    i386_arch_setup,
> -  sizeof (mappings) / sizeof (mappings[0]),
> +  i386_win32_num_regs,
>    i386_initial_stuff,
>    i386_get_thread_context,
>    i386_prepare_to_resume,
> diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
> index 5a6f0df39f..d3fc31914d 100644
> --- a/gdbserver/win32-low.cc
> +++ b/gdbserver/win32-low.cc
> @@ -88,15 +88,32 @@ static int soft_interrupt_requested = 0;
>     by suspending all the threads.  */
>  static int faked_breakpoint = 0;
>  
> +#ifdef __x86_64__
> +bool wow64_process = false;
> +#endif
> +
>  const struct target_desc *win32_tdesc;
> +#ifdef __x86_64__
> +const struct target_desc *wow64_win32_tdesc;
> +#endif
>  
> -#define NUM_REGS (the_low_target.num_regs)
> +#define NUM_REGS (the_low_target.num_regs ())
>  
>  typedef BOOL (WINAPI *winapi_DebugActiveProcessStop) (DWORD dwProcessId);
>  typedef BOOL (WINAPI *winapi_DebugSetProcessKillOnExit) (BOOL KillOnExit);
>  typedef BOOL (WINAPI *winapi_DebugBreakProcess) (HANDLE);
>  typedef BOOL (WINAPI *winapi_GenerateConsoleCtrlEvent) (DWORD, DWORD);
>  
> +#ifdef __x86_64__
> +typedef DWORD (WINAPI *winapi_Wow64SuspendThread) (HANDLE);
> +typedef BOOL (WINAPI *winapi_Wow64SetThreadContext) (HANDLE,
> +						     const WOW64_CONTEXT *);
> +
> +static winapi_Wow64SuspendThread win32_Wow64SuspendThread;

win32_Wow64SuspendThread is never actually used, is this expected?

> @@ -195,7 +231,14 @@ child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
>    if ((th = thread_rec (ptid, DONT_INVALIDATE_CONTEXT)))
>      return th;
>  
> -  th = new windows_thread_info (tid, h, (CORE_ADDR) (uintptr_t) tlb);
> +  CORE_ADDR base = (CORE_ADDR) (uintptr_t) tlb;
> +#ifdef __x86_64__
> +  /* For WOW64 processes, this is actually the pointer to the 64bit TIB,
> +     and the 32bit TIB is exactly 2 pages after it.  */

Do you have a reference for this, ideally from MSDN?  If so, it would be nice to
include the link in the comment.

> +  if (wow64_process)
> +    base += 0x2000;

Could you make it a PAGE_SIZE macro, and wirte `2 * PAGE_SIZE` here?  It would
make it more obvious that it matches the comment above.

> +#endif
> +  th = new windows_thread_info (tid, h, base);
>  
>    add_thread (ptid, th);
>  
> @@ -345,8 +388,27 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
>  
>    memset (&current_event, 0, sizeof (current_event));
>  
> +#ifdef __x86_64__
> +  BOOL wow64;
> +  if (IsWow64Process (proch, &wow64))
> +    wow64_process = wow64;

If this function fails, it means we couldn't get the information we requested.  In
this case, I think we should error out, because we can't continue reliably.  So,
something like:

BOOL wow64;
if (!IsWow64Process (proch, &wow64_process))
  {
    // Call error() with a message formatted from GetLastError().
  }

> @@ -1096,13 +1197,53 @@ win32_add_all_dlls (void)
>    if (!DllHandle)
>      return;
>  
> -  ok = (*win32_EnumProcessModules) (current_process_handle,
> -				    DllHandle,
> -				    cbNeeded,
> -				    &cbNeeded);
> +#ifdef __x86_64__
> +  if (wow64_process)
> +    ok = (*win32_EnumProcessModulesEx) (current_process_handle,
> +					DllHandle,
> +					cbNeeded,
> +					&cbNeeded,
> +					LIST_MODULES_32BIT);
> +  else
> +#endif
> +    ok = (*win32_EnumProcessModules) (current_process_handle,
> +				      DllHandle,
> +				      cbNeeded,
> +				      &cbNeeded);
>    if (!ok)
>      return;
>  
> +  char system_dir[MAX_PATH];
> +  char syswow_dir[MAX_PATH];
> +  size_t system_dir_len = 0;
> +  bool convert_syswow_dir = false;
> +#ifdef __x86_64__
> +  if (wow64_process)
> +#endif
> +    {
> +      /* This fails on 32bit Windows because it has no SysWOW64 directory,
> +	 and in this case a path conversion isn't necessary.  */
> +      UINT len = GetSystemWow64DirectoryA (syswow_dir, sizeof (syswow_dir));
> +      if (len > 0)
> +	{
> +	  /* Check that we have passed a large enough buffer.  */
> +	  gdb_assert (len < sizeof (syswow_dir));
> +
> +	  len = GetSystemDirectoryA (system_dir, sizeof (system_dir));
> +	  /* Error check.  */
> +	  gdb_assert (len != 0);
> +	  /* Check that we have passed a large enough buffer.  */
> +	  gdb_assert (len < sizeof (system_dir));
> +
> +	  strcat (system_dir, "\\");
> +	  strcat (syswow_dir, "\\");
> +	  system_dir_len = strlen (system_dir);
> +
> +	  convert_syswow_dir = true;
> +	}
> +
> +    }

Since this is copied from gdb/windows-nat.c, it would be nice to factor it
out (as well as the small snippet below), so it can be shared.  Otherwise,
if we make a fix to one, we'll most likely forget to fix the other.

The shared versions would go into gdb/nat/windows-nat.c.

> +
>    for (i = 1; i < ((size_t) cbNeeded / sizeof (HMODULE)); i++)
>      {
>        MODULEINFO mi;
> @@ -1118,7 +1259,22 @@ win32_add_all_dlls (void)
>  					 dll_name,
>  					 MAX_PATH) == 0)
>  	continue;
> -      win32_add_one_solib (dll_name, (CORE_ADDR) (uintptr_t) mi.lpBaseOfDll);
> +
> +      const char *name = dll_name;
> +      /* Convert the DLL path of 32bit processes returned by
> +	 GetModuleFileNameEx from the 64bit system directory to the
> +	 32bit syswow64 directory if necessary.  */
> +      std::string syswow_dll_path;
> +      if (convert_syswow_dir
> +	  && strncasecmp (dll_name, system_dir, system_dir_len) == 0
> +	  && strchr (dll_name + system_dir_len, '\\') == nullptr)
> +	{
> +	  syswow_dll_path = syswow_dir;
> +	  syswow_dll_path += dll_name + system_dir_len;
> +	  name = syswow_dll_path.c_str();
> +	}
> +
> +      win32_add_one_solib (name, (CORE_ADDR) (uintptr_t) mi.lpBaseOfDll);
>      }
>  }
>  #endif
> @@ -1221,8 +1377,10 @@ maybe_adjust_pc ()
>    th->stopped_at_software_breakpoint = false;
>  
>    if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
> -      && (current_event.u.Exception.ExceptionRecord.ExceptionCode
> -	  == EXCEPTION_BREAKPOINT)
> +      && ((current_event.u.Exception.ExceptionRecord.ExceptionCode
> +	   == EXCEPTION_BREAKPOINT)
> +	  || (current_event.u.Exception.ExceptionRecord.ExceptionCode
> +	      == STATUS_WX86_BREAKPOINT))
>        && child_initialization_done)
>      {
>        th->stopped_at_software_breakpoint = true;
> @@ -1684,13 +1842,34 @@ win32_process_target::qxfer_siginfo (const char *annex,
>    if (readbuf == nullptr)
>      return -1;
>  
> -  if (offset > sizeof (siginfo_er))
> +  char *buf = (char *) &siginfo_er;
> +  size_t bufsize = sizeof (siginfo_er);
> +
> +#ifdef __x86_64__
> +  EXCEPTION_RECORD32 er32;
> +  if (wow64_process)
> +    {
> +      buf = (char *) &er32;
> +      bufsize = sizeof (er32);
> +
> +      er32.ExceptionCode = siginfo_er.ExceptionCode;
> +      er32.ExceptionFlags = siginfo_er.ExceptionFlags;
> +      er32.ExceptionRecord = (uintptr_t) siginfo_er.ExceptionRecord;
> +      er32.ExceptionAddress = (uintptr_t) siginfo_er.ExceptionAddress;
> +      er32.NumberParameters = siginfo_er.NumberParameters;
> +      int i;
> +      for (i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; i++)
> +	er32.ExceptionInformation[i] = siginfo_er.ExceptionInformation[i];
> +    }
> +#endif
> +
> +  if (offset > bufsize)
>      return -1;
>  
> -  if (offset + len > sizeof (siginfo_er))
> -    len = sizeof (siginfo_er) - offset;
> +  if (offset + len > bufsize)
> +    len = bufsize - offset;
>  
> -  memcpy (readbuf, (char *) &siginfo_er + offset, len);
> +  memcpy (readbuf, buf + offset, len);
>  
>    return len;
>  }
> @@ -1760,4 +1939,11 @@ initialize_low (void)
>  {
>    set_target_ops (&the_win32_target);
>    the_low_target.arch_setup ();
> +
> +#ifdef __x86_64__
> +  HMODULE dll = GetModuleHandle (_T("KERNEL32.DLL"));
> +  win32_Wow64SuspendThread = GETPROCADDRESS (dll, Wow64SuspendThread);
> +  win32_Wow64GetThreadContext = GETPROCADDRESS (dll, Wow64GetThreadContext);
> +  win32_Wow64SetThreadContext = GETPROCADDRESS (dll, Wow64SetThreadContext);

Why do we need to get these functions like this, instead of just calling them?

Simon

Re: [PATCH v2] Implement debugging of WOW64 processes in gdbserver

[Hannes Domani]

 Am Dienstag, 28. April 2020, 17:53:15 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:

> On 2020-04-27 9:34 a.m., Hannes Domani via Gdb-patches wrote:
> > @@ -496,7 +601,7 @@ i386_win32_set_pc (struct regcache *regcache, CORE_ADDR pc)
> >
> >  struct win32_target_ops the_low_target = {
> >    i386_arch_setup,
> > -  sizeof (mappings) / sizeof (mappings[0]),
> > +  i386_win32_num_regs,
> >    i386_initial_stuff,
> >    i386_get_thread_context,
> >    i386_prepare_to_resume,
> > diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
> > index 5a6f0df39f..d3fc31914d 100644
> > --- a/gdbserver/win32-low.cc
> > +++ b/gdbserver/win32-low.cc
> > @@ -88,15 +88,32 @@ static int soft_interrupt_requested = 0;
> >    by suspending all the threads.  */
> >  static int faked_breakpoint = 0;
> >
> > +#ifdef __x86_64__
> > +bool wow64_process = false;
> > +#endif
> > +
> >  const struct target_desc *win32_tdesc;
> > +#ifdef __x86_64__
> > +const struct target_desc *wow64_win32_tdesc;
> > +#endif
> >
> > -#define NUM_REGS (the_low_target.num_regs)
> > +#define NUM_REGS (the_low_target.num_regs ())
> >
> >  typedef BOOL (WINAPI *winapi_DebugActiveProcessStop) (DWORD dwProcessId);
> >  typedef BOOL (WINAPI *winapi_DebugSetProcessKillOnExit) (BOOL KillOnExit);
> >  typedef BOOL (WINAPI *winapi_DebugBreakProcess) (HANDLE);
> >  typedef BOOL (WINAPI *winapi_GenerateConsoleCtrlEvent) (DWORD, DWORD);
> >
> > +#ifdef __x86_64__
> > +typedef DWORD (WINAPI *winapi_Wow64SuspendThread) (HANDLE);
> > +typedef BOOL (WINAPI *winapi_Wow64SetThreadContext) (HANDLE,
> > +                            const WOW64_CONTEXT *);
> > +
> > +static winapi_Wow64SuspendThread win32_Wow64SuspendThread;
>
> win32_Wow64SuspendThread is never actually used, is this expected?

Well, in this case, kinda.
I only noticed afterwards that the same Wow64SuspendThread was unused in the
gdb equivalent, because unexpectedly, SuspendThread does work (at least on
my PC).

But I will remove it from this patch, and will try to fix it for both
gdb & gdbserver with a later patch.


> > @@ -195,7 +231,14 @@ child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
> >    if ((th = thread_rec (ptid, DONT_INVALIDATE_CONTEXT)))
> >      return th;
> >
> > -  th = new windows_thread_info (tid, h, (CORE_ADDR) (uintptr_t) tlb);
> > +  CORE_ADDR base = (CORE_ADDR) (uintptr_t) tlb;
> > +#ifdef __x86_64__
> > +  /* For WOW64 processes, this is actually the pointer to the 64bit TIB,
> > +    and the 32bit TIB is exactly 2 pages after it.  */
>
> Do you have a reference for this, ideally from MSDN?  If so, it would be nice to
> include the link in the comment.

Sadly no, I could find no official document for this.

There is an alternative way for to get the 32bit TIB, via the address at
offset 0 of the 64bit TIB:
https://docs.microsoft.com/en-us/windows/win32/debug/thread-environment-block--debugging-notes-

But even here it is mentioned that this may change in later Windows versions.


> > +  if (wow64_process)
> > +    base += 0x2000;
>
> Could you make it a PAGE_SIZE macro, and wirte `2 * PAGE_SIZE` here?  It would
> make it more obvious that it matches the comment above.

OK.


> > +#endif
> > +  th = new windows_thread_info (tid, h, base);
> >
> >    add_thread (ptid, th);
> >
> > @@ -345,8 +388,27 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
> >
> >    memset (&current_event, 0, sizeof (current_event));
> >
> > +#ifdef __x86_64__
> > +  BOOL wow64;
> > +  if (IsWow64Process (proch, &wow64))
> > +    wow64_process = wow64;
>
> If this function fails, it means we couldn't get the information we requested.  In
> this case, I think we should error out, because we can't continue reliably.  So,
> something like:
>
> BOOL wow64;
> if (!IsWow64Process (proch, &wow64_process))
>   {
>     // Call error() with a message formatted from GetLastError().
>   }

OK.


> > @@ -1096,13 +1197,53 @@ win32_add_all_dlls (void)
> >    if (!DllHandle)
> >      return;
> >
> > -  ok = (*win32_EnumProcessModules) (current_process_handle,
> > -                    DllHandle,
> > -                    cbNeeded,
> > -                    &cbNeeded);
> > +#ifdef __x86_64__
> > +  if (wow64_process)
> > +    ok = (*win32_EnumProcessModulesEx) (current_process_handle,
> > +                    DllHandle,
> > +                    cbNeeded,
> > +                    &cbNeeded,
> > +                    LIST_MODULES_32BIT);
> > +  else
> > +#endif
> > +    ok = (*win32_EnumProcessModules) (current_process_handle,
> > +                      DllHandle,
> > +                      cbNeeded,
> > +                      &cbNeeded);
> >    if (!ok)
> >      return;
> >
> > +  char system_dir[MAX_PATH];
> > +  char syswow_dir[MAX_PATH];
> > +  size_t system_dir_len = 0;
> > +  bool convert_syswow_dir = false;
> > +#ifdef __x86_64__
> > +  if (wow64_process)
> > +#endif
> > +    {
> > +      /* This fails on 32bit Windows because it has no SysWOW64 directory,
> > +    and in this case a path conversion isn't necessary.  */
> > +      UINT len = GetSystemWow64DirectoryA (syswow_dir, sizeof (syswow_dir));
> > +      if (len > 0)
> > +    {
> > +      /* Check that we have passed a large enough buffer.  */
> > +      gdb_assert (len < sizeof (syswow_dir));
> > +
> > +      len = GetSystemDirectoryA (system_dir, sizeof (system_dir));
> > +      /* Error check.  */
> > +      gdb_assert (len != 0);
> > +      /* Check that we have passed a large enough buffer.  */
> > +      gdb_assert (len < sizeof (system_dir));
> > +
> > +      strcat (system_dir, "\\");
> > +      strcat (syswow_dir, "\\");
> > +      system_dir_len = strlen (system_dir);
> > +
> > +      convert_syswow_dir = true;
> > +    }
> > +
> > +    }
>
> Since this is copied from gdb/windows-nat.c, it would be nice to factor it
> out (as well as the small snippet below), so it can be shared.  Otherwise,
> if we make a fix to one, we'll most likely forget to fix the other.
>
> The shared versions would go into gdb/nat/windows-nat.c.

There might be even more code that can be shared now, I planned to do this
with a later patch.


> > +
> >    for (i = 1; i < ((size_t) cbNeeded / sizeof (HMODULE)); i++)
> >      {
> >        MODULEINFO mi;
> > @@ -1118,7 +1259,22 @@ win32_add_all_dlls (void)
> >                      dll_name,
> >                      MAX_PATH) == 0)
> >      continue;
> > -      win32_add_one_solib (dll_name, (CORE_ADDR) (uintptr_t) mi.lpBaseOfDll);
> > +
> > +      const char *name = dll_name;
> > +      /* Convert the DLL path of 32bit processes returned by
> > +    GetModuleFileNameEx from the 64bit system directory to the
> > +    32bit syswow64 directory if necessary.  */
> > +      std::string syswow_dll_path;
> > +      if (convert_syswow_dir
> > +      && strncasecmp (dll_name, system_dir, system_dir_len) == 0
> > +      && strchr (dll_name + system_dir_len, '\\') == nullptr)
> > +    {
> > +      syswow_dll_path = syswow_dir;
> > +      syswow_dll_path += dll_name + system_dir_len;
> > +      name = syswow_dll_path.c_str();
> > +    }
> > +
> > +      win32_add_one_solib (name, (CORE_ADDR) (uintptr_t) mi.lpBaseOfDll);
> >      }
> >  }
> >  #endif
> > @@ -1221,8 +1377,10 @@ maybe_adjust_pc ()
> >    th->stopped_at_software_breakpoint = false;
> >
> >    if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
> > -      && (current_event.u.Exception.ExceptionRecord.ExceptionCode
> > -      == EXCEPTION_BREAKPOINT)
> > +      && ((current_event.u.Exception.ExceptionRecord.ExceptionCode
> > +      == EXCEPTION_BREAKPOINT)
> > +      || (current_event.u.Exception.ExceptionRecord.ExceptionCode
> > +          == STATUS_WX86_BREAKPOINT))
> >        && child_initialization_done)
> >      {
> >        th->stopped_at_software_breakpoint = true;
> > @@ -1684,13 +1842,34 @@ win32_process_target::qxfer_siginfo (const char *annex,
> >    if (readbuf == nullptr)
> >      return -1;
> >
> > -  if (offset > sizeof (siginfo_er))
> > +  char *buf = (char *) &siginfo_er;
> > +  size_t bufsize = sizeof (siginfo_er);
> > +
> > +#ifdef __x86_64__
> > +  EXCEPTION_RECORD32 er32;
> > +  if (wow64_process)
> > +    {
> > +      buf = (char *) &er32;
> > +      bufsize = sizeof (er32);
> > +
> > +      er32.ExceptionCode = siginfo_er.ExceptionCode;
> > +      er32.ExceptionFlags = siginfo_er.ExceptionFlags;
> > +      er32.ExceptionRecord = (uintptr_t) siginfo_er.ExceptionRecord;
> > +      er32.ExceptionAddress = (uintptr_t) siginfo_er.ExceptionAddress;
> > +      er32.NumberParameters = siginfo_er.NumberParameters;
> > +      int i;
> > +      for (i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; i++)
> > +    er32.ExceptionInformation[i] = siginfo_er.ExceptionInformation[i];
> > +    }
> > +#endif
> > +
> > +  if (offset > bufsize)
> >      return -1;
> >
> > -  if (offset + len > sizeof (siginfo_er))
> > -    len = sizeof (siginfo_er) - offset;
> > +  if (offset + len > bufsize)
> > +    len = bufsize - offset;
> >
> > -  memcpy (readbuf, (char *) &siginfo_er + offset, len);
> > +  memcpy (readbuf, buf + offset, len);
> >
> >    return len;
> >  }
> > @@ -1760,4 +1939,11 @@ initialize_low (void)
> >  {
> >    set_target_ops (&the_win32_target);
> >    the_low_target.arch_setup ();
> > +
> > +#ifdef __x86_64__
> > +  HMODULE dll = GetModuleHandle (_T("KERNEL32.DLL"));
> > +  win32_Wow64SuspendThread = GETPROCADDRESS (dll, Wow64SuspendThread);
> > +  win32_Wow64GetThreadContext = GETPROCADDRESS (dll, Wow64GetThreadContext);
> > +  win32_Wow64SetThreadContext = GETPROCADDRESS (dll, Wow64SetThreadContext);
>
> Why do we need to get these functions like this, instead of just calling them?

These functions are not available on WinXP, and the extra check for them
was actually the reason of this v2:

+  if (wow64_process
+      && (win32_Wow64SuspendThread == nullptr
+      || win32_Wow64GetThreadContext == nullptr
+      || win32_Wow64SetThreadContext == nullptr))
+    error ("WOW64 debugging is not supported on this system.\n");


Hannes

Re: [PATCH v2] Implement debugging of WOW64 processes in gdbserver

[Simon Marchi]

On 2020-04-29 6:54 a.m., Hannes Domani via Gdb-patches wrote:
>> Since this is copied from gdb/windows-nat.c, it would be nice to factor it
>> out (as well as the small snippet below), so it can be shared.  Otherwise,
>> if we make a fix to one, we'll most likely forget to fix the other.
>>
>> The shared versions would go into gdb/nat/windows-nat.c.
> 
> There might be even more code that can be shared now, I planned to do this
> with a later patch.

Ok.

>> Why do we need to get these functions like this, instead of just calling them?
> 
> These functions are not available on WinXP, and the extra check for them
> was actually the reason of this v2:
> 
> +  if (wow64_process
> +      && (win32_Wow64SuspendThread == nullptr
> +      || win32_Wow64GetThreadContext == nullptr
> +      || win32_Wow64SetThreadContext == nullptr))
> +    error ("WOW64 debugging is not supported on this system.\n");

Ok, please add a comment to that effect then.  This way if we decide we no longer want
to support Windows XP, we can remove this in favor of just calling the functions directly.

Simon

Re: [PATCH v2] Implement debugging of WOW64 processes in gdbserver

[Eli Zaretskii]

> From: Simon Marchi <simark@simark.ca>
> Date: Wed, 29 Apr 2020 11:09:08 -0400
> 
> > These functions are not available on WinXP, and the extra check for them
> > was actually the reason of this v2:
> > 
> > +  if (wow64_process
> > +      && (win32_Wow64SuspendThread == nullptr
> > +      || win32_Wow64GetThreadContext == nullptr
> > +      || win32_Wow64SetThreadContext == nullptr))
> > +    error ("WOW64 debugging is not supported on this system.\n");
> 
> Ok, please add a comment to that effect then.  This way if we decide we no longer want
> to support Windows XP, we can remove this in favor of just calling the functions directly.

Do the functions exist on 32-bit Windows systems newer than XP?  The
documentation says:

  It is not supported on 32-bit Windows; such calls fail and set the
  last error code to ERROR_INVALID_FUNCTION.

I'm not sure what this means for calling these functions directly.

Re: [PATCH v2] Implement debugging of WOW64 processes in gdbserver

[Simon Marchi]

On 2020-04-29 11:16 a.m., Eli Zaretskii wrote:
>> From: Simon Marchi <simark@simark.ca>
>> Date: Wed, 29 Apr 2020 11:09:08 -0400
>>
>>> These functions are not available on WinXP, and the extra check for them
>>> was actually the reason of this v2:
>>>
>>> +  if (wow64_process
>>> +      && (win32_Wow64SuspendThread == nullptr
>>> +      || win32_Wow64GetThreadContext == nullptr
>>> +      || win32_Wow64SetThreadContext == nullptr))
>>> +    error ("WOW64 debugging is not supported on this system.\n");
>>
>> Ok, please add a comment to that effect then.  This way if we decide we no longer want
>> to support Windows XP, we can remove this in favor of just calling the functions directly.
> 
> Do the functions exist on 32-bit Windows systems newer than XP?  The
> documentation says:
> 
>   It is not supported on 32-bit Windows; such calls fail and set the
>   last error code to ERROR_INVALID_FUNCTION.
> 
> I'm not sure what this means for calling these functions directly.

From what I understand, the program will still compile and link, but the call will fail.

Anyway, if we only use them in some `#ifdef __x86_64__` (as this patch does), we won't
have this problem.

Simon

Re: [PATCH v2] Implement debugging of WOW64 processes in gdbserver

[Eli Zaretskii]

> Cc: ssbssa@yahoo.de, gdb-patches@sourceware.org
> From: Simon Marchi <simark@simark.ca>
> Date: Wed, 29 Apr 2020 11:28:57 -0400
> 
> >   It is not supported on 32-bit Windows; such calls fail and set the
> >   last error code to ERROR_INVALID_FUNCTION.
> > 
> > I'm not sure what this means for calling these functions directly.
> 
> >From what I understand, the program will still compile and link, but the call will fail.

If that's what happens, then OK.  I was afraid GDB will refuse to run
because it doesn't find the entries for these functions in the
respective DLL.

> Anyway, if we only use them in some `#ifdef __x86_64__` (as this patch does), we won't
> have this problem.

Yes.