Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix shutdown with multiple unhandled HW exceptions #105578

Merged
merged 1 commit into from
Jul 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/pal/src/exception/seh.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ SEHCleanup()
{
TRACE("Cleaning up SEH\n");

SEHCleanupSignals();
SEHCleanupSignals(false /* isChildProcess */);
}

/*++
Expand Down
44 changes: 26 additions & 18 deletions src/coreclr/pal/src/exception/signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ Function :
Restore default signal handlers

Parameters :
None
isChildProcess - indicates that it is called from a child process fork

(no return value)

Expand All @@ -288,34 +288,42 @@ reason for this function is that during PAL_Terminate, we reach a point where
SEH isn't possible anymore (handle manager is off, etc). Past that point,
we can't avoid crashing on a signal.
--*/
void SEHCleanupSignals()
void SEHCleanupSignals(bool isChildProcess)
{
TRACE("Restoring default signal handlers\n");

if (g_registered_signal_handlers)
if (isChildProcess)
{
restore_signal(SIGILL, &g_previous_sigill);
if (g_registered_signal_handlers)
{
restore_signal(SIGILL, &g_previous_sigill);
#if !HAVE_MACH_EXCEPTIONS
restore_signal(SIGTRAP, &g_previous_sigtrap);
restore_signal(SIGTRAP, &g_previous_sigtrap);
#endif
restore_signal(SIGFPE, &g_previous_sigfpe);
restore_signal(SIGBUS, &g_previous_sigbus);
restore_signal(SIGABRT, &g_previous_sigabrt);
restore_signal(SIGSEGV, &g_previous_sigsegv);
restore_signal(SIGINT, &g_previous_sigint);
restore_signal(SIGQUIT, &g_previous_sigquit);
}
restore_signal(SIGFPE, &g_previous_sigfpe);
restore_signal(SIGBUS, &g_previous_sigbus);
restore_signal(SIGSEGV, &g_previous_sigsegv);
restore_signal(SIGINT, &g_previous_sigint);
restore_signal(SIGQUIT, &g_previous_sigquit);
}

#ifdef INJECT_ACTIVATION_SIGNAL
if (g_registered_activation_handler)
{
restore_signal(INJECT_ACTIVATION_SIGNAL, &g_previous_activation);
}
if (g_registered_activation_handler)
{
restore_signal(INJECT_ACTIVATION_SIGNAL, &g_previous_activation);
}
#endif

if (g_registered_sigterm_handler)
if (g_registered_sigterm_handler)
{
restore_signal(SIGTERM, &g_previous_sigterm);
}
}

// Restore only the SIGABRT so that abort that ends up the process can actually end it
if (g_registered_signal_handlers)
{
restore_signal(SIGTERM, &g_previous_sigterm);
restore_signal(SIGABRT, &g_previous_sigabrt);
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/pal/src/include/pal/signal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,12 @@ Function :
SEHCleanupSignals

Restore default signal handlers
Parameters :
isChildProcess - indicates that it is called from a child process fork

(no parameters, no return value)
--*/
void SEHCleanupSignals();
void SEHCleanupSignals(bool isChildProcess);

/*++
Function :
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/pal/src/thread/process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2257,7 +2257,7 @@ PROCCreateCrashDump(
if (g_createdumpCallback != nullptr)
{
// Remove the signal handlers inherited from the runtime process
SEHCleanupSignals();
SEHCleanupSignals(true /* isChildProcess */);

// Call the statically linked createdump code
g_createdumpCallback(argv.size(), argv.data());
Expand Down Expand Up @@ -2556,7 +2556,7 @@ PROCAbort(int signal, siginfo_t* siginfo)

// Restore all signals; the SIGABORT handler to prevent recursion and
// the others to prevent multiple core dumps from being generated.
SEHCleanupSignals();
SEHCleanupSignals(false /* isChildProcess */);

// Abort the process after waiting for the core dump to complete
abort();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Runtime.InteropServices;
using System.Text;
using Xunit;

Expand All @@ -22,12 +23,27 @@ static void TestStackOverflow(string testName, string testArgs, out List<string>
testProcess.StartInfo.Arguments = $"{Path.Combine(Directory.GetCurrentDirectory(), "..", testName, $"{testName}.dll")} {testArgs}";
testProcess.StartInfo.UseShellExecute = false;
testProcess.StartInfo.RedirectStandardError = true;
testProcess.StartInfo.Environment.Add("DOTNET_DbgEnableMiniDump", "0");
bool endOfStackTrace = false;

testProcess.ErrorDataReceived += (sender, line) =>
{
Console.WriteLine($"\"{line.Data}\"");
if (!string.IsNullOrEmpty(line.Data))
if (!endOfStackTrace && !string.IsNullOrEmpty(line.Data))
{
lines.Add(line.Data);
// Store lines only till the end of the stack trace.
// In the CI it can also contain lines with createdump info.
if (line.Data.StartsWith("Stack overflow.") ||
line.Data.StartsWith("Repeated ") ||
line.Data.StartsWith("------") ||
line.Data.StartsWith(" at "))
{
lines.Add(line.Data);
}
else
{
endOfStackTrace = true;
}
}
};

Expand Down Expand Up @@ -99,6 +115,15 @@ public static void TestStackOverflowSmallFrameMainThread()
[Fact]
public static void TestStackOverflowLargeFrameMainThread()
{
if ((RuntimeInformation.ProcessArchitecture == Architecture.Arm64) &&
((Environment.OSVersion.Platform == PlatformID.Unix) || (Environment.OSVersion.Platform == PlatformID.MacOSX)))
{
// Disabled on Unix ARM64 due to https://github.com/dotnet/runtime/issues/13519
// The current stack probing doesn't move the stack pointer and so the runtime sometimes cannot
// recognize the underlying sigsegv as stack overflow when it probes too far from SP.
return;
}

TestStackOverflow("stackoverflow", "largeframe main", out List<string> lines);

if (!lines[lines.Count - 1].EndsWith("at TestStackOverflow.Program.Main(System.String[])"))
Expand Down Expand Up @@ -156,6 +181,15 @@ public static void TestStackOverflowSmallFrameSecondaryThread()
[Fact]
public static void TestStackOverflowLargeFrameSecondaryThread()
{
if ((RuntimeInformation.ProcessArchitecture == Architecture.Arm64) &&
((Environment.OSVersion.Platform == PlatformID.Unix) || (Environment.OSVersion.Platform == PlatformID.MacOSX)))
{
// Disabled on Unix ARM64 due to https://github.com/dotnet/runtime/issues/13519
// The current stack probing doesn't move the stack pointer and so the runtime sometimes cannot
// recognize the underlying sigsegv as stack overflow when it probes too far from SP.
return;
}

TestStackOverflow("stackoverflow", "largeframe secondary", out List<string> lines);

if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.Test(Boolean)")))
Expand Down
3 changes: 0 additions & 3 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@

<!-- All Unix targets on CoreCLR Runtime -->
<ItemGroup Condition="'$(XunitTestBinBase)' != '' and '$(TargetsWindows)' != 'true' and '$(RuntimeFlavor)' == 'coreclr' ">
<ExcludeList Include="$(XunitTestBinBase)/baseservices/exceptions/stackoverflow/stackoverflowtester/*">
<Issue>https://github.com/dotnet/runtime/issues/46175</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/baseservices/typeequivalence/signatures/nopiatestil/*">
<Issue>CoreCLR doesn't support type equivalence on Unix</Issue>
</ExcludeList>
Expand Down
Loading