This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA] exec_cleanup murder.


Presently, async mode uses to mechanisms to execute code after
the target is stopped -- continuation and exec_cleanup. Continuations
are installed with the add_continuation function.  They are executed
with the do_all_continuations function, commonly called from 
inferior_event_handler  when the target finally stops. In case of error 
from the the target, discard_all_continuations is called.

The exec_cleanup mechanism is similar to the standard cleanup mechanism.
Such exec cleanups are set in commands that run the targets.  The cleanups
are run in the continuations, and in addition, all cleanups are run in
throw_exception (if the target is not running).

The common form of code is like this:
   
  void some_command()
  { 
    if (target_async_p())
       cleanup = make_exec_cleanup (...);
    else
       cleanup = make_cleanup (...);
    ...
    add_continuation (some_continuation_function);
    <resume-target>
    if (!target_async_p())
        do_cleanups (cleanup);
  }

  void some_cleanup_function()
  {
    do_exec_cleanups (...);
  }	
   
For a concrete example, take a look at finish_command.

There is a number of problems with this arrangement.

1. If an exception is thrown, and not caught, between adding continuation,
and actually resuming target, we'll end up with target not running, but
continuation still installed. It will happily run the next time the target is
stopped.

2. If an exception is thrown before the target is resumed, then even if
the exception is caught, throw_exception will run all exec_cleanups. What
happens when the continuation tries to run the same cleanup is easily
imaginable.

3. If the target is successfully resumed, then stops, but we get
an error fetching event from it, then inferior_event_handler will
discard all continuations, but won't run exec_cleanups.

Now, I could try to address those points, however:

- Having two mechanisms for basically the same thing (calling code
after target stops) is not the best design. It does not actually
helps much, because in most cases the call to do_exec_cleanups
inside continuation can be replaced with code that just does
whatever needed. It actually hurts, because currently continuations
and exec_cleanups are run/discarded in different places, so it's
pretty hard to understand if that logic is correct.

- For non-stop mode, we definitely don't want continuations to be
run if a breakpoint is hit in unrelated thread. Say, if I do 'finish'
in one thread, and another thread hits breakpoint, I do want 'finish'
to go on.  This requires that continuations be per-thread, and making
exec cleanups be per-thread too is extra work.

What I propose is:

- continuation is the only mechanism for running code after the target
stops. A continuation callback gets extra parameter that tells if
an error is occurred. If this parameter is true, the continuation
just cleans up, and does not do any regular processing it would do on
success.

- continuation should be installed only when the target was
successfully resumed.

- once continuation is installed, GDB core guarantees that the continuation
will be called on target stop -- either with '0' or '1' for the error
parameter. It's not hard to guarantee -- inferior_event_handler is the
single function called to handle target stop, and if anything called from
it throws, we can detect that and call continuations with 1 for 'error'.

Then, the standard code for async commands should be:

    cleanup = make_cleanup (...);
    ...
    <resume-target>
    if (target_async_p ())
    {
        /* If we're not executing, an exception should have been thrown.  */
        gdb_assert (target_executing);
        /* Continuation will clean up.  */
        discard_cleanups (cleanup);
        add_continuation (...);
    }
    else
        do_cleanups (cleanup);

This patch does exactly that, and causes no regressions on x86 in sync
and async modes. We also used this patch in our non-stop work, and have not
found any issues.

OK?

- Volodya

	* breakpoint.c (until_break_command_continuation): Add
	the 'error' parameter.  Directly delete the breakoint as
	opposed to running cleanups.
	(until_break_command): Install continuation only
	after starting the target.  Don't use exec cleanups,
	use ordinary cleanups.  Discard cleanups is successfully
	started the target in async mode.
	(make_cleanup_delete_breakpoint): Remove.
	* breakpoint.h (make_cleanup_delete_breakpoint): Remove
	declaration.
	* defs.h (do_exec_cleanups, make_exec_cleanup): Remove
	declarations.
	(struct continations): Add the 'error' parameter to the
	continuation_hook field.
	(add_continuation, do_all_continuations)
	(add_intermediate_continuation)
	(do_all_intermediate_continuations): Add the 'error' parameter.
	* exceptions.c (throw_exception): Don't call do_exec_cleanups.
	* inf-loop.c (inferior_event_handler): Instead of calling
	discard_all_continuations, use do_all_continuations with 1 as
	'error' parameter.  Pass 0 as 'error' parameter in existing uses
	of discard_all_continuations.
	* infcmd.c (step_1): Do not use exec cleanup.  For async case, discard
	cleanups.
	(step_once): Install continuation only after resuming the target.
	(step_1_continuation): Disable longjmp breakpoint on error.
	(finish_command_continuation): Add the error parameter.  Delete
	the finish breakpoint directly, do not use cleanups.
	(finish_command): Do not use exec_cleanups. Always setup
	continuation.  For sync case, immediately run them.
	(attach_command_continuation): Add the error parameter.
	* infrun.c (fetch_inferior_event): Do not use exec cleanups to
	remove step_resume_breakpoint -- adjust delete it directly.
	* interps.c (interp_set): Adjust call to do_all_continations.
	* mi/mi-interp.c (mi_interpreter_exec_continuation): Do not
	do exec cleanups.
	* mi/mi-main.c (mi_cmd_target_select): Do not do exec
	cleanups.
	(mi_cmd_execute): Do not use exec_cleanup.
	(mi_execute_async_cli_command): Simplify the string concatenation
	logic.  Do no use exec cleanup.
	(mi_exec_async_cli_cmd_continuation): New parameter error.
	Free last_async_command.
	* top.c (command_line_handler_continuation): New parameter error.
	* utils.c (exec_cleanup_chain, make_exec_cleanup)
	(do_exec_cleanups): Remove.
	(add_continuation, do_all_continations)
	(add_intermediate_continuation)
	(do_all_intermediate_continuations): New parameter error.
