[PATCH] Cygwin: pty: Add missing guard for close_pseudoconsole().

2021-04-20 Thread Takashi Yano
- This patch adds a missing mutex guard for close_pseudoconsole()
  call when GDB exits.
---
 winsup/cygwin/fhandler_tty.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 530321513..9c03e09a7 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -177,7 +177,9 @@ atexit_func (void)
input_available_event);
ReleaseMutex (ptys->input_mutex);
  }
+   WaitForSingleObject (ptys->pcon_mutex, INFINITE);
ptys->close_pseudoconsole (ttyp, force_switch_to);
+   ReleaseMutex (ptys->pcon_mutex);
break;
  }
   CloseHandle (h_gdb_process);
-- 
2.31.1



[PATCH] Cygwin: pty: Fix fallback processing in setup_pseudoconsole().

2021-04-20 Thread Takashi Yano
- Currently, the fallback processing in setup_pseudoconsole()
  when helper process error occurs does not work properly.
  This patch fixes the issue.
---
 winsup/cygwin/fhandler_tty.cc | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index e4480771b..530321513 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -3226,15 +3226,15 @@ fhandler_pty_slave::setup_pseudoconsole (bool nopcon)
  if (wait_result == WAIT_OBJECT_0)
break;
  if (wait_result != WAIT_TIMEOUT)
-   goto cleanup_helper_process;
+   goto cleanup_helper_with_hello;
  DWORD exit_code;
  if (!GetExitCodeProcess (pi.hProcess, &exit_code))
-   goto cleanup_helper_process;
+   goto cleanup_helper_with_hello;
  if (exit_code == STILL_ACTIVE)
continue;
  if (exit_code != 0 ||
  WaitForSingleObject (hello, 500) != WAIT_OBJECT_0)
-   goto cleanup_helper_process;
+   goto cleanup_helper_with_hello;
  break;
}
   CloseHandle (hello);
@@ -3349,6 +3349,10 @@ skip_create:
 
   return true;
 
+cleanup_helper_with_hello:
+  CloseHandle (hello);
+  CloseHandle (pi.hThread);
+  goto cleanup_helper_process;
 cleanup_pcon_in:
   CloseHandle (hpConIn);
 cleanup_helper_process:
@@ -3358,10 +3362,10 @@ cleanup_helper_process:
   goto skip_close_hello;
 cleanup_event_and_pipes:
   CloseHandle (hello);
+skip_close_hello:
   get_ttyp ()->pcon_start = false;
   get_ttyp ()->pcon_start_pid = 0;
   get_ttyp ()->pcon_activated = false;
-skip_close_hello:
   CloseHandle (goodbye);
   CloseHandle (hr);
   CloseHandle (hw);
-- 
2.31.1



[PATCH] Cygwin: pty: Additional race issue fix regarding pseudo console.

2021-04-20 Thread Takashi Yano
- In commit bb93c6d7, the race issue was not completely fixed. In
  the pseudo console inheritance, if the destination process to
  which the ownership of pseudo console switches, is found but exits
  before switching, the inheritance fails. Currently, this extremely
  rarely happens. This patch fixes the issue.
