Patchwork [1/2] Eliminate catch_errors

login
register
mail settings
Submitter Pedro Alves
Date Oct. 4, 2017, 5:08 p.m.
Message ID <1507136882-20553-2-git-send-email-palves@redhat.com>
Download mbox | patch
Permalink /patch/23333/
State New
Headers show

Comments

Pedro Alves - Oct. 4, 2017, 5:08 p.m.
If you want to use catch_errors with a function with parameters, then
currently you have to manually write a "capture" struct wrapping the
arguments and marshall/unmarshall that.

https://sourceware.org/ml/gdb-patches/2017-09/msg00834.html proposed
adjusting catch_errors to use gdb::function_view, which would allow
passing lambdas with automatic captures.  However, it seems like using
TRY/CATCH directly instead ends up producing clearer and easier to
debug code.  This is what this commit does.

Note that removing catch_errors exposes further cleanup opportunities
around no longer having to follow catch_errors callback type, and also
removes a few cleanups.

I didn't do anything to save/restore current_uiout because I think
that should be the responsibility of the code that changes
current_uiout in the first place.

(Another approach could be to make catch_errors a variadic template
like:

  template<typename Function, typename... Args>
  int catch_errors (const char *errstring, return_mask mask,
		    Function &&func, Args... args);

and then with:

  extern void function_with_args (int, int);
  extern void function_with_no_args ();

calls to the above functions would be wrapped like this:

  catch_errors ("some error happened", RETURN_MASK_ERROR,
                function_with_args, arg1, arg2);

  catch_errors ("some error happened", RETURN_MASK_ERROR,
                function_with_no_args);

but I'm thinking that that doesn't improve much if at all either.)

gdb/ChangeLog
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
	    Tom Tromey  <tom@tromey.com>

	* breakpoint.c (breakpoint_cond_eval): Change return type to bool
	and reverse logic.
	(WP_DELETED, WP_VALUE_CHANGED, WP_VALUE_NOT_CHANGED, WP_IGNORE):
	No longer macros.  Instead ...
	(enum wp_check_result): They're now values of this new
	enumeration.
	(watchpoint_check): Change return type to wp_check_result and
	parameter type to bpstat.
	(bpstat_check_watchpoint): Use TRY/CATCH instead of catch_errors.
	(bpstat_check_breakpoint_conditions): Use TRY/CATCH instead of
	catch_errors.  Reverse logic of watchpoint_check call.
	(breakpoint_re_set_one): Now returns void and takes a breakpoint
	pointer as parameter.
	(breakpoint_re_set): Use TRY/CATCH instead of catch_errors.
	* common/common-exceptions.c (throw_exception_sjlj): Update
	comments to avoid mentioning catch_errors.
	* exceptions.c (catch_errors): Delete.
	* exceptions.h: Update comments to avoid mentioning catch_errors.
	(catch_errors_ftype, catch_errors): Delete.
	* infrun.c (normal_stop): Use TRY/CATCH instead of catch_errors.
	(hook_stop_stub): Delete.
	(restore_selected_frame): Change return type to void, and
	parameter type to const frame_id &.
	(restore_infcall_control_state): Use TRY/CATCH instead of
	catch_errors.
	* main.c (captured_command_loop): Return void and remove
	parameter.  Remove references to catch_errors.
	(captured_main): Use TRY/CATCH instead of catch_errors.
	* objc-lang.c (objc_submethod_helper_data)
	(find_objc_msgcall_submethod_helper): Delete.
	(find_objc_msgcall_submethod): Use TRY/CATCH instead of
	catch_errors.
	* record-full.c (record_full_message): Return void.
	(record_full_message_args, record_full_message_wrapper): Delete.
	(record_full_message_wrapper_safe): Return bool and use TRY/CATCH
	instead of catch_errors.
	* solib-aix.c (solib_aix_open_symbol_file_object): Change
	parameter type to int.
	* solib-darwin.c (open_symbol_file_object): Ditto.
	* solib-dsbt.c (open_symbol_file_object): Ditto.
	* solib-frv.c (open_symbol_file_object): Ditto.
	* solib-svr4.c (open_symbol_file_object): Ditto.
	* solib-target.c (solib_target_open_symbol_file_object): Ditto.
	* solib.c (update_solib_list): Use TRY/CATCH instead of
	catch_errors.
	* solist.h (struct target_so_ops) <open_symbol_file_object>:
	Change type.
	* symmisc.c (struct print_symbol_args): Remove.
	(dump_symtab_1): Use TRY/CATCH instead of catch_errors.
	(print_symbol): Change type.
	* windows-nat.c (handle_load_dll, handle_unload_dll): Return void
	and remove parameters.
	(catch_errors): New.
	(get_windows_debug_event): Adjust.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* lib/selftest-support.exp (selftest_setup): Update for
	captured_command_loop's prototype change.
