[RFC] FAST_CWD warnings on ARM64 insider preview

[RFC] FAST_CWD warnings on ARM64 insider preview

[Jeremy Drake]

[-- Attachment #1: Type: text/plain, Size: 745 bytes --]

I have been trying out x86_64 msys2 (a fork of cygwin) on Windows insider
preview builds on ARM64, which added support for x86_64 emulation.  I was
getting the notorious FAST_CWD warnings, which were interfering with
MINGW CMake.  Last night late, I went looking at the cause, and found some
code that attempted to disable FAST_CWD warnings, but only on i686, and
trying to look at GetNativeSystemInfo, which was lying.  I quickly put
together this patch, and it seems to work.

Note I did this late at night, with no real regard to investigating or
matching code style.  This patch is currently more in an RFC state, if the
approach looks OK, and I'd be grateful for any pointers on getting it into
shape for evental acceptance.

Thanks,
Jeremy

[-- Attachment #2: Type: text/plain, Size: 2368 bytes --]

From 346dfb78fc5522d5d7288571d635303c69a5270a Mon Sep 17 00:00:00 2001
From: Jeremy Drake <cygwin@jdrake.com>
Date: Tue, 18 May 2021 00:48:52 -0700
Subject: [PATCH] cygwin: suppress FAST_CWD warnings on ARM64.

The old check was insufficient: new insider preview builds of Windows
allow running x86_64 process on ARM64.  The IsWow64Process2 function
seems to be the intended way to figure this situation out.
---
 winsup/cygwin/path.cc | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index dd7048486..5c4adcd49 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -4708,29 +4708,19 @@ find_fast_cwd ()
     {
       bool warn = 1;
 
-#ifndef __x86_64__
-      #ifndef PROCESSOR_ARCHITECTURE_ARM64
-      #define PROCESSOR_ARCHITECTURE_ARM64 12
-      #endif
+#ifndef IMAGE_FILE_MACHINE_ARM64
+#define IMAGE_FILE_MACHINE_ARM64 0xAA64
+#endif
 
-      SYSTEM_INFO si;
+      USHORT procmachine, nativemachine;
 
       /* Check if we're running in WOW64 on ARM64.  Skip the warning as long as
-	 there's no solution for finding the FAST_CWD pointer on that system.
-
-	 2018-07-12: Apparently current ARM64 WOW64 has a bug:
-	 It's GetNativeSystemInfo returns PROCESSOR_ARCHITECTURE_INTEL in
-	 wProcessorArchitecture.  Since that's an invalid value (a 32 bit
-	 host system hosting a 32 bit emulator for itself?) we can use this
-	 value as an indicator to skip the message as well. */
-      if (wincap.is_wow64 ())
-	{
-	  GetNativeSystemInfo (&si);
-	  if (si.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_ARM64
-	      || si.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_INTEL)
-	    warn = 0;
-	}
-#endif /* !__x86_64__ */
+	 there's no solution for finding the FAST_CWD pointer on that system. */
+
+      typedef BOOL (WINAPI * IsWow64Process2_t)(HANDLE hProcess, USHORT *pProcessMachine, USHORT *pNativeMachine);
+      IsWow64Process2_t pfnIsWow64Process2 = (IsWow64Process2_t)GetProcAddress(GetModuleHandle("kernel32.dll"), "IsWow64Process2");
+      if (pfnIsWow64Process2 && pfnIsWow64Process2(GetCurrentProcess(), &procmachine, &nativemachine) && nativemachine == IMAGE_FILE_MACHINE_ARM64)
+	warn = 0;
 
       if (warn)
 	small_printf ("Cygwin WARNING:\n"
-- 
2.31.1.windows.1

Re: [RFC] FAST_CWD warnings on ARM64 insider preview

[Corinna Vinschen]

Hi Jeremy,

On May 18 11:56, Jeremy Drake via Cygwin-patches wrote:
> I have been trying out x86_64 msys2 (a fork of cygwin) on Windows insider
> preview builds on ARM64, which added support for x86_64 emulation.  I was
> getting the notorious FAST_CWD warnings, which were interfering with
> MINGW CMake.  Last night late, I went looking at the cause, and found some
> code that attempted to disable FAST_CWD warnings, but only on i686, and
> trying to look at GetNativeSystemInfo, which was lying.  I quickly put
> together this patch, and it seems to work.
> 
> Note I did this late at night, with no real regard to investigating or
> matching code style.  This patch is currently more in an RFC state, if the
> approach looks OK, and I'd be grateful for any pointers on getting it into
> shape for evental acceptance.

> +#ifndef IMAGE_FILE_MACHINE_ARM64
> +#define IMAGE_FILE_MACHINE_ARM64 0xAA64
> +#endif

IMAGE_FILE_MACHINE_ARM64 is already defined for some time in winnt.h
so we won't need the ifdef.

> +      typedef BOOL (WINAPI * IsWow64Process2_t)(HANDLE hProcess, USHORT *pProcessMachine, USHORT *pNativeMachine);
> +      IsWow64Process2_t pfnIsWow64Process2 = (IsWow64Process2_t)GetProcAddress(GetModuleHandle("kernel32.dll"), "IsWow64Process2");
> +      if (pfnIsWow64Process2 && pfnIsWow64Process2(GetCurrentProcess(), &procmachine, &nativemachine) && nativemachine == IMAGE_FILE_MACHINE_ARM64)

We're having the autoload mechanism for that, i. e., your patch can get
rid of the GetModuleHandle/GetProcAddress preliminaries entirely.

By using the LoadDLLfuncEx() expression, failure to load the function
will result in a return value of FALSE with GetLastError ==
ERROR_PROC_NOT_FOUND, so this is safe on older systems.

It's easier to understand with a full example, so I took the liberty to
rewrite your patch accordingly.  Since the idea and the basic work is
yours, I'd push this under your name, see below.


Thanks,
Corinna


From ee6dfb3bb0a5d51416d788133f818df0b8c1a207 Mon Sep 17 00:00:00 2001
From: Jeremy Drake <cygwin@jdrake.com>
Date: Wed, 19 May 2021 11:43:48 +0200
Subject: [PATCH] Cygwin: suppress FAST_CWD warnings on ARM64

The old check was insufficient: new insider preview builds of Windows
allow running x86_64 process on ARM64.  The IsWow64Process2 function
seems to be the intended way to figure this situation out.
---
 winsup/cygwin/autoload.cc |  1 +
 winsup/cygwin/path.cc     | 33 +++++++++------------------------
 2 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/winsup/cygwin/autoload.cc b/winsup/cygwin/autoload.cc
index b2de2c3f42ce..0ebe15332013 100644
--- a/winsup/cygwin/autoload.cc
+++ b/winsup/cygwin/autoload.cc
@@ -587,6 +587,7 @@ LoadDLLfuncEx (GetLogicalProcessorInformationEx, 12, kernel32, 1)
 LoadDLLfuncEx (GetProcessGroupAffinity, 12, kernel32, 1)
 LoadDLLfunc (GetSystemTimePreciseAsFileTime, 4, kernel32)
 LoadDLLfuncEx (GetThreadGroupAffinity, 8, kernel32, 1)
+LoadDLLfuncEx (IsWow64Process2, 12, kernel32, 1)
 LoadDLLfuncEx (PrefetchVirtualMemory, 16, kernel32, 1)
 LoadDLLfunc (SetThreadGroupAffinity, 12, kernel32)
 
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index dd7048486a65..9b38793aa7d9 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -4707,30 +4707,15 @@ find_fast_cwd ()
   if (!f_cwd_ptr)
     {
       bool warn = 1;
-
-#ifndef __x86_64__
-      #ifndef PROCESSOR_ARCHITECTURE_ARM64
-      #define PROCESSOR_ARCHITECTURE_ARM64 12
-      #endif
-
-      SYSTEM_INFO si;
-
-      /* Check if we're running in WOW64 on ARM64.  Skip the warning as long as
-	 there's no solution for finding the FAST_CWD pointer on that system.
-
-	 2018-07-12: Apparently current ARM64 WOW64 has a bug:
-	 It's GetNativeSystemInfo returns PROCESSOR_ARCHITECTURE_INTEL in
-	 wProcessorArchitecture.  Since that's an invalid value (a 32 bit
-	 host system hosting a 32 bit emulator for itself?) we can use this
-	 value as an indicator to skip the message as well. */
-      if (wincap.is_wow64 ())
-	{
-	  GetNativeSystemInfo (&si);
-	  if (si.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_ARM64
-	      || si.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_INTEL)
-	    warn = 0;
-	}
-#endif /* !__x86_64__ */
+      USHORT emulated, hosted;
+
+      /* Check if we're running in WOW64 on ARM64.  Check on 64 bit as well,
+	 given that ARM64 Windows 10 provides a x86_64 emulation soon.  Skip
+	 warning as long as there's no solution for finding the FAST_CWD
+	 pointer on that system. */
+      if (IsWow64Process2 (GetCurrentProcess (), &emulated, &hosted)
+	  && hosted == IMAGE_FILE_MACHINE_ARM64)
+	warn = 0;
 
       if (warn)
 	small_printf ("Cygwin WARNING:\n"
-- 
2.30.2

Re: [RFC] FAST_CWD warnings on ARM64 insider preview

[Jeremy Drake]

On Wed, 19 May 2021, Corinna Vinschen wrote:

> > +#ifndef IMAGE_FILE_MACHINE_ARM64
> > +#define IMAGE_FILE_MACHINE_ARM64 0xAA64
> > +#endif
>
> IMAGE_FILE_MACHINE_ARM64 is already defined for some time in winnt.h
> so we won't need the ifdef.

OK.  Was just matching what was done with PROCESSOR_ARCHITECTURE_ARM64.

> > +      typedef BOOL (WINAPI * IsWow64Process2_t)(HANDLE hProcess, USHORT *pProcessMachine, USHORT *pNativeMachine);
> > +      IsWow64Process2_t pfnIsWow64Process2 = (IsWow64Process2_t)GetProcAddress(GetModuleHandle("kernel32.dll"), "IsWow64Process2");
> > +      if (pfnIsWow64Process2 && pfnIsWow64Process2(GetCurrentProcess(), &procmachine, &nativemachine) && nativemachine == IMAGE_FILE_MACHINE_ARM64)
>
> We're having the autoload mechanism for that, i. e., your patch can get
> rid of the GetModuleHandle/GetProcAddress preliminaries entirely.
>
> By using the LoadDLLfuncEx() expression, failure to load the function
> will result in a return value of FALSE with GetLastError ==
> ERROR_PROC_NOT_FOUND, so this is safe on older systems.

Nice.

> It's easier to understand with a full example, so I took the liberty to
> rewrite your patch accordingly.  Since the idea and the basic work is
> yours, I'd push this under your name, see below.

Looks good.  Thanks

Re: [RFC] FAST_CWD warnings on ARM64 insider preview

[Corinna Vinschen]

On May 19 11:02, Jeremy Drake via Cygwin-patches wrote:
> On Wed, 19 May 2021, Corinna Vinschen wrote:
> 
> > > +#ifndef IMAGE_FILE_MACHINE_ARM64
> > > +#define IMAGE_FILE_MACHINE_ARM64 0xAA64
> > > +#endif
> >
> > IMAGE_FILE_MACHINE_ARM64 is already defined for some time in winnt.h
> > so we won't need the ifdef.
> 
> OK.  Was just matching what was done with PROCESSOR_ARCHITECTURE_ARM64.
> 
> > > +      typedef BOOL (WINAPI * IsWow64Process2_t)(HANDLE hProcess, USHORT *pProcessMachine, USHORT *pNativeMachine);
> > > +      IsWow64Process2_t pfnIsWow64Process2 = (IsWow64Process2_t)GetProcAddress(GetModuleHandle("kernel32.dll"), "IsWow64Process2");
> > > +      if (pfnIsWow64Process2 && pfnIsWow64Process2(GetCurrentProcess(), &procmachine, &nativemachine) && nativemachine == IMAGE_FILE_MACHINE_ARM64)
> >
> > We're having the autoload mechanism for that, i. e., your patch can get
> > rid of the GetModuleHandle/GetProcAddress preliminaries entirely.
> >
> > By using the LoadDLLfuncEx() expression, failure to load the function
> > will result in a return value of FALSE with GetLastError ==
> > ERROR_PROC_NOT_FOUND, so this is safe on older systems.
> 
> Nice.
> 
> > It's easier to understand with a full example, so I took the liberty to
> > rewrite your patch accordingly.  Since the idea and the basic work is
> > yours, I'd push this under your name, see below.
> 
> Looks good.  Thanks

Pushed.

Thanks,
Corinna