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

[Gary Benson <gbenson@redhat.com>]

Eli Zaretskii wrote:
> > Date: Mon, 19 May 2014 16:48:23 +0100
> > From: Gary Benson <gbenson@redhat.com>
> > Cc: gdb-patches@sourceware.org, aburgess@broadcom.com, xdje42@gmail.com,
> >         fw@deneb.enyo.de, mark.kettenis@xs4all.nl, palves@redhat.com,
> >         tromey@redhat.com
> > 
> > > 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)
>
> Yes, I knew all that (because I've read all the deliberations here
> about this feature).  I'm asking why do we need this option, instead
> of having its ON effect by default?

Ah, my apologies.  On by default is my preference--it seems to work,
and it doesn't rob performance--but I don't think that will to be
accepted because there's no way to do this without (sig)longjmp, and
that isn't safe to call from a signal handler.  A disabled-by-default
catcher is at least somewhat helpful in triaging all these demangler
bugs that are coming in now people are starting to use C++11 features.

> > --- 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
> 
> Please add here
> 
>  @cindex demangler crashes
> 
> Otherwise, this part is OK.  Thanks.

Thank you.
Gary

-- 
http://gbenson.net/