---
 gdb/breakpoint.c                       | 148 +++++++++++++++++----------------
 gdb/common/common-exceptions.c         |   8 +-
 gdb/exceptions.c                       |  39 ---------
 gdb/exceptions.h                       |  32 ++-----
 gdb/infrun.c                           |  58 +++++++------
 gdb/main.c                             |  23 +++--
 gdb/objc-lang.c                        |  44 +++-------
 gdb/record-full.c                      |  38 +++------
 gdb/solib-aix.c                        |   2 +-
 gdb/solib-darwin.c                     |   2 +-
 gdb/solib-dsbt.c                       |  10 +--
 gdb/solib-frv.c                        |   2 +-
 gdb/solib-svr4.c                       |  10 +--
 gdb/solib-target.c                     |   2 +-
 gdb/solib.c                            |  16 +++-
 gdb/solist.h                           |   7 +-
 gdb/symmisc.c                          |  47 ++++-------
 gdb/testsuite/lib/selftest-support.exp |   4 +-
 gdb/windows-nat.c                      |  35 +++++---
 19 files changed, 221 insertions(+), 306 deletions(-)

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 59cb354..22d1df7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -103,8 +103,6 @@  static void map_breakpoint_numbers (const char *,
 
 static void ignore_command (char *, int);
 
-static int breakpoint_re_set_one (void *);
-
 static void breakpoint_re_set_default (struct breakpoint *);
 
 static void
@@ -178,8 +176,6 @@  static void info_breakpoints_command (char *, int);
 
 static void info_watchpoints_command (char *, int);
 
-static int breakpoint_cond_eval (void *);
-
 static void cleanup_executing_breakpoints (void *);
 
 static void commands_command (char *, int);
@@ -191,8 +187,6 @@  static int remove_breakpoint_1 (struct bp_location *, enum remove_bp_reason);
 
 static enum print_stop_action print_bp_stop_message (bpstat bs);
 
-static int watchpoint_check (void *);
-
 static int hw_breakpoint_used_count (void);
 
 static int hw_watchpoint_use_count (struct breakpoint *);
@@ -4842,21 +4836,16 @@  bpstat_print (bpstat bs, int kind)
   return PRINT_UNKNOWN;
 }
 
-/* Evaluate the expression EXP and return 1 if value is zero.
-   This returns the inverse of the condition because it is called
-   from catch_errors which returns 0 if an exception happened, and if an
-   exception happens we want execution to stop.
-   The argument is a "struct expression *" that has been cast to a
-   "void *" to make it pass through catch_errors.  */
+/* Evaluate the boolean expression EXP and return the result.  */
 
-static int
-breakpoint_cond_eval (void *exp)
+static bool
+breakpoint_cond_eval (expression *exp)
 {
   struct value *mark = value_mark ();
-  int i = !value_true (evaluate_expression ((struct expression *) exp));
+  bool res = value_true (evaluate_expression (exp));
 
   value_free_to_mark (mark);
-  return i;
+  return res;
 }
 
 /* Allocate a new bpstat.  Link it to the FIFO list by BS_LINK_POINTER.  */
@@ -4966,30 +4955,31 @@  watchpoints_triggered (struct target_waitstatus *ws)
   return 1;
 }
 
-/* Possible return values for watchpoint_check (this can't be an enum
-   because of check_errors).  */
-/* The watchpoint has been deleted.  */
-#define WP_DELETED 1
-/* The value has changed.  */
-#define WP_VALUE_CHANGED 2
-/* The value has not changed.  */
-#define WP_VALUE_NOT_CHANGED 3
-/* Ignore this watchpoint, no matter if the value changed or not.  */
-#define WP_IGNORE 4
+/* Possible return values for watchpoint_check.  */
+enum wp_check_result
+  {
+    /* The watchpoint has been deleted.  */
+    WP_DELETED = 1,
+
+    /* The value has changed.  */
+    WP_VALUE_CHANGED = 2,
+
+    /* The value has not changed.  */
+    WP_VALUE_NOT_CHANGED = 3,
+
+    /* Ignore this watchpoint, no matter if the value changed or not.  */
+    WP_IGNORE = 4,
+  };
 
 #define BP_TEMPFLAG 1
 #define BP_HARDWAREFLAG 2
 
 /* Evaluate watchpoint condition expression and check if its value
-   changed.
-
-   P should be a pointer to struct bpstat, but is defined as a void *
-   in order for this function to be usable with catch_errors.  */
+   changed.  */
 
