[Support] Enable file + line info in LLVM stack traces on Darwin.

[lhames]

Summary

This patch provides an implementation of findModulesAndOffsets for Darwin.
This function maps stack frames to (image name, vm-address) pairs which the
generic function printSymbolizedStackTrace (in Signals.cpp) can feed to
llvm-symbolize. Where a dSYM is present, llvm-symbolize will use this to find
source file and line info and add it to the trace.

[lhames]

Looking for feedback on the approach before I go too much further with this. I was motivated to enable file + line info by Don Hinton's work in https://reviews.llvm.org/D70259 and https://reviews.llvm.org/D70263, where he attaches source location information certain calls to aid in debugging of error handling failures. I have a review out for an alternative scheme that produces full backtraces for error handling failures (see https://reviews.llvm.org/D70600), but the lack of file and line info in the traces is a drawback compared to Don's approach.

As for why we don't have this on Darwin already: I'm not sure whether we had support for it and lost it at some point, or just never had it. It looks like we experimented with other approaches a while back (e.g. _Unwind_Backtrace in https://reviews.llvm.org/D28265) but ultimately gave up.

[lhames]

• Remove byte-swapping code. This will run in process, so endianness shouldn't

[lhames]

• Remove byte-swapping code. This will run in process, so endianness shouldn't
• Fix variable naming.

[hintonda]

This is awesome, thanks for working on it!

I pulled down your patch and built it, but am not getting expected results. Did I miss something?

I just munged an existing test to force a failure and emit a stack trace, and got the following
(I have a recent llvm-symbolizer in my path):

/Users/dhinton/projects/llvm/llvm-project/llvm/unittests/Support/ErrorTest.cpp:394: Failure
Death test: FailToHandle()
    Result: died but not with expected error.
  Expected: Failure value returned from cantFail wrapped call
CustomError \{7\}x
Actual msg:
[  DEATH   ] Failure value returned from cantFail wrapped call
[  DEATH   ] CustomError {7}
[  DEATH   ] UNREACHABLE executed at /Users/dhinton/projects/llvm/llvm-project/llvm/include/llvm/Support/Error.h:713!
[  DEATH   ]  #0 0x0000000109e65c9c (SupportTests+0x100759c9c)
[  DEATH   ]  #1 0x0000000109e66229 (SupportTests+0x10075a229)
[  DEATH   ]  #2 0x0000000109e63fb6 (SupportTests+0x100757fb6)
[  DEATH   ]  #3 0x0000000109e687ac (SupportTests+0x10075c7ac)
<snip>

[lhames]

• Remove check for an impossible magic value.

[lhames]

In D70628#1757813, @hintonda wrote:

This is awesome, thanks for working on it!

I pulled down your patch and built it, but am not getting expected results. Did I miss something?

Did you generate a dSYM for the binary with dsymutil? Unfortunately llvm-symbolize doesn't seem to know how to symbolize directly from objects containing debug info, but I'm hoping we can eventually teach it to do that.

[hintonda]

In D70628#1757860, @lhames wrote:

In D70628#1757813, @hintonda wrote:

This is awesome, thanks for working on it!

I pulled down your patch and built it, but am not getting expected results. Did I miss something?

Did you generate a dSYM for the binary with dsymutil? Unfortunately llvm-symbolize doesn't seem to know how to symbolize directly from objects containing debug info, but I'm hoping we can eventually teach it to do that.

Just a standard llvm Debug build. Is there an cmake option I'm missing?

[lhames]

In D70628#1758031, @hintonda wrote:

Just a standard llvm Debug build. Is there an cmake option I'm missing?

I'm not sure whether there is a cmake option to generate dSYMs yet -- you'll have to run dsymutil on whatever tool you're testing with.

I think the best long term solution to this problem is to teach llvm-symbolize how to cope with debug info in objects, rather than requiring a dSYM. An intermediate step (possibly generically useful) would be to add an option to llvm-symbolize to run llvm-dsymutil if there's no dSYM available.

[hintonda]

In D70628#1761330, @lhames wrote:

In D70628#1758031, @hintonda wrote:

Just a standard llvm Debug build. Is there an cmake option I'm missing?

I'm not sure whether there is a cmake option to generate dSYMs yet -- you'll have to run dsymutil on whatever tool you're testing with.

I think the best long term solution to this problem is to teach llvm-symbolize how to cope with debug info in objects, rather than requiring a dSYM. An intermediate step (possibly generically useful) would be to add an option to llvm-symbolize to run llvm-dsymutil if there's no dSYM available.

I suppose that might be useful for individuals, but I don't think it would be a good idea for the bots, which is my primary target.

[hintonda]

In D70628#1762116, @hintonda wrote:

In D70628#1761330, @lhames wrote:

In D70628#1758031, @hintonda wrote:

Just a standard llvm Debug build. Is there an cmake option I'm missing?

I'm not sure whether there is a cmake option to generate dSYMs yet -- you'll have to run dsymutil on whatever tool you're testing with.

I think the best long term solution to this problem is to teach llvm-symbolize how to cope with debug info in objects, rather than requiring a dSYM. An intermediate step (possibly generically useful) would be to add an option to llvm-symbolize to run llvm-dsymutil if there's no dSYM available.

I suppose that might be useful for individuals, but I don't think it would be a good idea for the bots, which is my primary target.

My last comment concerned the intermediate step. I think your long-term solution would be great.

[lhames]

In D70628#1762343, @hintonda wrote:

In D70628#1762116, @hintonda wrote:

In D70628#1761330, @lhames wrote:

...
I think the best long term solution to this problem is to teach llvm-symbolize how to cope with debug info in objects, rather than requiring a dSYM. An intermediate step (possibly generically useful) would be to add an option to llvm-symbolize to run llvm-dsymutil if there's no dSYM available.

I suppose that might be useful for individuals, but I don't think it would be a good idea for the bots, which is my primary target.

My last comment concerned the intermediate step. I think your long-term solution would be great.

If the option is added (and LLVM's stack symbolication call-out knows to use it) then I think it should work on the bots too, right? The only issue would be that crashing test cases would be slightly slower (since we'd have to produce the dSYMs).

[dblaikie]

In D70628#1803671, @lhames wrote:

In D70628#1762343, @hintonda wrote:

In D70628#1762116, @hintonda wrote:

In D70628#1761330, @lhames wrote:

...
I think the best long term solution to this problem is to teach llvm-symbolize how to cope with debug info in objects, rather than requiring a dSYM. An intermediate step (possibly generically useful) would be to add an option to llvm-symbolize to run llvm-dsymutil if there's no dSYM available.

I suppose that might be useful for individuals, but I don't think it would be a good idea for the bots, which is my primary target.

My last comment concerned the intermediate step. I think your long-term solution would be great.

If the option is added (and LLVM's stack symbolication call-out knows to use it) then I think it should work on the bots too, right? The only issue would be that crashing test cases would be slightly slower (since we'd have to produce the dSYMs).

Bit of an aside: the speed of crashing tests in the presence of debug info is actually sort of important/currently a bit of a bottleneck - all gunit death tests (for instance, llvm::Error has a few of those) crash, and run the symbolizer as the normal part of the crash process. That symbolization is pretty slow currently in a debug/unoptimized build of the symbolizer itself. It'd be great to find a way to disable crash symbolizing for death tests. (I guess maybe this performance issue doesn't come up on the MachO platform because of the missing functionality that's causing it not to symbolize)

[lhames]

Yep. I'd actually settled on the idea of teaching llvm-symbolizer how to chase down the debug info in the individual objects, but I'm not sure when I'll have time to actually do the work.

llvm/lib/Support/Unix/Signals.inc

 // Adding work to a signal handler requires lock-freedom (and assume atomics are
 // always lock-free) because the signal handler could fire while new work is
 // being added.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Unix.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/BinaryFormat/MachO.h"
 #include "llvm/Config/config.h"
 #include "llvm/Demangle/Demangle.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Mutex.h"
 #include "llvm/Support/Program.h"
static bool findModulesAndOffsets(void **StackTrace, int Depth,
                                   const char **Modules, intptr_t *Offsets,
                                   const char *MainExecutableName,
                                   StringSaver &StrPool) {
   DlIteratePhdrData data = {StackTrace, Depth,   true,
                             Modules,    Offsets, MainExecutableName};
   dl_iterate_phdr(dl_iterate_phdr_cb, &data);
   return true;
 }
+#elif defined(__APPLE__)
+
+static uintptr_t getImagePreferredLoadAddress(const void *ImageBase) {
+  const char *ImagePtr = reinterpret_cast<const char *>(ImageBase);
+
+  // Process header.
+  const auto *MachHdr = reinterpret_cast<const MachO::mach_header*>(ImagePtr);
+
+  if (MachHdr->magic != MachO::MH_MAGIC && MachHdr->magic != MachO::MH_MAGIC_64)
+    return ~0ULL;
+  bool Is64Bit = MachHdr->magic == MachO::MH_MAGIC_64;
+  ImagePtr +=
+    Is64Bit ? sizeof(MachO::mach_header_64) : sizeof(MachO::mach_header);
+
+  // Process load commands.
+  for (uint32_t I = 0; I < MachHdr->ncmds; ++I) {
+    const auto *LC = reinterpret_cast<const MachO::load_command*>(ImagePtr);
+    if (Is64Bit && LC->cmd == MachO::LC_SEGMENT_64) {
+      const auto *LCSeg64 =
+        reinterpret_cast<const MachO::segment_command_64*>(ImagePtr);
+      if (strncmp(LCSeg64->segname, "__TEXT", 16) == 0)
+        return LCSeg64->vmaddr;
+    } else if (!Is64Bit && LC->cmd == MachO::LC_SEGMENT) {
+      const auto *LCSeg = reinterpret_cast<const MachO::segment_command*>(ImagePtr);
+      if (strncmp(LCSeg->segname, "__TEXT", 16) == 0)
+        return LCSeg->vmaddr;
+    }
+
+    ImagePtr += LC->cmdsize;
+
+    // Bail out with an error value if the load command walk takes us out of
+    // bounds. This should never happen with well-formed images.
+    if (ImagePtr - reinterpret_cast<const char *>(ImageBase) >=
+        MachHdr->sizeofcmds)
+      return ~0U;
+  }
+
+  // No segment load command found. Return an error value.
+  return ~0U;
+}
+
+static bool findModulesAndOffsets(void **StackTrace, int Depth,
+                                  const char **Modules, intptr_t *Offsets,
+                                  const char *MainExecutableName,
+                                  StringSaver &StrPool) {
+  for (int I = 0; I != Depth; ++I) {
+    Dl_info dlinfo;
+    dladdr(StackTrace[I], &dlinfo);
+
+    uintptr_t OffsetInImage =
+      (uintptr_t)StackTrace[I] - (uintptr_t)dlinfo.dli_fbase;
+
+    uintptr_t PreferredLoadAddress =
+      getImagePreferredLoadAddress(dlinfo.dli_fbase);
+
+    if (PreferredLoadAddress == (uintptr_t)~0ULL)
+      return false;
+
+    if (auto *Basename = strrchr(dlinfo.dli_fname, '/'))
+      Modules[I] = Basename + 1;
+    else
+      Modules[I] = dlinfo.dli_fname;
+    Offsets[I] = OffsetInImage + PreferredLoadAddress;
+  }
+  return true;
+}
+
 #else
 /// This platform does not have dl_iterate_phdr, so we do not yet know how to
 /// find all loaded DSOs.
 static bool findModulesAndOffsets(void **StackTrace, int Depth,
                                   const char **Modules, intptr_t *Offsets,
                                   const char *MainExecutableName,
                                   StringSaver &StrPool) {
   return false;