Optionally print symbolizer markup backtraces.

[mysterymath]

Summary

When the environment LLVM_ENABLE_SYMBOLIZER_MARKUP is set, if
llvm-symbolizer fails or is disabled, this change will print a backtrace
in llvm-symbolizer markup instead of falling back to in-process
symbolization mechanisms.

This allows llvm-symbolizer to be run on the output later to produce a
high quality backtrace, even for fully-stripped LLVM utilities.

[mysterymath]

Add negative test.

[mcgrathr]

I thought we agreed that the desirable semantics to keep things simple is that if markup is requested then markup is always printed, period. It shouldn't have anything to do with whether llvm-symbolizer works.

[phosek]

In D139750#3985896, @mcgrathr wrote:

I thought we agreed that the desirable semantics to keep things simple is that if markup is requested then markup is always printed, period. It shouldn't have anything to do with whether llvm-symbolizer works.

I agree, I think it's fine if we keep markup as a fallback in the case when llvm-symbolizer fails or is disabled, but there should also be a way to force the use markup without trying anything else.

[mysterymath]

In D139750#3986437, @phosek wrote:

In D139750#3985896, @mcgrathr wrote:

I thought we agreed that the desirable semantics to keep things simple is that if markup is requested then markup is always printed, period. It shouldn't have anything to do with whether llvm-symbolizer works.

I agree, I think it's fine if we keep markup as a fallback in the case when llvm-symbolizer fails or is disabled, but there should also be a way to force the use markup without trying anything else.

The idea was to keep LLVM_DISABLE_SYMBOLIZATION and LLVM_ENABLE_SYMBOLIZER_MARKUP orthogonal; it seems prima facie that setting LLVM_ENABLE_SYMBOLIZER_MARKUP shouldn't also have the effect of setting LLVM_DISABLE_SYMBOLIZATION.

But the available options form a priority ordering; at most one method should be used, and some are superior to others. We could reasonable consider the markup workflow to have superior characteristics the others in the scenarios where it applies, so I think it's reasonable for enabling it to implicitly disable the others. Accordingly, I've moved the markup option to the top of the list.

[mysterymath]

Give markup priority over other symbolization options.

[mysterymath]

Add Windows version of printMarkupContext.

[mysterymath]

Apply git clang-format.

[phosek]

llvm/lib/Support/Unix/Signals.inc

539

Can you use reinterpret_cast instead of C-style cast?

562

Can you use C++-style instead of C-style cast here and below?

572

What does Amt refer to? Might be better to use something more self-descriptive.

[mysterymath]

Address review comments.

llvm/lib/Support/Unix/Signals.inc

539

This is just an array to pointer decay, so static_cast works. format just doesn't like being handed an array type.

562

Done, throughout.

572

Renamed these as the bytes remaining until the next item.

[jhenderson]

No idea where would the most appropriate place be for this, but should this be added to the docs somewhere?

[mysterymath]

Add blurb about LLVM_ENABLE_SYMBOLIZER_MARKUP to markup doc.

[mysterymath]

I'd like to ping this; having symbolizer markup would be boon for debugging crashes in CI environments where the toolchain is stripped. Fuchsia does this to minimize the install image for the toolchain, but it currently means that we must reproduce all crashes locally with a debug build to get a usable stack trace.

[mcgrathr]

Some minor improvements could be made, but this generally lgtm.

llvm/lib/Support/Signals.cpp

271

Best format is to append :ra or :pc depending on whether it's a return-address or exact PC frame.
When it's from a crash, the first PC is an exact PC and usually others are return-address frames (though when using full CFI unwinding, individual frames might be exact PC frames).

llvm/lib/Support/Unix/Signals.inc

527

It's less useful but you can still print the context if there's no build ID. The name might be helpful, though automated lookup likely won't work.

539

IMHO &ModeStr[0] is more natural.

604

This can use char Str[4] in the decl to document the requirement on the size (and some compilers will even do warnings based on that). But IMHO it would be better for this function to simply return std::array<char, 4> itself.

617

You may want to use the new reset:begin / reset:end pair around the context + backtrace, in which case IMHO neither really belongs in the "context" part of the printing per se.

[mysterymath]

Address review comments.

llvm/lib/Support/Signals.cpp

271

Unfortunately, the first PC will be well within the signal handler, so the source of the signal will be in some earlier frame. It seems difficult to find it, and separately difficult to tell whether it is a raise, direct function call, or a e.g. SIGSEGV. Accordingly, this seems like it would be a useful enhancement, but I'd expect it to be of a similar size to this patch.

