From ac955c42fbe5be3b11eba49f6ebc827ad60ece16 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 2 Feb 2024 13:40:28 +0100 Subject: [PATCH 1/5] Work around fragile `#include` in binutils MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `bfd.h` header file that is included in `binutils` has the line `#include "ansidecl.h"`, which is fragile because it prefers Cygwin's `include/ansidecl.h` (as opposed to `#include `, which would only look in the system include paths). This matters because as of v2.42, `bfd.h` also makes use of the `ATTRIBUTE_WARN_UNUSED_RESULT` macro. So let's just copy that macro (and while at it, the other `ATTRIBUTE_*` macros) from binutils' `ansidecl.h` file, to avoid compile errors while compiling `dumper.o` that look like this: /usr/include/bfd.h:2770:1: error: expected initializer before ‘ATTRIBUTE_WARN_UNUSED_RESULT’ 2770 | ATTRIBUTE_WARN_UNUSED_RESULT; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ Signed-off-by: Johannes Schindelin --- include/ansidecl.h | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/include/ansidecl.h b/include/ansidecl.h index 6e4bfc21f2..ceb356e74e 100644 --- a/include/ansidecl.h +++ b/include/ansidecl.h @@ -283,6 +283,49 @@ So instead we use the macro below and test it against specific values. */ # endif /* GNUC >= 4.9 */ #endif /* ATTRIBUTE_NO_SANITIZE_UNDEFINED */ +/* Attribute 'nonstring' was valid as of gcc 8. */ +#ifndef ATTRIBUTE_NONSTRING +# if GCC_VERSION >= 8000 +# define ATTRIBUTE_NONSTRING __attribute__ ((__nonstring__)) +# else +# define ATTRIBUTE_NONSTRING +# endif +#endif + +/* Attribute `alloc_size' was valid as of gcc 4.3. */ +#ifndef ATTRIBUTE_RESULT_SIZE_1 +# if (GCC_VERSION >= 4003) +# define ATTRIBUTE_RESULT_SIZE_1 __attribute__ ((alloc_size (1))) +# else +# define ATTRIBUTE_RESULT_SIZE_1 +#endif +#endif + +#ifndef ATTRIBUTE_RESULT_SIZE_2 +# if (GCC_VERSION >= 4003) +# define ATTRIBUTE_RESULT_SIZE_2 __attribute__ ((alloc_size (2))) +# else +# define ATTRIBUTE_RESULT_SIZE_2 +#endif +#endif + +#ifndef ATTRIBUTE_RESULT_SIZE_1_2 +# if (GCC_VERSION >= 4003) +# define ATTRIBUTE_RESULT_SIZE_1_2 __attribute__ ((alloc_size (1, 2))) +# else +# define ATTRIBUTE_RESULT_SIZE_1_2 +#endif +#endif + +/* Attribute `warn_unused_result' was valid as of gcc 3.3. */ +#ifndef ATTRIBUTE_WARN_UNUSED_RESULT +# if GCC_VERSION >= 3003 +# define ATTRIBUTE_WARN_UNUSED_RESULT __attribute__ ((__warn_unused_result__)) +# else +# define ATTRIBUTE_WARN_UNUSED_RESULT +# endif +#endif + /* We use __extension__ in some places to suppress -pedantic warnings about GCC extensions. This feature didn't work properly before gcc 2.8. */ From 0028ae30f96c360c652ebb11ff4c65f8686e49da Mon Sep 17 00:00:00 2001 From: Takashi Yano Date: Sat, 3 Feb 2024 00:54:23 +0900 Subject: [PATCH 2/5] Cygwin: console: Avoid slipping past disable_master_thread check. If disable_master_thread flag is set between the code checking that flag not be set and the code acquiring input_mutex, input record is processed once after setting disable_master_thread flag. This patch prevents that. Fixes: d4aacd50e6cf ("Cygwin: console: Add missing input_mutex guard.") Signed-off-by: Takashi Yano Signed-off-by: Johannes Schindelin --- winsup/cygwin/fhandler/console.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc index c9b27c9c50..5028194a9e 100644 --- a/winsup/cygwin/fhandler/console.cc +++ b/winsup/cygwin/fhandler/console.cc @@ -361,6 +361,12 @@ fhandler_console::cons_master_thread (handle_set_t *p, tty *ttyp) } WaitForSingleObject (p->input_mutex, mutex_timeout); + /* Ensure accessing input recored is not disabled. */ + if (con.disable_master_thread) + { + ReleaseMutex (p->input_mutex); + continue; + } total_read = 0; switch (cygwait (p->input_handle, (DWORD) 0)) { From 915e1461ad44b52d413595211522e0d907411a43 Mon Sep 17 00:00:00 2001 From: Takashi Yano Date: Tue, 13 Feb 2024 11:17:46 +0900 Subject: [PATCH 3/5] Cygwin: pty: Fix handle leak in master process. If non-cygwin process is started in pty, closing from_master_nat pipe handle was missing in fhandler_pty_slave::input_transfer(). This occured because the handle was duplicated but not closed. https://github.com/msys2/msys2-runtime/issues/198 Fixes: 29431fcb5b14 ("Cygwin: pty: Inherit typeahead data between two input pipes.") Reported-by: Hakkin Lain Signed-off-by: Takashi Yano Signed-off-by: Johannes Schindelin --- winsup/cygwin/fhandler/pty.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc index dbeffc9e3f..9d1a119af9 100644 --- a/winsup/cygwin/fhandler/pty.cc +++ b/winsup/cygwin/fhandler/pty.cc @@ -4066,6 +4066,7 @@ fhandler_pty_slave::transfer_input (tty::xfer_dir dir, HANDLE from, tty *ttyp, transfered = true;; } } + CloseHandle (to); /* Fix input_available_event which indicates availability in cyg pipe. */ if (dir == tty::to_nat) /* all data is transfered to nat pipe, From 9bf75c45968582d2c5d19c8fe7879ccf4a0d4655 Mon Sep 17 00:00:00 2001 From: Takashi Yano Date: Tue, 13 Feb 2024 11:36:05 +0900 Subject: [PATCH 4/5] Cygwin: pty: Fix potential handle leak regarding CallNamedPipe(). In pty master_thread, 6 handles are duplicated when CallNamedPipe() requests that. Though some of them are not used so should be closed, they were not. This causes handle leak potentially. Signed-off-by: Takashi Yano Signed-off-by: Johannes Schindelin --- winsup/cygwin/fhandler/pty.cc | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc index 9d1a119af9..17b7b32df7 100644 --- a/winsup/cygwin/fhandler/pty.cc +++ b/winsup/cygwin/fhandler/pty.cc @@ -940,6 +940,8 @@ fhandler_pty_slave::open (int flags, mode_t) errmsg = "can't call master, %E"; goto err; } + CloseHandle (repl.to_slave_nat); /* not used. */ + CloseHandle (repl.to_slave); /* not used. */ from_master_nat_local = repl.from_master_nat; from_master_local = repl.from_master; to_master_nat_local = repl.to_master_nat; @@ -1218,6 +1220,10 @@ fhandler_pty_slave::reset_switch_to_nat_pipe (void) if (!CallNamedPipe (pipe, &req, sizeof req, &repl, sizeof repl, &len, 500)) return; /* What can we do? */ + CloseHandle (repl.from_master); /* not used. */ + CloseHandle (repl.to_master); /* not used. */ + CloseHandle (repl.to_slave_nat); /* not used. */ + CloseHandle (repl.to_slave); /* not used. */ CloseHandle (get_handle_nat ()); set_handle_nat (repl.from_master_nat); CloseHandle (get_output_handle_nat ()); @@ -3932,10 +3938,20 @@ fhandler_pty_slave::transfer_input (tty::xfer_dir dir, HANDLE from, tty *ttyp, if (!CallNamedPipe (pipe, &req, sizeof req, &repl, sizeof repl, &len, 500)) return; /* What can we do? */ + CloseHandle (repl.from_master_nat); /* not used. */ + CloseHandle (repl.from_master); /* not used. */ + CloseHandle (repl.to_master_nat); /* not used. */ + CloseHandle (repl.to_master); /* not used. */ if (dir == tty::to_nat) - to = repl.to_slave_nat; + { + CloseHandle (repl.to_slave); /* not used. */ + to = repl.to_slave_nat; + } else - to = repl.to_slave; + { + CloseHandle (repl.to_slave_nat); /* not used. */ + to = repl.to_slave; + } } UINT cp_from = 0, cp_to = 0; From c292b3d7df5938b08f7531357e3858ddcb186bb2 Mon Sep 17 00:00:00 2001 From: Takashi Yano Date: Tue, 13 Feb 2024 11:42:42 +0900 Subject: [PATCH 5/5] Cygwin: console: Make VMIN and VTIME work. Previously, VMIN and VTIME did not work at all. This patch fixes that. Signed-off-by: Takashi Yano Signed-off-by: Johannes Schindelin --- winsup/cygwin/fhandler/console.cc | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc index 5028194a9e..1fe72fc8b8 100644 --- a/winsup/cygwin/fhandler/console.cc +++ b/winsup/cygwin/fhandler/console.cc @@ -1011,10 +1011,14 @@ fhandler_console::read (void *pv, size_t& buflen) push_process_state process_state (PID_TTYIN); - int copied_chars = 0; + size_t copied_chars = 0; - DWORD timeout = is_nonblocking () ? 0 : INFINITE; + DWORD timeout = is_nonblocking () ? 0 : + (get_ttyp ()->ti.c_lflag & ICANON ? INFINITE : + (get_ttyp ()->ti.c_cc[VMIN] == 0 ? 0 : + (get_ttyp ()->ti.c_cc[VTIME]*100 ? : INFINITE))); +read_more: while (!input_ready && !get_cons_readahead_valid ()) { int bgres; @@ -1037,6 +1041,11 @@ fhandler_console::read (void *pv, size_t& buflen) pthread::static_cancel_self (); /*NOTREACHED*/ case WAIT_TIMEOUT: + if (copied_chars) + { + buflen = copied_chars; + return; + } set_sig_errno (EAGAIN); buflen = (size_t) -1; return; @@ -1082,19 +1091,20 @@ fhandler_console::read (void *pv, size_t& buflen) } /* Check console read-ahead buffer filled from terminal requests */ - while (con.cons_rapoi && *con.cons_rapoi && buflen) - { - buf[copied_chars++] = *con.cons_rapoi++; - buflen --; - } + while (con.cons_rapoi && *con.cons_rapoi && buflen > copied_chars) + buf[copied_chars++] = *con.cons_rapoi++; copied_chars += - get_readahead_into_buffer (buf + copied_chars, buflen); + get_readahead_into_buffer (buf + copied_chars, buflen - copied_chars); if (!con_ra.ralen) input_ready = false; release_input_mutex (); + if (buflen > copied_chars && !(get_ttyp ()->ti.c_lflag & ICANON) + && copied_chars < get_ttyp ()->ti.c_cc[VMIN]) + goto read_more; + #undef buf buflen = copied_chars;