Use external tool for creating backtraces on crashes if available. (v3)xserver-cygwin-1.12.1-1

Based on a patch by Matthias Hopf mhopf at suse.de

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 <mhopf-l3A5Bk7waGM-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org>
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.

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>

2 files changed, 39 insertions, 10 deletions

diff --git a/os/Makefile.am b/os/Makefile.am
index 88914852f..30ce9e1be 100644
--- a/os/Makefile.am
+++ b/os/Makefile.am
@@ -1,6 +1,6 @@
 noinst_LTLIBRARIES = libos.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 81348f417..9d525b545 100644
--- a/os/backtrace.c
+++ b/os/backtrace.c
@@ -30,6 +30,13 @@
 #include <errno.h>
 #include <string.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
@@ -138,9 +145,12 @@ xorg_backtrace_frame(uintptr_t pc, int signo, void *arg)
 }
 #endif                          /* HAVE_WALKCONTEXT */
 
-#ifdef HAVE_PSTACK
+/*
+  fork/exec a program to create a backtrace
+  Returns 0 if successful.
+*/
 static int
-xorg_backtrace_pstack(void)
+xorg_backtrace_exec_wrapper(const char *path)
 {
     pid_t kidpid;
     int pipefd[2];
@@ -149,7 +159,7 @@ xorg_backtrace_pstack(void)
         return -1;
     }
 
-    kidpid = fork1();
+    kidpid = fork();
 
     if (kidpid == -1) {
         /* ERROR */
@@ -162,11 +172,13 @@ 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 {
@@ -185,17 +197,33 @@ xorg_backtrace_pstack(void)
                 btline[bytesread] = 0;
                 ErrorF("%s", btline);
             }
-            else if ((bytesread < 0) || ((errno != EINTR) && (errno != EAGAIN)))
+            else if ((bytesread == 0) || ((errno != EINTR) && (errno != EAGAIN)))
                 done = 1;
         }
         close(pipefd[0]);
         waitpid(kidpid, &kidstat, 0);
-        if (kidstat != 0)
+        if (!(WIFEXITED(kidstat) && WEXITSTATUS(kidstat) == 0)) {
+            ErrorF("%s failed with returncode %d\n", path,
+                   WEXITSTATUS(kidstat));
             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_script(void)
+{
+    return xorg_backtrace_exec_wrapper(BINDIR "/xorg-backtrace");
+}
 
 #if defined(HAVE_PSTACK) || defined(HAVE_WALKCONTEXT)
 
@@ -232,7 +260,8 @@ xorg_backtrace(void)
 void
 xorg_backtrace(void)
 {
-    return;
+    if (xorg_backtrace_script() == 0)
+        return;
 }
 
 #endif