Re: [PATCH] Cygwin: console: Fix segfault on shared_console_info access.
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
> >
> > 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
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
Description: PGP signature
Re: [PATCH] fhandler_proc/cpuinfo: support fast short REP MOVSB
On Feb 21 14:28, Brian Inglis wrote: > Added in Linux 5.6: > Check FSRM and use REP MOVSB for short copies on systems that have it. > > >From the Intel Optimization Reference Manual: > > 3.7.6.1 Fast Short REP MOVSB > Beginning with processors based on Ice Lake Client microarchitecture, > REP MOVSB performance is enhanced with string lengths up to 128 bytes. > Support for fast-short REP MOVSB is indicated by the CPUID feature flag: > CPUID [EAX=7H, ECX=0H).EDX.FAST_SHORT_REP_MOVSB[bit 4] = 1. > There is no change in the REP STOS performance. > --- > winsup/cygwin/fhandler_proc.cc | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/winsup/cygwin/fhandler_proc.cc b/winsup/cygwin/fhandler_proc.cc > index 78a69703d..030ade68a 100644 > --- a/winsup/cygwin/fhandler_proc.cc > +++ b/winsup/cygwin/fhandler_proc.cc > @@ -1346,6 +1346,7 @@ format_proc_cpuinfo (void *, char *&destbuf) > >ftcprint (features1, 2, "avx512_4vnniw");/* vec dot prod dw */ >ftcprint (features1, 3, "avx512_4fmaps"); /* vec 4 FMA > single */ > + ftcprint (features1, 4, "fsrm"); /* fast short REP > MOVSB */ >ftcprint (features1, 8, "avx512_vp2intersect"); /* vec intcpt d/q > */ >ftcprint (features1, 10, "md_clear");/* verw clear buf > */ >ftcprint (features1, 18, "pconfig"); /* platform > config */ > -- > 2.21.0 Pushed. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer signature.asc Description: PGP signature
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
>
> 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
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
[PATCH v2] Cygwin: console: Fix segfault on shared_console_info access.
- Accessing shared_console_info before initialization causes access
violation because 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() when it is
called from close(). This patch makes sure that shared_console_info
is not NULL before calling request_xterm_mode_output().
---
winsup/cygwin/fhandler_console.cc | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/winsup/cygwin/fhandler_console.cc
b/winsup/cygwin/fhandler_console.cc
index 42040a971..328424a7d 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 && 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 ();
--
2.21.0
Re: [PATCH] Cygwin: console: Fix segfault on shared_console_info access.
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
> >
> > 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
> 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
Description: PGP signature
Re: [PATCH v2] Cygwin: console: Fix segfault on shared_console_info access.
On Feb 25 01:12, Takashi Yano wrote: > - Accessing shared_console_info before initialization causes access > violation because 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() when it is > called from close(). This patch makes sure that shared_console_info > is not NULL before calling request_xterm_mode_output(). > --- > winsup/cygwin/fhandler_console.cc | 23 +++ > 1 file changed, 11 insertions(+), 12 deletions(-) Pushed. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer signature.asc Description: PGP signature
Re: [PATCH] Cygwin: console: Fix segfault on shared_console_info access.
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
>>>
>>> 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
>> 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.
Re: [PATCH] Cygwin: console: Fix segfault on shared_console_info access.
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
Re: [PATCH] Cygwin: console: Fix segfault on shared_console_info access.
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
