Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.
On Wed, 4 Sep 2019 12:34:31 +0900 Takashi Yano wrote: > Attached is the raw output from pseudo console when the screen shows > the simple text below. > > from here > [yano@Express5800-S70 ~]$ cmd > Microsoft Windows [Version 10.0.18362.329] > (c) 2019 Microsoft Corporation. All rights reserved. > > C:\cygwin\home\yano>exit > [yano@Express5800-S70 ~]$ exit > exit > to here > > You will noticed that the screen is cleared if you execute > cat pcon-output.log > in a terminal which support ANSI escape sequences. This pcon-output.log was recorded in screen of 80x24 size. Please try above in 80x24 terminal. -- Takashi Yano
Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.
Hi Brian, On Tue, 3 Sep 2019 20:47:14 -0600 Brian Inglis wrote: > On 2019-09-03 19:46, Takashi Yano wrote: > > - Pseudo console support introduced by commit > > 169d65a5774acc76ce3f3feeedcbae7405aa9b57 shows garbage ^[[H^[[J in > > some of emacs screens. These screens do not handle ANSI escape > > sequences. Therefore, clear screen is disabled on these screens. > > Dealing with escape sequences is way out of the scope of any pty driver. > It is up to the terminal emulator or applications running in the terminal to > handle terminal characteristics appropriately. > > The pty driver should not touch *ANY* escape sequences coming from the system, > nor should it generate any to it, as TERM may not be set at the time, or > appropriately or usefully in some shells e.g. cmd or powershell. > > Most folks probably use mintty or cmd as their Cygwin terminal, but some use > other terminals, like various flavours of xterm and rxvt, with ssh sessions in > and out, so they could be on Linux consoles or proprietary AIX/HP-UX/Sun > terminals, and operate properly as long as they have a good terminfo > definition. > > I see this issue as similar to the Windows text file handling changes required > when coreutils/textutils went POSIX and removed '\r\n' crlf handling to give > the > same results as on Unix systems. > To handle terminal characteristics properly would require terminfo support in > the pty driver: I doubt anyone wants that, so the best approach is to do > nothing, and let the terminal or application handle it: they are more likely > to > have the configuration options or hooks to do so easily. You are definitely right. However, the essence of the problem is that the pseudo console itself outputs a lot of ANSI escape sequences even if client program output only ASCII characters. Attached is the raw output from pseudo console when the screen shows the simple text below. from here [yano@Express5800-S70 ~]$ cmd Microsoft Windows [Version 10.0.18362.329] (c) 2019 Microsoft Corporation. All rights reserved. C:\cygwin\home\yano>exit [yano@Express5800-S70 ~]$ exit exit to here You will noticed that the screen is cleared if you execute cat pcon-output.log in a terminal which support ANSI escape sequences. So clearing the screen when creating pseudo console is the result of compromise to synchronize real screen and screen buffer of pseudo console. -- Takashi Yano pcon-output.log Description: Binary data
Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.
On 2019-09-03 19:46, Takashi Yano wrote: > - Pseudo console support introduced by commit > 169d65a5774acc76ce3f3feeedcbae7405aa9b57 shows garbage ^[[H^[[J in > some of emacs screens. These screens do not handle ANSI escape > sequences. Therefore, clear screen is disabled on these screens. Dealing with escape sequences is way out of the scope of any pty driver. It is up to the terminal emulator or applications running in the terminal to handle terminal characteristics appropriately. The pty driver should not touch *ANY* escape sequences coming from the system, nor should it generate any to it, as TERM may not be set at the time, or appropriately or usefully in some shells e.g. cmd or powershell. Most folks probably use mintty or cmd as their Cygwin terminal, but some use other terminals, like various flavours of xterm and rxvt, with ssh sessions in and out, so they could be on Linux consoles or proprietary AIX/HP-UX/Sun terminals, and operate properly as long as they have a good terminfo definition. I see this issue as similar to the Windows text file handling changes required when coreutils/textutils went POSIX and removed '\r\n' crlf handling to give the same results as on Unix systems. To handle terminal characteristics properly would require terminfo support in the pty driver: I doubt anyone wants that, so the best approach is to do nothing, and let the terminal or application handle it: they are more likely to have the configuration options or hooks to do so easily. -- Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada This email may be disturbing to some readers as it contains too much technical detail. Reader discretion is advised.
Re: [PATCH v4 1/1] Cygwin: pty: Fix state management for pseudo console support.
Hi Corinna, I have posted several patches for PTY with pseudo console support. Please apply them in the following order. [PATCH 1/4] Cygwin: pty: Code cleanup - Cleanup the code which is commented out by #if 0 regarding pseudo console. - Remove #if 1 for experimental code which seems to be stable. [PATCH 2/4] Cygwin: pty: Speed up a little hooked Win32 API for pseudo console. - Some Win32 APIs are hooked in pty code for pseudo console support. This causes slow down. This patch improves speed a little. [PATCH 3/4] Cygwin: pty: Move function hook_api() into hookapi.cc. - PTY uses Win32 API hook for pseudo console suppot. The function hook_api() is used for this purpose and defined in fhandler_tty.cc previously. This patch moves it into hookapi.cc. [PATCH 4/4] Cygwin: pty: Limit API hook to the program linked with the APIs. - API hook used for pseudo console support causes slow down. This patch limits API hook to only program which is linked with the corresponding APIs. Normal cygwin program is not linked with such APIs (such as WriteFile, etc...) directly, therefore, no slow down occurs. However, console access by cygwin.dll itself cannot switch the r/w pipe to pseudo console side. Therefore, the code to switch it forcely to pseudo console side is added to smallprint.cc and strace.cc. [PATCH v5 1/1] Cygwin: pty: Fix state management for pseudo console support. - Pseudo console support introduced by commit 169d65a5774acc76ce3f3feeedcbae7405aa9b57 has some bugs which cause mismatch between state variables and real pseudo console state regarding console attaching and r/w pipe switching. This patch fixes this issue by redesigning the state management. This hopefully fixes the problem 3 in https://cygwin.com/ml/cygwin/2019-08/msg00401.html This also fixes the first problem regarding "Bad file descriptor" error reported in https://cygwin.com/ml/cygwin-patches/2019-q3/msg00104.html [PATCH 1/2] Cygwin: pty: Add a workaround for ^C handling. - Pseudo console support introduced by commit 169d65a5774acc76ce3f3feeedcbae7405aa9b57 sometimes cause random crash or freeze by pressing ^C while cygwin and non-cygwin processes are executed simultaneously in the same pty. This patch is a workaround for this issue. [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*. - Pseudo console support introduced by commit 169d65a5774acc76ce3f3feeedcbae7405aa9b57 shows garbage ^[[H^[[J in some of emacs screens. These screens do not handle ANSI escape sequences. Therefore, clear screen is disabled on these screens. This fixes the second problem on emacs reported in https://cygwin.com/ml/cygwin-patches/2019-q3/msg00104.html On Tue, 3 Sep 2019 11:16:38 +0200 Corinna Vinschen wrote: > This is a slowdown of about 15%. That's quite a lot. Can't you just > check the incoming handle against the interesting handles somehow? > If there's no other way around it, we should at least make sure (in a > separate patch) that Cygwin calls NtReadFile/NtWriteFile throughout, > except in tty and console code. I came up with an idea, and implemented it. As described obove, Win32 APIs are not hooked any more in normal cygwin process. Hook is done only if the program is directly linked with corresponding APIs. However, this strategy does not have the effect for console access by cygwin1.dll itself. So, to switch r/w pipe to pseudo console side, I added the code in strace.cc and smallprint.cc. Could you please have a look? -- Takashi Yano
[PATCH 0/2] Some fixes for PTY with pseudo console support (2)
[PATCH 1/2] Cygwin: pty: Add a workaround for ^C handling. - Pseudo console support introduced by commit 169d65a5774acc76ce3f3feeedcbae7405aa9b57 sometimes cause random crash or freeze by pressing ^C while cygwin and non-cygwin processes are executed simultaneously in the same pty. This patch is a workaround for this issue. [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*. - Pseudo console support introduced by commit 169d65a5774acc76ce3f3feeedcbae7405aa9b57 shows garbage ^[[H^[[J in some of emacs screens. These screens do not handle ANSI escape sequences. Therefore, clear screen is disabled on these screens. Takashi Yano (2): Cygwin: pty: Add a workaround for ^C handling. Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*. winsup/cygwin/fhandler_tty.cc | 31 --- winsup/cygwin/spawn.cc| 6 ++ winsup/cygwin/tty.cc | 1 + winsup/cygwin/tty.h | 1 + 4 files changed, 32 insertions(+), 7 deletions(-) -- 2.21.0
[PATCH 1/2] Cygwin: pty: Add a workaround for ^C handling.
- Pseudo console support introduced by commit
169d65a5774acc76ce3f3feeedcbae7405aa9b57 sometimes cause random
crash or freeze by pressing ^C while cygwin and non-cygwin
processes are executed simultaneously in the same pty. This
patch is a workaround for this issue.
---
winsup/cygwin/fhandler_tty.cc | 5 +
winsup/cygwin/spawn.cc| 6 ++
2 files changed, 11 insertions(+)
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 240ee017c..283558985 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -2846,6 +2846,9 @@ fhandler_pty_slave::fixup_after_fork (HANDLE parent)
// fork_fixup (parent, inuse, "inuse");
// fhandler_pty_common::fixup_after_fork (parent);
report_tty_counts (this, "inherited", "");
+ if (getPseudoConsole ())
+myself->ctty = 0; /* Avoid setting init_console_handler() in fork.cc.
+This is a workaround for ^C handling problem. */
}
void
@@ -2882,6 +2885,8 @@ fhandler_pty_slave::fixup_after_exec ()
FreeConsole ();
}
}
+ if (getPseudoConsole () && myself->ctty == 0)
+myself->ctty = get_ttyp ()->ntty; /* Restore ctty */
#if USE_API_HOOK
/* Hook Console API */
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 4bb28c47b..22dafbce3 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -634,6 +634,12 @@ child_info_spawn::worker (const char *prog_arg, const char
*const *argv,
}
if (ptys)
ptys->fixup_after_attach (!iscygwin ());
+ if (attach_to_pcon && !iscygwin ())
+ {
+ myself->ctty = 0; /* Random freezes caused by ^C can be avoided
+ with this. */
+ init_console_handler (true);
+ }
loop:
/* When ruid != euid we create the new process under the current original
--
2.21.0
[PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.
- Pseudo console support introduced by commit
169d65a5774acc76ce3f3feeedcbae7405aa9b57 shows garbage ^[[H^[[J in
some of emacs screens. These screens do not handle ANSI escape
sequences. Therefore, clear screen is disabled on these screens.
---
winsup/cygwin/fhandler_tty.cc | 26 +++---
winsup/cygwin/tty.cc | 1 +
winsup/cygwin/tty.h | 1 +
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 283558985..a74c3eecf 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -972,6 +972,19 @@ skip_console_setting:
void
fhandler_pty_slave::reset_switch_to_pcon (void)
{
+ if (get_ttyp ()->need_clear_screen)
+{
+ const char *term = getenv ("TERM");
+ if (term && strcmp (term, "dumb") && !strstr (term, "emacs"))
+ {
+ /* FIXME: Clearing sequence may not be "^[[H^[[J"
+depending on the terminal type. */
+ DWORD n;
+ WriteFile (get_output_handle_cyg (), "\033[H\033[J", 6, , NULL);
+ }
+ get_ttyp ()->need_clear_screen = false;
+}
+
if (ALWAYS_USE_PCON)
return;
if (isHybrid)
@@ -2808,14 +2821,13 @@ fhandler_pty_slave::fixup_after_attach (bool
native_maybe)
/* Clear screen to synchronize pseudo console screen buffer
with real terminal. This is necessary because pseudo
console screen buffer is empty at start. */
- /* FIXME: Clearing sequence may not be "^[[H^[[J"
-depending on the terminal type. */
- DWORD n;
- if (get_ttyp ()->num_pcon_attached_slaves == 0
- && !ALWAYS_USE_PCON)
+ const char *term = getenv ("TERM");
+ if (get_ttyp ()->num_pcon_attached_slaves == 0 &&
+ term && strcmp (term, "dumb") &&
+ term && !strstr (term, "emacs") &&
+ !ALWAYS_USE_PCON)
/* Assume this is the first process using this pty slave. */
- WriteFile (get_output_handle_cyg (),
- "\033[H\033[J", 6, , NULL);
+ get_ttyp ()->need_clear_screen = true;
get_ttyp ()->num_pcon_attached_slaves ++;
}
diff --git a/winsup/cygwin/tty.cc b/winsup/cygwin/tty.cc
index 9244267c0..c94aee3ba 100644
--- a/winsup/cygwin/tty.cc
+++ b/winsup/cygwin/tty.cc
@@ -243,6 +243,7 @@ tty::init ()
pcon_pid = 0;
num_pcon_attached_slaves = 0;
TermCodePage = 20127; /* ASCII */
+ need_clear_screen = false;
}
HANDLE
diff --git a/winsup/cygwin/tty.h b/winsup/cygwin/tty.h
index d59b2027d..c2b0490d0 100644
--- a/winsup/cygwin/tty.h
+++ b/winsup/cygwin/tty.h
@@ -104,6 +104,7 @@ private:
pid_t pcon_pid;
int num_pcon_attached_slaves;
UINT TermCodePage;
+ bool need_clear_screen;
public:
HANDLE from_master () const { return _from_master; }
--
2.21.0
[PATCH v5 1/1] Cygwin: pty: Fix state management for pseudo console support.
- Pseudo console support introduced by commit
169d65a5774acc76ce3f3feeedcbae7405aa9b57 has some bugs which
cause mismatch between state variables and real pseudo console
state regarding console attaching and r/w pipe switching. This
patch fixes this issue by redesigning the state management.
---
winsup/cygwin/dtable.cc | 38 +--
winsup/cygwin/fhandler.h | 6 +-
winsup/cygwin/fhandler_console.cc | 25 +-
winsup/cygwin/fhandler_tty.cc | 385 --
winsup/cygwin/fork.cc | 24 +-
winsup/cygwin/spawn.cc| 65 ++---
6 files changed, 289 insertions(+), 254 deletions(-)
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index ba5d16206..4e9b6ed56 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -147,18 +147,16 @@ dtable::get_debugger_info ()
void
dtable::stdio_init ()
{
- bool need_fixup_handle = false;
- fhandler_pty_slave *ptys = NULL;
- bool is_pty[3] = {false, false, false};
- for (int fd = 0; fd < 3; fd ++)
+ int chk_order[] = {1, 0, 2};
+ for (int i = 0; i < 3; i ++)
{
+ int fd = chk_order[i];
fhandler_base *fh = cygheap->fdtab[fd];
if (fh && fh->get_major () == DEV_PTYS_MAJOR)
{
- ptys = (fhandler_pty_slave *) fh;
+ fhandler_pty_slave *ptys = (fhandler_pty_slave *) fh;
if (ptys->getPseudoConsole ())
{
- is_pty[fd] = true;
bool attached = !!fhandler_console::get_console_process_id
(ptys->getHelperProcessId (), true);
if (!attached)
@@ -167,15 +165,12 @@ dtable::stdio_init ()
by some reason. This happens if the executable is
a windows GUI binary, such as mintty. */
FreeConsole ();
- AttachConsole (ptys->getHelperProcessId ());
- need_fixup_handle = true;
+ if (AttachConsole (ptys->getHelperProcessId ()))
+ break;
}
- ptys->reset_switch_to_pcon ();
}
}
}
- if (need_fixup_handle)
-goto fixup_handle;
if (myself->cygstarted || ISSTATE (myself, PID_CYGPARENT))
{
@@ -185,27 +180,6 @@ dtable::stdio_init ()
return;
}
-fixup_handle:
- if (need_fixup_handle)
-{
- HANDLE h;
- h = CreateFile ("CONIN$", GENERIC_READ, FILE_SHARE_READ,
- NULL, OPEN_EXISTING, 0, 0);
- if (is_pty[0])
- {
- SetStdHandle (STD_INPUT_HANDLE, h);
- ptys->set_handle (h);
- }
- h = CreateFile ("CONOUT$", GENERIC_WRITE, FILE_SHARE_WRITE,
- NULL, OPEN_EXISTING, 0, 0);
- if (is_pty[1])
- SetStdHandle (STD_OUTPUT_HANDLE, h);
- if (is_pty[2])
- SetStdHandle (STD_ERROR_HANDLE, h);
- if (is_pty[1] || is_pty[2])
- ptys->set_output_handle (h);
-}
-
HANDLE in = GetStdHandle (STD_INPUT_HANDLE);
HANDLE out = GetStdHandle (STD_OUTPUT_HANDLE);
HANDLE err = GetStdHandle (STD_ERROR_HANDLE);
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index c75e40c0a..e8c165100 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2106,19 +2106,22 @@ class fhandler_pty_common: public fhandler_termios
protected:
BOOL process_opost_output (HANDLE h,
const void *ptr, ssize_t& len, bool is_echo);
- bool check_switch_to_pcon (void);
};
class fhandler_pty_slave: public fhandler_pty_common
{
HANDLE inuse;// used to indicate that a tty is in use
HANDLE output_handle_cyg, io_handle_cyg;
+ DWORD pid_restore;
/* Helper functions for fchmod and fchown. */
bool fch_open_handles (bool chown);
int fch_set_sd (security_descriptor , bool chown);
void fch_close_handles ();
+ bool try_reattach_pcon ();
+ void restore_reattach_pcon ();
+
public:
/* Constructor */
fhandler_pty_slave (int);
@@ -2172,7 +2175,6 @@ class fhandler_pty_slave: public fhandler_pty_common
void set_switch_to_pcon (void);
void reset_switch_to_pcon (void);
void push_to_pcon_screenbuffer (const char *ptr, size_t len);
- bool has_master_opened (void);
void mask_switch_to_pcon (bool mask)
{
get_ttyp ()->mask_switch_to_pcon = mask;
diff --git a/winsup/cygwin/fhandler_console.cc
b/winsup/cygwin/fhandler_console.cc
index 997c50d23..1b034f4b9 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -3136,16 +3136,29 @@ DWORD
fhandler_console::get_console_process_id (DWORD pid, bool match)
{
DWORD tmp;
- int num = GetConsoleProcessList (, 1);
- DWORD *list = (DWORD *)
- HeapAlloc (GetProcessHeap (), 0, num * sizeof (DWORD));
- num = GetConsoleProcessList (list, num);
+ DWORD num, num_req;
+ num = 1;
+ num_req = GetConsoleProcessList (, num);
+ DWORD *list;
+ while (true)
+{
+ list = (DWORD *)
+ HeapAlloc
[PATCH v5 0/1] Fix PTY state management in pseudo console support.
Pseudo console support in test release TEST: Cygwin 3.1.0-0.3, introduced by commit 169d65a5774acc76ce3f3feeedcbae7405aa9b57, has some bugs which cause mismatch between state variables and real pseudo console state regarding console attaching and r/w pipe switching. This patch fixes this issue by redesigning the state management. v5: Revise based on https://cygwin.com/ml/cygwin-patches/2019-q3/msg00111.html v4: Small bug fix again. v3: Fix the first issue (Bad file descriptor) reported in https://cygwin.com/ml/cygwin-patches/2019-q3/msg00104.html v2: Small bug fixed from v1. Takashi Yano (1): Cygwin: pty: Fix state management for pseudo console support. winsup/cygwin/dtable.cc | 38 +-- winsup/cygwin/fhandler.h | 6 +- winsup/cygwin/fhandler_console.cc | 25 +- winsup/cygwin/fhandler_tty.cc | 385 -- winsup/cygwin/fork.cc | 24 +- winsup/cygwin/spawn.cc| 65 ++--- 6 files changed, 289 insertions(+), 254 deletions(-) -- 2.21.0
[PATCH 4/4] Cygwin: pty: Limit API hook to the program linked with the APIs.
- API hook used for pseudo console support causes slow down.
This patch limits API hook to only program which is linked
with the corresponding APIs. Normal cygwin program is not
linked with such APIs (such as WriteFile, etc...) directly,
therefore, no slow down occurs. However, console access by
cygwin.dll itself cannot switch the r/w pipe to pseudo console
side. Therefore, the code to switch it forcely to pseudo
console side is added to smallprint.cc and strace.cc.
---
winsup/cygwin/fhandler_tty.cc | 60 ---
winsup/cygwin/smallprint.cc | 5 +++
winsup/cygwin/strace.cc | 29 +++--
3 files changed, 51 insertions(+), 43 deletions(-)
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index f76f7b262..3a23083de 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -89,6 +89,18 @@ set_switch_to_pcon (void)
}
}
+void set_ishybrid_and_switch_to_pcon (HANDLE h)
+{
+ DWORD dummy;
+ if (!isHybrid
+ && GetFileType (h) == FILE_TYPE_CHAR
+ && GetConsoleMode (h, ))
+{
+ isHybrid = true;
+ set_switch_to_pcon ();
+}
+}
+
#define DEF_HOOK(name) static __typeof__ (name) *name##_Orig
DEF_HOOK (WriteFile);
DEF_HOOK (WriteConsoleA);
@@ -101,6 +113,7 @@ DEF_HOOK (WriteConsoleOutputW);
DEF_HOOK (WriteConsoleOutputCharacterA);
DEF_HOOK (WriteConsoleOutputCharacterW);
DEF_HOOK (WriteConsoleOutputAttribute);
+DEF_HOOK (SetConsoleTextAttribute);
DEF_HOOK (WriteConsoleInputA);
DEF_HOOK (WriteConsoleInputW);
DEF_HOOK (ReadConsoleInputA);
@@ -197,6 +210,13 @@ WriteConsoleOutputAttribute_Hooked
return WriteConsoleOutputAttribute_Orig (h, a, l, c, n);
}
static BOOL WINAPI
+SetConsoleTextAttribute_Hooked
+ (HANDLE h, WORD a)
+{
+ CHK_CONSOLE_ACCESS (h);
+ return SetConsoleTextAttribute_Orig (h, a);
+}
+static BOOL WINAPI
WriteConsoleInputA_Hooked
(HANDLE h, CONST INPUT_RECORD *r, DWORD l, LPDWORD n)
{
@@ -242,6 +262,7 @@ PeekConsoleInputW_Hooked
#define WriteFile_Orig 0
#define ReadFile_Orig 0
#define PeekConsoleInputA_Orig 0
+void set_ishybrid_and_switch_to_pcon (void) {}
#endif /* USE_API_HOOK */
bool
@@ -2855,25 +2876,26 @@ fhandler_pty_slave::fixup_after_exec ()
{ \
void *api = hook_api (module, #name, (void *) name##_Hooked); \
name##_Orig = (__typeof__ (name) *) api; \
- if (!api) system_printf("Hooking " #name " failed."); \
- }
- DO_HOOK ("kernel32.dll", WriteFile);
- DO_HOOK ("kernel32.dll", WriteConsoleA);
- DO_HOOK ("kernel32.dll", WriteConsoleW);
- DO_HOOK ("kernel32.dll", ReadFile);
- DO_HOOK ("kernel32.dll", ReadConsoleA);
- DO_HOOK ("kernel32.dll", ReadConsoleW);
- DO_HOOK ("kernel32.dll", WriteConsoleOutputA);
- DO_HOOK ("kernel32.dll", WriteConsoleOutputW);
- DO_HOOK ("kernel32.dll", WriteConsoleOutputCharacterA);
- DO_HOOK ("kernel32.dll", WriteConsoleOutputCharacterW);
- DO_HOOK ("kernel32.dll", WriteConsoleOutputAttribute);
- DO_HOOK ("kernel32.dll", WriteConsoleInputA);
- DO_HOOK ("kernel32.dll", WriteConsoleInputW);
- DO_HOOK ("kernel32.dll", ReadConsoleInputA);
- DO_HOOK ("kernel32.dll", ReadConsoleInputW);
- DO_HOOK ("kernel32.dll", PeekConsoleInputA);
- DO_HOOK ("kernel32.dll", PeekConsoleInputW);
+ /*if (api) system_printf(#name " hooked.");*/ \
+ }
+ DO_HOOK (NULL, WriteFile);
+ DO_HOOK (NULL, WriteConsoleA);
+ DO_HOOK (NULL, WriteConsoleW);
+ DO_HOOK (NULL, ReadFile);
+ DO_HOOK (NULL, ReadConsoleA);
+ DO_HOOK (NULL, ReadConsoleW);
+ DO_HOOK (NULL, WriteConsoleOutputA);
+ DO_HOOK (NULL, WriteConsoleOutputW);
+ DO_HOOK (NULL, WriteConsoleOutputCharacterA);
+ DO_HOOK (NULL, WriteConsoleOutputCharacterW);
+ DO_HOOK (NULL, WriteConsoleOutputAttribute);
+ DO_HOOK (NULL, SetConsoleTextAttribute);
+ DO_HOOK (NULL, WriteConsoleInputA);
+ DO_HOOK (NULL, WriteConsoleInputW);
+ DO_HOOK (NULL, ReadConsoleInputA);
+ DO_HOOK (NULL, ReadConsoleInputW);
+ DO_HOOK (NULL, PeekConsoleInputA);
+ DO_HOOK (NULL, PeekConsoleInputW);
}
#endif /* USE_API_HOOK */
}
diff --git a/winsup/cygwin/smallprint.cc b/winsup/cygwin/smallprint.cc
index a7a19132b..bd19c1f5f 100644
--- a/winsup/cygwin/smallprint.cc
+++ b/winsup/cygwin/smallprint.cc
@@ -384,6 +384,9 @@ __small_sprintf (char *dst, const char *fmt, ...)
return r;
}
+/* Defined in fhandler_tty.cc */
+extern void set_ishybrid_and_switch_to_pcon (HANDLE);
+
void
small_printf (const char *fmt, ...)
{
@@ -405,6 +408,7 @@ small_printf (const char *fmt, ...)
count = __small_vsprintf (buf, fmt, ap);
va_end (ap);
+ set_ishybrid_and_switch_to_pcon (GetStdHandle (STD_ERROR_HANDLE));
WriteFile (GetStdHandle (STD_ERROR_HANDLE), buf, count, , NULL);
FlushFileBuffers (GetStdHandle (STD_ERROR_HANDLE));
}
@@ -431,6 +435,7 @@ console_printf (const char
[PATCH 3/4] Cygwin: pty: Move function hook_api() into hookapi.cc.
- PTY uses Win32 API hook for pseudo console suppot. The function
hook_api() is used for this purpose and defined in fhandler_tty.cc
previously. This patch moves it into hookapi.cc.
---
winsup/cygwin/fhandler_tty.cc | 44 ---
winsup/cygwin/hookapi.cc | 34 +++
winsup/cygwin/winsup.h| 1 +
3 files changed, 35 insertions(+), 44 deletions(-)
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 94ef2f8d4..f76f7b262 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -75,50 +75,6 @@ static bool pcon_attached[NTTYS];
static bool isHybrid;
#if USE_API_HOOK
-/* Hook WIN32 API */
-static
-void *hook_api (const char *mname, const char *name, const void *fn)
-{
- HMODULE hm = GetModuleHandle (mname);
- PIMAGE_NT_HEADERS pExeNTHdr = PIMAGE_NT_HEADERS (PBYTE (hm)
- + PIMAGE_DOS_HEADER (hm)->e_lfanew);
- DWORD importRVA = pExeNTHdr->OptionalHeader.DataDirectory
-[IMAGE_DIRECTORY_ENTRY_IMPORT].VirtualAddress;
- PIMAGE_IMPORT_DESCRIPTOR pdfirst =
-(PIMAGE_IMPORT_DESCRIPTOR) ((char *) hm + importRVA);
- for (PIMAGE_IMPORT_DESCRIPTOR pd = pdfirst; pd->FirstThunk; pd++)
-{
- if (pd->OriginalFirstThunk == 0)
- continue;
- PIMAGE_THUNK_DATA pt =
- (PIMAGE_THUNK_DATA) ((char *) hm + pd->FirstThunk);
- PIMAGE_THUNK_DATA pn =
- (PIMAGE_THUNK_DATA) ((char *) hm + pd->OriginalFirstThunk);
- for (PIMAGE_THUNK_DATA pi = pt; pn->u1.Ordinal; pi++, pn++)
- {
- if (IMAGE_SNAP_BY_ORDINAL (pn->u1.Ordinal))
- continue;
- PIMAGE_IMPORT_BY_NAME pimp =
- (PIMAGE_IMPORT_BY_NAME) ((char *) hm + pn->u1.AddressOfData);
- if (strcmp (name, (char *) pimp->Name) != 0)
- continue;
-#ifdef __x86_64__
-#define THUNK_FUNC_TYPE ULONGLONG
-#else
-#define THUNK_FUNC_TYPE DWORD
-#endif
- DWORD ofl = PAGE_READWRITE;
- if (!VirtualProtect (pi, sizeof (THUNK_FUNC_TYPE), ofl, ))
- return NULL;
- void *origfn = (void *) pi->u1.Function;
- pi->u1.Function = (THUNK_FUNC_TYPE) fn;
- VirtualProtect (pi, sizeof (THUNK_FUNC_TYPE), ofl, );
- return origfn;
- }
-}
- return NULL;
-}
-
static void
set_switch_to_pcon (void)
{
diff --git a/winsup/cygwin/hookapi.cc b/winsup/cygwin/hookapi.cc
index 4078e65bd..dcd9b1df8 100644
--- a/winsup/cygwin/hookapi.cc
+++ b/winsup/cygwin/hookapi.cc
@@ -428,6 +428,40 @@ hook_or_detect_cygwin (const char *name, const void *fn,
WORD& subsys, HANDLE h)
return fh.origfn;
}
+/* Hook a function in any DLL such as kernel32.dll */
+/* The DLL must be loaded in advance. */
+/* Used in fhandler_tty.cc */
+void *hook_api (const char *mname, const char *name, const void *fn)
+{
+ HMODULE hm = GetModuleHandle (mname);
+ PIMAGE_NT_HEADERS pExeNTHdr =
+rva (PIMAGE_NT_HEADERS, hm, PIMAGE_DOS_HEADER (hm)->e_lfanew);
+ DWORD importRVA = pExeNTHdr->OptionalHeader.DataDirectory
+[IMAGE_DIRECTORY_ENTRY_IMPORT].VirtualAddress;
+ PIMAGE_IMPORT_DESCRIPTOR pdfirst =
+rva (PIMAGE_IMPORT_DESCRIPTOR, hm, importRVA);
+ for (PIMAGE_IMPORT_DESCRIPTOR pd = pdfirst; pd->FirstThunk; pd++)
+{
+ if (pd->OriginalFirstThunk == 0)
+ continue;
+ PIMAGE_THUNK_DATA pt = rva (PIMAGE_THUNK_DATA, hm, pd->FirstThunk);
+ PIMAGE_THUNK_DATA pn =
+ rva (PIMAGE_THUNK_DATA, hm, pd->OriginalFirstThunk);
+ for (PIMAGE_THUNK_DATA pi = pt; pn->u1.Ordinal; pi++, pn++)
+ {
+ if (IMAGE_SNAP_BY_ORDINAL (pn->u1.Ordinal))
+ continue;
+ PIMAGE_IMPORT_BY_NAME pimp =
+ rva (PIMAGE_IMPORT_BY_NAME, hm, pn->u1.AddressOfData);
+ if (strcmp (name, (char *) pimp->Name) != 0)
+ continue;
+ void *origfn = putmem (pi, fn);
+ return origfn;
+ }
+}
+ return NULL;
+}
+
void
ld_preload ()
{
diff --git a/winsup/cygwin/winsup.h b/winsup/cygwin/winsup.h
index 95ab41e6b..ab7b3bbdc 100644
--- a/winsup/cygwin/winsup.h
+++ b/winsup/cygwin/winsup.h
@@ -199,6 +199,7 @@ ino_t __reg2 hash_path_name (ino_t hash, const char *name);
void __reg2 nofinalslash (const char *src, char *dst);
void __reg3 *hook_or_detect_cygwin (const char *, const void *, WORD&, HANDLE
h = NULL);
+void __reg3 *hook_api (const char *mname, const char *name, const void *fn);
/* Time related */
void __stdcall totimeval (struct timeval *, PLARGE_INTEGER, int, int);
--
2.21.0
[PATCH 1/4] Cygwin: pty: Code cleanup
- Cleanup the code which is commented out by #if 0 regarding pseudo
console.
- Remove #if 1 for experimental code which seems to be stable.
---
winsup/cygwin/fhandler_tty.cc | 28
1 file changed, 28 deletions(-)
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index dd5ab528a..4dbe96b4a 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -855,26 +855,6 @@ fhandler_pty_slave::cleanup ()
int
fhandler_pty_slave::close ()
{
-#if 0
- if (getPseudoConsole ())
-{
- INPUT_RECORD inp[128];
- DWORD n;
- PeekFunc =
- PeekConsoleInputA_Orig ? PeekConsoleInputA_Orig : PeekConsoleInput;
- PeekFunc (get_handle (), inp, 128, );
- bool pipe_empty = true;
- while (n-- > 0)
- if (inp[n].EventType == KEY_EVENT && inp[n].Event.KeyEvent.bKeyDown)
- pipe_empty = false;
- if (pipe_empty)
- {
- /* Flush input buffer */
- size_t len = UINT_MAX;
- read (NULL, len);
- }
-}
-#endif
termios_printf ("closing last open %s handle", ttyname ());
if (inuse && !CloseHandle (inuse))
termios_printf ("CloseHandle (inuse), %E");
@@ -1524,7 +1504,6 @@ fhandler_pty_slave::read (void *ptr, size_t& len)
out:
termios_printf ("%d = read(%p, %lu)", totalread, ptr, len);
len = (size_t) totalread;
-#if 1 /* Experimenta code */
/* Push slave read as echo to pseudo console screen buffer. */
if (getPseudoConsole () && ptr0 && (get_ttyp ()->ti.c_lflag & ECHO))
{
@@ -1532,7 +1511,6 @@ out:
push_to_pcon_screenbuffer (ptr0, len);
release_output_mutex ();
}
-#endif
mask_switch_to_pcon (false);
}
@@ -2748,10 +2726,6 @@ restart:
if (p)
*p = L'-';
LCID lcid = LocaleNameToLCID (lc, 0);
-#if 0
- if (lcid == (LCID) -1)
-return lcid;
-#endif
if (!lcid && !strcmp (charset, "ASCII"))
return 0;
@@ -2842,7 +2816,6 @@ fhandler_pty_slave::fixup_after_attach (bool native_maybe)
}
}
-#if 1 /* Experimental code */
/* Clear screen to synchronize pseudo console screen buffer
with real terminal. This is necessary because pseudo
console screen buffer is empty at start. */
@@ -2854,7 +2827,6 @@ fhandler_pty_slave::fixup_after_attach (bool native_maybe)
/* Assume this is the first process using this pty slave. */
WriteFile (get_output_handle_cyg (),
"\033[H\033[J", 6, , NULL);
-#endif
pcon_attached[get_minor ()] = true;
get_ttyp ()->num_pcon_attached_slaves ++;
--
2.21.0
[PATCH 2/4] Cygwin: pty: Speed up a little hooked Win32 API for pseudo console.
- Some Win32 APIs are hooked in pty code for pseudo console support.
This causes slow down. This patch improves speed a little.
---
winsup/cygwin/fhandler_tty.cc | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 4dbe96b4a..94ef2f8d4 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -155,7 +155,9 @@ DEF_HOOK (PeekConsoleInputW);
#define CHK_CONSOLE_ACCESS(h) \
{ \
DWORD dummy; \
- if (!isHybrid && GetConsoleMode (h, )) \
+ if (!isHybrid \
+ && GetFileType (h) == FILE_TYPE_CHAR \
+ && GetConsoleMode (h, )) \
{ \
isHybrid = true; \
set_switch_to_pcon (); \
--
2.21.0
[PATCH 0/4] Some fixes for PTY with pseudo console support (1)
[PATCH 1/4] Cygwin: pty: Code cleanup - Cleanup the code which is commented out by #if 0 regarding pseudo console. - Remove #if 1 for experimental code which seems to be stable. [PATCH 2/4] Cygwin: pty: Speed up a little hooked Win32 API for pseudo console. - Some Win32 APIs are hooked in pty code for pseudo console support. This causes slow down. This patch improves speed a little. [PATCH 3/4] Cygwin: pty: Move function hook_api() into hookapi.cc. - PTY uses Win32 API hook for pseudo console suppot. The function hook_api() is used for this purpose and defined in fhandler_tty.cc previously. This patch moves it into hookapi.cc. [PATCH 4/4] Cygwin: pty: Limit API hook to the program linked with the APIs. - API hook used for pseudo console support causes slow down. This patch limits API hook to only program which is linked with the corresponding APIs. Normal cygwin program is not linked with such APIs (such as WriteFile, etc...) directly, therefore, no slow down occurs. However, console access by cygwin.dll itself cannot switch the r/w pipe to pseudo console side. Therefore, the code to switch it forcely to pseudo console side is added to smallprint.cc and strace.cc. Takashi Yano (4): Cygwin: pty: Code cleanup Cygwin: pty: Speed up a little hooked Win32 API for pseudo console. Cygwin: pty: Move function hook_api() into hookapi.cc. Cygwin: pty: Limit API hook to the program linked with the APIs. winsup/cygwin/fhandler_tty.cc | 136 +++--- winsup/cygwin/hookapi.cc | 34 + winsup/cygwin/smallprint.cc | 5 ++ winsup/cygwin/strace.cc | 29 ++-- winsup/cygwin/winsup.h| 1 + 5 files changed, 89 insertions(+), 116 deletions(-) -- 2.21.0
Re: [PATCH v4 1/1] Cygwin: pty: Fix state management for pseudo console support.
Hi Takashi,
On Sep 3 12:05, Takashi Yano wrote:
> > > - Sleep (60); /* Wait for pty_master_fwd_thread() */
> > > + Sleep (20); /* Wait for pty_master_fwd_thread() */
> >
> > Isn't that a separate issue as well? A separate patch may be in order
> > here, kind of like "Cygwin: pseudo console: reduce time sleeping ..."
> > with a short description why that makes sense?
>
> Actually it is not. The wait time became able to be reduced by
> redesigning switching of r/w pipes which managed via variable
> switch_to_pcon. So I think this should be included in this patch.
Ok.
> > However, I have a few questions in terms of the code in general, namely
> > in terms of
> >
> > ALWAYS_USE_PCON
> > USE_API_HOOK
> > USE_OWN_NLS_FUNC
> >
> > Can you describe again why you introduced these macros?
>
> These are defined for debugging purpose.
>
> If ALWAYS_USE_PCON is defined to true, pseudo console pipe is used for
> all process including pure cygwin process. Usually, this should be false
> so that the cygwin process use named pipe as previous.
>
> USE_API_HOOK is for enabling/disabling the API hook to detect direct
> console access in cygwin process. This should be true so that the
> r/w pipe switching is set to pseudo console side for the cygwin
> process which directly access console.
>
> As for USE_OWN_NLS_FUNC, I have not decided yet which codes should be
> used. If USE_OWN_NLS_FUNC is false, setlocale (LC_CTYPE, "") is
> called therefore it may affect to some programs wihch do not call
> setlocale().
>
> > In terms of USE_API_HOOK:
> >
> > - Shouldn't the hook_api function be moved to hookapi.cc?
>
> I will move it into hookapi.cc, and post it as a separate patch.
>
> > - Do we really want to hook every invocation of WriteFile/ReadFile?
> > Doesn't that potentially slow down an exec'ed process a lot?
> > We're still not using the NT functions throughout outside of the
> > console/tty code.
>
> I measured the time for calling WriteFile() 100 times writing
> 1 byte to a disk file for each call.
Files are not affected in Cygwin, fortunately. They use
NtReadFile/NtWriteFile, not ReadFile/WriteFile. However, other
stuff is still affected...
> Not hooked:
> Total: 4.558321 seconsd
>
> Hooked:
> Total: 6.522692 seconsd
>
> Hooking causes slow down indeed. It seems that GetConsoleMode()
> is slow. So I have added the check for GetFileType() before
> GetConsoleMode() and made check in two stages.
>
> Hooked (new):
> Total: 5.217996 seconsd
>
> This results in speed up a little. I will post another patch for this.
This is a slowdown of about 15%. That's quite a lot. Can't you just
check the incoming handle against the interesting handles somehow?
If there's no other way around it, we should at least make sure (in a
separate patch) that Cygwin calls NtReadFile/NtWriteFile throughout,
except in tty and console code.
> > In terms of USE_OWN_NLS_FUNC:
> >
> > - Why do we need this function at all? Can't this be handled by
> > __loadlocale instead? If not, what is __loadlocale missing to make
> > this work without duplicating the function?
>
> Calling __loadlocale() here causes execution error.
>
> mintty:
> 0 [main] tcsh 1901 sig_send: error sending signal 6, pid 1901, pipe
> handle 0x0, nb 0, packsize 164, Win32 error 6
>
> script:
> Script started, file is typescript
> script: failed to execute /bin/tcsh: Bad address
> Script done, file is typescript
>
> I could not find out the reason. Some kind of initialization which
> is needed by __loadlocale() may not be done yet. So I copied
> only necessary part from __loadlocale() and nl_langinfo().
>
> Simply,
> path_conv a ("/usr/share/locale/locale.alias");
> also causes errors on starting mintty.
>
> Ideally, the cause of the error should be found out, I suppose.
Indeed. We can keep the code in for now, but the end result should
call a tweaked version of __loadlocale instead. As long as the
tweak only requires a single extra argument, or if the category or
new_locale argument can be used as indicator to trigger the required
special behavour, we should have no problem to get that into newlib.
Thanks,
Corinna
--
Corinna Vinschen
Cygwin Maintainer
signature.asc
Description: PGP signature