---
 gdb/breakpoint.c   |   84 ++++++++++++++-----------------
 gdb/breakpoint.h   |    2 -
 gdb/defs.h         |   13 ++---
 gdb/exceptions.c   |    6 --
 gdb/inf-loop.c     |   10 ++--
 gdb/infcmd.c       |  144 ++++++++++++++++++++++------------------------------
 gdb/infrun.c       |    9 +---
 gdb/interps.c      |    2 +-
 gdb/mi/mi-interp.c |    5 +-
 gdb/mi/mi-main.c   |   64 ++++++++++++++---------
 gdb/top.c          |    5 ++-
 gdb/utils.c        |   25 ++-------
 12 files changed, 165 insertions(+), 204 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 76cd54b..b640816 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -64,7 +64,8 @@
 
 /* Prototypes for local functions. */
 
-static void until_break_command_continuation (struct continuation_arg *arg);
+static void until_break_command_continuation (struct continuation_arg *arg, 
+					      int error);
 
 static void catch_command_1 (char *, int, int);
 
@@ -6158,12 +6159,11 @@ awatch_command (char *arg, int from_tty)
    care of cleaning up the temporary breakpoints set up by the until
    command. */
 static void
-until_break_command_continuation (struct continuation_arg *arg)
+until_break_command_continuation (struct continuation_arg *arg, int error)
 {
-  struct cleanup *cleanups;
-
-  cleanups = (struct cleanup *) arg->data.pointer;
-  do_exec_cleanups (cleanups);
+  delete_breakpoint ((struct breakpoint *)(arg->data.pointer));
+  if (arg->next)
+    delete_breakpoint ((struct breakpoint *)(arg->next->data.pointer));
 }
 
 void
@@ -6174,8 +6174,10 @@ until_break_command (char *arg, int from_tty, int anywhere)
   struct frame_info *frame = get_selected_frame (NULL);
   struct frame_info *prev_frame = get_prev_frame (frame);
   struct breakpoint *breakpoint;
+  struct breakpoint *breakpoint2 = NULL;
   struct cleanup *old_chain;
   struct continuation_arg *arg1;
+  struct continuation_arg *arg2;
 
 
   clear_proceed_status ();
@@ -6211,31 +6213,7 @@ until_break_command (char *arg, int from_tty, int anywhere)
     breakpoint = set_momentary_breakpoint (sal, get_frame_id (frame),
 					   bp_until);
 
-  if (!target_can_async_p ())
-    old_chain = make_cleanup_delete_breakpoint (breakpoint);
-  else
-    old_chain = make_exec_cleanup_delete_breakpoint (breakpoint);
-
-  /* If we are running asynchronously, and the target supports async
-     execution, we are not waiting for the target to stop, in the call
-     tp proceed, below. This means that we cannot delete the
-     brekpoints until the target has actually stopped. The only place
-     where we get a chance to do that is in fetch_inferior_event, so
-     we must set things up for that. */
-
-  if (target_can_async_p ())
-    {
-      /* In this case the arg for the continuation is just the point
-         in the exec_cleanups chain from where to start doing
-         cleanups, because all the continuation does is the cleanups in
-         the exec_cleanup_chain. */
-      arg1 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg1->next         = NULL;
-      arg1->data.pointer = old_chain;
-
-      add_continuation (until_break_command_continuation, arg1);
-    }
+  old_chain = make_cleanup_delete_breakpoint (breakpoint);
 
   /* Keep within the current frame, or in frames called by the current
      one.  */
