Re: [PATCH 0/2] Demangler crash handler

[Gary Benson <gbenson@redhat.com>]

Doug Evans wrote:
> On Fri, May 9, 2014 at 8:33 AM, Gary Benson <gbenson@redhat.com> wrote:
> > [...]
> > +  if (crash_signal != 0)
> > +    {
> > +      static int warning_printed = 0;
> > +
> > +      if (!warning_printed)
> > +       {
> > +         warning ("internal error: demangler failed with signal %d\n"
> > +                  "Unable to demangle '%s'\n"
> > +                  "This is a bug, "
> > +                  "please report it to the GDB maintainers.",
> > +                  crash_signal, name);
> > +
> > +         warning_printed = 1;
> > +       }
> > +
> > +      result = NULL;
> > +    }
> > +
> > +  return result;
> 
> Applying "Consistency Is Good" to this patch, I wonder if we should
> do something similar to what we do for internal errors.  I'm not
> sure I would use the same flag (grep for internal_problem_modes and
> friends in utils.c), but OTOH I wouldn't want a proliferation of
> options for controlling each particular kind of "crash".
> 
> What do you think?

I did not realize internal_error had the ability to continue over an
error.  Rather than doing something similar to internal_error I can
just update the patch to use it.  I actually prefer this to using
regular warning as the user has to explicity continue over the error.
They cannot miss that something is wrong and that they are now in
uncharted territory, but at the same time they have to option to
try and continue using GDB to debug their program.

I attached an updated patch that uses internal_warning below.  It
might be desirable to create a new warning (demangler_warning) that
would work like internal_warning but not offer the possibility of
saving a not-particularly-useful core dump.  That's a refinement
I'm not sure is necessary, however: this isn't everyday code.

As an aside, would it be worth printing an extra line or two from
internal_vproblem asking the user to report the bug and directing
them to the bug tracker and/or the mailing lists?

Thanks,
Gary

--
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 91533e8..a99c827 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -36,6 +36,7 @@
 #include "value.h"
 #include "cp-abi.h"
 #include "language.h"
+#include <signal.h>
 
 #include "safe-ctype.h"
 
@@ -1505,12 +1506,88 @@ cp_lookup_rtti_type (const char *name, struct block *block)
   return rtti_type;
 }
 
+#ifdef SIGSEGV
+
+/* PortabiWrap set/long jmp so that it's more portable.  */
+
+#if defined(HAVE_SIGSETJMP)
+#define SIGJMP_BUF		sigjmp_buf
+#define SIGSETJMP(buf)		sigsetjmp((buf), 1)
+#define SIGLONGJMP(buf,val)	siglongjmp((buf), (val))
+#else
+#define SIGJMP_BUF		jmp_buf
+#define SIGSETJMP(buf)		setjmp(buf)
+#define SIGLONGJMP(buf,val)	longjmp((buf), (val))
+#endif
+
+/* Stack context and environment for demangler crash recovery.  */
+
+static SIGJMP_BUF gdb_demangle_jmp_buf;
+
+/* Signal handler for gdb_demangle.  */
+
+static void
+gdb_demangle_signal_handler (int signo)
+{
+  SIGLONGJMP (gdb_demangle_jmp_buf, signo);
+}
+
+#endif
+
 /* A wrapper for bfd_demangle.  */
 
 char *
 gdb_demangle (const char *name, int options)
 {
-  return bfd_demangle (NULL, name, options);
+  char *result = NULL;
+  int crash_signal = 0;
+
+#ifdef SIGSEGV
+#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
+  struct sigaction sa, old_sa;
+
+  sa.sa_handler = gdb_demangle_signal_handler;
+  sigemptyset (&sa.sa_mask);
+  sa.sa_flags = 0;
+  sigaction (SIGSEGV, &sa, &old_sa);
+#else
+  void (*ofunc) ();
+
+  ofunc = (void (*)()) signal (SIGSEGV, gdb_demangle_signal_handler);
+#endif
+
+  crash_signal = SIGSETJMP (gdb_demangle_jmp_buf);
+#endif
+
+  if (crash_signal == 0)
+    result = bfd_demangle (NULL, name, options);
+
+#ifdef SIGSEGV
+#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
+  sigaction (SIGSEGV, &old_sa, NULL);
+#else
+  signal (SIGSEGV, ofunc);
+#endif
+#endif
+
+  if (crash_signal != 0)
+    {
+      static int error_reported = 0;
+
+      if (!error_reported)
+	{
+	  internal_warning (__FILE__, __LINE__,
+			    _("unable to demangle '%s' "
+			      "(demangler failed with signal %d)"),
+			    name, crash_signal);
+
+	  error_reported = 1;
+	}
+
+      result = NULL;
+    }
+
+  return result;
 }
 
 /* Don't allow just "maintenance cplus".  */