llvm/lib/Support/Unix/Signals.inc

527

At least at present, llvm-symbolizer doesn't accept module lines without a supported module type. The definition for "elf" also requires a build ID. It's possible to change this of course, but we'd probably want to do some surgery to the language of the spec too to make it more explicit that the typed portion of module and mmap are optional.

617

Ack; this seems a better fit for a change set where we actually do something with them (plus define them in the reference).

[jhenderson]

Looks like clang-format is complaining. Best make sure you've reformatted everything you've added using it.

[mysterymath]

clang-format

[thakis]

This may not build on Mac: http://45.33.8.238/macm1/67288/step_4.txt

It possibly breaks tests on Linux: http://45.33.8.238/linux/115648/step_12.txt

It's possible that's due to bot weirdness, but maybe it's something real.

[arphaman]

This breaks the build on Darwin: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/37054/:

Undefined symbols for architecture x86_64:
  "printMarkupContext(llvm::raw_ostream&, char const*)", referenced from:
      printMarkupStackTrace(llvm::StringRef, void**, int, llvm::raw_ostream&) in libLLVMSupport.a(Signals.cpp.o)

@mysterymath could you take a look please?

[mysterymath]

In D139750#4596368, @arphaman wrote:

This breaks the build on Darwin: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/37054/:

Undefined symbols for architecture x86_64:
  "printMarkupContext(llvm::raw_ostream&, char const*)", referenced from:
      printMarkupStackTrace(llvm::StringRef, void**, int, llvm::raw_ostream&) in libLLVMSupport.a(Signals.cpp.o)

@mysterymath could you take a look please?

Ah, thanks, both of these real oversights. Missing stub for the Darwin path, and overly-restrictive regex for the Linux case. I'll have a fix out in an hour or so; if that's too late, feel free to revert.

[hliao]

I committed two quick fixes for Linux builds, where that unit test failed unreliably. a2dbb195d8ae and 7026a0cacaa0. Please correct that if the "0x" prefix is mandatory. If 0x is mandatory, please don't rely on the '#' flag, as it only outputs 0x for non-zero values.

[mysterymath]

8d3ff601a3 should fix the Darwin build issue and make the Linux unit tests sufficiently lenient. The %#x behavior was intentional; the spec is written with it in mind.

[mcgrathr]

llvm/lib/Support/Signals.cpp

271

The distinction is a feature of the unwinder semantics, not something you need to invent.

[mcgrathr]

llvm/lib/Support/Signals.cpp

271

It's not very obvious, but the Boolean (as int) value returned in the out parameter of _Unwind_GetIPInfo is the "is signal frame" flag.

llvm/docs/SymbolizerMarkupFormat.rst

 This format identifies markup elements with a syntax that is both simple and
 distinctive. It's simple enough to be matched and parsed with straightforward
 code. It's distinctive enough that character sequences that look like the start
 or end of a markup element should rarely if ever appear incidentally in logging
 text. It's specifically intended not to require sanitizing plain text, such as
 the HTML/XML requirement to replace ``<`` with ``&lt;`` and the like.
 
 :doc:`llvm-symbolizer <CommandGuide/llvm-symbolizer>` includes a symbolizing
-filter via its ``--filter-markup`` option.
+filter via its ``--filter-markup`` option. Also, LLVM utilites emit stack
+traces as markup when the ``LLVM_ENABLE_SYMBOLIZER_MARKUP`` environment
+variable is set.
 
 Scope and assumptions
 =====================
 
 A symbolizing filter implementation will be independent both of the target
 operating system and machine architecture where the logs are generated and of
 the host operating system and machine architecture where the filter runs.
 

llvm/lib/Support/Signals.cpp

void llvm::initSignalsOptions() {
   static ManagedStatic<cl::opt<std::string, true>, CreateCrashDiagnosticsDir>
       CrashDiagnosticsDir;
   *DisableSymbolication;
   *CrashDiagnosticsDir;
 }
 
 constexpr char DisableSymbolizationEnv[] = "LLVM_DISABLE_SYMBOLIZATION";
 constexpr char LLVMSymbolizerPathEnv[] = "LLVM_SYMBOLIZER_PATH";
