From: Doug Evans <xdje42@gmail.com>
To: Gary Benson <gbenson@redhat.com>
Cc: gdb-patches@sourceware.org,
Andrew Burgess <aburgess@broadcom.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>
Subject: Re: [PATCH 2/2 v3] Demangler crash handler
Date: Wed, 04 Jun 2014 16:05:00 -0000 [thread overview]
Message-ID: <m37g4wd5ji.fsf@sspiff.org> (raw)
In-Reply-To: <20140604100957.GC7570@blade.nx> (Gary Benson's message of "Wed, 4 Jun 2014 11:09:57 +0100")
Hi. A few comments inline.
Gary Benson <gbenson@redhat.com> writes:
> This patch wraps calls to the demangler with a segmentation fault
> handler. The first time a segmentation fault is caught a core file
> is generated and the user is prompted to file a bug and offered the
> choice to exit or to continue their GDB session. A maintainence
> option is provided to allow the user to disable the crash handler
> if required.
>
> Eli pointed out that SIGSEGV is an ANSI-standard signal but I found
> various other SIGSEGV checks in GDB so I have left the preprocessor
> conditionals intact for consistency. I hope this is ok.
>
>
> gdb/
> 2014-06-04 Gary Benson <gbenson@redhat.com>
>
> * utils.h (dump_core): New declaration.
> * utils.c (dump_core): Make non-static.
> * cp-support.c (signal.h): New include.
> (catch_demangler_crashes): New flag.
> (SIGJMP_BUF): New define.
> (SIGSETJMP): Likewise.
> (SIGLONGJMP): Likewise.
> (gdb_demangle_jmp_buf): New static global.
> (gdb_demangle_signal_handler): New function.
> (gdb_demangle): If catch_demangler_crashes is set, install the
> above signal handler before calling bfd_demangle, and restore
> the original signal handler afterwards. Display the offending
> symbol and call demangler_warning the first time a segmentation
> fault is caught.
> (_initialize_cp_support): New maint set/show command.
>
> [...]
>
> +/* Stack context and environment for demangler crash recovery. */
> +
> +static SIGJMP_BUF gdb_demangle_jmp_buf;
> +
> +/* Signal handler for gdb_demangle. */
> +
> +static void
> +gdb_demangle_signal_handler (int signo)
> +{
> + static int core_dumped = 0;
> +
> + if (!core_dumped)
> + {
> + if (fork () == 0)
> + dump_core ();
IIUC you're skipping the can_dump_core() check.
If the user has set ulimit -c 0, I think that needs to be obeyed.
I realize can_dump_core may call fprintf which we can't do here,
but you could still IMO call getrlimit.
IWBN to still call can_dump_core (or whatever) so that the
implementation of the check is still tucked away in a function.
> +
> + core_dumped = 1;
> + }
> +
> + SIGLONGJMP (gdb_demangle_jmp_buf, signo);
> +}
> +
> +#endif
> +
> /* A wrapper for bfd_demangle. */
>
> char *
> gdb_demangle (const char *name, int options)
> {
> - return bfd_demangle (NULL, name, options);
> + char *result = NULL;
> + int crash_signal = 0;
> +
> +#if defined (SIGSEGV) && defined (HAVE_WORKING_FORK)
> +#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
> + struct sigaction sa, old_sa;
> +
> + if (catch_demangler_crashes)
> + {
> + sa.sa_handler = gdb_demangle_signal_handler;
> + sigemptyset (&sa.sa_mask);
> + sa.sa_flags = 0;
> + sigaction (SIGSEGV, &sa, &old_sa);
> + }
> +#else
> + void (*ofunc) ();
> +
> + if (catch_demangler_crashes)
> + ofunc = (void (*)()) signal (SIGSEGV, gdb_demangle_signal_handler);
> +#endif
> +
> + if (catch_demangler_crashes)
> + crash_signal = SIGSETJMP (gdb_demangle_jmp_buf);
> +#endif
> +
> + if (crash_signal == 0)
> + result = bfd_demangle (NULL, name, options);
> +
> +#if defined (SIGSEGV) && defined (HAVE_WORKING_FORK)
> + if (catch_demangler_crashes)
> + {
> +#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
> + sigaction (SIGSEGV, &old_sa, NULL);
> +#else
> + signal (SIGSEGV, ofunc);
> +#endif
> +
> + if (crash_signal != 0)
> + {
> + static int error_reported = 0;
> +
> + if (!error_reported)
For myself as a user I'd like the warning for every demangle failure.
[I'd throttle it by unique symbols though.]
> + {
> + demangler_warning (__FILE__, __LINE__,
> + _("unable to demangle '%s' "
> + "(demangler failed with signal %d)"),
> + name, crash_signal);
> +
> + error_reported = 1;
> + }
> +
> + result = NULL;
> + }
> + }
> +#endif
> +
> + return result;
> }
next prev parent reply other threads:[~2014-06-04 16:05 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-04 10:08 [PATCH 0/2 " Gary Benson
2014-06-04 10:09 ` [PATCH 1/2 v3] Add new internal problem for demangler warnings Gary Benson
2014-06-04 10:18 ` Eli Zaretskii
2014-06-04 13:34 ` Gary Benson
2014-06-04 10:10 ` [PATCH 2/2 v3] Demangler crash handler Gary Benson
2014-06-04 10:20 ` Eli Zaretskii
2014-06-04 13:36 ` Gary Benson
2014-06-04 13:41 ` Eli Zaretskii
2014-06-04 14:28 ` Gary Benson
2014-06-04 15:24 ` Doug Evans
2014-06-04 18:25 ` Gary Benson
2014-06-05 1:11 ` Doug Evans
2014-06-05 2:54 ` Eli Zaretskii
2014-06-04 16:05 ` Doug Evans [this message]
2014-06-04 18:34 ` Gary Benson
2014-06-04 10:21 ` [PATCH 0/2 " Eli Zaretskii
2014-06-04 13:41 ` Gary Benson
2014-06-04 13:49 ` Eli Zaretskii
2014-06-04 14:28 ` Gary Benson
2014-06-04 10:28 ` Mark Kettenis
2014-06-04 13:34 ` Gary Benson
2014-06-04 14:54 ` Andrew Burgess
2014-06-04 15:52 ` Doug Evans
2014-06-04 15:57 ` Gary Benson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m37g4wd5ji.fsf@sspiff.org \
--to=xdje42@gmail.com \
--cc=aburgess@broadcom.com \
--cc=eliz@gnu.org \
--cc=fw@deneb.enyo.de \
--cc=gbenson@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=mark.kettenis@xs4all.nl \
--cc=palves@redhat.com \
--cc=tromey@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).