[PATCH 0/3 v5] Demangler crash handler

[PATCH 0/3 v5] Demangler crash handler

[Gary Benson]

Hi all,

This series is an updated version of the demangler crash handler
I posted last week.  The main changes are:

 1) check_can_dump_core has been renamed back to the can_dump_core,
    and I renamed check_can_dump_core_warn as can_dump_core_warn as
    per https://sourceware.org/ml/gdb-patches/2014-06/msg00307.html.

 2) can_dump_core and can_dump_core_warn have a new parameter,
    limit_kind, to specify which limit to check.  The original
    calls in internal_vproblem use the hard limit, and the
    calls from the crash catcher that this series adds use the
    soft limit.

 3) The crash catcher prints "Attempting to dump core" to alert
    the user that a core file might have been created.

The patches are split as before:

 1/3 - adds a new category of internal problem for demangler
       warnings.  This patch is unchanged from the previous
       two versions (PATCH 1/2 v3 and v4).
       https://sourceware.org/ml/gdb-patches/2014-06/msg00142.html
       https://sourceware.org/ml/gdb-patches/2014-06/msg00252.html

 2/3 - refactors and exposes the core-dumping functions in utils.c.
       This patch differs from the previous version as described in
       points 1 and 2 in the above list.

 3/3 - the crash catcher itself.  This patch differs from the
       previous version as described in points 2 and 3 in the
       above list.

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/

[PATCH 1/3 v5] Add new internal problem for demangler warnings

[Gary Benson]

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-09  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-09  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
@@ -3529,4 +3557,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
@@ -33268,13 +33268,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
@@ -33294,15 +33299,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
@@ -33316,7 +33326,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

[PATCH 2/3 v5] Refactor and expose core-dumping functionality

[Gary Benson]

This patch exposes the functions that dump core outside utils.c.
can_dump_core gains a new parameter, "limit_kind", to allow either
the soft or hard limit to be checked, and its printing has separated
into the new function warn_cant_dump_core.  The new function
can_dump_core_warn does what can_dump_core previously did (print and
warn).

gdb/
2014-06-09  Gary Benson  <gbenson@redhat.com>

	* utils.h (resource_limit_kind): New enum.
	(can_dump_core): New declaration.
	(warn_cant_dump_core): Likewise.
	(dump_core): Likewise.
	* utils.c (dump_core): Made nonstatic.  Added new
	parameter "limit_kind".
	(can_dump_core): Made nonstatic. Moved printing code to...
	(warn_cant_dump_core): New function.
	(can_dump_core_warn): Likewise.
	(internal_vproblem): Replace calls to can_dump_core with
	calls to can_dump_core_warn.  Supply new argument to each.

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,28 @@ extern ULONGEST align_down (ULONGEST v, int n);
 
 extern LONGEST gdb_sign_extend (LONGEST value, int bit);
 
+/* Resource limits used by getrlimit and setrlimit.  */
+
+enum resource_limit_kind
+  {
+    LIMIT_CUR,
+    LIMIT_MAX
+  };
+
+/* Check whether GDB will be able to dump core using the dump_core
+   function.  Returns zero if GDB cannot or should not dump core.
+   If LIMIT_KIND is LIMIT_CUR the user's soft limit will be respected.
+   If LIMIT_KIND is LIMIT_MAX only the hard limit will be respected.  */
+
+extern int can_dump_core (enum resource_limit_kind limit_kind);
+
+/* 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
@@ -613,10 +613,12 @@ dump_core (void)
 }
 
 /* Check whether GDB will be able to dump core using the dump_core
-   function.  */
+   function.  Returns zero if GDB cannot or should not dump core.
+   If LIMIT_KIND is LIMIT_CUR the user's soft limit will be respected.
+   If LIMIT_KIND is LIMIT_MAX only the hard limit will be respected.  */
 