+constexpr char EnableSymbolizerMarkupEnv[] = "LLVM_ENABLE_SYMBOLIZER_MARKUP";
 
 // Callbacks to run in signal handler must be lock-free because a signal handler
 // could be running as we add new callbacks. We don't add unbounded numbers of
 // callbacks, an array is therefore sufficient.
 struct CallbackAndCookie {
   sys::SignalHandlerCallback Callback;
   void *Cookie;
   enum class Status { Empty, Initializing, Initialized, Executing };
for (;;) {
       else
         OS << "(" << Modules[i] << '+' << format_hex(Offsets[i], 0) << ")";
       OS << "\n";
     }
   }
   return true;
 }
 
+static bool printMarkupContext(raw_ostream &OS, const char *MainExecutableName);
+
+LLVM_ATTRIBUTE_USED
+static bool printMarkupStackTrace(StringRef Argv0, void **StackTrace, int Depth,
+                                  raw_ostream &OS) {
+  const char *Env = getenv(EnableSymbolizerMarkupEnv);
+  if (!Env || !*Env)
+    return false;
+
+  std::string MainExecutableName =
+      sys::fs::exists(Argv0) ? std::string(Argv0)
+                             : sys::fs::getMainExecutable(nullptr, nullptr);
+  if (!printMarkupContext(OS, MainExecutableName.c_str()))
+    return false;
+  for (int I = 0; I < Depth; I++)
+    OS << format("{{{bt:%d:%#016x}}}\n", I, StackTrace[I]);
+  return true;
+}
+
 // Include the platform-specific parts of this class.
 #ifdef LLVM_ON_UNIX
 #include "Unix/Signals.inc"
 #endif
 #ifdef _WIN32
 #include "Windows/Signals.inc"
 #endif

llvm/lib/Support/Unix/Signals.inc

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;
 }