-static int
-watchpoint_check (void *p)
+static wp_check_result
+watchpoint_check (bpstat bs)
 {
-  bpstat bs = (bpstat) p;
   struct watchpoint *b;
   struct frame_info *fr;
   int within_current_scope;
@@ -5185,13 +5175,29 @@  bpstat_check_watchpoint (bpstat bs)
 
       if (must_check_value)
 	{
-	  char *message
-	    = xstrprintf ("Error evaluating expression for watchpoint %d\n",
-			  b->number);
-	  struct cleanup *cleanups = make_cleanup (xfree, message);
-	  int e = catch_errors (watchpoint_check, bs, message,
-				RETURN_MASK_ALL);
-	  do_cleanups (cleanups);
+	  wp_check_result e;
+
+	  TRY
+	    {
+	      e = watchpoint_check (bs);
+	    }
+	  CATCH (ex, RETURN_MASK_ALL)
+	    {
+	      exception_fprintf (gdb_stderr, ex,
+				 "Error evaluating expression "
+				 "for watchpoint %d\n",
+				 b->number);
+
+	      SWITCH_THRU_ALL_UIS ()
+		{
+		  printf_filtered (_("Watchpoint %d deleted.\n"),
+				   b->number);
+		}
+	      watchpoint_del_at_next_stop (b);
+	      e = WP_DELETED;
+	    }
+	  END_CATCH
+
 	  switch (e)
 	    {
 	    case WP_DELETED:
@@ -5287,18 +5293,6 @@  bpstat_check_watchpoint (bpstat bs)
 	      break;
 	    default:
 	      /* Can't happen.  */
-	    case 0:
-	      /* Error from catch_errors.  */
-	      {
-		SWITCH_THRU_ALL_UIS ()
-	          {
-		    printf_filtered (_("Watchpoint %d deleted.\n"),
-				     b->number);
-		  }
-		watchpoint_del_at_next_stop (b);
-		/* We've already printed what needs to be printed.  */
-		bs->print_it = print_it_done;
-	      }
 	      break;
 	    }
 	}
@@ -5324,7 +5318,8 @@  bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
 {
   const struct bp_location *bl;
   struct breakpoint *b;
-  int value_is_zero = 0;
+  /* Assume stop.  */
+  bool condition_result = true;
   struct expression *cond;
 
   gdb_assert (bs->stop);
@@ -5420,23 +5415,30 @@  bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
 	    within_current_scope = 0;
 	}
       if (within_current_scope)
-	value_is_zero
-	  = catch_errors (breakpoint_cond_eval, cond,
-			  "Error in testing breakpoint condition:\n",
-			  RETURN_MASK_ALL);
+	{
+	  TRY
+	    {
+	      condition_result = breakpoint_cond_eval (cond);
+	    }
+	  CATCH (ex, RETURN_MASK_ALL)
+	    {
+	      exception_fprintf (gdb_stderr, ex,
+				 "Error in testing breakpoint condition:\n");
+	    }
+	  END_CATCH
+	}
       else
 	{
 	  warning (_("Watchpoint condition cannot be tested "
 		     "in the current scope"));
 	  /* If we failed to set the right context for this
 	     watchpoint, unconditionally report it.  */
-	  value_is_zero = 0;
 	}
       /* FIXME-someday, should give breakpoint #.  */
       value_free_to_mark (mark);
     }
 
-  if (cond && value_is_zero)
+  if (cond && !condition_result)
     {
       bs->stop = 0;
     }
@@ -14148,21 +14150,16 @@  prepare_re_set_context (struct breakpoint *b)
   return make_cleanup (null_cleanup, NULL);
 }
 
-/* Reset a breakpoint given it's struct breakpoint * BINT.
-   The value we return ends up being the return value from catch_errors.
-   Unused in this case.  */
+/* Reset a breakpoint.  */
 
-static int
-breakpoint_re_set_one (void *bint)
+static void
+breakpoint_re_set_one (breakpoint *b)
 {
-  /* Get past catch_errs.  */
-  struct breakpoint *b = (struct breakpoint *) bint;
   struct cleanup *cleanups;
 
   cleanups = prepare_re_set_context (b);
   b->ops->re_set (b);
   do_cleanups (cleanups);
-  return 0;
 }
 
 /* Re-set breakpoint locations for the current program space.
@@ -14188,12 +14185,17 @@  breakpoint_re_set (void)
 
     ALL_BREAKPOINTS_SAFE (b, b_tmp)
       {
-	/* Format possible error msg.  */
-	char *message = xstrprintf ("Error in re-setting breakpoint %d: ",
-				    b->number);
-	struct cleanup *cleanups = make_cleanup (xfree, message);
-	catch_errors (breakpoint_re_set_one, b, message, RETURN_MASK_ALL);
-	do_cleanups (cleanups);
+	TRY
+	  {
+	    breakpoint_re_set_one (b);
+	  }
+	CATCH (ex, RETURN_MASK_ALL)
+	  {
+	    exception_fprintf (gdb_stderr, ex,
+			       "Error in re-setting breakpoint %d: ",
+			       b->number);
+	  }
+	END_CATCH
       }
     set_language (save_language);
     input_radix = save_input_radix;
