[PATCH 0/5] Some fixes for PTY with pseudo console support (4)

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/5] Some fixes for PTY with pseudo console support (4)

Takashi Yano
Takashi Yano (5):
  Cygwin: pty: Avoid potential segfault in PTY code when ppid = 1.
  Cygwin: pty: Make GDB work again on pty.
  Cygwin: pty: Unify the charset conversion codes into a function.
  Cygwin: pty: Add charset conversion for console apps in legacy PTY.
  Cygwin: pty: Add missing guard when PTY is in the legacy mode.

 winsup/cygwin/fhandler_tty.cc | 188 +++++++++++++++++++---------------
 1 file changed, 104 insertions(+), 84 deletions(-)

--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/5] Cygwin: pty: Avoid potential segfault in PTY code when ppid = 1.

Takashi Yano
---
 winsup/cygwin/fhandler_tty.cc | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 659e7b595..2a1c34f7d 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -137,9 +137,16 @@ force_attach_to_pcon (HANDLE h)
  /* If the process is running on a console,
    the parent process should be attached
    to the same console. */
- pinfo p (myself->ppid);
+ DWORD attach_wpid;
+ if (myself->ppid == 1)
+  attach_wpid = ATTACH_PARENT_PROCESS;
+ else
+  {
+    pinfo p (myself->ppid);
+    attach_wpid = p->dwProcessId;
+  }
  FreeConsole ();
- if (AttachConsole (p->dwProcessId))
+ if (AttachConsole (attach_wpid))
   {
     pcon_attached_to = -1;
     attach_done = true;
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/5] Cygwin: pty: Make GDB work again on pty.

Takashi Yano
In reply to this post by Takashi Yano
---
 winsup/cygwin/fhandler_tty.cc | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 2a1c34f7d..843807aab 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -197,6 +197,9 @@ DEF_HOOK (ReadConsoleInputA);
 DEF_HOOK (ReadConsoleInputW);
 DEF_HOOK (PeekConsoleInputA);
 DEF_HOOK (PeekConsoleInputW);
+/* CreateProcess() is hooked for GDB etc. */
+DEF_HOOK (CreateProcessA);
+DEF_HOOK (CreateProcessW);
 
 static BOOL WINAPI
 WriteFile_Hooked
@@ -331,6 +334,35 @@ PeekConsoleInputW_Hooked
   set_ishybrid_and_switch_to_pcon (h);
   return PeekConsoleInputW_Orig (h, r, l, n);
 }
+/* CreateProcess() is hooked for GDB etc. */
+static BOOL WINAPI
+CreateProcessA_Hooked
+     (LPCSTR n, LPSTR c, LPSECURITY_ATTRIBUTES pa, LPSECURITY_ATTRIBUTES ta,
+      BOOL inh, DWORD f, LPVOID e, LPCSTR d,
+      LPSTARTUPINFOA si, LPPROCESS_INFORMATION pi)
+{
+  HANDLE h;
+  if (si->dwFlags & STARTF_USESTDHANDLES)
+    h = si->hStdOutput;
+  else
+    h = GetStdHandle (STD_OUTPUT_HANDLE);
+  set_ishybrid_and_switch_to_pcon (h);
+  return CreateProcessA_Orig (n, c, pa, ta, inh, f, e, d, si, pi);
+}
+static BOOL WINAPI
+CreateProcessW_Hooked
+     (LPCWSTR n, LPWSTR c, LPSECURITY_ATTRIBUTES pa, LPSECURITY_ATTRIBUTES ta,
+      BOOL inh, DWORD f, LPVOID e, LPCWSTR d,
+      LPSTARTUPINFOW si, LPPROCESS_INFORMATION pi)
+{
+  HANDLE h;
+  if (si->dwFlags & STARTF_USESTDHANDLES)
+    h = si->hStdOutput;
+  else
+    h = GetStdHandle (STD_OUTPUT_HANDLE);
+  set_ishybrid_and_switch_to_pcon (h);
+  return CreateProcessW_Orig (n, c, pa, ta, inh, f, e, d, si, pi);
+}
 #else /* USE_API_HOOK */
 #define WriteFile_Orig 0
 #define ReadFile_Orig 0
