Skip to content
Closed
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
16 changes: 12 additions & 4 deletions src/coreclr/pal/src/exception/signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,16 +254,15 @@ Function :
Restore default signal handlers

Parameters :
None

isChildProcess - indicates that it is called from a child process fork
(no return value)

note :
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");

Expand All @@ -276,7 +275,16 @@ void SEHCleanupSignals()
restore_signal(SIGFPE, &g_previous_sigfpe);
restore_signal(SIGBUS, &g_previous_sigbus);
restore_signal(SIGABRT, &g_previous_sigabrt);
restore_signal(SIGSEGV, &g_previous_sigsegv);
#if !HAVE_MACH_EXCEPTIONS
// Do not restore the sigsegv when stack overflow was hit on a thread,
// since all other sigsegvs have to wait in the handler until the process
// dies.
// If it is called by a forked child though, restore the sigsegv unconditionally
if (isChildProcess || (g_stackOverflowHandlerStack != 0))
#endif
{
restore_signal(SIGSEGV, &g_previous_sigsegv);
}
restore_signal(SIGINT, &g_previous_sigint);
restore_signal(SIGQUIT, &g_previous_sigquit);
}
Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/pal/src/include/pal/signal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,11 @@ Function :
SEHCleanupSignals

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

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

#endif /* _PAL_SIGNAL_HPP_ */
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 @@ -2229,7 +2229,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 @@ -2524,7 +2524,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 */);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How high is the risk of reentrancy here? We probably don't want to end in a recursive case here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProcAbort is calling abort() right after this line, so I don't see how it could be reentered on the same thread.


// 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;

namespace TestStackOverflow
Expand All @@ -24,12 +25,26 @@ static bool TestStackOverflow(string testName, string testArgs, out List<string>
testProcess.StartInfo.Arguments = $"{Path.Combine(s_currentPath, "..", 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("Repeat ") ||
line.Data.StartsWith("------") ||
line.Data.StartsWith(" at "))
{
lines.Add(line.Data);
}
else
{
endOfStackTrace = true;
}
}
};

Expand Down Expand Up @@ -256,24 +271,28 @@ static int Main()
return 101;
}

if (!TestStackOverflowLargeFrameMainThread())
{
return 102;
}

if (!TestStackOverflowSmallFrameSecondaryThread())
{
return 103;
}

if (!TestStackOverflowLargeFrameSecondaryThread())
if ((RuntimeInformation.ProcessArchitecture != Architecture.Arm64) ||
((Environment.OSVersion.Platform != PlatformID.Unix) && (Environment.OSVersion.Platform != PlatformID.MacOSX)))
{
return 104;
}
if (!TestStackOverflowLargeFrameMainThread())
{
return 102;
}

if (!TestStackOverflow3())
{
return 105;
if (!TestStackOverflowLargeFrameSecondaryThread())
{
return 104;
}

if (!TestStackOverflow3())
{
return 105;
}
}

return 100;
Expand Down
3 changes: 0 additions & 3 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,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)/tracing/eventpipe/eventsourceerror/eventsourceerror/*">
<Issue>https://github.com/dotnet/runtime/issues/80666</Issue>
</ExcludeList>
Expand Down