[PATCH v2] Implement debugging of WOW64 processes in gdbserver

Simon Marchi simark@simark.ca
Tue Apr 28 15:53:10 GMT 2020


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


More information about the Gdb-patches mailing list