This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 0/2 v3] Demangler crash handler
- From: Mark Kettenis <mark dot kettenis at xs4all dot nl>
- To: gbenson at redhat dot com
- Cc: gdb-patches at sourceware dot org, aburgess at broadcom dot com, xdje42 at gmail dot com, eliz at gnu dot org, fw at deneb dot enyo dot de, mark dot kettenis at xs4all dot nl, palves at redhat dot com, tromey at redhat dot com
- Date: Wed, 4 Jun 2014 12:27:33 +0200 (CEST)
- Subject: Re: [PATCH 0/2 v3] Demangler crash handler
- Authentication-results: sourceware.org; auth=none
- References: <20140604100755 dot GA7570 at blade dot nx>
> Date: Wed, 4 Jun 2014 11:07:55 +0100
> From: Gary Benson <gbenson@redhat.com>
>
> Hi all,
>
> This patch is an updated version of the demangler crash handler I
> posted two weeks ago. There are two main changes from the previous
> version:
>
> 1) The signal handler creates a core file before returning.
> This ensures that a core file is created at the correct
> location.
>
> 2) I have switched the crash handler to be enabled by default.
> I think this is appropriate now that a core file is always
> created.
>
> To make this work correctly I have had to add a new class of
> internal problem, which I've called demangler_warning. This
> has the same behaviour as internal_warning except that it does
> not prompt the user to create a core file because that's already
> been done. I know Doug didn't want this but it's necessary to
> avoid either overwriting the useful core file with a non-useful
> one or confusing the user with a second, non-useful core file.
>
> I've split this patch into two parts for ease of discussion. The
> first patch adds the new internal problem functionality, and the
> second patch is the crash handler itself. This differs from the
> previous version by the addition of core file generation and in
> that it calls demangler_warning instead of internal_warning.
>
> I would push both patches as one commit. The news file entries
> for the commit would be:
>
> * New options
>
> maint set catch-demangler-crashes (on|off)
> maint show catch-demangler-crashes
> Control whether GDB should attempt to catch crashes in the symbol
> name demangler. The default is to attempt to catch crashes. If
> enabled, the first time a crash is caught, a core file is created,
> the offending symbol is displayed and the user is presented with
> the option to terminate the current session.
>
> maint set demangler-warning quit (yes|no|ask)
> When GDB catches a crash in the symbol name demangler it can offer
> the user the opportunity to both quit GDB and create a core file of
> the current GDB session. These options control whether or not to
> do either of these. The default is to create a core file and to ask
> the user whether to quit.
>
> * New commands
>
> maint demangler-warning
> Cause GDB to call the internal function demangler_warning and
> hence behave as though an internal error in the demangler has
> been detected.
>
> Is this ok to commit?
Not for me. You're running too much code between entering the SIGSEGV
handler and dumping core for my taste.
I still very much agree with Pedro; this should not be necessary.
Drop this. Spend your time on fixing the actual bugs.