diff --git a/gdb/common/common-exceptions.c b/gdb/common/common-exceptions.c
index 6b7d5ec..6b8534c 100644
--- a/gdb/common/common-exceptions.c
+++ b/gdb/common/common-exceptions.c
@@ -264,16 +264,16 @@  gdb_exception_sliced_copy (struct gdb_exception *to, const struct gdb_exception
 
 #endif /* !GDB_XCPT_SJMP */
 
-/* Return EXCEPTION to the nearest containing catch_errors().  */
+/* Return EXCEPTION to the nearest containing CATCH_SJLJ block.  */
 
 void
 throw_exception_sjlj (struct gdb_exception exception)
 {
   do_cleanups (all_cleanups ());
 
-  /* Jump to the containing catch_errors() call, communicating REASON
-     to that call via setjmp's return value.  Note that REASON can't
-     be zero, by definition in defs.h.  */
+  /* 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);
   current_catcher->exception = exception;
   longjmp (current_catcher->buf, exception.reason);
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index dd11b6e..8b1ce58 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -217,45 +217,6 @@  catch_exceptions_with_msg (struct ui_out *func_uiout,
   return val;
 }
 
-/* This function is superseded by catch_exceptions().  */
-
-int
-catch_errors (catch_errors_ftype *func, void *func_args,
-	      const char *errstring, return_mask mask)
-{
-  struct gdb_exception exception = exception_none;
-  volatile int val = 0;
-  struct ui_out *saved_uiout;
-
-  /* Save the global ``struct ui_out'' builder.  */
-  saved_uiout = current_uiout;
-
-  TRY
-    {
-      val = func (func_args);
-    }
-  CATCH (ex, RETURN_MASK_ALL)
-    {
-      exception = ex;
-    }
-  END_CATCH
-
-  /* Restore the global builder.  */
-  current_uiout = saved_uiout;
-
-  if (exception.reason < 0 && (mask & RETURN_MASK (exception.reason)) == 0)
-    {
-      /* The caller didn't request that the event be caught.
-	 Rethrow.  */
-      throw_exception (exception);
-    }
-
-  exception_fprintf (gdb_stderr, exception, "%s", errstring);
-  if (exception.reason != 0)
-    return 0;
-  return val;
-}
-
 /* See exceptions.h.  */
 
 int
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index b2cdee3..37b8a19 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -48,21 +48,16 @@  extern void exception_fprintf (struct ui_file *file, struct gdb_exception e,
    copy of the gdb error message.  This is used when a silent error is 
    issued and the caller wants to manually issue the error message.
 
-   MASK specifies what to catch; it is normally set to
-   RETURN_MASK_ALL, if for no other reason than that the code which
-   calls catch_errors might not be set up to deal with a quit which
-   isn't caught.  But if the code can deal with it, it generally
-   should be RETURN_MASK_ERROR, unless for some reason it is more
-   useful to abort only the portion of the operation inside the
-   catch_errors.  Note that quit should return to the command line
+   MASK specifies what to catch; it is normally set to RETURN_MASK_ALL
+   if the code which calls catch_exceptions is not set up to deal with
+   a quit which isn't caught.  But if the code can deal with it, it
+   generally should be RETURN_MASK_ERROR, unless for some reason it is
+   more useful to abort only the portion of the operation inside the
+   catch_exceptions.  Note that quit should return to the command line
    fairly quickly, even if some further processing is being done.
 
    FIXME; cagney/2001-08-13: The need to override the global UIOUT
-   builder variable should just go away.
-
-   This function supersedes catch_errors().
-
-   This function uses SETJMP() and LONGJUMP().  */
+   builder variable should just go away.  */
 
 struct ui_out;
 typedef int (catch_exceptions_ftype) (struct ui_out *ui_out, void *args);
@@ -76,19 +71,6 @@  extern int catch_exceptions_with_msg (struct ui_out *uiout,
 			     	      char **gdberrmsg,
 				      return_mask mask);
 
-/* If CATCH_ERRORS_FTYPE throws an error, catch_errors() returns zero
-   otherwize the result from CATCH_ERRORS_FTYPE is returned.  It is
-   probably useful for CATCH_ERRORS_FTYPE to always return a non-zero
-   value.  It's unfortunate that, catch_errors() does not return an
-   indication of the exact exception that it caught - quit_flag might
-   help.
-
-   This function is superseded by catch_exceptions().  */
-
-typedef int (catch_errors_ftype) (void *);
-extern int catch_errors (catch_errors_ftype *, void *,
-			 const char *, return_mask);
-
 /* Compare two exception objects for print equality.  */
 extern int exception_print_same (struct gdb_exception e1,
 				 struct gdb_exception e2);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index e82f61f..1bec116 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -80,10 +80,6 @@  static void sig_print_header (void);
 
 static void resume_cleanups (void *);
 
-static int hook_stop_stub (void *);
-
-static int restore_selected_frame (void *);
-
 static int follow_fork (void);
 
 static int follow_fork_inferior (int follow_child, int detach_fork);
@@ -8311,8 +8307,16 @@  normal_stop (void)
       struct cleanup *old_chain
 	= make_cleanup (release_stop_context_cleanup, saved_context);
 
-      catch_errors (hook_stop_stub, stop_command,
-		    "Error while running hook_stop:\n", RETURN_MASK_ALL);
+      TRY
+	{
+	  execute_cmd_pre_hook (stop_command);
+	}
+      CATCH (ex, RETURN_MASK_ALL)
+	{
+	  exception_fprintf (gdb_stderr, ex,
+			     "Error while running hook_stop:\n");
+	}
+      END_CATCH
 
       /* If the stop hook resumes the target, then there's no point in
 	 trying to notify about the previous stop; its context is
@@ -8353,13 +8357,6 @@  normal_stop (void)
 
   return 0;
 }
-
-static int
-hook_stop_stub (void *cmd)
-{
-  execute_cmd_pre_hook ((struct cmd_list_element *) cmd);
-  return (0);
-}
 
 int
 signal_stop_state (int signo)
@@ -8980,25 +8977,20 @@  save_infcall_control_state (void)
   return inf_status;
 }
 
-static int
-restore_selected_frame (void *args)
+static void
+restore_selected_frame (const frame_id &fid)
 {
-  struct frame_id *fid = (struct frame_id *) args;
-  struct frame_info *frame;
-
-  frame = frame_find_by_id (*fid);
+  frame_info *frame = frame_find_by_id (fid);
 
   /* If inf_status->selected_frame_id is NULL, there was no previously
      selected frame.  */
   if (frame == NULL)
     {
       warning (_("Unable to restore previously selected frame."));
-      return 0;
+      return;
     }
 
   select_frame (frame);
-
-  return (1);
 }
 
 /* Restore inferior session state to INF_STATUS.  */
@@ -9028,16 +9020,22 @@  restore_infcall_control_state (struct infcall_control_state *inf_status)
 
   if (target_has_stack)
     {
-      /* The point of catch_errors is that if the stack is clobbered,
+      /* The point of the try/catch is that if the stack is clobbered,
          walking the stack might encounter a garbage pointer and
          error() trying to dereference it.  */
-      if (catch_errors
-	  (restore_selected_frame, &inf_status->selected_frame_id,
-	   "Unable to restore previously selected frame:\n",
-	   RETURN_MASK_ERROR) == 0)
-	/* Error in restoring the selected frame.  Select the innermost
-	   frame.  */
-	select_frame (get_current_frame ());
+      TRY
+	{
+	  restore_selected_frame (inf_status->selected_frame_id);
+	}
+      CATCH (ex, RETURN_MASK_ERROR)
+	{
+	  exception_fprintf (gdb_stderr, ex,
+			     "Unable to restore previously selected frame:\n");
+	  /* Error in restoring the selected frame.  Select the
+	     innermost frame.  */
+	  select_frame (get_current_frame ());
+	}
+      END_CATCH
     }
 
   xfree (inf_status);
diff --git a/gdb/main.c b/gdb/main.c
index 79f14b7..beb8203 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -305,11 +305,10 @@  setup_alternate_signal_stack (void)
 #endif
 }
 
-/* Call command_loop.  If it happens to return, pass that through as a
-   non-zero return status.  */
+/* Call command_loop.  */
 
-static int
-captured_command_loop (void *data)
+static void
+captured_command_loop ()
 {
   struct ui *ui = current_ui;
 
@@ -333,11 +332,9 @@  captured_command_loop (void *data)
      check to detect bad FUNCs code.  */
   do_cleanups (all_cleanups ());
   /* If the command_loop returned, normally (rather than threw an
-     error) we try to quit.  If the quit is aborted, catch_errors()
-     which called this catch the signal and restart the command
-     loop.  */
+     error) we try to quit.  If the quit is aborted, our caller
+     catches the signal and restarts the command loop.  */
   quit_command (NULL, ui->instream == ui->stdin_stream);
-  return 1;
 }
 
 /* Handle command errors thrown from within catch_command_errors.  */
