On 12/01/2024 14:09, Jon Turney wrote:
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.
So, there is a small problem with this change: because dumper itself
terminates the dumped process, it doesn't go on to exit with the
signal+128 exit status.
(In fact, it seems to exit with status 0 when terminated by an attached
debugger terminating, which isn't great)
That's relatively easy to fix: just use the '-n' option to dumper so it
detaches before exiting, to prevent that terminating the dumped process,
but then we run into the difficulties of reliably detecting that dumper
has attached and done it's work, so it's safe for us to exit.
Attached patch does that, and documents the expectations on the
error_start command a bit more clearly.
Even then this is clearly not totally bullet-proof. Maybe the right
thing to do is add a suitable timeout here, so even if we fail to notice
the DebugActiveProcess() (or there's a custom JIT debugger which just
writes the fact a process crashed to a logfile or something), we'll exit
eventually?
From 85120a1697294cd93ff68f6b1840145251ce4185 Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.tur...@dronecode.org.uk>
Date: Tue, 16 Jan 2024 16:12:51 +0000
Subject: [PATCH] Cygwin: Don't terminate via dumper
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.
---
winsup/cygwin/exceptions.cc | 7 +++----
winsup/doc/cygwinenv.xml | 6 ++++++
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 8b1c5493e..0e1a804ca 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -149,7 +149,7 @@ dumper_init (void)
/* Calculate the length of the command, allowing for an appended DWORD PID
and
terminating null */
- int cmd_len = 1 + wcslen(dll_dir) + 11 + 2 + 1 + wcslen(global_progname) + 1
+ 10 + 1;
+ int cmd_len = 1 + wcslen(dll_dir) + 11 + 5 + 1 + wcslen(global_progname) + 1
+ 10 + 1;
if (cmd_len > 32767)
{
/* If this comes to more than the 32,767 characters CreateProcess() can
@@ -163,7 +163,7 @@ dumper_init (void)
cp = wcpcpy (cp, L"\"");
cp = wcpcpy (cp, dll_dir);
cp = wcpcpy (cp, L"\\dumper.exe");
- cp = wcpcpy (cp, L"\" ");
+ cp = wcpcpy (cp, L"\" -n ");
cp = wcpcpy (cp, L"\"");
cp = wcpcpy (cp, global_progname);
wcscat (cp, L"\"");
@@ -570,9 +570,8 @@ int exec_prepared_command (PWCHAR command)
system_printf ("Failed to start, %E");
else
{
- SetThreadPriority (GetCurrentThread (), THREAD_PRIORITY_IDLE);
while (!being_debugged ())
- Sleep (1);
+ Sleep (0);
Sleep (2000);
}
diff --git a/winsup/doc/cygwinenv.xml b/winsup/doc/cygwinenv.xml
index d97f2b77d..05672c404 100644
--- a/winsup/doc/cygwinenv.xml
+++ b/winsup/doc/cygwinenv.xml
@@ -46,6 +46,12 @@ to the command as arguments.
Note: This has no effect if a debugger is already attached when the fatal
error occurs.
</para>
+<para>
+ Note: The command invoked must either (i) attach to the errored process with
+ <function>DebugActiveProcess()</function>, or (ii) forcibly terminate the
+ errored process (with <function>TerminateProcess()</function> or similar), as
+ otherwise the errored process will wait forever for a debugger to attach.
+</para>
</listitem>
<listitem>
--
2.43.0
