From bb707dc0fe331c9af112a0552b7aa6fde755dd83 Mon Sep 17 00:00:00 2001 From: Pekka Paalanen Date: Wed, 7 Feb 2018 12:51:14 +0200 Subject: [PATCH] weston: remove SEGV and ABRT handlers Catching an ABRT is kind of ok, catching a SEGV is russian roulette. We have been quite lucky with it, but I've started hitting crashes inside malloc() which causes a deadlock when our SEGV handler needs to malloc() as well (weston_log_timestamp()). One reason to catch SEGV and ABRT was to attempt to restore the VT on the DRM-backend. Nowadays that job is done by logind or weston-launch. The signal handler also printed a backtrace, which for me personally has been extremely helpful. Arguably it's not necessary though, when we have core files and services that catch cores. For instance, if using systemd, 'coredumpctl gdb' is delightfully easy for getting into the saved core. Therefore, this code does more harm than it is useful, so remove it. We also drop an optional dependency to libunwind. Signed-off-by: Pekka Paalanen Reviewed-by: Daniel Stone --- Makefile.am | 8 ++-- compositor/main.c | 119 ---------------------------------------------- configure.ac | 20 -------- man/weston.man | 14 ------ 4 files changed, 4 insertions(+), 157 deletions(-) diff --git a/Makefile.am b/Makefile.am index 5132e35a..32c9a0f2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -71,8 +71,8 @@ install-libweston_moduleLTLIBRARIES install-moduleLTLIBRARIES: install-libLTLIBR lib_LTLIBRARIES = libweston-@LIBWESTON_MAJOR@.la libweston_@LIBWESTON_MAJOR@_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON libweston_@LIBWESTON_MAJOR@_la_CFLAGS = $(AM_CFLAGS) \ - $(COMPOSITOR_CFLAGS) $(EGL_CFLAGS) $(LIBUNWIND_CFLAGS) $(LIBDRM_CFLAGS) -libweston_@LIBWESTON_MAJOR@_la_LIBADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \ + $(COMPOSITOR_CFLAGS) $(EGL_CFLAGS) $(LIBDRM_CFLAGS) +libweston_@LIBWESTON_MAJOR@_la_LIBADD = $(COMPOSITOR_LIBS) \ $(DL_LIBS) -lm $(CLOCK_GETTIME_LIBS) \ $(LIBINPUT_BACKEND_LIBS) libshared.la libweston_@LIBWESTON_MAJOR@_la_LDFLAGS = -version-info $(LT_VERSION_INFO) @@ -190,9 +190,9 @@ weston_LDFLAGS = -export-dynamic weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON \ -DMODULEDIR='"$(moduledir)"' \ -DXSERVER_PATH='"@XSERVER_PATH@"' -weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS) +weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) weston_LDADD = libshared.la libweston-@LIBWESTON_MAJOR@.la \ - $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \ + $(COMPOSITOR_LIBS) \ $(DL_LIBS) $(LIBINPUT_BACKEND_LIBS) \ $(CLOCK_GETRES_LIBS) \ -lm diff --git a/compositor/main.c b/compositor/main.c index 7feb4cb0..18810f28 100644 --- a/compositor/main.c +++ b/compositor/main.c @@ -43,11 +43,6 @@ #include #include -#ifdef HAVE_LIBUNWIND -#define UNW_LOCAL_ONLY -#include -#endif - #include "weston.h" #include "compositor.h" #include "../shared/os-compatibility.h" @@ -190,85 +185,6 @@ sigchld_handler(int signal_number, void *data) return 1; } -#ifdef HAVE_LIBUNWIND - -static void -print_backtrace(void) -{ - unw_cursor_t cursor; - unw_context_t context; - unw_word_t off; - unw_proc_info_t pip; - int ret, i = 0; - char procname[256]; - const char *filename; - Dl_info dlinfo; - - pip.unwind_info = NULL; - ret = unw_getcontext(&context); - if (ret) { - weston_log("unw_getcontext: %d\n", ret); - return; - } - - ret = unw_init_local(&cursor, &context); - if (ret) { - weston_log("unw_init_local: %d\n", ret); - return; - } - - ret = unw_step(&cursor); - while (ret > 0) { - ret = unw_get_proc_info(&cursor, &pip); - if (ret) { - weston_log("unw_get_proc_info: %d\n", ret); - break; - } - - ret = unw_get_proc_name(&cursor, procname, 256, &off); - if (ret && ret != -UNW_ENOMEM) { - if (ret != -UNW_EUNSPEC) - weston_log("unw_get_proc_name: %d\n", ret); - procname[0] = '?'; - procname[1] = 0; - } - - if (dladdr((void *)(pip.start_ip + off), &dlinfo) && dlinfo.dli_fname && - *dlinfo.dli_fname) - filename = dlinfo.dli_fname; - else - filename = "?"; - - weston_log("%u: %s (%s%s+0x%x) [%p]\n", i++, filename, procname, - ret == -UNW_ENOMEM ? "..." : "", (int)off, (void *)(pip.start_ip + off)); - - ret = unw_step(&cursor); - if (ret < 0) - weston_log("unw_step: %d\n", ret); - } -} - -#else - -static void -print_backtrace(void) -{ - void *buffer[32]; - int i, count; - Dl_info info; - - count = backtrace(buffer, ARRAY_LENGTH(buffer)); - for (i = 0; i < count; i++) { - dladdr(buffer[i], &info); - weston_log(" [%016lx] %s (%s)\n", - (long) buffer[i], - info.dli_sname ? info.dli_sname : "--", - info.dli_fname); - } -} - -#endif - static void child_client_exec(int sockfd, const char *path) { @@ -643,39 +559,6 @@ static int on_term_signal(int signal_number, void *data) return 1; } -static void -on_caught_signal(int s, siginfo_t *siginfo, void *context) -{ - /* This signal handler will do a best-effort backtrace, and - * then call the backend restore function, which will switch - * back to the vt we launched from or ungrab X etc and then - * raise SIGTRAP. If we run weston under gdb from X or a - * different vt, and tell gdb "handle *s* nostop", this - * will allow weston to switch back to gdb on crash and then - * gdb will catch the crash with SIGTRAP.*/ - - weston_log("caught signal: %d\n", s); - - print_backtrace(); - - if (segv_compositor && segv_compositor->backend) - segv_compositor->backend->restore(segv_compositor); - - raise(SIGTRAP); -} - -static void -catch_signals(void) -{ - struct sigaction action; - - action.sa_flags = SA_SIGINFO | SA_RESETHAND; - action.sa_sigaction = on_caught_signal; - sigemptyset(&action.sa_mask); - sigaction(SIGSEGV, &action, NULL); - sigaction(SIGABRT, &action, NULL); -} - static const char * clock_name(clockid_t clk_id) { @@ -1830,8 +1713,6 @@ int main(int argc, char *argv[]) weston_log_set_handler(vlog, vlog_continue); weston_log_file_open(log); - catch_signals(); - weston_log("%s\n" STAMP_SPACE "%s\n" STAMP_SPACE "Bug reports to: %s\n" diff --git a/configure.ac b/configure.ac index 61ba14fb..dd344d6a 100644 --- a/configure.ac +++ b/configure.ac @@ -603,26 +603,6 @@ if test "x$GCC" = "xyes"; then fi AC_SUBST(GCC_CFLAGS) -AC_ARG_ENABLE(libunwind, - AS_HELP_STRING([--disable-libunwind], - [Disable libunwind usage for backtraces]),, - enable_libunwind=auto) -AM_CONDITIONAL(HAVE_LIBUNWIND, [test "x$enable_libunwind" = xyes]) -have_libunwind=no -if test "x$enable_libunwind" != "xno"; then - PKG_CHECK_MODULES(LIBUNWIND, - libunwind, - have_libunwind=yes, - have_libunwind=no) - if test "x$have_libunwind" = "xno" -a "x$enable_libunwind" = "xyes"; then - AC_MSG_ERROR([libunwind support explicitly requested, but libunwind couldn't be found]) - fi - if test "x$have_libunwind" = "xyes"; then - enable_libunwind=yes - AC_DEFINE(HAVE_LIBUNWIND, 1, [Have libunwind support]) - fi -fi - if test "x$WESTON_NATIVE_BACKEND" = "x"; then WESTON_NATIVE_BACKEND="drm-backend.so" diff --git a/man/weston.man b/man/weston.man index 7c3416c2..596041df 100644 --- a/man/weston.man +++ b/man/weston.man @@ -324,20 +324,6 @@ The directory for Weston's socket and lock files. Wayland clients will automatically use this. . .\" *************************************************************** -.SH DIAGNOSTICS -Weston has a segmentation fault handler, that attempts to restore -the virtual console or ungrab X before raising -.BR SIGTRAP . -If you run -.BR weston " under " gdb (1) -from an X11 terminal or a different virtual terminal, and tell gdb -.IP -handle SIGSEGV nostop -.PP -This will allow weston to switch back to gdb on crash and then -gdb will catch the crash with SIGTRAP. -. -.\" *************************************************************** .SH BUGS Bugs should be reported to the freedesktop.org bugzilla at https://bugs.freedesktop.org with product "Wayland" and