---
 winsup/cygwin/fhandler_tty.cc | 47 +++
 1 file changed, 14 insertions(+), 33 deletions(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index d44728795..e4480771b 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -64,6 +64,8 @@ struct pipe_reply {
 
 extern HANDLE attach_mutex; /* Defined in fhandler_console.cc */
 
+inline static bool pcon_pid_alive (DWORD pid);
+
 DWORD
 fhandler_pty_common::get_console_process_id (DWORD pid, bool match,
 bool cygwin, bool stub_only)
@@ -90,20 +92,18 @@ fhandler_pty_common::get_console_process_id (DWORD pid, 
bool match,
else
  {
pinfo p (cygwin_pid (list[i]));
-   if (!!p && p->dwProcessId && p->exec_dwProcessId
-   && p->dwProcessId != p->exec_dwProcessId)
+   if (!!p && p->exec_dwProcessId)
  {
-   res_pri = list[i];
+   res_pri = stub_only ? p->exec_dwProcessId : list[i];
break;
  }
-   if (!!p && !res)
+   if (!p && !res && pcon_pid_alive (list[i]) && stub_only)
+ res = list[i];
+   if (!!p && !res && !stub_only)
  res = list[i];
  }
   }
-  if (stub_only)
-return res_pri;
-  else
-return res_pri ?: res;
+  return res_pri ?: res;
 }
 
 static bool isHybrid;
@@ -3384,35 +3384,17 @@ fallback:
 void
 fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to)
 {
-  DWORD switch_to_stub = 0, switch_to = 0;
-  DWORD new_pcon_pid = 0;
+  DWORD switch_to = 0;
   if (force_switch_to)
 {
-  switch_to_stub = force_switch_to;
-  new_pcon_pid = force_switch_to;
+  switch_to = force_switch_to;
   ttyp->setpgid (force_switch_to + MAX_PID);
 }
   else if (pcon_pid_self (ttyp->pcon_pid))
 {
   /* Search another process which attaches to the pseudo console */
   DWORD current_pid = myself->exec_dwProcessId ?: myself->dwProcessId;
-  switch_to = get_console_process_id (current_pid, false, true);
-  if (switch_to)
-   {
- pinfo p (cygwin_pid (switch_to));
- if (p)
-   {
- if (p->exec_dwProcessId)
-   switch_to_stub = p->exec_dwProcessId;
- new_pcon_pid = p->exec_dwProcessId;
-   }
-   }
-  else
-   {
- switch_to = get_console_process_id (current_pid, false, false);
- if (switch_to)
-   new_pcon_pid = switch_to;
-   }
+  switch_to = get_console_process_id (current_pid, false, true, true);
 }
   if (ttyp->pcon_activated)
 {
@@ -3420,7 +3402,6 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD 
force_switch_to)
   ttyp->previous_output_code_page = GetConsoleOutputCP ();
   if (pcon_pid_self (ttyp->pcon_pid))
{
- switch_to = switch_to_stub ?: switch_to;
  if (switch_to)
{
  /* Change pseudo console owner to another process */
@@ -3454,7 +3435,7 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD 
force_switch_to)
  CloseHandle (ttyp->h_pcon_conhost_process);
  CloseHandle (ttyp->h_pcon_in);
  CloseHandle (ttyp->h_pcon_out);
- ttyp->pcon_pid = new_pcon_pid;
+ ttyp->pcon_pid = switch_to;
  ttyp->h_pcon_write_pipe = new_write_pipe;
  ttyp->h_pcon_condrv_reference = new_condrv_reference;
  ttyp->h_pcon_conhost_process = new_conhost_process;
@@ -3513,8 +3494,8 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD 
force_switch_to)
 }
   else if (pcon_pid_self (ttyp->pcon_pid))
 {
-  if (switch_to_stub)
-   ttyp->pcon_pid = new_pcon_pid;
+  if (switch_to)
+   ttyp->pcon_pid = switch_to;
   else
{
  ttyp->pcon_pid = 0;
-- 
2.31.1



Re: [PATCH] Use automake (v5)

2021-04-20 Thread Jon Turney

On 20/04/2021 21:13, Jon Turney wrote:

For ease of reviewing, this patch doesn't contain changes to generated
files which would be made by running ./autogen.sh.


Sorry about getting distracted from this.  To summarize what I believe 
were the outstanding issues with v3 [1]:


[1] https://cygwin.com/pipermail/cygwin-patches/2020q4/010827.html

* 'INCLUDES' is the old name for 'AM_CPPFLAGS' warning from autogen.sh

I plan to clean this up in a future patch

* 'ps$(EXEEXT)' previously defined' warning from autogen.sh

It seems to be a shortcoming of automake that there's no way to suppress 
just that warning.


One possible solution is build ps.exe with a different name and rename 
it while installing, but I think that is counter-productive (in the 
sense that it trades this warning for making the build more complex to 
understand)


* some object files are in a unexpected places in the build file 
hierarchy (compared to naive expectations and/or the non-automake build)


I'm not sure if this is merely an aesthetic issue, or if there are 
problems this causes.




[PATCH] Use automake (v5)

2021-04-20 Thread Jon Turney
For ease of reviewing, this patch doesn't contain changes to generated
files which would be made by running ./autogen.sh.

v2:
* Include tzmap.h in BUILT_SOURCES
* Make per-file flags appear after user-supplied CXXFLAGS, so they can
override optimization level.
* Correct .o files used to define symbols exported by libm.a
* Drop gcrt0.o mistakenly included in libgmon.a
* Add missing line continuations in GMON_FILES value

v3:
* use per-file flags for .c compilation
* override C{XX,}FLAGS, as they are set on the command line by top-level make

v4:
* Drop -Wno-error=write-strings from path_testsuite CXXFLAGS

v5:
* Update for changes in master
- Add -fno-threadsafe-statics to CXX flags
- Add hypotl.cc
- Remove fenv.cc (in favour of newlib), add fenv.c stub
- Add proc.5 manpage rules
---
 winsup/Makefile.am |  19 +
 winsup/Makefile.am.common  |  15 +
 winsup/Makefile.common |  51 --
 winsup/autogen.sh  |   1 +
 winsup/configure.ac|  21 +-
 winsup/cygserver/Makefile.am   |  58 ++
 winsup/cygwin/Makefile.am  | 770 +
 winsup/cygwin/config.h.in  |   2 +-
 winsup/doc/Makefile.am | 162 ++
 winsup/testsuite/Makefile.am   |  64 ++
 winsup/testsuite/config/default.exp|   4 +-
 winsup/testsuite/cygrun/Makefile.am|  21 +
 winsup/testsuite/winsup.api/winsup.exp |   6 +-
 winsup/utils/Makefile.am   |  81 +++
 winsup/utils/mingw/Makefile.am |  50 ++
 15 files changed, 1266 insertions(+), 59 deletions(-)
 create mode 100644 winsup/Makefile.am
 create mode 100644 winsup/Makefile.am.common
 delete mode 100644 winsup/Makefile.common
 create mode 100644 winsup/cygserver/Makefile.am
 create mode 100644 winsup/cygwin/Makefile.am
 create mode 100644 winsup/doc/Makefile.am
 create mode 100644 winsup/testsuite/Makefile.am
 create mode 100644 winsup/testsuite/cygrun/Makefile.am
 create mode 100644 winsup/utils/Makefile.am
 create mode 100644 winsup/utils/mingw/Makefile.am

diff --git a/winsup/Makefile.am b/winsup/Makefile.am
new file mode 100644
index 0..067f74688
--- /dev/null
+++ b/winsup/Makefile.am
@@ -0,0 +1,19 @@
+# Makefile.am for winsup stuff
+#
+# This file is part of Cygwin.
+#
+# This software is a copyrighted work licensed under the terms of the
+# Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
+# details.
+
+# This makefile requires GNU make.
+
+cygdocdir = $(datarootdir)/doc/Cygwin
+
+cygdoc_DATA = \
+   CYGWIN_LICENSE \
+   COPYING
+
+SUBDIRS = cygwin cygserver doc utils testsuite
+
+cygserver utils testsuite: cygwin
diff --git a/winsup/Makefile.am.common b/winsup/Makefile.am.common
new file mode 100644
index 0..884194df2
--- /dev/null
+++ b/winsup/Makefile.am.common
@@ -0,0 +1,15 @@
+# Makefile.am.common - common definitions for the winsup directory
+#
+# This file is part of Cygwin.
+#
+# This software is a copyrighted work licensed under the terms of the
+# Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
+# details.
+
+flags_common=-Wall -Wstrict-aliasing -Wwrite-strings -fno-common -pipe 
-fbuiltin -fmessage-length=0
+
+# compiler flags commonly used (but not for MinGW compilation, because they
+# include the Cygwin header paths via @INCLUDES@)
+
+cxxflags_common=$(INCLUDES) -fno-rtti -fno-exceptions -fno-use-cxa-atexit 
$(flags_common)
+cflags_common=$(INCLUDES) $(flags_common)
diff --git a/winsup/Makefile.common b/winsup/Makefile.common
deleted file mode 100644
index 3141bd111..0
--- a/winsup/Makefile.common
+++ /dev/null
@@ -1,51 +0,0 @@
-# Makefile.common - common definitions for the winsup directory
-#
-# This file is part of Cygwin.
-#
-# This software is a copyrighted work licensed under the terms of the
-# Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
-# details.
-
-define justdir
-$(patsubst %/,%,$(dir $1))
-endef
-
-define libname
-$(realpath $(shell ${CC} --print-file-name=$1 $2))
-endef
-
-export PATH:=${winsup_srcdir}:${PATH}
-
-# Allow CFLAGS=-O,-g to control CXXFLAGS too
-opt=$(filter -O%,${CFLAGS}) $(filter -g%,${CFLAGS})
-override CXXFLAGS:=${filter-out -g%,$(filter-out -O%,${CXXFLAGS})} ${opt}
-
-cflags_common:=-Wall -Wstrict-aliasing -Wwrite-strings -fno-common -pipe 
-fbuiltin -fmessage-length=0
-COMPILE.cc=${CXX} ${INCLUDES} ${CXXFLAGS} -fno-rtti -fno-exceptions 
-fno-use-cxa-atexit ${cflags_common}
-COMPILE.c=${CC} ${INCLUDES} ${CFLAGS} ${cflags_common}
-
-top_srcdir:=$(call justdir,${winsup_srcdir})
-top_builddir:=$(call justdir,${target_builddir})
-
-cygwin_build:=${target_builddir}/winsup/cygwin
-newlib_build:=${target_builddir}/newlib
-
-VPATH:=${srcdir}
-
-.SUFFIXES:
-.SUFFIXES: .c .cc .def .S .a .o .d .s .E
-
-%.o: %.cc
-   $(strip ${COMPILE.cc} -c -o $@ $<)
-
-%.o: %.c
-   $(strip ${COMPILE.c} -c -o $@ $<)
-
-%.E: %.cc
-   $(strip ${COMPILE.cc} -E -dD -o $@ $<)
-
-%.E: %.c
-   $(strip ${COMPILE.c} -E

RE: [PATCH 0/2] Fix race issues.

2021-04-20 Thread David Allsopp
Corinna Vinschen wrote:
> On Apr 19 19:30, Takashi Yano wrote:
> > Takashi Yano (2):
> >   Cygwin: console: Fix race issue regarding cons_master_thread().
> >   Cygwin: pty: Fix race issue in inheritance of pseudo console.
> >
> >  winsup/cygwin/fhandler_console.cc |  10 ++-
> >  winsup/cygwin/fhandler_tty.cc | 108 ++
> >  winsup/cygwin/tty.cc  |  15 ++---
> >  winsup/cygwin/tty.h   |   2 +-
> >  4 files changed, 77 insertions(+), 58 deletions(-)
> >
> > --
> > 2.31.1
> 
> Pushed.

Armed with this morning's Cygwin snapshot, OCaml builds again as well. Many 
thanks!


David


Re: [PATCH] Cygwin: pty: Make rlwrap work with cmd.exe.

2021-04-20 Thread Corinna Vinschen
On Apr 20 05:51, Takashi Yano wrote:
> On Mon, 19 Apr 2021 16:19:09 +0200
> Corinna Vinschen wrote:
> > Hi Takashi,
> > 
> > On Apr 19 20:51, Takashi Yano wrote:
> > > - After the commit 919dea66, "rlwrap cmd" fails to start pseudo
> > >   console. This patch fixes the issue.
> > > ---
> > >  winsup/cygwin/fhandler_tty.cc | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> > > index ba9f4117f..d44728795 100644
> > > --- a/winsup/cygwin/fhandler_tty.cc
> > > +++ b/winsup/cygwin/fhandler_tty.cc
> > > @@ -1170,6 +1170,8 @@ fhandler_pty_slave::reset_switch_to_pcon (void)
> > >  }
> > >if (isHybrid)
> > >  return;
> > > +  if (get_ttyp ()->pcon_start)
> > > +return;
> > >WaitForSingleObject (pcon_mutex, INFINITE);
> > >if (!pcon_pid_self (get_ttyp ()->pcon_pid)
> > >&& pcon_pid_alive (get_ttyp ()->pcon_pid))
> > > -- 
> > > 2.31.1
> > 
> > this patch doesn't apply.  Does it depend on the race issue fixes?
> 
> Yes. This depends on the race issue patches.
> Please apply this patch after the race issue patches.
> 
> Thanks.

Pushed.


Thanks,
Corinna


Re: [PATCH 0/2] Fix race issues.

2021-04-20 Thread Corinna Vinschen
On Apr 19 19:30, Takashi Yano wrote:
> Takashi Yano (2):
>   Cygwin: console: Fix race issue regarding cons_master_thread().
>   Cygwin: pty: Fix race issue in inheritance of pseudo console.
> 
>  winsup/cygwin/fhandler_console.cc |  10 ++-
>  winsup/cygwin/fhandler_tty.cc | 108 ++
>  winsup/cygwin/tty.cc  |  15 ++---
>  winsup/cygwin/tty.h   |   2 +-
>  4 files changed, 77 insertions(+), 58 deletions(-)
> 
> -- 
> 2.31.1

Pushed.


Thanks,
Corinna