[2/2,v2] Demangler crash handler
Commit Message
Hi all,
This patch is an updated version of patch 2 of the demangler crash
handler I posted last week. Patch 1 of this series [1] remains the
same.
The main change I have made is to cause the crash handler to be
disabled by default. The user must explicitly enable the handler
with "maint set catch-demangler-crashes on". This will simplify
triage of bugs such as PR 16957 [2], a new demangler crash that was
reported on Friday. The user did not supply enough information to
see the offending symbol, and I don't have the necessary compiler
or libraries to reproduce the bug locally. With this patch we can
instruct the user to enter "maint set catch-demangler-crashes on"
and repeat whatever they did to cause the crash in the first place.
We can then easily obtain the first offending symbol GDB encountered.
The other main change I have made from the original patch is to
remove as much code as possible from the signal handler itself
such that only a (sig)longjmp remains. While this is still not
strictly safe, it should be clear if any future problems have been
caused by this because the problem will only occur if the user has
enabled the crash handler.
One final change from my original patch is that this patch uses
internal_warning to offer the same "quit this session/create a
core file" choices as offered for other internal problems.
Is this ok to commit?
Thanks,
Gary
--
[1] https://sourceware.org/ml/gdb-patches/2014-05/msg00109.html
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=16957
--
This patch adds a new maintainence boolean, catch_demangler_crashes.
If set, calls to the demangler are wrapped with a segmentation fault
handler. The first time a segmentation fault is caught the offending
symbol is displayed and internal_warning is called.
The NEWS file entry for this patch is as follows:
* New options
maint set catch-demangler-crashes (on|off)
maint show catch-demangler-crashes
Control whether the debugger should attempt to catch crashes in the
symbol name demangler. The default is not to attempt to catch
crashes. The first time a crash is caught the offending symbol is
displayed and the user is presented with options to terminate the
current session and/or to create a core file.
gdb/
2014-05-19 Gary Benson <gbenson@redhat.com>
* cp-support.c (signal.h): New include.
(catch_demangler_crashes): New flag.
(SIGJMP_BUF): New define.
(SIGSETJMP): Likewise.
(SIGLONGJMP): Likewise.
(gdb_demangle_jmp_buf): New variable.
(gdb_demangle_signal_handler): New function.
(gdb_demangle): If catch_demangler_crashes is set, install the
above signal handler before calling bfd_demangle, and restore
the original signal handler afterwards. Display the offending
symbol and call internal_warning the first time a segmentation
fault is caught.
(_initialize_cp_support): New maint set/show command.
gdb/doc/
2014-05-19 Gary Benson <gbenson@redhat.com>
* gdb.texinfo (Maintenance Commands): Document new
"maint set/show catch-demangler-crashes" option.
Comments
> Date: Mon, 19 May 2014 12:48:02 +0100
> From: Gary Benson <gbenson@redhat.com>
> Cc: Andrew Burgess <aburgess@broadcom.com>, Doug Evans <xdje42@gmail.com>,
> Eli Zaretskii <eliz@gnu.org>, Florian Weimer <fw@deneb.enyo.de>,
> Mark Kettenis <mark.kettenis@xs4all.nl>,
> Pedro Alves <palves@redhat.com>, Tom Tromey <tromey@redhat.com>
>
> The main change I have made is to cause the crash handler to be
> disabled by default. The user must explicitly enable the handler
> with "maint set catch-demangler-crashes on". This will simplify
> triage of bugs such as PR 16957 [2], a new demangler crash that was
> reported on Friday. The user did not supply enough information to
> see the offending symbol, and I don't have the necessary compiler
> or libraries to reproduce the bug locally. With this patch we can
> instruct the user to enter "maint set catch-demangler-crashes on"
> and repeat whatever they did to cause the crash in the first place.
> We can then easily obtain the first offending symbol GDB encountered.
Can't say this option makes sense to me. Isn't there a way to display
the necessary information in a message, even though you catch the
signal?
> maint set catch-demangler-crashes (on|off)
> maint show catch-demangler-crashes
> Control whether the debugger should attempt to catch crashes in the
> symbol name demangler. The default is not to attempt to catch
> crashes. The first time a crash is caught the offending symbol is
> displayed and the user is presented with options to terminate the
> current session and/or to create a core file.
Given this description, it sounds like all the necessary information
is already displayed when the crash is caught. So why would we need
an option?
> gdb/doc/
> 2014-05-19 Gary Benson <gbenson@redhat.com>
>
> * gdb.texinfo (Maintenance Commands): Document new
> "maint set/show catch-demangler-crashes" option.
This part of the patch was absent from what you sent.
> +#ifdef SIGSEGV
AFAIK, SIGSEGV is an ANSI-standard signal, so I don't think you need a
preprocessor conditional here.
> + add_setshow_boolean_cmd ("catch-demangler-crashes", class_maintenance,
> + &catch_demangler_crashes, _("\
> +Set whether to attempt to catch demangler crashes."), _("\
> +Show whether GDB will attempt to catch demangler crashes."), _("\
The "Set" and "Show" lines should be identical except for the initial
word.
Thanks.
@@ -36,6 +36,7 @@
#include "value.h"
#include "cp-abi.h"
#include "language.h"
+#include <signal.h>
#include "safe-ctype.h"
@@ -1505,12 +1506,101 @@ cp_lookup_rtti_type (const char *name, struct block *block)
return rtti_type;
}
+#ifdef SIGSEGV
+
+/* If nonzero, attempt to catch crashes in the demangler and print
+ useful debugging information. */
+
+static int catch_demangler_crashes = 0;
+
+/* Wrap 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;
+
+ if (catch_demangler_crashes)
+ {
+ sa.sa_handler = gdb_demangle_signal_handler;
+ sigemptyset (&sa.sa_mask);
+ sa.sa_flags = 0;
+ sigaction (SIGSEGV, &sa, &old_sa);
+ }
+#else
+ void (*ofunc) ();
+
+ if (catch_demangler_crashes)
+ ofunc = (void (*)()) signal (SIGSEGV, gdb_demangle_signal_handler);
+#endif
+
+ if (catch_demangler_crashes)
+ crash_signal = SIGSETJMP (gdb_demangle_jmp_buf);
+#endif
+
+ if (crash_signal == 0)
+ result = bfd_demangle (NULL, name, options);
+
+#ifdef SIGSEGV
+ if (catch_demangler_crashes)
+ {
+#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
+ sigaction (SIGSEGV, &old_sa, NULL);
+#else
+ signal (SIGSEGV, ofunc);
+#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;
+ }
+ }
+#endif
+
+ return result;
}
/* Don't allow just "maintenance cplus". */
@@ -1585,4 +1675,17 @@ _initialize_cp_support (void)
Usage: info vtbl EXPRESSION\n\
Evaluate EXPRESSION and display the virtual function table for the\n\
resulting object."));
+
+#ifdef SIGSEGV
+ add_setshow_boolean_cmd ("catch-demangler-crashes", class_maintenance,
+ &catch_demangler_crashes, _("\
+Set whether to attempt to catch demangler crashes."), _("\
+Show whether GDB will attempt to catch demangler crashes."), _("\
+If enabled GDB will attempt to catch demangler crashes and\n\
+display the offending symbol."),
+ NULL,
+ NULL,
+ &maintenance_set_cmdlist,
+ &maintenance_show_cmdlist);
+#endif
}