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

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

[Tom Tromey]

I recently ran across the ASLR feature on Windows and wanted to be
able to disable it, as is done on Linux with the
"disable-randomization" feature.  I didn't think it was possible, but
Christian pointed out some documentation showing how to do it.  So, I
wrote this series.  It's split up to make each part a bit easier to
understand.

No new test case, as I have never managed to run dejagnu on Windows.
I've tested it by hand.

Note that this feature requires Windows 8.

Tom

[PATCH 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 0bb87ec0674..29f03d37ba5 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 d8c5cfb2bd5..27f0e1fb4f6 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 736b794fc82..87bfd8969fa 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
@@ -2676,17 +2673,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);
@@ -2800,11 +2789,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 1e97f91d954..0e96a22bfed 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.31.1

[PATCH 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.

Note that, in order to see the necessary declarations for this code to
work, the patch bumps _WIN32_WINNT from 0x0501 to 0x0602 -- that is,
Windows 8.  I don't know whether this may is a problem for anyone.
---
 gdb/NEWS                 |  4 ++
 gdb/nat/windows-nat.c    | 87 ++++++++++++++++++++++++++++++++++++----
 gdb/nat/windows-nat.h    | 11 ++++-
 gdb/windows-nat.c        |  7 ++++
 gdbserver/win32-low.cc   |  1 +
 gdbserver/win32-low.h    |  5 +++
 gdbsupport/common-defs.h |  8 ++--
 7 files changed, 110 insertions(+), 13 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 7ca1f842cb1..c8bacffe10e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,10 @@
 
 *** Changes since GDB 11
 
+* Windows 8 is the minimum supported version.
+
+* The disable-randomization now works on Windows.
+
 * New commands
 
 maint set backtrace-on-fatal-signal on|off
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 29f03d37ba5..500782adc9f 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -19,6 +19,8 @@
 #include "gdbsupport/common-defs.h"
 #include "nat/windows-nat.h"
 #include "gdbsupport/common-debug.h"
+#include <winbase.h>
+#include <processthreadsapi.h>
 
 namespace windows_nat
 {
@@ -579,15 +581,80 @@ wait_for_debug_event (DEBUG_EVENT *event, DWORD timeout)
   return result;
 }
 
-/* Helper template for the CreateProcess wrappers.  */
-template<typename FUNC, typename CHAR, typename INFO>
+/* Helper template for the CreateProcess wrappers.
+
+   The EXTENDED type is the subtype of "startupinfo" to use, e.g.,
+   STARTUPINFOEXA.  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 EXTENDED, 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)
+    {
+      static bool tried_and_failed;
+
+      if (!tried_and_failed)
+	{
+	  EXTENDED 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 =
+	    (PROCESS_CREATION_MITIGATION_POLICY_FORCE_RELOCATE_IMAGES_ALWAYS_OFF
+	     | PROCESS_CREATION_MITIGATION_POLICY_BOTTOM_UP_ASLR_ALWAYS_OFF);
+	  if (!UpdateProcThreadAttribute (info_ex.lpAttributeList, 0,
+					  PROC_THREAD_ATTRIBUTE_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,12 +672,14 @@ 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,
-				 startup_info, process_info);
+  return create_process_wrapper<STARTUPINFOEXA>
+    (CreateProcessA, image, command_line, flags,
+     environment, cur_dir, no_randomization,
+     startup_info, process_info);
 }
 
 #ifdef __CYGWIN__
@@ -620,12 +689,14 @@ 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,
-				 startup_info, process_info);
+  return create_process_wrapper<STARTUPINFOEXW>
+    (CreateProcessW, image, command_line, flags,
+     environment, cur_dir, no_randomization,
+     startup_info, process_info);
 }
 
 #endif /* __CYGWIN__ */
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 27f0e1fb4f6..978bd926405 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
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 87bfd8969fa..468ac63ce9a 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -283,6 +283,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 true;
+  }
 };
 
 static windows_nat_target the_windows_nat_target;
@@ -2675,6 +2680,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. */
@@ -2794,6 +2800,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 0e96a22bfed..5570498fde4 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 11c318e25a5..5f45f7a8998 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 true;
+  }
 };
 
 /* Retrieve the context for this thread, if not already retrieved.  */
diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h
index 56780765756..996d25d5a1e 100644
--- a/gdbsupport/common-defs.h
+++ b/gdbsupport/common-defs.h
@@ -68,17 +68,17 @@
 #define _FORTIFY_SOURCE 2
 #endif
 
-/* We don't support Windows versions before XP, so we define
+/* We don't support Windows versions before Windows 8, so we define
    _WIN32_WINNT correspondingly to ensure the Windows API headers
    expose the required symbols.  */
 #if defined (__MINGW32__) || defined (__CYGWIN__)
 # ifdef _WIN32_WINNT
-#  if _WIN32_WINNT < 0x0501
+#  if _WIN32_WINNT < 0x0602
 #   undef _WIN32_WINNT
-#   define _WIN32_WINNT 0x0501
+#   define _WIN32_WINNT 0x0602
 #  endif
 # else
-#  define _WIN32_WINNT 0x0501
+#  define _WIN32_WINNT 0x0602
 # endif
 #endif	/* __MINGW32__ || __CYGWIN__ */
 
-- 
2.31.1

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

[Eli Zaretskii]

> Date: Wed,  6 Oct 2021 14:47:04 -0600
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Tom Tromey <tromey@adacore.com>
> 
> Note that, in order to see the necessary declarations for this code to
> work, the patch bumps _WIN32_WINNT from 0x0501 to 0x0602 -- that is,
> Windows 8.  I don't know whether this may is a problem for anyone.

It is for me.

Please, let's not unsupport old Windows versions just because some
optional feature needs a new API.  Instead, we should test for the
availability of that API at run time, and disable the feature if the
API is unavailable.  We already do that with several APIs, see
initialize_loadable in nat/windows-nat.c.  Why not do the same with
the APIs you need to support this feature?  Using that technique will
makes GDB able to run on older versions of Windows if the user doesn't
need to disable ASLR.

For example, I use GDB a lot on Windows XP, where ASLR doesn't even
exist, and therefore the problem you want to solve doesn't exist.  But
the changes you propose will prevent GDB from even starting on XP or
any other systems older than Windows 8.

> +#include <winbase.h>
> +#include <processthreadsapi.h>

I don't think this is the right way of including Windows headers.
Doesn't it work to just include <windows.h> (which nat/windows-nat.h
already does)?  The MS documentation seems to say it should be enough.

If including windows.h alone doesn't work, then we should have a
configure-time test for processthreadapi.h, instead of including it
unconditionally, as that header might not exist (e.g., my MinGW
doesn't have it).  Moreover, some prototypes and macros that are only
available since Windows 8 should be defined by hand in our sources, if
they are not already defined, because using this:

> -/* We don't support Windows versions before XP, so we define
> +/* We don't support Windows versions before Windows 8, so we define
>     _WIN32_WINNT correspondingly to ensure the Windows API headers
>     expose the required symbols.  */
>  #if defined (__MINGW32__) || defined (__CYGWIN__)
>  # ifdef _WIN32_WINNT
> -#  if _WIN32_WINNT < 0x0501
> +#  if _WIN32_WINNT < 0x0602
>  #   undef _WIN32_WINNT
> -#   define _WIN32_WINNT 0x0501
> +#   define _WIN32_WINNT 0x0602
>  #  endif
>  # else
> -#  define _WIN32_WINNT 0x0501
> +#  define _WIN32_WINNT 0x0602
>  # endif
>  #endif	/* __MINGW32__ || __CYGWIN__ */

will not help when building on older systems, and unnecessarily limits
the GDB support to Windows 8 and later systems.

> +	  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 =
> +	    (PROCESS_CREATION_MITIGATION_POLICY_FORCE_RELOCATE_IMAGES_ALWAYS_OFF
> +	     | PROCESS_CREATION_MITIGATION_POLICY_BOTTOM_UP_ASLR_ALWAYS_OFF);
> +	  if (!UpdateProcThreadAttribute (info_ex.lpAttributeList, 0,
> +					  PROC_THREAD_ATTRIBUTE_MITIGATION_POLICY,
> +					  &attr_flags,
> +					  sizeof (attr_flags),
> +					  nullptr, nullptr))
> +	    tried_and_failed = true;

As mentioned above, these new APIs should be loaded dynamically and
called through a function pointer.  And the requisite preprocessor
macros should be defined if they aren't defined by the headers.  If
the new APIs are not available, the wrapper should simply call
CreateProcess without doing anything else.

> +  bool supports_disable_randomization () override
> +  {
> +    return true;
> +  }

This cannot be unconditional, it should depend on whether the requisite
APIs could be dynamically loaded at run time.

> --- 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 true;
> +  }

