Add backtrace to crash logs

Send crash information from the signal handler to traffic_crashlog and
record it in crash logs.

This also:

* Adds a 10-second timeout around backtrace collection to prevent the
  crashlog helper from hanging indefinitely if ptrace/waitpid blocks.
  This addresses a TODO comment that has been in the code for a while.
* Removes the "Memory Regions" section from the crash logs. These are
  very rarely useful and otherwise add significant noise to the crash
  log. A post mordem gdb section on a core should be able to get this
  information if it is necessary in a particular instance.

Author: bneradt

doc/appendices/command-line/traffic_crashlog.en.rst

@@ -37,6 +37,8 @@ for users to run directly.
 
 When :ref:`traffic_server` starts, it will launch a :program:`traffic_crashlog`
 process and keep it stopped, activating it only if a crash occurs.
+The crash report includes backtraces for all threads, with the crashing thread
+listed first.
 
 Options
 =======
@@ -48,8 +50,8 @@ Options
     This option specifies the host triple for the process that
     :program:`traffic_crashlog` should examine. If a supported host
     triple is specified, :program:`traffic_crashlog` expects to
-    receive a ``siginfo_t`` structure on it's standard input,
-    followed by a ``ucontext_t``.
+    receive the crashing thread ID, then a ``siginfo_t`` structure on
+    its standard input, followed by a ``ucontext_t``.
 
 .. option:: --target PID
 

src/traffic_crashlog/backtrace.cc

@@ -30,6 +30,8 @@
  ****************************************************************************/
 #include "tscore/ink_config.h"
 
+#include <sys/types.h>
+
 #if TS_USE_REMOTE_UNWINDING
 #include "tscore/Diags.h"
 
@@ -38,7 +40,6 @@
 #include <libunwind.h>
 #include <libunwind-ptrace.h>
 #if defined(__FreeBSD__)
-#include <sys/types.h>
 #define __WALL        P_ALL
 #define PTRACE_ATTACH PT_ATTACH
 #define PTRACE_DETACH PT_DETACH
@@ -56,6 +57,7 @@ using threadlist = std::vector<pid_t>;
 
 DbgCtl dbg_ctl_backtrace{"backtrace"};
 
+/** Enumerate all threads for a given process by reading /proc/<pid>/task. */
 static threadlist
 threads_for_process(pid_t proc)
 {
@@ -104,58 +106,73 @@ backtrace_for_thread(pid_t threadid, TextBuffer &text)
   unw_cursor_t     cursor;
   void            *ap     = nullptr;
   pid_t            target = -1;
-  unsigned         level  = 0;
+  bool             attached{false};
+  unsigned         level = 0;
+  int              step_result;
 
   // First, attach to the child, causing it to stop.
   status = ptrace(PTRACE_ATTACH, threadid, 0, 0);
   if (status < 0) {
+    text.format("  [ptrace ATTACH failed: %s (%d)]\n", strerror(errno), errno);
     Dbg(dbg_ctl_backtrace, "ptrace(ATTACH, %ld) -> %s (%d)\n", (long)threadid, strerror(errno), errno);
     return;
   }
+  attached = true;
 
-  // Wait for it to stop (XXX should be a timed wait ...)
+  // Wait for it to stop. The caller uses alarm() to enforce a timeout.
   target = waitpid(threadid, &status, __WALL | WUNTRACED);
   Dbg(dbg_ctl_backtrace, "waited for target %ld, found PID %ld, %s\n", (long)threadid, (long)target,
       WIFSTOPPED(status) ? "STOPPED" : "???");
   if (target < 0) {
+    text.format("  [waitpid failed: %s (%d)]\n", strerror(errno), errno);
     goto done;
   }
 
   ap = _UPT_create(threadid);
   Dbg(dbg_ctl_backtrace, "created UPT %p", ap);
   if (ap == nullptr) {
+    text.format("  [_UPT_create failed]\n");
     goto done;
   }
 
   addr_space = unw_create_addr_space(&_UPT_accessors, 0 /* byteorder */);
   Dbg(dbg_ctl_backtrace, "created address space %p\n", addr_space);
   if (addr_space == nullptr) {
+    text.format("  [unw_create_addr_space failed]\n");
     goto done;
   }
 
   status = unw_init_remote(&cursor, addr_space, ap);
   Dbg(dbg_ctl_backtrace, "unw_init_remote(...) -> %d\n", status);
   if (status != 0) {
+    text.format("  [unw_init_remote failed: %d]\n", status);
     goto done;
   }
 
-  while (unw_step(&cursor) > 0) {
+  step_result = unw_step(&cursor);
+  if (step_result <= 0) {
+    text.format("  [unw_step returned %d on first call]\n", step_result);
+  }
+
+  while (step_result > 0) {
     unw_word_t ip;
-    unw_word_t offset;
+    unw_word_t offset = 0;
     char       buf[256];
 
     unw_get_reg(&cursor, UNW_REG_IP, &ip);
 
     if (unw_get_proc_name(&cursor, buf, sizeof(buf), &offset) == 0) {
-      int   status;
-      char *name = abi::__cxa_demangle(buf, nullptr, nullptr, &status);
-      text.format("%-4u 0x%016llx %s + %p\n", level, static_cast<unsigned long long>(ip), name ? name : buf, (void *)offset);
+      int   demangle_status;
+      char *name = abi::__cxa_demangle(buf, nullptr, nullptr, &demangle_status);
+      text.format("%-4u 0x%016llx %s + 0x%lx\n", level, static_cast<unsigned long long>(ip), name ? name : buf,
+                  static_cast<unsigned long>(offset));
       free(name);
     } else {
-      text.format("%-4u 0x%016llx 0x0 + %p\n", level, static_cast<unsigned long long>(ip), (void *)offset);
+      text.format("%-4u 0x%016llx <unknown>\n", level, static_cast<unsigned long long>(ip));
     }
 
     ++level;
+    step_result = unw_step(&cursor);
   }
 
 done:
@@ -167,38 +184,65 @@ backtrace_for_thread(pid_t threadid, TextBuffer &text)
     _UPT_destroy(ap);
   }
 
-  status = ptrace(PTRACE_DETACH, target, NULL, DATA_NULL);
-  Dbg(dbg_ctl_backtrace, "ptrace(DETACH, %ld) -> %d (errno %d)\n", (long)target, status, errno);
+  if (attached) {
+    status = ptrace(PTRACE_DETACH, threadid, nullptr, DATA_NULL);
+    Dbg(dbg_ctl_backtrace, "ptrace(DETACH, %ld) -> %d (errno %d)\n", (long)threadid, status, errno);
+  }
+}
+
+/** Format a thread header with the thread name from /proc. */
+static void
+format_thread_header(pid_t threadid, const char *prefix, TextBuffer &text)
+{
+  ats_scoped_fd fd;
+  char          path[128];
+
+  snprintf(path, sizeof(path), "/proc/%ld/comm", static_cast<long>(threadid));
+  fd = open(path, O_RDONLY);
+  if (fd >= 0) {
+    text.format("%s (TID %ld, ", prefix, static_cast<long>(threadid));
+    text.readFromFD(fd);
+    text.chomp();
+    text.format("):\n");
+  } else {
+    text.format("%s (TID %ld):\n", prefix, static_cast<long>(threadid));
+  }
 }
 } // namespace