@@ -1145,7 +1142,15 @@  captured_main (void *data)
      change - SET_TOP_LEVEL() - has been eliminated.  */
   while (1)
     {
-      catch_errors (captured_command_loop, 0, "", RETURN_MASK_ALL);
+      TRY
+	{
+	  captured_command_loop ();
+	}
+      CATCH (ex, RETURN_MASK_ALL)
+	{
+	  exception_print (gdb_stderr, ex);
+	}
+      END_CATCH
     }
   /* No exit -- exit is through quit_command.  */
 }
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index dc9c934..0423832 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -1310,43 +1310,25 @@  find_objc_msgsend (void)
  * dependent modules.
  */
 
-struct objc_submethod_helper_data {
-  int (*f) (CORE_ADDR, CORE_ADDR *);
-  CORE_ADDR pc;
-  CORE_ADDR *new_pc;
-};
-
-static int 
-find_objc_msgcall_submethod_helper (void * arg)
-{
-  struct objc_submethod_helper_data *s = 
-    (struct objc_submethod_helper_data *) arg;
-
-  if (s->f (s->pc, s->new_pc) == 0) 
-    return 1;
-  else 
-    return 0;
-}
-
 static int 
 find_objc_msgcall_submethod (int (*f) (CORE_ADDR, CORE_ADDR *),
 			     CORE_ADDR pc, 
 			     CORE_ADDR *new_pc)
 {
-  struct objc_submethod_helper_data s;
-
-  s.f = f;
-  s.pc = pc;
-  s.new_pc = new_pc;
+  TRY
+    {
+      if (f (pc, new_pc) == 0)
+	return 1;
+    }
+  CATCH (ex, RETURN_MASK_ALL)
+    {
+      exception_fprintf (gdb_stderr, ex,
+			 "Unable to determine target of "
+			 "Objective-C method call (ignoring):\n");
+    }
+  END_CATCH
 
-  if (catch_errors (find_objc_msgcall_submethod_helper,
-		    (void *) &s,
-		    "Unable to determine target of "
-		    "Objective-C method call (ignoring):\n",
-		    RETURN_MASK_ALL) == 0) 
-    return 1;
-  else 
-    return 0;
+  return 0;
 }
 
 int 
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 5bd8900..a3283d3 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -565,7 +565,7 @@  record_full_arch_list_cleanups (void *ignore)
    record the running message of inferior and set them to
    record_full_arch_list, and add it to record_full_list.  */
 
