Patch: catch_errors() cleanup chain restore

[nsd@cygnus.com]

Hi,

If a quit happens within catch_errors(RETURN_MASK_ERROR), the cleanup
chain gets lost.

This is the offending sequence of events:
  1. catch_errors() calls save_cleanups(), which zeros the cleanup chain.
  2. catch_errors() redirects the error longjmp target to a location that
     will call restore_cleanups(), but doesn't redirect the quit longjmp
     target similarly when RETURN_MASK_ERROR is specified.
  3. A subsequent quit event longjmps back to top level, skipping past
     the restore_cleanups() call in catch_errors().

An error within catch_errors(RETURN_MASK_QUIT) would have the same effect,
but there aren't any such calls in gdb at the moment.

It's easy to fix by:
  1. unconditionally redirecting both the error and quit longjmp targets;
  2. calling the function argument;
  3. restoring the cleanup chain;
  4. calling return_to_top_level() if appropriate.

I've appended a patch to do that.  Comments welcome,

Nick Duffek
nsd@cygnus.com

[patch follows]

Index: top.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/top.c,v
retrieving revision 2.176
diff -u -r2.176 top.c
--- top.c	2000/02/09 03:28:18	2.176
+++ top.c	2000/02/11 20:28:52
@@ -580,61 +580,65 @@
      char *errstring;
      return_mask mask;
 {
-  SIGJMP_BUF saved_error;
-  SIGJMP_BUF saved_quit;
-  SIGJMP_BUF tmp_jmp;
+  SIGJMP_BUF saved_error, tmp_error;
+  SIGJMP_BUF saved_quit, tmp_quit;
   int val;
   struct cleanup *saved_cleanup_chain;
   char *saved_error_pre_print;
   char *saved_quit_pre_print;
+  int caught = 0;
 
   saved_cleanup_chain = save_cleanups ();
   saved_error_pre_print = error_pre_print;
   saved_quit_pre_print = quit_pre_print;
 
+  memcpy ((char *) saved_error, (char *) error_return, sizeof (SIGJMP_BUF));
   if (mask & RETURN_MASK_ERROR)
-    {
-      memcpy ((char *) saved_error, (char *) error_return, sizeof (SIGJMP_BUF));
-      error_pre_print = errstring;
-    }
+    error_pre_print = errstring;
+
+  memcpy (saved_quit, quit_return, sizeof (SIGJMP_BUF));
   if (mask & RETURN_MASK_QUIT)
+    quit_pre_print = errstring;
+
+  if (SIGSETJMP (tmp_error) == 0)
+    memcpy (error_return, tmp_error, sizeof (SIGJMP_BUF));
+  else
     {
-      memcpy (saved_quit, quit_return, sizeof (SIGJMP_BUF));
-      quit_pre_print = errstring;
+      caught = RETURN_MASK_ERROR;
+      goto called;
     }
 
-  if (SIGSETJMP (tmp_jmp) == 0)
+  if (SIGSETJMP (tmp_quit) == 0)
+    memcpy (quit_return, tmp_quit, sizeof (SIGJMP_BUF));
+  else
     {
-      if (mask & RETURN_MASK_ERROR)
-	memcpy (error_return, tmp_jmp, sizeof (SIGJMP_BUF));
-      if (mask & RETURN_MASK_QUIT)
-	memcpy (quit_return, tmp_jmp, sizeof (SIGJMP_BUF));
-      val = (*func) (args);
-      /* FIXME: cagney/1999-11-05: A correct FUNC implementaton will
-         clean things up (restoring the cleanup chain) to the state
-         they were just prior to the call.  Technically, this means
-         that the below restore_cleanups call is redundant.
-         Unfortunatly, many FUNC's are not that well behaved.
-         restore_cleanups should either be replaced with a do_cleanups
-         call (to cover the problem) or an assertion check to detect
-         bad FUNCs code. */
+      caught = RETURN_MASK_QUIT;
+      goto called;
     }
-  else
-    val = 0;
 
+  val = (*func) (args);
+ called:
   restore_cleanups (saved_cleanup_chain);
 
+  memcpy (error_return, saved_error, sizeof (SIGJMP_BUF));
   if (mask & RETURN_MASK_ERROR)
-    {
-      memcpy (error_return, saved_error, sizeof (SIGJMP_BUF));
-      error_pre_print = saved_error_pre_print;
-    }
+    error_pre_print = saved_error_pre_print;
+
+  memcpy (quit_return, saved_quit, sizeof (SIGJMP_BUF));
   if (mask & RETURN_MASK_QUIT)
+    quit_pre_print = saved_quit_pre_print;
+
+  if (!caught)
+    return val;
+
+  if (!(mask & caught))
     {
-      memcpy (quit_return, saved_quit, sizeof (SIGJMP_BUF));
-      quit_pre_print = saved_quit_pre_print;
+      if (caught == RETURN_MASK_ERROR)
+	return_to_top_level (RETURN_ERROR);
+      else
+	return_to_top_level (RETURN_QUIT);
     }
-  return val;
+  return 0;
 }
 
 struct captured_command_args