+
 int
-ServerBacktrace(unsigned /* options */, int pid, char **trace)
+ServerBacktrace(unsigned /* options */, pid_t pid, pid_t crashing_tid, char **trace)
 {
   *trace = nullptr;
 
   threadlist threads(threads_for_process(pid));
   TextBuffer text(0);
 
-  Dbg(dbg_ctl_backtrace, "tracing %zd threads for traffic_server PID %ld\n", threads.size(), (long)pid);
+  Dbg(dbg_ctl_backtrace, "tracing %zd threads for traffic_server PID %ld, crashing TID %ld\n", threads.size(),
+      static_cast<long>(pid), static_cast<long>(crashing_tid));
 
+  // First, trace the crashing thread.
+  if (crashing_tid > 0) {
+    Dbg(dbg_ctl_backtrace, "tracing crashing thread %ld\n", static_cast<long>(crashing_tid));
+    format_thread_header(crashing_tid, "Crashing Thread", text);
+    backtrace_for_thread(crashing_tid, text);
+    text.format("\n");
+  }
+
+  // Then trace all other threads.
+  bool printed_header = false;
   for (auto threadid : threads) {
-    Dbg(dbg_ctl_backtrace, "tracing thread %ld\n", (long)threadid);
-    // Get the thread name using /proc/PID/comm
-    ats_scoped_fd fd;
-    char          threadname[128];
-
-    snprintf(threadname, sizeof(threadname), "/proc/%ld/comm", static_cast<long>(threadid));
-    fd = open(threadname, O_RDONLY);
-    if (fd >= 0) {
-      text.format("Thread %ld, ", static_cast<long>(threadid));
-      text.readFromFD(fd);
-      text.chomp();
-    } else {
-      text.format("Thread %ld", static_cast<long>(threadid));
+    if (threadid == crashing_tid) {
+      continue;
     }
 
-    text.format(":\n");
+    if (!printed_header) {
+      text.format("Other Non-Crashing Threads:\n\n");
+      printed_header = true;
+    }
 
+    Dbg(dbg_ctl_backtrace, "tracing thread %ld\n", static_cast<long>(threadid));
+    format_thread_header(threadid, "Thread", text);
     backtrace_for_thread(threadid, text);
     text.format("\n");
   }
@@ -210,7 +254,7 @@ ServerBacktrace(unsigned /* options */, int pid, char **trace)
 #else /* TS_USE_REMOTE_UNWINDING */
 
 int
-ServerBacktrace([[maybe_unused]] unsigned options, [[maybe_unused]] int pid, char **trace)
+ServerBacktrace([[maybe_unused]] unsigned options, [[maybe_unused]] pid_t pid, [[maybe_unused]] pid_t crashing_tid, char **trace)
 {
   *trace = nullptr;
   return -1;

src/traffic_crashlog/procinfo.cc

@@ -66,12 +66,6 @@ write_procfd_file(const char *filename, const char *label, FILE *fp, const crash
   return !text.empty();
 }
 
-bool
-crashlog_write_regions(FILE *fp, const crashlog_target &target)
-{
-  return write_procfd_file("maps", "Memory Regions", fp, target);
-}
-
 bool
 crashlog_write_procstatus(FILE *fp, const crashlog_target &target)
 {