+
+class DSOMarkupPrinter {
+  llvm::raw_ostream &OS;
+  const char *MainExecutableName;
+  size_t ModuleCount = 0;
+  bool IsFirst = true;
+
+public:
+  DSOMarkupPrinter(llvm::raw_ostream &OS, const char *MainExecutableName)
+      : OS(OS), MainExecutableName(MainExecutableName) {}
+
+  /// Print llvm-symbolizer markup describing the layout of the given DSO.
+  void printDSOMarkup(dl_phdr_info *Info) {
+    ArrayRef<uint8_t> BuildID = findBuildID(Info);
+    if (BuildID.empty())
+      return;
+    OS << format("{{{module:%d:%s:elf:", ModuleCount,
+                 IsFirst ? MainExecutableName : Info->dlpi_name);
+    for (uint8_t X : BuildID)
+      OS << format("%02x", X);
+    OS << "}}}\n";
+
+    for (int I = 0; I < Info->dlpi_phnum; I++) {
+      const auto *Phdr = &Info->dlpi_phdr[I];
+      if (Phdr->p_type != PT_LOAD)
+        continue;
+      uintptr_t StartAddress = Info->dlpi_addr + Phdr->p_vaddr;
+      uintptr_t ModuleRelativeAddress = Phdr->p_vaddr;
+      std::array<char, 4> ModeStr = modeStrFromFlags(Phdr->p_flags);
+      OS << format("{{{mmap:%#016x:%#x:load:%d:%s:%#016x}}}\n", StartAddress,
+                   Phdr->p_memsz, ModuleCount, &ModeStr[0],
+                   ModuleRelativeAddress);
+    }
+    IsFirst = false;
+    ModuleCount++;
+  }
+
+  /// Callback for use with dl_iterate_phdr. The last dl_iterate_phdr argument
+  /// must be a pointer to an instance of this class.
+  static int printDSOMarkup(dl_phdr_info *Info, size_t Size, void *Arg) {
+    static_cast<DSOMarkupPrinter *>(Arg)->printDSOMarkup(Info);
+    return 0;
+  }
+
+  // Returns the build ID for the given DSO as an array of bytes. Returns an
+  // empty array if none could be found.
+  ArrayRef<uint8_t> findBuildID(dl_phdr_info *Info) {
+    for (int I = 0; I < Info->dlpi_phnum; I++) {
+      const auto *Phdr = &Info->dlpi_phdr[I];
+      if (Phdr->p_type != PT_NOTE)
+        continue;
+
+      ArrayRef<uint8_t> Notes(
+          reinterpret_cast<const uint8_t *>(Info->dlpi_addr + Phdr->p_vaddr),
+          Phdr->p_memsz);
+      while (Notes.size() > 12) {
+        uint32_t NameSize = *reinterpret_cast<const uint32_t *>(Notes.data());
+        Notes = Notes.drop_front(4);
+        uint32_t DescSize = *reinterpret_cast<const uint32_t *>(Notes.data());
+        Notes = Notes.drop_front(4);
+        uint32_t Type = *reinterpret_cast<const uint32_t *>(Notes.data());
+        Notes = Notes.drop_front(4);
+
+        ArrayRef<uint8_t> Name = Notes.take_front(NameSize);
+        auto CurPos = reinterpret_cast<uintptr_t>(Notes.data());
+        uint32_t BytesUntilDesc =
+            alignToPowerOf2(CurPos + NameSize, 4) - CurPos;
+        if (BytesUntilDesc >= Notes.size())
+          break;
+        Notes = Notes.drop_front(BytesUntilDesc);
+
+        ArrayRef<uint8_t> Desc = Notes.take_front(DescSize);
+        CurPos = reinterpret_cast<uintptr_t>(Notes.data());
+        uint32_t BytesUntilNextNote =
+            alignToPowerOf2(CurPos + DescSize, 4) - CurPos;
+        if (BytesUntilNextNote > Notes.size())
+          break;
+        Notes = Notes.drop_front(BytesUntilNextNote);
+
+        if (Type == 3 /*NT_GNU_BUILD_ID*/ && Name.size() >= 3 &&
+            Name[0] == 'G' && Name[1] == 'N' && Name[2] == 'U')
+          return Desc;
+      }
+    }
+    return {};
+  }
+
+  // Returns a symbolizer markup string describing the permissions on a DSO
+  // with the given p_flags.
+  std::array<char, 4> modeStrFromFlags(uint32_t Flags) {
+    std::array<char, 4> Mode;
+    char *Cur = &Mode[0];
+    if (Flags & PF_R)
+      *Cur++ = 'r';
+    if (Flags & PF_W)
+      *Cur++ = 'w';
+    if (Flags & PF_X)
+      *Cur++ = 'x';
+    *Cur = '\0';
+    return Mode;
+  }
+};
+
+static bool printMarkupContext(llvm::raw_ostream &OS,
+                               const char *MainExecutableName) {
+  OS << "{{{reset}}}\n";
+  DSOMarkupPrinter MP(OS, MainExecutableName);
+  dl_iterate_phdr(DSOMarkupPrinter::printDSOMarkup, &MP);
+  return true;
+}
+
 #elif ENABLE_BACKTRACES && defined(__APPLE__) && defined(__LP64__)
 static bool findModulesAndOffsets(void **StackTrace, int Depth,
                                   const char **Modules, intptr_t *Offsets,
                                   const char *MainExecutableName,
                                   StringSaver &StrPool) {
   uint32_t NumImgs = _dyld_image_count();
   for (uint32_t ImageIndex = 0; ImageIndex < NumImgs; ImageIndex++) {
     const char *Name = _dyld_get_image_name(ImageIndex);
for (uint32_t CmdNum = 0; CmdNum < Header->ncmds; ++CmdNum) {
           }
         }
       }
       Cmd = (const load_command *)(((const char *)Cmd) + (Cmd->cmdsize));
     }
   }
   return true;
 }
+
 #else
 /// Backtraces are not enabled or we don't yet know how to find all loaded DSOs
 /// on this platform.
 static bool findModulesAndOffsets(void **StackTrace, int Depth,
                                   const char **Modules, intptr_t *Offsets,
                                   const char *MainExecutableName,
                                   StringSaver &StrPool) {
   return false;
 }
