[PATCH] Cygwin: pty: Add missing console API hooks.

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

[PATCH] Cygwin: pty: Add missing console API hooks.

Takashi Yano
- Following console APIs are additionally hooked for cygwin programs
  which directly call them.
  * FillConsoleOutputAttribute()
  * FillConsoleOutputCharacterA()
  * FillConsoleOutputCharacterW()
  * ScrollConsoleScreenBufferA()
  * ScrollConsoleScreenBufferW()
---
 winsup/cygwin/fhandler_tty.cc | 55 +++++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 404216bf6..05070aa3b 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -192,6 +192,11 @@ DEF_HOOK (ReadConsoleInputA);
 DEF_HOOK (ReadConsoleInputW);
 DEF_HOOK (PeekConsoleInputA);
 DEF_HOOK (PeekConsoleInputW);
+DEF_HOOK (FillConsoleOutputAttribute);
+DEF_HOOK (FillConsoleOutputCharacterA);
+DEF_HOOK (FillConsoleOutputCharacterW);
+DEF_HOOK (ScrollConsoleScreenBufferA);
+DEF_HOOK (ScrollConsoleScreenBufferW);
 /* CreateProcess() is hooked for GDB etc. */
 DEF_HOOK (CreateProcessA);
 DEF_HOOK (CreateProcessW);
@@ -329,6 +334,43 @@ PeekConsoleInputW_Hooked
   set_ishybrid_and_switch_to_pcon (h);
   return PeekConsoleInputW_Orig (h, r, l, n);
 }
+static BOOL WINAPI
+FillConsoleOutputAttribute_Hooked
+     (HANDLE h, WORD a, DWORD l, COORD d, LPDWORD n)
+{
+  set_ishybrid_and_switch_to_pcon (h);
+  return FillConsoleOutputAttribute_Orig (h, a, l, d, n);
+}
+static BOOL WINAPI
+FillConsoleOutputCharacterA_Hooked
+     (HANDLE h, CHAR c, DWORD l, COORD d, LPDWORD n)
+{
+  set_ishybrid_and_switch_to_pcon (h);
+  return FillConsoleOutputCharacterA_Orig (h, c, l, d, n);
+}
+static BOOL WINAPI
+FillConsoleOutputCharacterW_Hooked
+     (HANDLE h, WCHAR c, DWORD l, COORD d, LPDWORD n)
+{
+  set_ishybrid_and_switch_to_pcon (h);
+  return FillConsoleOutputCharacterW_Orig (h, c, l, d, n);
+}
+static BOOL WINAPI
+ScrollConsoleScreenBufferA_Hooked
+     (HANDLE h, const SMALL_RECT *r, const SMALL_RECT *c, COORD d,
+      const CHAR_INFO *f)
+{
+  set_ishybrid_and_switch_to_pcon (h);
+  return ScrollConsoleScreenBufferA_Orig (h, r, c, d, f);
+}
+static BOOL WINAPI
+ScrollConsoleScreenBufferW_Hooked
+     (HANDLE h, const SMALL_RECT *r, const SMALL_RECT *c, COORD d,
+      const CHAR_INFO *f)
+{
+  set_ishybrid_and_switch_to_pcon (h);
+  return ScrollConsoleScreenBufferW_Orig (h, r, c, d, f);
+}
 /* CreateProcess() is hooked for GDB etc. */
 static BOOL WINAPI
 CreateProcessA_Hooked
@@ -2749,8 +2791,12 @@ fhandler_pty_slave::fixup_after_attach (bool native_maybe, int fd_set)
     (SHORT) (sbi.dwSize.X -1), (SHORT) (sbi.dwSize.Y - 1)};
   COORD dest = {0, 0};
   CHAR_INFO fill = {' ', 0};
-  ScrollConsoleScreenBuffer (get_output_handle (),
-     &rect, NULL, dest, &fill);
+  BOOL (WINAPI *ScrollFunc)
+    (HANDLE, const SMALL_RECT *, const SMALL_RECT *,
+     COORD, const CHAR_INFO *);
+  ScrollFunc = ScrollConsoleScreenBufferA_Orig ? :
+    ScrollConsoleScreenBuffer;
+  ScrollFunc (get_output_handle (), &rect, NULL, dest, &fill);
   get_ttyp ()->need_redraw_screen = false;
  }
     }
@@ -2848,6 +2894,11 @@ fhandler_pty_slave::fixup_after_exec ()
       DO_HOOK (NULL, ReadConsoleInputW);
       DO_HOOK (NULL, PeekConsoleInputA);
       DO_HOOK (NULL, PeekConsoleInputW);