-static int
+static void
 record_full_message (struct regcache *regcache, enum gdb_signal signal)
 {
   int ret;
@@ -633,36 +633,24 @@  record_full_message (struct regcache *regcache, enum gdb_signal signal)
     record_full_list_release_first ();
   else
     record_full_insn_num++;
-
-  return 1;
-}
-
-struct record_full_message_args {
-  struct regcache *regcache;
-  enum gdb_signal signal;
-};
-
-static int
-record_full_message_wrapper (void *args)
-{
-  struct record_full_message_args *record_full_args
-    = (struct record_full_message_args *) args;
-
-  return record_full_message (record_full_args->regcache,
-			      record_full_args->signal);
 }
 
-static int
+static bool
 record_full_message_wrapper_safe (struct regcache *regcache,
 				  enum gdb_signal signal)
 {
-  struct record_full_message_args args;
-
-  args.regcache = regcache;
-  args.signal = signal;
+  TRY
+    {
+      record_full_message (regcache, signal);
+    }
+  CATCH (ex, RETURN_MASK_ALL)
+    {
+      exception_print (gdb_stderr, ex);
+      return false;
+    }
+  END_CATCH
 
-  return catch_errors (record_full_message_wrapper, &args, "",
-		       RETURN_MASK_ALL);
+  return true;
 }
 
 /* Set to 1 if record_full_store_registers and record_full_xfer_partial
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index 633c9e6..9233e78 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -589,7 +589,7 @@  solib_aix_current_sos (void)
 /* Implement the "open_symbol_file_object" target_so_ops method.  */
 
 static int
-solib_aix_open_symbol_file_object (void *from_ttyp)
+solib_aix_open_symbol_file_object (int from_tty)
 {
   return 0;
 }
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index 04bbf86..f63f924 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -222,7 +222,7 @@  find_program_interpreter (void)
     Note that darwin-nat.c implements pid_to_exec_file.  */
 
 static int
-open_symbol_file_object (void *from_ttyp)
+open_symbol_file_object (int from_tty)
 {
   return 0;
 }
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index 86b9e30..0b4055c 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -507,16 +507,10 @@  scan_dyntag (int dyntag, bfd *abfd, CORE_ADDR *ptr)
   return 0;
 }
 
-/* If no open symbol file, attempt to locate and open the main symbol
-   file.
-
-   If FROM_TTYP dereferences to a non-zero integer, allow messages to
-   be printed.  This parameter is a pointer rather than an int because
-   open_symbol_file_object is called via catch_errors and
-   catch_errors requires a pointer argument. */
+/* See solist.h. */
 
 static int
-open_symbol_file_object (void *from_ttyp)
+open_symbol_file_object (int from_tty)
 {
   /* Unimplemented.  */
   return 0;
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 6d74652..fd748f4 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -246,7 +246,7 @@  static int enable_break2 (void);
 /* Implement the "open_symbol_file_object" target_so_ops method.  */
 
 static int
-open_symbol_file_object (void *from_ttyp)
+open_symbol_file_object (int from_tty)
 {
   /* Unimplemented.  */
   return 0;
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 405de37..d334791 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -984,20 +984,14 @@  svr4_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
   return (name_lm >= vaddr && name_lm < vaddr + size);
 }
 
-/* Implement the "open_symbol_file_object" target_so_ops method.
-
-   If no open symbol file, attempt to locate and open the main symbol
-   file.  On SVR4 systems, this is the first link map entry.  If its
-   name is here, we can open it.  Useful when attaching to a process
-   without first loading its symbol file.  */
+/* See solist.h.  */
 
 static int
-open_symbol_file_object (void *from_ttyp)
+open_symbol_file_object (int from_tty)
 {
   CORE_ADDR lm, l_name;
   char *filename;
   int errcode;
-  int from_tty = *(int *)from_ttyp;
   struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
   struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
   int l_name_size = TYPE_LENGTH (ptr_type);
diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index dcd0f85..cf569ec 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -452,7 +452,7 @@  Could not relocate shared library \"%s\": bad offsets"), so->so_name);
 }
 
 static int
