fatal-signal: Catch SIGSEGV and print backtrace.

The patch catches the SIGSEGV signal and prints the backtrace
using libunwind at the monitor daemon. This makes debugging easier
when there is no debug symbol package or gdb installed on production
systems.

The patch works when the ovs-vswitchd compiles even without debug symbol
(no -g option), because the object files still have function symbols.
For example:
 |daemon_unix(monitor)|WARN|SIGSEGV detected, backtrace:
 |daemon_unix(monitor)|WARN|0x0000000000482752 <fatal_signal_handler+0x52>
 |daemon_unix(monitor)|WARN|0x00007fb4900734b0 <killpg+0x40>
 |daemon_unix(monitor)|WARN|0x00007fb49013974d <__poll+0x2d>
 |daemon_unix(monitor)|WARN|0x000000000052b348 <time_poll+0x108>
 |daemon_unix(monitor)|WARN|0x00000000005153ec <poll_block+0x8c>
 |daemon_unix(monitor)|WARN|0x000000000058630a <clean_thread_main+0x1aa>
 |daemon_unix(monitor)|WARN|0x00000000004ffd1d <ovsthread_wrapper+0x7d>
 |daemon_unix(monitor)|WARN|0x00007fb490b3b6ba <start_thread+0xca>
 |daemon_unix(monitor)|WARN|0x00007fb49014541d <clone+0x6d>
 |daemon_unix(monitor)|ERR|1 crashes: pid 122849 died, killed \
    (Segmentation fault), core dumped, restarting

However, if the object files' symbols are stripped, then we can only
get init function plus offset value. This is still useful when trying
to see if two bugs have the same root cause, Example:
 |daemon_unix(monitor)|WARN|SIGSEGV detected, backtrace:
 |daemon_unix(monitor)|WARN|0x0000000000482752 <_init+0x7d68a>
 |daemon_unix(monitor)|WARN|0x00007f5f7c8cf4b0 <killpg+0x40>
 |daemon_unix(monitor)|WARN|0x00007f5f7c99574d <__poll+0x2d>
 |daemon_unix(monitor)|WARN|0x000000000052b348 <_init+0x126280>
 |daemon_unix(monitor)|WARN|0x00000000005153ec <_init+0x110324>
 |daemon_unix(monitor)|WARN|0x0000000000407439 <_init+0x2371>
 |daemon_unix(monitor)|WARN|0x00007f5f7c8ba830 <__libc_start_main+0xf0>
 |daemon_unix(monitor)|WARN|0x0000000000408329 <_init+0x3261>
 |daemon_unix(monitor)|ERR|1 crashes: pid 106155 died, killed \
	(Segmentation fault), core dumped, restarting

Most C library functions are not async-signal-safe, meaning that
it is not safe to call them from a signal handler, for example
printf() or fflush(). To be async-signal-safe, the handler only
collects the stack info using libunwind, which is signal-safe, and
issues 'write' to the pipe, where the monitor thread reads and
prints to ovs-vswitchd.log.

Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/590503433
Signed-off-by: William Tu <u9012063@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

diff --git a/.travis.yml b/.travis.yml
index 68026312ba8405d0368c00eb0ac6227c93c5ac40..b547eb041791a17c21775ffb6c316d8bac1d34aa 100644 (file)
--- a/.travis.yml
+++ b/.travis.yml
@@ -25,6 +25,7 @@ addons:
       - selinux-policy-dev
       - libunbound-dev
       - libunbound-dev:i386
+      - libunwind-dev
 
 before_install: ./.travis/${TRAVIS_OS_NAME}-prepare.sh
 

diff --git a/configure.ac b/configure.ac
index afd1e83450eb1cc47002a884023af85186b3a57e..0c593f469e62097a7ae681c2322e6b3190079878 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -138,6 +138,7 @@ OVS_LIBTOOL_VERSIONS
 OVS_CHECK_CXX
 AX_FUNC_POSIX_MEMALIGN
 OVS_CHECK_UNBOUND
