[gimp] app, tools: use the new gimp_print_stack_trace() to output the...

[Jehan Pagès <jehanp src gnome org>]

commit 8d2ae895bd03269eedf147983c99cfd07eb79732
Author: Jehan <jehan girinstud io>
Date:   Thu Feb 8 03:52:54 2018 +0100

    app, tools: use the new gimp_print_stack_trace() to output the...
    
    ... stacktrace into a file on non-Win32 systems.
    This has a few advantages:
    - First, we don't need to duplicate stacktrace code inside the
      independent gimp-debug-tool (I even noticed that the version in the
      tool was gdb-only and not updated for lldb fallback; proof that code
      duplication is evil!). Instead, even on a crash, we can create the
      stacktrace from the main binary and simply pass it as a file.
    - Secondly, that allows to fallback to the backtrace() API even for
      crashes (this was not possible if the backtrace was done from a
      completely different process). That's nice because this makes that we
      will always get backtraces in Linux (even though backtrace() API is
      not as nice as gdb/lldb, it's better than nothing).
    - Finally this makes the code smaller (i.e. easier to maintain), more
      consistent and similar on all platforms.

 app/errors.c            |   31 +++++++++++------------
 app/signals.c           |   37 ++++++++++++++--------------
 tools/gimp-debug-tool.c |   61 ++++++----------------------------------------
 3 files changed, 42 insertions(+), 87 deletions(-)
---
diff --git a/app/errors.c b/app/errors.c
index 416ec7b..9fc9fd0 100644
--- a/app/errors.c
+++ b/app/errors.c
@@ -34,6 +34,7 @@
 
 #include <gio/gio.h>
 #include <glib/gprintf.h>
+#include <glib/gstdio.h>
 
 #include "libgimpbase/gimpbase.h"
 
@@ -300,6 +301,9 @@ gimp_eek (const gchar *reason,
 #ifndef GIMP_CONSOLE_COMPILATION
       if (generate_backtrace && ! the_errors_gimp->no_interface)
         {
+          FILE     *fd;
+          gboolean  has_backtrace = TRUE;
+
           /* If GUI backtrace enabled (it is disabled by default), it
            * takes precedence over the command line argument.
            */
@@ -318,30 +322,26 @@ gimp_eek (const gchar *reason,
           g_snprintf (pid, 16, "%u", (guint) getpid ());
           args[2] = pid;
 
+#ifndef G_OS_WIN32
+          /* On Win32, the trace has already been processed by ExcHnl
+           * and is waiting for us in a text file.
+           */
+          fd = g_fopen (backtrace_file, "w");
+          has_backtrace = gimp_print_stack_trace (fd, NULL);
+          fclose (fd);
+#endif
+
           /* We don't care about any return value. If it fails, too
            * bad, we just won't have any stack trace.
            * We still need to use the sync() variant because we have
            * to keep GIMP up long enough for the debugger to get its
            * trace.
            */
-#ifdef HAVE_EXCHNDL
-          /* On Win32, the trace has already been processed by ExcHnl
-           * and is waiting for us in a text file.
-           * We just want to spawn the trace GUI and exit GIMP directly.
-           */
-          if (g_file_test (backtrace_file, G_FILE_TEST_IS_REGULAR) &&
+          if (has_backtrace &&
+              g_file_test (backtrace_file, G_FILE_TEST_IS_REGULAR) &&
               g_spawn_async (NULL, args, NULL,
                              G_SPAWN_SEARCH_PATH | G_SPAWN_STDERR_TO_DEV_NULL | G_SPAWN_STDOUT_TO_DEV_NULL,
                              NULL, NULL, NULL, NULL))
-#else
-          /* On Unix machines, the spawned process will attach to the
-           * main GIMP process and will generate the backtrace. So we
-           * run it as a synced process.
-           */
-          if (g_spawn_sync (NULL, args, NULL,
-                            G_SPAWN_SEARCH_PATH | G_SPAWN_STDERR_TO_DEV_NULL | G_SPAWN_STDOUT_TO_DEV_NULL,
-                            NULL, NULL, NULL, NULL, NULL, NULL))
-#endif
             eek_handled = TRUE;
         }
 #endif /* !GIMP_CONSOLE_COMPILATION */
@@ -361,7 +361,6 @@ gimp_eek (const gchar *reason,
                   sigemptyset (&sigset);
                   sigprocmask (SIG_SETMASK, &sigset, NULL);
 
-
                   gimp_on_error_query ();
                 }
               break;
diff --git a/app/signals.c b/app/signals.c
index 54ea9f6..08550ef 100644
--- a/app/signals.c
+++ b/app/signals.c
@@ -49,27 +49,10 @@ static void         gimp_sigfatal_handler (gint sig_num) G_GNUC_NORETURN;
 void
 gimp_init_signal_handlers (gchar **backtrace_file)
 {
-#ifdef G_OS_WIN32
-  /* Use Dr. Mingw (dumps backtrace on crash) if it is available. Do
-   * nothing otherwise on Win32.
-   * The user won't get any stack trace from glib anyhow.
-   * Without Dr. MinGW, It's better to let Windows inform about the
-   * program error, and offer debugging (if the user has installed MSVC
-   * or some other compiler that knows how to install itself as a
-   * handler for program errors).
-   */
-#ifdef HAVE_EXCHNDL
   time_t  t;
   gchar  *filename;
   gchar  *dir;
 
-  /* Order is very important here. We need to add our signal handler
-   * first, then run ExcHndlInit() which will add its own handler, so
-   * that ExcHnl's handler runs first since that's in FILO order.
-   */
-  if (! g_prevExceptionFilter)
-    g_prevExceptionFilter = SetUnhandledExceptionFilter (gimp_sigfatal_handler);
-
   /* This has to be the non-roaming directory (i.e., the local
      directory) as backtraces correspond to the binaries on this
      system. */
@@ -81,11 +64,29 @@ gimp_init_signal_handlers (gchar **backtrace_file)
 
   time (&t);
   filename = g_strdup_printf ("%s-crash-%" G_GUINT64_FORMAT ".txt",
-                              g_get_prgname(), t);
+                              g_get_prgname (), t);
   *backtrace_file = g_build_filename (dir, filename, NULL);
   g_free (filename);
   g_free (dir);
 
+#ifdef G_OS_WIN32
+  /* Use Dr. Mingw (dumps backtrace on crash) if it is available. Do
+   * nothing otherwise on Win32.
+   * The user won't get any stack trace from glib anyhow.
+   * Without Dr. MinGW, It's better to let Windows inform about the
+   * program error, and offer debugging (if the user has installed MSVC
+   * or some other compiler that knows how to install itself as a
+   * handler for program errors).
+   */
+
+#ifdef HAVE_EXCHNDL
+  /* Order is very important here. We need to add our signal handler
+   * first, then run ExcHndlInit() which will add its own handler, so
+   * that ExcHnl's handler runs first since that's in FILO order.
+   */
+  if (! g_prevExceptionFilter)
+    g_prevExceptionFilter = SetUnhandledExceptionFilter (gimp_sigfatal_handler);
+
   ExcHndlInit ();
   ExcHndlSetLogFileNameA (*backtrace_file);
 
diff --git a/tools/gimp-debug-tool.c b/tools/gimp-debug-tool.c
index db4e096..e5bf2cd 100644
--- a/tools/gimp-debug-tool.c
+++ b/tools/gimp-debug-tool.c
@@ -16,16 +16,13 @@
  */
 
 /*
- * GimpDebug simply runs a debugger on a given executable and PID and
- * displays a dialog proposing to create a bug report. The reason why it
+ * GimpDebug simply displays a dialog with debug data (backtraces,
+ * version, etc.), proposing to create a bug report. The reason why it
  * is a separate executable is simply that when the program crashed,
  * even though some actions are possible before exit() by catching fatal
- * errors and signals, it may not be possible anymore to allocate memory
- * anymore. Therefore creating a new dialog, or even just allocating a
- * string with the contents of the debugger are impossible actions.
+ * errors and signals, it may not be possible to allocate memory
+ * anymore. Therefore creating a new dialog is an impossible action.
  * So we call instead a separate program, then exit.
- *
- * Note: this initial version does not handle Windows yet.
  */
 
 #include <stdlib.h>
@@ -43,10 +40,6 @@
 #include "app/widgets/gimpcriticaldialog.h"
 
 
-static gchar * gimp_debug_get_stack_trace (const gchar *full_prog_name,
-                                           const gchar *pid);
-
-
 
 int
 main (int    argc,
@@ -61,10 +54,9 @@ main (int    argc,
   gchar       *error;
   GtkWidget   *dialog;
 
-  if (argc != 5 && argc != 6)
+  if (argc != 6)
     {
-      g_print ("Usage: gimpdebug-2.0 [PROGRAM] [PID] [REASON] [MESSAGE] [BT_FILE]\n\n"
-               "Note: the backtrace file is optional and only used in Windows.\n");
+      g_print ("Usage: gimpdebug-2.0 [PROGRAM] [PID] [REASON] [MESSAGE] [BT_FILE]\n");
       exit (EXIT_FAILURE);
     }
 
@@ -75,15 +67,8 @@ main (int    argc,
 
   error   = g_strdup_printf ("%s: %s", reason, message);
 
-  if (argc == 6)
-    {
-      bt_file = argv[5];
-      g_file_get_contents (bt_file, &trace, NULL, NULL);
-    }
-  else
-    {
-      trace = gimp_debug_get_stack_trace (program, pid);
-    }
+  bt_file = argv[5];
+  g_file_get_contents (bt_file, &trace, NULL, NULL);
 
   if (trace == NULL || strlen (trace) == 0)
     exit (EXIT_FAILURE);
@@ -105,33 +90,3 @@ main (int    argc,
 
   exit (EXIT_SUCCESS);
 }
-
-
-static gchar *
-gimp_debug_get_stack_trace (const gchar *full_prog_name,
-                            const gchar *pid)
-{
-  gchar   *trace  = NULL;
-
-  /* This works only on UNIX systems. On Windows, we'll have to find
-   * another method, probably with DrMingW.
-   */
-#if defined(G_OS_UNIX)
-  const gchar *args[7] = { "gdb", "-batch", "-ex", "backtrace full",
-                       full_prog_name, pid, NULL };
-  gchar       *gdb_stdout;
-
-  if (g_spawn_sync (NULL, (gchar **) args, NULL,
-                    G_SPAWN_SEARCH_PATH | G_SPAWN_STDERR_TO_DEV_NULL,
-                    NULL, NULL, &gdb_stdout, NULL, NULL, NULL))
-    {
-      trace = g_strdup (gdb_stdout);
-    }
-  else if (gdb_stdout)
-    {
-      g_free (gdb_stdout);
-    }
-#endif
-
-  return trace;
-}