eSupport] Don't modify the current EH context during stack unwinding

[rnk]

Summary

Copy it instead. Otherwise, key registers (such as RBP) may get zeroed
out by the stack unwinder.

Fixes CrashRecoveryTest.DumpStackCleanup with MSVC in release builds.

[stella.stamenova]

LGTM and I verified that the test now passes.

[merge_guards_bot]

Unit tests: pass. 62373 tests passed, 0 failed and 839 were skipped.

clang-tidy: fail. clang-tidy found 1 errors and 0 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[aganea]

Thanks Reid for looking into this! So I suppose the problem is related to llvm-symbolizer not being invoked somehow in Stella's case? This seems to occur if either that exe is not in %PATH% or if printSymbolizedStackTrace() can't create the input/output files for symbolizer. Was that the bug?
And it looks like the llvm-symbolizer codepath already duplicates the CONTEXT like you do here, but then the fallback Dbghelp.dll doesn't duplicate, which most likely caused this issue.

[aganea]

llvm/lib/Support/Windows/Signals.inc

234

This part is duplicating the context.

311

This one is not (and is inherently corrected by your fix).

Ideally it'd be nice to factor out the stackwalk code, duplicated behavior always ends up leading to bad things :-)

[hans]

Pushed to 10.x as dbe9c3a82dd7db8e50acb008f21a273c55fa5c82, see https://bugs.llvm.org/show_bug.cgi?id=44697#c16

llvm/lib/Support/Windows/Signals.inc

static bool printStackTraceWithLLVMSymbolizer(llvm::raw_ostream &OS,
                                               CONTEXT *ContextOrig) {
   // StackWalk64 modifies the incoming stack frame and context, so copy them.
   STACKFRAME64 StackFrame = StackFrameOrig;
 
   // Copy the register context so that we don't modify it while we unwind. We
   // could use InitializeContext + CopyContext, but that's only required to get
   // at AVX registers, which typically aren't needed by StackWalk64. Reduce the
   // flag set to indicate that there's less data.
   CONTEXT Context = *ContextOrig;
   Context.ContextFlags = CONTEXT_CONTROL | CONTEXT_INTEGER;
 
   static void *StackTrace[256];
   size_t Depth = 0;
   while (fStackWalk64(NativeMachineType, hProcess, hThread, &StackFrame,
                       &Context, 0, fSymFunctionTableAccess64,
                       fSymGetModuleBase64, 0)) {
     if (StackFrame.AddrFrame.Offset == 0)
static void PrintStackTraceForThread(llvm::raw_ostream &OS, HANDLE hProcess,
   // Try llvm-symbolizer first. llvm-symbolizer knows how to deal with both PDBs
   // and DWARF, so it should do a good job regardless of what debug info or
   // linker is in use.
   if (printStackTraceWithLLVMSymbolizer(OS, hProcess, hThread, StackFrame,
                                         Context)) {
     return;
   }
 
   while (true) {
     if (!fStackWalk64(NativeMachineType, hProcess, hThread, &StackFrame,
                       Context, 0, fSymFunctionTableAccess64,
                       fSymGetModuleBase64, 0)) {
       break;
     }
 
     if (StackFrame.AddrFrame.Offset == 0)
       break;
if (!llvm::sys::Process::AreCoreFilesPrevented()) {
     ExceptionInfo.ExceptionPointers = ep;
     ExceptionInfo.ClientPointers = FALSE;
 
     if (std::error_code EC = WriteWindowsDumpFile(&ExceptionInfo))
       llvm::errs() << "Could not write crash dump file: " << EC.message()
                    << "\n";
   }
 
-  LocalPrintStackTrace(llvm::errs(), ep ? ep->ContextRecord : nullptr);
+  // Stack unwinding appears to modify the context. Copy it to preserve the
+  // caller's context.
+  CONTEXT ContextCopy;
+  if (ep)
+    memcpy(&ContextCopy, ep->ContextRecord, sizeof(ContextCopy));
+
+  LocalPrintStackTrace(llvm::errs(), ep ? &ContextCopy : nullptr);
 
   return EXCEPTION_EXECUTE_HANDLER;
 }
 
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) {
   // We are running in our very own thread, courtesy of Windows.
   EnterCriticalSection(&CriticalSection);
   Cleanup();