This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Cygwin: console: Fix segfault on shared_console_info access.


On Mon, 24 Feb 2020 11:08:35 +0100
Corinna Vinschen wrote:
> I debugged this situation as well and what strikes me as weird is
> that in fhandler_console::close, there are two calls to
> request_xterm_mode_output(false).  The first one is only called if
> shared_console_info is != NULL, the other one is always called if
> wincap.has_con_24bit_colors().  This looks a bit fishy.  Not only
> the shared_console_info test is missing, but also the con_is_legacy
> test.
> 
> What about something like this:
> 
> diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
> index 42040a97162e..edb71fffe48f 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> @@ -1159,18 +1159,17 @@ fhandler_console::close ()
>  
>    acquire_output_mutex (INFINITE);
>  
> -  if (shared_console_info && myself->pid == con.owner &&
> -      wincap.has_con_24bit_colors () && !con_is_legacy)
> -    request_xterm_mode_output (false);
> -
> -  /* Restore console mode if this is the last closure. */
> -  OBJECT_BASIC_INFORMATION obi;
> -  NTSTATUS status;
> -  status = NtQueryObject (get_handle (), ObjectBasicInformation,
> -			  &obi, sizeof obi, NULL);
> -  if (NT_SUCCESS (status) && obi.HandleCount == 1)
> -    if (wincap.has_con_24bit_colors ())
> -      request_xterm_mode_output (false);
> +  if (shared_console_info && !con_is_legacy && wincap.has_con_24bit_colors ())
> +    {
> +      /* Restore console mode if this is the last closure. */
> +      OBJECT_BASIC_INFORMATION obi;
> +      NTSTATUS status;
> +      status = NtQueryObject (get_handle (), ObjectBasicInformation,
> +			      &obi, sizeof obi, NULL);
> +      if ((NT_SUCCESS (status) && obi.HandleCount == 1)
> +	  || myself->pid == con.owner)
> +	request_xterm_mode_output (false);
> +    }
>  
>    release_output_mutex ();

OK. I will submit a v2 patch according to your suggestion.
As for con_is_legacy check, it is included in
request_xterm_mode_output(), so is not necessary here.

> Btw., are you testing the console with black background?  I'm asking
> because I'm using the console with grey background and black characters,
> and I'm always seeing artifacts when using vim in xterm mode.
> 
> E.g., open vim on the fork-setsid.c source in the console in xterm
> mode.  Move the cursor to the beginning of the word `setsid'.  Now
> press the three chars
> 
>   c h <CR>
> 
> this moves the setsid call to the next line.  But it also adds
> black background after `setsid();'.  Simiar further actions always
> create black background artifacts.
> 
> Is there anything we can do against that?

This seems to be a bug of windows console. It also occurs in wsl.
/bin/echo -e '\033[H\033[5L'
causes the similar result.

The following code cause the problem as well.

#include <windows.h>
int main()
{
    CONSOLE_SCREEN_BUFFER_INFO sbi;
    SMALL_RECT r;
    COORD c = {0, 0};
    CHAR_INFO f = {' ', 0};
    HANDLE h = GetStdHandle(STD_OUTPUT_HANDLE);
    DWORD n;
    ReadConsoleOutputAttribute(h, &f.Attributes, 1, c, &n);
    GetConsoleScreenBufferInfo(h, &sbi);
    c.X = 0;
    c.Y = sbi.srWindow.Top + 5;
    ScrollConsoleScreenBuffer(h, &sbi.srWindow, NULL, c, &f);
    return 0;
}

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]