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

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

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

Takashi Yano
- Accessing shared_console_info accidentaly causes segmentation
  fault when it is a NULL pointer. The cause of the problem reported
  in https://cygwin.com/ml/cygwin/2020-02/msg00197.html is this NULL
  pointer access in request_xterm_mode_output(). This patch fixes
  the issue.
---
 winsup/cygwin/fhandler_console.cc | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 42040a971..e298dd60c 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -256,7 +256,8 @@ fhandler_console::request_xterm_mode_input (bool req)
     return;
   if (req)
     {
-      if (InterlockedIncrement (&con.xterm_mode_input) == 1)
+      if (!shared_console_info ||
+  InterlockedIncrement (&con.xterm_mode_input) == 1)
  {
   DWORD dwMode;
   GetConsoleMode (get_handle (), &dwMode);
@@ -266,7 +267,8 @@ fhandler_console::request_xterm_mode_input (bool req)
     }
   else
     {
-      if (InterlockedDecrement (&con.xterm_mode_input) == 0)
+      if (!shared_console_info ||
+  InterlockedDecrement (&con.xterm_mode_input) == 0)
  {
   DWORD dwMode;
   GetConsoleMode (get_handle (), &dwMode);
@@ -283,7 +285,8 @@ fhandler_console::request_xterm_mode_output (bool req)
     return;
   if (req)
     {
-      if (InterlockedExchange (&con.xterm_mode_output, 1) == 0)
+      if (!shared_console_info ||
+  InterlockedExchange (&con.xterm_mode_output, 1) == 0)
  {
   DWORD dwMode;
   GetConsoleMode (get_output_handle (), &dwMode);
@@ -293,7 +296,8 @@ fhandler_console::request_xterm_mode_output (bool req)
     }
   else
     {
-      if (InterlockedExchange (&con.xterm_mode_output, 0) == 1)
+      if (!shared_console_info ||
+  InterlockedExchange (&con.xterm_mode_output, 0) == 1)
  {
   DWORD dwMode;
   GetConsoleMode (get_output_handle (), &dwMode);
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

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

Corinna Vinschen-2
Hi Takashi,

On Feb 22 04:10, Takashi Yano wrote:
> - Accessing shared_console_info accidentaly causes segmentation
>   fault when it is a NULL pointer. The cause of the problem reported
>   in https://cygwin.com/ml/cygwin/2020-02/msg00197.html is this NULL
>   pointer access in request_xterm_mode_output(). This patch fixes
>   the issue.

When does this occur?  I guess this is during initialization.  Is it
really necessary to switch to xterm mode at all at that time?  If not,
it might be simpler to just

-  if (con_is_legacy)
+  if (con_is_legacy || !shared_console_info)

at the start of the functions and only switch to xterm mode when
fully up and running.

> ---
>  winsup/cygwin/fhandler_console.cc | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
> index 42040a971..e298dd60c 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> @@ -256,7 +256,8 @@ fhandler_console::request_xterm_mode_input (bool req)
>      return;
>    if (req)
>      {
> -      if (InterlockedIncrement (&con.xterm_mode_input) == 1)
> +      if (!shared_console_info ||
> +  InterlockedIncrement (&con.xterm_mode_input) == 1)
Btw., that should be

         if (!shared_console_info
             || InterlockedIncrement (&con.xterm_mode_input) == 1)

Note the position of the ||


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

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

Takashi Yano
Hi Corinna,

On Fri, 21 Feb 2020 20:43:33 +0100
Corinna Vinschen wrote:

> On Feb 22 04:10, Takashi Yano wrote:
> > - Accessing shared_console_info accidentaly causes segmentation
> >   fault when it is a NULL pointer. The cause of the problem reported
> >   in https://cygwin.com/ml/cygwin/2020-02/msg00197.html is this NULL
> >   pointer access in request_xterm_mode_output(). This patch fixes
> >   the issue.
>
> When does this occur?  I guess this is during initialization.  Is it
> really necessary to switch to xterm mode at all at that time?  If not,
> it might be simpler to just
>
> -  if (con_is_legacy)
> +  if (con_is_legacy || !shared_console_info)
>
> at the start of the functions and only switch to xterm mode when
> fully up and running.

This happens when request_xterm_mode_output() is called from
close(). Usually, shared_console_info has been set when it is
called from close(), buf this happnes in mintty case. Since I
was not sure why shared_console_info is NULL in mintty case,
I have investigated deeper.

And I found the following code causes the same situation.

/* fork-setsid.c: */
/* gcc -mwindows fork-setsid.c -o fork-setsid */
#include <unistd.h>

int main()
{
    if (!fork()) setsid();
    return 0;
}

In this case, close() is called via cygheap->close_ctty() from
setsid() even though console is not opened.

Therefore, the following patch also fixes the mintty issue.
However, I am not sure this is the right thing.

diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 4652de929..47a78bae4 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -937,6 +937,7 @@ dtable::fixup_after_exec ()
 void
 dtable::fixup_after_fork (HANDLE parent)
 {
+  bool ctty_opened = false;
   fhandler_base *fh;
   for (size_t i = 0; i < size; i++)
     if ((fh = fds[i]) != NULL)
@@ -957,7 +958,11 @@ dtable::fixup_after_fork (HANDLE parent)
   SetStdHandle (std_consts[i], fh->get_handle ());
  else if (i <= 2)
   SetStdHandle (std_consts[i], fh->get_output_handle ());
+ if (cygheap->ctty == fh)
+  ctty_opened = true;
       }
+  if (!ctty_opened)
+    cygheap->ctty = NULL;
 }
 
 static void

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

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

Takashi Yano
On Sat, 22 Feb 2020 17:01:23 +0900
Takashi Yano wrote:

> Hi Corinna,
>
> On Fri, 21 Feb 2020 20:43:33 +0100
> Corinna Vinschen wrote:
> > On Feb 22 04:10, Takashi Yano wrote:
> > > - Accessing shared_console_info accidentaly causes segmentation
> > >   fault when it is a NULL pointer. The cause of the problem reported
> > >   in https://cygwin.com/ml/cygwin/2020-02/msg00197.html is this NULL
> > >   pointer access in request_xterm_mode_output(). This patch fixes
> > >   the issue.
> >
> > When does this occur?  I guess this is during initialization.  Is it
> > really necessary to switch to xterm mode at all at that time?  If not,
> > it might be simpler to just
> >
> > -  if (con_is_legacy)
> > +  if (con_is_legacy || !shared_console_info)
> >
> > at the start of the functions and only switch to xterm mode when
> > fully up and running.
>
> This happens when request_xterm_mode_output() is called from
> close(). Usually, shared_console_info has been set when it is
> called from close(), buf this happnes in mintty case. Since I
> was not sure why shared_console_info is NULL in mintty case,
> I have investigated deeper.
>
> And I found the following code causes the same situation.
>
> /* fork-setsid.c: */
> /* gcc -mwindows fork-setsid.c -o fork-setsid */
> #include <unistd.h>
>
> int main()
> {
>     if (!fork()) setsid();
>     return 0;
> }
>
> In this case, close() is called via cygheap->close_ctty() from
> setsid() even though console is not opened.
>
> Therefore, the following patch also fixes the mintty issue.
> However, I am not sure this is the right thing.
>
> diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
> index 4652de929..47a78bae4 100644
> --- a/winsup/cygwin/dtable.cc
> +++ b/winsup/cygwin/dtable.cc
> @@ -937,6 +937,7 @@ dtable::fixup_after_exec ()
>  void
>  dtable::fixup_after_fork (HANDLE parent)
>  {
> +  bool ctty_opened = false;
>    fhandler_base *fh;
>    for (size_t i = 0; i < size; i++)
>      if ((fh = fds[i]) != NULL)
> @@ -957,7 +958,11 @@ dtable::fixup_after_fork (HANDLE parent)
>    SetStdHandle (std_consts[i], fh->get_handle ());
>   else if (i <= 2)
>    SetStdHandle (std_consts[i], fh->get_output_handle ());
> + if (cygheap->ctty == fh)
> +  ctty_opened = true;
>        }
> +  if (!ctty_opened)
> +    cygheap->ctty = NULL;
>  }
>  
>  static void

This does not work as expected. How about this one?

diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 4652de929..138b7a1eb 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -937,6 +937,7 @@ dtable::fixup_after_exec ()
 void
 dtable::fixup_after_fork (HANDLE parent)
 {
+  bool ctty_opened = false;
   fhandler_base *fh;
   for (size_t i = 0; i < size; i++)
     if ((fh = fds[i]) != NULL)
@@ -957,7 +958,11 @@ dtable::fixup_after_fork (HANDLE parent)
   SetStdHandle (std_consts[i], fh->get_handle ());
  else if (i <= 2)
   SetStdHandle (std_consts[i], fh->get_output_handle ());
+ if (cygheap->ctty == fh->archetype)
+  ctty_opened = true;
       }
+  if (!ctty_opened)
+    cygheap->ctty = NULL;
 }
 
 static void


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

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

Corinna Vinschen-2
On Feb 22 22:35, Takashi Yano wrote:

> On Sat, 22 Feb 2020 17:01:23 +0900
> Takashi Yano wrote:
> > Hi Corinna,
> >
> > On Fri, 21 Feb 2020 20:43:33 +0100
> > Corinna Vinschen wrote:
> > > On Feb 22 04:10, Takashi Yano wrote:
> > > > - Accessing shared_console_info accidentaly causes segmentation
> > > >   fault when it is a NULL pointer. The cause of the problem reported
> > > >   in https://cygwin.com/ml/cygwin/2020-02/msg00197.html is this NULL
> > > >   pointer access in request_xterm_mode_output(). This patch fixes
> > > >   the issue.
> > >
> > > When does this occur?  [...]
> > This happens when request_xterm_mode_output() is called from
> > close(). Usually, shared_console_info has been set when it is
> > called from close(), buf this happnes in mintty case. Since I
> > was not sure why shared_console_info is NULL in mintty case,
> > I have investigated deeper.
> >
> > And I found the following code causes the same situation.
> >
> > /* fork-setsid.c: */
> > /* gcc -mwindows fork-setsid.c -o fork-setsid */
> > #include <unistd.h>
> >
> > int main()
> > {
> >     if (!fork()) setsid();
> >     return 0;
> > }
> >
> > In this case, close() is called via cygheap->close_ctty() from
> > setsid() even though console is not opened.
> >
> > Therefore, the following patch also fixes the mintty issue.
> > However, I am not sure this is the right thing.
> > [...]
> This does not work as expected. How about this one?
>
> diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
> index 4652de929..138b7a1eb 100644
> --- a/winsup/cygwin/dtable.cc
> +++ b/winsup/cygwin/dtable.cc
> @@ -937,6 +937,7 @@ dtable::fixup_after_exec ()
>  void
>  dtable::fixup_after_fork (HANDLE parent)
>  {
> +  bool ctty_opened = false;
>    fhandler_base *fh;
>    for (size_t i = 0; i < size; i++)
>      if ((fh = fds[i]) != NULL)
> @@ -957,7 +958,11 @@ dtable::fixup_after_fork (HANDLE parent)
>    SetStdHandle (std_consts[i], fh->get_handle ());
>   else if (i <= 2)
>    SetStdHandle (std_consts[i], fh->get_output_handle ());
> + if (cygheap->ctty == fh->archetype)
> +  ctty_opened = true;
>        }
> +  if (!ctty_opened)
> +    cygheap->ctty = NULL;
>  }
I'm not sure this is safe, archetype can be NULL.

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 ();
 
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?


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

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

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

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

Corinna Vinschen-2
On Feb 25 01:10, Takashi Yano wrote:

> On Mon, 24 Feb 2020 11:08:35 +0100
> Corinna Vinschen wrote:
> [...]
> > 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.
Oh well.

> 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;
> }
Is there some kind of workaround for that problem?  Otherwise defaulting
to a (broken) xterm mode instead of a (working) cygwin mode is a bit
questionable, isn't it?


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

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

Brian Inglis
On 2020-02-24 11:33, Corinna Vinschen wrote:

> On Feb 25 01:10, Takashi Yano wrote:
>> On Mon, 24 Feb 2020 11:08:35 +0100
>> Corinna Vinschen wrote:
>> [...]
>>> 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.
>
> Oh well.
>
>> 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;
>> }
>
> Is there some kind of workaround for that problem?  Otherwise defaulting
> to a (broken) xterm mode instead of a (working) cygwin mode is a bit
> questionable, isn't it?

I'm a windows with white backgrounds user and e.g. vim in latest Cygwin under
cmd requires typing ctrl-L to refresh the screen every couple of commands or
motions; looks okay under mintty in windows with white backgrounds.

--
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.
Reply | Threaded
Open this post in threaded view
|

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

Takashi Yano
In reply to this post by Corinna Vinschen-2
Hi Corinna,

On Mon, 24 Feb 2020 19:33:18 +0100
Corinna Vinschen wrote:
> Is there some kind of workaround for that problem?  Otherwise defaulting
> to a (broken) xterm mode instead of a (working) cygwin mode is a bit
> questionable, isn't it?

In my environment, legacy cygwin mode is not 'working' with
gray background and black foreground. You can confirm what
happens if xterm mode is disabled by reverting cygwin to 3.0.7.

If you type 'aaa' in shell prompt and hit backspace, then
whole line after cursor gets black. Furthermore, if you run
vim, whole screen gets into black background and gray fore-
ground.

Do not these happen in your environment?

Oh, wait. I was setting foreground and background color in
"terminal" tab in property. If I set them in "colors" tab,
cmd.exe behaves differently. In this setting, your problem
does not seems to occur.

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

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

Takashi Yano
On Tue, 25 Feb 2020 12:08:16 +0900
Takashi Yano wrote:

> On Mon, 24 Feb 2020 19:33:18 +0100
> Corinna Vinschen wrote:
> > Is there some kind of workaround for that problem?  Otherwise defaulting
> > to a (broken) xterm mode instead of a (working) cygwin mode is a bit
> > questionable, isn't it?
>
> In my environment, legacy cygwin mode is not 'working' with
> gray background and black foreground. You can confirm what
> happens if xterm mode is disabled by reverting cygwin to 3.0.7.
>
> If you type 'aaa' in shell prompt and hit backspace, then
> whole line after cursor gets black. Furthermore, if you run
> vim, whole screen gets into black background and gray fore-
> ground.
>
> Do not these happen in your environment?
>
> Oh, wait. I was setting foreground and background color in
> "terminal" tab in property. If I set them in "colors" tab,
> cmd.exe behaves differently. In this setting, your problem
> does not seems to occur.

I was wrong. The problem also occur with "colors" tab setting.
However, in this case, ScrollConsoleScreenBuffer() test case
does not cause the problem. Therefore it may be possible to
make a workaround for this. I will try.

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

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

Corinna Vinschen-2
On Feb 25 12:53, Takashi Yano wrote:

> On Tue, 25 Feb 2020 12:08:16 +0900
> Takashi Yano wrote:
> > On Mon, 24 Feb 2020 19:33:18 +0100
> > Corinna Vinschen wrote:
> > > Is there some kind of workaround for that problem?  Otherwise defaulting
> > > to a (broken) xterm mode instead of a (working) cygwin mode is a bit
> > > questionable, isn't it?
> >
> > In my environment, legacy cygwin mode is not 'working' with
> > gray background and black foreground.
I'm using the windows console with grey background and black
foreground for ages.  Apart from bugs we had in the past, this
setting always worked as expected, except in some border cases.

What's important is that you start your shell from a shortcut
with these console settings.  I.e., you have to change the
settings in the shortcut properties.  Otherwise, if you start
the shell with default grey on black and just switch the color
settings on the fly for this console, there will always be
artifacts.  Somehow the legacy console doesn't like switching
colors during a session that much.

> > You can confirm what
> > happens if xterm mode is disabled by reverting cygwin to 3.0.7.

I simply set has_con_24bit_colors to false and rebuilt the DLL.
With this enforced cygwin term setting, my vim example works
fine, just as /bin/echo -e '\033[H\033[5L'

> > If you type 'aaa' in shell prompt and hit backspace, then
> > whole line after cursor gets black. Furthermore, if you run
> > vim, whole screen gets into black background and gray fore-
> > ground.
> >
> > Do not these happen in your environment?

No, as I wrote, it works as expected and the background stays
grey.

> > Oh, wait. I was setting foreground and background color in
> > "terminal" tab in property. If I set them in "colors" tab,
> > cmd.exe behaves differently. In this setting, your problem
> > does not seems to occur.
>
> I was wrong. The problem also occur with "colors" tab setting.
> However, in this case, ScrollConsoleScreenBuffer() test case
> does not cause the problem. Therefore it may be possible to
> make a workaround for this. I will try.

That would be great!


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

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

Takashi Yano
Hi Corinna,

On Tue, 25 Feb 2020 10:16:56 +0100
Corinna Vinschen wrote:

> > > Oh, wait. I was setting foreground and background color in
> > > "terminal" tab in property. If I set them in "colors" tab,
> > > cmd.exe behaves differently. In this setting, your problem
> > > does not seems to occur.
> >
> > I was wrong. The problem also occur with "colors" tab setting.
> > However, in this case, ScrollConsoleScreenBuffer() test case
> > does not cause the problem. Therefore it may be possible to
> > make a workaround for this. I will try.
>
> That would be great!

I have successfully made a workaround for this issue.
I will submit it shortly.

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

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

Brian Inglis
In reply to this post by Takashi Yano
On 2020-02-24 20:53, Takashi Yano wrote:

> On Tue, 25 Feb 2020 12:08:16 +0900
> Takashi Yano wrote:
>> On Mon, 24 Feb 2020 19:33:18 +0100
>> Corinna Vinschen wrote:
>>> Is there some kind of workaround for that problem?  Otherwise defaulting
>>> to a (broken) xterm mode instead of a (working) cygwin mode is a bit
>>> questionable, isn't it?
>>
>> In my environment, legacy cygwin mode is not 'working' with
>> gray background and black foreground. You can confirm what
>> happens if xterm mode is disabled by reverting cygwin to 3.0.7.
>>
>> If you type 'aaa' in shell prompt and hit backspace, then
>> whole line after cursor gets black. Furthermore, if you run
>> vim, whole screen gets into black background and gray fore-
>> ground.
>>
>> Do not these happen in your environment?
>>
>> Oh, wait. I was setting foreground and background color in
>> "terminal" tab in property. If I set them in "colors" tab,
>> cmd.exe behaves differently. In this setting, your problem
>> does not seems to occur.

Terminal tab is only supported in latest W10 (Home - not necessarily Enterprise,
Education, etc.) and is "experimental" so properties could disappear, change,
work differently.

> I was wrong. The problem also occur with "colors" tab setting.
> However, in this case, ScrollConsoleScreenBuffer() test case
> does not cause the problem. Therefore it may be possible to
> make a workaround for this. I will try.

Please do not forget many users may still be running earlier W10 or pre-W10
releases as we still support Vista, 7, 8, 8.1, some may be running server
releases, and even running under Wine.

--
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.