From: E.Smith <31170571+azlm8t@users.noreply.github.com>
Date: Mon, 1 Oct 2018 17:05:26 +0000 (+0100)
Subject: FreeBSD: Add libunwind trap support for FreeBSD only.
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0cde40cbba3c7da760e8369c26e3274544e6d78e;p=thirdparty%2Ftvheadend.git

FreeBSD: Add libunwind trap support for FreeBSD only.

Although the existing backtrace works correctly on Linux, on
FreeBSD it frequently generates a backtrace with completely
wrong function names. (FreeBSD 11.2, current latest version).

For example, making htsp_build_dvrentry crash with SEGV, it
would either not generate a stacktrace or would generate a
backtrace of:
-pthread_sigmask
-pthread_getspecific
-service_remove_unseen
-htsp_get_subscription_status
-htsp_init
-tcp_server_done
-tvhthread_create.

...instead of the correct backtrace of:
-<signal>
-htsp_build_dvrentry
-htsp_method_async
-htsp_read_loop
-htsp_serve...

So on FreeBSD only, we use libunwind to generate the
backtrace and function names. We explicitly make
libunwind and libexecinfo mutually exclusive since
FreeBSD has both.

Line are logged similar to:
  CRASH: htsp_build_dvrentry+5d (ip=11f659d sp=7fffd8bc3930)

Note that it does not have line numbers since the addr2line
does not appear to work on FreeBSD (even with the original
backtrace code).

An example of the problem with the old backtrace code using
the frame from htsp_method_async from within the tvheadend
traphandler after the retrieval of the stack frames:

(gdb) print frames
$38 = {0x806473954, 0x806472eb2, 0x7ffffffff193, 0x11f1638 <htsp_method_async+1640>, 0x11fe400 <htsp_read_loop+880>, 0x11f58e6 <htsp_serve+502>, 0x11b9b11 <tcp_server_start+401>,
  0x11af45e <thread_wrapper+302>, 0x80646dc06, 0x0 <repeats 91 times>}

(gdb) print dladdr(0x11f1638, &dli)    <--- addr of htsp_method_async from frame 4.
$39 = 1  <--- success

(gdb) print dli
$40 = {dli_fname = 0x7fffffffef97 ".../build.freebsd/tvheadend", dli_fbase = 0x1021000, dli_sname = 0x1044f91 "service_remove_unseen",   <--- but wrong name
    dli_saddr = 0x11eff80 <service_remove_unseen>}   <--- and this is nearest symbol address

(gdb) print htsp_method_async+1640
    $41 = (htsmsg_t *(*)(htsp_connection_t *, htsmsg_t *)) 0x11f1638 <htsp_method_async+1640>   <---but gdb knows the original address is htsp_method_async

(gdb) print service_remove_unseen
    $42 = {void (const char *, int)} 0x11eff80 <service_remove_unseen> <--- and gdb knows sevice_remove_unseen is at the dli_saddr.

By contrast, with libunwind, we get:

(gdb) print buf
$50 = "htsp_method_async", '\000' <repeats 110 times> <--- libunwind detected correct function name

(gdb) where 10  <--- even though our signal has been delivered on its own stack
 #0  traphandler_libunwind () at src/trap.c:162
 #1  0x000000000120cf06 in traphandler (sig=11, si=0x7fffdbbdb860, UC=0x7fffdbbdb4f0) at src/trap.c:221
 #2  0x0000000806673954 in ?? ()
 #3  0x0000000000000000 in ?? ()

(gdb) print ip
$51 = 18814904

(gdb) disass 18814904  <--- and gdb knows that ip address is for the same method as libunwind detected
Dump of assembler code for function htsp_method_async:
   0x00000000011f1150 <+0>:     push   %rbp
---

diff --git a/configure b/configure
index 66f2e8ee1..1572c4ff6 100755
--- a/configure
+++ b/configure
@@ -144,7 +144,19 @@ fi
 
 # Valiate compiler
 check_cc || die 'No C compiler found'
-check_cc_header execinfo
+# We only want libunwind on non-linux plaforms where we have had bad
+# stack traces from existing implementation.
+if [ ${PLATFORM} = "freebsd" ]; then
+    check_cc_header libunwind
+    # If we don't have libunwind then fallback to execinfo.
+    if ! enabled libunwind
+    then
+        check_cc_header execinfo
+    fi
+
+else
+    check_cc_header execinfo
+fi
 check_cc_option mmx
 check_cc_option sse2
 check_cc_optionW unused-result
@@ -408,6 +420,9 @@ else
 
 fi
 
+if enabled libunwind; then
+    LDFLAGS="$LDFLAGS -lunwind"
+fi
 #
 # Gzip
 #
diff --git a/src/trap.c b/src/trap.c
index e25b54c00..952f55318 100644
--- a/src/trap.c
+++ b/src/trap.c
@@ -35,6 +35,11 @@ char tvh_binshasum[20];
 #include <execinfo.h>
 #include <dlfcn.h>
 #endif
+#if ENABLE_LIBUNWIND
+#define UNW_LOCAL_ONLY
+#include <libunwind.h>
+#endif
+
 #include <stdio.h>
 #include <stdarg.h>
 
@@ -72,6 +77,7 @@ sappend(char *buf, size_t l, const char *fmt, ...)
 /**
  *
  */
+#ifndef ENABLE_LIBUNWIND
 #if ENABLE_EXECINFO
 static int
 addr2lineresolve(const char *binary, intptr_t addr, char *buf0, size_t buflen)
@@ -133,8 +139,30 @@ addr2lineresolve(const char *binary, intptr_t addr, char *buf0, size_t buflen)
   return 0;
 }
 #endif /* ENABLE_EXECINFO */
+#endif
 
-
+#if ENABLE_LIBUNWIND
+static void
+traphandler_libunwind()
+{
+  tvhlog_spawn(LOG_ALERT, LS_CRASH, "STACKTRACE");
+  unw_cursor_t cursor;
+  unw_context_t uc;
+  unw_word_t ip, sp;
+  char buf[128];
+
+  unw_getcontext(&uc);
+  unw_init_local(&cursor, &uc);
+  while (unw_step(&cursor) > 0) {
+    unw_get_reg(&cursor, UNW_REG_IP, &ip);
+    unw_get_reg(&cursor, UNW_REG_SP, &sp);
+    unw_word_t offp = 0;
+    const int pn = unw_get_proc_name(&cursor, buf, sizeof buf, &offp);
+    if (pn) *buf=0;
+    tvhlog_spawn (LOG_ALERT, LS_CRASH, "%s+%lx (ip=%lx sp=%lx)", buf, (long)offp, (long)ip, (long)sp);
+  }
+}
+#endif
 
 static void 
 traphandler(int sig, siginfo_t *si, void *UC)
@@ -183,6 +211,17 @@ traphandler(int sig, siginfo_t *si, void *UC)
 #endif
   tvhlog_spawn(LOG_ALERT, LS_CRASH, "%s", tmpbuf);
 
+  /* Although libunwind is supported on Linux, our existing unwind
+   * handler is better (supplies line numbers). But, on FreeBSD, the
+   * libunwind provides better diagnostics since our existing unwind
+   * handler often terminates without a trace, or does not provide
+   * correct function name mapping.
+   */
+#if ENABLE_LIBUNWIND
+  traphandler_libunwind();
+  return;
+#endif
+
 #if ENABLE_EXECINFO
   tvhlog_spawn(LOG_ALERT, LS_CRASH, "STACKTRACE");