On 08/10/2010 21:43, Alan Coopersmith wrote: > Matthias Hopf wrote: >> This calls /usr/bin/xorg-backtrace to create reasonable commented backtraces >> with gdb. On errors it falls back to the generic method. > > There's already similar code in the backtrace.c file in the > # ifdef HAVE_PSTACK case, where we fork the external pstack > process on Solaris to get stack traces that have more information > than the libc builtins allow, like getting full symbol tables from > libelf. > > Could we make the fork/exec wrapper common between the two? I'd > even be okay with making the entire code in backtrace.c common and > just letting the script handle calling gdb or pstack. (Solaris > systems are more likely to have pstack installed than gdb.)
Attached is an updated version of this patch which uses a common fork/exec wrapper and looks for the backtrace script in $bindir. I had to drop the use of fork1() and closefrom() as they aren't portable to my platform of interest. Unfortunately, gdb can't backtrace through a signal handler on cygwin, so this patch isn't as much use to me as I hoped it might be, so I'll just leave it here :-)
>From 8b19f11d6e5ab82e510cdcc94f2db322843d6c73 Mon Sep 17 00:00:00 2001 From: Matthias Hopf <[email protected]> Date: Fri, 8 Oct 2010 18:25:12 +0200 Subject: [PATCH] Use external tool for creating backtraces on crashes if available. Make the code for fork/exec-ing a helper program to perform a backtrace more generic, so it can be used to call /usr/bin/xorg-backtrace, a script which invokes gdb to create reasonable backtraces. On errors it falls back to the generic method. Based on a patch by Matthias Hopf mhopf at suse.de Signed-off-by: Jon TURNEY <[email protected]> --- os/Makefile.am | 2 +- os/backtrace.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/os/Makefile.am b/os/Makefile.am index 3e4f2c5..b40ee78 100644 --- a/os/Makefile.am +++ b/os/Makefile.am @@ -1,6 +1,6 @@ noinst_LTLIBRARIES = libos.la liblog.la -AM_CFLAGS = $(DIX_CFLAGS) $(SHA1_CFLAGS) +AM_CFLAGS = $(DIX_CFLAGS) $(SHA1_CFLAGS) -DBINDIR=\"$(bindir)\" SECURERPC_SRCS = rpcauth.c XDMCP_SRCS = xdmcp.c diff --git a/os/backtrace.c b/os/backtrace.c index 7ca6dab..b1ad535 100644 --- a/os/backtrace.c +++ b/os/backtrace.c @@ -28,6 +28,13 @@ #include "os.h" #include "misc.h" +#include <sys/types.h> +#include <sys/wait.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <errno.h> + #ifdef HAVE_BACKTRACE #ifndef _GNU_SOURCE #define _GNU_SOURCE @@ -41,6 +48,10 @@ void xorg_backtrace(void) const char *mod; int size, i; Dl_info info; + + if (xorg_backtrace_gdb () == 0) + return; + ErrorF("\nBacktrace:\n"); size = backtrace(array, 64); for (i = 0; i < size; i++) { @@ -121,8 +132,12 @@ static int xorg_backtrace_frame(uintptr_t pc, int signo, void *arg) } # endif /* HAVE_WALKCONTEXT */ -# ifdef HAVE_PSTACK -static int xorg_backtrace_pstack(void) { +/* + fork/exec a program to create a backtrace + Returns 0 if successful. +*/ +static int xorg_backtrace_exec_wrapper(const char* path) +{ pid_t kidpid; int pipefd[2]; @@ -130,7 +145,7 @@ static int xorg_backtrace_pstack(void) { return -1; } - kidpid = fork1(); + kidpid = fork(); if (kidpid == -1) { /* ERROR */ @@ -142,11 +157,13 @@ static int xorg_backtrace_pstack(void) { seteuid(0); close(STDIN_FILENO); close(STDOUT_FILENO); + close(STDERR_FILENO); dup2(pipefd[1],STDOUT_FILENO); - closefrom(STDERR_FILENO); + dup2(pipefd[1],STDERR_FILENO); + close(pipefd[1]); snprintf(parent, sizeof(parent), "%d", getppid()); - execle("/usr/bin/pstack", "pstack", parent, NULL); + execle(path, path, parent, NULL, NULL); exit(1); } else { /* PARENT */ @@ -154,13 +171,15 @@ static int xorg_backtrace_pstack(void) { int kidstat; int bytesread; int done = 0; - + int status = -1; + close(pipefd[1]); while (!done) { bytesread = read(pipefd[0], btline, sizeof(btline) - 1); if (bytesread > 0) { + status++; btline[bytesread] = 0; ErrorF("%s", btline); } @@ -170,18 +189,38 @@ static int xorg_backtrace_pstack(void) { } close(pipefd[0]); waitpid(kidpid, &kidstat, 0); - if (kidstat != 0) - return -1; + if (!(WIFEXITED (kidstat) && WEXITSTATUS (kidstat) == 0)) + { + ErrorF ("%s failed with returncode %d\n", path, WEXITSTATUS (kidstat)); + return -1; + } + if (status < 10) { + ErrorF ("%s only produced %d lines of output\n", path, status); + return -1; + } } return 0; } -# endif /* HAVE_PSTACK */ +# ifdef HAVE_PSTACK +static int xorg_backtrace_pstack(void) +{ + return xorg_backtrace_exec_wrapper("/usr/bin/pstack"); +} +#endif + +static int xorg_backtrace_gdb(void) +{ + return xorg_backtrace_exec_wrapper(BINDIR "/xorg-backtrace"); +} # if defined(HAVE_PSTACK) || defined(HAVE_WALKCONTEXT) void xorg_backtrace(void) { + if (xorg_backtrace_gdb () == 0) + return; + ErrorF("\nBacktrace:\n"); # ifdef HAVE_PSTACK @@ -206,8 +245,11 @@ void xorg_backtrace(void) { # else -/* Default fallback if we can't find any way to get a backtrace */ -void xorg_backtrace(void) { return; } +/* Default fallback if we can't find any better way to get a backtrace */ +void xorg_backtrace(void) { + if (xorg_backtrace_gdb () == 0) + return; +} # endif #endif -- 1.7.2.3
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
