From 0f669af4d7180b12142a41117ae2459d6960dd31 Mon Sep 17 00:00:00 2001 From: Martin Milata Date: Mon, 1 Dec 2014 12:05:36 +0100 Subject: [PATCH] Create core backtrace in unwind hook Related to #829. We need to implement #882 in order for this to work. This change requires (yet unreleased) satyr-0.16. The feature is turned off by default, you need to pass --enable-dump-time-unwind to configure in order to enable it. Signed-off-by: Martin Milata --- configure.ac | 12 ++ doc/abrt-CCpp.conf.txt | 18 +++ .../en-US/Automatic_Bug_Reporting_Tool_ABRT.xml | 2 +- src/hooks/CCpp.conf | 15 +++ src/hooks/abrt-hook-ccpp.c | 126 ++++++++++++++------- src/hooks/abrt-install-ccpp-hook.in | 4 +- 6 files changed, 132 insertions(+), 45 deletions(-) diff --git a/configure.ac b/configure.ac index 71d7c18..d7e0ea5 100644 --- a/configure.ac +++ b/configure.ac @@ -266,6 +266,18 @@ AC_ARG_ENABLE([addon-vmcore], AM_CONDITIONAL(BUILD_ADDON_VMCORE, false) [fi] +# Perform stack unwind on live/dying process in the core handler? + +AC_ARG_ENABLE([dump-time-unwind], + AS_HELP_STRING([--enable-dump-time-unwind], + [create core stack trace while the crashed process is still in memory (default is no)]), + [], [enable_dump_time_unwind=no]) + +[if test "$enable_native_unwinder" = "yes" -a "$enable_dump_time_unwind" = "yes"] +[then] + AC_DEFINE([ENABLE_DUMP_TIME_UNWIND], [1], [Create core stacktrace while the process is still in memory.]) +[fi] + AC_SUBST(CONF_DIR) AC_SUBST(DEFAULT_CONF_DIR) AC_SUBST(VAR_RUN) diff --git a/doc/abrt-CCpp.conf.txt b/doc/abrt-CCpp.conf.txt index ad3830b..498d53d 100644 --- a/doc/abrt-CCpp.conf.txt +++ b/doc/abrt-CCpp.conf.txt @@ -19,12 +19,30 @@ SaveBinaryImage = 'yes' / 'no' ...:: Useful, for example, when _deleted binary_ segfaults. Default is 'no'. +CreateCoreBacktrace = 'yes' / 'no' ...:: + When this option is set to 'yes', core backtrace is generated + from the memory image of the crashing process. Only the crash + thread is present in the backtrace. This feature requires + kernel 3.18 or newer, otherwise the core backtrace is not + created. + Default is 'yes'. + +SaveFullCore = 'yes' / 'no' ...:: + Save full coredump? If set to 'no', coredump won't be saved + and you won't be able to report the crash to Bugzilla. Only + useful with 'CreateCoreBacktrace' set to 'yes'. Please + note that if this option is set to 'no' and MakeCompatCore + is set to 'yes', the core is still written to the current + directory. + Default is 'yes'. + VerboseLog = NUM:: Used to make the hook more verbose SEE ALSO -------- abrt.conf(5) +abrt-action-generate-core-backtrace(1) AUTHORS ------- diff --git a/doc/deployment/en-US/Automatic_Bug_Reporting_Tool_ABRT.xml b/doc/deployment/en-US/Automatic_Bug_Reporting_Tool_ABRT.xml index e11307a..daee375 100644 --- a/doc/deployment/en-US/Automatic_Bug_Reporting_Tool_ABRT.xml +++ b/doc/deployment/en-US/Automatic_Bug_Reporting_Tool_ABRT.xml @@ -187,7 +187,7 @@ Starting abrt daemon: [ OK ] Please note that installing ABRT packages overwrites the /proc/sys/kernel/core_pattern file which can contain a template used to name core dump files. The content of this file will be overwritten to: - |/usr/libexec/abrt-hook-ccpp %s %c %p %u %g %t e + |/usr/libexec/abrt-hook-ccpp %s %c %p %u %g %t e %i Finally, if you run ABRT in a graphical desktop environment, you can verify that the ABRT notification applet is running: diff --git a/src/hooks/CCpp.conf b/src/hooks/CCpp.conf index d199116..b1a0a22 100644 --- a/src/hooks/CCpp.conf +++ b/src/hooks/CCpp.conf @@ -8,6 +8,21 @@ MakeCompatCore = yes # (useful, for example, when _deleted binary_ segfaults) SaveBinaryImage = no +# When this option is set to 'yes', core backtrace is generated +# from the memory image of the crashing process. Only the crash +# thread is present in the backtrace. This feature requires +# kernel 3.18 or newer, otherwise the core backtrace is not +# created. +CreateCoreBacktrace = yes + +# Save full coredump? If set to 'no', coredump won't be saved +# and you won't be able to report the crash to Bugzilla. Only +# useful with CreateCoreBacktrace set to 'yes'. Please +# note that if this option is set to 'no' and MakeCompatCore +# is set to 'yes', the core is still written to the current +# directory. +SaveFullCore = yes + # Used for debugging the hook #VerboseLog = 2 diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c index 3785d89..57c56a7 100644 --- a/src/hooks/abrt-hook-ccpp.c +++ b/src/hooks/abrt-hook-ccpp.c @@ -21,6 +21,11 @@ #include #include "libabrt.h" +#ifdef ENABLE_DUMP_TIME_UNWIND +#include +#include +#endif /* ENABLE_DUMP_TIME_UNWIND */ + #define DUMP_SUID_UNSAFE 1 #define DUMP_SUID_SAFE 2 @@ -151,13 +156,13 @@ static struct dump_dir *dd; * %g - gid * %t - UNIX time of dump * %e - executable filename - * %h - hostname + * %i - crash thread tid * %% - output one "%" */ /* Hook must be installed with exactly the same sequence of %c specifiers. * Last one, %h, may be omitted (we can find it out). */ -static const char percent_specifiers[] = "%scpugteh"; +static const char percent_specifiers[] = "%scpugtei"; static char *core_basename = (char*) "core"; /* * Used for error messages only. @@ -453,6 +458,24 @@ static int create_or_die(const char *filename, int user_core_fd) perror_msg_and_die("Can't open '%s'", filename); } +static void create_core_backtrace(pid_t tid, const char *executable, int signal_no, const char *dd_path) +{ +#ifdef ENABLE_DUMP_TIME_UNWIND + if (g_verbose > 1) + sr_debug_parser = true; + + char *error_message = NULL; + bool success = sr_abrt_create_core_stacktrace_from_core_hook(dd_path, tid, executable, + signal_no, &error_message); + + if (!success) + { + log("Failed to create core_backtrace: %s", error_message); + free(error_message); + } +#endif /* ENABLE_DUMP_TIME_UNWIND */ +} + static int create_user_core(int user_core_fd, pid_t pid, off_t ulimit_c) { if (user_core_fd >= 0) @@ -493,9 +516,9 @@ int main(int argc, char** argv) if (argc < 8) { - /* percent specifier: %s %c %p %u %g %t %e %h */ + /* percent specifier: %s %c %p %u %g %t %e %i */ /* argv: [0] [1] [2] [3] [4] [5] [6] [7] [8]*/ - error_msg_and_die("Usage: %s SIGNO CORE_SIZE_LIMIT PID UID GID TIME BINARY_NAME [HOSTNAME]", argv[0]); + error_msg_and_die("Usage: %s SIGNO CORE_SIZE_LIMIT PID UID GID TIME BINARY_NAME [TID]", argv[0]); } /* Not needed on 2.6.30. @@ -520,6 +543,8 @@ int main(int argc, char** argv) /* ... and plugins/CCpp.conf */ bool setting_MakeCompatCore; bool setting_SaveBinaryImage; + bool setting_SaveFullCore; + bool setting_CreateCoreBacktrace; { map_string_t *settings = new_map_string(); load_abrt_plugin_conf_file("CCpp.conf", settings); @@ -528,6 +553,10 @@ int main(int argc, char** argv) setting_MakeCompatCore = value && string_to_bool(value); value = get_map_string_item_or_NULL(settings, "SaveBinaryImage"); setting_SaveBinaryImage = value && string_to_bool(value); + value = get_map_string_item_or_NULL(settings, "SaveFullCore"); + setting_SaveFullCore = value ? string_to_bool(value) : true; + value = get_map_string_item_or_NULL(settings, "CreateCoreBacktrace"); + setting_CreateCoreBacktrace = value ? string_to_bool(value) : true; value = get_map_string_item_or_NULL(settings, "VerboseLog"); if (value) g_verbose = xatoi_positive(value); @@ -560,11 +589,10 @@ int main(int argc, char** argv) free(s); } - struct utsname uts; - if (!argv[8]) /* no HOSTNAME? */ + pid_t tid = 0; + if (argv[8]) { - uname(&uts); - argv[8] = uts.nodename; + tid = xatoi_positive(argv[8]); } char path[PATH_MAX]; @@ -769,49 +797,57 @@ int main(int argc, char** argv) off_t sz = copyfd_eof(src_fd_binary, dst_fd, COPYFD_SPARSE); if (fsync(dst_fd) != 0 || close(dst_fd) != 0 || sz < 0) { - dd_delete(dd); - error_msg_and_die("Error saving '%s'", path); + dd_delete(dd); error_msg_and_die("Error saving '%s'", path); } close(src_fd_binary); } - strcpy(path + path_len, "/"FILENAME_COREDUMP); - int abrt_core_fd = create_or_die(path, user_core_fd); - - /* We write both coredumps at once. - * We can't write user coredump first, since it might be truncated - * and thus can't be copied and used as abrt coredump; - * and if we write abrt coredump first and then copy it as user one, - * then we have a race when process exits but coredump does not exist yet: - * $ echo -e '#include\nmain(){raise(SIGSEGV);}' | gcc -o test -x c - - * $ rm -f core*; ulimit -c unlimited; ./test; ls -l core* - * 21631 Segmentation fault (core dumped) ./test - * ls: cannot access core*: No such file or directory <=== BAD - */ - off_t core_size = copyfd_sparse(STDIN_FILENO, abrt_core_fd, user_core_fd, ulimit_c); - if (fsync(abrt_core_fd) != 0 || close(abrt_core_fd) != 0 || core_size < 0) + off_t core_size = 0; + if (setting_SaveFullCore) { - unlink(path); - dd_delete(dd); - if (user_core_fd >= 0) + strcpy(path + path_len, "/"FILENAME_COREDUMP); + int abrt_core_fd = create_or_die(path, user_core_fd); + + /* We write both coredumps at once. + * We can't write user coredump first, since it might be truncated + * and thus can't be copied and used as abrt coredump; + * and if we write abrt coredump first and then copy it as user one, + * then we have a race when process exits but coredump does not exist yet: + * $ echo -e '#include\nmain(){raise(SIGSEGV);}' | gcc -o test -x c - + * $ rm -f core*; ulimit -c unlimited; ./test; ls -l core* + * 21631 Segmentation fault (core dumped) ./test + * ls: cannot access core*: No such file or directory <=== BAD + */ + core_size = copyfd_sparse(STDIN_FILENO, abrt_core_fd, user_core_fd, ulimit_c); + if (fsync(abrt_core_fd) != 0 || close(abrt_core_fd) != 0 || core_size < 0) { + unlink(path); + dd_delete(dd); + if (user_core_fd >= 0) + { + xchdir(user_pwd); + unlink(core_basename); + } + /* copyfd_sparse logs the error including errno string, + * but it does not log file name */ + error_msg_and_die("Error writing '%s'", path); + } + if (user_core_fd >= 0 + /* error writing user coredump? */ + && (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 + /* user coredump is too big? */ + || (ulimit_c == 0 /* paranoia */ || core_size > ulimit_c) + ) + ) { + /* nuke it (silently) */ xchdir(user_pwd); unlink(core_basename); } - /* copyfd_sparse logs the error including errno string, - * but it does not log file name */ - error_msg_and_die("Error writing '%s'", path); } - if (user_core_fd >= 0 - /* error writing user coredump? */ - && (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 - /* user coredump is too big? */ - || (ulimit_c == 0 /* paranoia */ || core_size > ulimit_c) - ) - ) { - /* nuke it (silently) */ - xchdir(user_pwd); - unlink(core_basename); + else + { + /* User core is created even if WriteFullCore is off. */ + create_user_core(user_core_fd, pid, ulimit_c); } /* Save JVM crash log if it exists. (JVM's coredump per se @@ -847,6 +883,10 @@ int main(int argc, char** argv) } } + /* Perform crash-time unwind of the guilty thread. */ + if (tid > 0 && setting_CreateCoreBacktrace) + create_core_backtrace(tid, executable, signal_no, dd->dd_dirname); + /* We close dumpdir before we start catering for crash storm case. * Otherwise, delete_dump_dir's from other concurrent * CCpp's won't be able to delete our dump (their delete_dump_dir @@ -860,7 +900,9 @@ int main(int argc, char** argv) strcpy(path, newpath); free(newpath); - log_notice("Saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size); + if (core_size > 0) + log_notice("Saved core dump of pid %lu (%s) to %s (%llu bytes)", + (long)pid, executable, path, (long long)core_size); notify_new_path(path); diff --git a/src/hooks/abrt-install-ccpp-hook.in b/src/hooks/abrt-install-ccpp-hook.in index aa01231..d4ed4a5 100755 --- a/src/hooks/abrt-install-ccpp-hook.in +++ b/src/hooks/abrt-install-ccpp-hook.in @@ -11,9 +11,9 @@ SAVED_PATTERN_DIR="@VAR_RUN@/abrt" SAVED_PATTERN_FILE="@VAR_RUN@/abrt/saved_core_pattern" HOOK_BIN="@libexecdir@/abrt-hook-ccpp" # Must match percent_specifiers[] order in abrt-hook-ccpp.c: -PATTERN="|$HOOK_BIN %s %c %p %u %g %t %e" +PATTERN="|$HOOK_BIN %s %c %p %u %g %t %e %i" # Same, but with bogus "executable name" parameter -PATTERN1="|$HOOK_BIN %s %c %p %u %g %t e" +PATTERN1="|$HOOK_BIN %s %c %p %u %g %t e %i" # core_pipe_limit specifies how many dump_helpers can run at the same time # 0 - means unlimited, but it's not guaranteed that /proc/ of crashing -- 2.1.0