[windows][support] Improve backtrace emitted in crash report without llvm-symbolizer

[bd1976llvm]

Summary

Currently the backtrace emitted on windows when llvm-symbolizer is not available looks like:

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.      Program arguments: c:\\u\\br2\\bin\\ld.lld.exe -o C:\\Users\\BDUNBO~1\\AppData\\Local\\Temp\\lit-tmp-8ymp966z\\tmpztn1fyw1
0x00007FF64EE51D20 (0x0000000000000083 0x00007FF64EEFCAA1 0x0000000000000000 0x000001750CD01140)
0x00007FF64EEFC9DB (0x0000000000000000 0x0000000703B8EBA9 0x0000017603357418 0x0000000000000001)
...

This is not useful in many circumstance as the program counter (RIP) (the numbers in the left-most column) cannot be easily decoded because the addresses have the containing module's run-time base address added into them, but we don't know what those base addresses are. This change emits a module offset rather than an address.

Example output after this change:

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.      Program arguments: c:\\u\\br2\\bin\\ld.lld.exe -o C:\\Users\\BDUNBO~1\\AppData\\Local\\Temp\\lit-tmp-8ymp966z\\tmpztn1fyw1
0x00007FF64EE51D20 (0x0000000000000083 0x00007FF64EEFCAA1 0x0000000000000000 0x000001750CD01140), c:\u\br2\bin\ld.lld.exe(0x0000000000B70000) + 0x17E8F7 byte(s)
0x00007FF64EEFC9DB (0x0000000000000000 0x0000000703B8EBA9 0x0000017603357418 0x0000000000000001), C:\WINDOWS\SYSTEM32\ntdll.dll(0x0000000077560000) + 0x46E2C byte(s)
...

Note that the above output is an example of the output when file and line information cannot be retrieved for symbols. This is reasonably likely, for example if the .pdb files were not shipped with the toolchain binaries. If the file and line info can be retrieved lines like the following would be output:

0x00007FF64EE51D20 (0x0000000000000083 0x00007FF64EEFCAA1 0x0000000000000000 0x000001750CD01140), C:\WINDOWS\SYSTEM32\ntdll.dll(0x0000000077560000) + 0x46E2C byte(s), RtlAllocateHeap() + 0x109C byte(s)

[rnk]

Seems useful

llvm/lib/Support/Windows/Signals.inc

339–340

I propose we scrap this parameter printing. I've never found it useful. It's a bunch of hex noise that makes the crash report look scary. My understanding is that doesn't work well on win64, since it assumes parameters are homed into the shadow stack space, which Clang doesn't generally do.

[bd1976llvm]

llvm/lib/Support/Windows/Signals.inc

339–340

Yes. Nice suggestion. We recently debugged out a horrible close-to-impossible-to-reproduce issue where this backtrace information was all we had available. I'll ask tomorrow whether anyone made use of these parameters and get back to you...

[dblaikie]

how's this compare to the linux behavior? be nice to have a similar format

[aaron.ballman]

llvm/lib/Support/Windows/Signals.inc

339–340