@@ -2778,6 +2810,9 @@ fhandler_pty_slave::fixup_after_exec ()
       DO_HOOK (NULL, ReadConsoleInputW);
       DO_HOOK (NULL, PeekConsoleInputA);
       DO_HOOK (NULL, PeekConsoleInputW);
+      /* CreateProcess() is hooked for GDB etc. */
+      DO_HOOK (NULL, CreateProcessA);
+      DO_HOOK (NULL, CreateProcessW);
     }
 #endif /* USE_API_HOOK */
 }
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/5] Cygwin: pty: Unify the charset conversion codes into a function.

Takashi Yano
In reply to this post by Takashi Yano
---
 winsup/cygwin/fhandler_tty.cc | 130 +++++++++++++---------------------
 1 file changed, 49 insertions(+), 81 deletions(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 843807aab..f723ec7cf 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -370,7 +370,43 @@ CreateProcessW_Hooked
 void set_ishybrid_and_switch_to_pcon (HANDLE) {}
 #endif /* USE_API_HOOK */
 
-bool
+static char *
+convert_mb_str (UINT cp_to, size_t *len_to,
+ UINT cp_from, const char *ptr_from, size_t len_from)
+{
+  char *buf;
+  size_t nlen;
+  if (cp_to != cp_from)
+    {
+      size_t wlen =
+ MultiByteToWideChar (cp_from, 0, ptr_from, len_from, NULL, 0);
+      wchar_t *wbuf = (wchar_t *)
+ HeapAlloc (GetProcessHeap (), 0, wlen * sizeof (wchar_t));
+      wlen =
+ MultiByteToWideChar (cp_from, 0, ptr_from, len_from, wbuf, wlen);
+      nlen = WideCharToMultiByte (cp_to, 0, wbuf, wlen, NULL, 0, NULL, NULL);
+      buf = (char *) HeapAlloc (GetProcessHeap (), 0, nlen);
+      nlen = WideCharToMultiByte (cp_to, 0, wbuf, wlen, buf, nlen, NULL, NULL);
+      HeapFree (GetProcessHeap (), 0, wbuf);
+    }
+  else
+    {
+      /* Just copy */
+      buf = (char *) HeapAlloc (GetProcessHeap (), 0, len_from);
+      memcpy (buf, ptr_from, len_from);
+      nlen = len_from;
+    }
+  *len_to = nlen;
+  return buf;
+}
+
+static void
+mb_str_free (char *ptr)
+{
+  HeapFree (GetProcessHeap (), 0, ptr);
+}
+
+static bool
 bytes_available (DWORD& n, HANDLE h)
 {
   DWORD navail, nleft;
@@ -1270,34 +1306,11 @@ fhandler_pty_slave::write (const void *ptr, size_t len)
 
   reset_switch_to_pcon ();
 
-  char *buf;
-  ssize_t nlen;
-  UINT targetCodePage = get_ttyp ()->switch_to_pcon_out ?
+  UINT target_code_page = get_ttyp ()->switch_to_pcon_out ?
     GetConsoleOutputCP () : get_ttyp ()->term_code_page;
-  if (targetCodePage != get_ttyp ()->term_code_page)
-    {
-      size_t wlen =
- MultiByteToWideChar (get_ttyp ()->term_code_page, 0,
-     (char *)ptr, len, NULL, 0);
-      wchar_t *wbuf = (wchar_t *)
- HeapAlloc (GetProcessHeap (), 0, wlen * sizeof (wchar_t));
-      wlen =
- MultiByteToWideChar (get_ttyp ()->term_code_page, 0,
-     (char *)ptr, len, wbuf, wlen);
-      nlen = WideCharToMultiByte (targetCodePage, 0,
-  wbuf, wlen, NULL, 0, NULL, NULL);
-      buf = (char *) HeapAlloc (GetProcessHeap (), 0, nlen);
-      nlen = WideCharToMultiByte (targetCodePage, 0,
-  wbuf, wlen, buf, nlen, NULL, NULL);
-      HeapFree (GetProcessHeap (), 0, wbuf);
-    }
-  else
-    {
-      /* Just copy */
-      buf = (char *) HeapAlloc (GetProcessHeap (), 0, len);
-      memcpy (buf, (char *)ptr, len);
-      nlen = len;
-    }
+  ssize_t nlen;
+  char *buf = convert_mb_str (target_code_page, (size_t *) &nlen,
+ get_ttyp ()->term_code_page, (const char *) ptr, len);
 
   /* If not attached to this pseudo console, try to attach temporarily. */
   pid_restore = 0;
@@ -1334,7 +1347,7 @@ fhandler_pty_slave::write (const void *ptr, size_t len)
       towrite = -1;
     }
   release_output_mutex ();
-  HeapFree (GetProcessHeap (), 0, buf);
+  mb_str_free (buf);
   flags = ENABLE_VIRTUAL_TERMINAL_PROCESSING;
   if (get_ttyp ()->switch_to_pcon_out && !fallback)
     SetConsoleMode (get_output_handle (), dwMode | flags);
@@ -2260,33 +2273,10 @@ fhandler_pty_master::write (const void *ptr, size_t len)
      if current application is native console application. */
   if (to_be_read_from_pcon ())
     {
-      char *buf;
       size_t nlen;
+      char *buf = convert_mb_str
+ (CP_UTF8, &nlen, get_ttyp ()->term_code_page, (const char *) ptr, len);
 
-      if (get_ttyp ()->term_code_page != CP_UTF8)
- {
-  size_t wlen =
-    MultiByteToWideChar (get_ttyp ()->term_code_page, 0,
- (char *)ptr, len, NULL, 0);
-  wchar_t *wbuf = (wchar_t *)
-    HeapAlloc (GetProcessHeap (), 0, wlen * sizeof (wchar_t));
-  wlen =
-    MultiByteToWideChar (get_ttyp ()->term_code_page, 0,
- (char *)ptr, len, wbuf, wlen);
-  nlen = WideCharToMultiByte (CP_UTF8, 0,
-      wbuf, wlen, NULL, 0, NULL, NULL);
-  buf = (char *) HeapAlloc (GetProcessHeap (), 0, nlen);
-  nlen = WideCharToMultiByte (CP_UTF8, 0,
-      wbuf, wlen, buf, nlen, NULL, NULL);
-  HeapFree (GetProcessHeap (), 0, wbuf);
- }
-      else
- {
-  /* Just copy */
-  buf = (char *) HeapAlloc (GetProcessHeap (), 0, len);
-  memcpy (buf, (char *)ptr, len);
-  nlen = len;
- }
       DWORD wLen;
       WriteFile (to_slave, buf, nlen, &wLen, NULL);
 
@@ -2302,7 +2292,7 @@ fhandler_pty_master::write (const void *ptr, size_t len)
       else
  SetEvent (input_available_event);
 
-      HeapFree (GetProcessHeap (), 0, buf);
+      mb_str_free (buf);
       return len;
     }
 
@@ -3039,32 +3029,10 @@ fhandler_pty_master::pty_master_fwd_thread ()
     }
   wlen = rlen;
 
