public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/5] Remove exception_none
  2019-04-25 16:53 [PATCH 0/5] More exception-handling improvements Tom Tromey
  2019-04-25 16:53 ` [PATCH 2/5] Make SJLJ exceptions more efficient Tom Tromey
  2019-04-25 16:53 ` [PATCH 3/5] Avoid undefined behavior in Guile exception handling Tom Tromey
@ 2019-04-25 16:53 ` Tom Tromey
  2019-04-25 16:58 ` [PATCH 4/5] Make exception handling more efficient Tom Tromey
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2019-04-25 16:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Now that gdb_exception has a constructor, there's no need for
exception_none.  This patch removes it.

gdb/ChangeLog
2019-04-25  Tom Tromey  <tromey@adacore.com>

	* xml-support.c (gdb_xml_parser::gdb_xml_parser): Update.
	* python/py-value.c (valpy_getitem, valpy_nonzero): Update.
	* python/py-inferior.c (infpy_write_memory, infpy_search_memory):
	Update.
	* python/py-breakpoint.c (bppy_set_condition, bppy_set_commands):
	Update.
	* mi/mi-interp.c (mi_interp::exec): Update.
	* linespec.c (parse_linespec): Update.
	* infcall.c (run_inferior_call): Update.
	* guile/scm-value.c (gdbscm_value_to_lazy_string): Update.
	* guile/scm-symbol.c (gdbscm_lookup_symbol)
	(gdbscm_lookup_global_symbol): Update.
	* guile/scm-param.c (gdbscm_parameter_value): Update.
	* guile/scm-frame.c (gdbscm_frame_read_register)
	(gdbscm_frame_read_var): Update.
	* guile/scm-breakpoint.c (gdbscm_register_breakpoint_x): Update.
	* exec.c (try_open_exec_file): Update.
	* event-top.c (gdb_rl_callback_read_char_wrapper_noexcept)
	(gdb_rl_callback_handler): Update.
	* common/common-exceptions.h (exception_none): Don't declare.
	* common/common-exceptions.c (exception_none): Don't define.
	(struct catcher) <exception>: Update.
	* cli/cli-interp.c (safe_execute_command): Update.
	* breakpoint.c (insert_bp_location, location_to_sals): Update.
