Don't preserve `uiout' across TRY_CATCH.

[Pedro Alves <pedro@codesourcery.com>]

Looking at the differences between tui/tui-interp.c:tui_command_loop
and event-top.c:cli_event_loop,
thinking of getting rid of tui_command_loop, the only difference is
that tui_command_loop inlines event-loop.c:start_event_loop, and
adds this bit:

+      /* Update gdb output according to TUI mode.  Since catch_errors
+         preserves the uiout from changing, this must be done at top
+         level of event loop.  */
+      if (tui_active)
+        current_uiout = tui_out;
+      else
+        current_uiout = tui_old_uiout;

I thought, "why don't we just use TRY_CATCH instead of
catch_errors then?".  Turns out we can't as is, because it is
TRY_CATCH itself that preserves the uiout from changing...
I think that's wrong for TRY_CATCH to do - TRY_CATCH should be a
low level exceptions framework, and it should be catch_errors and
whatever other callers that want to preserve uiout that should
guarantee uiout is preserved.  That's what the patch does.  If
it turns out there are places that flip the uiout temporarily and
we inadvertently end up at the top command loop with it still
flipped, then that calls for a missing cleanup instead, at
the place we temporarily swapped the uiout global.

I then found functions that look like:

int
catch_exceptions_with_msg (struct ui_out *uiout, ...
{
  ...
  TRY_CATCH (exception, mask)
   {
     // do stuff assuming `uiout' is the current builder
   }

Notice the subtlety.  We're relying on masking the
global `uiout' with the function's parameter.  If we
rename the parameter, TRY_CATCH would then run the
TRY_CATCH body with the global uiout instead of
temporarily switching to the incoming argument uiout...

`uiout' is not a great name for grepping for uses.  I'll post a
followup that renames it to `current_uiout'.

Tested on x86_64-linux.  Anyone see a problem with this?

-- 
Pedro Alves

2011-08-03  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* exceptions.c (struct catcher): Remove saved_uiout field.
	(exceptions_state_mc_init): Remove the `func_uiout' parameter, and
	no longer save/resvore the global ui_out builder.
	(catch_exceptions_with_msg): Save/override/restore the global
	ui_out builder manually instead of relying on TRY_CATCH to do it.
	(catch_errors): Save/restore the global ui_out builder manually
	instead of relying on TRY_CATCH to do it.
	* exceptions.h (exceptions_state_mc_init): Remove the `func_uiout'
	parameter.
	(TRY_CATCH): Adjust.
	* cli/cli-interp.c (safe_execute_command): Save/override/restore
	the global ui_out builder manually instead of relying on TRY_CATCH
	to do it.

---
 gdb/cli/cli-interp.c |   11 ++++++++++-
 gdb/exceptions.c     |   47 +++++++++++++++++++++++++++++++++++------------
 gdb/exceptions.h     |    5 ++---
 3 files changed, 47 insertions(+), 16 deletions(-)

Index: src/gdb/exceptions.c
===================================================================
--- src.orig/gdb/exceptions.c	2011-08-03 17:53:31.000000000 +0100
+++ src/gdb/exceptions.c	2011-08-03 18:08:28.876634279 +0100
@@ -60,7 +60,6 @@ struct catcher
   volatile struct gdb_exception *exception;
   /* Saved/current state.  */
   int mask;
-  struct ui_out *saved_uiout;
   struct cleanup *saved_cleanup_chain;
   /* Back link.  */
   struct catcher *prev;
@@ -70,8 +69,7 @@ struct catcher
 static struct catcher *current_catcher;
 
 EXCEPTIONS_SIGJMP_BUF *
-exceptions_state_mc_init (struct ui_out *func_uiout,
-			  volatile struct gdb_exception *exception,
+exceptions_state_mc_init (volatile struct gdb_exception *exception,
 			  return_mask mask)
 {
   struct catcher *new_catcher = XZALLOC (struct catcher);
@@ -84,10 +82,6 @@ exceptions_state_mc_init (struct ui_out
 
   new_catcher->mask = mask;
 
-  /* Override the global ``struct ui_out'' builder.  */
-  new_catcher->saved_uiout = uiout;
-  uiout = func_uiout;
-
   /* Prevent error/quit during FUNC from calling cleanups established
      prior to here.  */
   new_catcher->saved_cleanup_chain = save_cleanups ();
@@ -112,8 +106,6 @@ catcher_pop (void)
 
   restore_cleanups (old_catcher->saved_cleanup_chain);
 
-  uiout = old_catcher->saved_uiout;
-
   xfree (old_catcher);
 }
 
@@ -459,7 +451,7 @@ catch_exceptions (struct ui_out *uiout,
 }
 
 int
-catch_exceptions_with_msg (struct ui_out *uiout,
+catch_exceptions_with_msg (struct ui_out *func_uiout,
 		  	   catch_exceptions_ftype *func,
 		  	   void *func_args,
 			   char **gdberrmsg,
@@ -467,11 +459,27 @@ catch_exceptions_with_msg (struct ui_out
 {
   volatile struct gdb_exception exception;
   volatile int val = 0;
+  struct ui_out *saved_uiout;
 
-  TRY_CATCH (exception, mask)
+  /* Save and override the global ``struct ui_out'' builder.  */
+  saved_uiout = uiout;
+  uiout = func_uiout;
+
+  TRY_CATCH (exception, RETURN_MASK_ALL)
     {
       val = (*func) (uiout, func_args);
     }
+
+  /* Restore the global builder.  */
+  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);
+    }
+
   print_any_exception (gdb_stderr, NULL, exception);
   gdb_assert (val >= 0);
   gdb_assert (exception.reason <= 0);
@@ -500,11 +508,26 @@ catch_errors (catch_errors_ftype *func,
 {
   volatile int val = 0;
   volatile struct gdb_exception exception;
+  struct ui_out *saved_uiout;
 
-  TRY_CATCH (exception, mask)
+  /* Save the global ``struct ui_out'' builder.  */
+  saved_uiout = uiout;
+
+  TRY_CATCH (exception, RETURN_MASK_ALL)
     {
       val = func (func_args);
     }
+
+  /* Restore the global builder.  */
+  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);
+    }
+
   print_any_exception (gdb_stderr, errstring, exception);
   if (exception.reason != 0)
     return 0;
Index: src/gdb/exceptions.h
===================================================================
--- src.orig/gdb/exceptions.h	2011-08-03 17:53:31.000000000 +0100
+++ src/gdb/exceptions.h	2011-08-03 17:58:30.946634175 +0100
@@ -114,8 +114,7 @@ extern const struct gdb_exception except
 
 /* Functions to drive the exceptions state m/c (internal to
    exceptions).  */
-EXCEPTIONS_SIGJMP_BUF *exceptions_state_mc_init (struct ui_out *func_uiout,
-						 volatile struct
+EXCEPTIONS_SIGJMP_BUF *exceptions_state_mc_init (volatile struct
 						 gdb_exception *exception,
 						 return_mask mask);
 int exceptions_state_mc_action_iter (void);
@@ -146,7 +145,7 @@ int exceptions_state_mc_action_iter_1 (v
 #define TRY_CATCH(EXCEPTION,MASK) \
      { \
        EXCEPTIONS_SIGJMP_BUF *buf = \
-	 exceptions_state_mc_init (uiout, &(EXCEPTION), (MASK)); \
+	 exceptions_state_mc_init (&(EXCEPTION), (MASK)); \
        EXCEPTIONS_SIGSETJMP (*buf); \
      } \
      while (exceptions_state_mc_action_iter ()) \
Index: src/gdb/cli/cli-interp.c
===================================================================
--- src.orig/gdb/cli/cli-interp.c	2011-08-03 17:53:31.000000000 +0100
+++ src/gdb/cli/cli-interp.c	2011-08-03 18:08:15.186634277 +0100
@@ -112,14 +112,23 @@ cli_interpreter_exec (void *data, const
 }
 
 static struct gdb_exception
-safe_execute_command (struct ui_out *uiout, char *command, int from_tty)
+safe_execute_command (struct ui_out *command_uiout, char *command, int from_tty)
 {
   volatile struct gdb_exception e;
+  struct ui_out *saved_uiout;
+
+  /* Override the global ``struct ui_out'' builder.  */
+  saved_uiout = uiout;
+  uiout = command_uiout;
 
   TRY_CATCH (e, RETURN_MASK_ALL)
     {
       execute_command (command, from_tty);
     }
+
+  /* Restore the global builder.  */
+  uiout = saved_uiout;
+
   /* FIXME: cagney/2005-01-13: This shouldn't be needed.  Instead the
      caller should print the exception.  */
   exception_print (gdb_stderr, e);