From 3c9f4357676b63a321b187ee67001a9eb531f37f Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 30 Apr 2020 12:24:46 +0200 Subject: [PATCH 1/2] Fix Unix stack overflow trace logging for native failure locations When stack overflow happens in native runtime code (e.g. in one of the QCALLs, FCALLs etc.), we do not print stack trace to the console and we fail fast with just a "Stack overflow" message. The reason is that the stack overflow handling is executed on the failing thread and it is not safe to call methods that are not async safe (reentrant). The reason is that we could end up hanging or crashing when trying to report the stack overflow. However, we have the same potential issue on Windows and yet we try to report the stack trace anyways. So this change modifies the behavior on Unix to be the same. I have also added a bunch of stack overflow reporting tests. --- .../src/pal/src/exception/machexception.cpp | 5 + src/coreclr/src/pal/src/exception/seh.cpp | 18 -- src/coreclr/src/pal/src/exception/signal.cpp | 16 +- src/coreclr/src/vm/exceptionhandling.cpp | 2 + .../exceptions/stackoverflow/stackoverflow.cs | 149 ++++++++++ .../stackoverflow/stackoverflow.csproj | 12 + .../stackoverflow/stackoverflow3.cs | 30 ++ .../stackoverflow/stackoverflow3.csproj | 13 + .../stackoverflow/stackoverflowtester.cs | 268 ++++++++++++++++++ .../stackoverflow/stackoverflowtester.csproj | 12 + 10 files changed, 505 insertions(+), 20 deletions(-) create mode 100644 src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.cs create mode 100644 src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.csproj create mode 100644 src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.cs create mode 100644 src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.csproj create mode 100644 src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.cs create mode 100644 src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.csproj diff --git a/src/coreclr/src/pal/src/exception/machexception.cpp b/src/coreclr/src/pal/src/exception/machexception.cpp index aa7d7018f51b26..4acafddbce74ed 100644 --- a/src/coreclr/src/pal/src/exception/machexception.cpp +++ b/src/coreclr/src/pal/src/exception/machexception.cpp @@ -840,6 +840,11 @@ HijackFaultingThread( ts64.__rflags &= ~EFL_TF; } + if (fIsStackOverflow) + { + exceptionRecord.ExceptionCode = EXCEPTION_STACK_OVERFLOW; + } + exceptionRecord.ExceptionFlags = EXCEPTION_IS_SIGNAL; exceptionRecord.ExceptionRecord = NULL; exceptionRecord.ExceptionAddress = (void *)ts64.__rip; diff --git a/src/coreclr/src/pal/src/exception/seh.cpp b/src/coreclr/src/pal/src/exception/seh.cpp index f194156509d49f..ee022b45fa7ce3 100644 --- a/src/coreclr/src/pal/src/exception/seh.cpp +++ b/src/coreclr/src/pal/src/exception/seh.cpp @@ -264,24 +264,6 @@ SEHProcessException(PAL_SEHException* exception) // or in a jitter helper or it is a debugger breakpoint) if (g_safeExceptionCheckFunction(contextRecord, exceptionRecord)) { - if (exceptionRecord->ExceptionCode == EXCEPTION_ACCESS_VIOLATION) - { - // Check if the failed access has hit a stack guard page. In such case, it - // was a stack probe that detected that there is not enough stack left. - void* stackLimit = CPalThread::GetStackLimit(); - void* stackOverflowBottom = (void*)((size_t)stackLimit - GetVirtualPageSize()); - // On some versions of glibc / platforms the stackLimit is an address of the guard page, on some - // it is right above the guard page. - // So consider SIGSEGV in one page above and below stack limit to be stack overflow. - void* stackOverflowTop = (void*)((size_t)stackLimit + GetVirtualPageSize()); - void* violationAddr = (void*)exceptionRecord->ExceptionInformation[1]; - - if ((violationAddr >= stackOverflowBottom) && (violationAddr < stackOverflowTop)) - { - exceptionRecord->ExceptionCode = EXCEPTION_STACK_OVERFLOW; - } - } - EnsureExceptionRecordsOnHeap(exception); if (g_hardwareExceptionHandler(exception)) { diff --git a/src/coreclr/src/pal/src/exception/signal.cpp b/src/coreclr/src/pal/src/exception/signal.cpp index 27ea7c2df015d0..b142bbd82f77b9 100644 --- a/src/coreclr/src/pal/src/exception/signal.cpp +++ b/src/coreclr/src/pal/src/exception/signal.cpp @@ -116,6 +116,10 @@ int g_common_signal_handler_context_locvar_offset = 0; // TOP of special stack for handling stack overflow volatile void* g_stackOverflowHandlerStack = NULL; + +// Flag that is or-ed with SIGSEGV to indicate that the SIGSEGV was a stack overflow +const int StackOverflowFlag = 0x40000000; + #endif // !HAVE_MACH_EXCEPTIONS /* public function definitions ************************************************/ @@ -529,7 +533,7 @@ static void sigsegv_handler(int code, siginfo_t *siginfo, void *context) } } - if (SwitchStackAndExecuteHandler(code, siginfo, context, (size_t)handlerStackTop)) + if (SwitchStackAndExecuteHandler(code | StackOverflowFlag, siginfo, context, (size_t)handlerStackTop)) { PROCAbort(); } @@ -841,7 +845,15 @@ static bool common_signal_handler(int code, siginfo_t *siginfo, void *sigcontext ucontext = (native_context_t *)sigcontext; g_common_signal_handler_context_locvar_offset = (int)((char*)&signalContextRecord - (char*)__builtin_frame_address(0)); - exceptionRecord.ExceptionCode = CONTEXTGetExceptionCodeForSignal(siginfo, ucontext); + if (code == (SIGSEGV | StackOverflowFlag)) + { + exceptionRecord.ExceptionCode = EXCEPTION_STACK_OVERFLOW; + code &= ~StackOverflowFlag; + } + else + { + exceptionRecord.ExceptionCode = CONTEXTGetExceptionCodeForSignal(siginfo, ucontext); + } exceptionRecord.ExceptionFlags = EXCEPTION_IS_SIGNAL; exceptionRecord.ExceptionRecord = NULL; exceptionRecord.ExceptionAddress = GetNativeContextPC(ucontext); diff --git a/src/coreclr/src/vm/exceptionhandling.cpp b/src/coreclr/src/vm/exceptionhandling.cpp index e8f835a43b6c8c..1612f395b1750c 100644 --- a/src/coreclr/src/vm/exceptionhandling.cpp +++ b/src/coreclr/src/vm/exceptionhandling.cpp @@ -5157,6 +5157,7 @@ BOOL IsSafeToHandleHardwareException(PCONTEXT contextRecord, PEXCEPTION_RECORD e return g_fEEStarted && ( exceptionRecord->ExceptionCode == STATUS_BREAKPOINT || exceptionRecord->ExceptionCode == STATUS_SINGLE_STEP || + exceptionRecord->ExceptionCode == STATUS_STACK_OVERFLOW || (IsSafeToCallExecutionManager() && ExecutionManager::IsManagedCode(controlPc)) || IsIPinVirtualStub(controlPc) || // access violation comes from DispatchStub of Interface call IsIPInMarkedJitHelper(controlPc)); @@ -5192,6 +5193,7 @@ BOOL HandleHardwareException(PAL_SEHException* ex) if (ex->GetExceptionRecord()->ExceptionCode == EXCEPTION_STACK_OVERFLOW) { GetThread()->SetExecutingOnAltStack(); + Thread::VirtualUnwindToFirstManagedCallFrame(ex->GetContextRecord()); EEPolicy::HandleFatalStackOverflow(&ex->ExceptionPointers, FALSE); UNREACHABLE(); } diff --git a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.cs b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.cs new file mode 100644 index 00000000000000..9a6a45329b90a8 --- /dev/null +++ b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.cs @@ -0,0 +1,149 @@ +using System; +using System.Threading; + +namespace TestStackOverflow +{ + struct LargeStruct256 + { + Guid g0; + Guid g1; + Guid g2; + Guid g3; + Guid g4; + Guid g5; + Guid g6; + Guid g7; + Guid g8; + Guid g9; + Guid ga; + Guid gb; + Guid gc; + Guid gd; + Guid ge; + Guid gf; + } + + struct LargeStruct4096 + { + LargeStruct256 s0; + LargeStruct256 s1; + LargeStruct256 s2; + LargeStruct256 s3; + LargeStruct256 s4; + LargeStruct256 s5; + LargeStruct256 s6; + LargeStruct256 s7; + LargeStruct256 s8; + LargeStruct256 s9; + LargeStruct256 sa; + LargeStruct256 sb; + LargeStruct256 sc; + LargeStruct256 sd; + LargeStruct256 se; + LargeStruct256 sf; + } + + struct LargeStruct65536 + { + LargeStruct4096 s0; + LargeStruct4096 s1; + LargeStruct4096 s2; + LargeStruct4096 s3; + LargeStruct4096 s4; + LargeStruct4096 s5; + LargeStruct4096 s6; + LargeStruct4096 s7; + LargeStruct4096 s8; + LargeStruct4096 s9; + LargeStruct4096 sa; + LargeStruct4096 sb; + LargeStruct4096 sc; + LargeStruct4096 sd; + LargeStruct4096 se; + LargeStruct4096 sf; + } + class Program + { + static void InfiniteRecursionA() + { + InfiniteRecursionB(); + } + + static void InfiniteRecursionB() + { + InfiniteRecursionC(); + } + static void InfiniteRecursionC() + { + InfiniteRecursionA(); + } + + static void InfiniteRecursionA2() + { + LargeStruct65536 s; + InfiniteRecursionB2(); + } + + static void InfiniteRecursionB2() + { + LargeStruct65536 s; + InfiniteRecursionC2(); + } + + static void InfiniteRecursionC2() + { + LargeStruct65536 s; + InfiniteRecursionA2(); + } + + static void MainThreadTest(bool smallframe) + { + if (smallframe) + { + InfiniteRecursionA(); + } + else + { + InfiniteRecursionA2(); + } + } + + static void SecondaryThreadsTest(bool smallframe) + { + Thread[] threads = new Thread[32]; + for (int i = 0; i < threads.Length; i++) + { + threads[i] = new Thread(() => { + if (smallframe) + { + InfiniteRecursionA(); + } + else + { + InfiniteRecursionA2(); + } + }); + threads[i].Start(); + } + + for (int i = 0; i < threads.Length; i++) + { + threads[i].Join(); + } + } + + static void Main(string[] args) + { + bool smallframe = (args[0] == "smallframe"); + if (args[1] == "secondary") + { + SecondaryThreadsTest(smallframe); + } + else if (args[1] == "main") + { + MainThreadTest(smallframe); + } + } + } +} + diff --git a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.csproj b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.csproj new file mode 100644 index 00000000000000..119fceb14efc96 --- /dev/null +++ b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.csproj @@ -0,0 +1,12 @@ + + + Exe + false + BuildOnly + 0 + + + + + + diff --git a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.cs b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.cs new file mode 100644 index 00000000000000..c3750864fa03eb --- /dev/null +++ b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.cs @@ -0,0 +1,30 @@ +using System; + +namespace TestStackOverflow3 +{ + class Program + { + private const int MAX_RECURSIVE_CALLS = 1000000; + static int ctr = 0; + + public static void Main() + { + Program ex = new Program(); + ex.Execute(); + } + + private unsafe void Execute(string arg1 = "") + { + long* bar = stackalloc long [1000]; + ctr++; + if (ctr % 50 == 0) + Console.WriteLine("Call number {0} to the Execute method", ctr); + + if (ctr <= MAX_RECURSIVE_CALLS) + Execute(string.Format("{0}", (IntPtr)bar)); + + ctr--; + } + } +} + diff --git a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.csproj b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.csproj new file mode 100644 index 00000000000000..d75dd6ecbbe3d6 --- /dev/null +++ b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.csproj @@ -0,0 +1,13 @@ + + + Exe + false + true + BuildOnly + 0 + + + + + + diff --git a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.cs b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.cs new file mode 100644 index 00000000000000..b2378850507f3b --- /dev/null +++ b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.cs @@ -0,0 +1,268 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Text; + +namespace TestStackOverflow +{ + class Program + { + static string s_corerunPath; + static string s_currentPath; + + static bool TestStackOverflow(string testName, string testArgs, out List stderrLines) + { + Console.WriteLine($"Running {testName} test({testArgs})"); + List lines = new List(); + + Process testProcess = new Process(); + + testProcess.StartInfo.FileName = s_corerunPath; + testProcess.StartInfo.Arguments = $"{Path.Combine(s_currentPath, "..", testName, $"{testName}.dll")} {testArgs}"; + testProcess.StartInfo.UseShellExecute = false; + testProcess.StartInfo.RedirectStandardError = true; + testProcess.ErrorDataReceived += (sender, line) => + { + Console.WriteLine($"\"{line.Data}\""); + if (!string.IsNullOrEmpty(line.Data)) + { + lines.Add(line.Data); + } + }; + + testProcess.Start(); + testProcess.BeginErrorReadLine(); + testProcess.WaitForExit(); + testProcess.CancelErrorRead(); + + stderrLines = lines; + + int expectedExitCode; + if ((Environment.OSVersion.Platform == PlatformID.Unix) || (Environment.OSVersion.Platform == PlatformID.MacOSX)) + { + expectedExitCode = 128 + 6; + } + else + { + expectedExitCode = unchecked((int)0xC00000FD); + } + + if (testProcess.ExitCode != expectedExitCode) + { + Console.WriteLine($"Exit code: 0x{testProcess.ExitCode:X8}, expected 0x{expectedExitCode:X8}"); + return false; + } + + if (lines[0] != "Stack overflow.") + { + Console.WriteLine("Missing \"Stack overflow.\" at the first line"); + return false; + } + + return true; + } + + static bool TestStackOverflowSmallFrameMainThread() + { + List lines; + if (TestStackOverflow("stackoverflow", "smallframe main", out lines)) + { + if (!lines[lines.Count - 1].EndsWith("at TestStackOverflow.Program.Main(System.String[])")) + { + Console.WriteLine("Missing \"Main\" method frame at the last line"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.InfiniteRecursionA()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionA\" method frame"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.InfiniteRecursionB()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionB\" method frame"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.InfiniteRecursionC()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionC\" method frame"); + return false; + } + + return true; + } + + return false; + } + + static bool TestStackOverflowLargeFrameMainThread() + { + List lines; + if (TestStackOverflow("stackoverflow", "largeframe main", out lines)) + { + if (!lines[lines.Count - 1].EndsWith("at TestStackOverflow.Program.Main(System.String[])")) + { + Console.WriteLine("Missing \"Main\" method frame at the last line"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("TestStackOverflow.Program.MainThreadTest(Boolean)"))) + { + Console.WriteLine("Missing \"MainThreadTest\" method frame"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.InfiniteRecursionA2()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionA2\" method frame"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.InfiniteRecursionB2()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionB2\" method frame"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.InfiniteRecursionC2()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionC2\" method frame"); + return false; + } + + return true; + } + + return false; + } + + static bool TestStackOverflowSmallFrameSecondaryThread() + { + List lines; + if (TestStackOverflow("stackoverflow", "smallframe secondary", out lines)) + { + if (!lines[lines.Count - 1].EndsWith("at System.Threading.ThreadHelper.ThreadStart()")) + { + Console.WriteLine("Missing \"System.Threading.ThreadHelper.ThreadStart\" method frame at the last line"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.InfiniteRecursionA()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionA\" method frame"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.InfiniteRecursionB()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionB\" method frame"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.InfiniteRecursionC()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionC\" method frame"); + return false; + } + + return true; + } + + return false; + } + + static bool TestStackOverflowLargeFrameSecondaryThread() + { + List lines; + if (TestStackOverflow("stackoverflow", "largeframe secondary", out lines)) + { + if (!lines[lines.Count - 1].EndsWith("at System.Threading.ThreadHelper.ThreadStart()")) + { + Console.WriteLine("Missing \"System.Threading.ThreadHelper.ThreadStart\" method frame at the last line"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.InfiniteRecursionA2()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionA2\" method frame"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("TestStackOverflow.Program.InfiniteRecursionB2()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionB2\" method frame"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("TestStackOverflow.Program.InfiniteRecursionC2()"))) + { + Console.WriteLine("Missing \"InfiniteRecursionC2\" method frame"); + return false; + } + + return true; + } + + return false; + } + + static bool TestStackOverflow3() + { + List lines; + if (TestStackOverflow("stackoverflow3", "", out lines)) + { + if (!lines[lines.Count - 1].EndsWith("at TestStackOverflow3.Program.Main()")) + { + Console.WriteLine("Missing \"Main\" method frame at the last line"); + return false; + } + + if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow3.Program.Execute(System.String)"))) + { + Console.WriteLine("Missing \"Execute\" method frame"); + return false; + } + + return true; + } + + return false; + } + + static int Main(string[] args) + { + s_currentPath = Directory.GetCurrentDirectory(); + s_corerunPath = Path.Combine(Environment.GetEnvironmentVariable("CORE_ROOT"), "corerun"); + + if (!TestStackOverflowSmallFrameMainThread()) + { + return 101; + } + + if (!TestStackOverflowLargeFrameMainThread()) + { + return 102; + } + + if (!TestStackOverflowSmallFrameSecondaryThread()) + { + return 103; + } + + if (!TestStackOverflowLargeFrameSecondaryThread()) + { + return 104; + } + + if (!TestStackOverflow3()) + { + return 105; + } + + return 100; + } + } +} diff --git a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.csproj b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.csproj new file mode 100644 index 00000000000000..5cb94c1225699b --- /dev/null +++ b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.csproj @@ -0,0 +1,12 @@ + + + Exe + false + BuildAndRun + 0 + + + + + + From d383094b0230e4390a41181f9c0bdb6253d406ab Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 30 Apr 2020 18:00:13 +0200 Subject: [PATCH 2/2] Add license headers to the tests --- .../src/baseservices/exceptions/stackoverflow/stackoverflow.cs | 3 +++ .../baseservices/exceptions/stackoverflow/stackoverflow3.cs | 3 +++ .../exceptions/stackoverflow/stackoverflowtester.cs | 3 +++ 3 files changed, 9 insertions(+) diff --git a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.cs b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.cs index 9a6a45329b90a8..2cb9aa6ae82f49 100644 --- a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.cs +++ b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow.cs @@ -1,3 +1,6 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. using System; using System.Threading; diff --git a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.cs b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.cs index c3750864fa03eb..bb53a0fb324c98 100644 --- a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.cs +++ b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflow3.cs @@ -1,3 +1,6 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. using System; namespace TestStackOverflow3 diff --git a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.cs b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.cs index b2378850507f3b..f30305658c1007 100644 --- a/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.cs +++ b/src/coreclr/tests/src/baseservices/exceptions/stackoverflow/stackoverflowtester.cs @@ -1,3 +1,6 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. using System; using System.Collections.Generic; using System.Diagnostics;