Same here.

Thanks.

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

[Christian Biesinger]

On Thu, Oct 7, 2021 at 2:47 AM Eli Zaretskii via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>
> > Date: Wed,  6 Oct 2021 14:47:04 -0600
> > From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> > Cc: Tom Tromey <tromey@adacore.com>
> >
> > Note that, in order to see the necessary declarations for this code to
> > work, the patch bumps _WIN32_WINNT from 0x0501 to 0x0602 -- that is,
> > Windows 8.  I don't know whether this may is a problem for anyone.
>
> It is for me.

Hm, not sure that that define is an issue for runtime per se.

> > +#include <winbase.h>
> > +#include <processthreadsapi.h>
>
> I don't think this is the right way of including Windows headers.
> Doesn't it work to just include <windows.h> (which nat/windows-nat.h
> already does)?  The MS documentation seems to say it should be enough.

It looks like gdb does not define WIN32_LEAN_AND_MEAN so I agree this
should not be necessary.

> > -/* We don't support Windows versions before XP, so we define
> > +/* We don't support Windows versions before Windows 8, so we define
> >     _WIN32_WINNT correspondingly to ensure the Windows API headers
> >     expose the required symbols.  */
> >  #if defined (__MINGW32__) || defined (__CYGWIN__)
> >  # ifdef _WIN32_WINNT
> > -#  if _WIN32_WINNT < 0x0501
> > +#  if _WIN32_WINNT < 0x0602
> >  #   undef _WIN32_WINNT
> > -#   define _WIN32_WINNT 0x0501
> > +#   define _WIN32_WINNT 0x0602
> >  #  endif
> >  # else
> > -#  define _WIN32_WINNT 0x0501
> > +#  define _WIN32_WINNT 0x0602
> >  # endif
> >  #endif       /* __MINGW32__ || __CYGWIN__ */
>
> will not help when building on older systems, and unnecessarily limits
> the GDB support to Windows 8 and later systems.

This does not affect *building* on older systems at all. But see above
re running on older systems.

Christian

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

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> Please, let's not unsupport old Windows versions just because some
Eli> optional feature needs a new API.  Instead, we should test for the
Eli> availability of that API at run time, and disable the feature if the
Eli> API is unavailable.  We already do that with several APIs, see
Eli> initialize_loadable in nat/windows-nat.c.  Why not do the same with
Eli> the APIs you need to support this feature?

The issue here is that the types in question aren't available in older
headers.

I'm happy to try to work around that if you can suggest how.

The type in question is the extended startupinfo type:

https://docs.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-startupinfoexa

I think we could redeclare this in gdb, but the issue is that the type
of the field is also not declared, IIRC.

thanks,
Tom

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

[Eli Zaretskii]

> From: Tom Tromey <tromey@adacore.com>
> Cc: Tom Tromey <tromey@adacore.com>,  gdb-patches@sourceware.org
> Date: Mon, 08 Nov 2021 11:34:53 -0700
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Eli> Please, let's not unsupport old Windows versions just because some
> Eli> optional feature needs a new API.  Instead, we should test for the
> Eli> availability of that API at run time, and disable the feature if the
> Eli> API is unavailable.  We already do that with several APIs, see
> Eli> initialize_loadable in nat/windows-nat.c.  Why not do the same with
> Eli> the APIs you need to support this feature?
> 
> The issue here is that the types in question aren't available in older
> headers.
> 
> I'm happy to try to work around that if you can suggest how.

The usual way is to define them in our sources.  If we want to be
holier than the Pope, we can define them only if _WIN32_WINNT's value
is less than the one under which they are defined in the w32api
headers.

> I think we could redeclare this in gdb, but the issue is that the type
> of the field is also not declared, IIRC.

Yes, we'd need to declare every type except the opaque ones.

Let me know if you need help in doing that.

Thanks.