[gimp] app: replace g_on_error_query() and g_on_error_stack_trace() by...

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

commit 5de7aab482885f1e82c8818ef7db75cfbbbf40d2
Author: Jehan <jehan girinstud io>
Date:   Thu Feb 8 03:01:39 2018 +0100

    app: replace g_on_error_query() and g_on_error_stack_trace() by...
    
    ... our own implementation.
    Though the GUI stacktrace is better for most (because it is visible even
    when not run in a terminal), the CLI options are quite useful too and
    may still be preferred by some, in particular developers. So it may as
    well be benefiting from the better implementation. Glib traces are quite
    weak even though they also use gdb and debug info are present (often,
    even though I had these traces, I had to run gdb separately; now it
    won't be necessary in many cases). My traces include more information.
    
    Note that I didn't implement gimp_print_stack_trace() from previous
    gimp_get_stack_trace() because I cannot allocate a string after some
    types of crash (e.g. segmentation faults). So instead,
    gimp_print_stack_trace() now take care optionally of both cases: either
    allocating a string, or directly pipe to a file descriptor.

 app/errors.c |  220 +++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 163 insertions(+), 57 deletions(-)
---
diff --git a/app/errors.c b/app/errors.c
index 8315365..416ec7b 100644
--- a/app/errors.c
+++ b/app/errors.c
@@ -20,16 +20,20 @@
 #define _GNU_SOURCE  /* need the POSIX signal API */
 
 #include <stdlib.h>
+#include <string.h>
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>
 #endif
 
+#include <sys/wait.h>
+
 #ifdef HAVE_EXECINFO_H
 /* Allowing backtrace() API. */
 #include <execinfo.h>
 #endif
 
 #include <gio/gio.h>
+#include <glib/gprintf.h>
 
 #include "libgimpbase/gimpbase.h"
 
@@ -73,7 +77,9 @@ static G_GNUC_NORETURN void  gimp_eek            (const gchar        *reason,
                                                   const gchar        *message,
                                                   gboolean            use_handler);
 
-static gchar * gimp_get_stack_trace              (void);
+static gboolean  gimp_print_stack_trace          (FILE               *file,
+                                                  gchar            **trace);
+static void      gimp_on_error_query             (void);
 
 
 /*  public functions  */
@@ -230,7 +236,7 @@ gimp_message_log_func (const gchar    *log_domain,
            * Hence when this happens, critical errors are simply processed as
            * lower level errors.
            */
-          trace = gimp_get_stack_trace ();
+          gimp_print_stack_trace (NULL, &trace);
           n_traces++;
         }
     }
@@ -355,10 +361,8 @@ gimp_eek (const gchar *reason,
                   sigemptyset (&sigset);
                   sigprocmask (SIG_SETMASK, &sigset, NULL);
 
-                  if (the_errors_gimp)
-                    gimp_gui_ungrab (the_errors_gimp);
 
-                  g_on_error_query (full_prog_name);
+                  gimp_on_error_query ();
                 }
               break;
 
@@ -369,7 +373,7 @@ gimp_eek (const gchar *reason,
                   sigemptyset (&sigset);
                   sigprocmask (SIG_SETMASK, &sigset, NULL);
 
-                  g_on_error_stack_trace (full_prog_name);
+                  gimp_print_stack_trace (stdout, NULL);
                 }
               break;
 
@@ -391,86 +395,188 @@ gimp_eek (const gchar *reason,
   exit (EXIT_FAILURE);
 }
 
