[PATCH v2] Cygwin: pty: Fix nat_pipe_owner_pid when gdb runs non-cygwin app

[Johannes Schindelin]

Hi Takashi,

On Wed, 8 Jul 2026, Takashi Yano wrote:

> Previously, nat_pipe_owner_pid was incorrectly set to 0 when the
> inferior of gdb was a non-cygwin app. Due to this bug, repeatedly
> running a non-cygwin app under gdb could lead to an unexpected crash.
> 
> This occurred because the previous code in setup_for_non_cygwin_app()
> set nat_pipe_owner_pid to exec_dwProcessId, which is correct when the
> caller is the stub process of the non-cygwin app. However, when the
> caller is gdb, the owner should be gdb itself, so nat_pipe_owner_pid
> must be set to myself->dwProcessId.
> 
> With this fix, attach_console_temporarily() can be called with target
> pid equal to the process's own pid, in which case the attach operation
> is skipped.
> 
> Fixes: 1e6c51d74136 ("Cygwin: pty: Reorganize the code path of setting up and closing pcon.")
> Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
> Reviewed-by:
> ---
> v2: Skip attaching operation when attaching to myself is requested.

From what I can see, the fix is correct. Three non-blocking notes:

The new source comment (and the commit message) has exec_dwProcessId and
dwProcessId the wrong way round, I think. exec_dwProcessId holds the
stub's own pid, saved before the overlay; dwProcessId is what gets
repointed to the native child. The code still does the right thing because
`exec_dwProcessId ?: dwProcessId` reduces to "our own pid" on every path
that reaches it, which is exactly what the owner-self check already
assumes. But the commentary as written will mislead the next reader.

The sibling assignment in `setup_pseudoconsole()` still writes
exec_dwProcessId directly, without the fallback. Harmless today because
its sole caller sets the owner first, but leaving the two spots
inconsistent invites a future regression. Worth applying the same `?:`
there.

Also: I believe that the bug is not really gdb-specific: _any_ Cygwin
process that spawns a non-cygwin app through the CreateProcess hook
without _P_OVERLAY hits it. If that is so, the subject and log undersell
the scope.

In any case, I happily provide my:

  Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

Ciao,
Johannes

> 
>  winsup/cygwin/fhandler/pty.cc | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
> index 1b453a499..6ef4fa506 100644
> --- a/winsup/cygwin/fhandler/pty.cc
> +++ b/winsup/cygwin/fhandler/pty.cc
> @@ -4734,7 +4734,11 @@ fhandler_pty_slave::setup_for_non_cygwin_app (bool nopcon,
>        fhandler_pty_slave *ptys = (fhandler_pty_slave *) fh;
>        ptys->get_ttyp ()->switch_to_nat_pipe = true;
>        if (!process_alive (ptys->get_ttyp ()->nat_pipe_owner_pid))
> -	ptys->get_ttyp ()->nat_pipe_owner_pid = myself->exec_dwProcessId;
> +	/* In normal case where the current process is the stub process for
> +	   non-cygwin app, set owner to exec_dwProcessId (non-cygwin app).
> +	   However, in gdb case, gdb itself should be the owner. */
> +	ptys->get_ttyp ()->nat_pipe_owner_pid =
> +	  myself->exec_dwProcessId ? : myself->dwProcessId;
>      }
>    bool pcon_enabled = false;
>    if (!nopcon)
> @@ -4862,6 +4866,8 @@ fhandler_pty_common::attach_console_temporarily (DWORD target_pid)
>  {
>    DWORD resume_pid = 0;
>    acquire_attach_mutex (mutex_timeout);
> +  if (target_pid == GetCurrentProcessId ())
> +    return target_pid;
>    pinfo pinfo_resume (myself->ppid);
>    if (pinfo_resume)
>      resume_pid = pinfo_resume->dwProcessId;
> @@ -4880,6 +4886,11 @@ fhandler_pty_common::attach_console_temporarily (DWORD target_pid)
>  void
>  fhandler_pty_common::resume_from_temporarily_attach (DWORD resume_pid)
>  {
> +  if (resume_pid == GetCurrentProcessId ())
> +    {
> +      release_attach_mutex ();
> +      return;
> +    }
>    bool console_exists = (resume_pid != (DWORD) -1);
>    if (!console_exists || resume_pid)
>      {
> -- 
> 2.51.0
> 
>