+OVS_CHECK_UNWIND
 
 OVS_CHECK_INCLUDE_NEXT([stdio.h string.h])
 AC_CONFIG_FILES([

diff --git a/lib/backtrace.c b/lib/backtrace.c
index 5cb2954f816b3753cc409efcef6332325e516ab7..9347634487c83223f421d516f9fdc6147e8062f8 100644 (file)
--- a/lib/backtrace.c
+++ b/lib/backtrace.c
@@ -15,10 +15,15 @@
  */
 
 #include <config.h>
+#include <errno.h>
+#include <fcntl.h>
 #include <inttypes.h>
+#include <string.h>
+#include <unistd.h>
 
 #include "backtrace.h"
 #include "openvswitch/vlog.h"
+#include "util.h"
 
 VLOG_DEFINE_THIS_MODULE(backtrace);
 
@@ -76,3 +81,37 @@ log_backtrace_at(const char *msg, const char *where)
 
     ds_destroy(&ds);
 }
+
+#ifdef HAVE_UNWIND
+void
+log_received_backtrace(int fd) {
+    int byte_read;
+    struct unw_backtrace backtrace[UNW_MAX_DEPTH];
+
+    VLOG_WARN("%s fd %d", __func__, fd);
+    fcntl(fd, F_SETFL, O_NONBLOCK);
+    memset(backtrace, 0, UNW_MAX_BUF);
+
+    byte_read = read(fd, backtrace, UNW_MAX_BUF);
+    if (byte_read < 0) {
+        VLOG_ERR("Read fd %d failed: %s", fd,
+                 ovs_strerror(errno));
+    } else if (byte_read > 0) {
+        VLOG_WARN("SIGSEGV detected, backtrace:");
+        for (int i = 0; i < UNW_MAX_DEPTH; i++) {
+            if (backtrace[i].func[0] == 0) {
+                break;
+            }
+            VLOG_WARN("0x%016lx <%s+0x%lx>\n",
+                      backtrace[i].ip,
+                      backtrace[i].func,
+                      backtrace[i].offset);
+        }
+    }
+}
+#else /* !HAVE_UNWIND */
+void
+log_received_backtrace(int daemonize_fd OVS_UNUSED) {
+    VLOG_WARN("Backtrace using libunwind not supported.");
+}
+#endif /* HAVE_UNWIND */

diff --git a/lib/backtrace.h b/lib/backtrace.h
index 384f2700d94cd1b057ddee905dbe2d298a7d8a35..5708bf9c68317244b738d149de9ad306aca2ce24 100644 (file)
--- a/lib/backtrace.h
+++ b/lib/backtrace.h
@@ -20,6 +20,11 @@
 #include <stdint.h>
 #include "openvswitch/dynamic-string.h"
 
+#ifdef HAVE_UNWIND
+#define UNW_LOCAL_ONLY
+#include <libunwind.h>
+#endif
+
 /* log_backtrace() will save the backtrace of a running program
  * into the log at the DEBUG level.
  *
@@ -68,7 +73,21 @@ struct backtrace {
     uintptr_t frames[BACKTRACE_MAX_FRAMES];
 };
 
+#ifdef HAVE_UNWIND
+#define UNW_MAX_DEPTH 32
+#define UNW_MAX_FUNCN 32
+#define UNW_MAX_BUF \
+    (UNW_MAX_DEPTH * sizeof(struct unw_backtrace))
+
+struct unw_backtrace {
+    char func[UNW_MAX_FUNCN];
+    unw_word_t ip;
+    unw_word_t offset;
+};
+#endif
+
 void backtrace_capture(struct backtrace *);
 void log_backtrace_at(const char *msg, const char *where);
+void log_received_backtrace(int fd);
 
 #endif /* backtrace.h */

diff --git a/lib/daemon-private.h b/lib/daemon-private.h
index 8bc71e2a638d2cc4a294066516333b6339e5d9bd..4e0667601001239e3381dc602206a79a2e22232b 100644 (file)
--- a/lib/daemon-private.h
+++ b/lib/daemon-private.h
@@ -19,6 +19,7 @@
 
 extern bool detach;
 extern char *pidfile;
+extern int daemonize_fd;
 
 char *make_pidfile_name(const char *name);
 

diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
index 6169763c294c922e128960cfc5cb4cd63a0546f5..7e48630f0e93abdc26d9205314a14e5ee65c880f 100644 (file)
--- a/lib/daemon-unix.c
+++ b/lib/daemon-unix.c
@@ -15,6 +15,7 @@
  */
 
 #include <config.h>
+#include "backtrace.h"
 #include "daemon.h"
 #include "daemon-private.h"
 #include <errno.h>
@@ -75,7 +76,7 @@ static bool overwrite_pidfile;
 static bool chdir_ = true;
 
 /* File descriptor used by daemonize_start() and daemonize_complete(). */
-static int daemonize_fd = -1;
+int daemonize_fd = -1;
 
 /* --monitor: Should a supervisory process monitor the daemon and restart it if
  * it dies due to an error signal? */
@@ -291,8 +292,7 @@ fork_and_wait_for_startup(int *fdp, pid_t *child_pid)
                 OVS_NOT_REACHED();
             }
         }
-        close(fds[0]);
-        *fdp = -1;
+        *fdp = fds[0];
     } else if (!pid) {
         /* Running in child process. */
         close(fds[0]);
@@ -313,8 +313,6 @@ fork_notify_startup(int fd)
         if (error) {
             VLOG_FATAL("pipe write failed (%s)", ovs_strerror(error));
         }
-
-        close(fd);
     }
 }
 
@@ -393,6 +391,8 @@ monitor_daemon(pid_t daemon_pid)
                     }
                 }
 