-static int
-can_dump_core (const char *reason)
+int
+can_dump_core (enum resource_limit_kind limit_kind)
 {
 #ifdef HAVE_GETRLIMIT
   struct rlimit rlim;
@@ -625,19 +627,47 @@ can_dump_core (const char *reason)
   if (getrlimit (RLIMIT_CORE, &rlim) != 0)
     return 1;
 
-  if (rlim.rlim_max == 0)
+  switch (limit_kind)
     {
-      fprintf_unfiltered (gdb_stderr,
-			  _("%s\nUnable to dump core, use `ulimit -c"
-			    " unlimited' before executing GDB next time.\n"),
-			  reason);
-      return 0;
+    case LIMIT_CUR:
+      if (rlim.rlim_cur == 0)
+	return 0;
+
+    case LIMIT_MAX:
+      if (rlim.rlim_max == 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
+can_dump_core_warn (enum resource_limit_kind limit_kind,
+		    const char *reason)
+{
+  int core_dump_allowed = can_dump_core (limit_kind);
+
+  if (!core_dump_allowed)
+    warn_cant_dump_core (reason);
+
+  return core_dump_allowed;
+}
+
 /* Allow the user to configure the debugger behavior with respect to
    what to do when an internal problem is detected.  */
 
@@ -756,7 +786,7 @@ internal_vproblem (struct internal_problem *problem,
 
   if (problem->should_dump_core == internal_problem_ask)
     {
-      if (!can_dump_core (reason))
+      if (!can_dump_core_warn (LIMIT_MAX, reason))
 	dump_core_p = 0;
       else
 	{
@@ -767,7 +797,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 = can_dump_core_warn (LIMIT_MAX, reason);
   else if (problem->should_dump_core == internal_problem_no)
     dump_core_p = 0;
   else

[PATCH 3/3 v5] Demangler crash handler

[Gary Benson]

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-09  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-09  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
@@ -35,6 +35,7 @@
 #include "expression.h"
 #include "value.h"
 #include "cp-abi.h"
+#include <signal.h>
 
 #include "safe-ctype.h"
 
@@ -1482,12 +1483,142 @@ 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 core_dump_allowed = -1;
+
+  if (core_dump_allowed == -1)
+    {
+      core_dump_allowed = can_dump_core (LIMIT_CUR);
+
+      if (!core_dump_allowed)
+	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 *short_msg, *long_msg;
+	      struct cleanup *back_to;
+
+	      short_msg = xstrprintf (_("unable to demangle '%s' "
+				      "(demangler failed with signal %d)"),
+				    name, crash_signal);
+	      back_to = make_cleanup (xfree, short_msg);
+
+	      long_msg = xstrprintf ("%s:%d: %s: %s", __FILE__, __LINE__,
+				    "demangler-warning", short_msg);
+	      make_cleanup (xfree, long_msg);
+
+	      target_terminal_ours ();
+	      begin_line ();
+	      if (core_dump_allowed)
+		fprintf_unfiltered (gdb_stderr,
+				    _("%s\nAttempting to dump core.\n"),
+				    long_msg);
+	      else
+		warn_cant_dump_core (long_msg);
+
+	      demangler_warning (__FILE__, __LINE__, "%s", short_msg);
+
+	      do_cleanups (back_to);
+
+	      error_reported = 1;
+	    }
+
+	  result = NULL;
+	}
+    }
+#endif
+
+  return result;
 }
 
 /* Don't allow just "maintenance cplus".  */
@@ -1562,4 +1693,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
@@ -33236,6 +33236,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}.

Re: [PATCH 1/3 v5] Add new internal problem for demangler warnings

[Eli Zaretskii]

> Date: Mon, 9 Jun 2014 16:23:15 +0100
> From: Gary Benson <gbenson@redhat.com>
> Cc: Andrew Burgess <aburgess@broadcom.com>, Doug Evans <xdje42@gmail.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>
> 
> 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.

The documentation changes are still OK.

Thanks.

Re: [PATCH 3/3 v5] Demangler crash handler

[Eli Zaretskii]

> Date: Mon, 9 Jun 2014 16:24:34 +0100
> From: Gary Benson <gbenson@redhat.com>
> Cc: Andrew Burgess <aburgess@broadcom.com>, Doug Evans <xdje42@gmail.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>
> 
> 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.

OK for the documentation part.

Thanks.

Re: [PATCH 3/3 v5] Demangler crash handler

[Gary Benson]

Eli Zaretskii wrote:
> > Date: Mon, 9 Jun 2014 16:24:34 +0100
> > From: Gary Benson <gbenson@redhat.com>
> > Cc: Andrew Burgess <aburgess@broadcom.com>, Doug Evans <xdje42@gmail.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>
> > 
> > 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.
> 
> OK for the documentation part.

Thanks Eli.

Cheers,
Gary

--
http://gbenson.net/

Re: [PATCH 0/3 v5] Demangler crash handler

[Tom Tromey]

>>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:

Gary> This series is an updated version of the demangler crash handler I
Gary> posted last week.

Thanks, Gary.

I re-read the various discussions and my belief is that you've addressed
all the comments.  I believe Mark has proposed a compromise which you
implemented, and I believe Stan's objections were contingent on Mark's.

This patch has been unusually contentious.  So I'd give folks yet
another opportunity to express their disapprovals; let's say another
week.

Disapprovals, I believe, must be accompanied by technical reasons for a
rejection.  Gary has already addressed most such arguments, so if you
are responding, please make sure to read the previous threads.

In the absence of that, this is ok.  Thank you once again.

Tom

Re: [PATCH 0/3 v5] Demangler crash handler

[Mark Kettenis]

> From: Tom Tromey <tromey@redhat.com>
> Date: Wed, 11 Jun 2014 14:41:30 -0600
> 
> >>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:
> 
> Gary> This series is an updated version of the demangler crash handler I
> Gary> posted last week.
> 
> Thanks, Gary.
> 
> I re-read the various discussions and my belief is that you've addressed
> all the comments.  I believe Mark has proposed a compromise which you
> implemented, and I believe Stan's objections were contingent on Mark's.
> 
> This patch has been unusually contentious.  So I'd give folks yet
> another opportunity to express their disapprovals; let's say another
> week.
> 
> Disapprovals, I believe, must be accompanied by technical reasons for a
> rejection.  Gary has already addressed most such arguments, so if you
> are responding, please make sure to read the previous threads.

My position still is that this is duct tape being applied because of
extremely bad engineering practices.  Don't expect me to approve the
compromise.  I still think the whole excercise was a tremendous waste
of time.  In fact I feel a little bit blackmailed because Gary kept on
putting work into this diff.  But I'll shut up now.

[COMMITTED PATCH 0/3 v5] Demangler crash handler

[Gary Benson]

Tom Tromey wrote:
> >>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:
> 
> Gary> This series is an updated version of the demangler crash
> Gary> handler I posted last week.
> 
> Thanks, Gary.
> 
> I re-read the various discussions and my belief is that you've
> addressed all the comments.  I believe Mark has proposed a
> compromise which you implemented, and I believe Stan's objections
> were contingent on Mark's.
> 
> This patch has been unusually contentious.  So I'd give folks yet
> another opportunity to express their disapprovals; let's say another
> week.
> 
> Disapprovals, I believe, must be accompanied by technical reasons
> for a rejection.  Gary has already addressed most such arguments,
> so if you are responding, please make sure to read the previous
> threads.
> 
> In the absence of that, this is ok.  Thank you once again.

Committed and pushed.

Thanks everyone,
Gary

-- 
http://gbenson.net/

Cygwin build failure following [PATCH 3/3 v5] Demangler crash handler

[Pierre Muller]

   Hi Gary,
I think your patch generates
build failure on cygwin32:

../../../binutils-gdb/gdb/cp-support.c: In function 'gdb_demangle':
../../../binutils-gdb/gdb/cp-support.c:1560:21: erreur: 'SA_ONSTACK'
undeclared (first use in this function)
       sa.sa_flags = SA_ONSTACK;
                     ^
../../../binutils-gdb/gdb/cp-support.c:1560:21: note: each undeclared
identifier is reported only once for each function it appears in
Makefile:1075: recipe for target 'cp-support.o' failed

The reason is that SA_ONSTACK
is not defined in cygwin's /usr/include/signal.h header

whereas SA_RESTART is defined in signal header,
and HAVE_SIGACTION is set in config.h


> +#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

A simple patch would probably be to add a separate check
#ifdef SA_ONSTACK
  sa.sa_flags = SA_O?STACK;
#endif

But I honestly don't know enough about
Cygwin signal emulation to know if this is a correct fix or not.

  Maybe Corinna Vinschen can comment on this?

Pierre Muller

Re: Cygwin build failure following [PATCH 3/3 v5] Demangler crash handler

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1742 bytes --]

On Jul 13 18:11, Pierre Muller wrote:
>    Hi Gary,
> I think your patch generates
> build failure on cygwin32:
> 
> ../../../binutils-gdb/gdb/cp-support.c: In function 'gdb_demangle':
> ../../../binutils-gdb/gdb/cp-support.c:1560:21: erreur: 'SA_ONSTACK'
> undeclared (first use in this function)
>        sa.sa_flags = SA_ONSTACK;
>                      ^
> ../../../binutils-gdb/gdb/cp-support.c:1560:21: note: each undeclared
> identifier is reported only once for each function it appears in
> Makefile:1075: recipe for target 'cp-support.o' failed
> 
> The reason is that SA_ONSTACK
> is not defined in cygwin's /usr/include/signal.h header
> 
> whereas SA_RESTART is defined in signal header,
> and HAVE_SIGACTION is set in config.h
> 
> 
> > +#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
> 
> A simple patch would probably be to add a separate check
> #ifdef SA_ONSTACK
>   sa.sa_flags = SA_O?STACK;
> #endif
> 
> But I honestly don't know enough about
> Cygwin signal emulation to know if this is a correct fix or not.
> 
>   Maybe Corinna Vinschen can comment on this?

Right, Cygwin doesn't support SA_ONSTACK yet.  It should be
possible to add, but there are no immediate plans to do so.

As for the above, wouldn't it be better to add something like

  #ifndef SA_ONSTACK
  #define SA_ONSTACK 0
  #endif

to some header?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: Cygwin build failure following [PATCH 3/3 v5] Demangler crash handler

[Gary Benson]

Hi Pierre,

Pierre Muller wrote:
> I think your patch generates build failure on cygwin32:
> 
> ../../../binutils-gdb/gdb/cp-support.c: In function 'gdb_demangle':
> ../../../binutils-gdb/gdb/cp-support.c:1560:21: erreur: 'SA_ONSTACK'
> undeclared (first use in this function)
>        sa.sa_flags = SA_ONSTACK;
>                      ^
[snip]
> 
> The reason is that SA_ONSTACK is not defined in cygwin's
> /usr/include/signal.h header whereas SA_RESTART is defined
> in signal header, and HAVE_SIGACTION is set in config.h
> 
[snip]
> 
> A simple patch would probably be to add a separate check
> #ifdef SA_ONSTACK
>   sa.sa_flags = SA_ONSTACK;
> #endif
> 
> But I honestly don't know enough about Cygwin signal emulation to
> know if this is a correct fix or not.  Maybe Corinna Vinschen can
> comment on this?

I don't know Cygwin, but I had a quick look and indeed, it doesn't
seem to support this functionality.

Could you try the patch inlined below and let me know if it fixes
the build?

Thanks,
Gary

-- 
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index a8ea6fc..6a09b46 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1557,7 +1557,9 @@ gdb_demangle (const char *name, int options)
 #if defined (HAVE_SIGACTION) && defined (SA_RESTART)
       sa.sa_handler = gdb_demangle_signal_handler;
       sigemptyset (&sa.sa_mask);
+#ifdef HAVE_SIGALTSTACK
       sa.sa_flags = SA_ONSTACK;
+#endif
       sigaction (SIGSEGV, &sa, &old_sa);
 #else
       ofunc = (void (*)()) signal (SIGSEGV, gdb_demangle_signal_handler);

Re: Cygwin build failure following [PATCH 3/3 v5] Demangler crash handler

[Andreas Schwab]

Gary Benson <gbenson@redhat.com> writes:

> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index a8ea6fc..6a09b46 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -1557,7 +1557,9 @@ gdb_demangle (const char *name, int options)
>  #if defined (HAVE_SIGACTION) && defined (SA_RESTART)
>        sa.sa_handler = gdb_demangle_signal_handler;
>        sigemptyset (&sa.sa_mask);
> +#ifdef HAVE_SIGALTSTACK
>        sa.sa_flags = SA_ONSTACK;
> +#endif

You probably need to initialize sa.sa_flags in any case.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Cygwin build failure following [PATCH 3/3 v5] Demangler crash handler

[Gary Benson]

Andreas Schwab wrote:
> Gary Benson <gbenson@redhat.com> writes:
> > diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> > index a8ea6fc..6a09b46 100644
> > --- a/gdb/cp-support.c
> > +++ b/gdb/cp-support.c
> > @@ -1557,7 +1557,9 @@ gdb_demangle (const char *name, int options)
> >  #if defined (HAVE_SIGACTION) && defined (SA_RESTART)
> >        sa.sa_handler = gdb_demangle_signal_handler;
> >        sigemptyset (&sa.sa_mask);
> > +#ifdef HAVE_SIGALTSTACK
> >        sa.sa_flags = SA_ONSTACK;
> > +#endif
> 
> You probably need to initialize sa.sa_flags in any case.

Good spot, thanks Andreas.

Pierre, I've inlined an updated patch below.

Thanks,
Gary

-- 
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index a8ea6fc..5464117 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1557,7 +1557,11 @@ gdb_demangle (const char *name, int options)
 #if defined (HAVE_SIGACTION) && defined (SA_RESTART)
       sa.sa_handler = gdb_demangle_signal_handler;
       sigemptyset (&sa.sa_mask);
+#ifdef HAVE_SIGALTSTACK
       sa.sa_flags = SA_ONSTACK;
+#else
+      sa.sa_flags = 0;
+#endif
       sigaction (SIGSEGV, &sa, &old_sa);
 #else
       ofunc = (void (*)()) signal (SIGSEGV, gdb_demangle_signal_handler);

Re: Cygwin build failure following [PATCH 3/3 v5] Demangler crash handler

[Gary Benson]

Corinna Vinschen wrote:
> On Jul 13 18:11, Pierre Muller wrote:
> > A simple patch would probably be to add a separate check
> > #ifdef SA_ONSTACK
> >   sa.sa_flags = SA_O?STACK;
> > #endif
> 
> Right, Cygwin doesn't support SA_ONSTACK yet.  It should be
> possible to add, but there are no immediate plans to do so.
> 
> As for the above, wouldn't it be better to add something like
> 
>   #ifndef SA_ONSTACK
>   #define SA_ONSTACK 0
>   #endif
> 
> to some header?

I'd prefer not to.  Having the conditional where the handler
is installed makes it obvious that the alternate stack is not
available on all systems.

Thanks,
Gary

-- 
http://gbenson.net/

Re: Cygwin build failure following [PATCH 3/3 v5] Demangler crash handler

[Tom Tromey]

>>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:

Gary> Pierre, I've inlined an updated patch below.

This is fine, with a suitable ChangeLog entry, if it fixes the build.

Tom

RE: Cygwin build failure following [PATCH 3/3 v5] Demangler crash handler

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Gary Benson
> Envoyé : lundi 14 juillet 2014 13:43
> À : Andreas Schwab
> Cc : Pierre Muller; gdb-patches@sourceware.org; 'Andrew Burgess'; 'Doug
> Evans'; 'Eli Zaretskii'; 'Florian Weimer'; 'Mark Kettenis'; 'Pedro
> Alves'; 'Tom Tromey'; 'Corinna Vinschen'
> Objet : Re: Cygwin build failure following [PATCH 3/3 v5] Demangler
> crash handler
> 
> Andreas Schwab wrote:
> > Gary Benson <gbenson@redhat.com> writes:
> > > diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> > > index a8ea6fc..6a09b46 100644
> > > --- a/gdb/cp-support.c
> > > +++ b/gdb/cp-support.c
> > > @@ -1557,7 +1557,9 @@ gdb_demangle (const char *name, int options)
> > >  #if defined (HAVE_SIGACTION) && defined (SA_RESTART)
> > >        sa.sa_handler = gdb_demangle_signal_handler;
> > >        sigemptyset (&sa.sa_mask);
> > > +#ifdef HAVE_SIGALTSTACK
> > >        sa.sa_flags = SA_ONSTACK;
> > > +#endif
> >
> > You probably need to initialize sa.sa_flags in any case.
> 
> Good spot, thanks Andreas.
> 
> Pierre, I've inlined an updated patch below.

  I can confirm that the proposed patch
fixes the Cygwin build failure.

  Thanks, Gary,


Pierre Muller