-static gchar *
-gimp_get_stack_trace (void)
+/* In some error cases (e.g. segmentation fault), trying to allocate
+ * more memory will trigger more segmentation faults and therefore loop
+ * our error handling (which is just wrong). Therefore printing to a
+ * file description is an implementation without any memory allocation.
+ * On the other hand, if trace is not #NULL, a newly-allocated string
+ * will be returned.
+ */
+static gboolean
+gimp_print_stack_trace (FILE   *file,
+                        gchar **trace)
 {
-  gchar   *trace  = NULL;
+  gboolean stack_printed = FALSE;
 
-  /* This works only on UNIX systems. On Windows, we'll have to find
-   * another method, probably with DrMingW.
-   */
+  /* This works only on UNIX systems. */
 #ifndef G_OS_WIN32
-  gchar *args[7] = { "gdb", "-batch", "-ex", "backtrace full",
-                     full_prog_name, NULL, NULL };
-  gchar *gdb_stdout;
-  gchar  pid[16];
+  GString *gtrace = NULL;
+  gchar    gimp_pid[16];
+  pid_t    pid;
+  gchar    buffer[256];
+  ssize_t  read_n;
+  int      out_fd[2];
 
-  g_snprintf (pid, 16, "%u", (guint) getpid ());
-  args[5] = pid;
+  g_snprintf (gimp_pid, 16, "%u", (guint) getpid ());
 
-  if (g_spawn_sync (NULL, args, NULL,
-                    G_SPAWN_SEARCH_PATH | G_SPAWN_STDERR_TO_DEV_NULL,
-                    NULL, NULL, &gdb_stdout, NULL, NULL, NULL))
+  if (pipe (out_fd) == -1)
     {
-      trace = g_strdup (gdb_stdout);
+      return FALSE;
     }
-  else if (gdb_stdout)
+
+  pid = fork ();
+  if (pid == 0)
     {
-      g_free (gdb_stdout);
+      /* Child process. */
+      gchar *args[7] = { "gdb", "-batch", "-ex", "backtrace full",
+                         full_prog_name, NULL, NULL };
+
+      args[5] = gimp_pid;
+
+      /* Redirect the debugger output. */
+      dup2 (out_fd[1], STDOUT_FILENO);
+      close (out_fd[0]);
+      close (out_fd[1]);
+
+      /* Run GDB. */
+      if (execvp (args[0], args) == -1)
+        {
+          /* LLDB as alternative. */
+          gchar *args_lldb[11] = { "lldb", "--attach-pid", NULL, "--batch",
+                                   "--one-line", "bt",
+                                   "--one-line-on-crash", "bt",
+                                   "--one-line-on-crash", "quit", NULL };
+
+          args_lldb[2] = gimp_pid;
+
+          execvp (args_lldb[0], args_lldb);
+        }
+
+      _exit (0);
     }
-  if (! trace)
+  else if (pid > 0)
     {
-      /* Alternatively, use LLDB. It seems to be more common on some
-       * platforms, especially macOS.
+      /* Main process */
+      int status;
+
+      waitpid (pid, &status, 0);
+
+      /* It is important to close the writing side of the pipe, otherwise
+       * the read() will wait forever without getting the information that
+       * writing is finished.
        */
-      gchar *args_lldb[11] = { "lldb", "--attach-pid", NULL, "--batch",
-                               "--one-line", "bt",
-                               "--one-line-on-crash", "bt",
-                               "--one-line-on-crash", "quit", NULL };
-
-      args_lldb[2] = pid;
-      if (g_spawn_sync (NULL, args_lldb, 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)
+      close (out_fd[1]);
+
+      while ((read_n = read (out_fd[0], buffer, 256)) > 0)
         {
-          g_free (gdb_stdout);
+          /* It's hard to know if the debugger was found since it
+           * happened in the child. Let's just assume that any output
+           * means it succeeded.
+           */
+          stack_printed = TRUE;
+
+          buffer[read_n] = '\0';
+          if (file)
+            g_fprintf (file, "%s", buffer);
+          if (trace)
+            {
+              if (! gtrace)
+                gtrace = g_string_new (NULL);
+              g_string_append (gtrace, (const gchar *) buffer);
+            }
         }
+      close (out_fd[0]);
+    }
+  else if (pid == (pid_t) -1)
+    {
+      /* Fork failed. */
+      return FALSE;
     }
-#endif
 
 #ifdef HAVE_EXECINFO_H
-  /* As a last resort, try using the backtrace() Linux API. It is a bit
-   * less fancy than gdb or lldb, which is why it is not given priority.
-   */
-  if (! trace)
+  if (! stack_printed)
     {
-      void  *buffer[100];
+      /* As a last resort, try using the backtrace() Linux API. It is a bit
+       * less fancy than gdb or lldb, which is why it is not given priority.
+       */
+      void  *bt_buf[100];
       char **symbols;
       int    n_symbols;
       int    i;
 
-      n_symbols = backtrace (buffer, 100);
-      symbols = backtrace_symbols (buffer, n_symbols);
+      n_symbols = backtrace (bt_buf, 100);
+      symbols = backtrace_symbols (bt_buf, n_symbols);
       if (symbols)
         {
-          GString *gtrace = g_string_new (NULL);
+          stack_printed = TRUE;
 
           for (i = 0; i < n_symbols; i++)
             {
-              g_string_append (gtrace,
-                               (const gchar *) symbols[i]);
-              g_string_append_c (gtrace, '\n');
+              if (file)
+                g_fprintf (file, "%s\n", (const gchar *) symbols[i]);
+              if (trace)
+                {
+                  if (! gtrace)
+                    gtrace = g_string_new (NULL);
+                  g_string_append (gtrace,
+                                   (const gchar *) symbols[i]);
+                  g_string_append_c (gtrace, '\n');
+                }
             }
-          trace = g_string_free (gtrace, FALSE);
-
           free (symbols);
         }
     }
-#endif
+#endif /* HAVE_EXECINFO_H */
+
+  if (trace)
+    {
+      if (gtrace)
+        *trace = g_string_free (gtrace, FALSE);
+      else
+        *trace = NULL;
+    }
+#endif /* G_OS_WIN32 */
+
+  return stack_printed;
+}
 
-  return trace;
+/* This is mostly the same as g_on_error_query() except that we use our
+ * own backtrace function, much more complete.
+ */
+static void
+gimp_on_error_query (void)
+{
+#ifndef G_OS_WIN32
+  gchar buf[16];
+
+ retry:
+
+  g_fprintf (stdout,
+             "%s (pid:%u): %s: ",
+             full_prog_name,
+             (guint) getpid (),
+             "[E]xit, show [S]tack trace or [P]roceed");
+  fflush (stdout);
+
+  if (isatty(0) && isatty(1))
+    fgets (buf, 8, stdin);
+  else
+    strcpy (buf, "E\n");
+
+  if ((buf[0] == 'E' || buf[0] == 'e')
+      && buf[1] == '\n')
+    _exit (0);
+  else if ((buf[0] == 'P' || buf[0] == 'p')
+           && buf[1] == '\n')
+    return;
+  else if ((buf[0] == 'S' || buf[0] == 's')
+           && buf[1] == '\n')
+    {
+      if (! gimp_print_stack_trace (stdout, NULL))
+        g_fprintf (stderr, "%s\n", "Stack trace not available on your system.");
+      goto retry;
+    }
+  else
+    goto retry;
+#endif
 }