-  char *buf;
   size_t nlen;
-  if (get_ttyp ()->term_code_page != CP_UTF8)
-    {
-      size_t wlen2 =
- MultiByteToWideChar (CP_UTF8, 0,
-     (char *)ptr, wlen, NULL, 0);
-      wchar_t *wbuf = (wchar_t *)
- HeapAlloc (GetProcessHeap (), 0, wlen2 * sizeof (wchar_t));
-      wlen2 =
- MultiByteToWideChar (CP_UTF8, 0,
-     (char *)ptr, wlen, wbuf, wlen2);
-      nlen = WideCharToMultiByte (get_ttyp ()->term_code_page, 0,
-  wbuf, wlen2, NULL, 0, NULL, NULL);
-      buf = (char *) HeapAlloc (GetProcessHeap (), 0, nlen);
-      nlen = WideCharToMultiByte (get_ttyp ()->term_code_page, 0,
-  wbuf, wlen2, buf, nlen, NULL, NULL);
-      HeapFree (GetProcessHeap (), 0, wbuf);
-    }
-  else
-    {
-      /* Just copy */
-      buf = (char *) HeapAlloc (GetProcessHeap (), 0, wlen);
-      memcpy (buf, (char *)ptr, wlen);
-      nlen = wlen;
-    }
+  char *buf = convert_mb_str
+    (get_ttyp ()->term_code_page, &nlen, CP_UTF8, ptr, wlen);
+
   ptr = buf;
   wlen = rlen = nlen;
 