+      DO_HOOK (NULL, FillConsoleOutputAttribute);
+      DO_HOOK (NULL, FillConsoleOutputCharacterA);
+      DO_HOOK (NULL, FillConsoleOutputCharacterW);
+      DO_HOOK (NULL, ScrollConsoleScreenBufferA);
+      DO_HOOK (NULL, ScrollConsoleScreenBufferW);
       /* CreateProcess() is hooked for GDB etc. */
       DO_HOOK (NULL, CreateProcessA);
       DO_HOOK (NULL, CreateProcessW);
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Add missing console API hooks.

Corinna Vinschen-2
On Jan 23 13:33, Takashi Yano wrote:
> - Following console APIs are additionally hooked for cygwin programs
>   which directly call them.
>   * FillConsoleOutputAttribute()
>   * FillConsoleOutputCharacterA()
>   * FillConsoleOutputCharacterW()
>   * ScrollConsoleScreenBufferA()
>   * ScrollConsoleScreenBufferW()

Which Cygwin programs are doing that?  They wouldn't work correctly in
ptys anyway, isn't it?  Does it really make sense to make them happy
rather than requesting to change them?


Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Add missing console API hooks.

Takashi Yano
On Thu, 23 Jan 2020 13:48:13 +0100
Corinna Vinschen wrote:

> On Jan 23 13:33, Takashi Yano wrote:
> > - Following console APIs are additionally hooked for cygwin programs
> >   which directly call them.
> >   * FillConsoleOutputAttribute()
> >   * FillConsoleOutputCharacterA()
> >   * FillConsoleOutputCharacterW()
> >   * ScrollConsoleScreenBufferA()
> >   * ScrollConsoleScreenBufferW()
>
> Which Cygwin programs are doing that?  They wouldn't work correctly in
> ptys anyway, isn't it?  Does it really make sense to make them happy
> rather than requesting to change them?

Just a possibility. There is no specific example.
With this patch, the code below can work even if it is compiled as
cygwin binary.

#include <stdio.h>
#include <windows.h>

int main() {
        COORD dest = {0, 0};
        printf("\033[H\033[J\n");
        DWORD n;
        FillConsoleOutputCharacter (GetStdHandle(STD_OUTPUT_HANDLE),
                        'A', 80, dest, &n);
        FillConsoleOutputAttribute (GetStdHandle(STD_OUTPUT_HANDLE),
                        FOREGROUND_RED, 80, dest, &n);
        return 0;
}

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Add missing console API hooks.

Corinna Vinschen-2
Hi Takashi,

On Jan 23 22:05, Takashi Yano wrote:

> On Thu, 23 Jan 2020 13:48:13 +0100
> Corinna Vinschen wrote:
> > On Jan 23 13:33, Takashi Yano wrote:
> > > - Following console APIs are additionally hooked for cygwin programs
> > >   which directly call them.
> > >   * FillConsoleOutputAttribute()
> > >   * FillConsoleOutputCharacterA()
> > >   * FillConsoleOutputCharacterW()
> > >   * ScrollConsoleScreenBufferA()
> > >   * ScrollConsoleScreenBufferW()
> >
> > Which Cygwin programs are doing that?  They wouldn't work correctly in
> > ptys anyway, isn't it?  Does it really make sense to make them happy
> > rather than requesting to change them?
>
> Just a possibility. There is no specific example.
In that case I'd prefer not to apply this patch.  Using native Windows
console functions in a Cygwin application just doesn't make sense, and
we shouldn't support that beyond what's necessary for older, existing
applications.


Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Add missing console API hooks.

Takashi Yano
Hi Corinna,

On Fri, 24 Jan 2020 11:26:27 +0100
Corinna Vinschen wrote:

> On Jan 23 22:05, Takashi Yano wrote:
> > On Thu, 23 Jan 2020 13:48:13 +0100
> > Corinna Vinschen wrote:
> > > On Jan 23 13:33, Takashi Yano wrote:
> > > > - Following console APIs are additionally hooked for cygwin programs
> > > >   which directly call them.
> > > >   * FillConsoleOutputAttribute()
> > > >   * FillConsoleOutputCharacterA()
> > > >   * FillConsoleOutputCharacterW()
> > > >   * ScrollConsoleScreenBufferA()
> > > >   * ScrollConsoleScreenBufferW()
> > >
> > > Which Cygwin programs are doing that?  They wouldn't work correctly in
> > > ptys anyway, isn't it?  Does it really make sense to make them happy
> > > rather than requesting to change them?
> >
> > Just a possibility. There is no specific example.
>
> In that case I'd prefer not to apply this patch.  Using native Windows
> console functions in a Cygwin application just doesn't make sense, and
> we shouldn't support that beyond what's necessary for older, existing
> applications.

I searched in /bin and found that many of cygwin programs calls win32 api
directly. However, the only cygwin programs which calls console APIs are
cygrunsrv.exe, gdb.exe and w3m.exe in my installation.