---
 gdb/ChangeLog                  | 27 +++++++++++++++++++++++++++
 gdb/breakpoint.c               |  4 ++--
 gdb/cli/cli-interp.c           |  2 +-
 gdb/common/common-exceptions.c |  4 +---
 gdb/common/common-exceptions.h |  3 ---
 gdb/event-top.c                |  4 ++--
 gdb/exec.c                     |  2 +-
 gdb/guile/scm-breakpoint.c     |  2 +-
 gdb/guile/scm-frame.c          |  4 ++--
 gdb/guile/scm-param.c          |  2 +-
 gdb/guile/scm-symbol.c         |  4 ++--
 gdb/guile/scm-value.c          |  2 +-
 gdb/infcall.c                  |  2 +-
 gdb/linespec.c                 |  2 +-
 gdb/mi/mi-interp.c             |  2 +-
 gdb/python/py-breakpoint.c     |  4 ++--
 gdb/python/py-inferior.c       |  4 ++--
 gdb/python/py-value.c          |  4 ++--
 gdb/xml-support.c              |  1 -
 19 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3047ef3827d..c74fc61ea42 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2434,7 +2434,7 @@ insert_bp_location (struct bp_location *bl,
 		    int *hw_breakpoint_error,
 		    int *hw_bp_error_explained_already)
 {
-  gdb_exception bp_excpt = exception_none;
+  gdb_exception bp_excpt;
 
   if (!should_be_inserted (bl) || (bl->inserted && !bl->needs_update))
     return 0;
@@ -13593,7 +13593,7 @@ static std::vector<symtab_and_line>
 location_to_sals (struct breakpoint *b, struct event_location *location,
 		  struct program_space *search_pspace, int *found)
 {
-  struct gdb_exception exception = exception_none;
+  struct gdb_exception exception;
 
   gdb_assert (b->ops != NULL);
 
diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index c150f40feed..17639d0c3f3 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -357,7 +357,7 @@ static struct gdb_exception
 safe_execute_command (struct ui_out *command_uiout, const char *command,
 		      int from_tty)
 {
-  struct gdb_exception e = exception_none;
+  struct gdb_exception e;
 
   /* Save and override the global ``struct ui_out'' builder.  */
   scoped_restore saved_uiout = make_scoped_restore (&current_uiout,
diff --git a/gdb/common/common-exceptions.c b/gdb/common/common-exceptions.c
index 6378dc40d6d..59c27ab52d4 100644
--- a/gdb/common/common-exceptions.c
+++ b/gdb/common/common-exceptions.c
@@ -21,8 +21,6 @@
 #include "common-exceptions.h"
 #include <forward_list>
 
-const struct gdb_exception exception_none;
-
 /* Possible catcher states.  */
 enum catcher_state {
   /* Initial state, a new catcher has just been created.  */
@@ -47,7 +45,7 @@ struct catcher
   /* Jump buffer pointing back at the exception handler.  */
   jmp_buf buf;
   /* Status buffer belonging to the exception handler.  */
-  struct gdb_exception exception = exception_none;
+  struct gdb_exception exception;
 };
 
 /* Where to go for throw_exception().  */
diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
index 3f47caec775..33fa8a92ec2 100644
--- a/gdb/common/common-exceptions.h
+++ b/gdb/common/common-exceptions.h
@@ -295,7 +295,4 @@ extern void throw_error (enum errors error, const char *fmt, ...)
 extern void throw_quit (const char *fmt, ...)
      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
 
-/* A pre-defined non-exception.  */
-extern const struct gdb_exception exception_none;
-
 #endif /* COMMON_COMMON_EXCEPTIONS_H */
diff --git a/gdb/event-top.c b/gdb/event-top.c
index cd54eb5a2c5..959792d9e7a 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -164,7 +164,7 @@ void (*after_char_processing_hook) (void);
 static struct gdb_exception
 gdb_rl_callback_read_char_wrapper_noexcept () noexcept
 {
-  struct gdb_exception gdb_expt = exception_none;
+  struct gdb_exception gdb_expt;
 
   /* C++ exceptions can't normally be thrown across readline (unless
      it is built with -fexceptions, but it won't by default on many
@@ -205,7 +205,7 @@ gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
 static void
 gdb_rl_callback_handler (char *rl) noexcept
 {
-  struct gdb_exception gdb_rl_expt = exception_none;
+  struct gdb_exception gdb_rl_expt;
   struct ui *ui = current_ui;
 
   try
diff --git a/gdb/exec.c b/gdb/exec.c
index b0eb9ff02a3..7ff77f99160 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -148,7 +148,7 @@ void
 try_open_exec_file (const char *exec_file_host, struct inferior *inf,
 		    symfile_add_flags add_flags)
 {
-  struct gdb_exception prev_err = exception_none;
+  struct gdb_exception prev_err;
 
   /* exec_file_attach and symbol_file_add_main may throw an error if the file
      cannot be opened either locally or remotely.
diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index 3ba2dab18f6..f86c26390c4 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -411,7 +411,7 @@ gdbscm_register_breakpoint_x (SCM self)
 {
   breakpoint_smob *bp_smob
     = bpscm_get_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
-  struct gdb_exception except = exception_none;
+  struct gdb_exception except;
   const char *location, *copy;
 
   /* We only support registering breakpoints created with make-breakpoint.  */
diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
index ca271dbab5c..f3795f83f72 100644
--- a/gdb/guile/scm-frame.c
+++ b/gdb/guile/scm-frame.c
@@ -777,7 +777,7 @@ gdbscm_frame_read_register (SCM self, SCM register_scm)
   gdbscm_parse_function_args (FUNC_NAME, SCM_ARG2, NULL, "s",
 			      register_scm, &register_str);
 
-  struct gdb_exception except = exception_none;
+  struct gdb_exception except;
 
   try
     {
@@ -864,7 +864,7 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
     }
   else if (scm_is_string (symbol_scm))
     {
-      struct gdb_exception except = exception_none;
+      struct gdb_exception except;
 
       if (! SCM_UNBNDP (block_scm))
 	{
diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c
index a55deabf56c..cc10806d97e 100644
--- a/gdb/guile/scm-param.c
+++ b/gdb/guile/scm-param.c
@@ -1056,7 +1056,7 @@ gdbscm_parameter_value (SCM self)
       struct cmd_list_element *alias, *prefix, *cmd;
       char *newarg;
       int found = -1;
-      struct gdb_exception except = exception_none;
+      struct gdb_exception except;
 
       gdb::unique_xmalloc_ptr<char> name
 	= gdbscm_scm_to_host_string (self, NULL, &except_scm);
diff --git a/gdb/guile/scm-symbol.c b/gdb/guile/scm-symbol.c
index c135ff5ac37..ab39123dff0 100644
--- a/gdb/guile/scm-symbol.c
+++ b/gdb/guile/scm-symbol.c
@@ -614,7 +614,7 @@ gdbscm_lookup_symbol (SCM name_scm, SCM rest)
 	}
     }
 
-  struct gdb_exception except = exception_none;
+  struct gdb_exception except;
   try
     {
       symbol = lookup_symbol (name, block, (domain_enum) domain,
@@ -646,7 +646,7 @@ gdbscm_lookup_global_symbol (SCM name_scm, SCM rest)
   int domain_arg_pos = -1;
   int domain = VAR_DOMAIN;
   struct symbol *symbol = NULL;
-  struct gdb_exception except = exception_none;
+  struct gdb_exception except;
 
   gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#i",
 			      name_scm, &name, rest,
diff --git a/gdb/guile/scm-value.c b/gdb/guile/scm-value.c
index 5bbc3e8b8df..00d1c182e72 100644
--- a/gdb/guile/scm-value.c
+++ b/gdb/guile/scm-value.c
@@ -1048,7 +1048,7 @@ gdbscm_value_to_lazy_string (SCM self, SCM rest)
   char *encoding = NULL;
   int length = -1;
   SCM result = SCM_BOOL_F; /* -Wall */
-  struct gdb_exception except = exception_none;
+  struct gdb_exception except;
 
   /* The sequencing here, as everywhere else, is important.
      We can't have existing cleanups when a Scheme exception is thrown.  */
diff --git a/gdb/infcall.c b/gdb/infcall.c
index c102b301e00..52f9bc907e2 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -568,7 +568,7 @@ static struct gdb_exception
 run_inferior_call (struct call_thread_fsm *sm,
 		   struct thread_info *call_thread, CORE_ADDR real_pc)
 {
-  struct gdb_exception caught_error = exception_none;
+  struct gdb_exception caught_error;
   int saved_in_infcall = call_thread->control.in_infcall;
   ptid_t call_thread_ptid = call_thread->ptid;
   enum prompt_state saved_prompt_state = current_ui->prompt_state;
diff --git a/gdb/linespec.c b/gdb/linespec.c
index c42ddba7315..6d26638296e 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2508,7 +2508,7 @@ parse_linespec (linespec_parser *parser, const char *arg,
 		symbol_name_match_type match_type)
 {
   linespec_token token;
-  struct gdb_exception file_exception = exception_none;
+  struct gdb_exception file_exception;
 
   /* A special case to start.  It has become quite popular for
      IDEs to work around bugs in the previous parser by quoting
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 25c79b150ef..4568d398d94 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -181,7 +181,7 @@ gdb_exception
 mi_interp::exec (const char *command)
 {
   mi_execute_command_wrapper (command);
-  return exception_none;
+  return gdb_exception ();
 }
 
 void
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 03430e69e39..dfc30f70bb2 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -445,7 +445,7 @@ bppy_set_condition (PyObject *self, PyObject *newvalue, void *closure)
   gdb::unique_xmalloc_ptr<char> exp_holder;
   const char *exp = NULL;
   gdbpy_breakpoint_object *self_bp = (gdbpy_breakpoint_object *) self;
-  struct gdb_exception except = exception_none;
+  struct gdb_exception except;
 
   BPPY_SET_REQUIRE_VALID (self_bp);
 
@@ -515,7 +515,7 @@ static int
 bppy_set_commands (PyObject *self, PyObject *newvalue, void *closure)
 {
   gdbpy_breakpoint_object *self_bp = (gdbpy_breakpoint_object *) self;
-  struct gdb_exception except = exception_none;
+  struct gdb_exception except;
 
   BPPY_SET_REQUIRE_VALID (self_bp);
 
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 984cebb101d..1b7e3c24917 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -546,7 +546,7 @@ infpy_read_memory (PyObject *self, PyObject *args, PyObject *kw)
 static PyObject *
 infpy_write_memory (PyObject *self, PyObject *args, PyObject *kw)
 {
-  struct gdb_exception except = exception_none;
+  struct gdb_exception except;
   Py_ssize_t buf_len;
   const gdb_byte *buffer;
   CORE_ADDR addr, length;
@@ -682,7 +682,7 @@ get_char_buffer (PyObject *self, Py_ssize_t segment, char **ptrptr)
 static PyObject *
 infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
 {
-  struct gdb_exception except = exception_none;
+  struct gdb_exception except;
   CORE_ADDR start_addr, length;
   static const char *keywords[] = { "address", "length", "pattern", NULL };
   PyObject *start_addr_obj, *length_obj;
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index d3f4de40540..3349802f7fa 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -903,7 +903,7 @@ get_field_type (PyObject *field)
 static PyObject *
 valpy_getitem (PyObject *self, PyObject *key)
 {
-  struct gdb_exception except = exception_none;
+  struct gdb_exception except;
   value_object *self_value = (value_object *) self;
   gdb::unique_xmalloc_ptr<char> field;
   struct type *base_class_type = NULL, *field_type = NULL;
@@ -1480,7 +1480,7 @@ valpy_absolute (PyObject *self)
 static int
 valpy_nonzero (PyObject *self)
 {
-  struct gdb_exception except = exception_none;
+  struct gdb_exception except;
   value_object *self_value = (value_object *) self;
   struct type *type;
   int nonzero = 0; /* Appease GCC warning.  */
diff --git a/gdb/xml-support.c b/gdb/xml-support.c
index 4145878bf3a..d4cd89c0339 100644
--- a/gdb/xml-support.c
+++ b/gdb/xml-support.c
@@ -479,7 +479,6 @@ gdb_xml_parser::gdb_xml_parser (const char *name,
 				void *user_data)
   : m_name (name),
     m_user_data (user_data),
-    m_error (exception_none),
     m_last_line (0),
     m_dtd_name (NULL),
     m_is_xinclude (false)
-- 
2.20.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/5] Avoid undefined behavior in Guile exception handling
  2019-04-25 16:53 [PATCH 0/5] More exception-handling improvements Tom Tromey
  2019-04-25 16:53 ` [PATCH 2/5] Make SJLJ exceptions more efficient Tom Tromey
@ 2019-04-25 16:53 ` Tom Tromey
  2019-04-25 16:53 ` [PATCH 1/5] Remove exception_none Tom Tromey
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2019-04-25 16:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The Guile code will longjmp (via scm_throw) when an object requiring
destruction is on the stack.  This is undefined behavior.

This changes this code to run any destructors in inner scopes, and to
pass a POD to gdbscm_throw_gdb_exception.

gdb/ChangeLog
2019-04-25  Tom Tromey  <tromey@adacore.com>

	* guile/scm-exception.c (gdbscm_scm_from_gdb_exception)
	(gdbscm_throw_gdb_exception): Take a gdbscm_gdb_exception.
	* guile/scm-block.c, guile/scm-breakpoint.c, guile/scm-cmd.c,
	guile/scm-disasm.c, guile/scm-frame.c, guile/scm-lazy-string.c,
	guile/scm-math.c, guile/scm-param.c, guile/scm-ports.c,
	guile/scm-symbol.c, guile/scm-symtab.c, guile/scm-type.c,
	guile/scm-value.c: Use unpack.
	* guile/guile-internal.h (gdbscm_scm_from_gdb_exception): Take a
	gdbscm_gdb_exception.
	(gdbscm_throw_gdb_exception): Likewise.
	(struct gdbscm_gdb_exception): New.
	(unpack): New function.
	(gdbscm_wrap): Use unpack.
---
 gdb/ChangeLog               | 16 ++++++++
 gdb/guile/guile-internal.h  | 38 +++++++++++++++++--
 gdb/guile/scm-block.c       |  4 +-
 gdb/guile/scm-breakpoint.c  | 33 ++++++++++++-----
 gdb/guile/scm-cmd.c         |  4 +-
 gdb/guile/scm-disasm.c      |  4 +-
 gdb/guile/scm-exception.c   | 10 +++--
 gdb/guile/scm-frame.c       | 73 ++++++++++++++++++++++++++-----------
 gdb/guile/scm-lazy-string.c |  2 +-
 gdb/guile/scm-math.c        |  2 +-
 gdb/guile/scm-param.c       |  8 ++--
 gdb/guile/scm-ports.c       |  4 +-
 gdb/guile/scm-symbol.c      | 20 ++++++----
 gdb/guile/scm-symtab.c      |  4 +-
 gdb/guile/scm-type.c        | 42 +++++++++++++++------
 gdb/guile/scm-value.c       | 53 +++++++++++++++++++--------
 16 files changed, 235 insertions(+), 82 deletions(-)

diff --git a/gdb/guile/guile-internal.h b/gdb/guile/guile-internal.h
index a3e02abe41b..d316d1ce353 100644
--- a/gdb/guile/guile-internal.h
+++ b/gdb/guile/guile-internal.h
@@ -353,9 +353,11 @@ extern void gdbscm_misc_error (const char *subr, int arg_pos,
 
 extern void gdbscm_throw (SCM exception) ATTRIBUTE_NORETURN;
 
-extern SCM gdbscm_scm_from_gdb_exception (struct gdb_exception exception);
+struct gdbscm_gdb_exception;
+extern SCM gdbscm_scm_from_gdb_exception
+  (const struct gdbscm_gdb_exception &exception);
 
-extern void gdbscm_throw_gdb_exception (struct gdb_exception exception)
+extern void gdbscm_throw_gdb_exception (struct gdbscm_gdb_exception exception)
   ATTRIBUTE_NORETURN;
 
 extern void gdbscm_print_exception_with_stack (SCM port, SCM stack,
@@ -650,6 +652,33 @@ extern void gdbscm_initialize_values (void);
    with a TRY/CATCH, because the dtors won't otherwise be run when a
    Guile exceptions is thrown.  */
 
+/* This is a destructor-less clone of gdb_exception.  */
+
+struct gdbscm_gdb_exception
+{
+  enum return_reason reason;
+  enum errors error;
+  /* The message is xmalloc'd.  */
+  char *message;
+};
+
+/* Return a gdbscm_gdb_exception representing EXC.  */
+
+inline struct gdbscm_gdb_exception
+unpack (const gdb_exception &exc)
+{
+  struct gdbscm_gdb_exception result;
+  result.reason = exc.reason;
+  result.error = exc.error;
+  if (exc.message == nullptr)
+    result.message = nullptr;
+  else
+    result.message = xstrdup (exc.message->c_str ());
+  /* The message should be NULL iff the reason is zero.  */
+  gdb_assert ((result.reason == 0) == (result.message == nullptr));
+  return result;
+}
+
 /* Use this after a TRY/CATCH to throw the appropriate Scheme
    exception if a GDB error occurred.  */
 
@@ -676,6 +705,7 @@ SCM
 gdbscm_wrap (Function &&func, Args &&... args)
 {
   SCM result = SCM_BOOL_F;
+  struct gdbscm_gdb_exception exc {};
 
   try
     {
@@ -683,9 +713,11 @@ gdbscm_wrap (Function &&func, Args &&... args)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
+
   if (gdbscm_is_exception (result))
     gdbscm_throw (result);
 
diff --git a/gdb/guile/scm-block.c b/gdb/guile/scm-block.c
index 2114ca1f199..3a78465f9e2 100644
--- a/gdb/guile/scm-block.c
+++ b/gdb/guile/scm-block.c
@@ -680,6 +680,7 @@ gdbscm_lookup_block (SCM pc_scm)
 
   gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, NULL, "U", pc_scm, &pc);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       cust = find_pc_compunit_symtab (pc);
@@ -689,9 +690,10 @@ gdbscm_lookup_block (SCM pc_scm)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (cust == NULL || COMPUNIT_OBJFILE (cust) == NULL)
     {
       gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG1, pc_scm,
diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index f86c26390c4..29ab859e07b 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -411,7 +411,7 @@ gdbscm_register_breakpoint_x (SCM self)
 {
   breakpoint_smob *bp_smob
     = bpscm_get_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
-  struct gdb_exception except;
+  struct gdbscm_gdb_exception except {};
   const char *location, *copy;
 
   /* We only support registering breakpoints created with make-breakpoint.  */
@@ -467,7 +467,7 @@ gdbscm_register_breakpoint_x (SCM self)
     }
   catch (const gdb_exception &ex)
     {
-      except = ex;
+      except = unpack (ex);
     }
 
   /* Ensure this gets reset, even if there's an error.  */
@@ -489,15 +489,17 @@ gdbscm_delete_breakpoint_x (SCM self)
   breakpoint_smob *bp_smob
     = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       delete_breakpoint (bp_smob->bp);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return SCM_UNSPECIFIED;
 }
 
@@ -586,6 +588,7 @@ gdbscm_set_breakpoint_enabled_x (SCM self, SCM newvalue)
   SCM_ASSERT_TYPE (gdbscm_is_bool (newvalue), newvalue, SCM_ARG2, FUNC_NAME,
 		   _("boolean"));
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       if (gdbscm_is_true (newvalue))
@@ -595,9 +598,10 @@ gdbscm_set_breakpoint_enabled_x (SCM self, SCM newvalue)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return SCM_UNSPECIFIED;
 }
 
@@ -623,15 +627,17 @@ gdbscm_set_breakpoint_silent_x (SCM self, SCM newvalue)
   SCM_ASSERT_TYPE (gdbscm_is_bool (newvalue), newvalue, SCM_ARG2, FUNC_NAME,
 		   _("boolean"));
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       breakpoint_set_silent (bp_smob->bp, gdbscm_is_true (newvalue));
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return SCM_UNSPECIFIED;
 }
 
@@ -663,15 +669,17 @@ gdbscm_set_breakpoint_ignore_count_x (SCM self, SCM newvalue)
   if (value < 0)
     value = 0;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       set_ignore_count (bp_smob->number, (int) value, 0);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return SCM_UNSPECIFIED;
 }
 
@@ -783,15 +791,17 @@ gdbscm_set_breakpoint_task_x (SCM self, SCM newvalue)
     {
       id = scm_to_long (newvalue);
 
+      struct gdbscm_gdb_exception exc {};
       try
 	{
 	  valid_id = valid_task_id (id);
 	}
       catch (const gdb_exception &except)
 	{
-	  GDBSCM_HANDLE_GDB_EXCEPTION (except);
+	  exc = unpack (except);
 	}
 
+      GDBSCM_HANDLE_GDB_EXCEPTION (exc);
       if (! valid_id)
 	{
 	  gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG2, newvalue,
@@ -803,15 +813,17 @@ gdbscm_set_breakpoint_task_x (SCM self, SCM newvalue)
   else
     SCM_ASSERT_TYPE (0, newvalue, SCM_ARG2, FUNC_NAME, _("integer or #f"));
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       breakpoint_set_task (bp_smob->bp, id);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return SCM_UNSPECIFIED;
 }
 
@@ -968,17 +980,18 @@ gdbscm_breakpoint_commands (SCM self)
   string_file buf;
 
   current_uiout->redirect (&buf);
+  struct gdbscm_gdb_exception exc {};
   try
     {
       print_command_lines (current_uiout, breakpoint_commands (bp), 0);
     }
   catch (const gdb_exception &except)
     {
-      current_uiout->redirect (NULL);
-      gdbscm_throw_gdb_exception (except);
+      exc = unpack (except);
     }
 
   current_uiout->redirect (NULL);
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   result = gdbscm_scm_from_c_string (buf.c_str ());
 
   return result;
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 38db7f5c71f..0a9cd44a17a 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -758,6 +758,7 @@ gdbscm_register_command_x (SCM self)
   c_smob->cmd_name = gdbscm_gc_xstrdup (cmd_name);
   xfree (cmd_name);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       if (c_smob->is_prefix)
@@ -778,8 +779,9 @@ gdbscm_register_command_x (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
 
   /* Note: At this point the command exists in gdb.
      So no more errors after this point.  */
diff --git a/gdb/guile/scm-disasm.c b/gdb/guile/scm-disasm.c
index d673a1eb90a..86129773931 100644
--- a/gdb/guile/scm-disasm.c
+++ b/gdb/guile/scm-disasm.c
@@ -247,6 +247,7 @@ gdbscm_arch_disassemble (SCM self, SCM start_scm, SCM rest)
       int insn_len = 0;
       string_file buf;
 
+      struct gdbscm_gdb_exception exc {};
       try
 	{
 	  if (using_port)
@@ -259,9 +260,10 @@ gdbscm_arch_disassemble (SCM self, SCM start_scm, SCM rest)
 	}
       catch (const gdb_exception &except)
 	{
-	  GDBSCM_HANDLE_GDB_EXCEPTION (except);
+	  exc = unpack (except);
 	}
 
+      GDBSCM_HANDLE_GDB_EXCEPTION (exc);
       result = scm_cons (dascm_make_insn (pc, buf.c_str (), insn_len),
 			 result);
 
diff --git a/gdb/guile/scm-exception.c b/gdb/guile/scm-exception.c
index 44cd7b35750..be24b8ac9d0 100644
--- a/gdb/guile/scm-exception.c
+++ b/gdb/guile/scm-exception.c
@@ -428,7 +428,7 @@ gdbscm_throw (SCM exception)
 /* Convert a GDB exception to a <gdb:exception> object.  */
 
 SCM
-gdbscm_scm_from_gdb_exception (struct gdb_exception exception)
+gdbscm_scm_from_gdb_exception (const struct gdbscm_gdb_exception &exception)
 {
   SCM key;
 
@@ -446,7 +446,7 @@ gdbscm_scm_from_gdb_exception (struct gdb_exception exception)
 
   return gdbscm_make_error (key, NULL, "~A",
 			    scm_list_1 (gdbscm_scm_from_c_string
-					(exception.what ())),
+					(exception.message)),
 			    SCM_BOOL_F);
 }
 
@@ -454,9 +454,11 @@ gdbscm_scm_from_gdb_exception (struct gdb_exception exception)
    This function does not return.  */
 
 void
-gdbscm_throw_gdb_exception (struct gdb_exception exception)
+gdbscm_throw_gdb_exception (struct gdbscm_gdb_exception exception)
 {
-  gdbscm_throw (gdbscm_scm_from_gdb_exception (exception));
+  SCM scm_exception = gdbscm_scm_from_gdb_exception (exception);
+  xfree (exception.message);
+  gdbscm_throw (scm_exception);
 }
 
 /* Print the error message portion of an exception.
diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
index f3795f83f72..fc7c7bff32a 100644
--- a/gdb/guile/scm-frame.c
+++ b/gdb/guile/scm-frame.c
@@ -250,7 +250,7 @@ frscm_scm_from_frame (struct frame_info *frame, struct inferior *inferior)
     }
   catch (const gdb_exception &except)
     {
-      return gdbscm_scm_from_gdb_exception (except);
+      return gdbscm_scm_from_gdb_exception (unpack (except));
     }
 
   f_scm = frscm_make_frame_smob ();
@@ -396,15 +396,17 @@ gdbscm_frame_valid_p (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return scm_from_bool (frame != NULL);
 }
 
@@ -423,6 +425,7 @@ gdbscm_frame_name (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
@@ -431,9 +434,10 @@ gdbscm_frame_name (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -460,6 +464,7 @@ gdbscm_frame_type (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
@@ -468,9 +473,10 @@ gdbscm_frame_type (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -491,15 +497,17 @@ gdbscm_frame_arch (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -521,15 +529,17 @@ gdbscm_frame_unwind_stop_reason (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -553,6 +563,7 @@ gdbscm_frame_pc (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
@@ -561,9 +572,10 @@ gdbscm_frame_pc (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -585,6 +597,7 @@ gdbscm_frame_block (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
@@ -593,9 +606,10 @@ gdbscm_frame_block (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -635,6 +649,7 @@ gdbscm_frame_function (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
@@ -643,9 +658,10 @@ gdbscm_frame_function (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -671,6 +687,7 @@ gdbscm_frame_older (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
@@ -679,9 +696,10 @@ gdbscm_frame_older (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -707,6 +725,7 @@ gdbscm_frame_newer (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
@@ -715,9 +734,10 @@ gdbscm_frame_newer (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -742,6 +762,7 @@ gdbscm_frame_sal (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
@@ -750,9 +771,10 @@ gdbscm_frame_sal (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -777,7 +799,7 @@ gdbscm_frame_read_register (SCM self, SCM register_scm)
   gdbscm_parse_function_args (FUNC_NAME, SCM_ARG2, NULL, "s",
 			      register_scm, &register_str);
 
-  struct gdb_exception except;
+  struct gdbscm_gdb_exception except {};
 
   try
     {
@@ -795,7 +817,7 @@ gdbscm_frame_read_register (SCM self, SCM register_scm)
     }
   catch (const gdb_exception &ex)
     {
-      except = ex;
+      except = unpack (ex);
     }
 
   xfree (register_str);
@@ -838,15 +860,17 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -864,7 +888,7 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
     }
   else if (scm_is_string (symbol_scm))
     {
-      struct gdb_exception except;
+      struct gdbscm_gdb_exception except {};
 
       if (! SCM_UNBNDP (block_scm))
 	{
@@ -896,7 +920,7 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
 	  }
 	catch (const gdb_exception &ex)
 	  {
-	    except = ex;
+	    except = unpack (ex);
 	  }
       }
 
@@ -919,9 +943,10 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return vlscm_scm_from_value (value);
 }
 
@@ -936,6 +961,7 @@ gdbscm_frame_select (SCM self)
 
   f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = frscm_frame_smob_to_frame (f_smob);
@@ -944,9 +970,10 @@ gdbscm_frame_select (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (frame == NULL)
     {
       gdbscm_invalid_object_error (FUNC_NAME, SCM_ARG1, self,
@@ -964,15 +991,17 @@ gdbscm_newest_frame (void)
 {
   struct frame_info *frame = NULL;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = get_current_frame ();
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return frscm_scm_from_frame_unsafe (frame, current_inferior ());
 }
 
@@ -984,15 +1013,17 @@ gdbscm_selected_frame (void)
 {
   struct frame_info *frame = NULL;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       frame = get_selected_frame (_("No frame is currently selected"));
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return frscm_scm_from_frame_unsafe (frame, current_inferior ());
 }
 
diff --git a/gdb/guile/scm-lazy-string.c b/gdb/guile/scm-lazy-string.c
index 4d69b234bb2..b0bbc827c8f 100644
--- a/gdb/guile/scm-lazy-string.c
+++ b/gdb/guile/scm-lazy-string.c
@@ -338,7 +338,7 @@ lsscm_safe_lazy_string_to_value (SCM string, int arg_pos,
     }
   catch (const gdb_exception &except)
     {
-      *except_scmp = gdbscm_scm_from_gdb_exception (except);
+      *except_scmp = gdbscm_scm_from_gdb_exception (unpack (except));
       return NULL;
     }
 
diff --git a/gdb/guile/scm-math.c b/gdb/guile/scm-math.c
index dc7245ba7b4..35ad4aa6035 100644
--- a/gdb/guile/scm-math.c
+++ b/gdb/guile/scm-math.c
@@ -826,7 +826,7 @@ vlscm_convert_typed_value_from_scheme (const char *func_name,
     }
   catch (const gdb_exception &except)
     {
-      except_scm = gdbscm_scm_from_gdb_exception (except);
+      except_scm = gdbscm_scm_from_gdb_exception (unpack (except));
     }
 
   if (gdbscm_is_true (except_scm))
diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c
index cc10806d97e..eb5472db6bb 100644
--- a/gdb/guile/scm-param.c
+++ b/gdb/guile/scm-param.c
@@ -1006,6 +1006,7 @@ gdbscm_register_parameter_x (SCM self)
 		_("parameter exists, \"show\" command is already defined"));
     }
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       add_setshow_generic (p_smob->type, p_smob->cmd_class,
@@ -1020,9 +1021,10 @@ gdbscm_register_parameter_x (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   /* Note: At this point the parameter exists in gdb.
      So no more errors after this point.  */
 
@@ -1056,7 +1058,7 @@ gdbscm_parameter_value (SCM self)
       struct cmd_list_element *alias, *prefix, *cmd;
       char *newarg;
       int found = -1;
-      struct gdb_exception except;
+      struct gdbscm_gdb_exception except {};
 
       gdb::unique_xmalloc_ptr<char> name
 	= gdbscm_scm_to_host_string (self, NULL, &except_scm);
@@ -1069,7 +1071,7 @@ gdbscm_parameter_value (SCM self)
 	}
       catch (const gdb_exception &ex)
 	{
-	  except = ex;
+	  except = unpack (ex);
 	}
 
       xfree (newarg);
diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index 57d3b18f962..c388a7769ec 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -272,6 +272,7 @@ ioscm_write (SCM port, const void *data, size_t size)
   if (scm_is_eq (port, input_port_scm))
     return;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       if (scm_is_eq (port, error_port_scm))
@@ -281,8 +282,9 @@ ioscm_write (SCM port, const void *data, size_t size)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
 }
 
 /* Flush gdb's stdout or stderr.  */
diff --git a/gdb/guile/scm-symbol.c b/gdb/guile/scm-symbol.c
index ab39123dff0..0cd6418b14b 100644
--- a/gdb/guile/scm-symbol.c
+++ b/gdb/guile/scm-symbol.c
@@ -486,15 +486,17 @@ gdbscm_symbol_needs_frame_p (SCM self)
   struct symbol *symbol = s_smob->symbol;
   int result = 0;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       result = symbol_read_needs_frame (symbol);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return scm_from_bool (result);
 }
 
@@ -538,6 +540,7 @@ gdbscm_symbol_value (SCM self, SCM rest)
 				 _("cannot get the value of a typedef"));
     }
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       if (f_smob != NULL)
@@ -558,9 +561,10 @@ gdbscm_symbol_value (SCM self, SCM rest)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return vlscm_scm_from_value (value);
 }
 \f
@@ -602,6 +606,7 @@ gdbscm_lookup_symbol (SCM name_scm, SCM rest)
     {
       struct frame_info *selected_frame;
 
+      struct gdbscm_gdb_exception exc {};
       try
 	{
 	  selected_frame = get_selected_frame (_("no frame selected"));
@@ -610,11 +615,12 @@ gdbscm_lookup_symbol (SCM name_scm, SCM rest)
       catch (const gdb_exception &ex)
 	{
 	  xfree (name);
-	  GDBSCM_HANDLE_GDB_EXCEPTION (ex);
+	  exc = unpack (ex);
 	}
+      GDBSCM_HANDLE_GDB_EXCEPTION (exc);
     }
 
-  struct gdb_exception except;
+  struct gdbscm_gdb_exception except {};
   try
     {
       symbol = lookup_symbol (name, block, (domain_enum) domain,
@@ -622,7 +628,7 @@ gdbscm_lookup_symbol (SCM name_scm, SCM rest)
     }
   catch (const gdb_exception &ex)
     {
-      except = ex;
+      except = unpack (ex);
     }
 
   xfree (name);
@@ -646,7 +652,7 @@ gdbscm_lookup_global_symbol (SCM name_scm, SCM rest)
   int domain_arg_pos = -1;
   int domain = VAR_DOMAIN;
   struct symbol *symbol = NULL;
-  struct gdb_exception except;
+  struct gdbscm_gdb_exception except {};
 
   gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#i",
 			      name_scm, &name, rest,
@@ -658,7 +664,7 @@ gdbscm_lookup_global_symbol (SCM name_scm, SCM rest)
     }
   catch (const gdb_exception &ex)
     {
-      except = ex;
+      except = unpack (ex);
     }
 
   xfree (name);
diff --git a/gdb/guile/scm-symtab.c b/gdb/guile/scm-symtab.c
index 60ed7072556..0be09ab66c2 100644
--- a/gdb/guile/scm-symtab.c
+++ b/gdb/guile/scm-symtab.c
@@ -591,6 +591,7 @@ gdbscm_find_pc_line (SCM pc_scm)
 
   gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, NULL, "U", pc_scm, &pc_ull);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       CORE_ADDR pc = (CORE_ADDR) pc_ull;
@@ -599,9 +600,10 @@ gdbscm_find_pc_line (SCM pc_scm)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return stscm_scm_from_sal (sal);
 }
 \f
diff --git a/gdb/guile/scm-type.c b/gdb/guile/scm-type.c
index 8f47ef68cc1..d326044d1e9 100644
--- a/gdb/guile/scm-type.c
+++ b/gdb/guile/scm-type.c
@@ -105,6 +105,7 @@ tyscm_type_smob_type (type_smob *t_smob)
 static std::string
 tyscm_type_name (struct type *type)
 {
+  SCM excp;
   try
     {
       string_file stb;
@@ -114,11 +115,10 @@ tyscm_type_name (struct type *type)
     }
   catch (const gdb_exception &except)
     {
-      SCM excp = gdbscm_scm_from_gdb_exception (except);
-      gdbscm_throw (excp);
+      excp = gdbscm_scm_from_gdb_exception (unpack (except));
     }
 
-  gdb_assert_not_reached ("no way to get here");
+  gdbscm_throw (excp);
 }
 \f
 /* Administrivia for type smobs.  */
@@ -234,15 +234,17 @@ tyscm_equal_p_type_smob (SCM type1_scm, SCM type2_scm)
   type1 = type1_smob->type;
   type2 = type2_smob->type;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       result = types_deeply_equal (type1, type2);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return scm_from_bool (result);
 }
 
@@ -650,15 +652,17 @@ gdbscm_type_strip_typedefs (SCM self)
     = tyscm_get_type_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
   struct type *type = t_smob->type;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       type = check_typedef (type);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return tyscm_scm_from_type (type);
 }
 
@@ -671,15 +675,17 @@ tyscm_get_composite (struct type *type)
 
   for (;;)
     {
+      struct gdbscm_gdb_exception exc {};
       try
 	{
 	  type = check_typedef (type);
 	}
       catch (const gdb_exception &except)
 	{
-	  GDBSCM_HANDLE_GDB_EXCEPTION (except);
+	  exc = unpack (except);
 	}
 
+      GDBSCM_HANDLE_GDB_EXCEPTION (exc);
       if (TYPE_CODE (type) != TYPE_CODE_PTR
 	  && TYPE_CODE (type) != TYPE_CODE_REF)
 	break;
@@ -725,6 +731,7 @@ tyscm_array_1 (SCM self, SCM n1_scm, SCM n2_scm, int is_vector,
 				 _("Array length must not be negative"));
     }
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       array = lookup_array_range_type (type, n1, n2);
@@ -733,9 +740,10 @@ tyscm_array_1 (SCM self, SCM n1_scm, SCM n2_scm, int is_vector,
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return tyscm_scm_from_type (array);
 }
 
@@ -781,15 +789,17 @@ gdbscm_type_pointer (SCM self)
     = tyscm_get_type_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
   struct type *type = t_smob->type;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       type = lookup_pointer_type (type);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return tyscm_scm_from_type (type);
 }
 
@@ -842,15 +852,17 @@ gdbscm_type_reference (SCM self)
     = tyscm_get_type_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
   struct type *type = t_smob->type;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       type = lookup_lvalue_reference_type (type);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return tyscm_scm_from_type (type);
 }
 
@@ -879,15 +891,17 @@ gdbscm_type_const (SCM self)
     = tyscm_get_type_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
   struct type *type = t_smob->type;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       type = make_cv_type (1, 0, type, NULL);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return tyscm_scm_from_type (type);
 }
 
@@ -901,15 +915,17 @@ gdbscm_type_volatile (SCM self)
     = tyscm_get_type_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
   struct type *type = t_smob->type;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       type = make_cv_type (0, 1, type, NULL);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return tyscm_scm_from_type (type);
 }
 
@@ -923,15 +939,17 @@ gdbscm_type_unqualified (SCM self)
     = tyscm_get_type_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
   struct type *type = t_smob->type;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       type = make_cv_type (0, 0, type, NULL);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return tyscm_scm_from_type (type);
 }
 \f
diff --git a/gdb/guile/scm-value.c b/gdb/guile/scm-value.c
index 00d1c182e72..70e13a8ec52 100644
--- a/gdb/guile/scm-value.c
+++ b/gdb/guile/scm-value.c
@@ -156,6 +156,7 @@ vlscm_print_value_smob (SCM self, SCM port, scm_print_state *pstate)
      instead of writingp.  */
   opts.raw = !!pstate->writingp;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       string_file stb;
@@ -165,9 +166,10 @@ vlscm_print_value_smob (SCM self, SCM port, scm_print_state *pstate)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (pstate->writingp)
     scm_puts (">", port);
 
@@ -186,15 +188,17 @@ vlscm_equal_p_value_smob (SCM v1, SCM v2)
   const value_smob *v2_smob = (value_smob *) SCM_SMOB_DATA (v2);
   int result = 0;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       result = value_equal (v1_smob->value, v2_smob->value);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return scm_from_bool (result);
 }
 
@@ -493,6 +497,7 @@ gdbscm_value_dynamic_type (SCM self)
   if (! SCM_UNBNDP (v_smob->dynamic_type))
     return v_smob->dynamic_type;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       scoped_value_mark free_values;
@@ -531,9 +536,10 @@ gdbscm_value_dynamic_type (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (type == NULL)
     v_smob->dynamic_type = gdbscm_value_type (self);
   else
@@ -680,15 +686,17 @@ gdbscm_value_call (SCM self, SCM args)
   long args_count;
   struct value **vargs = NULL;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       ftype = check_typedef (value_type (function));
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   SCM_ASSERT_TYPE (TYPE_CODE (ftype) == TYPE_CODE_FUNC, self,
 		   SCM_ARG1, FUNC_NAME,
 		   _("function (value of TYPE_CODE_FUNC)"));
@@ -746,6 +754,7 @@ gdbscm_value_to_bytevector (SCM self)
 
   type = value_type (value);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       type = check_typedef (type);
@@ -754,9 +763,10 @@ gdbscm_value_to_bytevector (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   bv = scm_c_make_bytevector (length);
   memcpy (SCM_BYTEVECTOR_CONTENTS (bv), contents, length);
 
@@ -789,15 +799,17 @@ gdbscm_value_to_bool (SCM self)
 
   type = value_type (value);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       type = check_typedef (type);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   SCM_ASSERT_TYPE (is_intlike (type, 1), self, SCM_ARG1, FUNC_NAME,
 		   _("integer-like gdb value"));
 
@@ -810,9 +822,10 @@ gdbscm_value_to_bool (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   return scm_from_bool (l != 0);
 }
 
@@ -830,15 +843,17 @@ gdbscm_value_to_integer (SCM self)
 
   type = value_type (value);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       type = check_typedef (type);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   SCM_ASSERT_TYPE (is_intlike (type, 1), self, SCM_ARG1, FUNC_NAME,
 		   _("integer-like gdb value"));
 
@@ -851,9 +866,10 @@ gdbscm_value_to_integer (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   if (TYPE_UNSIGNED (type))
     return gdbscm_scm_from_ulongest (l);
   else
@@ -875,15 +891,17 @@ gdbscm_value_to_real (SCM self)
 
   type = value_type (value);
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       type = check_typedef (type);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   SCM_ASSERT_TYPE (is_intlike (type, 0) || TYPE_CODE (type) == TYPE_CODE_FLT,
 		   self, SCM_ARG1, FUNC_NAME, _("number"));
 
@@ -907,9 +925,10 @@ gdbscm_value_to_real (SCM self)
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   /* TODO: Is there a better way to check if the value fits?  */
   if (!value_equal (value, check))
     gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG1, self,
@@ -992,6 +1011,7 @@ gdbscm_value_to_string (SCM self, SCM rest)
   /* We don't assume anything about the result of scm_port_conversion_strategy.
      From this point on, if errors is not 'errors, use 'substitute.  */
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       gdb::unique_xmalloc_ptr<gdb_byte> buffer;
@@ -1001,8 +1021,9 @@ gdbscm_value_to_string (SCM self, SCM rest)
   catch (const gdb_exception &except)
     {
       xfree (encoding);
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
 
   /* If errors is "error", scm_from_stringn may throw a Scheme exception.
      Make sure we don't leak.  This is done via scm_dynwind_begin, et.al.  */
@@ -1048,7 +1069,7 @@ gdbscm_value_to_lazy_string (SCM self, SCM rest)
   char *encoding = NULL;
   int length = -1;
   SCM result = SCM_BOOL_F; /* -Wall */
-  struct gdb_exception except;
+  struct gdbscm_gdb_exception except {};
 
   /* The sequencing here, as everywhere else, is important.
      We can't have existing cleanups when a Scheme exception is thrown.  */
@@ -1121,7 +1142,7 @@ gdbscm_value_to_lazy_string (SCM self, SCM rest)
     }
   catch (const gdb_exception &ex)
     {
-      except = ex;
+      except = unpack (ex);
     }
 
   xfree (encoding);
@@ -1177,15 +1198,17 @@ gdbscm_value_print (SCM self)
 
   string_file stb;
 
+  struct gdbscm_gdb_exception exc {};
   try
     {
       common_val_print (value, &stb, 0, &opts, current_language);
     }
   catch (const gdb_exception &except)
     {
-      GDBSCM_HANDLE_GDB_EXCEPTION (except);
+      exc = unpack (except);
     }
 
+  GDBSCM_HANDLE_GDB_EXCEPTION (exc);
   /* Use SCM_FAILED_CONVERSION_QUESTION_MARK to ensure this doesn't
      throw an error if the encoding fails.
      IWBN to use scm_take_locale_string here, but we'd have to temporarily
-- 
2.20.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/5] Make SJLJ exceptions more efficient
  2019-04-25 16:53 [PATCH 0/5] More exception-handling improvements Tom Tromey
@ 2019-04-25 16:53 ` Tom Tromey
  2019-04-25 16:53 ` [PATCH 3/5] Avoid undefined behavior in Guile exception handling Tom Tromey
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2019-04-25 16:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes the SJLJ exception handling code to be a bit more
efficient, by using rvalue references and move assignment when
possible.

Tested by the buildbot.

gdb/ChangeLog
2019-04-25  Tom Tromey  <tromey@adacore.com>

	* event-top.c (gdb_rl_callback_read_char_wrapper_noexcept)
	(gdb_rl_callback_handler): Use std::move.
	* common/common-exceptions.h (struct gdb_exception): Add move
	assignment operator.
	(throw_exception_sjlj): Change "exception" to const reference.
	* common/common-exceptions.c (exceptions_state_mc_catch): Update.
	(throw_exception_sjlj): Change "exception" to const reference.
---
 gdb/ChangeLog                  | 10 ++++++++++
 gdb/common/common-exceptions.c |  5 +++--
 gdb/common/common-exceptions.h |  4 +++-
 gdb/event-top.c                |  6 +++---
 4 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/gdb/common/common-exceptions.c b/gdb/common/common-exceptions.c
index 59c27ab52d4..0b96cc679da 100644
--- a/gdb/common/common-exceptions.c
+++ b/gdb/common/common-exceptions.c
@@ -166,14 +166,15 @@ exceptions_state_mc_action_iter_1 (void)
 /* Return EXCEPTION to the nearest containing CATCH_SJLJ block.  */
 
 void
-throw_exception_sjlj (struct gdb_exception exception)
+throw_exception_sjlj (const struct gdb_exception &exception)
 {
   /* Jump to the nearest CATCH_SJLJ block, communicating REASON to
      that call via setjmp's return value.  Note that REASON can't be
      zero, by definition in common-exceptions.h.  */
   exceptions_state_mc (CATCH_THROWING);
+  enum return_reason reason = exception.reason;
   catchers.front ().exception = exception;
-  longjmp (catchers.front ().buf, exception.reason);
+  longjmp (catchers.front ().buf, reason);
 }
 
 /* Implementation of throw_exception that uses C++ try/catch.  */
diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
index 33fa8a92ec2..d7b25502262 100644
--- a/gdb/common/common-exceptions.h
+++ b/gdb/common/common-exceptions.h
@@ -152,6 +152,8 @@ struct gdb_exception
     return *this;
   }
 
+  gdb_exception &operator= (gdb_exception &&other) noexcept = default;
+
   /* Return the contents of the exception message, as a C string.  The
      string remains owned by the exception object.  */
   const char *what () const noexcept
@@ -281,7 +283,7 @@ extern void throw_exception (const gdb_exception &exception)
    containing exception handler established using TRY_SJLJ.  Necessary
    in some cases where we need to throw GDB exceptions across
    third-party library code (e.g., readline).  */
-extern void throw_exception_sjlj (struct gdb_exception exception)
+extern void throw_exception_sjlj (const struct gdb_exception &exception)
      ATTRIBUTE_NORETURN;
 
 /* Convenience wrappers around throw_exception that throw GDB
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 959792d9e7a..bb8ba5cfe57 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -178,7 +178,7 @@ gdb_rl_callback_read_char_wrapper_noexcept () noexcept
     }
   CATCH_SJLJ (ex, RETURN_MASK_ALL)
     {
-      gdb_expt = ex;
+      gdb_expt = std::move (ex);
     }
   END_CATCH_SJLJ
 
@@ -212,9 +212,9 @@ gdb_rl_callback_handler (char *rl) noexcept
     {
       ui->input_handler (gdb::unique_xmalloc_ptr<char> (rl));
     }
-  catch (const gdb_exception &ex)
+  catch (gdb_exception &ex)
     {
-      gdb_rl_expt = ex;
+      gdb_rl_expt = std::move (ex);
     }
 
   /* If we caught a GDB exception, longjmp out of the readline
-- 
2.20.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 0/5] More exception-handling improvements
@ 2019-04-25 16:53 Tom Tromey
  2019-04-25 16:53 ` [PATCH 2/5] Make SJLJ exceptions more efficient Tom Tromey
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Tom Tromey @ 2019-04-25 16:53 UTC (permalink / raw)
  To: gdb-patches

Philippe noticed a memory leak with the new exception-handling code,
and when I debugged it, I realized that the current code is relying on
undefined behavior: namely, calling longjmp in a context where a
destructor would normally run.

This series fixes these problems, and provides some other improvements
as well.

Let me know what you think.  Tested on x86-64 Fedora 29.

Tom


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/5] Make exception handling more efficient
  2019-04-25 16:53 [PATCH 0/5] More exception-handling improvements Tom Tromey
                   ` (2 preceding siblings ...)
  2019-04-25 16:53 ` [PATCH 1/5] Remove exception_none Tom Tromey
@ 2019-04-25 16:58 ` Tom Tromey
  2019-04-25 16:58 ` [PATCH 5/5] Fix memory leak in exception code Tom Tromey
  2019-04-25 18:08 ` [PATCH 0/5] More exception-handling improvements Pedro Alves
  5 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2019-04-25 16:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This makes exception handling more efficient in a few spots, through
the use of const- and rvalue-references.

I wrote this patch by commenting out the gdb_exception copy
constructor and then examining the resulting error messages one by
one, introducing the use of std::move where appropriate.

gdb/ChangeLog
2019-04-25  Tom Tromey  <tromey@adacore.com>

	* xml-support.c (struct gdb_xml_parser) <set_error>: Take an
	rvalue reference.
	(gdb_xml_start_element_wrapper, gdb_xml_end_element_wrapper)
	(gdb_xml_parser::parse): Use std::move.
	* python/python-internal.h (gdbpy_convert_exception): Take a const
	reference.
	* python/py-value.c (valpy_getitem, valpy_nonzero): Use
	std::move.
	* python/py-utils.c (gdbpy_convert_exception): Take a const
	reference.
	* python/py-inferior.c (infpy_write_memory, infpy_search_memory):
	Use std::move.
	* python/py-breakpoint.c (bppy_set_condition, bppy_set_commands):
	Use std::move.
	* mi/mi-main.c (mi_print_exception): Take a const reference.
	* main.c (handle_command_errors): Take a const reference.
	* linespec.c (parse_linespec): Use std::move.
	* infcall.c (run_inferior_call): Use std::move.
	(call_function_by_hand_dummy): Use std::move.
	* exec.c (try_open_exec_file): Use std::move.
	* exceptions.h (exception_print, exception_fprintf)
	(exception_print_same): Update.
	* exceptions.c (print_exception, exception_print)
	(exception_fprintf, exception_print_same): Change parameters to
	const reference.
	* event-top.c (gdb_rl_callback_read_char_wrapper): Update.
	* common/new-op.c: Use std::move.
	* common/common-exceptions.h (struct gdb_exception): Add move
	constructor.
	(struct gdb_exception_error, struct gdb_exception_quit, struct
	gdb_quit_bad_alloc): Change constructor to move constructor.
	(throw_exception): Change parameter to rvalue reference.
	* common/common-exceptions.c (throw_exception): Take rvalue
	reference.
	* cli/cli-interp.c (safe_execute_command): Use std::move.
	* breakpoint.c (insert_bp_location, location_to_sals): Use
	std::move.
---
 gdb/ChangeLog                  | 40 ++++++++++++++++++++++++++++++++++
 gdb/breakpoint.c               | 18 +++++++--------
 gdb/cli/cli-interp.c           |  4 ++--
 gdb/common/common-exceptions.c |  6 ++---
 gdb/common/common-exceptions.h | 18 +++++++++------
 gdb/common/new-op.c            |  4 ++--
 gdb/event-top.c                |  2 +-
 gdb/exceptions.c               |  9 ++++----
 gdb/exceptions.h               | 10 +++++----
 gdb/exec.c                     |  4 ++--
 gdb/infcall.c                  |  6 ++---
 gdb/linespec.c                 |  6 ++---
 gdb/main.c                     |  2 +-
 gdb/mi/mi-main.c               |  2 +-
 gdb/python/py-breakpoint.c     |  8 +++----
 gdb/python/py-inferior.c       |  8 +++----
 gdb/python/py-utils.c          |  2 +-
 gdb/python/py-value.c          |  8 +++----
 gdb/python/python-internal.h   |  2 +-
 gdb/xml-support.c              | 14 ++++++------
 20 files changed, 110 insertions(+), 63 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c74fc61ea42..f6d2f36d0a4 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2545,9 +2545,9 @@ insert_bp_location (struct bp_location *bl,
 	      if (val)
 		bp_excpt = gdb_exception {RETURN_ERROR, GENERIC_ERROR};
 	    }
-	  catch (const gdb_exception &e)
+	  catch (gdb_exception &e)
 	    {
-	      bp_excpt = e;
+	      bp_excpt = std::move (e);
 	    }
 	}
       else
@@ -2584,9 +2584,9 @@ insert_bp_location (struct bp_location *bl,
 			bp_excpt
 			  = gdb_exception {RETURN_ERROR, GENERIC_ERROR};
 		    }
-		  catch (const gdb_exception &e)
+		  catch (gdb_exception &e)
 		    {
-		      bp_excpt = e;
+		      bp_excpt = std::move (e);
 		    }
 
 		  if (bp_excpt.reason != 0)
@@ -2608,9 +2608,9 @@ insert_bp_location (struct bp_location *bl,
 		  if (val)
 		    bp_excpt = gdb_exception {RETURN_ERROR, GENERIC_ERROR};
 	        }
-	      catch (const gdb_exception &e)
+	      catch (gdb_exception &e)
 	        {
-		  bp_excpt = e;
+		  bp_excpt = std::move (e);
 	        }
 	    }
 	  else
@@ -13603,12 +13603,10 @@ location_to_sals (struct breakpoint *b, struct event_location *location,
     {
       sals = b->ops->decode_location (b, location, search_pspace);
     }
-  catch (const gdb_exception_error &e)
+  catch (gdb_exception_error &e)
     {
       int not_found_and_ok = 0;
 
-      exception = e;
-
       /* For pending breakpoints, it's expected that parsing will
 	 fail until the right shared library is loaded.  User has
 	 already told to create pending breakpoints and don't need
@@ -13637,6 +13635,8 @@ location_to_sals (struct breakpoint *b, struct event_location *location,
 	  b->enable_state = bp_disabled;
 	  throw;
 	}
+
+      exception = std::move (e);
     }
 
   if (exception.reason == 0 || exception.error != NOT_FOUND_ERROR)
diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index 17639d0c3f3..fc4b39a9c2a 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -367,9 +367,9 @@ safe_execute_command (struct ui_out *command_uiout, const char *command,
     {
       execute_command (command, from_tty);
     }
-  catch (const gdb_exception &exception)
+  catch (gdb_exception &exception)
     {
-      e = exception;
+      e = std::move (exception);
     }
 
   /* FIXME: cagney/2005-01-13: This shouldn't be needed.  Instead the
diff --git a/gdb/common/common-exceptions.c b/gdb/common/common-exceptions.c
index 0b96cc679da..9f210250a6f 100644
--- a/gdb/common/common-exceptions.c
+++ b/gdb/common/common-exceptions.c
@@ -180,12 +180,12 @@ throw_exception_sjlj (const struct gdb_exception &exception)
 /* Implementation of throw_exception that uses C++ try/catch.  */
 
 void
-throw_exception (const gdb_exception &exception)
+throw_exception (gdb_exception &&exception)
 {
   if (exception.reason == RETURN_QUIT)
-    throw gdb_exception_quit (exception);
+    throw gdb_exception_quit (std::move (exception));
   else if (exception.reason == RETURN_ERROR)
-    throw gdb_exception_error (exception);
+    throw gdb_exception_error (std::move (exception));
   else
     gdb_assert_not_reached ("invalid return reason");
 }
diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
index d7b25502262..ebcaf031354 100644
--- a/gdb/common/common-exceptions.h
+++ b/gdb/common/common-exceptions.h
@@ -133,6 +133,10 @@ struct gdb_exception
   {
   }
 
+  /* The move constructor exists so that we can mark it "noexcept",
+     which is a good practice for any sort of exception object.  */
+  explicit gdb_exception (gdb_exception &&other) noexcept = default;
+
   /* The copy constructor exists so that we can mark it "noexcept",
      which is a good practice for any sort of exception object.  */
   gdb_exception (const gdb_exception &other) noexcept
@@ -232,8 +236,8 @@ struct gdb_exception_error : public gdb_exception
   {
   }
 
-  explicit gdb_exception_error (const gdb_exception &ex) noexcept
-    : gdb_exception (ex)
+  explicit gdb_exception_error (gdb_exception &&ex) noexcept
+    : gdb_exception (std::move (ex))
   {
     gdb_assert (ex.reason == RETURN_ERROR);
   }
@@ -247,8 +251,8 @@ struct gdb_exception_quit : public gdb_exception
   {
   }
 
-  explicit gdb_exception_quit (const gdb_exception &ex) noexcept
-    : gdb_exception (ex)
+  explicit gdb_exception_quit (gdb_exception &&ex) noexcept
+    : gdb_exception (std::move (ex))
   {
     gdb_assert (ex.reason == RETURN_QUIT);
   }
@@ -264,8 +268,8 @@ struct gdb_quit_bad_alloc
   : public gdb_exception_quit,
     public std::bad_alloc
 {
-  explicit gdb_quit_bad_alloc (const gdb_exception &ex) noexcept
-    : gdb_exception_quit (ex),
+  explicit gdb_quit_bad_alloc (gdb_exception &&ex) noexcept
+    : gdb_exception_quit (std::move (ex)),
       std::bad_alloc ()
   {
   }
@@ -276,7 +280,7 @@ struct gdb_quit_bad_alloc
 /* Throw an exception (as described by "struct gdb_exception"),
    landing in the inner most containing exception handler established
    using TRY/CATCH.  */
-extern void throw_exception (const gdb_exception &exception)
+extern void throw_exception (gdb_exception &&exception)
      ATTRIBUTE_NORETURN;
 
 /* Throw an exception by executing a LONG JUMP to the inner most
diff --git a/gdb/common/new-op.c b/gdb/common/new-op.c
index b230f111ae7..7c5dba0be6d 100644
--- a/gdb/common/new-op.c
+++ b/gdb/common/new-op.c
@@ -64,9 +64,9 @@ operator new (std::size_t sz)
 	{
 	  malloc_failure (sz);
 	}
-      catch (const gdb_exception &ex)
+      catch (gdb_exception &ex)
 	{
-	  throw gdb_quit_bad_alloc (ex);
+	  throw gdb_quit_bad_alloc (std::move (ex));
 	}
     }
   return p;
diff --git a/gdb/event-top.c b/gdb/event-top.c
index bb8ba5cfe57..9fa46c8ad44 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -193,7 +193,7 @@ gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
 
   /* Rethrow using the normal EH mechanism.  */
   if (gdb_expt.reason < 0)
-    throw_exception (gdb_expt);
+    throw_exception (std::move (gdb_expt));
 }
 
 /* GDB's readline callback handler.  Calls the current INPUT_HANDLER,
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index 078f3c3bf00..ebdc71d98d4 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -74,7 +74,7 @@ print_flush (void)
 }
 
 static void
-print_exception (struct ui_file *file, struct gdb_exception e)
+print_exception (struct ui_file *file, const struct gdb_exception &e)
 {
   /* KLUGE: cagney/2005-01-13: Write the string out one line at a time
      as that way the MI's behavior is preserved.  */
@@ -110,7 +110,7 @@ print_exception (struct ui_file *file, struct gdb_exception e)
 }
 
 void
-exception_print (struct ui_file *file, struct gdb_exception e)
+exception_print (struct ui_file *file, const struct gdb_exception &e)
 {
   if (e.reason < 0 && e.message != NULL)
     {
@@ -120,7 +120,7 @@ exception_print (struct ui_file *file, struct gdb_exception e)
 }
 
 void
-exception_fprintf (struct ui_file *file, struct gdb_exception e,
+exception_fprintf (struct ui_file *file, const struct gdb_exception &e,
 		   const char *prefix, ...)
 {
   if (e.reason < 0 && e.message != NULL)
@@ -141,7 +141,8 @@ exception_fprintf (struct ui_file *file, struct gdb_exception e,
 /* See exceptions.h.  */
 
 int
-exception_print_same (struct gdb_exception e1, struct gdb_exception e2)
+exception_print_same (const struct gdb_exception &e1,
+		      const struct gdb_exception &e2)
 {
   const char *msg1 = e1.message == nullptr ? "" : e1.what ();
   const char *msg2 = e2.message == nullptr ? "" : e2.what ();
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index 083313b9fa9..e169b25c677 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -24,12 +24,14 @@
 
 /* If E is an exception, print it's error message on the specified
    stream.  For _fprintf, prefix the message with PREFIX...  */
-extern void exception_print (struct ui_file *file, struct gdb_exception e);
-extern void exception_fprintf (struct ui_file *file, struct gdb_exception e,
+extern void exception_print (struct ui_file *file,
+			     const struct gdb_exception &e);
+extern void exception_fprintf (struct ui_file *file,
+			       const struct gdb_exception &e,
 			       const char *prefix,
 			       ...) ATTRIBUTE_PRINTF (3, 4);
 
 /* Compare two exception objects for print equality.  */
-extern int exception_print_same (struct gdb_exception e1,
-				 struct gdb_exception e2);
+extern int exception_print_same (const struct gdb_exception &e1,
+				 const struct gdb_exception &e2);
 #endif
diff --git a/gdb/exec.c b/gdb/exec.c
index 7ff77f99160..7de92347f2e 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -167,12 +167,12 @@ try_open_exec_file (const char *exec_file_host, struct inferior *inf,
 	 exec_file_attach will clear state.  */
       exec_file_attach (exec_file_host, add_flags & SYMFILE_VERBOSE);
     }
-  catch (const gdb_exception_error &err)
+  catch (gdb_exception_error &err)
     {
       if (err.message != NULL)
 	warning ("%s", err.what ());
 
-      prev_err = err;
+      prev_err = std::move (err);
     }
 
   if (exec_file_host != NULL)
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 52f9bc907e2..af60fdc56b2 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -605,9 +605,9 @@ run_inferior_call (struct call_thread_fsm *sm,
 	 target supports asynchronous execution.  */
       wait_sync_command_done ();
     }
-  catch (const gdb_exception &e)
+  catch (gdb_exception &e)
     {
-      caught_error = e;
+      caught_error = std::move (e);
     }
 
   /* If GDB has the prompt blocked before, then ensure that it remains
@@ -1195,7 +1195,7 @@ When the function is done executing, GDB will silently stop."),
 		       e.what (), name);
 	case RETURN_QUIT:
 	default:
-	  throw_exception (e);
+	  throw_exception (std::move (e));
 	}
     }
 
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 6d26638296e..f418e03b774 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2613,9 +2613,9 @@ parse_linespec (linespec_parser *parser, const char *arg,
 	    = symtabs_from_filename (user_filename.get (),
 				     PARSER_STATE (parser)->search_pspace);
 	}
-      catch (const gdb_exception_error &ex)
+      catch (gdb_exception_error &ex)
 	{
-	  file_exception = ex;
+	  file_exception = std::move (ex);
 	}
 
       if (file_exception.reason >= 0)
@@ -2663,7 +2663,7 @@ parse_linespec (linespec_parser *parser, const char *arg,
       /* The linespec didn't parse.  Re-throw the file exception if
 	 there was one.  */
       if (file_exception.reason < 0)
-	throw_exception (file_exception);
+	throw_exception (std::move (file_exception));
 
       /* Otherwise, the symbol is not found.  */
       symbol_not_found_error (PARSER_EXPLICIT (parser)->function_name,
diff --git a/gdb/main.c b/gdb/main.c
index e67efc7bcdf..35df1e497f4 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -339,7 +339,7 @@ captured_command_loop ()
 /* Handle command errors thrown from within catch_command_errors.  */
 
 static int
-handle_command_errors (struct gdb_exception e)
+handle_command_errors (const struct gdb_exception &e)
 {
   if (e.reason < 0)
     {
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 17ca8074719..2b9883cb99f 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1875,7 +1875,7 @@ captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
 /* Print a gdb exception to the MI output stream.  */
 
 static void
-mi_print_exception (const char *token, struct gdb_exception exception)
+mi_print_exception (const char *token, const struct gdb_exception &exception)
 {
   struct mi_interp *mi = (struct mi_interp *) current_interpreter ();
 
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index dfc30f70bb2..fc9543eba0e 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -469,9 +469,9 @@ bppy_set_condition (PyObject *self, PyObject *newvalue, void *closure)
     {
       set_breakpoint_condition (self_bp->bp, exp, 0);
     }
-  catch (const gdb_exception &ex)
+  catch (gdb_exception &ex)
     {
-      except = ex;
+      except = std::move (ex);
     }
 
   GDB_PY_SET_HANDLE_EXCEPTION (except);
@@ -540,9 +540,9 @@ bppy_set_commands (PyObject *self, PyObject *newvalue, void *closure)
       counted_command_line lines = read_command_lines_1 (reader, 1, nullptr);
       breakpoint_set_commands (self_bp->bp, std::move (lines));
     }
-  catch (const gdb_exception &ex)
+  catch (gdb_exception &ex)
     {
-      except = ex;
+      except = std::move (ex);
     }
 
   GDB_PY_SET_HANDLE_EXCEPTION (except);
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 1b7e3c24917..7e7d518c557 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -574,9 +574,9 @@ infpy_write_memory (PyObject *self, PyObject *args, PyObject *kw)
     {
       write_memory_with_notification (addr, buffer, length);
     }
-  catch (const gdb_exception &ex)
+  catch (gdb_exception &ex)
     {
-      except = ex;
+      except = std::move (ex);
     }
 
   GDB_PY_HANDLE_EXCEPTION (except);
@@ -728,9 +728,9 @@ infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
 				    buffer, pattern_size,
 				    &found_addr);
     }
-  catch (const gdb_exception &ex)
+  catch (gdb_exception &ex)
     {
-      except = ex;
+      except = std::move (ex);
     }
 
   GDB_PY_HANDLE_EXCEPTION (except);
diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 9fee8817781..e07da034e34 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -229,7 +229,7 @@ gdbpy_err_fetch::type_to_string () const
    This sets the Python error indicator.  */
 
 void
-gdbpy_convert_exception (struct gdb_exception exception)
+gdbpy_convert_exception (const struct gdb_exception &exception)
 {
   PyObject *exc_class;
 
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 3349802f7fa..512e5d0220c 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1031,9 +1031,9 @@ valpy_getitem (PyObject *self, PyObject *key)
       if (res_val)
 	result = value_to_value_object (res_val);
     }
-  catch (const gdb_exception &ex)
+  catch (gdb_exception &ex)
     {
-      except = ex;
+      except = std::move (ex);
     }
 
   GDB_PY_HANDLE_EXCEPTION (except);
@@ -1498,9 +1498,9 @@ valpy_nonzero (PyObject *self)
 	/* All other values are True.  */
 	nonzero = 1;
     }
-  catch (const gdb_exception &ex)
+  catch (gdb_exception &ex)
     {
-      except = ex;
+      except = std::move (ex);
     }
 
   /* This is not documented in the Python documentation, but if this
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 449926ca874..69ff1fe30de 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -729,7 +729,7 @@ extern PyObject *gdbpy_gdb_error;
 extern PyObject *gdbpy_gdb_memory_error;
 extern PyObject *gdbpy_gdberror_exc;
 
-extern void gdbpy_convert_exception (struct gdb_exception)
+extern void gdbpy_convert_exception (const struct gdb_exception &)
     CPYCHECKER_SETS_EXCEPTION;
 
 int get_addr_from_python (PyObject *obj, CORE_ADDR *addr)
diff --git a/gdb/xml-support.c b/gdb/xml-support.c
index d4cd89c0339..ae727da03b3 100644
--- a/gdb/xml-support.c
+++ b/gdb/xml-support.c
@@ -113,9 +113,9 @@ struct gdb_xml_parser
   { m_is_xinclude = is_xinclude; }
 
   /* A thrown error, if any.  */
-  void set_error (gdb_exception error)
+  void set_error (gdb_exception &&error)
   {
-    m_error = error;
+    m_error = std::move (error);
 #ifdef HAVE_XML_STOPPARSER
     XML_StopParser (m_expat_parser, XML_FALSE);
 #endif
@@ -387,9 +387,9 @@ gdb_xml_start_element_wrapper (void *data, const XML_Char *name,
     {
       parser->start_element (name, attrs);
     }
-  catch (const gdb_exception &ex)
+  catch (gdb_exception &ex)
     {
-      parser->set_error (ex);
+      parser->set_error (std::move (ex));
     }
 }
 
@@ -459,9 +459,9 @@ gdb_xml_end_element_wrapper (void *data, const XML_Char *name)
     {
       parser->end_element (name);
     }
-  catch (const gdb_exception &ex)
+  catch (gdb_exception &ex)
     {
-      parser->set_error (ex);
+      parser->set_error (std::move (ex));
     }
 }
 
@@ -603,7 +603,7 @@ gdb_xml_parser::parse (const char *buffer)
   else
     {
       gdb_assert (m_error.reason < 0);
-      throw_exception (m_error);
+      throw_exception (std::move (m_error));
     }
 
   if (m_last_line != 0)
-- 
2.20.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 5/5] Fix memory leak in exception code
  2019-04-25 16:53 [PATCH 0/5] More exception-handling improvements Tom Tromey
                   ` (3 preceding siblings ...)
  2019-04-25 16:58 ` [PATCH 4/5] Make exception handling more efficient Tom Tromey
@ 2019-04-25 16:58 ` Tom Tromey
  2019-04-25 18:06   ` Pedro Alves
  2019-04-25 18:08 ` [PATCH 0/5] More exception-handling improvements Pedro Alves
  5 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2019-04-25 16:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

PR gdb/24475 concerns a memory leak coming from gdb's exception
handling code.

The leak occurs because throw_exception_sjlj does not arrange to
destroy the exception object it is passed.  However, because
gdb_exception has a destructor, it's undefined to longjmp in this
situation.

This patch fixes the problem by avoiding the need to run any
destructors in gdb_rl_callback_handler, by making the gdb_exception
"static".

gdb/ChangeLog
2019-04-25  Tom Tromey  <tromey@adacore.com>

	PR gdb/24475:
	* event-top.c (gdb_rl_callback_handler): Make "gdb_rl_expt"
	static.
---
 gdb/ChangeLog   | 6 ++++++
 gdb/event-top.c | 6 +++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 9fa46c8ad44..f4a7574741a 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -205,11 +205,15 @@ gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
 static void
 gdb_rl_callback_handler (char *rl) noexcept
 {
-  struct gdb_exception gdb_rl_expt;
+  /* This is static to avoid undefined behavior when calling
+     longjmp.  */
+  static struct gdb_exception gdb_rl_expt;
   struct ui *ui = current_ui;
 
   try
     {
+      /* Ensure the exception is reset on each call.  */
+      gdb_rl_expt = {};
       ui->input_handler (gdb::unique_xmalloc_ptr<char> (rl));
     }
   catch (gdb_exception &ex)
-- 
2.20.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] Fix memory leak in exception code
  2019-04-25 16:58 ` [PATCH 5/5] Fix memory leak in exception code Tom Tromey
@ 2019-04-25 18:06   ` Pedro Alves
  2019-04-25 18:18     ` Tom Tromey
  0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2019-04-25 18:06 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 4/25/19 5:52 PM, Tom Tromey wrote:
> PR gdb/24475 concerns a memory leak coming from gdb's exception
> handling code.
> 
> The leak occurs because throw_exception_sjlj does not arrange to
> destroy the exception object it is passed.  However, because
> gdb_exception has a destructor, it's undefined to longjmp in this
> situation.
> 
> This patch fixes the problem by avoiding the need to run any
> destructors in gdb_rl_callback_handler, by making the gdb_exception
> "static".
> 

Why fix it like this, instead of fixing it like in the guile patch?
I'd think you could even use a common POD type for both guile and here?

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/5] More exception-handling improvements
  2019-04-25 16:53 [PATCH 0/5] More exception-handling improvements Tom Tromey
                   ` (4 preceding siblings ...)
  2019-04-25 16:58 ` [PATCH 5/5] Fix memory leak in exception code Tom Tromey
@ 2019-04-25 18:08 ` Pedro Alves
  5 siblings, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2019-04-25 18:08 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 4/25/19 5:52 PM, Tom Tromey wrote:
> Philippe noticed a memory leak with the new exception-handling code,
> and when I debugged it, I realized that the current code is relying on
> undefined behavior: namely, calling longjmp in a context where a
> destructor would normally run.
> 
> This series fixes these problems, and provides some other improvements
> as well.
> 
> Let me know what you think.  Tested on x86-64 Fedora 29.

I think patches #1-#4 look fine.  

I'd remove the explicit "struct"s throughout while at it,
but no biggie.

Not so clear on patch #5 -- see direct reply to that one.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] Fix memory leak in exception code
  2019-04-25 18:06   ` Pedro Alves
@ 2019-04-25 18:18     ` Tom Tromey
  2019-04-25 18:30       ` Tom Tromey
  0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2019-04-25 18:18 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Why fix it like this, instead of fixing it like in the guile patch?
Pedro> I'd think you could even use a common POD type for both guile and here?

No particularly good reason.  I'll change it.

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] Fix memory leak in exception code
  2019-04-25 18:18     ` Tom Tromey
@ 2019-04-25 18:30       ` Tom Tromey
  2019-04-25 18:38         ` Pedro Alves
  0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2019-04-25 18:30 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Pedro Alves, gdb-patches

Pedro> Why fix it like this, instead of fixing it like in the guile patch?
Pedro> I'd think you could even use a common POD type for both guile and here?

Tom> No particularly good reason.  I'll change it.

Actually, there was a reason.  Converting an exception to a POD means
copying the string contents.

Perhaps that isn't a big concern.  I guess only exceptions escaping to
the top level will do this.

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] Fix memory leak in exception code
  2019-04-25 18:30       ` Tom Tromey
@ 2019-04-25 18:38         ` Pedro Alves
  2019-04-25 18:50           ` Tom Tromey
  0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2019-04-25 18:38 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 4/25/19 7:30 PM, Tom Tromey wrote:
> Pedro> Why fix it like this, instead of fixing it like in the guile patch?
> Pedro> I'd think you could even use a common POD type for both guile and here?
> 
> Tom> No particularly good reason.  I'll change it.
> 
> Actually, there was a reason.  Converting an exception to a POD means
> copying the string contents.
> 
> Perhaps that isn't a big concern.  I guess only exceptions escaping to
> the top level will do this.

Yeah, I don't think it's a concern here.  

It seemed better than adding a global, thinking about threads, but now
that I think a bit more about it, I realize that we won't ever be
able to have multiple threads call into readline simultaneously -- it's
full of global state.  So I'm fine with your patch as is, afterall.

I think it'd be good to extend the comment, something like:

-  /* This is static to avoid undefined behavior when calling
-     longjmp.  */
+  /* This is static to avoid undefined behavior when calling
+     longjmp -- gdb_exception has a dtor with side effects.  */

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] Fix memory leak in exception code
  2019-04-25 18:38         ` Pedro Alves
@ 2019-04-25 18:50           ` Tom Tromey
  0 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2019-04-25 18:50 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> It seemed better than adding a global, thinking about threads, but now
Pedro> that I think a bit more about it, I realize that we won't ever be
Pedro> able to have multiple threads call into readline simultaneously -- it's
Pedro> full of global state.  So I'm fine with your patch as is, afterall.

Alright, I'll check in that version, after making the tweaks you
requested.

thanks,
Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-04-25 18:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-25 16:53 [PATCH 0/5] More exception-handling improvements Tom Tromey
2019-04-25 16:53 ` [PATCH 2/5] Make SJLJ exceptions more efficient Tom Tromey
2019-04-25 16:53 ` [PATCH 3/5] Avoid undefined behavior in Guile exception handling Tom Tromey
2019-04-25 16:53 ` [PATCH 1/5] Remove exception_none Tom Tromey
2019-04-25 16:58 ` [PATCH 4/5] Make exception handling more efficient Tom Tromey
2019-04-25 16:58 ` [PATCH 5/5] Fix memory leak in exception code Tom Tromey
2019-04-25 18:06   ` Pedro Alves
2019-04-25 18:18     ` Tom Tromey
2019-04-25 18:30       ` Tom Tromey
2019-04-25 18:38         ` Pedro Alves
2019-04-25 18:50           ` Tom Tromey
2019-04-25 18:08 ` [PATCH 0/5] More exception-handling improvements Pedro Alves

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).