RE: catch SIGSEGV in the demangler

Pierre Muller
2013-01-16T22:16:57Z

Hi Tom,

before I forget to reply:

sigaction is not available everywhere and is tested by configure
see remote-sim.c:999

I don't know the exact status of sigaltstack function, but
I would be surprised that it is supported on systems that
don't support sigaction...

Also, sigaction can return the previous signal handler,
so why not use this previous handle instead of SIG_DFL?

Would it make sense to still throw an exception even if not inside the
demangler?

I was also surprised by the fact that you called the new file
safe-demangle.c, why not simply name it gdb-demangle.c?

But this are of course only minor details...
which don't mean that I don't like the overall idea of the patch.

Pierre Muller

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Tom Tromey
> Envoyé : lundi 14 janvier 2013 21:16
> À : gdb-patches@sourceware.org
> Objet : RFC: catch SIGSEGV in the demangler
>
> More than once I have seen gdb crashes caused by bugs in the demangler.
> There is one open right now. These crashes are particularly nasty
> because they occur when reading debuginfo, or even when reading minsyms
> -- and so prevent the user from debugging their program at all.
>
> This patch arranges for gdb to catch SIGSEGV in the demangler and
> recover from it. It prints a warning and arranges for the demangling to
> fail. This lets debugging continue.
>
> Example:
>
> (gdb) maint demangle
>
_ZSt7forwardIRN1x14refobjiteratorINS0_3refINS0_4mime30multipart_section_proc
>
essorObjIZ15get_body_parserIZZN14mime_processor21make_section_iteratorERKNS2
>
_INS3_10sectionObjENS0_10ptrrefBaseEEEbENKUlvE_clEvEUlSB_bE_ZZNS6_21make_sec
>
tion_iteratorESB_bENKSC_clEvEUlSB_E0_ENS1_INS2_INS0_20outputrefiteratorObjIi
>
EES8_EEEERKSsSB_OT_OT0_EUlmE_NS3_32make_multipart_default_discarderISP_EEEES
> 8_EEEEEOT_RNSt16remove_referenceISW_E4typeE
> warning: Attempting to demangle the symbol
>
'_ZSt7forwardIRN1x14refobjiteratorINS0_3refINS0_4mime30multipart_section_pro
>
cessorObjIZ15get_body_parserIZZN14mime_processor21make_section_iteratorERKNS
>
2_INS3_10sectionObjENS0_10ptrrefBaseEEEbENKUlvE_clEvEUlSB_bE_ZZNS6_21make_se
>
ction_iteratorESB_bENKSC_clEvEUlSB_E0_ENS1_INS2_INS0_20outputrefiteratorObjI
>
iEES8_EEEERKSsSB_OT_OT0_EUlmE_NS3_32make_multipart_default_discarderISP_EEEE
> S8_EEEEEOT_RNSt16remove_referenceISW_E4typeE' caused a crash in the
> demangler. Please report this to the GDB maintainers.
>
>
> Built and regtested on x86-64 Fedora 16.
>
> I'm curious to know what you think.
>
> Tom
>
> * Makefile.in (SFILES): Add safe-demangle.c.
> (COMMON_OBS): Add safe-demangle.o.
> * safe-demangle.c: New file.
> * exceptions.h (enum errors) <SEGV_ERROR>: New constant.
> * gdb-demangle.h: Include demangle.h.
> (gdb_cplus_demangle): Declare.
> * c-lang.c: Use gdb-demangle.h, gdb_cplus_demangle.
> * c-typeprint.c: Use gdb-demangle.h, gdb_cplus_demangle.
> * cp-name-parser.y: Use gdb-demangle.h, gdb_cplus_demangle.
> * cp-support.c: Use gdb-demangle.h, gdb_cplus_demangle.
> * dwarf2read.c: Use gdb-demangle.h, gdb_cplus_demangle.
> * gdbtypes.c: Use gdb-demangle.h, gdb_cplus_demangle.
> * gnu-v2-abi.c: Use gdb-demangle.h, gdb_cplus_demangle.
> * gnu-v3-abi.c: Use gdb-demangle.h, gdb_cplus_demangle.
> * jv-lang.c: Use gdb-demangle.h, gdb_cplus_demangle.
> * jv-typeprint.c: Use gdb-demangle.h, gdb_cplus_demangle.
> * language.c: Use gdb-demangle.h, gdb_cplus_demangle.
> * symtab.c: Use gdb-demangle.h, gdb_cplus_demangle.
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index b065d41..4397965 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -733,7 +733,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-
> valprint.c ada-tasks.c \
> proc-service.list progspace.c \
> prologue-value.c psymtab.c \
> regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c
> reverse.c \
> - sentinel-frame.c \
> + safe-demangle.c sentinel-frame.c \
> serial.c ser-base.c ser-unix.c skip.c \
> solib.c solib-target.c source.c \
> stabsread.c stack.c probe.c stap-probe.c std-regs.c \
> @@ -899,7 +899,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
> go-lang.o go-valprint.o go-typeprint.o \
> jv-lang.o jv-valprint.o jv-typeprint.o \
> m2-lang.o opencl-lang.o p-lang.o p-typeprint.o p-valprint.o \
> - sentinel-frame.o \
> + safe-demangle.o sentinel-frame.o \
> complaints.o typeprint.o \
> ada-typeprint.o c-typeprint.o f-typeprint.o m2-typeprint.o \
> ada-valprint.o c-valprint.o cp-valprint.o d-valprint.o f-valprint.o
\
> diff --git a/gdb/c-lang.c b/gdb/c-lang.c
> index c9b0f51..79768b1 100644
> --- a/gdb/c-lang.c
> +++ b/gdb/c-lang.c
> @@ -29,7 +29,7 @@
> #include "gdb_assert.h"
> #include "charset.h"
> #include "gdb_string.h"
> -#include "demangle.h"
> +#include "gdb-demangle.h"
> #include "cp-abi.h"
> #include "cp-support.h"
> #include "gdb_obstack.h"
> @@ -972,7 +972,7 @@ const struct language_defn cplus_language_defn =
> "this", /* name_of_this */
> cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> cp_lookup_transparent_type, /* lookup_transparent_type */
> - cplus_demangle, /* Language specific symbol demangler */
> + gdb_cplus_demangle, /* Language specific symbol
demangler */
> cp_class_name_from_physname, /* Language specific
> class_name_from_physname */
> c_op_print_tab, /* expression operators for printing */
> diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
> index cb75a4e..ac36fa3 100644
> --- a/gdb/c-typeprint.c
> +++ b/gdb/c-typeprint.c
> @@ -26,7 +26,7 @@
> #include "gdbcore.h"
> #include "target.h"
> #include "language.h"
> -#include "demangle.h"
> +#include "gdb-demangle.h"
> #include "c-lang.h"
> #include "typeprint.h"
> #include "cp-abi.h"
> @@ -1219,8 +1219,8 @@ c_type_print_base (struct type *type, struct ui_file
> *stream,
> mangled_name = TYPE_FN_FIELD_PHYSNAME (f, j);
>
> demangled_name =
> - cplus_demangle (mangled_name,
> - DMGL_ANSI | DMGL_PARAMS);
> + gdb_cplus_demangle (mangled_name,
> + DMGL_ANSI | DMGL_PARAMS);
> if (demangled_name == NULL)
> {
> /* In some cases (for instance with the HP
> diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
> index 01a1fa4..1305f3d 100644
> --- a/gdb/cp-name-parser.y
> +++ b/gdb/cp-name-parser.y
> @@ -2201,7 +2201,7 @@ main (int argc, char **argv)
> buf[strlen (buf) - 1] = 0;
> /* Use DMGL_VERBOSE to get expanded standard substitutions. */
> c = trim_chars (buf, &extra_chars);
> - str2 = cplus_demangle (buf, DMGL_PARAMS | DMGL_ANSI | DMGL_VERBOSE);
> + str2 = gdb_cplus_demangle (buf, DMGL_PARAMS | DMGL_ANSI |
> DMGL_VERBOSE);
> if (str2 == NULL)
> {
> printf ("Demangling error\n");
> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index 22e8fc4..5a47ddc 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -21,7 +21,7 @@
> #include "defs.h"
> #include "cp-support.h"
> #include "gdb_string.h"
> -#include "demangle.h"
> +#include "gdb-demangle.h"
> #include "gdb_assert.h"
> #include "gdbcmd.h"
> #include "dictionary.h"
> @@ -635,7 +635,7 @@ mangled_name_to_comp (const char *mangled_name, int
> options,
>
> /* If it doesn't, or if that failed, then try to demangle the
> name. */
> - demangled_name = cplus_demangle (mangled_name, options);
> + demangled_name = gdb_cplus_demangle (mangled_name, options);
> if (demangled_name == NULL)
> return NULL;
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 7af89c6..fe5d2a1 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -7643,7 +7643,7 @@ dwarf2_physname (char *name, struct die_info *die,
> struct dwarf2_cu *cu)
> }
> else
> {
> - demangled = cplus_demangle (mangled,
> + demangled = gdb_cplus_demangle (mangled,
> (DMGL_PARAMS | DMGL_ANSI
> | (cu->language == language_java
> ? DMGL_JAVA | DMGL_RET_POSTFIX
> @@ -13945,7 +13945,7 @@ fixup_partial_die (struct partial_die_info
> *part_die,
> {
> char *demangled;
>
> - demangled = cplus_demangle (part_die->linkage_name, DMGL_TYPES);
> + demangled = gdb_cplus_demangle (part_die->linkage_name,
DMGL_TYPES);
> if (demangled)
> {
> const char *base;
> @@ -16995,7 +16995,7 @@ dwarf2_name (struct die_info *die, struct
dwarf2_cu
> *cu)
> /* Avoid demangling DW_STRING (attr) the second time on a second
> call for the same DIE. */
> if (!DW_STRING_IS_CANONICAL (attr))
> - demangled = cplus_demangle (DW_STRING (attr), DMGL_TYPES);
> + demangled = gdb_cplus_demangle (DW_STRING (attr), DMGL_TYPES);
>
> if (demangled)
> {
> diff --git a/gdb/exceptions.h b/gdb/exceptions.h
> index 0d67719..c4a424f 100644
> --- a/gdb/exceptions.h
> +++ b/gdb/exceptions.h
> @@ -86,6 +86,10 @@ enum errors {
> /* DW_OP_GNU_entry_value resolving failed. */
> NO_ENTRY_VALUE_ERROR,
>
> + /* SIGSEGV. These are only generated under very special
> + circumstances. */
> + SEGV_ERROR,
> +
> /* Add more errors here. */
> NR_ERRORS
> };
> diff --git a/gdb/gdb-demangle.h b/gdb/gdb-demangle.h
> index 2d62956..ec0677e 100644
> --- a/gdb/gdb-demangle.h
> +++ b/gdb/gdb-demangle.h
> @@ -19,6 +19,8 @@
> #ifndef GDB_DEMANGLE_H
> #define GDB_DEMANGLE_H
>
> +#include "demangle.h"
> +
> /* Nonzero means that encoded C++/ObjC names should be printed out in
their
> C++/ObjC form rather than raw. */
> extern int demangle;
> @@ -34,4 +36,9 @@ extern void set_demangling_style (char *);
> /* Check if a character is one of the commonly used C++ marker
characters.
> */
> extern int is_cplus_marker (int);
>
> +/* gdb's demangler wrapper. This is like gdb_cplus_demangle, but it has
> + special SEGV handling. */
> +
> +extern char *gdb_cplus_demangle (const char *mangled, int options);
> +
> #endif /* GDB_DEMANGLE_H */
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 85ffbf1..3c5b4dd 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -30,7 +30,7 @@
> #include "language.h"
> #include "target.h"
> #include "value.h"
> -#include "demangle.h"
> +#include "gdb-demangle.h"
> #include "complaints.h"
> #include "gdbcmd.h"
> #include "cp-abi.h"
> @@ -1753,8 +1753,8 @@ check_stub_method (struct type *type, int method_id,
> int signature_id)
> struct gdbarch *gdbarch = get_type_arch (type);
> struct fn_field *f;
> char *mangled_name = gdb_mangle_name (type, method_id, signature_id);
> - char *demangled_name = cplus_demangle (mangled_name,
> - DMGL_PARAMS | DMGL_ANSI);
> + char *demangled_name = gdb_cplus_demangle (mangled_name,
> + DMGL_PARAMS | DMGL_ANSI);
> char *argtypetext, *p;
> int depth = 0, argcount = 1;
> struct field *argtypes;
> diff --git a/gdb/gnu-v2-abi.c b/gdb/gnu-v2-abi.c
> index 4a600d3..2b3f21c 100644
> --- a/gdb/gnu-v2-abi.c
> +++ b/gdb/gnu-v2-abi.c
> @@ -251,7 +251,7 @@ gnuv2_value_rtti_type (struct value *v, int *full, int
> *top, int *using_enc)
> return NULL;
>
> /* If we just skip the prefix, we get screwed by namespaces. */
> - demangled_name=cplus_demangle(linkage_name,DMGL_PARAMS|DMGL_ANSI);
> + demangled_name=gdb_cplus_demangle(linkage_name,DMGL_PARAMS|DMGL_ANSI);
> p = strchr (demangled_name, ' ');
> if (p)
> *p = '\0';
> diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
> index aef5ae0..948fd93 100644
> --- a/gdb/gnu-v3-abi.c
> +++ b/gdb/gnu-v3-abi.c
> @@ -22,7 +22,7 @@
> #include "value.h"
> #include "cp-abi.h"
> #include "cp-support.h"
> -#include "demangle.h"
> +#include "gdb-demangle.h"
> #include "objfiles.h"
> #include "valprint.h"
> #include "c-lang.h"
> @@ -593,8 +593,8 @@ gnuv3_print_method_ptr (const gdb_byte *contents,
> possible paths to the method based on the adjustment. */
> if (physname)
> {
> - char *demangled_name = cplus_demangle (physname,
> - DMGL_ANSI | DMGL_PARAMS);
> + char *demangled_name = gdb_cplus_demangle (physname,
> + DMGL_ANSI |
DMGL_PARAMS);
>
> fprintf_filtered (stream, "&virtual ");
> if (demangled_name == NULL)
> diff --git a/gdb/jv-lang.c b/gdb/jv-lang.c
> index f8e8718..f0064da 100644
> --- a/gdb/jv-lang.c
> +++ b/gdb/jv-lang.c
> @@ -33,7 +33,7 @@
> #include "jv-lang.h"
> #include "gdbcore.h"
> #include "block.h"
> -#include "demangle.h"
> +#include "gdb-demangle.h"
> #include "dictionary.h"
> #include <ctype.h>
> #include "gdb_assert.h"
> @@ -1014,7 +1014,7 @@ nosideret:
>
> static char *java_demangle (const char *mangled, int options)
> {
> - return cplus_demangle (mangled, options | DMGL_JAVA);
> + return gdb_cplus_demangle (mangled, options | DMGL_JAVA);
> }
>
> /* Find the member function name of the demangled name NAME. NAME
> diff --git a/gdb/jv-typeprint.c b/gdb/jv-typeprint.c
> index 69ebf5a..48da871 100644
> --- a/gdb/jv-typeprint.c
> +++ b/gdb/jv-typeprint.c
> @@ -286,7 +286,7 @@ java_type_print_base (struct type *type, struct
ui_file
> *stream, int show,
> mangled_name = physname;
>
> demangled_name =
> - cplus_demangle (mangled_name,
> + gdb_cplus_demangle (mangled_name,
> DMGL_ANSI | DMGL_PARAMS | DMGL_JAVA);
>
> if (demangled_name == NULL)
> diff --git a/gdb/language.c b/gdb/language.c
> index c873f2e..cf95d55 100644
> --- a/gdb/language.c
> +++ b/gdb/language.c
> @@ -41,7 +41,7 @@
> #include "target.h"
> #include "parser-defs.h"
> #include "jv-lang.h"
> -#include "demangle.h"
> +#include "gdb-demangle.h"
> #include "symfile.h"
>
> extern void _initialize_language (void);
> @@ -764,7 +764,7 @@ static CORE_ADDR unk_lang_trampoline (struct
frame_info
> *frame, CORE_ADDR pc)
> /* Unknown languages just use the cplus demangler. */
> static char *unk_lang_demangle (const char *mangled, int options)
> {
> - return cplus_demangle (mangled, options);
> + return gdb_cplus_demangle (mangled, options);
> }
>
> static char *unk_lang_class_name (const char *mangled)
> diff --git a/gdb/safe-demangle.c b/gdb/safe-demangle.c
> new file mode 100644
> index 0000000..6003298
> --- /dev/null
> +++ b/gdb/safe-demangle.c
> @@ -0,0 +1,103 @@
> +/* Safe interface to the demangler.
> + Copyright (c) 2013 Free Software Foundation, Inc.
> +
> + This file is part of GDB.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
> +
> +#include "defs.h"
> +#include "gdb-demangle.h"
> +#include "exceptions.h"
> +#include <signal.h>
> +
> +/* The stack where SEGV should be delivered. */
> +
> +static stack_t segv_stack;
> +
> +/* If non-zero, we are in the demangler, and SIGSEGV should be turned
> + into an exception. */
> +
> +static int in_demangler;
> +
> +/* The SIGSEGV handler. */
> +
> +static void
> +handle_segv (int sig)
> +{
> + struct gdb_exception except;
> +
> + if (!in_demangler)
> + {
> + signal (sig, SIG_DFL);
> + raise (sig);
> + }
> +
> + except.reason = RETURN_ERROR;
> + except.error = SEGV_ERROR;
> + except.message = NULL;
> + throw_exception (except);
> +}
> +
> +

> +
> +/* A safe wrapper for cplus_demangle. It catches SEGV and prints a
> + warning. */
> +
> +char *
> +gdb_cplus_demangle (const char *mangled, int options)
> +{
> + volatile struct gdb_exception except;
> + char *result;
> +
> + TRY_CATCH (except, RETURN_MASK_ERROR)
> + {
> + int save = in_demangler;
> +
> + in_demangler = 1;
> + result = cplus_demangle (mangled, options);
> + in_demangler = save;
> + }
> +
> + if (except.reason < 0 && except.error == SEGV_ERROR)
> + {
> + warning (_("Attempting to demangle the symbol '%s' caused a "
> + "crash in the demangler. Please report this to "
> + "the GDB maintainers."),
> + mangled);
> + result = NULL;
> + }
> +
> + return result;
> +}
> +
> +

> +
> +initialize_file_ftype _initialize_safe_demangle;
> +
> +void
> +_initialize_safe_demangle (void)
> +{
> + struct sigaction segv_action;
> +
> + segv_stack.ss_size = SIGSTKSZ;
> + segv_stack.ss_sp = xmalloc (segv_stack.ss_size);
> + segv_stack.ss_flags = SS_ONSTACK;
> + sigaltstack (&segv_stack, NULL);
> +
> + segv_action.sa_handler = handle_segv;
> + sigemptyset (&segv_action.sa_mask);
> + segv_action.sa_flags = SA_ONSTACK;
> +
> + sigaction (SIGSEGV, &segv_action, NULL);
> +}
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index f4ed8b9..490fb00 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -30,7 +30,7 @@
> #include "gdb_regex.h"
> #include "expression.h"
> #include "language.h"
> -#include "demangle.h"
> +#include "gdb-demangle.h"
> #include "inferior.h"
> #include "source.h"
> #include "filenames.h" /* for FILENAME_CMP */
> @@ -586,7 +586,7 @@ symbol_find_demangled_name (struct general_symbol_info
> *gsymbol,
> || gsymbol->language == language_auto)
> {
> demangled =
> - cplus_demangle (mangled, DMGL_PARAMS | DMGL_ANSI);
> + gdb_cplus_demangle (mangled, DMGL_PARAMS | DMGL_ANSI);
> if (demangled != NULL)
> {
> gsymbol->language = language_cplus;
> @@ -596,7 +596,7 @@ symbol_find_demangled_name (struct general_symbol_info
> *gsymbol,
> if (gsymbol->language == language_java)
> {
> demangled =
> - cplus_demangle (mangled,
> + gdb_cplus_demangle (mangled,
> DMGL_PARAMS | DMGL_ANSI | DMGL_JAVA);
> if (demangled != NULL)
> {
> @@ -1144,7 +1144,7 @@ demangle_for_lookup (const char *name, enum language
> lang,
> lookup, so we can always binary search. */
> if (lang == language_cplus)
> {
> - demangled_name = cplus_demangle (name, DMGL_ANSI | DMGL_PARAMS);
> + demangled_name = gdb_cplus_demangle (name, DMGL_ANSI |
DMGL_PARAMS);
> if (demangled_name)
> {
> modified_name = demangled_name;
> @@ -1164,7 +1164,7 @@ demangle_for_lookup (const char *name, enum language
> lang,
> }
> else if (lang == language_java)
> {
> - demangled_name = cplus_demangle (name,
> + demangled_name = gdb_cplus_demangle (name,
> DMGL_ANSI | DMGL_PARAMS | DMGL_JAVA);
> if (demangled_name)
> {