[PATCH] move backtrace generation to a separate program (abrt-action-generate-backtrace)
16 Sep
2010
16 Sep
'10
1:50 p.m.
This patch splits off CCpp's backtrace generation into a separate tool:
# abrt-action-generate-backtrace -z abrt-action-generate-backtrace: invalid option -- 'z' Usage: abrt-action-generate-backtrace -d DIR [-i DIR1:DIR2] [-t SECONDS] [-vs]
Generate backtrace, its quality rating, hash, and crashed function
Options: -d DIR Crash dump directory -i DIR1:DIR2 Additional debuginfo directories -t SECONDS Kill gdb if it runs for more than SECONDS -v Verbose -s Log to syslog
This allows for simpler timeout handling using SIGALRM, and makes timers for tracking client timeouts unnecessary.
This also allows for debugging and regression testing of abrt-action-generate-backtrace separately - it can be simply run from command-line. Also it provides a better fault isolation - crash in abrt-action-generate-backtrace does not take down abrtd.
The code is based on CCpp.cpp.
CCpp analyzer is made to swawn a child to do the backtrace generation.
Run tested.
Please review.
--
vda
diff -x '*.po' -d -urpN abrt.1/abrt.spec abrt.2/abrt.spec
--- abrt.1/abrt.spec 2010-09-14 19:21:46.000000000 +0200
+++ abrt.2/abrt.spec 2010-09-14 19:30:13.081789613 +0200
@@ -349,6 +349,7 @@ fi
%endif
%{_sbindir}/abrtd
%{_sbindir}/abrt-server
+%{_sbindir}/abrt-action-generate-backtrace
%{_bindir}/%{name}-debuginfo-install
%{_bindir}/%{name}-handle-upload
%{_bindir}/%{name}-backtrace
diff -x '*.po' -d -urpN abrt.1/lib/plugins/CCpp.cpp abrt.2/lib/plugins/CCpp.cpp
--- abrt.1/lib/plugins/CCpp.cpp 2010-09-16 14:40:30.920648834 +0200
+++ abrt.2/lib/plugins/CCpp.cpp 2010-09-16 14:04:21.723648896 +0200
@@ -141,136 +141,42 @@ static char* exec_vp(char **args, uid_t
}
close(pipeout[0]);
- int st;
- waitpid(child, &st, 0); /* prevent having zombie child process */
- if (status)
- *status = st;
+ /* Prevent having zombie child process, and maybe collect status
+ * (note that status == NULL is ok too) */
+ waitpid(child, status, 0);
return strbuf_free_nobuf(buf_out);
}
-static char *get_backtrace(const char *pDebugDumpDir, const char *pDebugInfoDirs, unsigned timeout_sec)
+static void gen_backtrace(const char *pDebugDumpDir, const char *pDebugInfoDirs, unsigned timeout_sec)
{
update_client(_("Generating backtrace"));
- dump_dir_t *dd = dd_init();
- if (!dd_opendir(dd, pDebugDumpDir))
- {
- dd_close(dd);
- VERB1 log(_("Unable to open debug dump '%s'"), pDebugDumpDir);
- return NULL;
- }
-
- char *uid_str = dd_load_text(dd, CD_UID);
- uid_t uid = xatoi_u(uid_str);
- free(uid_str);
- char *executable = dd_load_text(dd, FILENAME_EXECUTABLE);
- dd_close(dd);
-
- // Workaround for
- // http://sourceware.org/bugzilla/show_bug.cgi?id=9622
- unsetenv("TERM");
- // This is not necessary, and was observed to cause
- // environment corruption (because we run in a thread?):
- //putenv((char*)"TERM=dumb");
-
- char *args[21];
- args[0] = (char*)"gdb";
- args[1] = (char*)"-batch";
-
- // when/if gdb supports "set debug-file-directory DIR1:DIR2":
- // (https://bugzilla.redhat.com/show_bug.cgi?id=528668):
- args[2] = (char*)"-ex";
- string dfd = "set debug-file-directory /usr/lib/debug";
- const char *p = pDebugInfoDirs;
- while (1)
+ pid_t pid = fork();
+ if (pid < 0)
{
- const char *colon_or_nul = strchrnul(p, ':');
- dfd += ':';
- dfd.append(p, colon_or_nul - p);
- dfd += "/usr/lib/debug";
- if (*colon_or_nul != ':')
- break;
- p = colon_or_nul + 1;
+ perror_msg("fork");
+ return;
}
- args[3] = (char*)dfd.c_str();
-
- /* "file BINARY_FILE" is needed, without it gdb cannot properly
- * unwind the stack. Currently the unwind information is located
- * in .eh_frame which is stored only in binary, not in coredump
- * or debuginfo.
- *
- * Fedora GDB does not strictly need it, it will find the binary
- * by its build-id. But for binaries either without build-id
- * (= built on non-Fedora GCC) or which do not have
- * their debuginfo rpm installed gdb would not find BINARY_FILE
- * so it is still makes sense to supply "file BINARY_FILE".
- *
- * Unfortunately, "file BINARY_FILE" doesn't work well if BINARY_FILE
- * was deleted (as often happens during system updates):
- * gdb uses specified BINARY_FILE
- * even if it is completely unrelated to the coredump.
- * See https://bugzilla.redhat.com/show_bug.cgi?id=525721
- *
- * TODO: check mtimes on COREFILE and BINARY_FILE and not supply
- * BINARY_FILE if it is newer (to at least avoid gdb complaining).
- */
- args[4] = (char*)"-ex";
- args[5] = xasprintf("file %s", executable);
- free(executable);
-
- args[6] = (char*)"-ex";
- args[7] = xasprintf("core-file %s/"FILENAME_COREDUMP, pDebugDumpDir);
-
- args[8] = (char*)"-ex";
- /*args[9] = ... see below */
- args[10] = (char*)"-ex";
- args[11] = (char*)"info sharedlib";
- /* glibc's abort() stores its message in __abort_msg variable */
- args[12] = (char*)"-ex";
- args[13] = (char*)"print (char*)__abort_msg";
- args[14] = (char*)"-ex";
- args[15] = (char*)"print (char*)__glib_assert_msg";
- args[16] = (char*)"-ex";
- args[17] = (char*)"info registers";
- args[18] = (char*)"-ex";
- args[19] = (char*)"disassemble";
- args[20] = NULL;
-
- /* Get the backtrace, but try to cap its size */
- /* Limit bt depth. With no limit, gdb sometimes OOMs the machine */
- unsigned bt_depth = 2048;
- const char *thread_apply_all = "thread apply all";
- const char *full = " full";
- char *bt = NULL;
- while (1)
+ if (pid == 0) /* child */
{
- args[9] = xasprintf("%s backtrace %u%s", thread_apply_all, bt_depth, full);
- bt = exec_vp(args, uid, /*redirect_stderr:*/ 1, timeout_sec, NULL);
- free(args[9]);
- if ((bt && strnlen(bt, 256*1024) < 256*1024) || bt_depth <= 32)
- {
- break;
- }
+ char *argv[8]; /* abrt-action-generate-backtrace [-s] -tSEC -d DIR -i DIR1:DIR2 NULL */
+ char **pp = argv;
+ *pp++ = (char*)"abrt-action-generate-backtrace";
+ if (logmode & LOGMODE_SYSLOG)
+ *pp++ = (char*)"-s";
+ *pp++ = xasprintf("-t%u", timeout_sec);
+ *pp++ = (char*)"-d";
+ *pp++ = (char*)pDebugDumpDir;
+ *pp++ = (char*)"-i";
+ *pp++ = (char*)pDebugInfoDirs;
+ *pp = NULL;
- free(bt);
- bt_depth /= 2;
- if (bt_depth <= 64 && thread_apply_all[0] != '\0')
- {
- /* This program likely has gazillion threads, dont try to bt them all */
- bt_depth = 256;
- thread_apply_all = "";
- }
- if (bt_depth <= 64 && full[0] != '\0')
- {
- /* Looks like there are gigantic local structures or arrays, disable "full" bt */
- bt_depth = 256;
- full = "";
- }
+ execvp(argv[0], argv);
+ perror_msg_and_die("Can't execute '%s'", argv[0]);
}
- free(args[5]);
- free(args[7]);
- return bt;
+ /* parent */
+ waitpid(pid, NULL, 0);
}
static void GetIndependentBuildIdPC(const char *unstrip_n_output,
@@ -764,8 +670,6 @@ void CAnalyzerCCpp::CreateReport(const c
}
}
- char *package = dd_load_text(dd, FILENAME_PACKAGE);
- char *executable = dd_load_text(dd, FILENAME_EXECUTABLE);
char *uid = dd_load_text(dd, CD_UID);
dd_close(dd); /* do not keep dir locked longer than needed */
@@ -780,118 +684,30 @@ void CAnalyzerCCpp::CreateReport(const c
VERB1 log(_("Skipping the debuginfo installation"));
free(uid);
- /* Create and store backtrace. */
- char *backtrace_str = get_backtrace(pDebugDumpDir, m_sDebugInfoDirs.c_str(), m_nGdbTimeoutSec);
- if (!backtrace_str)
- {
- backtrace_str = xstrdup("");
- VERB3 log("get_backtrace() returns NULL, broken core/gdb?");
- }
-
- char *bt_build_ids = xasprintf("%s%s", backtrace_str, (build_ids) ? build_ids : "");
- free(build_ids);
+ /* Create and store backtrace and its hash. */
+ gen_backtrace(pDebugDumpDir, m_sDebugInfoDirs.c_str(), m_nGdbTimeoutSec);
dd = dd_init();
if (!dd_opendir(dd, pDebugDumpDir))
{
dd_close(dd);
+ free(build_ids);
VERB1 log(_("Unable to open debug dump '%s'"), pDebugDumpDir);
return;
}
+
+ /* Add build_ids to backtrace */
+ char *backtrace_str = dd_load_text(dd, FILENAME_BACKTRACE);
+ char *bt_build_ids = xasprintf("%s%s", backtrace_str, (build_ids) ? build_ids : "");
dd_save_text(dd, FILENAME_BACKTRACE, bt_build_ids);
+ free(build_ids);
free(bt_build_ids);
+ free(backtrace_str);
+ /* TODO: remove, it's much easier to collect in the coredump helper */
if (m_bMemoryMap)
dd_save_text(dd, FILENAME_MEMORYMAP, "memory map of the crashed C/C++ application, not implemented yet");
- /* Compute and store UUID from the backtrace. */
- char *backtrace_cpy = xstrdup(backtrace_str);
-
- struct backtrace *backtrace = backtrace_parse(backtrace_cpy, false, false);
- free(backtrace_cpy);
-
- if (backtrace)
- {
- /* Get the quality of the full backtrace. */
- float q1 = backtrace_quality(backtrace);
-
- /* Remove all the other threads except the crash thread. */
- struct thread *crash_thread = backtrace_find_crash_thread(backtrace);
- if (crash_thread)
- backtrace_remove_threads_except_one(backtrace, crash_thread);
- else
- log_msg("Detection of crash thread failed");
-
- /* Get the quality of the crash thread. */
- float q2 = backtrace_quality(backtrace);
-
- backtrace_remove_noncrash_frames(backtrace);
-
- /* Do the frame removal now. */
- backtrace_limit_frame_depth(backtrace, 5);
- /* Frame removal can be done before removing exit handlers. */
- backtrace_remove_exit_handlers(backtrace);
-
- /* Get the quality of frames around the crash. */
- float q3 = backtrace_quality(backtrace);
-
- /* Compute UUID. */
- struct strbuf *bt = backtrace_tree_as_str(backtrace, false);
- strbuf_prepend_str(bt, executable);
- strbuf_prepend_str(bt, package);
- char hash_str[SHA1_RESULT_LEN*2 + 1];
- create_hash(hash_str, bt->buf);
- dd_save_text(dd, FILENAME_GLOBAL_UUID, hash_str);
- strbuf_free(bt);
-
- /* Compute and store backtrace rating. The crash frame
- is more important that the others. The frames around
- the crash are more important than the rest. */
- float qtot = 0.25f * q1 + 0.35f * q2 + 0.4f * q3;
-
- /* Turn the quality to rating. */
- const char *rating;
- if (qtot < 0.6f) rating = "0";
- else if (qtot < 0.7f) rating = "1";
- else if (qtot < 0.8f) rating = "2";
- else if (qtot < 0.9f) rating = "3";
- else rating = "4";
- dd_save_text(dd, FILENAME_RATING, rating);
-
- /* Get the function name from the crash frame. */
- if (crash_thread)
- {
- struct frame *crash_frame = crash_thread->frames;
- struct frame *abort_frame = thread_find_abort_frame(crash_thread);
- if (abort_frame)
- crash_frame = abort_frame->next;
- if (crash_frame && crash_frame->function && 0 != strcmp(crash_frame->function, "??"))
- dd_save_text(dd, FILENAME_CRASH_FUNCTION, crash_frame->function);
- }
-
- backtrace_free(backtrace);
- }
- else
- {
- /* If the parser failed fall back to the independent backtrace. */
- /* If we write and use a hand-written parser instead of the bison one,
- the parser never fails, and it will be possible to get rid of
- the independent_backtrace and backtrace_rate_old. */
- struct strbuf *ibt = independent_backtrace(backtrace_str);
- strbuf_prepend_str(ibt, executable);
- strbuf_prepend_str(ibt, package);
- char hash_str[SHA1_RESULT_LEN*2 + 1];
- create_hash(hash_str, ibt->buf);
- dd_save_text(dd, FILENAME_GLOBAL_UUID, hash_str);
- strbuf_free(ibt);
-
- /* Compute and store backtrace rating. */
- /* Crash frame is not known so store nothing. */
- dd_save_text(dd, FILENAME_RATING, to_string(backtrace_rate_old(backtrace_str)).c_str());
- }
- free(executable);
- free(package);
- free(backtrace_str);
dd_close(dd);
}
diff -x '*.po' -d -urpN abrt.1/src/daemon/abrt-action-generate-backtrace.c abrt.2/src/daemon/abrt-action-generate-backtrace.c
--- abrt.1/src/daemon/abrt-action-generate-backtrace.c 1970-01-01 01:00:00.000000000 +0100
+++ abrt.2/src/daemon/abrt-action-generate-backtrace.c 2010-09-16 14:18:37.317648093 +0200
@@ -0,0 +1,428 @@
+/*
+ Copyright (C) 2009 Zdenek Prikryl (zprikryl@redhat.com)
+ Copyright (C) 2009 RedHat inc.
+
+ 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.
+*/
+#include "abrtlib.h"
+#include "backtrace.h"
+
+
+#define DEBUGINFO_CACHE_DIR LOCALSTATEDIR"/cache/abrt-di"
+
+
+static const char *dump_dir_name = ".";
+static char *debuginfo_dirs;
+static unsigned exec_timeout_sec = 60;
+
+
+static void create_hash(char hash_str[SHA1_RESULT_LEN*2 + 1], const char *pInput)
+{
+ unsigned len;
+ unsigned char hash2[SHA1_RESULT_LEN];
+ sha1_ctx_t sha1ctx;
+
+ sha1_begin(&sha1ctx);
+ sha1_hash(pInput, strlen(pInput), &sha1ctx);
+ sha1_end(hash2, &sha1ctx);
+ len = SHA1_RESULT_LEN;
+
+ char *d = hash_str;
+ unsigned char *s = hash2;
+ while (len)
+ {
+ *d++ = "0123456789abcdef"[*s >> 4];
+ *d++ = "0123456789abcdef"[*s & 0xf];
+ s++;
+ len--;
+ }
+ *d = '\0';
+ //log("hash2:%s str:'%s'", hash_str, pInput);
+}
+
+/**
+ *
+ * @param[out] status See `man 2 wait` for status information.
+ * @return Malloc'ed string
+ */
+static char* exec_vp(char **args, uid_t uid, int redirect_stderr, int *status)
+{
+ /* Nuke everything which may make setlocale() switch to non-POSIX locale:
+ * we need to avoid having gdb output in some obscure language.
+ */
+ static const char *const unsetenv_vec[] = {
+ "LANG",
+ "LC_ALL",
+ "LC_COLLATE",
+ "LC_CTYPE",
+ "LC_MESSAGES",
+ "LC_MONETARY",
+ "LC_NUMERIC",
+ "LC_TIME",
+ NULL
+ };
+
+ int flags = EXECFLG_INPUT_NUL | EXECFLG_OUTPUT | EXECFLG_SETGUID | EXECFLG_SETSID | EXECFLG_QUIET;
+ if (redirect_stderr)
+ flags |= EXECFLG_ERR2OUT;
+ VERB1 flags &= ~EXECFLG_QUIET;
+
+ int pipeout[2];
+ pid_t child = fork_execv_on_steroids(flags, args, pipeout, (char**)unsetenv_vec, /*dir:*/ NULL, uid);
+
+ /* We use this function to run gdb and unstrip. Bugs in gdb or corrupted
+ * coredumps were observed to cause gdb to enter infinite loop.
+ * Therefore we have a (largish) timeout, after which we kill the child.
+ */
+ int t = time(NULL); /* int is enough, no need to use time_t */
+ int endtime = t + exec_timeout_sec;
+
+ struct strbuf *buf_out = strbuf_new();
+
+ while (1)
+ {
+ int timeout = endtime - t;
+ if (timeout < 0)
+ {
+ kill(child, SIGKILL);
+ strbuf_append_strf(buf_out, "\nTimeout exceeded: %u seconds, killing %s\n", exec_timeout_sec, args[0]);
+ break;
+ }
+
+ /* We don't check poll result - checking read result is enough */
+ struct pollfd pfd;
+ pfd.fd = pipeout[0];
+ pfd.events = POLLIN;
+ poll(&pfd, 1, timeout * 1000);
+
+ char buff[1024];
+ int r = read(pipeout[0], buff, sizeof(buff) - 1);
+ if (r <= 0)
+ break;
+ buff[r] = '\0';
+ strbuf_append_str(buf_out, buff);
+ t = time(NULL);
+ }
+ close(pipeout[0]);
+
+ /* Prevent having zombie child process, and maybe collect status
+ * (note that status == NULL is ok too) */
+ waitpid(child, status, 0);
+
+ return strbuf_free_nobuf(buf_out);
+}
+
+static char *get_backtrace(dump_dir_t *dd)
+{
+ char *uid_str = dd_load_text(dd, CD_UID);
+ uid_t uid = xatoi_u(uid_str);
+ free(uid_str);
+ char *executable = dd_load_text(dd, FILENAME_EXECUTABLE);
+ dd_close(dd);
+
+ // Workaround for
+ // http://sourceware.org/bugzilla/show_bug.cgi?id=9622
+ unsetenv("TERM");
+ // This is not necessary
+ //putenv((char*)"TERM=dumb");
+
+ char *args[21];
+ args[0] = (char*)"gdb";
+ args[1] = (char*)"-batch";
+
+ // when/if gdb supports "set debug-file-directory DIR1:DIR2":
+ // (https://bugzilla.redhat.com/show_bug.cgi?id=528668):
+ args[2] = (char*)"-ex";
+ struct strbuf *set_debug_file_directory = strbuf_new();
+ strbuf_append_str(set_debug_file_directory, "set debug-file-directory /usr/lib/debug");
+ const char *p = debuginfo_dirs;
+ while (1)
+ {
+ while (*p == ':')
+ p++;
+ if (*p == '\0')
+ break;
+ const char *colon_or_nul = strchrnul(p, ':');
+ strbuf_append_strf(set_debug_file_directory, ":%.*s/usr/lib/debug", (int)(colon_or_nul - p), p);
+ p = colon_or_nul;
+ }
+ args[3] = strbuf_free_nobuf(set_debug_file_directory);
+
+ /* "file BINARY_FILE" is needed, without it gdb cannot properly
+ * unwind the stack. Currently the unwind information is located
+ * in .eh_frame which is stored only in binary, not in coredump
+ * or debuginfo.
+ *
+ * Fedora GDB does not strictly need it, it will find the binary
+ * by its build-id. But for binaries either without build-id
+ * (= built on non-Fedora GCC) or which do not have
+ * their debuginfo rpm installed gdb would not find BINARY_FILE
+ * so it is still makes sense to supply "file BINARY_FILE".
+ *
+ * Unfortunately, "file BINARY_FILE" doesn't work well if BINARY_FILE
+ * was deleted (as often happens during system updates):
+ * gdb uses specified BINARY_FILE
+ * even if it is completely unrelated to the coredump.
+ * See https://bugzilla.redhat.com/show_bug.cgi?id=525721
+ *
+ * TODO: check mtimes on COREFILE and BINARY_FILE and not supply
+ * BINARY_FILE if it is newer (to at least avoid gdb complaining).
+ */
+ args[4] = (char*)"-ex";
+ args[5] = xasprintf("file %s", executable);
+ free(executable);
+
+ args[6] = (char*)"-ex";
+ args[7] = xasprintf("core-file %s/"FILENAME_COREDUMP, dump_dir_name);
+
+ args[8] = (char*)"-ex";
+ /*args[9] = ... see below */
+ args[10] = (char*)"-ex";
+ args[11] = (char*)"info sharedlib";
+ /* glibc's abort() stores its message in __abort_msg variable */
+ args[12] = (char*)"-ex";
+ args[13] = (char*)"print (char*)__abort_msg";
+ args[14] = (char*)"-ex";
+ args[15] = (char*)"print (char*)__glib_assert_msg";
+ args[16] = (char*)"-ex";
+ args[17] = (char*)"info registers";
+ args[18] = (char*)"-ex";
+ args[19] = (char*)"disassemble";
+ args[20] = NULL;
+
+ /* Get the backtrace, but try to cap its size */
+ /* Limit bt depth. With no limit, gdb sometimes OOMs the machine */
+ unsigned bt_depth = 2048;
+ const char *thread_apply_all = "thread apply all";
+ const char *full = " full";
+ char *bt = NULL;
+ while (1)
+ {
+ args[9] = xasprintf("%s backtrace %u%s", thread_apply_all, bt_depth, full);
+ bt = exec_vp(args, uid, /*redirect_stderr:*/ 1, NULL);
+ free(args[9]);
+ if ((bt && strnlen(bt, 256*1024) < 256*1024) || bt_depth <= 32)
+ {
+ break;
+ }
+
+ free(bt);
+ bt_depth /= 2;
+ if (bt_depth <= 64 && thread_apply_all[0] != '\0')
+ {
+ /* This program likely has gazillion threads, dont try to bt them all */
+ bt_depth = 256;
+ thread_apply_all = "";
+ }
+ if (bt_depth <= 64 && full[0] != '\0')
+ {
+ /* Looks like there are gigantic local structures or arrays, disable "full" bt */
+ bt_depth = 256;
+ full = "";
+ }
+ }
+
+ free(args[3]);
+ free(args[5]);
+ free(args[7]);
+ return bt;
+}
+
+int main(int argc, char **argv)
+{
+ char *env_verbose = getenv("ABRT_VERBOSE");
+ if (env_verbose)
+ g_verbose = atoi(env_verbose);
+
+ debuginfo_dirs = xstrdup(DEBUGINFO_CACHE_DIR);
+ enum {
+ OPT_s = (1 << 0),
+ };
+ int optflags = 0;
+ int opt;
+ while ((opt = getopt(argc, argv, "d:i:t:vs")) != -1)
+ {
+ switch (opt)
+ {
+ case 'd':
+ dump_dir_name = optarg;
+ break;
+ case 'i': {
+ char *old = debuginfo_dirs;
+ if (optarg[0])
+ debuginfo_dirs = xasprintf("%s:%s", debuginfo_dirs, optarg);
+ else
+ debuginfo_dirs = xstrdup("");
+ free(old);
+ break;
+ }
+ case 't':
+ exec_timeout_sec = xatoi_u(optarg);
+ break;
+ case 'v':
+ g_verbose++;
+ break;
+ case 's':
+ optflags |= OPT_s;
+ break;
+ default:
+ /* Careful: the string below contains tabs, dont replace with spaces */
+ error_msg_and_die(
+ "Usage: abrt-action-generate-backtrace -d DIR [-i DIR1:DIR2] [-t SECONDS] [-vs]"
+ "\n"
+ "\nGenerate backtrace, its quality rating, hash, and crashed function"
+ "\n"
+ "\nOptions:"
+ "\n -d DIR Crash dump directory"
+ "\n -i DIR1:DIR2 Additional debuginfo directories"
+ "\n -t SECONDS Kill gdb if it runs for more than SECONDS"
+ "\n -v Verbose"
+ "\n -s Log to syslog"
+ );
+ }
+ }
+
+ putenv(xasprintf("ABRT_VERBOSE=%u", g_verbose));
+ msg_prefix = xasprintf("abrt-action-generate-backtrace[%u]", getpid());
+ if (optflags & OPT_s)
+ {
+ openlog(msg_prefix, 0, LOG_DAEMON);
+ logmode = LOGMODE_SYSLOG;
+ }
+
+ dump_dir_t *dd = dd_init();
+ if (!dd_opendir(dd, dump_dir_name))
+ {
+ dd_close(dd);
+ VERB1 log(_("Unable to open debug dump '%s'"), dump_dir_name);
+ return 1;
+ }
+
+ char *package = dd_load_text(dd, FILENAME_PACKAGE);
+ char *executable = dd_load_text(dd, FILENAME_EXECUTABLE);
+
+ /* Create and store backtrace */
+ /* NB: get_backtrace() closes dd */
+ char *backtrace_str = get_backtrace(dd);
+ if (!backtrace_str)
+ {
+ backtrace_str = xstrdup("");
+ VERB3 log("get_backtrace() returns NULL, broken core/gdb?");
+ }
+
+ dd = dd_init();
+ if (!dd_opendir(dd, dump_dir_name))
+ {
+ dd_close(dd);
+ VERB1 log(_("Unable to open debug dump '%s'"), dump_dir_name);
+ return 1;
+ }
+ dd_save_text(dd, FILENAME_BACKTRACE, backtrace_str);
+
+ /* Compute and store backtrace hash. */
+ char *backtrace_cpy = xstrdup(backtrace_str);
+ struct backtrace *backtrace = backtrace_parse(backtrace_cpy, false, false);
+ free(backtrace_cpy);
+ if (backtrace)
+ {
+ /* Get the quality of the full backtrace. */
+ float q1 = backtrace_quality(backtrace);
+
+ /* Remove all the other threads except the crash thread. */
+ struct thread *crash_thread = backtrace_find_crash_thread(backtrace);
+ if (crash_thread)
+ backtrace_remove_threads_except_one(backtrace, crash_thread);
+ else
+ log_msg("Detection of crash thread failed");
+
+ /* Get the quality of the crash thread. */
+ float q2 = backtrace_quality(backtrace);
+
+ backtrace_remove_noncrash_frames(backtrace);
+
+ /* Do the frame removal now. */
+ backtrace_limit_frame_depth(backtrace, 5);
+ /* Frame removal can be done before removing exit handlers. */
+ backtrace_remove_exit_handlers(backtrace);
+
+ /* Get the quality of frames around the crash. */
+ float q3 = backtrace_quality(backtrace);
+
+ /* Compute UUID. */
+ struct strbuf *bt = backtrace_tree_as_str(backtrace, false);
+ strbuf_prepend_str(bt, executable);
+ strbuf_prepend_str(bt, package);
+ char hash_str[SHA1_RESULT_LEN*2 + 1];
+ create_hash(hash_str, bt->buf);
+ dd_save_text(dd, FILENAME_GLOBAL_UUID, hash_str);
+ strbuf_free(bt);
+
+ /* Compute and store backtrace rating. The crash frame
+ is more important that the others. The frames around
+ the crash are more important than the rest. */
+ float qtot = 0.25f * q1 + 0.35f * q2 + 0.4f * q3;
+
+ /* Turn the quality to rating. */
+ const char *rating;
+ if (qtot < 0.6f) rating = "0";
+ else if (qtot < 0.7f) rating = "1";
+ else if (qtot < 0.8f) rating = "2";
+ else if (qtot < 0.9f) rating = "3";
+ else rating = "4";
+ dd_save_text(dd, FILENAME_RATING, rating);
+
+ /* Get the function name from the crash frame. */
+ if (crash_thread)
+ {
+ struct frame *crash_frame = crash_thread->frames;
+ struct frame *abort_frame = thread_find_abort_frame(crash_thread);
+ if (abort_frame)
+ crash_frame = abort_frame->next;
+ if (crash_frame && crash_frame->function && 0 != strcmp(crash_frame->function, "??"))
+ dd_save_text(dd, FILENAME_CRASH_FUNCTION, crash_frame->function);
+ }
+
+ backtrace_free(backtrace);
+ }
+ else
+ {
+ /* If the parser failed fall back to the independent backtrace. */
+ /* If we write and use a hand-written parser instead of the bison one,
+ the parser never fails, and it will be possible to get rid of
+ the independent_backtrace and backtrace_rate_old. */
+ struct strbuf *ibt = independent_backtrace(backtrace_str);
+ strbuf_prepend_str(ibt, executable);
+ strbuf_prepend_str(ibt, package);
+ char hash_str[SHA1_RESULT_LEN*2 + 1];
+ create_hash(hash_str, ibt->buf);
+ dd_save_text(dd, FILENAME_GLOBAL_UUID, hash_str);
+ strbuf_free(ibt);
+
+ /* Compute and store backtrace rating. */
+ /* Crash frame is not known so store nothing. */
+ int rate = backtrace_rate_old(backtrace_str);
+ char rate_buf[sizeof(rate)*3 + 2];
+ sprintf(rate_buf, "%d", rate);
+ dd_save_text(dd, FILENAME_RATING, rate_buf);
+ }
+
+ dd_close(dd);
+
+ free(executable);
+ free(package);
+ free(backtrace_str);
+
+ return 0;
+}
diff -x '*.po' -d -urpN abrt.1/src/daemon/Makefile.am abrt.2/src/daemon/Makefile.am
--- abrt.1/src/daemon/Makefile.am 2010-09-14 19:21:46.000000000 +0200
+++ abrt.2/src/daemon/Makefile.am 2010-09-14 19:30:13.084789698 +0200
@@ -1,6 +1,6 @@
bin_SCRIPTS = abrt-debuginfo-install abrt-handle-upload
-sbin_PROGRAMS = abrtd abrt-server
+sbin_PROGRAMS = abrtd abrt-server abrt-action-generate-backtrace
abrtd_SOURCES = \
PluginManager.h PluginManager.cpp \
@@ -52,6 +52,24 @@ abrt_server_CPPFLAGS = \
abrt_server_LDADD = \
../../lib/utils/libABRTUtils.la
+abrt_action_generate_backtrace_SOURCES = \
+ abrt-action-generate-backtrace.c
+abrt_action_generate_backtrace_CPPFLAGS = \
+ -I$(srcdir)/../../inc \
+ -I$(srcdir)/../../lib/utils \
+ -DBIN_DIR="$(bindir)" \
+ -DVAR_RUN="$(VAR_RUN)" \
+ -DCONF_DIR="$(CONF_DIR)" \
+ -DLOCALSTATEDIR='"$(localstatedir)"' \
+ -DDEBUG_DUMPS_DIR="$(DEBUG_DUMPS_DIR)" \
+ -DDEBUG_INFO_DIR="$(DEBUG_INFO_DIR)" \
+ -DPLUGINS_LIB_DIR="$(PLUGINS_LIB_DIR)" \
+ -DPLUGINS_CONF_DIR="$(PLUGINS_CONF_DIR)" \
+ -D_GNU_SOURCE \
+ -Wall -Werror
+abrt_action_generate_backtrace_LDADD = \
+ ../../lib/utils/libABRTUtils.la
+
dbusabrtconfdir = ${sysconfdir}/dbus-1/system.d/
dist_dbusabrtconf_DATA = dbus-abrt.conf
17 Sep
17 Sep
10:04 a.m.
New subject: [PATCH] move backtrace generation to a separate program (abrt-action-generate-backtrace)
It seems to work well.
With this patch we would have the functions create_hash and exec_vp in ABRT twice: CCpp.cpp, abrt-action-generate-backtrace.c.
Is this just a temporary state?
Dne 16.9.2010 14:50, Denys Vlasenko napsal(a):
This patch splits off CCpp's backtrace generation into a separate tool:
# abrt-action-generate-backtrace -z abrt-action-generate-backtrace: invalid option -- 'z' Usage: abrt-action-generate-backtrace -d DIR [-i DIR1:DIR2] [-t SECONDS] [-vs]
Generate backtrace, its quality rating, hash, and crashed function
Options: -d DIR Crash dump directory -i DIR1:DIR2 Additional debuginfo directories -t SECONDS Kill gdb if it runs for more than SECONDS -v Verbose -s Log to syslog
This allows for simpler timeout handling using SIGALRM, and makes timers for tracking client timeouts unnecessary.
This also allows for debugging and regression testing of abrt-action-generate-backtrace separately - it can be simply run from command-line. Also it provides a better fault isolation - crash in abrt-action-generate-backtrace does not take down abrtd.
The code is based on CCpp.cpp.
CCpp analyzer is made to swawn a child to do the backtrace generation.
Run tested.
Please review.
20 Sep
20 Sep
1:09 p.m.
New subject: [PATCH] move backtrace generation to a separate program (abrt-action-generate-backtrace)
On 09/17/2010 02:00 PM, Denys Vlasenko wrote:
On Fri, 2010-09-17 at 11:04 +0200, Karel Klic wrote:
It seems to work well.
With this patch we would have the functions create_hash and exec_vp in ABRT twice: CCpp.cpp, abrt-action-generate-backtrace.c.
Is this just a temporary state?
Yes, it's temporary.
Then the patch looks good to me.
Reviewed by: Karel Klic kklic@redhat.com
I am not sure about the signal to kill the GDB process, and we will want a manual page for every program explaining what that program does. However, those issues can be solved later.
17 Sep
17 Sep
10:11 a.m.
On Thu, 16 Sep 2010 14:50:04 +0200, Denys Vlasenko wrote:
if (timeout < 0){kill(child, SIGKILL);strbuf_append_strf(buf_out, "\nTimeout exceeded: %u seconds, killing %s\n", exec_timeout_sec, args[0]);break;}
SIGKILL is really inappropriate. Such a problem should be bugreported (via ABRT...) for GDB. There is even a (completed) project to make GDB fast: http://fedoraproject.org/wiki/Features/GdbIndex
Some SIGSEGV or which signal would be appropriate to get it caught by ABRT.
Regards, Jan
20 Sep
20 Sep
6:33 p.m.
On Fri, 2010-09-17 at 11:11 +0200, Jan Kratochvil wrote:
On Thu, 16 Sep 2010 14:50:04 +0200, Denys Vlasenko wrote:
if (timeout < 0){kill(child, SIGKILL);strbuf_append_strf(buf_out, "\nTimeout exceeded: %u seconds, killing %s\n", exec_timeout_sec, args[0]);break;}SIGKILL is really inappropriate.
Why do you think so?
Such a problem should be bugreported (via ABRT...) for GDB. There is even a (completed) project to make GDB fast: http://fedoraproject.org/wiki/Features/GdbIndex
Some SIGSEGV or which signal would be appropriate to get it caught by ABRT.
--
vda
6:41 p.m.
On Mon, 20 Sep 2010 19:33:19 +0200, Denys Vlasenko wrote:
On Fri, 2010-09-17 at 11:11 +0200, Jan Kratochvil wrote:
SIGKILL is really inappropriate.
Why do you think so?
Because:
Such a problem should be bugreported (via ABRT...) for GDB. There is even a (completed) project to make GDB fast: http://fedoraproject.org/wiki/Features/GdbIndex
I do not have reproducible an F14+ case where a GDB backtrace would take longer than 120 seconds.
The GDB crash info (backtrace etc.) as provided by ABRT may give some hints why GDB was hanging.
And even if SIGKILL is used still such case should be reported to the GDB maintainers some way. The SIGKILL case is currently not being reported AFAIK, at least I haven't seen any such GDB Bug yet.
Thanks, Jan
5031
Age (days ago)
5035
Last active (days ago)
crash-catcher@lists.fedorahosted.org
6 comments
3 participants
participants (3)
-
Denys Vlasenko -
Jan Kratochvil -
Karel Klic