-solib_target_open_symbol_file_object (void *from_ttyp)
+solib_target_open_symbol_file_object (int from_tty)
 {
   /* We can't locate the main symbol file based on the target's
      knowledge; the user has to specify it.  */
diff --git a/gdb/solib.c b/gdb/solib.c
index 4f7fd90..e605e6b 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -760,9 +760,19 @@  update_solib_list (int from_tty)
 	 have not opened a symbol file, we may be able to get its
 	 symbols now!  */
       if (inf->attach_flag && symfile_objfile == NULL)
-	catch_errors (ops->open_symbol_file_object, &from_tty,
-		      "Error reading attached process's symbol file.\n",
-		      RETURN_MASK_ALL);
+	{
+	  TRY
+	    {
+	      ops->open_symbol_file_object (from_tty);
+	    }
+	  CATCH (ex, RETURN_MASK_ALL)
+	    {
+	      exception_fprintf (gdb_stderr, ex,
+				 "Error reading attached "
+				 "process's symbol file.\n");
+	    }
+	  END_CATCH
+	}
     }
 
   /* GDB and the inferior's dynamic linker each maintain their own
diff --git a/gdb/solist.h b/gdb/solist.h
index 5eb2d39..992986e 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -121,11 +121,8 @@  struct target_so_ops
     struct so_list *(*current_sos) (void);
 
     /* Find, open, and read the symbols for the main executable.  If
-       FROM_TTYP dereferences to a non-zero integer, allow messages to
-       be printed.  This parameter is a pointer rather than an int
-       because open_symbol_file_object is called via catch_errors and
-       catch_errors requires a pointer argument.  */
-    int (*open_symbol_file_object) (void *from_ttyp);
+       FROM_TTY is non-zero, allow messages to be printed.  */
+    int (*open_symbol_file_object) (int from_ttyp);
 
     /* Determine if PC lies in the dynamic symbol resolution code of
        the run time loader.  */
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 86422e7..ed2e8d2 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -54,15 +54,8 @@  FILE *std_err;
 
 static int block_depth (struct block *);
 
-struct print_symbol_args
-  {
-    struct gdbarch *gdbarch;
-    struct symbol *symbol;
-    int depth;
-    struct ui_file *outfile;
-  };
-
-static int print_symbol (void *);
+static void print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
+			  int depth, ui_file *outfile);
 
 
 void
@@ -357,14 +350,16 @@  dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
 	     block, not any blocks from included symtabs.  */
 	  ALL_DICT_SYMBOLS (BLOCK_DICT (b), iter, sym)
 	    {
-	      struct print_symbol_args s;
-
-	      s.gdbarch = gdbarch;
-	      s.symbol = sym;
-	      s.depth = depth + 1;
-	      s.outfile = outfile;
-	      catch_errors (print_symbol, &s, "Error printing symbol:\n",
-			    RETURN_MASK_ERROR);
+	      TRY
+		{
+		  print_symbol (gdbarch, sym, depth + 1, outfile);
+		}
+	      CATCH (ex, RETURN_MASK_ERROR)
+		{
+		  exception_fprintf (gdb_stderr, ex,
+				     "Error printing symbol:\n");
+		}
+	      END_CATCH
 	    }
 	}
       fprintf_filtered (outfile, "\n");
@@ -515,18 +510,12 @@  maintenance_print_symbols (const char *args, int from_tty)
     }
 }
 
-/* Print symbol ARGS->SYMBOL on ARGS->OUTFILE.  ARGS->DEPTH says how
-   far to indent.  ARGS is really a struct print_symbol_args *, but is
-   declared as char * to get it past catch_errors.  Returns 0 for error,
-   1 for success.  */
+/* Print symbol SYMBOL on OUTFILE.  DEPTH says how far to indent.  */
 
-static int
-print_symbol (void *args)
+static void
+print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
+	      int depth, ui_file *outfile)
 {
-  struct gdbarch *gdbarch = ((struct print_symbol_args *) args)->gdbarch;
-  struct symbol *symbol = ((struct print_symbol_args *) args)->symbol;
-  int depth = ((struct print_symbol_args *) args)->depth;
-  struct ui_file *outfile = ((struct print_symbol_args *) args)->outfile;
   struct obj_section *section;
 
   if (SYMBOL_OBJFILE_OWNED (symbol))
@@ -546,8 +535,9 @@  print_symbol (void *args)
 					    section->the_bfd_section));
       else
 	fprintf_filtered (outfile, "\n");
-      return 1;
+      return;
     }
+
   if (SYMBOL_DOMAIN (symbol) == STRUCT_DOMAIN)
     {
       if (TYPE_TAG_NAME (SYMBOL_TYPE (symbol)))
@@ -694,7 +684,6 @@  print_symbol (void *args)
 	}
     }
   fprintf_filtered (outfile, "\n");
