In _cygtls::handle_SIGCONT(), the sig thread waits for the main thread
to process the signal without unlocking the TLS area. This causes a
deadlock if the main thread tries to acquire a lock for the TLS area
in the meantime. With this patch, unlock the TLS before calling yield()
in handle_SIGCONT().
Addresses: https://cygwin.com/pipermail/cygwin/2024-November/256744.html
Fixes: 26158dc3e9c2("* exceptions.cc (sigpacket::process): Lock _cygtls area of thread before accessing it.")
Reported-by: Christian Franke <Christian.Franke@t-online.de>
Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
The currently handled signal in a thread is called _cygtls::sig.
The variable name "sig" is used pretty often in the Cygwin source.
This makes it tricky to distinguish the currently handled signal
from any other usage of "sig".
Therefore, rename _cygtls::sig to _cygtls::current_sig
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
Previously, two bugs exist in sigtimedwait(). One is, that since
_my_tls.sigwait_mask was left non-zero if the signal arrives after
the timeout, sigpacket::process() would wrongly try to handle it.
The other is if a timeout occurs after sigpacket::process() is
called, but not completed yet, the signal handler can be called
accidentally. If the signal handler is set to SIG_DFL or SIG_IGN,
access violation will occur in both cases.
With this patch, in sigwait_common(), check if sigwait_mask == 0
to confirm that sigpacket::process() cleared it. In this case,
do not treat WAIT_TIMEOUT, but call cygwait() again to retrieve
the signal. Furthermore, sigpacket::process() checks whether
timeout occurs in sigwait_common() and if timeout already happens,
do not treat the signal as waited. In both cases, to avoid race
issues, the code is guarded by cygtls::lock().
Addresses: https://cygwin.com/pipermail/cygwin/2024-November/256762.html
Fixes: 24ff42d79a ("Cygwin: Implement sigtimedwait")
Reported-by: Christian Franke <Christian.Franke@t-online.de>
Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Fix warnings with gcc 12 about narrowing conversions of NTSTATUS
constants when used as case labels, e.g:
> ../../../../src/winsup/cygwin/exceptions.cc: In static member function ‘static int exception::handle(EXCEPTION_RECORD*, void*, CONTEXT*, PDISPATCHER_CONTEXT)’:
> ../../../../src/winsup/cygwin/exceptions.cc:670:10: error: narrowing conversion of ‘-1073741682’ from ‘NTSTATUS’ {aka ‘int’} to ‘unsigned int’ [-Wnarrowing]
See also: c5bdf60ac4
Signed-off-by: Jon Turney <jon.turney@dronecode.org.uk>
w32api 12.0.0 adds the returns_twice attribute to RtlCaptureContext().
There's some data-flow interaction with using it inside a while loop
which causes a maybe-uninitialized warning.
../../../../winsup/cygwin/exceptions.cc: In member function 'int _cygtls::call_signal_handler()':
../../../../winsup/cygwin/exceptions.cc:1720:33: error: '<anonymous>' may be used uninitialized in this function [-Werror=maybe-uninitialized]
A process which is exiting due to a core dumping signal doesn't
propagate the correct exist status after dumping core, because 'dumper'
itself forcibly terminates the process.
Use 'dumper -n' to avoid killing the dumped process, so we continue to
the end of signal_exit(), to exit with the 128+signal exit status.
Busy-wait in exec_prepared_command() in an attempt to reliably notice
the dumper attaching, so we don't get stuck there.
Also: document these important facts for custom uses of error_start.
Provide the same debugging opportunities for api_fatal() as we do for a
core-dumping signal:
1) Break into any attached debugger
2) Start JIT debugger (if configured) (keeping these under DEBUGGING doesn't seem helpful)
3) Write a coredump (if rlim_core > 1MB)
4) Write a stackdump (if that failed, or 0 < rlim_core <= 1MB)
Pre-format a command to be executed on a fatal error to run 'dumper'
(using an absolute path).
Factor out executing a pre-formatted command, so we can use that for
invoking the JIT debugger in try_to_debug() (if error_start is present
in the CYGWIN env var) and to invoke dumper when a fatal error occurs.
On a fatal error, if the core file size limit is greater than 1MB,
invoke dumper to write a core dump. Otherwise, if that limit is greater
than 0, write a .stackdump file, as previously.
Adjust and clarify the associated documentation.
Also: Fix so that the error_start JIT debugger is now invoked, even when
ulimit -c is zero.
Also: Fix uses of console_printf() inside exec_prepared_command(). It's
output is written via the Windows console device, so needs to use
Windows-style line endings.
Also: consistently return non-zero from try_to_debug() if we debugged.
Future work: Truncate or remove the file written, if it exceeds the
maximum size set by the ulimit.
Future work: Using the words "fatal error" could probably be improved
on. This means exiting on one of the "certain signals whose default
action is to cause the process to terminate and produce a core dump
file".
This patch replaces ctty constants with more descriptive macros
(CTTY_UNINITIALIZED and CTTY_RELEASED) rather than -1 and -2 as
well as checking sign with CTTY_IS_VALID().
Fixes: 3b7df69aaa (Cygwin: ctty: Add comments for the special values: -1 and -2.)
Suggested-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
This adds an extra section to the stackdump, which lists the loaded
modules and their base address. This is perhaps useful as it makes it
immediately clear if RandomCrashInjectedDll.dll is loaded...
Future work: It seems like the 'InMemoryOrder' part of
'InMemoryOrderModuleList' is a lie?
> Loaded modules
> 000100400000 segv-test.exe
> 7FFF2AC30000 ntdll.dll
> 7FFF29050000 KERNEL32.DLL
> 7FFF28800000 KERNELBASE.dll
> 000180040000 cygwin1.dll
> 7FFF28FA0000 advapi32.dll
> 7FFF29F20000 msvcrt.dll
> 7FFF299E0000 sechost.dll
> 7FFF29B30000 RPCRT4.dll
> 7FFF27C10000 CRYPTBASE.DLL
> 7FFF28770000 bcryptPrimitives.dll
This adds an additional column to the stack trace in a .stackdump file,
which gives the stack frame return address as a module name+offset. This
makes it a possible to convert the address to a function name without
having to guess what module the address belongs to.
> Stack trace:
> Frame Function Args
> 0007FFFFCC30 0001004010E9 (000180048055, 000180046FA0, 000000000002, 00018031E160) segv-test.exe+0x10E9
> 0007FFFFCD30 0001800480C1 (000000000000, 000000000000, 000000000000, 000000000000) cygwin1.dll+0x80C1
> 0007FFFFFFF0 000180045C86 (000000000000, 000000000000, 000000000000, 000000000000) cygwin1.dll+0x5C86
> 0007FFFFFFF0 000180045D34 (000000000000, 000000000000, 000000000000, 000000000000) cygwin1.dll+0x5D34
> End of stack trace
Loosely based on this patch [1] by Brian Dessent.
[1] https://cygwin.com/pipermail/cygwin-patches/2008q1/006306.html
Exception handling was *still* printing addresses as 44 bit values,
but Windows supports a 48 bit virtual address space since Windows
8.1. Fix that.
Fixes: e1254add73 ("Cygwin: Allow accessing 48 bit address space in Windows 8.1 or later")
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
These have no effect on x86_64. Retain a few occurrences of __cdecl
in files imported from other sources.
Also retain all occurrences of WINAPI, even though the latter is
simply a macro that expands to __stdcall. Most of these occurrences
are associated with Windows API functions, and removing them might
make the code confusing instead of simpler.
Leave x86_64 CPU-specific code and #error out when trying to build
for another target. Access special registers CPU-agnostic.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
- When Ctrl-C terminates a non-cygwin process on a pseudo console,
pty master attaches to the pseudo console first, and send
CTRL_C_EVENT. If the non-cygwin process closes the pseudo console
before the pty master calls FreeConsole(), the pty master process
will crash. With this patch, pty master process takes over the
ownership of the pseudo console, and closes it by myself.
- The recent commit "Cygwin: pinfo: Fix exit code when non-cygwin app
exits by Ctrl-C." did not fix enough the issue. If a non-cygwin app
is reading the console, it will not return STATUS_CONTROL_C_EXIT
even if it is terminated by Ctrl-C. As a result, the previous patch
does not take effect.
This patch solves this issue by setting sigExeced to SIGINT in
ctrl_c_handler(). In addition, sigExeced will be cleared if the app
does not terminated within predetermined time period. The reason is
that the app does not seem to be terminated by the signal sigExeced.
- The commit "Cygwin: console: Restore CTRL_BREAK_EVENT handling."
was accidentally mixed with experimental code in exceptions.cc.
Due to this, non-cygwin app receives CTRL_C_EVENT twice in the
following scenario.
1) Run 'sleep 10 | <non-cygwin app>'
2) Hit Ctrl-C.
3) The non-cygwin app receives CTRL_C_EVENT twice.
This patch reverts the code with the problem.
- The recent change by the commit "Cygwin: console: Redesign handling
of special keys." breaks the handling of CTRL_BREAK_EVENT. The login
shell in console exits on Ctrl-Break key. This patch fixes the issue.
- This patch commonize the code which processes special keys in pty
and console to improve maintanancibility. As a result, some small
bugs have been fixed.
- Currently, Ctrl-Z, Ctrl-\ and SIGWINCH does not work in console
if the process does not call read() or select(). This is because
these are processed in process_input_message() which is called
from read() or select(). This is a long standing issue of console.
Addresses:
https://cygwin.com/pipermail/cygwin/2020-May/244898.htmlhttps://cygwin.com/pipermail/cygwin/2021-February/247779.html
With this patch, new thread which handles only input signals is
introduced so that Crtl-Z, etc. work without calling read() or
select(). Ctrl-S and Ctrl-Q are also handled in this thread.
- Currently, thread created by pthread_create() is not suspended by
the signal SIGTSTP. For example, even if a process with a thread
is suspended by Ctrl-Z, the thread continues running. This patch
fixes the issue.
Currently, when using CYGWIN's error_start facility, the faulting
process isn't stopped while the error_start process is started when the
fault is caused by an exception. (it even seems possible in theory that
the faulting process could have exited before the error_start process
attaches).
This leads to e.g. the core dump written by CYGWIN='error_start=dumper'
in response to an exception being non-deterministic.
Remove the waitloop argument from try_to_debug(), only used in the
exception case, so the faulting process busy-waits until the error_start
process attaches.
Code archaeology to determine why the code is this way didn't really
turn up any answers, but this seems a low-risk change, as this only
changes the behaviour when:
- a debugger isn't already attached
- an error_start is specified in CYGWIN env var
- an exception has occurred which will be translated to a signal
If error_start invokes something which doesn't attach using
DebugActiveProcess(), we will spin indefinitely, but that will also
currently occur for any of the existing other uses of try_to_debug(),
which default to waitloop=TRUE.
This is necessary in order to be consistent with the following comment
in the definition of _Unwind_RaiseException() in the GCC source file
libgcc/unwind-seh.c:
The exception handler installed in crt0 will continue any GCC
exception that reaches there (and isn't marked non-continuable).
Previously we failed to do this and, as a consequence, the C++ runtime
didn't call std::terminate after an unhandled exception.
This fixes the problem reported here:
https://cygwin.com/pipermail/cygwin/2019-October/242795.htmlhttps://sourceware.org/pipermail/cygwin/2020-August/245897.html
This patch has been inspired by the Linux kernel patch
294f69e662d1 compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use
written by Joe Perches <joe AT perches DOT com> based on an idea from
Dan Carpenter <dan DOT carpenter AT oracle DOT com>. The following text
is from the original log message:
Reserve the pseudo keyword 'fallthrough' for the ability to convert the
various case block /* fallthrough */ style comments to appear to be an
actual reserved word with the same gcc case block missing fallthrough
warning capability.
All switch/case blocks now should end in one of:
break;
fallthrough;
goto <label>;
return [expression];
continue;
In C mode, GCC supports the __fallthrough__ attribute since 7.1,
the same time the warning and the comment parsing were introduced.
Cygwin-only: add an explicit -Wimplicit-fallthrough=5 to the build
flags.
NSIG is a deprecated symbol only visible under MISC visibility.
_NSIG is used widely instead, and on most systems NSIG is
defined in terms of _NSIG.
Follow suit: Change NSIG to _NSIG throughout and change visiblity
of NSIG to be defined only in __MISC_VISIBLE case.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
Rather than waiting for signalfd_select_wait in a thread, which is racy,
create a global event "my_pendingsigs_evt" which is set and reset by
wait_sig depending only on the fact if blocked signals are pending or not.
This in turn allows to WFMO on this event in select as soon as signalfds
are present in the read descriptor set. Select's peek and verify
will then check if one of the present signalfds is affected.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
On sigwaitinfo or reading from a signalfd, signal processing sets up
signal handling via sigdelayed even if the handler address is NULL.
This doesn't have any impact on sigwaitinfo scenarios (or at least, I
wasn't able to come up with a reproducer) but it breaks signalfd
scenarios, where eventually a call to call_signal_handler from
sigdelayed will try to call the NULL function.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
In case SA_SIGINFO flag is given, the signal handler may change
the context and the application is supposed to pick up from the
changed context. So far we don't do that, so the context given
to the signal handler is basically read-only, unless the signal
handler calls setcontext or swapcontext.
For a start, restore the thread's signal mask from the uc_sigmask
value of the context given to the signal handler.
If that's feasible for Cygwin, we restore the entire context from
the context changed by the signal handler in a followup patch.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
- Rename files timer.* to posix_timer.*.
- Reimplement using an OS timer rather than a handcrafted wait loop.
- Use a Slim R/W Lock for synchronization.
- Drop timer chaining. It doesn't server a purpose since all timers
are local only.
- Rename ttstart to itimer_tracker to better reflect its purpose.
It's not the anchor for a timer chain anymore anyway.
- Drop fixup_timers_after_fork. Everything is process-local, nothing
gets inherited.
- Rename timer_tracker::disarm_event to disarm_overrun_event for
better readability.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
Takashi Yano
Corinna Vinschen
Jon Turney
Ken Brown
Takashi Yano via Cygwin-patches
Mark Geisert