[PATCH v7 2/7] Cygwin: pty: Add workaround for handling of backspace when pcon enabled
Johannes Schindelin
Johannes.Schindelin@gmx.de
Fri Mar 27 13:25:50 GMT 2026
Hi Takashi,
First, I want to acknowledge that this version is a significant
improvement over the previous version that interleaved `WriteFile()` calls
to the pipe with `WriteConsoleInput()` calls to a different handle, which
was fundamentally racy. The v7 approach is much more robust: pre-convert
0x08 to 0x7F before writing to the pipe, knowing that conhost's buggy
reverse path will convert it back. This is a legitimate work-around.
That said, I have a few comments, starting with the commit message:
On Wed, 25 Mar 2026, Takashi Yano wrote:
> In Windows 11, pseudo console has a weird behaviour that the Ctrl-H
> is translated into Ctrl-Backspace (not Backspace). Similary, Backspace
> (0x7f) is translated into Ctrl-H.
As I pointed out in
https://inbox.sourceware.org/cygwin-patches/2f8628d2-b79a-95a6-480d-7508375958d5@gmx.de/
(where I also included a proper fix for the upstream bug, to be submitted
pending testing), this should not be described vaguely as "a weird
behaviour". The root cause is well understood: the reverse VT input path
in conhost's `_DoControlCharacter()` maps the byte 0x08 to a
Ctrl+Backspace key event (VK_BACK with LEFT_CTRL_PRESSED and character
0x7F). This was introduced in PR #3935 (Jan 2020) to make Ctrl+Backspace
delete whole words. In September 2022, PR #13894 rewrote the forward path
to properly implement DECBKM (Backarrow Key Mode), but the reverse path
was never updated to match, breaking the roundtrip.
The commit message should say that, not "a weird behaviour".
> Due to this behaviour, inrec_eq() in cons_master_thread() fails to
> compare backspace/Ctrl-H events in the input record sequence. This patch
> is a workaround for the issue that replaces Ctrl-H with backspace
> (0x7f), which will be translated into Ctrl-H in pseudo console.
>
> Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ---
> winsup/cygwin/fhandler/console.cc | 12 ++++++-
> winsup/cygwin/fhandler/pty.cc | 57 ++++++++++++++++++++++++++-----
> 2 files changed, 60 insertions(+), 9 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
> index 2f59f8f24..9678775d1 100644
> --- a/winsup/cygwin/fhandler/console.cc
> +++ b/winsup/cygwin/fhandler/console.cc
> @@ -318,6 +318,16 @@ inrec_eq (const INPUT_RECORD *a, const INPUT_RECORD *b, DWORD n)
> written event. Therefore they are ignored. */
> const KEY_EVENT_RECORD *ak = &a[i].Event.KeyEvent;
> const KEY_EVENT_RECORD *bk = &b[i].Event.KeyEvent;
> + WCHAR c1 = ak->uChar.UnicodeChar;
> + WCHAR c2 = bk->uChar.UnicodeChar;
> + if (inside_pcon)
> + {
> + /* Workaround for pseudo console in Windows 11 */
> + if (c1 == 8) /* Ctrl-H */
> + c1 = 127; /* Backspace */
> + if (c2 == 8) /* Ctrl-H */
> + c2 = 127; /* Backspace */
> + }
This change in `inrec_eq()` makes sense as a companion to the
pre-conversion: `transfer_input()` reads back INPUT_RECORDs that went
through conhost's buggy reverse path, so the comparison needs to treat
0x08 and 0x7F as equivalent.
> /* Fixup repeat count */
> WORD r1 = ak->wRepeatCount;
> WORD r2 = bk->wRepeatCount;
> @@ -326,7 +336,7 @@ inrec_eq (const INPUT_RECORD *a, const INPUT_RECORD *b, DWORD n)
> if (r2 == 0)
> r2 = 1;
> if (ak->bKeyDown != bk->bKeyDown
> - || ak->uChar.UnicodeChar != bk->uChar.UnicodeChar
> + || c1 != c2
> || r1 != r2)
> return false;
> }
> diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
> index 371e67103..72a8ba140 100644
> --- a/winsup/cygwin/fhandler/pty.cc
> +++ b/winsup/cygwin/fhandler/pty.cc
> @@ -2266,28 +2266,65 @@ fhandler_pty_master::write (const void *ptr, size_t len)
> { /* Reaches here when non-cygwin app is foreground and pseudo console
> is activated. */
> tmp_pathbuf tp;
> - char *buf = (char *) ptr;
> + char *buf = tp.c_get ();
> size_t nlen = len;
> if (get_ttyp ()->term_code_page != CP_UTF8)
> {
> static mbstate_t mbp;
> - buf = tp.c_get ();
> nlen = NT_MAX_PATH;
> convert_mb_str (CP_UTF8, buf, &nlen,
> get_ttyp ()->term_code_page, (const char *) ptr, len,
> &mbp);
> }
> + else
> + memcpy (buf, ptr, nlen);
> +
> + /* Retrieve console mode */
> + HANDLE h_pcon_in = get_ttyp ()->h_pcon_in;
> + DWORD cons_mode;
> + if (!nat_pipe_owner_self (get_ttyp ()->nat_pipe_owner_pid))
> + {
> + HANDLE pcon_owner = OpenProcess (PROCESS_DUP_HANDLE, FALSE,
> + get_ttyp ()->nat_pipe_owner_pid);
> + DuplicateHandle (pcon_owner, h_pcon_in,
> + GetCurrentProcess (), &h_pcon_in,
> + 0, FALSE, DUPLICATE_SAME_ACCESS);
> + CloseHandle(pcon_owner);
Two issues here.
First, a correctness issue: there is no NULL check on the `OpenProcess()`
return value. If the owner process exited between reading
`nat_pipe_owner_pid` from shared memory and calling `OpenProcess()`, then
`pcon_owner` is NULL, `DuplicateHandle(NULL, ...)` will fail, `h_pcon_in`
will have an undefined value, and `GetConsoleMode()` below will read
garbage (or crash). This needs at minimum a NULL check on `pcon_owner`,
with a fallback path.
Second, a performance concern: this entire sequence (`OpenProcess()` +
`DuplicateHandle()` + `attach_console_temporarily()` + `GetConsoleMode()`
+ `resume_from_temporarily_attach()` + `CloseHandle()`) runs on _every_
call to master::write() in the fast path, even when no 0x08 byte is
present in the buffer. The console mode retrieval should either be moved
inside the conversion loop (only executed when a 0x08 byte is actually
encountered), or cached (although caching the value might run afoul of
time of check vs time of use ("TOCTOU") issues).
> + DWORD resume_pid =
> + attach_console_temporarily (get_ttyp()->nat_pipe_owner_pid);
> + GetConsoleMode (h_pcon_in, &cons_mode);
> + resume_from_temporarily_attach (resume_pid);
> + CloseHandle (h_pcon_in);
> + }
> + else
> + GetConsoleMode (h_pcon_in, &cons_mode);
>
> - for (size_t i = 0; i < nlen; i++)
> + for (size_t i = 0, j = 0; i < nlen; i++)
> {
> process_sig_state r = process_sigs (buf[i], get_ttyp (), this);
> - if (r == done_with_debugger)
> + if (r != done_with_debugger)
> {
> - for (size_t j = i; j < nlen - 1; j++)
> - buf[j] = buf[j + 1];
> - nlen--;
> - i--;
> + char c = buf[i];
> + if (!(cons_mode & ENABLE_VIRTUAL_TERMINAL_INPUT))
> + /* Workaround for pseudo console in Windows 11 */
> + /* Undesired backspace conversion in pseudo console does
> + not happen if ENABLE_VIRTUAL_TERMINAL_INPUT is set. */
> + switch (c)
> + {
> + case '\010': /* Ctrl-H */
> + c = '\177'; /* Backspace */
> + break;
> + case '\177': /* Backspace */
> +#if 0 /* Unfortunately, Ctrl-H will be translated into Ctrl-Backspace
> + (not Backspace) */
> + c = '\010'; /* Ctrl-H */
> +#endif
The `#if 0` dead code should either be removed entirely or replaced with
a comment explaining _why_ the reverse mapping cannot work (and ideally
referencing the conhost bug, so a future reader knows when it might
become safe to revisit). Dead code guarded by `#if 0` without a tracking
reference is confusing for future readers (which might very well be me).
> + break;
> + }
> + buf[j++] = c;
> }
> + else
> + nlen--;
> }
>
> DWORD n;
> @@ -4031,6 +4068,10 @@ fhandler_pty_slave::transfer_input (tty::xfer_dir dir, HANDLE from, tty *ttyp,
> if (r[i].EventType == KEY_EVENT && r[i].Event.KeyEvent.bKeyDown)
> {
> DWORD ctrl_key_state = r[i].Event.KeyEvent.dwControlKeyState;
> + if (r[i].Event.KeyEvent.uChar.AsciiChar == '\010' /* Ctrl-H */
> + && !(ctrl_key_state & ALT_PRESSED))
> + /* Workaround for pseudo console in Windows 11 */
> + r[i].Event.KeyEvent.uChar.AsciiChar = '\177'; /* Backspace */
This is the corresponding fix in `transfer_input()`: When reading
INPUT_RECORDs back from the console buffer in `transfer_input()`, they
have already been through conhost's buggy reverse path, so the 0x08 needs
to be mapped back to 0x7F here too. Consistent with the pre-conversion in
`master::write()`.
One open question that I already asked in
https://inbox.sourceware.org/cygwin-patches/c4dc071d-fa7e-ed2e-0c14-3fddb5240f1c@gmx.de/
but have not yet received an answer to: how was this bug originally
discovered/reproduced? I still do not know how to trigger the Backspace
problem in a regular Windows Terminal running on OpenConsole.exe, and I
need a concrete repro to test the upstream fix I prepared.
Thanks,
Johannes
> if (r[i].Event.KeyEvent.uChar.AsciiChar)
> {
> if ((ctrl_key_state & ALT_PRESSED)
> --
> 2.51.0
>
>
More information about the Cygwin-patches
mailing list