-  return 1;
 }
 
 static void
diff --git a/gdb/testsuite/lib/selftest-support.exp b/gdb/testsuite/lib/selftest-support.exp
index 18d32dd..987b5c8 100644
--- a/gdb/testsuite/lib/selftest-support.exp
+++ b/gdb/testsuite/lib/selftest-support.exp
@@ -88,10 +88,10 @@  proc selftest_setup { executable function } {
 
     set description "run until breakpoint at $function"
     gdb_test_multiple "run $INTERNAL_GDBFLAGS" "$description" {
-        -re "Starting program.*Breakpoint \[0-9\]+,.*$function .data.* at .*main.c:.*$gdb_prompt $" {
+        -re "Starting program.*Breakpoint \[0-9\]+,.*$function \\(\\).* at .*main.c:.*$gdb_prompt $" {
             pass "$description"
         }
-        -re "Starting program.*Breakpoint \[0-9\]+,.*$function .data.*$gdb_prompt $" {
+        -re "Starting program.*Breakpoint \[0-9\]+,.*$function \\(\\).*$gdb_prompt $" {
             xfail "$description (line numbers scrambled?)"
         }
 	-re "Starting program.*Breakpoint \[0-9\]+,.* at .*main.c:.*$function.*$gdb_prompt $" {
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 1ef38fb..e30b8b5 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -755,8 +755,8 @@  get_image_name (HANDLE h, void *address, int unicode)
    do_initial_windows_stuff and windows_add_all_dlls for more info
    on how we handle DLL loading during that phase).  */
 
-static int
-handle_load_dll (void *dummy)
+static void
+handle_load_dll ()
 {
   LOAD_DLL_DEBUG_INFO *event = &current_event.u.LoadDll;
   char *dll_name;
@@ -769,7 +769,7 @@  handle_load_dll (void *dummy)
   dll_name = get_image_name (current_process_handle,
 			     event->lpImageName, event->fUnicode);
   if (!dll_name)
-    return 1;
+    return;
 
   solib_end->next = windows_make_so (dll_name, event->lpBaseOfDll);
   solib_end = solib_end->next;
@@ -778,8 +778,6 @@  handle_load_dll (void *dummy)
 
   DEBUG_EVENTS (("gdb: Loading dll \"%s\" at %s.\n", solib_end->so_name,
 		 host_address_to_string (li->load_addr)));
-
-  return 1;
 }
 
 static void
@@ -799,8 +797,8 @@  windows_free_so (struct so_list *so)
    do_initial_windows_stuff and windows_add_all_dlls for more info
    on how we handle DLL loading during that phase).  */
 
-static int
-handle_unload_dll (void *dummy)
+static void
+handle_unload_dll ()
 {
   LPVOID lpBaseOfDll = current_event.u.UnloadDll.lpBaseOfDll;
   struct so_list *so;
@@ -819,7 +817,7 @@  handle_unload_dll (void *dummy)
 	  DEBUG_EVENTS (("gdb: Unloading dll \"%s\".\n", sodel->so_name));
 
 	  windows_free_so (sodel);
-	  return 1;
+	  return;
 	}
     }
 
@@ -832,8 +830,23 @@  handle_unload_dll (void *dummy)
      32bit and 64bit worlds).  */
   complaint (&symfile_complaints, _("dll starting at %s not found."),
 	     host_address_to_string (lpBaseOfDll));
+}
 
-  return 0;
+/* Call FUNC wrapped in a TRY/CATCH that swallows all GDB
+   exceptions.  */
+
+static void
+catch_errors (void (*func) ())
+{
+  TRY
+    {
+      func ();
+    }
+  CATCH (ex, RETURN_MASK_ALL)
+    {
+      exception_print (gdb_stderr, ex);
+    }
+  END_CATCH
 }
 
 /* Clear list of loaded DLLs.  */
@@ -1541,7 +1554,7 @@  get_windows_debug_event (struct target_ops *ops,
       CloseHandle (current_event.u.LoadDll.hFile);
       if (saw_create != 1 || ! windows_initialization_done)
 	break;
-      catch_errors (handle_load_dll, NULL, (char *) "", RETURN_MASK_ALL);
+      catch_errors (handle_load_dll);
       ourstatus->kind = TARGET_WAITKIND_LOADED;
       ourstatus->value.integer = 0;
       thread_id = main_thread_id;
@@ -1554,7 +1567,7 @@  get_windows_debug_event (struct target_ops *ops,
 		     "UNLOAD_DLL_DEBUG_EVENT"));
       if (saw_create != 1 || ! windows_initialization_done)
 	break;
-      catch_errors (handle_unload_dll, NULL, (char *) "", RETURN_MASK_ALL);
+      catch_errors (handle_unload_dll);
       ourstatus->kind = TARGET_WAITKIND_LOADED;
       ourstatus->value.integer = 0;
       thread_id = main_thread_id;