[PATCH v2 0/2] Make "set disable-randomization" work on Windows

[PATCH v2 0/2] Make "set disable-randomization" work on Windows

[Tom Tromey]

Here's v2 of the series to make "set disable-randomization" work on
Windows.

v1 was here:

https://sourceware.org/pipermail/gdb-patches/2021-October/182438.html

I believe this version addresses all the review comments.  In
particular, it now dynamically checks for the needed APIs and should
not need a newer version of Windows in order to build.

There are some caveats:

This passes regular testing against the AdaCore internal test suite.
I've never gotten dejagnu to work on Windows, so I can't run the gdb
test suite there.

I can no longer actually test whether ASLR disabling works, as it is
globally disabled on all the Windows machines I can access.

I can't actually test building on an older version of Windows, so I
don't know whether my attempts were actually successful.

Tom

[PATCH v2 1/2] Introduce wrapper for CreateProcess

[Tom Tromey]

This is a small refactoring that introduces a wrapper for the Windows
CreateProcess function.  This is done to make the next patch a bit
simpler.
---
 gdb/nat/windows-nat.c  | 51 ++++++++++++++++++++++++++++++++++++++++++
 gdb/nat/windows-nat.h  | 15 +++++++++++++
 gdb/windows-nat.c      | 22 ++++--------------
 gdbserver/win32-low.cc |  5 +----
 4 files changed, 71 insertions(+), 22 deletions(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 21445f3f859..fdfc8e702f8 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -579,6 +579,57 @@ wait_for_debug_event (DEBUG_EVENT *event, DWORD timeout)
   return result;
 }
 
+/* Helper template for the CreateProcess wrappers.  */
+template<typename FUNC, typename CHAR, typename INFO>
+BOOL
+create_process_wrapper (FUNC *do_create_process, const CHAR *image,
+			CHAR *command_line, DWORD flags,
+			void *environment, const CHAR *cur_dir,
+			INFO *startup_info,
+			PROCESS_INFORMATION *process_info)
+{
+  return do_create_process (image,
+			    command_line, /* command line */
+			    nullptr,	  /* Security */
+			    nullptr,	  /* thread */
+			    TRUE,	  /* inherit handles */
+			    flags,	  /* start flags */
+			    environment,  /* environment */
+			    cur_dir,	  /* current directory */
+			    startup_info,
+			    process_info);
+}
+
+/* See nat/windows-nat.h.  */
+
+BOOL
+create_process (const char *image, char *command_line, DWORD flags,
+		void *environment, const char *cur_dir,
+		STARTUPINFOA *startup_info,
+		PROCESS_INFORMATION *process_info)
+{
+  return create_process_wrapper (CreateProcessA, image, command_line, flags,
+				 environment, cur_dir,
+				 startup_info, process_info);
+}
+
+#ifdef __CYGWIN__
+
+/* See nat/windows-nat.h.  */
+
+BOOL
+create_process (const wchar_t *image, wchar_t *command_line, DWORD flags,
+		void *environment, const wchar_t *cur_dir,
+		STARTUPINFOW *startup_info,
+		PROCESS_INFORMATION *process_info);
+{
+  return create_process_wrapper (CreateProcessW, image, command_line, flags,
+				 environment, cur_dir,
+				 startup_info, process_info);
+}
+
+#endif /* __CYGWIN__ */
+
 /* Define dummy functions which always return error for the rare cases where
    these functions could not be found.  */
 template<typename... T>
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index f0abd7d795c..a0267cd96ba 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -263,6 +263,21 @@ extern BOOL continue_last_debug_event (DWORD continue_status,
 
 extern BOOL wait_for_debug_event (DEBUG_EVENT *event, DWORD timeout);
 
+/* Wrappers for CreateProcess.  */
+
+extern BOOL create_process (const char *image, char *command_line,
+			    DWORD flags, void *environment,
+			    const char *cur_dir,
+			    STARTUPINFOA *startup_info,
+			    PROCESS_INFORMATION *process_info);
+#ifdef __CYGWIN__
+extern BOOL create_process (const wchar_t *image, wchar_t *command_line,
+			    DWORD flags, void *environment,
+			    const wchar_t *cur_dir,
+			    STARTUPINFOW *startup_info,
+			    PROCESS_INFORMATION *process_info);
+#endif /* __CYGWIN__ */
+
 #define AdjustTokenPrivileges		dyn_AdjustTokenPrivileges
 #define DebugActiveProcessStop		dyn_DebugActiveProcessStop
 #define DebugBreakProcess		dyn_DebugBreakProcess
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 81e26fe4759..251876c7022 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -75,14 +75,12 @@
 using namespace windows_nat;
 
 #undef STARTUPINFO
-#undef CreateProcess
 #undef GetModuleFileNameEx
 
 #ifndef __CYGWIN__
 # define __PMAX	(MAX_PATH + 1)
 # define GetModuleFileNameEx GetModuleFileNameExA
 # define STARTUPINFO STARTUPINFOA
-# define CreateProcess CreateProcessA
 #else
 # define __PMAX	PATH_MAX
 /* The starting and ending address of the cygwin1.dll text segment.  */
@@ -92,7 +90,6 @@ using namespace windows_nat;
     typedef wchar_t cygwin_buf_t;
 #   define GetModuleFileNameEx GetModuleFileNameExW
 #   define STARTUPINFO STARTUPINFOW
-#   define CreateProcess CreateProcessW
 #endif
 
 static int have_saved_context;	/* True if we've saved context from a
@@ -2616,17 +2613,9 @@ windows_nat_target::create_inferior (const char *exec_file,
     }
 
   windows_init_thread_list ();
-  ret = CreateProcess (0,
-		       args,	/* command line */
-		       NULL,	/* Security */
-		       NULL,	/* thread */
-		       TRUE,	/* inherit handles */
-		       flags,	/* start flags */
-		       w32_env,	/* environment */
-		       inferior_cwd != NULL ? infcwd : NULL, /* current
-								directory */
-		       &si,
-		       &pi);
+  ret = create_process (args, flags, w32_env,
+			inferior_cwd != nullptr ? infcwd : nullptr,
+			&si, &pi);
   if (w32_env)
     /* Just free the Win32 environment, if it could be created. */
     free (w32_env);
