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

Eli Zaretskii
2014-05-19T15:01:48Z

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