From ae627737d5f370b3584fb07bf8f041226611148b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 18 Jan 2013 14:35:27 -0500 Subject: [PATCH] Remove SIGSEGV/SIGFPE etc. handling Modern operating systems come with systemwide "crash catching" facilities; for example, the Linux kernel can now pipe core dumps out to userspace, and programs like "systemd-coredump" and "abrt" record these. In this model, it's actively counterproductive for individual processes to catch SIGSEGV because: 1) Trying to unwind from inside the process after arbitrary corruption is destined to fail. 2) It hides the fact that a crash happened at all - my OS test framework wants to know if any process crashed, and I don't want to guess by running regexps against /var/log/Xorg.0.log or whatever. Signed-off-by: Colin Walters https://bugzilla.gnome.org/show_bug.cgi?id=692032 --- configure.ac | 7 ---- src/Makefile.am | 12 ------ src/logging/nm-logging.c | 86 --------------------------------------- src/logging/nm-logging.h | 2 - src/main.c | 23 ----------- src/nm-crash-logger.c | 87 ---------------------------------------- 6 files changed, 217 deletions(-) delete mode 100644 src/nm-crash-logger.c diff --git a/configure.ac b/configure.ac index dbbc1fdc08..51582917a6 100644 --- a/configure.ac +++ b/configure.ac @@ -611,13 +611,6 @@ fi AC_DEFINE_UNQUOTED(KERNEL_FIRMWARE_DIR, "$KERNEL_FIRMWARE_DIR", [Define to path of the kernel firmware directory]) AC_SUBST(KERNEL_FIRMWARE_DIR) -AC_ARG_ENABLE(crashtrace, - AS_HELP_STRING([--disable-crashtrace], [Disable GNU backtrace extensions]), - [enable_crashtrace=${enableval}], [enable_crashtrace=yes]) -if test x"$enable_crashtrace" = xyes; then - AC_DEFINE(ENABLE_CRASHTRACE, 1, [Define if you have GNU backtrace extensions]) -fi - PKG_CHECK_MODULES(LIBSOUP, [libsoup-2.4 >= 2.26], [have_libsoup=yes],[have_libsoup=no]) AC_ARG_ENABLE(concheck, AS_HELP_STRING([--enable-concheck], [enable connectivity checking support]), [enable_concheck=${enableval}], [enable_concheck=${have_libsoup}]) diff --git a/src/Makefile.am b/src/Makefile.am index db11d6b784..cd124f6b97 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -355,18 +355,6 @@ endif NetworkManager_LDFLAGS = -rdynamic -libexec_PROGRAMS = nm-crash-logger -nm_crash_logger_SOURCES = nm-crash-logger.c -nm_crash_logger_CPPFLAGS = \ - $(GLIB_CFLAGS) \ - -DBINDIR=\"$(bindir)\" \ - -DNMBINARY=\"$(nmbinary)\" \ - -DNMDATADIR=\"$(nmdatadir)\" \ - -DSYSCONFDIR=\"$(sysconfdir)\" \ - -DNMSTATEDIR=\"$(nmstatedir)\" -nm_crash_logger_LDADD = $(GLIB_LIBS) - - NetworkManagerdir = $(datadir)/NetworkManager NetworkManager_DATA = gdb-cmd diff --git a/src/logging/nm-logging.c b/src/logging/nm-logging.c index 3461412e82..1818203149 100644 --- a/src/logging/nm-logging.c +++ b/src/logging/nm-logging.c @@ -32,10 +32,6 @@ #include #include -#ifdef ENABLE_CRASHTRACE -#include -#endif - #include #include "nm-logging.h" @@ -270,88 +266,6 @@ _nm_log (const char *loc, /************************************************************************/ -static void -fallback_get_backtrace (void) -{ -#ifdef ENABLE_CRASHTRACE - void *frames[64]; - Dl_info info; - size_t size; - guint32 i; - const char *name; - - size = backtrace (frames, G_N_ELEMENTS (frames)); - - syslog (LOG_CRIT, "******************* START **********************************"); - for (i = 0; i < size; i++) { - dladdr (frames[i], &info); - name = (info.dli_fname && *info.dli_fname) ? info.dli_fname : "(vdso)"; - if (info.dli_saddr) { - syslog (LOG_CRIT, "Frame %d: %s (%s+0x%lx) [%p]", - i, name, - info.dli_sname, - (gulong)((guchar *)frames[i] - (guchar *)info.dli_saddr), - frames[i]); - } else { - syslog (LOG_CRIT, "Frame %d: %s (%p+0x%lx) [%p]", - i, name, - info.dli_fbase, - (gulong)((guchar *)frames[i] - (guchar *)info.dli_saddr), - frames[i]); - } - } - syslog (LOG_CRIT, "******************* END **********************************"); -#endif /* ENABLE_CRASHTRACE */ -} - - -static gboolean -crashlogger_get_backtrace (void) -{ - gboolean success = FALSE; - int pid; - - pid = fork(); - if (pid > 0) - { - /* Wait for the child to finish */ - int estatus; - if (waitpid (pid, &estatus, 0) != -1) - { - /* Only succeed if the crashlogger succeeded */ - if (WIFEXITED (estatus) && (WEXITSTATUS (estatus) == 0)) - success = TRUE; - } - } - else if (pid == 0) - { - /* Child process */ - execl (LIBEXECDIR"/nm-crash-logger", - LIBEXECDIR"/nm-crash-logger", NULL); - } - - return success; -} - - -void -nm_logging_backtrace (void) -{ - struct stat s; - gboolean fallback = TRUE; - - /* Try to use gdb via nm-crash-logger if it exists, since - * we get much better information out of it. Otherwise - * fall back to execinfo. - */ - if (stat (LIBEXECDIR"/nm-crash-logger", &s) == 0) - fallback = crashlogger_get_backtrace () ? FALSE : TRUE; - - if (fallback) - fallback_get_backtrace (); -} - - static void nm_log_handler (const gchar *log_domain, GLogLevelFlags level, diff --git a/src/logging/nm-logging.h b/src/logging/nm-logging.h index b3c583f22d..6aa61b6c6b 100644 --- a/src/logging/nm-logging.h +++ b/src/logging/nm-logging.h @@ -107,7 +107,6 @@ gboolean nm_logging_level_enabled (guint32 level); gboolean nm_logging_domain_enabled (guint32 domain); /* Undefine the nm-utils.h logging stuff to ensure errors */ -#undef nm_print_backtrace #undef nm_get_timestamp #undef nm_info #undef nm_info_str @@ -120,7 +119,6 @@ gboolean nm_logging_domain_enabled (guint32 domain); gboolean nm_logging_setup (const char *level, const char *domains, GError **error); void nm_logging_start (gboolean become_daemon); -void nm_logging_backtrace (void); void nm_logging_shutdown (void); #endif /* NM_LOGGING_H */ diff --git a/src/main.c b/src/main.c index b074bd8a8e..7288c3d367 100644 --- a/src/main.c +++ b/src/main.c @@ -88,22 +88,6 @@ signal_handling_thread (void *arg) sigwait (&signal_set, &signo); switch (signo) { - case SIGSEGV: - case SIGBUS: - case SIGILL: - case SIGABRT: - case SIGQUIT: - nm_log_warn (LOGD_CORE, "caught signal %d. Generating backtrace...", signo); - nm_logging_backtrace (); - exit (1); - break; - case SIGFPE: - case SIGPIPE: - nm_log_warn (LOGD_CORE, "caught signal %d, shutting down abnormally. Generating backtrace...", signo); - nm_logging_backtrace (); - quit_early = TRUE; /* for quitting before entering the main loop */ - g_main_loop_quit (main_loop); - break; case SIGINT: case SIGTERM: nm_log_info (LOGD_CORE, "caught signal %d, shutting down normally.", signo); @@ -142,13 +126,6 @@ setup_signals (void) sigemptyset (&signal_set); sigaddset (&signal_set, SIGHUP); sigaddset (&signal_set, SIGINT); - sigaddset (&signal_set, SIGQUIT); - sigaddset (&signal_set, SIGILL); - sigaddset (&signal_set, SIGABRT); - sigaddset (&signal_set, SIGFPE); - sigaddset (&signal_set, SIGBUS); - sigaddset (&signal_set, SIGSEGV); - sigaddset (&signal_set, SIGPIPE); sigaddset (&signal_set, SIGTERM); sigaddset (&signal_set, SIGUSR1); diff --git a/src/nm-crash-logger.c b/src/nm-crash-logger.c deleted file mode 100644 index f10e9ffe01..0000000000 --- a/src/nm-crash-logger.c +++ /dev/null @@ -1,87 +0,0 @@ -/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ -/* NetworkManager -- Network link manager - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Copyright (C) 2006 - 2008 Red Hat, Inc. - */ - - -#include -#include -#include -#include -#include -#include -#include - -int main (int argc, char ** argv) -{ - GPid gdb_pid; - int out; - char nm_pid[16]; - char line[256]; - int gdb_stat; - int bytes_read; - gboolean done = FALSE; - char * args[] = { BINDIR "/gdb", - "--batch", - "--quiet", - "--command=" NMDATADIR "/gdb-cmd", - NMBINARY, - NULL, NULL }; - - snprintf (nm_pid, sizeof (nm_pid), "%d", getppid ()); - args[5] = &nm_pid[0]; - if (!g_spawn_async_with_pipes (NULL, args, NULL, G_SPAWN_DO_NOT_REAP_CHILD, NULL, NULL, - &gdb_pid, NULL, &out, NULL, NULL)) - { - exit (1); - } - - openlog ("NetworkManager", LOG_CONS | LOG_PERROR, LOG_DAEMON); - syslog (LOG_CRIT, "******************* START **********************************"); - while (!done) - { - line[sizeof (line) - 1] = '\0'; - bytes_read = read (out, line, sizeof (line) - 1); - if (bytes_read > 0) - { - char *end = &line[0]; - char *start = &line[0]; - - /* Can't just funnel the output to syslog, have to do a separate - * syslog () for each line in the output. - */ - line[bytes_read] = '\0'; - while (*end != '\0') - { - if (*end == '\n') - { - *end = '\0'; - syslog (LOG_CRIT, "%s", start); - start = end + 1; - } - end++; - } - } - else if ((bytes_read <= 0) || ((errno != EINTR) && (errno != EAGAIN))) - done = TRUE; - } - syslog (LOG_CRIT, "******************* END **********************************"); - close (out); - waitpid (gdb_pid, &gdb_stat, 0); - exit (0); -}