public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
From: Corinna Vinschen <corinna-cygwin@cygwin.com>
To: Jeremy Drake <cygwin@jdrake.com>
Cc: cygwin-patches@cygwin.com
Subject: Re: [RFC] FAST_CWD warnings on ARM64 insider preview
Date: Wed, 19 May 2021 11:49:27 +0200	[thread overview]
Message-ID: <YKTfJ2kVCsV+yT3g@calimero.vinschen.de> (raw)
In-Reply-To: <alpine.BSO.2.21.2105181151200.14962@resin.csoft.net>

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


  reply	other threads:[~2021-05-19  9:49 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-18 18:56 Jeremy Drake
2021-05-19  9:49 ` Corinna Vinschen [this message]
2021-05-19 18:02   ` Jeremy Drake
2021-05-20  7:17     ` Corinna Vinschen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YKTfJ2kVCsV+yT3g@calimero.vinschen.de \
    --to=corinna-cygwin@cygwin.com \
    --cc=cygwin-patches@cygwin.com \
    --cc=cygwin@jdrake.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).