I think the stack traces we print out are basically just there for the call stack alone -- in order to do any real debugging of the situation, I've found I've needed to go to the minidump anyway (https://reviews.llvm.org/D18216). I'd be fine dropping the parameter information to reduce the amount of noise in the crash report.

[bd1976llvm]

I have asked around and no one made use of those numbers so I have removed them in the latest diff. However, people would have found the exception code useful so I have added that. Output now looks like:

With llvm-symbolizer:
Stack dump:
0. Program arguments: C:\\u\\br2\\bin\\ld.lld.exe
Exception Code: 0x80000003
#0 0x00007ff61d276294 (C:\u\br2\bin\ld.lld.exe+0x1f6294)
#1 0x00007ff61d2761dd (C:\u\br2\bin\ld.lld.exe+0x1f61dd)
#2 0x00007ff61d12652c (C:\u\br2\bin\ld.lld.exe+0xa652c)
...

Without llvm-symbolizer:
Stack dump:
0. Program arguments: C:\\u\\br2\\bin\\ld.lld.exe
Exception Code: 0x80000003
0x00007FF61D276294, C:\u\br2\bin\ld.lld.exe(0x00007FF61D080000) + 0x1F6294 byte(s)
0x00007FF61D2761DD, C:\u\br2\bin\ld.lld.exe(0x00007FF61D080000) + 0x1F61DD byte(s)
0x00007FF61D12652C, C:\u\br2\bin\ld.lld.exe(0x00007FF61D080000) + 0xA652C byte(s)
...

[bd1976llvm]

In D127915#3590333, @dblaikie wrote:

how's this compare to the linux behavior? be nice to have a similar format

There are differences. This doesn't really bother me as a crash dump is platform specific but if this is important I can try to make the output consistent (or maybe that should be another change)?

[aaron.ballman]

In D127915#3592406, @bd1976llvm wrote:

I have asked around and no one made use of those numbers so I have removed them in the latest diff. However, people would have found the exception code useful so I have added that. Output now looks like:

+1 to keeping the exception code, that can be very helpful information to have. (I would not be sad if we decided to map the hex value to a more human readable string as well.)

[rnk]

lgtm

+1 for the exception code. I think it may appear as the process return code and some systems will log that, but logging it ourselves, too, is for the best.

I think we could probably go further here. The unsymbolized output when we use llvm-symbolizer and it can't find PDB files is practically the same as the new output with your change. It would be nice to be able to fetch symbols and then re-pipe the crash report text through llvm-symbolizer and get a symbolized stack trace. That can be future work, this is a nice improvement as is.

[dblaikie]

In D127915#3592409, @bd1976llvm wrote:

In D127915#3590333, @dblaikie wrote:

how's this compare to the linux behavior? be nice to have a similar format

There are differences. This doesn't really bother me as a crash dump is platform specific but if this is important I can try to make the output consistent (or maybe that should be another change)?

oh, wasn't so much about it being exactly the same formatting (though that seems nice to have) - but whether it contained the same information (or is there a real difference between the platforms that motivates the differences?) - like I don't recall any of this parameter dumping in the linux crash dump.

(& maybe if the OS-specific part was lower level (surfacing the module base addresses and offsets) leaving the rendering to a higher level portable piece of code, that'd address the inconsistencies and reduce code duplication?)

[dblaikie]

In D127915#3592534, @rnk wrote:

lgtm

+1 for the exception code. I think it may appear as the process return code and some systems will log that, but logging it ourselves, too, is for the best.

I think we could probably go further here. The unsymbolized output when we use llvm-symbolizer and it can't find PDB files is practically the same as the new output with your change. It would be nice to be able to fetch symbols and then re-pipe the crash report text through llvm-symbolizer and get a symbolized stack trace. That can be future work, this is a nice improvement as is.

That sounds like this: https://reviews.llvm.org/D126980 perhaps? (we could change the crash dump output to include the markup required for the filtering, specifically when we can't already symbolize at crash-time)

[bd1976llvm]

I have made a commit with the current improvements.

@dblaikie I like your ideas for further improvements. These can be addressed in future patches.

llvm/lib/Support/Windows/Signals.inc

while (true) {
     using namespace llvm;
     // Print the PC in hexadecimal.
     DWORD64 PC = StackFrame.AddrPC.Offset;
 #if defined(_M_X64) || defined(_M_ARM64)
     OS << format("0x%016llX", PC);
 #elif defined(_M_IX86) || defined(_M_ARM)
     OS << format("0x%08lX", static_cast<DWORD>(PC));
 #endif
 
-// Print the parameters.  Assume there are four.
-#if defined(_M_X64) || defined(_M_ARM64)
-    OS << format(" (0x%016llX 0x%016llX 0x%016llX 0x%016llX)",
-            StackFrame.Params[0], StackFrame.Params[1], StackFrame.Params[2],
-            StackFrame.Params[3]);
-#elif defined(_M_IX86) || defined(_M_ARM)
-    OS << format(" (0x%08lX 0x%08lX 0x%08lX 0x%08lX)",
-            static_cast<DWORD>(StackFrame.Params[0]),
-            static_cast<DWORD>(StackFrame.Params[1]),
-            static_cast<DWORD>(StackFrame.Params[2]),
-            static_cast<DWORD>(StackFrame.Params[3]));
-#endif
     // Verify the PC belongs to a module in this process.
     if (!fSymGetModuleBase64(hProcess, PC)) {
       OS << " <unknown module>\n";
       continue;
     }
 
+    IMAGEHLP_MODULE64 M;
+    memset(&M, 0, sizeof(IMAGEHLP_MODULE64));
+    M.SizeOfStruct = sizeof(IMAGEHLP_MODULE64);
+    if (fSymGetModuleInfo64(hProcess, fSymGetModuleBase64(hProcess, PC), &M)) {
+      DWORD64 const disp = PC - M.BaseOfImage;
+      OS << format(", %s(0x%016llX) + 0x%llX byte(s)",
+                   static_cast<char *>(M.ImageName), M.BaseOfImage,
+                   static_cast<long long>(disp));
+    } else {
+      OS << ", <unknown module>";
+    }
+
     // Print the symbol name.
     char buffer[512];
     IMAGEHLP_SYMBOL64 *symbol = reinterpret_cast<IMAGEHLP_SYMBOL64 *>(buffer);
     memset(symbol, 0, sizeof(IMAGEHLP_SYMBOL64));
     symbol->SizeOfStruct = sizeof(IMAGEHLP_SYMBOL64);
     symbol->MaxNameLength = 512 - sizeof(IMAGEHLP_SYMBOL64);
 
     DWORD64 dwDisp;
     if (!fSymGetSymFromAddr64(hProcess, PC, &dwDisp, symbol)) {
       OS << '\n';
       continue;
     }
 
     buffer[511] = 0;
-    if (dwDisp > 0)
-      OS << format(", %s() + 0x%llX bytes(s)", (const char*)symbol->Name,
-                   dwDisp);
-    else
-      OS << format(", %s", (const char*)symbol->Name);
+    OS << format(", %s() + 0x%llX byte(s)", static_cast<char *>(symbol->Name),
+                 static_cast<long long>(dwDisp));
 
     // Print the source file and line number information.
     IMAGEHLP_LINE64 line = {};
     DWORD dwLineDisp;
     line.SizeOfStruct = sizeof(line);
     if (fSymGetLineFromAddr64(hProcess, PC, &dwLineDisp, &line)) {
-      OS << format(", %s, line %lu", line.FileName, line.LineNumber);
-      if (dwLineDisp > 0)
-        OS << format(" + 0x%lX byte(s)", dwLineDisp);
+      OS << format(", %s, line %lu + 0x%lX byte(s)", line.FileName,
+                   line.LineNumber, dwLineDisp);
     }
 
     OS << '\n';
   }
 }
 
 namespace llvm {
 

 
 void sys::CleanupOnSignal(uintptr_t Context) {
   LLVMUnhandledExceptionFilter((LPEXCEPTION_POINTERS)Context);
 }
 
 static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
   Cleanup(true);
 
+  // Write out the exception code.
+  if (ep && ep->ExceptionRecord)
+    llvm::errs() << format("Exception Code: 0x%08X",
+                           ep->ExceptionRecord->ExceptionCode)
+                 << "\n";
+
   // We'll automatically write a Minidump file here to help diagnose
   // the nasty sorts of crashes that aren't 100% reproducible from a set of
   // inputs (or in the event that the user is unable or unwilling to provide a
   // reproducible case).
   if (!llvm::sys::Process::AreCoreFilesPrevented()) {
     MINIDUMP_EXCEPTION_INFORMATION ExceptionInfo;
     ExceptionInfo.ThreadId = ::GetCurrentThreadId();
     ExceptionInfo.ExceptionPointers = ep;