@@ -6243,18 +6221,38 @@ until_break_command (char *arg, int from_tty, int anywhere)
     {
       sal = find_pc_line (get_frame_pc (prev_frame), 0);
       sal.pc = get_frame_pc (prev_frame);
-      breakpoint = set_momentary_breakpoint (sal, get_frame_id (prev_frame),
-					     bp_until);
-      if (!target_can_async_p ())
-	make_cleanup_delete_breakpoint (breakpoint);
-      else
-	make_exec_cleanup_delete_breakpoint (breakpoint);
+      breakpoint2 = set_momentary_breakpoint (sal, get_frame_id (prev_frame),
+					      bp_until);
+      make_cleanup_delete_breakpoint (breakpoint2);
     }
 
   proceed (-1, TARGET_SIGNAL_DEFAULT, 0);
-  /* Do the cleanups now, anly if we are not running asynchronously,
-     of if we are, but the target is still synchronous. */
-  if (!target_can_async_p ())
+
+  /* If we are running asynchronously, and proceed call above has actually
+     managed to start the target, arrange for breakpoints to be
+     deleted when the target stops.  Otherwise, we're already stopped and
+     delete breakpoints via cleanup chain.  */
+
+  if (target_can_async_p () && target_executing)
+    {
+      arg1 =
+	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
+      arg1->next         = NULL;
+      arg1->data.pointer = breakpoint;
+
+      if (breakpoint2)
+	{
+	  arg2 = (struct continuation_arg *)
+	    xmalloc ( sizeof (struct continuation_arg));
+	  arg2->next         = NULL;
+	  arg2->data.pointer = breakpoint2;
+	  arg1->next = arg2;	   
+	}
+
+      discard_cleanups (old_chain);
+      add_continuation (until_break_command_continuation, arg1);
+    }
+  else
     do_cleanups (old_chain);
 }
 
@@ -7236,12 +7234,6 @@ make_cleanup_delete_breakpoint (struct breakpoint *b)
   return make_cleanup (do_delete_breakpoint_cleanup, b);
 }
 
-struct cleanup *
-make_exec_cleanup_delete_breakpoint (struct breakpoint *b)
-{
-  return make_exec_cleanup (do_delete_breakpoint_cleanup, b);
-}
-
 void
 delete_command (char *arg, int from_tty)
 {
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 2423a1a..3e54b0f 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -711,8 +711,6 @@ extern void breakpoint_init_inferior (enum inf_context);
 
 extern struct cleanup *make_cleanup_delete_breakpoint (struct breakpoint *);
 
-extern struct cleanup *make_exec_cleanup_delete_breakpoint (struct breakpoint *);
-
 extern void delete_breakpoint (struct breakpoint *);
 
 extern void breakpoint_auto_delete (bpstat);
diff --git a/gdb/defs.h b/gdb/defs.h
index 4b02637..5c35051 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -318,7 +318,6 @@ extern char *safe_strerror (int);
 
 extern void do_cleanups (struct cleanup *);
 extern void do_final_cleanups (struct cleanup *);
-extern void do_exec_cleanups (struct cleanup *);
 
 extern void discard_cleanups (struct cleanup *);
 extern void discard_final_cleanups (struct cleanup *);
@@ -351,8 +350,6 @@ extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *);
 extern struct cleanup *make_my_cleanup (struct cleanup **,
 					make_cleanup_ftype *, void *);
 
-extern struct cleanup *make_exec_cleanup (make_cleanup_ftype *, void *);
-
 extern struct cleanup *save_cleanups (void);
 extern struct cleanup *save_final_cleanups (void);
 extern struct cleanup *save_my_cleanups (struct cleanup **);
@@ -686,7 +683,7 @@ struct continuation_arg
 
 struct continuation
   {
-    void (*continuation_hook) (struct continuation_arg *);
+    void (*continuation_hook) (struct continuation_arg *, int);
     struct continuation_arg *arg_list;
     struct continuation *next;
   };
@@ -697,14 +694,14 @@ extern struct continuation *cmd_continuation;
 extern struct continuation *intermediate_continuation;
 
 /* From utils.c */
