Re: [PATCH 2/2 v2] Demangler crash handler

Gary Benson
2014-05-19T15:48:33Z

Eli Zaretskii wrote:
> > 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?

To clarify, the current situation in GDB is that crashes in the
demangler are not caught:

(gdb) set lang c++
(gdb) maint demangle _Z1-Av23*;cG~Wo2Vu
Segmentation fault (core dumped)

With the patch, that is also the default situation. But with the
patch, with "maint set catch-demangler-crashes on", a signal handler
is installed across calls to the demangler, so that if the demangler
crashes you get something like this:

(gdb) set lang c++
(gdb) maint set catch-demangler-crashes on
(gdb) maint demangle _Z1-Av23*;cG~Wo2Vu
/home/gary/work/archer/demangle-crashcatcher/src/gdb/cp-support.c:1590: internal-warning: unable to demangle '_Z1-Av23*;cG~Wo2Vu' (demangler failed with signal 11)
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) y

/home/gary/work/archer/demangle-crashcatcher/src/gdb/cp-support.c:1590: internal-warning: unable to demangle '_Z1-Av23*;cG~Wo2Vu' (demangler failed with signal 11)
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Create a core file of GDB? (y or n) y
Aborted (core dumped)

> > 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?

Did my above explanation answer this question?

> > 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.

My apologies. And the whole reason I Cc'd you was because the patch
contained documentation changes :) I've inlined that part at the end
of this mail.

> > +#ifdef SIGSEGV
>
> AFAIK, SIGSEGV is an ANSI-standard signal, so I don't think you need a
> preprocessor conditional here.

Ok, I can remove this.

> > + 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.

Ok, I will change the second to:

+Show whether to attempt to catch demangler crashes."), _("\

Thanks,
Gary

--
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a6bde12..32f33a9 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33142,6 +33142,16 @@ Expand symbol tables.
If @var{regexp} is specified, only expand symbol tables for file
names matching @var{regexp}.

+@kindex maint set catch-demangler-crashes
+@kindex maint show catch-demangler-crashes
+@item maint set catch-demangler-crashes [on|off]
+@itemx maint show catch-demangler-crashes
+Control whether @value{GDBN} should attempt to catch crashes in the
+symbol name demangler. The default is not to attempt to catch
+crashes. If enabled, 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 to create a core file.
+
@kindex maint cplus first_component
@item maint cplus first_component @var{name}
Print the first C@t{++} class/namespace component of @var{name}.