* [PATCH 0/3 v4] Demangler crash handler @ 2014-06-05 13:01 Gary Benson 2014-06-05 13:02 ` [PATCH 1/3 v4] Add new internal problem for demangler warnings Gary Benson ` (3 more replies) 0 siblings, 4 replies; 15+ messages in thread From: Gary Benson @ 2014-06-05 13:01 UTC (permalink / raw) To: gdb-patches Cc: Andrew Burgess, Doug Evans, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey Hi all, This patch is an updated version of the demangler crash handler I posted yesterday. The main changes from the previous version are: 1) All #ifdef SIGSEGV conditionals have been removed. 2) If the user set "ulimit -c 0" then no core file will be created and a warning will be printed. This mirrors what internal_error and internal_warning currently do. 3) A separate signal stack is created for the SIGSEGV handler to allow it to function correctly if the normal process stack overflows. This signal stack is currently only used by the SIGSEGV handler--all other signal handlers use the normal process stack as before. Doug requested that I change the patch to emit warnings for every demangler crash, not just the first. I've not done this, my reason being that subsequent failures could have been caused by the first, by memory corruption or some leftover state: they could be bogus, and could lead to us chasing bugs that don't exist. I prefer this way, but I'm not hung up on it and if it's a blocker for Doug or anyone else I will add the extra warnings. I've split the patches as follows: 1/3 - adds a new category of internal problem for demangler warnings. This patch is unchanged from the previous version (PATCH 1/2 v3): https://sourceware.org/ml/gdb-patches/2014-06/msg00142.html 2/3 - refactors and exposes the core-dumping functions in utils.c. This is a completely new patch. 3/3 - the crash catcher itself. This patch differs from the previous version by the removal of the #ifdef SIGSEGV conditionals, the addition of a check to see if a core dump should be performed, and the creation of a separate stack to allow the signal handler to function when the normal stack is exhausted. I would push all three 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. maint set demangler-warning quit (yes|no|ask) maint show demangler-warning quit Control whether GDB should exit if it catches a crash in the symbol name demangler. * 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? Thanks, Gary -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3 v4] Add new internal problem for demangler warnings 2014-06-05 13:01 [PATCH 0/3 v4] Demangler crash handler Gary Benson @ 2014-06-05 13:02 ` Gary Benson 2014-06-05 13:03 ` [PATCH 2/3 v4] Refactor and expose core-dumping functionality Gary Benson ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ messages in thread From: Gary Benson @ 2014-06-05 13:02 UTC (permalink / raw) To: gdb-patches Cc: Andrew Burgess, Doug Evans, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey This patch adds a new category of internal problem for demangler warnings. Demangler warnings behave in much the same way as internal warnings except that they do not create core files and no option to change this is presented to the user. gdb/ 2014-06-05 Gary Benson <gbenson@redhat.com> * utils.h (demangler_vwarning): New declaration. (demangler_warning): Likewise. * utils.c (struct internal_problem) <user_settable_should_quit>: New field. <user_settable_should_dump_core>: Likewise (internal_error_problem): Add values for above new fields. (internal_warning_problem): Likewise. (demangler_warning_problem): New static global. (demangler_vwarning): New function. (demangler_warning): Likewise. (add_internal_problem_command): Selectively add commands. (_initialize_utils): New internal problem command. * maint.c (maintenance_demangler_warning): New function. (_initialize_maint_cmds): New command. gdb/doc/ 2014-06-05 Gary Benson <gbenson@redhat.com> * gdb.texinfo (Maintenance Commands): Document new "maint demangler-warning" command and new "maint set/show demangler-warning" option. diff --git a/gdb/utils.h b/gdb/utils.h index 33371ac..31f9c19 100644 --- a/gdb/utils.h +++ b/gdb/utils.h @@ -313,6 +313,14 @@ extern void internal_warning (const char *file, int line, extern void warning (const char *, ...) ATTRIBUTE_PRINTF (1, 2); extern void vwarning (const char *, va_list args) ATTRIBUTE_PRINTF (1, 0); + +extern void demangler_vwarning (const char *file, int line, + const char *, va_list ap) + ATTRIBUTE_PRINTF (3, 0); + +extern void demangler_warning (const char *file, int line, + const char *, ...) ATTRIBUTE_PRINTF (3, 4); + \f /* Misc. utilities. */ diff --git a/gdb/utils.c b/gdb/utils.c index 86df1c7..686b153 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -659,7 +659,9 @@ static const char *const internal_problem_modes[] = struct internal_problem { const char *name; + int user_settable_should_quit; const char *should_quit; + int user_settable_should_dump_core; const char *should_dump_core; }; @@ -794,7 +796,7 @@ internal_vproblem (struct internal_problem *problem, } static struct internal_problem internal_error_problem = { - "internal-error", internal_problem_ask, internal_problem_ask + "internal-error", 1, internal_problem_ask, 1, internal_problem_ask }; void @@ -815,7 +817,7 @@ internal_error (const char *file, int line, const char *string, ...) } static struct internal_problem internal_warning_problem = { - "internal-warning", internal_problem_ask, internal_problem_ask + "internal-warning", 1, internal_problem_ask, 1, internal_problem_ask }; void @@ -834,6 +836,26 @@ internal_warning (const char *file, int line, const char *string, ...) va_end (ap); } +static struct internal_problem demangler_warning_problem = { + "demangler-warning", 1, internal_problem_ask, 0, internal_problem_no +}; + +void +demangler_vwarning (const char *file, int line, const char *fmt, va_list ap) +{ + internal_vproblem (&demangler_warning_problem, file, line, fmt, ap); +} + +void +demangler_warning (const char *file, int line, const char *string, ...) +{ + va_list ap; + + va_start (ap, string); + demangler_vwarning (file, line, string, ap); + va_end (ap); +} + /* Dummy functions to keep add_prefix_cmd happy. */ static void @@ -894,45 +916,51 @@ add_internal_problem_command (struct internal_problem *problem) (char *) NULL), 0/*allow-unknown*/, &maintenance_show_cmdlist); - set_doc = xstrprintf (_("Set whether GDB should quit " - "when an %s is detected"), - problem->name); - show_doc = xstrprintf (_("Show whether GDB will quit " - "when an %s is detected"), - problem->name); - add_setshow_enum_cmd ("quit", class_maintenance, - internal_problem_modes, - &problem->should_quit, - set_doc, - show_doc, - NULL, /* help_doc */ - NULL, /* setfunc */ - NULL, /* showfunc */ - set_cmd_list, - show_cmd_list); - - xfree (set_doc); - xfree (show_doc); - - set_doc = xstrprintf (_("Set whether GDB should create a core " - "file of GDB when %s is detected"), - problem->name); - show_doc = xstrprintf (_("Show whether GDB will create a core " - "file of GDB when %s is detected"), - problem->name); - add_setshow_enum_cmd ("corefile", class_maintenance, - internal_problem_modes, - &problem->should_dump_core, - set_doc, - show_doc, - NULL, /* help_doc */ - NULL, /* setfunc */ - NULL, /* showfunc */ - set_cmd_list, - show_cmd_list); + if (problem->user_settable_should_quit) + { + set_doc = xstrprintf (_("Set whether GDB should quit " + "when an %s is detected"), + problem->name); + show_doc = xstrprintf (_("Show whether GDB will quit " + "when an %s is detected"), + problem->name); + add_setshow_enum_cmd ("quit", class_maintenance, + internal_problem_modes, + &problem->should_quit, + set_doc, + show_doc, + NULL, /* help_doc */ + NULL, /* setfunc */ + NULL, /* showfunc */ + set_cmd_list, + show_cmd_list); + + xfree (set_doc); + xfree (show_doc); + } - xfree (set_doc); - xfree (show_doc); + if (problem->user_settable_should_dump_core) + { + set_doc = xstrprintf (_("Set whether GDB should create a core " + "file of GDB when %s is detected"), + problem->name); + show_doc = xstrprintf (_("Show whether GDB will create a core " + "file of GDB when %s is detected"), + problem->name); + add_setshow_enum_cmd ("corefile", class_maintenance, + internal_problem_modes, + &problem->should_dump_core, + set_doc, + show_doc, + NULL, /* help_doc */ + NULL, /* setfunc */ + NULL, /* showfunc */ + set_cmd_list, + show_cmd_list); + + xfree (set_doc); + xfree (show_doc); + } } /* Return a newly allocated string, containing the PREFIX followed @@ -3523,4 +3551,5 @@ _initialize_utils (void) { add_internal_problem_command (&internal_error_problem); add_internal_problem_command (&internal_warning_problem); + add_internal_problem_command (&demangler_warning_problem); } diff --git a/gdb/maint.c b/gdb/maint.c index 873c33c..3f83891 100644 --- a/gdb/maint.c +++ b/gdb/maint.c @@ -131,6 +131,15 @@ maintenance_internal_warning (char *args, int from_tty) internal_warning (__FILE__, __LINE__, "%s", (args == NULL ? "" : args)); } +/* Stimulate the internal error mechanism that GDB uses when an + demangler problem is detected. Allows testing of the mechanism. */ + +static void +maintenance_demangler_warning (char *args, int from_tty) +{ + demangler_warning (__FILE__, __LINE__, "%s", (args == NULL ? "" : args)); +} + /* Someday we should allow demangling for things other than just explicit strings. For example, we might want to be able to specify the address of a string in either GDB's process space or the @@ -1052,6 +1061,12 @@ Give GDB an internal warning.\n\ Cause GDB to behave as if an internal warning was reported."), &maintenancelist); + add_cmd ("demangler-warning", class_maintenance, + maintenance_demangler_warning, _("\ +Give GDB a demangler warning.\n\ +Cause GDB to behave as if a demangler warning was reported."), + &maintenancelist); + add_cmd ("demangle", class_maintenance, maintenance_demangle, _("\ Demangle a C++/ObjC mangled name.\n\ Call internal GDB demangler routine to demangle a C++ link name\n\ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 9f7fa18..6104f55 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -33267,13 +33267,18 @@ with the @code{SIGQUIT} signal. @kindex maint internal-error @kindex maint internal-warning +@kindex maint demangler-warning +@cindex demangler crashes @item maint internal-error @r{[}@var{message-text}@r{]} @itemx maint internal-warning @r{[}@var{message-text}@r{]} -Cause @value{GDBN} to call the internal function @code{internal_error} -or @code{internal_warning} and hence behave as though an internal error -or internal warning has been detected. In addition to reporting the -internal problem, these functions give the user the opportunity to -either quit @value{GDBN} or create a core file of the current +@itemx maint demangler-warning @r{[}@var{message-text}@r{]} + +Cause @value{GDBN} to call the internal function @code{internal_error}, +@code{internal_warning} or @code{demangler_warning} and hence behave +as though an internal problam has been detected. In addition to +reporting the internal problem, these functions give the user the +opportunity to either quit @value{GDBN} or (for @code{internal_error} +and @code{internal_warning}) create a core file of the current @value{GDBN} session. These commands take an optional parameter @var{message-text} that is @@ -33293,15 +33298,20 @@ Create a core file? (y or n) @kbd{n} @cindex @value{GDBN} internal error @cindex internal errors, control of @value{GDBN} behavior +@cindex demangler crashes @kindex maint set internal-error @kindex maint show internal-error @kindex maint set internal-warning @kindex maint show internal-warning +@kindex maint set demangler-warning +@kindex maint show demangler-warning @item maint set internal-error @var{action} [ask|yes|no] @itemx maint show internal-error @var{action} @itemx maint set internal-warning @var{action} [ask|yes|no] @itemx maint show internal-warning @var{action} +@itemx maint set demangler-warning @var{action} [ask|yes|no] +@itemx maint show demangler-warning @var{action} When @value{GDBN} reports an internal problem (error or warning) it gives the user the opportunity to both quit @value{GDBN} and create a core file of the current @value{GDBN} session. These commands let you @@ -33315,7 +33325,10 @@ quit. The default is to ask the user what to do. @item corefile You can specify that @value{GDBN} should always (yes) or never (no) -create a core file. The default is to ask the user what to do. +create a core file. The default is to ask the user what to do. Note +that there is no @code{corefile} option for @code{demangler-warning}: +demangler warnings always create a core file and this cannot be +disabled. @end table @kindex maint packet ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/3 v4] Refactor and expose core-dumping functionality 2014-06-05 13:01 [PATCH 0/3 v4] Demangler crash handler Gary Benson 2014-06-05 13:02 ` [PATCH 1/3 v4] Add new internal problem for demangler warnings Gary Benson @ 2014-06-05 13:03 ` Gary Benson 2014-06-05 16:28 ` Doug Evans 2014-06-05 13:04 ` [PATCH 3/3 v4] Demangler crash handler Gary Benson 2014-06-05 16:19 ` [PATCH 0/3 " Doug Evans 3 siblings, 1 reply; 15+ messages in thread From: Gary Benson @ 2014-06-05 13:03 UTC (permalink / raw) To: gdb-patches Cc: Andrew Burgess, Doug Evans, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey This patch exposes the functions that dump core outside utils.c. The function can_dump_core has been split into two new functions, check_can_dump_core and warn_cant_dump_core so that the check and the printed warning can be separated. A new function check_can_dump_core_warn replaces the original can_dump_core. gdb/ 2014-06-05 Gary Benson <gbenson@redhat.com> * utils.h (check_can_dump_core): New declaration. (warn_cant_dump_core): Likewise. (dump_core): Likewise. * utils.c (dump_core): Made nonstatic. (can_dump_core): Removed function. (check_can_dump_core): New function. (warn_cant_dump_core): Likewise. (check_can_dump_core_warn): Likewise. (internal_vproblem): Replace calls to can_dump_core with calls to check_can_dump_core_warn. diff --git a/gdb/utils.h b/gdb/utils.h index 0ba7879..a6115c1 100644 --- a/gdb/utils.h +++ b/gdb/utils.h @@ -374,4 +374,18 @@ extern ULONGEST align_down (ULONGEST v, int n); extern LONGEST gdb_sign_extend (LONGEST value, int bit); +/* Check whether GDB will be able to dump core using the dump_core + function. */ + +extern int check_can_dump_core (void); + +/* Print a warning that we cannot dump core. */ + +extern void warn_cant_dump_core (const char *reason); + +/* Dump core trying to increase the core soft limit to hard limit + first. */ + +extern void dump_core (void); + #endif /* UTILS_H */ diff --git a/gdb/utils.c b/gdb/utils.c index a72f3bd..0f25436 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -600,7 +600,7 @@ error_stream (struct ui_file *stream) /* Dump core trying to increase the core soft limit to hard limit first. */ -static void +void dump_core (void) { #ifdef HAVE_SETRLIMIT @@ -615,8 +615,8 @@ dump_core (void) /* Check whether GDB will be able to dump core using the dump_core function. */ -static int -can_dump_core (const char *reason) +int +check_can_dump_core (void) { #ifdef HAVE_GETRLIMIT struct rlimit rlim; @@ -626,18 +626,37 @@ can_dump_core (const char *reason) return 1; if (rlim.rlim_max == 0) - { - fprintf_unfiltered (gdb_stderr, - _("%s\nUnable to dump core, use `ulimit -c" - " unlimited' before executing GDB next time.\n"), - reason); - return 0; - } + return 0; #endif /* HAVE_GETRLIMIT */ return 1; } +/* Print a warning that we cannot dump core. */ + +void +warn_cant_dump_core (const char *reason) +{ + fprintf_unfiltered (gdb_stderr, + _("%s\nUnable to dump core, use `ulimit -c" + " unlimited' before executing GDB next time.\n"), + reason); +} + +/* Check whether GDB will be able to dump core using the dump_core + function, and print a warning if we cannot. */ + +static int +check_can_dump_core_warn (const char *reason) +{ + int can_dump_core = check_can_dump_core (); + + if (!can_dump_core) + warn_cant_dump_core (reason); + + return can_dump_core; +} + /* Allow the user to configure the debugger behavior with respect to what to do when an internal problem is detected. */ @@ -756,7 +775,7 @@ internal_vproblem (struct internal_problem *problem, if (problem->should_dump_core == internal_problem_ask) { - if (!can_dump_core (reason)) + if (!check_can_dump_core_warn (reason)) dump_core_p = 0; else { @@ -767,7 +786,7 @@ internal_vproblem (struct internal_problem *problem, } } else if (problem->should_dump_core == internal_problem_yes) - dump_core_p = can_dump_core (reason); + dump_core_p = check_can_dump_core_warn (reason); else if (problem->should_dump_core == internal_problem_no) dump_core_p = 0; else ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3 v4] Refactor and expose core-dumping functionality 2014-06-05 13:03 ` [PATCH 2/3 v4] Refactor and expose core-dumping functionality Gary Benson @ 2014-06-05 16:28 ` Doug Evans 2014-06-06 9:09 ` Gary Benson 0 siblings, 1 reply; 15+ messages in thread From: Doug Evans @ 2014-06-05 16:28 UTC (permalink / raw) To: Gary Benson Cc: gdb-patches, Andrew Burgess, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey On Thu, Jun 5, 2014 at 6:03 AM, Gary Benson <gbenson@redhat.com> wrote: > This patch exposes the functions that dump core outside utils.c. > The function can_dump_core has been split into two new functions, > check_can_dump_core and warn_cant_dump_core so that the check and > the printed warning can be separated. A new function > check_can_dump_core_warn replaces the original can_dump_core. > > > gdb/ > 2014-06-05 Gary Benson <gbenson@redhat.com> > > * utils.h (check_can_dump_core): New declaration. > (warn_cant_dump_core): Likewise. > (dump_core): Likewise. > * utils.c (dump_core): Made nonstatic. > (can_dump_core): Removed function. > (check_can_dump_core): New function. > (warn_cant_dump_core): Likewise. > (check_can_dump_core_warn): Likewise. > (internal_vproblem): Replace calls to can_dump_core with > calls to check_can_dump_core_warn. check_can_dump_core feels a bit clumsy over the original can_dump_core. can_dump_core (or can_dump_core_p) reads better to me. [And now it does what it says it does, without the side-effect of the printf.] Not sure if I'd rename check_can_dump_core_warn (or delete it). ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3 v4] Refactor and expose core-dumping functionality 2014-06-05 16:28 ` Doug Evans @ 2014-06-06 9:09 ` Gary Benson 0 siblings, 0 replies; 15+ messages in thread From: Gary Benson @ 2014-06-06 9:09 UTC (permalink / raw) To: Doug Evans Cc: gdb-patches, Andrew Burgess, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey Doug Evans wrote: > On Thu, Jun 5, 2014 at 6:03 AM, Gary Benson <gbenson@redhat.com> wrote: > > This patch exposes the functions that dump core outside utils.c. > > The function can_dump_core has been split into two new functions, > > check_can_dump_core and warn_cant_dump_core so that the check and > > the printed warning can be separated. A new function > > check_can_dump_core_warn replaces the original can_dump_core. > > check_can_dump_core feels a bit clumsy over the original > can_dump_core. can_dump_core (or can_dump_core_p) reads better to > me. [And now it does what it says it does, without the side-effect > of the printf.] > > Not sure if I'd rename check_can_dump_core_warn (or delete it). Ok, I renamed check_can_dump_core back to the original can_dump_core, and I renamed check_can_dump_core_warn as can_dump_core_warn (it's called twice, so there would be duplication if I removed it). I won't post another patch series for such a small change but the updated patches are in my github (http://tinyurl.com/dmcc-v4-2 and http://tinyurl.com/dmcc-v4-3) if you would like to take a look. Patch 2 is the important one, patch 3 only has a couple of lines changed. Thanks, Gary -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3 v4] Demangler crash handler 2014-06-05 13:01 [PATCH 0/3 v4] Demangler crash handler Gary Benson 2014-06-05 13:02 ` [PATCH 1/3 v4] Add new internal problem for demangler warnings Gary Benson 2014-06-05 13:03 ` [PATCH 2/3 v4] Refactor and expose core-dumping functionality Gary Benson @ 2014-06-05 13:04 ` Gary Benson 2014-06-06 18:16 ` Florian Weimer 2014-06-06 21:12 ` Andrew Burgess 2014-06-05 16:19 ` [PATCH 0/3 " Doug Evans 3 siblings, 2 replies; 15+ messages in thread From: Gary Benson @ 2014-06-05 13:04 UTC (permalink / raw) To: gdb-patches Cc: Andrew Burgess, Doug Evans, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey 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. gdb/ 2014-06-05 Gary Benson <gbenson@redhat.com> * configure.ac [AC_CHECK_FUNCS] <sigaltstack>: New check. * configure: Regenerate. * config.in: Likewise. * main.c (signal.h): New include. (setup_alternate_signal_stack): New function. (captured_main): Call the above. * 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_attempt_core_dump): Likewise. (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. gdb/doc/ 2014-06-05 Gary Benson <gbenson@redhat.com> * gdb.texinfo (Maintenance Commands): Document new "maint set/show catch-demangler-crashes" option. diff --git a/gdb/configure.ac b/gdb/configure.ac index 903f378..f41ba2f 100644 --- a/gdb/configure.ac +++ b/gdb/configure.ac @@ -1328,7 +1328,7 @@ AC_CHECK_FUNCS([canonicalize_file_name realpath getrusage getuid getgid \ sigaction sigprocmask sigsetmask socketpair \ ttrace wborder wresize setlocale iconvlist libiconvlist btowc \ setrlimit getrlimit posix_madvise waitpid lstat \ - ptrace64]) + ptrace64 sigaltstack]) AM_LANGINFO_CODESET GDB_AC_COMMON diff --git a/gdb/configure b/gdb/configure index 56c92d3..e23a58f 100755 --- a/gdb/configure +++ b/gdb/configure @@ -10507,7 +10507,7 @@ for ac_func in canonicalize_file_name realpath getrusage getuid getgid \ sigaction sigprocmask sigsetmask socketpair \ ttrace wborder wresize setlocale iconvlist libiconvlist btowc \ setrlimit getrlimit posix_madvise waitpid lstat \ - ptrace64 + ptrace64 sigaltstack do : as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" diff --git a/gdb/config.in b/gdb/config.in index cd4ce92..8585b49 100644 --- a/gdb/config.in +++ b/gdb/config.in @@ -366,6 +366,9 @@ /* Define to 1 if you have the `sigaction' function. */ #undef HAVE_SIGACTION +/* Define to 1 if you have the `sigaltstack' function. */ +#undef HAVE_SIGALTSTACK + /* Define to 1 if you have the <signal.h> header file. */ #undef HAVE_SIGNAL_H diff --git a/gdb/main.c b/gdb/main.c index a9fc378..7031cc3 100644 --- a/gdb/main.c +++ b/gdb/main.c @@ -45,6 +45,7 @@ #include "filenames.h" #include "filestuff.h" +#include <signal.h> /* The selected interpreter. This will be used as a set command variable, so it should always be malloc'ed - since @@ -288,6 +289,27 @@ get_init_files (const char **system_gdbinit, *local_gdbinit = localinit; } +/* Try to set up an alternate signal stack for SIGSEGV handlers. + This allows us to handle SIGSEGV signals generated when the + normal process stack is exhausted. If this stack is not set + up (sigaltstack is unavailable or fails) and a SIGSEGV is + generated when the normal stack is exhausted then the program + will behave as though no SIGSEGV handler was installed. */ + +static void +setup_alternate_signal_stack (void) +{ +#ifdef HAVE_SIGALTSTACK + stack_t ss; + + ss.ss_sp = xmalloc (SIGSTKSZ); + ss.ss_size = SIGSTKSZ; + ss.ss_flags = 0; + + sigaltstack(&ss, NULL); +#endif +} + /* Call command_loop. If it happens to return, pass that through as a non-zero return status. */ @@ -785,6 +807,9 @@ captured_main (void *data) quiet = 1; } + /* Try to set up an alternate signal stack for SIGSEGV handlers. */ + setup_alternate_signal_stack (); + /* Initialize all files. Give the interpreter a chance to take control of the console via the deprecated_init_ui_hook (). */ gdb_init (gdb_program_name); diff --git a/gdb/cp-support.c b/gdb/cp-support.c index 91533e8..f4dde70 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -36,6 +36,7 @@ #include "value.h" #include "cp-abi.h" #include "language.h" +#include <signal.h> #include "safe-ctype.h" @@ -1505,12 +1506,140 @@ cp_lookup_rtti_type (const char *name, struct block *block) return rtti_type; } +#ifdef HAVE_WORKING_FORK + +/* If nonzero, attempt to catch crashes in the demangler and print + useful debugging information. */ + +static int catch_demangler_crashes = 1; + +/* Wrap 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; + +/* If nonzero, attempt to dump core from the signal handler. */ + +static int gdb_demangle_attempt_core_dump = 1; + +/* Signal handler for gdb_demangle. */ + +static void +gdb_demangle_signal_handler (int signo) +{ + if (gdb_demangle_attempt_core_dump) + { + if (fork () == 0) + dump_core (); + + gdb_demangle_attempt_core_dump = 0; + } + + 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 HAVE_WORKING_FORK +#if defined (HAVE_SIGACTION) && defined (SA_RESTART) + struct sigaction sa, old_sa; +#else + void (*ofunc) (); +#endif + static int can_dump_core = -1; + + if (can_dump_core == -1) + { + can_dump_core = check_can_dump_core (); + + if (!can_dump_core) + gdb_demangle_attempt_core_dump = 0; + } + + if (catch_demangler_crashes) + { +#if defined (HAVE_SIGACTION) && defined (SA_RESTART) + sa.sa_handler = gdb_demangle_signal_handler; + sigemptyset (&sa.sa_mask); + sa.sa_flags = SA_ONSTACK; + sigaction (SIGSEGV, &sa, &old_sa); +#else + 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 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) + { + char *dmw_msg; + struct cleanup *back_to; + + dmw_msg = xstrprintf (_("unable to demangle '%s' " + "(demangler failed with signal %d)"), + name, crash_signal); + back_to = make_cleanup (xfree, dmw_msg); + + if (!can_dump_core) + { + char *cdc_msg = xstrprintf ("%s:%d: %s: %s", + __FILE__, __LINE__, + "demangler-warning", + dmw_msg); + + make_cleanup (xfree, cdc_msg); + warn_cant_dump_core (cdc_msg); + } + + demangler_warning (__FILE__, __LINE__, "%s", dmw_msg); + + do_cleanups (back_to); + + error_reported = 1; + } + + result = NULL; + } + } +#endif + + return result; } /* Don't allow just "maintenance cplus". */ @@ -1585,4 +1714,17 @@ _initialize_cp_support (void) Usage: info vtbl EXPRESSION\n\ Evaluate EXPRESSION and display the virtual function table for the\n\ resulting object.")); + +#ifdef HAVE_WORKING_FORK + add_setshow_boolean_cmd ("catch-demangler-crashes", class_maintenance, + &catch_demangler_crashes, _("\ +Set whether to attempt to catch demangler crashes."), _("\ +Show whether to attempt to catch demangler crashes."), _("\ +If enabled GDB will attempt to catch demangler crashes and\n\ +display the offending symbol."), + NULL, + NULL, + &maintenance_set_cmdlist, + &maintenance_show_cmdlist); +#endif } diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 9f7fa18..242117b 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -33235,6 +33235,17 @@ Expand symbol tables. If @var{regexp} is specified, only expand symbol tables for file names matching @var{regexp}. +@kindex maint set catch-demangler-crashes +@kindex maint show catch-demangler-crashes +@cindex demangler crashes +@item maint set catch-demangler-crashes [on|off] +@itemx maint show catch-demangler-crashes +Control whether @value{GDBN} 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. + @kindex maint cplus first_component @item maint cplus first_component @var{name} Print the first C@t{++} class/namespace component of @var{name}. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3 v4] Demangler crash handler 2014-06-05 13:04 ` [PATCH 3/3 v4] Demangler crash handler Gary Benson @ 2014-06-06 18:16 ` Florian Weimer 2014-06-06 19:27 ` Gary Benson 2014-06-06 21:12 ` Andrew Burgess 1 sibling, 1 reply; 15+ messages in thread From: Florian Weimer @ 2014-06-06 18:16 UTC (permalink / raw) To: Gary Benson Cc: gdb-patches, Andrew Burgess, Doug Evans, Eli Zaretskii, Mark Kettenis, Pedro Alves, Tom Tromey * Gary Benson: > + sigaction (SIGSEGV, &sa, &old_sa); > + crash_signal = SIGSETJMP (gdb_demangle_jmp_buf); > + sigaction (SIGSEGV, &old_sa, NULL); That's quite a bit of additional work for each demangler invocation. Is this visible with things like tab completion? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3 v4] Demangler crash handler 2014-06-06 18:16 ` Florian Weimer @ 2014-06-06 19:27 ` Gary Benson 2014-06-06 19:42 ` Florian Weimer 0 siblings, 1 reply; 15+ messages in thread From: Gary Benson @ 2014-06-06 19:27 UTC (permalink / raw) To: Florian Weimer Cc: gdb-patches, Andrew Burgess, Doug Evans, Eli Zaretskii, Mark Kettenis, Pedro Alves, Tom Tromey Florian Weimer wrote: > * Gary Benson: > > + sigaction (SIGSEGV, &sa, &old_sa); > > + crash_signal = SIGSETJMP (gdb_demangle_jmp_buf); > > + sigaction (SIGSEGV, &old_sa, NULL); > > That's quite a bit of additional work for each demangler invocation. > Is this visible with things like tab completion? I tested this a while back: it's not noticable. FWIW I did time gdb -nx -batch \ /usr/lib64/libreoffice/program/soffice.bin \ -ex "start" -ex "complete b" > /dev/null That invokes the demangler some 3,100,000 times. I don't remember the exact times but they were of the order of 50s in both cases. Thanks, Gary -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3 v4] Demangler crash handler 2014-06-06 19:27 ` Gary Benson @ 2014-06-06 19:42 ` Florian Weimer 0 siblings, 0 replies; 15+ messages in thread From: Florian Weimer @ 2014-06-06 19:42 UTC (permalink / raw) To: Gary Benson Cc: gdb-patches, Andrew Burgess, Doug Evans, Eli Zaretskii, Mark Kettenis, Pedro Alves, Tom Tromey * Gary Benson: > Florian Weimer wrote: >> * Gary Benson: >> > + sigaction (SIGSEGV, &sa, &old_sa); >> > + crash_signal = SIGSETJMP (gdb_demangle_jmp_buf); >> > + sigaction (SIGSEGV, &old_sa, NULL); >> >> That's quite a bit of additional work for each demangler invocation. >> Is this visible with things like tab completion? > > I tested this a while back: it's not noticable. > > FWIW I did time gdb -nx -batch \ > /usr/lib64/libreoffice/program/soffice.bin \ > -ex "start" -ex "complete b" > /dev/null > > That invokes the demangler some 3,100,000 times. I don't remember > the exact times but they were of the order of 50s in both cases. Good to know, thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3 v4] Demangler crash handler 2014-06-05 13:04 ` [PATCH 3/3 v4] Demangler crash handler Gary Benson 2014-06-06 18:16 ` Florian Weimer @ 2014-06-06 21:12 ` Andrew Burgess 2014-06-09 9:01 ` Gary Benson 1 sibling, 1 reply; 15+ messages in thread From: Andrew Burgess @ 2014-06-06 21:12 UTC (permalink / raw) To: Gary Benson, gdb-patches Cc: Doug Evans, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey On 05/06/2014 2:03 PM, Gary Benson wrote: > diff --git a/gdb/cp-support.c b/gdb/cp-support.c > index 91533e8..f4dde70 100644 > --- a/gdb/cp-support.c > +++ b/gdb/cp-support.c > + > +/* Signal handler for gdb_demangle. */ > + > +static void > +gdb_demangle_signal_handler (int signo) > +{ > + if (gdb_demangle_attempt_core_dump) > + { > + if (fork () == 0) > + dump_core (); This worries me a little, when a problem case occurs gdb will dump core regardless of the users ulimit setting, without first asking the user, and doesn't tell the user that a core file was created. This feels quite unexpected behaviour to me, especially the bit about disregarding the ulimit setting without first asking for permission. Catching the crash feels like a good idea, but I'd prefer that gdb ask before circumventing the ulimit and dumping core. Alternatively we could just not dump core from gdb, report the bad symbol and let the user file a bug. With the demangler being so deterministic it should be possible to reproduce, if not, then we just ask the user to turn off the crash catch, adjust their ulimit (like we would with any other gdb SEGV crash), and rerun the test. If we really want to create the core file by default, but aren't going to ask, then I'd propose we honour the ulimit setting, and make sure that the user is told that a core file was just written. Thanks, Andrew ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3 v4] Demangler crash handler 2014-06-06 21:12 ` Andrew Burgess @ 2014-06-09 9:01 ` Gary Benson 2014-06-09 10:26 ` Andrew Burgess 0 siblings, 1 reply; 15+ messages in thread From: Gary Benson @ 2014-06-09 9:01 UTC (permalink / raw) To: Andrew Burgess Cc: gdb-patches, Doug Evans, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey Andrew Burgess wrote: > On 05/06/2014 2:03 PM, Gary Benson wrote: > > diff --git a/gdb/cp-support.c b/gdb/cp-support.c > > index 91533e8..f4dde70 100644 > > --- a/gdb/cp-support.c > > +++ b/gdb/cp-support.c > > > + > > +/* Signal handler for gdb_demangle. */ > > + > > +static void > > +gdb_demangle_signal_handler (int signo) > > +{ > > + if (gdb_demangle_attempt_core_dump) > > + { > > + if (fork () == 0) > > + dump_core (); > > This worries me a little, when a problem case occurs gdb will dump > core regardless of the users ulimit setting, without first asking > the user, and doesn't tell the user that a core file was created. > > This feels quite unexpected behaviour to me, especially the bit > about disregarding the ulimit setting without first asking for > permission. > > Catching the crash feels like a good idea, but I'd prefer that gdb > ask before circumventing the ulimit and dumping core. This part of the same patch: + if (core_dump_allowed == -1) + { + core_dump_allowed = can_dump_core (); + + if (!core_dump_allowed) + gdb_demangle_attempt_core_dump = 0; + } calls this: int can_dump_core (void) { #ifdef HAVE_GETRLIMIT struct rlimit rlim; /* Be quiet and assume we can dump if an error is returned. */ if (getrlimit (RLIMIT_CORE, &rlim) != 0) return 1; if (rlim.rlim_max == 0) return 0; #endif /* HAVE_GETRLIMIT */ return 1; } which inhibits the core dump if the user's ulimit is 0. > Alternatively we could just not dump core from gdb, report the bad > symbol and let the user file a bug. With the demangler being so > deterministic it should be possible to reproduce, if not, then we > just ask the user to turn off the crash catch, adjust their ulimit > (like we would with any other gdb SEGV crash), and rerun the test. That was and is my preferred solution, but Mark Kettenis indicated that he would not accept the patch unless a meaningful core file was created. > If we really want to create the core file by default, but aren't > going to ask, then I'd propose we honour the ulimit setting, and > make sure that the user is told that a core file was just written. The problem with asking is that you'd have to ask within the signal handler, and no code that prints to the screen is safe to call from within a signal handler. Even indicating that a core file was written is probably impossible: you just have to abort and hope for the best. The nearest I could do is set a flag in the signal handler and have the code it returns to print "Attempting to dump core" or some such thing. Thanks, Gary -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3 v4] Demangler crash handler 2014-06-09 9:01 ` Gary Benson @ 2014-06-09 10:26 ` Andrew Burgess 2014-06-09 11:48 ` Gary Benson 0 siblings, 1 reply; 15+ messages in thread From: Andrew Burgess @ 2014-06-09 10:26 UTC (permalink / raw) To: Gary Benson Cc: gdb-patches, Doug Evans, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey On 09/06/2014 10:01 AM, Gary Benson wrote: > Andrew Burgess wrote: >> On 05/06/2014 2:03 PM, Gary Benson wrote: >>> diff --git a/gdb/cp-support.c b/gdb/cp-support.c >>> index 91533e8..f4dde70 100644 >>> --- a/gdb/cp-support.c >>> +++ b/gdb/cp-support.c >> >>> + >>> +/* Signal handler for gdb_demangle. */ >>> + >>> +static void >>> +gdb_demangle_signal_handler (int signo) >>> +{ >>> + if (gdb_demangle_attempt_core_dump) >>> + { >>> + if (fork () == 0) >>> + dump_core (); >> >> This worries me a little, when a problem case occurs gdb will dump >> core regardless of the users ulimit setting, without first asking >> the user, and doesn't tell the user that a core file was created. >> >> This feels quite unexpected behaviour to me, especially the bit >> about disregarding the ulimit setting without first asking for >> permission. >> >> Catching the crash feels like a good idea, but I'd prefer that gdb >> ask before circumventing the ulimit and dumping core. > > This part of the same patch: > > + if (core_dump_allowed == -1) > + { > + core_dump_allowed = can_dump_core (); > + > + if (!core_dump_allowed) > + gdb_demangle_attempt_core_dump = 0; > + } > > calls this: > > int > can_dump_core (void) > { > #ifdef HAVE_GETRLIMIT > struct rlimit rlim; > > /* Be quiet and assume we can dump if an error is returned. */ > if (getrlimit (RLIMIT_CORE, &rlim) != 0) > return 1; > > if (rlim.rlim_max == 0) > return 0; > #endif /* HAVE_GETRLIMIT */ > > return 1; > } > > which inhibits the core dump if the user's ulimit is 0. Ahh, yes I see. So the problem here is this function is geared towards the /old/ use of the function where we are about to ask the user if we should dump core. For that, this function was correct, we check the hard limit of the resource. If the hard limit is high then we ask the user, and dump core. However, in doing so we circumvent the soft limit rlim.rlim_cur. So I think my point still stands. The user has said "no core files please", and we create one without asking. If we must go down this road then I think we need two functions to check the two different limits. >> Alternatively we could just not dump core from gdb, report the bad >> symbol and let the user file a bug. With the demangler being so >> deterministic it should be possible to reproduce, if not, then we >> just ask the user to turn off the crash catch, adjust their ulimit >> (like we would with any other gdb SEGV crash), and rerun the test. > > That was and is my preferred solution, but Mark Kettenis indicated > that he would not accept the patch unless a meaningful core file was > created. I don't understand that position, but I'd hope he'd agree that we should respect the user ulimit over creating a core file... > >> If we really want to create the core file by default, but aren't >> going to ask, then I'd propose we honour the ulimit setting, and >> make sure that the user is told that a core file was just written. > > The problem with asking is that you'd have to ask within the signal > handler, and no code that prints to the screen is safe to call from > within a signal handler. Indeed. I did wonder about some horrible synchronisation scheme where the "master" gdb process queries the user then signals the fork()ed child to indicate if it should dump core or not .... but it felt like huge overkill. > Even indicating that a core file was written is probably impossible: > you just have to abort and hope for the best. The nearest I could > do is set a flag in the signal handler and have the code it returns > to print "Attempting to dump core" or some such thing. I think an "attempting ..." style message would be enough, the gdb_demangle_attempt_core_dump flag could be used to indicate if we've tried to dump core or not. Thanks, Andrew ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3 v4] Demangler crash handler 2014-06-09 10:26 ` Andrew Burgess @ 2014-06-09 11:48 ` Gary Benson 0 siblings, 0 replies; 15+ messages in thread From: Gary Benson @ 2014-06-09 11:48 UTC (permalink / raw) To: Andrew Burgess Cc: gdb-patches, Doug Evans, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey Andrew Burgess wrote: > On 09/06/2014 10:01 AM, Gary Benson wrote: > > Andrew Burgess wrote: > > > On 05/06/2014 2:03 PM, Gary Benson wrote: > > > > diff --git a/gdb/cp-support.c b/gdb/cp-support.c > > > > index 91533e8..f4dde70 100644 > > > > --- a/gdb/cp-support.c > > > > +++ b/gdb/cp-support.c > > > > > > > + > > > > +/* Signal handler for gdb_demangle. */ > > > > + > > > > +static void > > > > +gdb_demangle_signal_handler (int signo) > > > > +{ > > > > + if (gdb_demangle_attempt_core_dump) > > > > + { > > > > + if (fork () == 0) > > > > + dump_core (); > > > > > > This worries me a little, when a problem case occurs gdb will > > > dump core regardless of the users ulimit setting, without first > > > asking the user, and doesn't tell the user that a core file was > > > created. > > > > > > This feels quite unexpected behaviour to me, especially the bit > > > about disregarding the ulimit setting without first asking for > > > permission. > > > > > > Catching the crash feels like a good idea, but I'd prefer that > > > gdb ask before circumventing the ulimit and dumping core. > > > > This part of the same patch: > > > > + if (core_dump_allowed == -1) > > + { > > + core_dump_allowed = can_dump_core (); > > + > > + if (!core_dump_allowed) > > + gdb_demangle_attempt_core_dump = 0; > > + } > > > > calls this: > > > > int > > can_dump_core (void) > > { > > #ifdef HAVE_GETRLIMIT > > struct rlimit rlim; > > > > /* Be quiet and assume we can dump if an error is returned. */ > > if (getrlimit (RLIMIT_CORE, &rlim) != 0) > > return 1; > > > > if (rlim.rlim_max == 0) > > return 0; > > #endif /* HAVE_GETRLIMIT */ > > > > return 1; > > } > > > > which inhibits the core dump if the user's ulimit is 0. > > Ahh, yes I see. > > So the problem here is this function is geared towards the /old/ use > of the function where we are about to ask the user if we should dump > core. For that, this function was correct, we check the hard limit > of the resource. If the hard limit is high then we ask the user, > and dump core. > > However, in doing so we circumvent the soft limit rlim.rlim_cur. So > I think my point still stands. The user has said "no core files > please", and we create one without asking. If we must go down this > road then I think we need two functions to check the two different > limits. Ah, I didn't realize the code in dump_core was to override the user's soft limit. I will update the patch. > > > Alternatively we could just not dump core from gdb, report the > > > bad symbol and let the user file a bug. With the demangler > > > being so deterministic it should be possible to reproduce, if > > > not, then we just ask the user to turn off the crash catch, > > > adjust their ulimit (like we would with any other gdb SEGV > > > crash), and rerun the test. > > > > That was and is my preferred solution, but Mark Kettenis indicated > > that he would not accept the patch unless a meaningful core file > > was created. > > I don't understand that position, but I'd hope he'd agree that we > should respect the user ulimit over creating a core file... Yes, this seems reasonable. > > > If we really want to create the core file by default, but aren't > > > going to ask, then I'd propose we honour the ulimit setting, and > > > make sure that the user is told that a core file was just written. > > > > The problem with asking is that you'd have to ask within the signal > > handler, and no code that prints to the screen is safe to call from > > within a signal handler. > > Indeed. I did wonder about some horrible synchronisation scheme > where the "master" gdb process queries the user then signals the > fork()ed child to indicate if it should dump core or not .... but > it felt like huge overkill. Yeah, I thought down this road too :) > > Even indicating that a core file was written is probably > > impossible: you just have to abort and hope for the best. > > The nearest I could do is set a flag in the signal handler > > and have the code it returns to print "Attempting to dump > > core" or some such thing. > > I think an "attempting ..." style message would be enough, the > gdb_demangle_attempt_core_dump flag could be used to indicate > if we've tried to dump core or not. I will add this to the updated patch. Thanks, Gary -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3 v4] Demangler crash handler 2014-06-05 13:01 [PATCH 0/3 v4] Demangler crash handler Gary Benson ` (2 preceding siblings ...) 2014-06-05 13:04 ` [PATCH 3/3 v4] Demangler crash handler Gary Benson @ 2014-06-05 16:19 ` Doug Evans 2014-06-06 9:19 ` Gary Benson 3 siblings, 1 reply; 15+ messages in thread From: Doug Evans @ 2014-06-05 16:19 UTC (permalink / raw) To: Gary Benson Cc: gdb-patches, Andrew Burgess, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey On Thu, Jun 5, 2014 at 6:01 AM, Gary Benson <gbenson@redhat.com> wrote: > Hi all, > > This patch is an updated version of the demangler crash handler I > posted yesterday. The main changes from the previous version are: > > 1) All #ifdef SIGSEGV conditionals have been removed. > > 2) If the user set "ulimit -c 0" then no core file will be > created and a warning will be printed. This mirrors what > internal_error and internal_warning currently do. > > 3) A separate signal stack is created for the SIGSEGV handler > to allow it to function correctly if the normal process > stack overflows. This signal stack is currently only used > by the SIGSEGV handler--all other signal handlers use the > normal process stack as before. > > Doug requested that I change the patch to emit warnings for every > demangler crash, not just the first. Umm, that's not what I said, to be precise, and it was more of a suggestion for discussion than a request. > I've not done this, my reason > being that subsequent failures could have been caused by the first, > by memory corruption or some leftover state: they could be bogus, > and could lead to us chasing bugs that don't exist. I prefer this > way, but I'm not hung up on it and if it's a blocker for Doug or > anyone else I will add the extra warnings. It's not a blocker to me. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3 v4] Demangler crash handler 2014-06-05 16:19 ` [PATCH 0/3 " Doug Evans @ 2014-06-06 9:19 ` Gary Benson 0 siblings, 0 replies; 15+ messages in thread From: Gary Benson @ 2014-06-06 9:19 UTC (permalink / raw) To: Doug Evans Cc: gdb-patches, Andrew Burgess, Eli Zaretskii, Florian Weimer, Mark Kettenis, Pedro Alves, Tom Tromey Doug Evans wrote: > On Thu, Jun 5, 2014 at 6:01 AM, Gary Benson <gbenson@redhat.com> wrote: > > Doug requested that I change the patch to emit warnings for every > > demangler crash, not just the first. > > Umm, that's not what I said, to be precise, and it was more of a > suggestion for discussion than a request. Ah, I misunderstood. > > > I've not done this, my reason > > being that subsequent failures could have been caused by the first, > > by memory corruption or some leftover state: they could be bogus, > > and could lead to us chasing bugs that don't exist. I prefer this > > way, but I'm not hung up on it and if it's a blocker for Doug or > > anyone else I will add the extra warnings. > > It's not a blocker to me. Great :) Thanks, Gary -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-06-09 11:48 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-05 13:01 [PATCH 0/3 v4] Demangler crash handler Gary Benson 2014-06-05 13:02 ` [PATCH 1/3 v4] Add new internal problem for demangler warnings Gary Benson 2014-06-05 13:03 ` [PATCH 2/3 v4] Refactor and expose core-dumping functionality Gary Benson 2014-06-05 16:28 ` Doug Evans 2014-06-06 9:09 ` Gary Benson 2014-06-05 13:04 ` [PATCH 3/3 v4] Demangler crash handler Gary Benson 2014-06-06 18:16 ` Florian Weimer 2014-06-06 19:27 ` Gary Benson 2014-06-06 19:42 ` Florian Weimer 2014-06-06 21:12 ` Andrew Burgess 2014-06-09 9:01 ` Gary Benson 2014-06-09 10:26 ` Andrew Burgess 2014-06-09 11:48 ` Gary Benson 2014-06-05 16:19 ` [PATCH 0/3 " Doug Evans 2014-06-06 9:19 ` Gary Benson
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).