Re: Stop breakpoint commands from poping the target

[Pedro Alves <pedro at codesourcery dot com>]

A Friday 14 March 2008 08:15:30, Vladimir Prus wrote:
> Pedro Alves wrote:
> > This patch goes on top of Vladimir's pending async fixes patch.
> >
> > commands.exp triggers the problem this patch addresses, in
> > async mode.
> >
> > In that test, we have a watch on a local variable, and then we
> > add a command to that watch to print the local variable's
> > value.  When the variable goes out of scope, gdb prints that,
> > and also runs the associated commands with the watch.  Since
> > the variable is out of scope, and error is thrown.  In sync
> > targets, the exception just ends the breakpoints command
> > processing, and goes on to proceed with the command loop.  But in
> > async mode, the exception ends up in
> > inf-loop.c:inferior_event_handler/INF_REG_EVENT, which considers
> > exceptions fatal, and pops the target.  The fix is to catch the exception
> > earlier.
> >
> > Imagine the following command list associated with a breakpoint:
> >   non_existing_command
> >   info target
> >
> > As and example, in sync mode, trying to execute
> > non_existing_command causes an error that stops the
> > interpreting of all subsequent commands, hence info target is
> > never executed.  The patch installs the same behaviour for
> > async targets.
> >
> > commands.exp passes cleanly with the linux native async patch
> > I'll post next.
>
> Heh, we seem to have a mid-flight collision here -- I've a local
> patch moving the call to bpstat_do_actions into
> inferior_event_handler. The call you modify, in
> command_line_handler_continuation, runs for CLI only and it's
> quite possible to have commands on breakpoints even if GDB uses
> MI on the top level.
>
> But that only chances which line of code must be changed, while
> this patch is still needed. One tweak though -- I think it's
> better to use TRY_CATCH, not catch_exceptions -- it's just
> fewer lines.
>

I have been using this instead now.  Should I go ahead while you
don't post your patch (if it looks ok)?  Or is it coming out soon (,
hopefully with a similar fix in)?  I'm fine either way.

-- 
Pedro Alves

2008-03-17  Pedro Alves  <pedro@codesourcery.com>

	* top.c (command_line_handler_continuation): Wrap call to
	bpstat_do_actions in TRY_CATCH.

---
 gdb/top.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c	2008-03-14 22:36:07.000000000 +0000
+++ src/gdb/top.c	2008-03-14 22:46:02.000000000 +0000
@@ -376,7 +376,12 @@ command_line_handler_continuation (struc
   long time_at_cmd_start  = arg->data.longint;
   long space_at_cmd_start = arg->next->data.longint;
 
-  bpstat_do_actions (&stop_bpstat);
+  volatile struct gdb_exception exception;
+  TRY_CATCH (exception, RETURN_MASK_ALL)
+    {
+      /* Don't propagate errors to inferior_event_handler/INF_REG_EVENT.  */
+      bpstat_do_actions (&stop_bpstat);
+    }
 
   if (display_time)
     {
@@ -539,7 +544,7 @@ execute_command (char *p, int from_tty)
 	}
     }
 
-  /* Set things up for this function to be compete later, once the
+  /* Set things up for this function to be finished later, once the
      execution has completed, if we are doing an execution command,
      otherwise, just go ahead and finish. */
   if (target_can_async_p () && target_executing)