@@ -3083,7 +3051,7 @@ fhandler_pty_master::pty_master_fwd_thread ()
       wlen = (rlen -= written);
     }
   release_output_mutex ();
-  HeapFree (GetProcessHeap (), 0, buf);
+  mb_str_free (buf);
   continue;
  }
       acquire_output_mutex (INFINITE);
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 4/5] Cygwin: pty: Add charset conversion for console apps in legacy PTY.

Takashi Yano
In reply to this post by Takashi Yano
---
 winsup/cygwin/fhandler_tty.cc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index f723ec7cf..2a92e44cf 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -3054,6 +3054,12 @@ fhandler_pty_master::pty_master_fwd_thread ()
   mb_str_free (buf);
   continue;
  }
+      size_t nlen;
+      char *buf = convert_mb_str
+ (get_ttyp ()->term_code_page, &nlen, GetConsoleOutputCP (), ptr, wlen);
+
+      ptr = buf;
+      wlen = rlen = nlen;
       acquire_output_mutex (INFINITE);
       while (rlen>0)
  {
@@ -3066,6 +3072,7 @@ fhandler_pty_master::pty_master_fwd_thread ()
   wlen = (rlen -= wlen);
  }
       release_output_mutex ();
+      mb_str_free (buf);
     }
   return 0;
 }
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 5/5] Cygwin: pty: Add missing guard when PTY is in the legacy mode.

Takashi Yano
In reply to this post by Takashi Yano
---
 winsup/cygwin/fhandler_tty.cc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 2a92e44cf..1095c82eb 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -87,7 +87,8 @@ set_switch_to_pcon (void)
       {
  fhandler_base *fh = cfd;
  fhandler_pty_slave *ptys = (fhandler_pty_slave *) fh;
- ptys->set_switch_to_pcon (fd);
+ if (ptys->getPseudoConsole ())
+  ptys->set_switch_to_pcon (fd);
       }
 }
 
@@ -105,6 +106,8 @@ force_attach_to_pcon (HANDLE h)
   {
     fhandler_base *fh = cfd;
     fhandler_pty_slave *ptys = (fhandler_pty_slave *) fh;
+    if (!ptys->getPseudoConsole ())
+      continue;
     if (n != 0
  || h == ptys->get_handle ()
  || h == ptys->get_output_handle ())
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/5] Some fixes for PTY with pseudo console support (4)

Ken Brown-6
In reply to this post by Takashi Yano
On 9/18/2019 10:29 AM, Takashi Yano wrote:
> Takashi Yano (5):
>    Cygwin: pty: Avoid potential segfault in PTY code when ppid = 1.
>    Cygwin: pty: Make GDB work again on pty.
>    Cygwin: pty: Unify the charset conversion codes into a function.
>    Cygwin: pty: Add charset conversion for console apps in legacy PTY.
>    Cygwin: pty: Add missing guard when PTY is in the legacy mode.
>
>   winsup/cygwin/fhandler_tty.cc | 188 +++++++++++++++++++---------------
>   1 file changed, 104 insertions(+), 84 deletions(-)

Pushed.  Thanks.

Ken