cygrunsrv calls:
        DLL Name: ADVAPI32.dll
        vma:  Hint/Ord Member-Name Bound-To
        264ec      87  CloseServiceHandle
        26502      92  ControlService
        26514     128  CreateServiceA
        26526     218  DeleteService
        26536     255  EnumServicesStatusA
        2654c     511  OpenSCManagerA
        2655e     513  OpenServiceA
        2656e     554  QueryServiceConfigA
        26584     559  QueryServiceStatus
        2659a     568  RegCloseKey
        265a8     569  RegConnectRegistryA
        265be     576  RegCreateKeyExA
        265d0     601  RegEnumValueA
        265e0     603  RegFlushKey
        265ee     616  RegOpenKeyExA
        265fe     629  RegQueryValueExA
        26612     645  RegSetValueExA
        26624     654  RegisterServiceCtrlHandlerExA
        26644     712  SetServiceStatus
        26658     718  StartServiceA
        26668     719  StartServiceCtrlDispatcherA

        DLL Name: KERNEL32.dll
        vma:  Hint/Ord Member-Name Bound-To
        26686      16  AllocConsole              <==============
        26696      83  CloseHandle
        266a4     239  EnterCriticalSection
        266bc     351  FormatMessageA
        266ce     353  FreeConsole               <==============
        266dc     356  FreeLibrary
        266ea     482  GetExitCodeProcess
        26700     515  GetLastError
        26710     531  GetModuleFileNameA
        26726     533  GetModuleHandleA
        2673a     581  GetProcAddress
        2674c     663  GetTickCount
        2675c     677  GetVersion
        2676a     747  InitializeCriticalSection
        26786     806  LeaveCriticalSection
        2679e     809  LoadLibraryA
        267ae     879  OpenProcess
        267bc    1035  SetConsoleTitleA          <==============
        267d0    1078  SetLastError
        267e0    1140  Sleep
        267e8    1218  WaitForSingleObject

        DLL Name: USER32.dll
        vma:  Hint/Ord Member-Name Bound-To
        267fe      13  BringWindowToTop
        26812     260  GetForegroundWindow
        26828     335  GetTopWindow
        26838     538  SetForegroundWindow

gdb calls:
        DLL Name: KERNEL32.dll
        vma:  Hint/Ord Member-Name Bound-To
        60a5bc     84  CloseHandle
        60a5ca    106  ContinueDebugEvent
        60a5e0    140  CreateFileA
        60a5ee    172  CreateProcessW
        60a600    201  DebugActiveProcess
        60a616    348  FlushInstructionCache
        60a62e    358  FreeLibrary
        60a63c    364  GenerateConsoleCtrlEvent   <==============
        60a658    440  GetConsoleScreenBufferInfo <==============
        60a676    454  GetCurrentProcess
        60a68a    485  GetExitCodeThread
        60a69e    517  GetLastError
        60a6ae    535  GetModuleHandleA
        60a6c2    538  GetModuleHandleW
        60a6d6    583  GetProcAddress
        60a6e8    628  GetSystemDirectoryW
        60a6fe    651  GetThreadContext
        60a712    661  GetThreadSelectorEntry
        60a72c    811  LoadLibraryA
        60a73c    882  OpenProcess
        60a74a    950  ReadProcessMemory
        60a75e    988  ResumeThread
        60a76e   1015  SetConsoleCtrlHandler     <==============
        60a786   1057  SetEnvironmentVariableW
        60a7a0   1059  SetEvent
        60a7ac   1115  SetThreadContext
        60a7c0   1154  SuspendThread
        60a7d0   1160  TerminateProcess
        60a7e4   1221  WaitForDebugEvent
        60a7f8   1224  WaitForSingleObject
        60a80e   1278  WriteProcessMemory

w3m calls:
        DLL Name: KERNEL32.dll
        vma:  Hint/Ord Member-Name Bound-To
        1384c0     83  CloseHandle
        1384ce    139  CreateFileA
        1384dc    356  FreeLibrary
        1384ea    441  GetConsoleTitleA        <==============
        1384fe    453  GetCurrentProcessId
        138514    533  GetModuleHandleA
        138528    581  GetProcAddress
        13853a    663  GetTickCount
        13854a    678  GetVersionExA
        13855a    809  LoadLibraryA
        13856a    888  PeekConsoleInputA       <==============
        13857e    929  ReadConsoleA            <==============
        13858e    930  ReadConsoleInputA       <==============
        1385a2   1035  SetConsoleTitleA        <==============
        1385b6   1140  Sleep

        DLL Name: USER32.dll
        vma:  Hint/Ord Member-Name Bound-To
        1385be    211  FindWindowA
        1385cc    401  IsWindowVisible
        1385de    644  wsprintfA

None of them calls:
FillConsoleOutputAttribute()
FillConsoleOutputCharacterA()
FillConsoleOutputCharacterW()
ScrollConsoleScreenBufferA()
ScrollConsoleScreenBufferW()

So, I would like to withdraw this patch for now. Thanks.

--
Takashi Yano <[hidden email]>