Patchwork [4/5] Make exception handling more efficient

login
register
mail settings
Submitter Tom Tromey
Date April 25, 2019, 4:52 p.m.
Message ID <20190425165256.31226-5-tromey@adacore.com>
Download mbox | patch
Permalink /patch/32416/
State New
Headers show

Comments

Tom Tromey - April 25, 2019, 4:52 p.m.
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(-)

Patch

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)