+                log_received_backtrace(daemonize_fd);
+
                 /* Throttle restarts to no more than once every 10 seconds. */
                 if (time(NULL) < last_restart + 10) {
                     VLOG_WARN("%s, waiting until 10 seconds since last "
@@ -508,7 +508,6 @@ daemonize_complete(void)
         detached = true;
 
         fork_notify_startup(daemonize_fd);
-        daemonize_fd = -1;
         daemonize_post_detach();
     }
 }

diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
index 3b905b6de7664b13b36d664f190a791316a1b792..7733850d5c4bf1a17ec2f0e721df262568152491 100644 (file)
--- a/lib/fatal-signal.c
+++ b/lib/fatal-signal.c
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 #include <config.h>
+#include "backtrace.h"
 #include "fatal-signal.h"
 #include <errno.h>
 #include <signal.h>
@@ -34,6 +35,10 @@
 
 #include "openvswitch/type-props.h"
 
+#ifdef HAVE_UNWIND
+#include "daemon-private.h"
+#endif
+
 #ifndef SIG_ATOMIC_MAX
 #define SIG_ATOMIC_MAX TYPE_MAXIMUM(sig_atomic_t)
 #endif
@@ -42,7 +47,8 @@ VLOG_DEFINE_THIS_MODULE(fatal_signal);
 
 /* Signals to catch. */
 #ifndef _WIN32
-static const int fatal_signals[] = { SIGTERM, SIGINT, SIGHUP, SIGALRM };
+static const int fatal_signals[] = { SIGTERM, SIGINT, SIGHUP, SIGALRM,
+                                     SIGSEGV };
 #else
 static const int fatal_signals[] = { SIGTERM };
 #endif
@@ -151,6 +157,44 @@ fatal_signal_add_hook(void (*hook_cb)(void *aux), void (*cancel_cb)(void *aux),
     ovs_mutex_unlock(&mutex);
 }
 
+#ifdef HAVE_UNWIND
+/* Send the backtrace buffer to monitor thread.
+ *
+ * Note that this runs in the signal handling context, any system
+ * library functions used here must be async-signal-safe.
+ */
+static inline void
+send_backtrace_to_monitor(void) {
+    int dep;
+    struct unw_backtrace unw_bt[UNW_MAX_DEPTH];
+    unw_cursor_t cursor;
+    unw_context_t uc;
+
+    if (daemonize_fd == -1) {
+        return;
+    }
+
+    dep = 0;
+    unw_getcontext(&uc);
+    unw_init_local(&cursor, &uc);
+
+    while (dep < UNW_MAX_DEPTH && unw_step(&cursor)) {
+        memset(unw_bt[dep].func, 0, UNW_MAX_FUNCN);
+        unw_get_reg(&cursor, UNW_REG_IP, &unw_bt[dep].ip);
+        unw_get_proc_name(&cursor, unw_bt[dep].func, UNW_MAX_FUNCN,
+                          &unw_bt[dep].offset);
+       dep++;
+    }
+
+    ignore(write(daemonize_fd, unw_bt, dep * sizeof(struct unw_backtrace)));
+}
+#else
+static inline void
+send_backtrace_to_monitor(void) {
+    /* Nothing. */
+}
+#endif
+
 /* Handles fatal signal number 'sig_nr'.
  *
  * Ordinarily this is the actual signal handler.  When other code needs to
@@ -164,6 +208,11 @@ void
 fatal_signal_handler(int sig_nr)
 {
 #ifndef _WIN32
+    if (sig_nr == SIGSEGV) {
+        signal(sig_nr, SIG_DFL); /* Set it back immediately. */
+        send_backtrace_to_monitor();
+        raise(sig_nr);
+    }
     ignore(write(signal_fds[1], "", 1));
 #else
     SetEvent(wevent);

diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index 78d70fb4e17eb3cd43d14ac2dc2efb9f18810d99..79e0be5a33ddd2dc3f8c48392d58710dd965b2a2 100644 (file)
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -637,3 +637,13 @@ AC_DEFUN([OVS_CHECK_UNBOUND],
    fi
    AM_CONDITIONAL([HAVE_UNBOUND], [test "$HAVE_UNBOUND" = yes])
    AC_SUBST([HAVE_UNBOUND])])
+
+dnl Checks for libunwind.
+AC_DEFUN([OVS_CHECK_UNWIND],
+  [AC_CHECK_LIB(unwind, unw_backtrace, [HAVE_UNWIND=yes], [HAVE_UNWIND=no])
+   if test "$HAVE_UNWIND" = yes; then
+     AC_DEFINE([HAVE_UNWIND], [1], [Define to 1 if unwind is detected.])
+     LIBS="$LIBS -lunwind"
+   fi
+   AM_CONDITIONAL([HAVE_UNWIND], [test "$HAVE_UNWIND" = yes])
+   AC_SUBST([HAVE_UNWIND])])