-extern void add_continuation (void (*)(struct continuation_arg *),
+extern void add_continuation (void (*)(struct continuation_arg *, int),
 			      struct continuation_arg *);
-extern void do_all_continuations (void);
+extern void do_all_continuations (int error);
 extern void discard_all_continuations (void);
 
-extern void add_intermediate_continuation (void (*)(struct continuation_arg *),
+extern void add_intermediate_continuation (void (*)(struct continuation_arg *, int),
 			      struct continuation_arg *);
-extern void do_all_intermediate_continuations (void);
+extern void do_all_intermediate_continuations (int error);
 extern void discard_all_intermediate_continuations (void);
 
 /* String containing the current directory (what getwd would return).  */
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index 89d1455..304c8c6 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -221,12 +221,6 @@ throw_exception (struct gdb_exception exception)
 
   disable_current_display ();
   do_cleanups (ALL_CLEANUPS);
-  /* When we implement non-stop mode, this should be redone.  If we get
-     exception in a command pertaining to one thread, or maybe even not
-     involving inferior at all, we should not do exec cleanups for all
-     threads.  */
-  if (target_can_async_p () && !target_executing)
-    do_exec_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
diff --git a/gdb/inf-loop.c b/gdb/inf-loop.c
index 9519c79..4c61dae 100644
--- a/gdb/inf-loop.c
+++ b/gdb/inf-loop.c
@@ -50,7 +50,7 @@ inferior_event_handler (enum inferior_event_type event_type,
       printf_unfiltered (_("error detected from target.\n"));
       target_async (NULL, 0);
       pop_target ();
-      discard_all_continuations ();
+      do_all_continuations (1);
       async_enable_stdin ();
       break;
 
@@ -64,7 +64,7 @@ inferior_event_handler (enum inferior_event_type event_type,
 	{
 	  target_async (NULL, 0);
 	  pop_target ();
-	  discard_all_continuations ();
+	  do_all_continuations (1);
 	  async_enable_stdin ();
 	  display_gdb_prompt (0);
 	}
@@ -95,9 +95,9 @@ inferior_event_handler (enum inferior_event_type event_type,
 	 got interrupted by a breakpoint, still do the pending
 	 continuations.  The continuation itself is responsible for
 	 distinguishing the cases.  */
-      do_all_intermediate_continuations ();
+      do_all_intermediate_continuations (0);
 
-      do_all_continuations ();
+      do_all_continuations (0);
 
       if (current_language != expected_language)
 	{
@@ -126,7 +126,7 @@ inferior_event_handler (enum inferior_event_type event_type,
     case INF_EXEC_CONTINUE:
       /* Is there anything left to do for the command issued to
          complete? */
-      do_all_intermediate_continuations ();
+      do_all_intermediate_continuations (0);
       break;
 
     case INF_QUIT_REQ: 
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index dea9e07..0b4dc5d 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -70,7 +70,7 @@ static void nofp_registers_info (char *, int);
 
 static void print_return_value (struct type *value_type);
 
-static void finish_command_continuation (struct continuation_arg *);
+static void finish_command_continuation (struct continuation_arg *, int error);
 
 static void until_next_command (int);
 
@@ -104,7 +104,7 @@ static void jump_command (char *, int);
 
 static void step_1 (int, int, char *);
 static void step_once (int skip_subroutines, int single_inst, int count);
-static void step_1_continuation (struct continuation_arg *arg);
+static void step_1_continuation (struct continuation_arg *arg, int error);
 
 static void next_command (char *, int);
 
@@ -696,7 +696,7 @@ step_1 (int skip_subroutines, int single_inst, char *count_string)
 {
   int count = 1;
   struct frame_info *frame;
-  struct cleanup *cleanups = 0;
+  struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
   int async_exec = 0;
 
   ERROR_NO_INFERIOR;
@@ -722,10 +722,7 @@ step_1 (int skip_subroutines, int single_inst, char *count_string)
   if (!single_inst || skip_subroutines)		/* leave si command alone */
     {
       enable_longjmp_breakpoint ();
-      if (!target_can_async_p ())
-	cleanups = make_cleanup (disable_longjmp_breakpoint_cleanup, 0 /*ignore*/);
-      else
-        make_exec_cleanup (disable_longjmp_breakpoint_cleanup, 0 /*ignore*/);
+      make_cleanup (disable_longjmp_breakpoint_cleanup, 0 /*ignore*/);
     }
 
   /* In synchronous case, all is well, just use the regular for loop. */
@@ -777,8 +774,7 @@ which has no line number information.\n"), name);
 	    break;
 	}
 
-      if (!single_inst || skip_subroutines)
-	do_cleanups (cleanups);
+      do_cleanups (cleanups);
       return;
     }
   /* In case of asynchronous target things get complicated, do only
@@ -787,8 +783,10 @@ which has no line number information.\n"), name);
      and handle them one at the time, through step_once(). */
   else
     {
-      if (target_can_async_p ())
-	step_once (skip_subroutines, single_inst, count);
+      step_once (skip_subroutines, single_inst, count);
+      /* We are running, and the contination is installed.  It will
+	 disable the longjmp breakpoint as appropriate.  */
+      discard_cleanups (cleanups);
     }
 }
 
@@ -798,21 +796,26 @@ which has no line number information.\n"), name);
    proceed(), via step_once(). Basically it is like step_once and
    step_1_continuation are co-recursive. */
 static void
-step_1_continuation (struct continuation_arg *arg)
+step_1_continuation (struct continuation_arg *arg, int error)
 {
-  int count;
-  int skip_subroutines;
-  int single_inst;
-
-  skip_subroutines = arg->data.integer;
-  single_inst      = arg->next->data.integer;
-  count            = arg->next->next->data.integer;
-
-  if (stop_step)
-    step_once (skip_subroutines, single_inst, count - 1);
+  if (error)
+    disable_longjmp_breakpoint ();
   else
-    if (!single_inst || skip_subroutines)
-      do_exec_cleanups (ALL_CLEANUPS);
+    {
+      int count;
+      int skip_subroutines;
+      int single_inst;
+      
+      skip_subroutines = arg->data.integer;
+      single_inst      = arg->next->data.integer;
+      count            = arg->next->next->data.integer;
+      
+      if (stop_step)
+	step_once (skip_subroutines, single_inst, count - 1);
+      else
+	if (!single_inst || skip_subroutines)
+	  disable_longjmp_breakpoint ();
+    }
 }
 
 /* Do just one step operation. If count >1 we will have to set up a
@@ -876,6 +879,7 @@ which has no line number information.\n"), name);
 	step_over_calls = STEP_OVER_ALL;
 
       step_multi = (count > 1);
+      proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 1);
       arg1 =
 	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
       arg2 =
@@ -889,7 +893,6 @@ which has no line number information.\n"), name);
       arg3->next = NULL;
       arg3->data.integer = count;
       add_intermediate_continuation (step_1_continuation, arg1);
-      proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 1);
     }
 }
 
@@ -1245,7 +1248,7 @@ print_return_value (struct type *value_type)
    called via the cmd_continuation pointer.  */
 
 static void
-finish_command_continuation (struct continuation_arg *arg)
+finish_command_continuation (struct continuation_arg *arg, int error)
 {
   struct symbol *function;
   struct breakpoint *breakpoint;
@@ -1255,21 +1258,24 @@ finish_command_continuation (struct continuation_arg *arg)
   function = (struct symbol *) arg->next->data.pointer;
   cleanups = (struct cleanup *) arg->next->next->data.pointer;
 
-  if (bpstat_find_breakpoint (stop_bpstat, breakpoint) != NULL
-      && function != NULL)
+  if (!error)
     {
-      struct type *value_type;
-
-      value_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (function));
-      if (!value_type)
-	internal_error (__FILE__, __LINE__,
-			_("finish_command: function has no target type"));
-
-      if (TYPE_CODE (value_type) != TYPE_CODE_VOID)
-	print_return_value (value_type); 
+      if (bpstat_find_breakpoint (stop_bpstat, breakpoint) != NULL
+	  && function != NULL)
+	{
+	  struct type *value_type;
+	  
+	  value_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (function));
+	  if (!value_type)
+	    internal_error (__FILE__, __LINE__,
+			    _("finish_command: function has no target type"));
+	  
+	  if (TYPE_CODE (value_type) != TYPE_CODE_VOID)
+	    print_return_value (value_type); 
+	}
     }
 
-  do_exec_cleanups (cleanups);
+  delete_breakpoint (breakpoint);
 }
 
 /* "finish": Set a temporary breakpoint at the place the selected
@@ -1320,10 +1326,7 @@ finish_command (char *arg, int from_tty)
 
   breakpoint = set_momentary_breakpoint (sal, get_frame_id (frame), bp_finish);
 
-  if (!target_can_async_p ())
-    old_chain = make_cleanup_delete_breakpoint (breakpoint);
-  else
-    old_chain = make_exec_cleanup_delete_breakpoint (breakpoint);
+  old_chain = make_cleanup_delete_breakpoint (breakpoint);
 
   /* Find the function we will return from.  */
 
@@ -1340,51 +1343,26 @@ finish_command (char *arg, int from_tty)
   proceed_to_finish = 1;	/* We want stop_registers, please...  */
   proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
 
-  /* If running asynchronously and the target support asynchronous
-     execution, set things up for the rest of the finish command to be
-     completed later on, when gdb has detected that the target has
-     stopped, in fetch_inferior_event.  
-     Setup it only after proceed, so that if proceed throws, we don't
-     set continuation.  */
-  if (target_can_async_p ())
-    {
-      arg1 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg2 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg3 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg1->next = arg2;
-      arg2->next = arg3;
-      arg3->next = NULL;
-      arg1->data.pointer = breakpoint;
-      arg2->data.pointer = function;
-      arg3->data.pointer = old_chain;
-      add_continuation (finish_command_continuation, arg1);
-    }
+  arg1 =
+    (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
+  arg2 =
+    (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
+  arg3 =
+    (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
+  arg1->next = arg2;
+  arg2->next = arg3;
+  arg3->next = NULL;
+  arg1->data.pointer = breakpoint;
+  arg2->data.pointer = function;
+  arg3->data.pointer = old_chain;
+  add_continuation (finish_command_continuation, arg1);
 
   /* Do this only if not running asynchronously or if the target
      cannot do async execution.  Otherwise, complete this command when
      the target actually stops, in fetch_inferior_event.  */
+  discard_cleanups (old_chain);
   if (!target_can_async_p ())
-    {
-      /* Did we stop at our breakpoint?  */
-      if (bpstat_find_breakpoint (stop_bpstat, breakpoint) != NULL
-	  && function != NULL)
-	{
-	  struct type *value_type;
-
-	  value_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (function));
-	  if (!value_type)
-	    internal_error (__FILE__, __LINE__,
-			    _("finish_command: function has no target type"));
-
-	  if (TYPE_CODE (value_type) != TYPE_CODE_VOID)
-	    print_return_value (value_type); 
-	}
-
-      do_cleanups (old_chain);
-    }
+    do_all_continuations (0);
 }
 
 
@@ -1930,7 +1908,7 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
 }
 
 static void
-attach_command_continuation (struct continuation_arg *arg)
+attach_command_continuation (struct continuation_arg *arg, int error)
 {
   char *args;
   int from_tty;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 2f77bbe..83715f3 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1385,9 +1385,6 @@ fetch_inferior_event (void *client_data)
 
   if (!async_ecs->wait_some_more)
     {
-      old_cleanups = make_exec_cleanup (delete_step_resume_breakpoint,
-					&step_resume_breakpoint);
-
       /* Fill in with reasonable starting values.  */
       init_execution_control_state (async_ecs);
 
@@ -1416,10 +1413,8 @@ fetch_inferior_event (void *client_data)
 
   if (!async_ecs->wait_some_more)
     {
-      /* Do only the cleanups that have been added by this
-         function. Let the continuations for the commands do the rest,
-         if there are any. */
-      do_exec_cleanups (old_cleanups);
+      delete_step_resume_breakpoint (&step_resume_breakpoint);
+
       normal_stop ();
       if (step_multi && stop_step)
 	inferior_event_handler (INF_EXEC_CONTINUE, NULL);
diff --git a/gdb/interps.c b/gdb/interps.c
index 7f48ef8..9d47843 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -148,7 +148,7 @@ interp_set (struct interp *interp, int top_level)
 
   if (current_interpreter != NULL)
     {
-      do_all_continuations ();
+      do_all_continuations (0);
       ui_out_flush (uiout);
       if (current_interpreter->procs->suspend_proc
 	  && !current_interpreter->procs->suspend_proc (current_interpreter->
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index c968a8f..ae9e07b 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -168,9 +168,11 @@ mi_interpreter_prompt_p (void *data)
 }
 
 static void
-mi_interpreter_exec_continuation (struct continuation_arg *arg)
+mi_interpreter_exec_continuation (struct continuation_arg *arg, int  error)
 {
   bpstat_do_actions (&stop_bpstat);
+  /* It's not clear what to do in the case of errror -- should we assume that
+     the target is stopped, or that it still runs?  */
   if (!target_executing)
     {
       fputs_unfiltered ("*stopped", raw_stdout);
@@ -178,7 +180,6 @@ mi_interpreter_exec_continuation (struct continuation_arg *arg)
       fputs_unfiltered ("\n", raw_stdout);
       fputs_unfiltered ("(gdb) \n", raw_stdout);
       gdb_flush (raw_stdout);
-      do_exec_cleanups (ALL_CLEANUPS);
     }
   else if (target_can_async_p ())
     {
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 153d2b5..cfc9b2b 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -104,7 +104,8 @@ static void mi_execute_cli_command (const char *cmd, int args_p,
 				    const char *args);
 static enum mi_cmd_result mi_execute_async_cli_command (char *mi, char *args, int from_tty);
 
-static void mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg);
+static void mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg, 
+						int error);
 
 static int register_changed_p (int regnum, struct regcache *,
 			       struct regcache *);
@@ -684,7 +685,6 @@ mi_cmd_target_select (char *args, int from_tty)
   mi_out_put (uiout, raw_stdout);
   mi_out_rewind (uiout);
   fputs_unfiltered ("\n", raw_stdout);
-  do_exec_cleanups (ALL_CLEANUPS);
   return MI_CMD_QUIET;
 }
 
@@ -1184,6 +1184,8 @@ mi_execute_command (char *cmd, int from_tty)
 static enum mi_cmd_result
 mi_cmd_execute (struct mi_parse *parse)
 {
+  struct cleanup *cleanup;
+  enum mi_cmd_result r;
   free_all_values ();
 
   if (parse->cmd->argv_func != NULL
@@ -1222,11 +1224,19 @@ mi_cmd_execute (struct mi_parse *parse)
 	    }
 	}
       last_async_command = xstrdup (parse->token);
-      make_exec_cleanup (free_current_contents, &last_async_command);
+      cleanup = make_cleanup (free_current_contents, &last_async_command);
       /* FIXME: DELETE THIS! */
       if (parse->cmd->args_func != NULL)
-	return parse->cmd->args_func (parse->args, 0 /*from_tty */ );
-      return parse->cmd->argv_func (parse->command, parse->argv, parse->argc);
+	r = parse->cmd->args_func (parse->args, 0 /*from_tty */ );
+      else
+	r = parse->cmd->argv_func (parse->command, parse->argv, parse->argc);
+      if (target_can_async_p () && target_executing)
+	/* last_async_command will be freed by continuation that
+	   all execution command set.  */
+	discard_cleanups (cleanup);
+      else
+	do_cleanups (cleanup);
+      return r;
     }
   else if (parse->cmd->cli.cmd != 0)
     {
@@ -1287,24 +1297,12 @@ mi_execute_async_cli_command (char *mi, char *args, int from_tty)
 {
   struct cleanup *old_cleanups;
   char *run;
-  char *async_args;
 
   if (target_can_async_p ())
-    {
-      async_args = (char *) xmalloc (strlen (args) + 2);
-      make_exec_cleanup (free, async_args);
-      strcpy (async_args, args);
-      strcat (async_args, "&");
-      run = xstrprintf ("%s %s", mi, async_args);
-      make_exec_cleanup (free, run);
-      add_continuation (mi_exec_async_cli_cmd_continuation, NULL);
-      old_cleanups = NULL;
-    }
+    run = xstrprintf ("%s %s&", mi, args);
   else
-    {
-      run = xstrprintf ("%s %s", mi, args);
-      old_cleanups = make_cleanup (xfree, run);
-    }
+    run = xstrprintf ("%s %s", mi, args);
+  old_cleanups = make_cleanup (xfree, run);  
 
   if (!target_can_async_p ())
     {
@@ -1326,11 +1324,24 @@ mi_execute_async_cli_command (char *mi, char *args, int from_tty)
       if (last_async_command)
 	fputs_unfiltered (last_async_command, raw_stdout);
       fputs_unfiltered ("^running\n", raw_stdout);
+
+      /* Ideally, we should be intalling continuation only when
+	 the target is already running. However, this will break right now,
+	 because continuation installed by the 'finish' command must be after
+	 the continuation that prints *stopped.  This issue will be
+	 fixed soon.  */
+      add_continuation (mi_exec_async_cli_cmd_continuation, NULL);
     }
 
   execute_command ( /*ui */ run, 0 /*from_tty */ );
 
-  if (!target_can_async_p ())
+  if (target_can_async_p ())
+    {
+      /* If we're not executing, an exception should have been throw.  */
+      gdb_assert (target_executing);
+      do_cleanups (old_cleanups);
+    }
+  else
     {
       /* Do this before doing any printing.  It would appear that some
          print code leaves garbage around in the buffer.  */
@@ -1346,13 +1357,14 @@ mi_execute_async_cli_command (char *mi, char *args, int from_tty)
       	print_diff_now (current_command_ts);
       fputs_unfiltered ("\n", raw_stdout);
       return MI_CMD_QUIET;
-    }
+    }    
   return MI_CMD_DONE;
 }
 
 void
-mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg)
+mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg, int error)
 {
+  /* Assume 'error' means that target is stopped, too.  */
   if (last_async_command)
     fputs_unfiltered (last_async_command, raw_stdout);
   fputs_unfiltered ("*stopped", raw_stdout);
@@ -1360,7 +1372,11 @@ mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg)
   fputs_unfiltered ("\n", raw_stdout);
   fputs_unfiltered ("(gdb) \n", raw_stdout);
   gdb_flush (raw_stdout);
-  do_exec_cleanups (ALL_CLEANUPS);
+  if (last_async_command)
+    {
+      free (last_async_command);
+      last_async_command = NULL;
+    }	
 }
 
 void
diff --git a/gdb/top.c b/gdb/top.c
index 418ff8e..b9688b1 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -368,7 +368,7 @@ do_chdir_cleanup (void *old_dir)
    are always running synchronously. Or if we have just executed a
    command that doesn't start the target.  */
 static void
-command_line_handler_continuation (struct continuation_arg *arg)
+command_line_handler_continuation (struct continuation_arg *arg, int error)
 {
   extern int display_time;
   extern int display_space;
@@ -376,6 +376,9 @@ command_line_handler_continuation (struct continuation_arg *arg)
   long time_at_cmd_start  = arg->data.longint;
   long space_at_cmd_start = arg->next->data.longint;
 
+  if (error)
+    return;
+
   bpstat_do_actions (&stop_bpstat);
 
   if (display_time)
diff --git a/gdb/utils.c b/gdb/utils.c
index 594fc73..d9953a0 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -104,7 +104,6 @@ static int debug_timestamp = 0;
 
 static struct cleanup *cleanup_chain;	/* cleaned up after a failed command */
 static struct cleanup *final_cleanup_chain;	/* cleaned up when gdb exits */
-static struct cleanup *exec_cleanup_chain;	/* cleaned up on each execution command */
 
 /* Pointer to what is left to do for an execution command after the
    target stops. Used only in asynchronous mode, by targets that
@@ -214,12 +213,6 @@ make_final_cleanup (make_cleanup_ftype *function, void *arg)
   return make_my_cleanup (&final_cleanup_chain, function, arg);
 }
 
-struct cleanup *
-make_exec_cleanup (make_cleanup_ftype *function, void *arg)
-{
-  return make_my_cleanup (&exec_cleanup_chain, function, arg);
-}
-
 static void
 do_freeargv (void *arg)
 {
@@ -316,12 +309,6 @@ do_final_cleanups (struct cleanup *old_chain)
   do_my_cleanups (&final_cleanup_chain, old_chain);
 }
 
-void
-do_exec_cleanups (struct cleanup *old_chain)
-{
-  do_my_cleanups (&exec_cleanup_chain, old_chain);
-}
-
 static void
 do_my_cleanups (struct cleanup **pmy_chain,
 		struct cleanup *old_chain)
@@ -440,7 +427,7 @@ null_cleanup (void *arg)
 /* Add a continuation to the continuation list, the global list
    cmd_continuation. The new continuation will be added at the front.*/
 void
-add_continuation (void (*continuation_hook) (struct continuation_arg *),
+add_continuation (void (*continuation_hook) (struct continuation_arg *, int),
 		  struct continuation_arg *arg_list)
 {
   struct continuation *continuation_ptr;
@@ -462,7 +449,7 @@ add_continuation (void (*continuation_hook) (struct continuation_arg *),
    and do the continuations from there on, instead of using the
    global beginning of list as our iteration pointer.  */
 void
-do_all_continuations (void)
+do_all_continuations (int error)
 {
   struct continuation *continuation_ptr;
   struct continuation *saved_continuation;
@@ -477,7 +464,7 @@ do_all_continuations (void)
   /* Work now on the list we have set aside.  */
   while (continuation_ptr)
     {
-      (continuation_ptr->continuation_hook) (continuation_ptr->arg_list);
+      (continuation_ptr->continuation_hook) (continuation_ptr->arg_list, error);
       saved_continuation = continuation_ptr;
       continuation_ptr = continuation_ptr->next;
       xfree (saved_continuation);
@@ -504,7 +491,7 @@ discard_all_continuations (void)
    the front.  */
 void
 add_intermediate_continuation (void (*continuation_hook)
-			       (struct continuation_arg *),
+			       (struct continuation_arg *, int),
 			       struct continuation_arg *arg_list)
 {
   struct continuation *continuation_ptr;
@@ -526,7 +513,7 @@ add_intermediate_continuation (void (*continuation_hook)
    and do the continuations from there on, instead of using the
    global beginning of list as our iteration pointer.*/
 void
-do_all_intermediate_continuations (void)
+do_all_intermediate_continuations (int error)
 {
   struct continuation *continuation_ptr;
   struct continuation *saved_continuation;
@@ -541,7 +528,7 @@ do_all_intermediate_continuations (void)
   /* Work now on the list we have set aside.  */
   while (continuation_ptr)
     {
-      (continuation_ptr->continuation_hook) (continuation_ptr->arg_list);
+      (continuation_ptr->continuation_hook) (continuation_ptr->arg_list, error);
       saved_continuation = continuation_ptr;
       continuation_ptr = continuation_ptr->next;
       xfree (saved_continuation);
-- 
1.5.3.5


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]