@@ -2740,11 +2729,8 @@ windows_nat_target::create_inferior (const char *exec_file,
   *temp = 0;
 
   windows_init_thread_list ();
-  ret = CreateProcessA (0,
+  ret = create_process (nullptr, /* image */
 			args,	/* command line */
-			NULL,	/* Security */
-			NULL,	/* thread */
-			TRUE,	/* inherit handles */
 			flags,	/* start flags */
 			w32env,	/* environment */
 			inferior_cwd, /* current directory */
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index e19bc2bd6e8..5164da59a21 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -572,11 +572,8 @@ create_process (const char *program, char *args,
   strcpy (program_and_args, program);
   strcat (program_and_args, " ");
   strcat (program_and_args, args);
-  ret = CreateProcessA (program,           /* image name */
+  ret = create_process (program,           /* image name */
 			program_and_args,  /* command line */
-			NULL,              /* security */
-			NULL,              /* thread */
-			TRUE,              /* inherit handles */
 			flags,             /* start flags */
 			NULL,              /* environment */
 			/* current directory */
-- 
2.34.1

[PATCH v2 2/2] Allow ASLR to be disabled on Windows

[Tom Tromey]

On Windows, it is possible to disable ASLR when creating a process.
This patch adds code to do this, and hooks it up to gdb's existing
disable-randomization feature.  Because the Windows documentation
cautions that this isn't available on all versions of Windows, the
CreateProcess wrapper function is updated to make the attempt, and
then fall back to the current approach if it fails.
---
 gdb/NEWS               |   2 +
 gdb/nat/windows-nat.c  | 115 +++++++++++++++++++++++++++++++++++++++--
 gdb/nat/windows-nat.h  |  38 +++++++++++++-
 gdb/windows-nat.c      |   7 +++
 gdbserver/win32-low.cc |   1 +
 gdbserver/win32-low.h  |   5 ++
 6 files changed, 164 insertions(+), 4 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index c9b6f42616b..b22f8825b99 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -9,6 +9,8 @@
   Python 2.  From GDB 13, it will only be possible to build GDB itself
   with Python 3 support.
 
+* The disable-randomization setting now works on Windows.
+
 * Improved C++ template support
 
   GDB now treats functions/types involving C++ templates like it does function
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index fdfc8e702f8..cb9f10afa06 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -68,6 +68,10 @@ Wow64GetThreadSelectorEntry_ftype *Wow64GetThreadSelectorEntry;
 #endif
 GenerateConsoleCtrlEvent_ftype *GenerateConsoleCtrlEvent;
 
+InitializeProcThreadAttributeList_ftype *InitializeProcThreadAttributeList;
+UpdateProcThreadAttribute_ftype *UpdateProcThreadAttribute;
+DeleteProcThreadAttributeList_ftype *DeleteProcThreadAttributeList;
+
 /* Note that 'debug_events' must be locally defined in the relevant
    functions.  */
 #define DEBUG_EVENTS(fmt, ...) \
@@ -579,15 +583,104 @@ wait_for_debug_event (DEBUG_EVENT *event, DWORD timeout)
   return result;
 }
 
-/* Helper template for the CreateProcess wrappers.  */
+/* Flags to pass to UpdateProcThreadAttribute.  */
+#define relocate_aslr_flags ((0x2 << 8) | (0x2 << 16))
+
+/* Attribute to pass to UpdateProcThreadAttribute.  */
+#define mitigation_policy 0x00020007
+
+/* Pick one of the symbols as a sentinel.  */
+#ifdef PROCESS_CREATION_MITIGATION_POLICY_FORCE_RELOCATE_IMAGES_ALWAYS_OFF
+
+static_assert ((PROCESS_CREATION_MITIGATION_POLICY_FORCE_RELOCATE_IMAGES_ALWAYS_OFF
+		| PROCESS_CREATION_MITIGATION_POLICY_BOTTOM_UP_ASLR_ALWAYS_OFF)
+	       == relocate_aslr_flags,
+	       "check that ASLR flag values are correct");
+
+static_assert (PROC_THREAD_ATTRIBUTE_MITIGATION_POLICY == mitigation_policy,
+	       "check that mitigation policy value is correct");
+
+#endif
+
+/* Helper template for the CreateProcess wrappers.
+
+   FUNC is the type of the underlying CreateProcess call.  CHAR is the
+   character type to use, and INFO is the "startupinfo" type to use.
+
+   DO_CREATE_PROCESS is the underlying CreateProcess function to use;
+   the remaining arguments are passed to it.  */
 template<typename FUNC, typename CHAR, typename INFO>
 BOOL
 create_process_wrapper (FUNC *do_create_process, const CHAR *image,
 			CHAR *command_line, DWORD flags,
 			void *environment, const CHAR *cur_dir,
+			bool no_randomization,
 			INFO *startup_info,
 			PROCESS_INFORMATION *process_info)
 {
+  if (no_randomization && disable_randomization_available ())
+    {
+      static bool tried_and_failed;
+
+      if (!tried_and_failed)
+	{
+	  /* Windows 8 is required for the real declaration, but to
+	     allow building on earlier versions of Windows, we declare
+	     the type locally.  */
+	  struct gdb_extended_info
+	  {
+	    INFO StartupInfo;
+	    gdb_lpproc_thread_attribute_list lpAttributeList;
+	  };
+
+	  gdb_extended_info info_ex {};
+
+	  if (startup_info != nullptr)
+	    info_ex.StartupInfo = *startup_info;
+	  info_ex.StartupInfo.cb = sizeof (info_ex);
+	  SIZE_T size = 0;
+	  /* Ignore the result here.  The documentation says the first
+	     call always fails, by design.  */
+	  InitializeProcThreadAttributeList (nullptr, 1, 0, &size);
+	  info_ex.lpAttributeList
+	    = (PPROC_THREAD_ATTRIBUTE_LIST) alloca (size);
+	  InitializeProcThreadAttributeList (info_ex.lpAttributeList,
+					     1, 0, &size);
+
+	  gdb::optional<BOOL> return_value;
+	  DWORD attr_flags = relocate_aslr_flags;
+	  if (!UpdateProcThreadAttribute (info_ex.lpAttributeList, 0,
+					  mitigation_policy,
+					  &attr_flags,
+					  sizeof (attr_flags),
+					  nullptr, nullptr))
+	    tried_and_failed = true;
+	  else
+	    {
+	      BOOL result = do_create_process (image, command_line,
+					       nullptr, nullptr,
+					       TRUE,
+					       (flags
+						| EXTENDED_STARTUPINFO_PRESENT),
+					       environment,
+					       cur_dir,
+					       (STARTUPINFO *) &info_ex,
+					       process_info);
+	      if (result)
+		return_value = result;
+	      else if (GetLastError () == ERROR_INVALID_PARAMETER)
+		tried_and_failed = true;
+	      else
+		return_value = FALSE;
+	    }
+
+	  DeleteProcThreadAttributeList (info_ex.lpAttributeList);
+
+	  if (return_value.has_value ())
+	    return *return_value;
+	}
+    }
+
   return do_create_process (image,
 			    command_line, /* command line */
 			    nullptr,	  /* Security */
@@ -605,11 +698,12 @@ create_process_wrapper (FUNC *do_create_process, const CHAR *image,
 BOOL
 create_process (const char *image, char *command_line, DWORD flags,
 		void *environment, const char *cur_dir,
+		bool no_randomization,
 		STARTUPINFOA *startup_info,
 		PROCESS_INFORMATION *process_info)
 {
   return create_process_wrapper (CreateProcessA, image, command_line, flags,
-				 environment, cur_dir,
+				 environment, cur_dir, no_randomization,
 				 startup_info, process_info);
 }
 
@@ -620,11 +714,12 @@ create_process (const char *image, char *command_line, DWORD flags,
 BOOL
 create_process (const wchar_t *image, wchar_t *command_line, DWORD flags,
 		void *environment, const wchar_t *cur_dir,
+		bool no_randomization,
 		STARTUPINFOW *startup_info,
 		PROCESS_INFORMATION *process_info);
 {
   return create_process_wrapper (CreateProcessW, image, command_line, flags,
-				 environment, cur_dir,
+				 environment, cur_dir, no_randomization,
 				 startup_info, process_info);
 }
 
@@ -664,6 +759,16 @@ bad_GetConsoleFontSize (HANDLE w, DWORD nFont)
  
 /* See windows-nat.h.  */
 
+bool
+disable_randomization_available ()
+{
+  return (InitializeProcThreadAttributeList != nullptr
+	  && UpdateProcThreadAttribute != nullptr
+	  && DeleteProcThreadAttributeList != nullptr);
+}
+
+/* See windows-nat.h.  */
+
 bool
 initialize_loadable ()
 {
@@ -689,6 +794,10 @@ initialize_loadable ()
       GPA (hm, Wow64GetThreadSelectorEntry);
 #endif
       GPA (hm, GenerateConsoleCtrlEvent);
+
+      GPA (hm, InitializeProcThreadAttributeList);
+      GPA (hm, UpdateProcThreadAttribute);
+      GPA (hm, DeleteProcThreadAttributeList);
     }
 
   /* Set variables to dummy versions of these processes if the function
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index a0267cd96ba..722de895498 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -263,17 +263,21 @@ extern BOOL continue_last_debug_event (DWORD continue_status,
 
 extern BOOL wait_for_debug_event (DEBUG_EVENT *event, DWORD timeout);
 
-/* Wrappers for CreateProcess.  */
+/* Wrappers for CreateProcess.  These exist primarily so that the
+   "disable randomization" feature can be implemented in a single
+   place.  */
 
 extern BOOL create_process (const char *image, char *command_line,
 			    DWORD flags, void *environment,
 			    const char *cur_dir,
+			    bool no_randomization,
 			    STARTUPINFOA *startup_info,
 			    PROCESS_INFORMATION *process_info);
 #ifdef __CYGWIN__
 extern BOOL create_process (const wchar_t *image, wchar_t *command_line,
 			    DWORD flags, void *environment,
 			    const wchar_t *cur_dir,
+			    bool no_randomization,
 			    STARTUPINFOW *startup_info,
 			    PROCESS_INFORMATION *process_info);
 #endif /* __CYGWIN__ */
@@ -282,10 +286,15 @@ extern BOOL create_process (const wchar_t *image, wchar_t *command_line,
 #define DebugActiveProcessStop		dyn_DebugActiveProcessStop
 #define DebugBreakProcess		dyn_DebugBreakProcess
 #define DebugSetProcessKillOnExit	dyn_DebugSetProcessKillOnExit
+#undef EnumProcessModules
 #define EnumProcessModules		dyn_EnumProcessModules
+#undef EnumProcessModulesEx
 #define EnumProcessModulesEx		dyn_EnumProcessModulesEx
+#undef GetModuleInformation
 #define GetModuleInformation		dyn_GetModuleInformation
+#undef GetModuleFileNameExA
 #define GetModuleFileNameExA		dyn_GetModuleFileNameExA
+#undef GetModuleFileNameExW
 #define GetModuleFileNameExW		dyn_GetModuleFileNameExW
 #define LookupPrivilegeValueA		dyn_LookupPrivilegeValueA
 #define OpenProcessToken		dyn_OpenProcessToken
@@ -296,6 +305,9 @@ extern BOOL create_process (const wchar_t *image, wchar_t *command_line,
 #define Wow64SetThreadContext		dyn_Wow64SetThreadContext
 #define Wow64GetThreadSelectorEntry	dyn_Wow64GetThreadSelectorEntry
 #define GenerateConsoleCtrlEvent	dyn_GenerateConsoleCtrlEvent
+#define InitializeProcThreadAttributeList dyn_InitializeProcThreadAttributeList
+#define UpdateProcThreadAttribute dyn_UpdateProcThreadAttribute
+#define DeleteProcThreadAttributeList dyn_DeleteProcThreadAttributeList
 
 typedef BOOL WINAPI (AdjustTokenPrivileges_ftype) (HANDLE, BOOL,
 						   PTOKEN_PRIVILEGES,
@@ -366,6 +378,30 @@ extern Wow64GetThreadSelectorEntry_ftype *Wow64GetThreadSelectorEntry;
 typedef BOOL WINAPI (GenerateConsoleCtrlEvent_ftype) (DWORD, DWORD);
 extern GenerateConsoleCtrlEvent_ftype *GenerateConsoleCtrlEvent;
 
+/* We use a local typedef for this type to avoid depending on
+   Windows 8.  */
+typedef void *gdb_lpproc_thread_attribute_list;
+
+typedef BOOL WINAPI (InitializeProcThreadAttributeList_ftype)
+     (gdb_lpproc_thread_attribute_list lpAttributeList,
+      DWORD dwAttributeCount, DWORD dwFlags, PSIZE_T lpSize);
+extern InitializeProcThreadAttributeList_ftype *InitializeProcThreadAttributeList;
+
+typedef BOOL WINAPI (UpdateProcThreadAttribute_ftype)
+     (gdb_lpproc_thread_attribute_list lpAttributeList,
+      DWORD dwFlags, DWORD_PTR Attribute, PVOID lpValue, SIZE_T cbSize,
+      PVOID lpPreviousValue, PSIZE_T lpReturnSize);
+extern UpdateProcThreadAttribute_ftype *UpdateProcThreadAttribute;
+
+typedef void WINAPI (DeleteProcThreadAttributeList_ftype)
+     (gdb_lpproc_thread_attribute_list lpAttributeList);
+extern DeleteProcThreadAttributeList_ftype *DeleteProcThreadAttributeList;
+
+/* Return true if it's possible to disable randomization on this
+   host.  */
+
+extern bool disable_randomization_available ();
+
 /* Load any functions which may not be available in ancient versions
    of Windows.  */
 
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 251876c7022..b2e27d9cb52 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -281,6 +281,11 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
   int get_windows_debug_event (int pid, struct target_waitstatus *ourstatus);
 
   void do_initial_windows_stuff (DWORD pid, bool attaching);
+
+  bool supports_disable_randomization () override
+  {
+    return disable_randomization_available ();
+  }
 };
 
 static windows_nat_target the_windows_nat_target;
@@ -2615,6 +2620,7 @@ windows_nat_target::create_inferior (const char *exec_file,
   windows_init_thread_list ();
   ret = create_process (args, flags, w32_env,
 			inferior_cwd != nullptr ? infcwd : nullptr,
+			disable_randomization,
 			&si, &pi);
   if (w32_env)
     /* Just free the Win32 environment, if it could be created. */
@@ -2734,6 +2740,7 @@ windows_nat_target::create_inferior (const char *exec_file,
 			flags,	/* start flags */
 			w32env,	/* environment */
 			inferior_cwd, /* current directory */
+			disable_randomization,
 			&si,
 			&pi);
   if (tty != INVALID_HANDLE_VALUE)
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 5164da59a21..67a10fceca6 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -580,6 +580,7 @@ create_process (const char *program, char *args,
 			(inferior_cwd.empty ()
 			 ? NULL
 			 : gdb_tilde_expand (inferior_cwd.c_str ()).c_str()),
+			get_client_state ().disable_randomization,
 			&si,               /* start info */
 			pi);               /* proc info */
 
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index 8856a84baa3..357854c4e82 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -158,6 +158,11 @@ class win32_process_target : public process_stratum_target
   bool stopped_by_sw_breakpoint () override;
 
   bool supports_stopped_by_sw_breakpoint () override;
+
+  bool supports_disable_randomization () override
+  {
+    return windows_nat::disable_randomization_available ();
+  }
 };
 
 /* Retrieve the context for this thread, if not already retrieved.  */
-- 
2.34.1

Re: [PATCH v2 0/2] Make "set disable-randomization" work on Windows

[Eli Zaretskii]

> Date: Fri, 11 Mar 2022 11:57:03 -0700
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> 
> Here's v2 of the series to make "set disable-randomization" work on
> Windows.
> 
> v1 was here:
> 
> https://sourceware.org/pipermail/gdb-patches/2021-October/182438.html
> 
> I believe this version addresses all the review comments.  In
> particular, it now dynamically checks for the needed APIs and should
> not need a newer version of Windows in order to build.
> 
> There are some caveats:
> 
> This passes regular testing against the AdaCore internal test suite.
> I've never gotten dejagnu to work on Windows, so I can't run the gdb
> test suite there.
> 
> I can no longer actually test whether ASLR disabling works, as it is
> globally disabled on all the Windows machines I can access.
> 
> I can't actually test building on an older version of Windows, so I
> don't know whether my attempts were actually successful.

Thanks, I just read the code, and it LGTM.  Will test when I have the
opportunity, probably when GDB 12 gets into pretest.

Re: [PATCH v2 0/2] Make "set disable-randomization" work on Windows

[Tom Tromey]

>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> Here's v2 of the series to make "set disable-randomization" work on
Tom> Windows.

Tom> v1 was here:

Tom> https://sourceware.org/pipermail/gdb-patches/2021-October/182438.html

Tom> I believe this version addresses all the review comments.  In
Tom> particular, it now dynamically checks for the needed APIs and should
Tom> not need a newer version of Windows in order to build.

I'm checking this in now.

Tom

Re: [PATCH v2 1/2] Introduce wrapper for CreateProcess

[Jon Turney]

On 11/03/2022 18:57, Tom Tromey via Gdb-patches wrote:
> This is a small refactoring that introduces a wrapper for the Windows
> CreateProcess function.  This is done to make the next patch a bit
> simpler.
> ---
>   gdb/nat/windows-nat.c  | 51 ++++++++++++++++++++++++++++++++++++++++++
>   gdb/nat/windows-nat.h  | 15 +++++++++++++
>   gdb/windows-nat.c      | 22 ++++--------------
>   gdbserver/win32-low.cc |  5 +----
>   4 files changed, 71 insertions(+), 22 deletions(-)
> 
> diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
> index 21445f3f859..fdfc8e702f8 100644
> --- a/gdb/nat/windows-nat.c
> +++ b/gdb/nat/windows-nat.c
> @@ -579,6 +579,57 @@ wait_for_debug_event (DEBUG_EVENT *event, DWORD timeout)
[...]
> +#ifdef __CYGWIN__
> +
> +/* See nat/windows-nat.h.  */
> +
> +BOOL
> +create_process (const wchar_t *image, wchar_t *command_line, DWORD flags,
> +		void *environment, const wchar_t *cur_dir,
> +		STARTUPINFOW *startup_info,
> +		PROCESS_INFORMATION *process_info);
> +{

Stray semicolon

> +  return create_process_wrapper (CreateProcessW, image, command_line, flags,
> +				 environment, cur_dir,
> +				 startup_info, process_info);
> +}
> +
> +#endif /* __CYGWIN__ */
[...]
> diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
> index 81e26fe4759..251876c7022 100644
> --- a/gdb/windows-nat.c
> +++ b/gdb/windows-nat.c
>     windows_init_thread_list ();
> -  ret = CreateProcess (0,
> -		       args,	/* command line */
> -		       NULL,	/* Security */
> -		       NULL,	/* thread */
> -		       TRUE,	/* inherit handles */
> -		       flags,	/* start flags */
> -		       w32_env,	/* environment */
> -		       inferior_cwd != NULL ? infcwd : NULL, /* current
> -								directory */
> -		       &si,
> -		       &pi);
> +  ret = create_process (args, flags, w32_env,
> +			inferior_cwd != nullptr ? infcwd : nullptr,
> +			&si, &pi);

This use of create_process() under CYGWIN has one less argument that the 
  prototype as the NULL program argument was dropped.

I pushed a patch to fix these cygwin build issues as trivial.

Re: [PATCH v2 2/2] Allow ASLR to be disabled on Windows

[Jon Turney]

On 11/03/2022 18:57, Tom Tromey via Gdb-patches wrote:
> On Windows, it is possible to disable ASLR when creating a process.
> This patch adds code to do this, and hooks it up to gdb's existing
> disable-randomization feature.  Because the Windows documentation
> cautions that this isn't available on all versions of Windows, the
> CreateProcess wrapper function is updated to make the attempt, and
> then fall back to the current approach if it fails.
> ---
[...]
> --- a/gdb/nat/windows-nat.c
> +++ b/gdb/nat/windows-nat.c
[...]
> +
> +/* Helper template for the CreateProcess wrappers.
> +
> +   FUNC is the type of the underlying CreateProcess call.  CHAR is the
> +   character type to use, and INFO is the "startupinfo" type to use.
> +
> +   DO_CREATE_PROCESS is the underlying CreateProcess function to use;
> +   the remaining arguments are passed to it.  */
>   template<typename FUNC, typename CHAR, typename INFO>
>   BOOL
>   create_process_wrapper (FUNC *do_create_process, const CHAR *image,
>   			CHAR *command_line, DWORD flags,
>   			void *environment, const CHAR *cur_dir,
> +			bool no_randomization,
>   			INFO *startup_info,
>   			PROCESS_INFORMATION *process_info)
>   {
> +  if (no_randomization && disable_randomization_available ())
> +    {
> +      static bool tried_and_failed;
> +
> +      if (!tried_and_failed)
> +	{
> +	  /* Windows 8 is required for the real declaration, but to
> +	     allow building on earlier versions of Windows, we declare
> +	     the type locally.  */
> +	  struct gdb_extended_info
> +	  {
> +	    INFO StartupInfo;
> +	    gdb_lpproc_thread_attribute_list lpAttributeList;
> +	  };
> +
> +	  gdb_extended_info info_ex {};
> +
> +	  if (startup_info != nullptr)
> +	    info_ex.StartupInfo = *startup_info;
> +	  info_ex.StartupInfo.cb = sizeof (info_ex);
> +	  SIZE_T size = 0;
> +	  /* Ignore the result here.  The documentation says the first
> +	     call always fails, by design.  */
> +	  InitializeProcThreadAttributeList (nullptr, 1, 0, &size);
> +	  info_ex.lpAttributeList
> +	    = (PPROC_THREAD_ATTRIBUTE_LIST) alloca (size);
> +	  InitializeProcThreadAttributeList (info_ex.lpAttributeList,
> +					     1, 0, &size);
> +
> +	  gdb::optional<BOOL> return_value;
> +	  DWORD attr_flags = relocate_aslr_flags;
> +	  if (!UpdateProcThreadAttribute (info_ex.lpAttributeList, 0,
> +					  mitigation_policy,
> +					  &attr_flags,
> +					  sizeof (attr_flags),
> +					  nullptr, nullptr))
> +	    tried_and_failed = true;
> +	  else
> +	    {
> +	      BOOL result = do_create_process (image, command_line,
> +					       nullptr, nullptr,
> +					       TRUE,
> +					       (flags
> +						| EXTENDED_STARTUPINFO_PRESENT),
> +					       environment,
> +					       cur_dir,
> +					       (STARTUPINFO *) &info_ex,

Absent _UNICODE being defined (which gdb's Makefile doesn't), windows.h 
will always define STARTUPINFO is as STARTUPINFOA, so this cast doesn't 
work when it should be STARTUPINFOW (i.e. in Cygwin build).

So either arrange for it to be defined correctly in 
gdb/nat/windows-nat.c (as done in lines 152-160 in gdb/windows-nat.c), 
or write this as &info_ex.StartupInfo (which is of the correct type) 
instead?