Hi Takashi,

On Wed, 17 Dec 2025, Takashi Yano wrote:

> After the commit f74dc93c6359, WSL cannot start by distribution name
> such as debian.exe, which has '.exe' extention but actually is an app
> execution alias. The commit f74dc93c6359 disables to follow windows
> reparse point by adding PC_SYM_NOFOLLOW_REP flag in spawn.cc. As a
> result, is_console_app () returns false for the app execution alias
> due to open-failure because CreateFileW() cannot open an app execution
> alias. This patch fixes the issue by converting the path again using
> PC_SYM_FOLLOW (without PC_SYM_NOFOLLOW_REP) option in path_conv if
> the path is an app execution alias.

Since you repeat this commit as the culprit, I guess that this is your way
to explain to me that before this commit, the standard handles were
correctly set even after 2533912fc76c (Allow executing Windows Store's
"app execution aliases", 2021-03-12) allowed executing Microsoft Store
Apps via their app execution aliases?

> Fixes: f74dc93c6359 ("fix native symlink spawn passing wrong arg0")
> Reviewed-by: Johannes Schindelin <[email protected]>
> Signed-off-by: Takashi Yano <[email protected]>
> ---
>  winsup/cygwin/fhandler/termios.cc       | 21 ++++++++++++++++-----
>  winsup/cygwin/local_includes/fhandler.h |  2 +-
>  winsup/cygwin/spawn.cc                  |  2 +-
>  3 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler/termios.cc 
> b/winsup/cygwin/fhandler/termios.cc
> index 19d6220bc..7fdbf6a97 100644
> --- a/winsup/cygwin/fhandler/termios.cc
> +++ b/winsup/cygwin/fhandler/termios.cc
> @@ -702,10 +702,21 @@ fhandler_termios::fstat (struct stat *buf)
>  }
>  
>  static bool
> -is_console_app (const WCHAR *filename)
> +is_console_app (path_conv &pc)

I see you insist of mixing the refactor where `path_conv &` is passed
instead of `WCHAR *` with the actual fix.

Not a fan. I regularly hit your commits when bisecting Cygwin runtime
regressions, and I have not yet learned to be okay with finding patches
that do too many things at the same time.

I'm weven less a fan of the non-descriptive variable name `pc` which
unnecessarily increases cognitive load.

But hey, rather than shouting my objections to the form in the void, I'll
just accept that my recommendations are not welcome, and this is the shape
of the patch that you want to have. It does fix the bug, so that's good.

Ciao,
Johannes

>  {
> +  tmp_pathbuf tp;
> +  WCHAR *native_path = tp.w_get ();
> +  pc.get_wide_win32_path (native_path);
> +  if (pc.is_app_execution_alias ())
> +    {
> +      UNICODE_STRING upath;
> +      RtlInitUnicodeString (&upath, native_path);
> +      path_conv target (&upath, PC_SYM_FOLLOW);
> +      target.get_wide_win32_path (native_path);
> +    }
> +
>    HANDLE h;
> -  h = CreateFileW (filename, GENERIC_READ, FILE_SHARE_READ,
> +  h = CreateFileW (native_path, GENERIC_READ, FILE_SHARE_READ,
>                  NULL, OPEN_EXISTING, 0, NULL);
>    char buf[1024];
>    DWORD n;
> @@ -716,7 +727,7 @@ is_console_app (const WCHAR *filename)
>    IMAGE_NT_HEADERS32 *p = (IMAGE_NT_HEADERS32 *) memmem (buf, n, "PE\0\0", 
> 4);
>    if (p && (char *) &p->OptionalHeader.DllCharacteristics <= buf + n)
>      return p->OptionalHeader.Subsystem == IMAGE_SUBSYSTEM_WINDOWS_CUI;
> -  wchar_t *e = wcsrchr (filename, L'.');
> +  wchar_t *e = wcsrchr (native_path, L'.');
>    if (e && (wcscasecmp (e, L".bat") == 0 || wcscasecmp (e, L".cmd") == 0))
>      return true;
>    return false;
> @@ -755,7 +766,7 @@ fhandler_termios::ioctl (unsigned int cmd, void *varg)
>  
>  void
>  fhandler_termios::spawn_worker::setup (bool iscygwin, HANDLE h_stdin,
> -                                    const WCHAR *runpath, bool nopcon,
> +                                    path_conv &pc, bool nopcon,
>                                      bool reset_sendsig,
>                                      const WCHAR *envblock)
>  {
> @@ -794,7 +805,7 @@ fhandler_termios::spawn_worker::setup (bool iscygwin, 
> HANDLE h_stdin,
>           ptys->setup_locale ();
>         }
>      }
> -  if (!iscygwin && ptys_primary && is_console_app (runpath))
> +  if (!iscygwin && ptys_primary && is_console_app (pc))
>      {
>        if (h_stdin == ptys_primary->get_handle_nat ())
>       stdin_is_ptys = true;
> diff --git a/winsup/cygwin/local_includes/fhandler.h 
> b/winsup/cygwin/local_includes/fhandler.h
> index 0de82163e..16f55b4f7 100644
> --- a/winsup/cygwin/local_includes/fhandler.h
> +++ b/winsup/cygwin/local_includes/fhandler.h
> @@ -2036,7 +2036,7 @@ class fhandler_termios: public fhandler_base
>      spawn_worker () :
>        ptys_need_cleanup (false), cons_need_cleanup (false),
>        stdin_is_ptys (false), ptys_ttyp (NULL) {}
> -    void setup (bool iscygwin, HANDLE h_stdin, const WCHAR *runpath,
> +    void setup (bool iscygwin, HANDLE h_stdin, path_conv &pc,
>               bool nopcon, bool reset_sendsig, const WCHAR *envblock);
>      bool need_cleanup () { return ptys_need_cleanup || cons_need_cleanup; }
>      void cleanup ();
> diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
> index 71add8755..7d993d081 100644
> --- a/winsup/cygwin/spawn.cc
> +++ b/winsup/cygwin/spawn.cc
> @@ -579,7 +579,7 @@ child_info_spawn::worker (const char *prog_arg, const 
> char *const *argv,
>  
>        bool no_pcon = mode != _P_OVERLAY && mode != _P_WAIT;
>        term_spawn_worker.setup (iscygwin (), handle (fileno_stdin, false),
> -                            runpath, no_pcon, reset_sendsig, envblock);
> +                            real_path, no_pcon, reset_sendsig, envblock);
>  
>        /* Set up needed handles for stdio */
>        si.dwFlags = STARTF_USESTDHANDLES;
> -- 
> 2.51.0
> 
> 

Reply via email to