[0/2] Demangler crash handler
Commit Message
Doug Evans wrote:
> On Fri, May 9, 2014 at 8:33 AM, Gary Benson <gbenson@redhat.com> wrote:
> > [...]
> > + if (crash_signal != 0)
> > + {
> > + static int warning_printed = 0;
> > +
> > + if (!warning_printed)
> > + {
> > + warning ("internal error: demangler failed with signal %d\n"
> > + "Unable to demangle '%s'\n"
> > + "This is a bug, "
> > + "please report it to the GDB maintainers.",
> > + crash_signal, name);
> > +
> > + warning_printed = 1;
> > + }
> > +
> > + result = NULL;
> > + }
> > +
> > + return result;
>
> Applying "Consistency Is Good" to this patch, I wonder if we should
> do something similar to what we do for internal errors. I'm not
> sure I would use the same flag (grep for internal_problem_modes and
> friends in utils.c), but OTOH I wouldn't want a proliferation of
> options for controlling each particular kind of "crash".
>
> What do you think?
I did not realize internal_error had the ability to continue over an
error. Rather than doing something similar to internal_error I can
just update the patch to use it. I actually prefer this to using
regular warning as the user has to explicity continue over the error.
They cannot miss that something is wrong and that they are now in
uncharted territory, but at the same time they have to option to
try and continue using GDB to debug their program.
I attached an updated patch that uses internal_warning below. It
might be desirable to create a new warning (demangler_warning) that
would work like internal_warning but not offer the possibility of
saving a not-particularly-useful core dump. That's a refinement
I'm not sure is necessary, however: this isn't everyday code.
As an aside, would it be worth printing an extra line or two from
internal_vproblem asking the user to report the bug and directing
them to the bug tracker and/or the mailing lists?
Thanks,
Gary
--
Comments
Gary> I attached an updated patch that uses internal_warning below. It
Gary> As an aside, would it be worth printing an extra line or two from
Gary> internal_vproblem asking the user to report the bug and directing
Gary> them to the bug tracker and/or the mailing lists?
I think that would be a good idea.
Tom
>>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:
Gary> char *
Gary> gdb_demangle (const char *name, int options)
Gary> {
[...]
Gary> + sigaction (SIGSEGV, &sa, &old_sa);
[...]
Gary> + crash_signal = SIGSETJMP (gdb_demangle_jmp_buf);
[...]
Gary> + sigaction (SIGSEGV, &old_sa, NULL);
This adds two calls to sigaction and a call to sigsetjmp to every
demangling invocation.
I wonder whether the performance cost of this is noticeable; and if so,
how large the effect is.
If it is too large we could perhaps arrange to do the sigaction calls
just once and see if that helps.
Tom
Tom Tromey wrote:
> >>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:
>
> Gary> char *
> Gary> gdb_demangle (const char *name, int options)
> Gary> {
> [...]
> Gary> + sigaction (SIGSEGV, &sa, &old_sa);
> [...]
> Gary> + crash_signal = SIGSETJMP (gdb_demangle_jmp_buf);
> [...]
> Gary> + sigaction (SIGSEGV, &old_sa, NULL);
>
> This adds two calls to sigaction and a call to sigsetjmp to every
> demangling invocation.
>
> I wonder whether the performance cost of this is noticeable; and if
> so, how large the effect is.
I tested this before I mailed the patch: it's not noticable.
FWIW I did time gdb -nx -batch \
/usr/lib64/libreoffice/program/soffice.bin \
-ex "start" -ex "complete b" > /dev/null
Cheers,
Gary
Tom Tromey wrote:
> Gary> I attached an updated patch that uses internal_warning below.
> Gary> It As an aside, would it be worth printing an extra line or
> Gary> two from internal_vproblem asking the user to report the bug
> Gary> and directing them to the bug tracker and/or the mailing
> Gary> lists?
>
> I think that would be a good idea.
https://sourceware.org/ml/gdb-patches/2014-05/msg00198.html
Cheers,
Gary
@@ -36,6 +36,7 @@
#include "value.h"
#include "cp-abi.h"
#include "language.h"
+#include <signal.h>
#include "safe-ctype.h"
@@ -1505,12 +1506,88 @@ cp_lookup_rtti_type (const char *name, struct block *block)
return rtti_type;
}
+#ifdef SIGSEGV
+
+/* PortabiWrap set/long jmp so that it's more portable. */
+
+#if defined(HAVE_SIGSETJMP)
+#define SIGJMP_BUF sigjmp_buf
+#define SIGSETJMP(buf) sigsetjmp((buf), 1)
+#define SIGLONGJMP(buf,val) siglongjmp((buf), (val))
+#else
+#define SIGJMP_BUF jmp_buf
+#define SIGSETJMP(buf) setjmp(buf)
+#define SIGLONGJMP(buf,val) longjmp((buf), (val))
+#endif
+
+/* 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)
+{
+ 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;
+
+#ifdef SIGSEGV
+#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
+ struct sigaction sa, old_sa;
+
+ sa.sa_handler = gdb_demangle_signal_handler;
+ sigemptyset (&sa.sa_mask);
+ sa.sa_flags = 0;
+ sigaction (SIGSEGV, &sa, &old_sa);
+#else
+ void (*ofunc) ();
+
+ ofunc = (void (*)()) signal (SIGSEGV, gdb_demangle_signal_handler);
+#endif
+
+ crash_signal = SIGSETJMP (gdb_demangle_jmp_buf);
+#endif
+
+ if (crash_signal == 0)
+ result = bfd_demangle (NULL, name, options);
+
+#ifdef SIGSEGV
+#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
+ sigaction (SIGSEGV, &old_sa, NULL);
+#else
+ signal (SIGSEGV, ofunc);
+#endif
+#endif
+
+ if (crash_signal != 0)
+ {
+ static int error_reported = 0;
+
+ if (!error_reported)
+ {
+ internal_warning (__FILE__, __LINE__,
+ _("unable to demangle '%s' "
+ "(demangler failed with signal %d)"),
+ name, crash_signal);
+
+ error_reported = 1;
+ }
+
+ result = NULL;
+ }
+
+ return result;
}
/* Don't allow just "maintenance cplus". */