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

[Eli Zaretskii <eliz@gnu.org>]

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