+
+static bool printMarkupContext(llvm::raw_ostream &OS,
+                               const char *MainExecutableName) {
+  return false;
+}
 #endif // ENABLE_BACKTRACES && ... (findModulesAndOffsets variants)
 
 #if ENABLE_BACKTRACES && defined(HAVE__UNWIND_BACKTRACE)
 static int unwindBacktrace(void **StackTrace, int MaxEntries) {
   if (MaxEntries < 0)
     return 0;
 
   // Skip the first frame ('unwindBacktrace' itself).
depth =
         unwindBacktrace(StackTrace, static_cast<int>(std::size(StackTrace)));
 #endif
   if (!depth)
     return;
   // If "Depth" is not provided by the caller, use the return value of
   // backtrace() for printing a symbolized stack trace.
   if (!Depth)
     Depth = depth;
+  if (printMarkupStackTrace(Argv0, StackTrace, Depth, OS))
+    return;
   if (printSymbolizedStackTrace(Argv0, StackTrace, Depth, OS))
     return;
   OS << "Stack dump without symbol names (ensure you have llvm-symbolizer in "
         "your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point "
         "to it):\n";
 #if HAVE_DLFCN_H && HAVE_DLADDR
   int width = 0;
   for (int i = 0; i < depth; ++i) {

llvm/lib/Support/Windows/Signals.inc

static bool findModulesAndOffsets(void **StackTrace, int Depth,
   Data.Depth = Depth;
   Data.Modules = Modules;
   Data.Offsets = Offsets;
   Data.StrPool = &StrPool;
   fEnumerateLoadedModules(GetCurrentProcess(), findModuleCallback, &Data);
   return true;
 }
 
+static bool printMarkupContext(llvm::raw_ostream &OS,
+                               const char *MainExecutableName) {
+  return false;
+}
+
 static void PrintStackTraceForThread(llvm::raw_ostream &OS, HANDLE hProcess,
                                      HANDLE hThread, STACKFRAME64 &StackFrame,
                                      CONTEXT *Context) {
   // It's possible that DbgHelp.dll hasn't been loaded yet (e.g. if this
   // function is called before the main program called `llvm::InitLLVM`).
   // In this case just return, not stacktrace will be printed.
   if (!isDebugHelpInitialized())
     return;

llvm/unittests/Support/CMakeLists.txt

add_llvm_unittest(SupportTests
   RegexTest.cpp
   ReverseIterationTest.cpp
   ReplaceFileTest.cpp
   RISCVAttributeParserTest.cpp
   RISCVISAInfoTest.cpp
   ScaledNumberTest.cpp
   ScopedPrinterTest.cpp
   SHA256.cpp
+  SignalsTest.cpp
   SourceMgrTest.cpp
   SpecialCaseListTest.cpp
   SuffixTreeTest.cpp
   SwapByteOrderTest.cpp
   TarWriterTest.cpp
   ThreadPool.cpp
   Threading.cpp
   TimerTest.cpp

llvm/unittests/Support/SignalsTest.cpp

+//===-- llvm/unittest/Support/SignalsTest.cpp - Signals unit tests --------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file contains unit tests for Signals.cpp and Signals.inc.
+///
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/Signals.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Config/config.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest-matchers.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::sys;
+using testing::MatchesRegex;
+using testing::Not;
+
+#define TAG_BEGIN "\\{\\{\\{"
+#define TAG_END "\\}\\}\\}"
+
+#if defined(HAVE_BACKTRACE) && ENABLE_BACKTRACES && HAVE_LINK_H &&             \
+    (defined(__linux__) || defined(__FreeBSD__) ||                             \
+     defined(__FreeBSD_kernel__) || defined(__NetBSD__))
+TEST(SignalsTest, PrintsSymbolizerMarkup) {
+  auto Exit =
+      make_scope_exit([]() { unsetenv("LLVM_ENABLE_SYMBOLIZER_MARKUP"); });
+  setenv("LLVM_ENABLE_SYMBOLIZER_MARKUP", "1", 1);
+  std::string Res;
+  raw_string_ostream RawStream(Res);
+  PrintStackTrace(RawStream);
+  EXPECT_THAT(Res, MatchesRegex(TAG_BEGIN "reset" TAG_END ".*"));
+  // Module line for main binary
+  EXPECT_THAT(Res,
+              MatchesRegex(".*" TAG_BEGIN
+                           "module:0:[^:]*SupportTests:elf:[0-9a-f]+" TAG_END
+                           ".*"));
+  // Text segment for main binary
+  EXPECT_THAT(
+      Res,
+      MatchesRegex(".*" TAG_BEGIN
+                   "mmap:0x[0-9a-f]+:0x[0-9a-f]+:load:0:rx:0x[0-9a-f]+" TAG_END
+                   ".*"));
+  // Backtrace line
+  EXPECT_THAT(Res, MatchesRegex(".*" TAG_BEGIN "bt:0:0x[0-9a-f]+"
+                                ".*"));
+}
+
+TEST(SignalsTest, SymbolizerMarkupDisabled) {
+  auto Exit = make_scope_exit([]() { unsetenv("LLVM_DISABLE_SYMBOLIZATION"); });
+  setenv("LLVM_DISABLE_SYMBOLIZATION", "1", 1);
+  std::string Res;
+  raw_string_ostream RawStream(Res);
+  PrintStackTrace(RawStream);
+  EXPECT_THAT(Res, Not(MatchesRegex(TAG_BEGIN "reset" TAG_END ".*")));
+}
+
+#endif // defined(HAVE_BACKTRACE) && ...