continuations and breakpoint commands

continuations and breakpoint commands

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]

This patch fixes continuation handling in async mode, by making
the handling sequence match the sync code path.

Exec commands sequence should be:

execute command -> wait for stop -> rest of command

After that, if the stop was due to a breakpoint, we check for
any breakpoint commands associated with it.  Any command is
allowed, so it's possible that one command be another exec
command, which resumes the target:

break main
commands
  continue
end

execute command
         -> wait for stop
                 -> rest of command
                          -> breakpoint commands
                                   execute command -> ...

breakpoint commands used to be ran in the
command_line_handler_continuation.  Now they're ran before
the continuations, which is wrong.  If the continuation
has to pull a breakpoint out of the inferior, and a breakpoint
command resumed the inferior, the removal will fail (on targets
where writing inferior memory on a running target is not
possible, like linux).

Looking at top.c to copy the exact sync sequence also shows that
any language change is printed before running breakpoint commands.

This patch applies on top of this one:
[RFA] bpstat_do_actions in one place
http://sourceware.org/ml/gdb-patches/2008-04/msg00557.html

This fixes commands.exp failures.  There's one case where the
removing a breakpoint from a running target was triggering, due
to a breakpoint command.

Tested on i686-pc-linux-gnu/async.

Ok, after the dependencies are in?

-- 
Pedro Alves

[-- Attachment #2: fix_commands.exp.diff --]
[-- Type: text/x-diff, Size: 2443 bytes --]

2008-04-29  Pedro Alves  <pedro@codesourcery.com>

	* inf-loop.c (inferior_event_handler): Run all continuations and
	print any language change before running the breakpoint commands.

---
 gdb/inf-loop.c |   33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

Index: src/gdb/inf-loop.c
===================================================================
--- src.orig/gdb/inf-loop.c	2008-04-29 20:45:35.000000000 +0100
+++ src/gdb/inf-loop.c	2008-04-29 22:04:03.000000000 +0100
@@ -92,30 +92,35 @@ inferior_event_handler (enum inferior_ev
       was_sync = sync_execution;
       async_enable_stdin ();
 
-      /* If there's an error doing breakpoint commands, we don't
-        want to throw -- continuation might still do something.  */
-      TRY_CATCH (e, RETURN_MASK_ERROR)
-       {
-         bpstat_do_actions (&stop_bpstat);
-       }
       /* If we were doing a multi-step (eg: step n, next n), but it
 	 got interrupted by a breakpoint, still do the pending
 	 continuations.  The continuation itself is responsible for
-	 distinguishing the cases.  */
+	 distinguishing the cases.  The continuations are allowed to
+	 touch the inferior memory, e.g. to remove breakpoints, so run
+	 them before running breakpoint commands, which may resume the
+	 target.  */
       do_all_intermediate_continuations (0);
 
+      /* Always finish the previous command before running any
+	 breakpoint commands.  Any stop cancels the previous command.
+	 E.g. a "finish" or "step-n" command interrupted by an
+	 unrelated breakpoint is canceled.  */
       do_all_continuations (0);
 
-      if (current_language != expected_language)
+      if (current_language != expected_language
+	  && language_mode == language_mode_auto)
+	language_info (1);	/* Print what changed.  */
+
+      /* Don't propagate breakpoint commands errors.  Either we're
+	 stopping or some command resumes the inferior.  The user will
+	 be informed.  */
+      TRY_CATCH (e, RETURN_MASK_ERROR)
 	{
-	  if (language_mode == language_mode_auto)
-	    {
-	      language_info (1);	/* Print what changed.  */
-	    }
+	  bpstat_do_actions (&stop_bpstat);
 	}
 
-      /* If the continuation did not start the target again,
-	 prepare for interation with the user.  */
+      /* If no breakpoint command resumed the inferior, prepare for
+	 interation with the user.  */
       if (!target_executing)
 	{              
 	  if (was_sync)

Re: continuations and breakpoint commands

[Luis Machado]

Pedro,

On Tue, 2008-04-29 at 22:21 +0100, Pedro Alves wrote:
> This patch fixes continuation handling in async mode, by making
> the handling sequence match the sync code path.

Thanks for the patch. The commands.exp testcase is now clear for PPC.
Didn't see any regressions.

Just a minor gotcha:

-      /* If the continuation did not start the target again,
-        prepare for interation with the user.  */
+      /* If no breakpoint command resumed the inferior, prepare for
+        >>interation<< with the user.  */

Is "interation" supposed to be "interaction"?

Regards,
Luis

Re: continuations and breakpoint commands

[Pedro Alves]

A Wednesday 30 April 2008 13:53:05, Luis Machado wrote:
> Pedro,
>
> On Tue, 2008-04-29 at 22:21 +0100, Pedro Alves wrote:
> > This patch fixes continuation handling in async mode, by making
> > the handling sequence match the sync code path.
>
> Thanks for the patch. The commands.exp testcase is now clear for PPC.
> Didn't see any regressions.
>

Thanks for testing.

> Just a minor gotcha:
>
> -      /* If the continuation did not start the target again,
> -        prepare for interation with the user.  */
> +      /* If no breakpoint command resumed the inferior, prepare for
> +        >>interation<< with the user.  */
>
> Is "interation" supposed to be "interaction"?
>

Yep.  Thanks for spotting that.  We're going to display the prompt
so the user can enter commands, that's what it means.

-- 
Pedro Alves

RE: continuations and breakpoint commands

[Pierre Muller]

>> Just a minor gotcha:
>>
>> -      /* If the continuation did not start the target again,
>> -        prepare for interation with the user.  */
>> +      /* If no breakpoint command resumed the inferior, prepare for
>> +        >>interation<< with the user.  */
>>
>> Is "interation" supposed to be "interaction"?
>>
>
>Yep.  Thanks for spotting that.  We're going to display the prompt
>so the user can enter commands, that's what it means.
>
>-- 
>Pedro Alves

  Hi Pedro,

  I am confused, but it is probably because I didn't
have time and courage to read the whole non-stop threads.
  
  What happens if the user is currently typing some
commands on prompt?
  Are there two different prompts to let the
user know if we are running or stopped?
  What happens to the already typed chars in the
case of a prompt switch?

Pierre Muller
Pascal language support maintainer for GDB



  

Re: continuations and breakpoint commands

[Pedro Alves]

A Wednesday 30 April 2008 15:41:40, Pierre Muller wrote:
> >> Just a minor gotcha:
> >>
> >> -      /* If the continuation did not start the target again,
> >> -        prepare for interation with the user.  */
> >> +      /* If no breakpoint command resumed the inferior, prepare for
> >> +        >>interation<< with the user.  */
> >>
> >> Is "interation" supposed to be "interaction"?
> >
> >Yep.  Thanks for spotting that.  We're going to display the prompt
> >so the user can enter commands, that's what it means.
> >
> >--
> >Pedro Alves
>
>   Hi Pedro,
>
>   I am confused, but it is probably because I didn't
> have time and courage to read the whole non-stop threads.
>
>   What happens if the user is currently typing some
> commands on prompt?

When the target supports async execution, and the exec
command was a background exec command (that is, continue& vs
continue; step& vs step), the user will be able to type
commands while the inferior is running.  If the target
supports async execution, but the exec command was
a foreground exec command (continue, next, step --
without an appended &), GDB will fake sync execution
by disabling stdin, transfering the terminal to the inferior
and not printing the prompt.  When the inferior stops, it needs
to print the prompt.  The user gets the same effect as
in a GDB where the target does not support async execution like so:

>/home/pedro/gdb/nonstop_head/build-32/gdb/gdb -ex "maint set 
linux-async" ./threads32
(gdb) r
Starting program: /home/pedro/gdb/tests/threads32
[Thread debugging using libthread_db enabled]
[New Thread 0xf7e27b90 (LWP 27641)]
[New Thread 0xf7626b90 (LWP 27642)]
any typed char is ignored by gdb   << enter
really                             << enter
                                   << ^C
Program received signal SIGINT, Interrupt.
[Switching to Thread 0x7f60629c96e0 (LWP 27505)]
0x00007f60625ba796 in pthread_join () from /lib/libpthread.so.0
(gdb)

>/home/pedro/gdb/nonstop_head/build-32/gdb/gdb -ex "maint set 
linux-async" ./threads32
(gdb) r&
Starting program: /home/pedro/gdb/tests/threads32
(gdb) [Thread debugging using libthread_db enabled]
[New Thread 0xf7d60b90 (LWP 27708)]
[New Thread 0xf755fb90 (LWP 27709)]
                                     << inferior is running, start typing,
(gdb) any char is ignored            << enter
Undefined command: "any".  Try "help".
(gdb)  

>   Are there two different prompts to let the
> user know if we are running or stopped?

No.

>   What happens to the already typed chars in the
> case of a prompt switch?
>

No sure I understood that.  When the inferior is running in
sync execution, any typed character goes to the inferior, while
in async execution the terminal stays with GDB and GDB interprets
any command you pass to it.

-- 
Pedro Alves

Re: continuations and breakpoint commands

[Daniel Jacobowitz]

On Wed, Apr 30, 2008 at 03:59:34PM +0100, Pedro Alves wrote:
> >   What happens to the already typed chars in the
> > case of a prompt switch?
> >
> 
> No sure I understood that.  When the inferior is running in
> sync execution, any typed character goes to the inferior, while
> in async execution the terminal stays with GDB and GDB interprets
> any command you pass to it.

I think he's asking what happens when the program hits a breakpoint
while you're mid-typing.  I assume we print some output and readline
redraws the prompt line?

-- 
Daniel Jacobowitz
CodeSourcery

Re: continuations and breakpoint commands

[Pedro Alves]

Sorry, I seem to have missed this,

A Friday 02 May 2008 16:36:55, Daniel Jacobowitz wrote:
> On Wed, Apr 30, 2008 at 03:59:34PM +0100, Pedro Alves wrote:
> > >   What happens to the already typed chars in the
> > > case of a prompt switch?
> >
> > No sure I understood that.  When the inferior is running in
> > sync execution, any typed character goes to the inferior, while
> > in async execution the terminal stays with GDB and GDB interprets
> > any command you pass to it.
>

> I think he's asking what happens when the program hits a breakpoint
> while you're mid-typing.  I assume we print some output and readline
> redraws the prompt line?

There's no prompt switch in async mode.  The code comment in question
has to do with sync_execution on top of async mode, where anything
the user typed while the inferior was executing went to the inferior.

In async, if some breakpoint is hit mid-typing we just print some
output, and don't mess with the prompt, so whatever the user was
mid-typing is unaffected.  The command line does gets a bit
garbled, because there's no distintion between the command line,
and other output, though.

With a source that looks like (line/code):

8	sleep (10); << stopped here.
9	sleep (10); << breakpoint here.

(gdb) c&
Continuing.
(gdb) help    << "enter" not pressed yet.

(wait for breakpoint hit)

(gdb) c&
Continuing.
(gdb) helpBreakpoint 3, thread1_really () at thread.cpp:9
9       in thread.cpp

<< press "enter" now.

List of classes of commands:

aliases -- Aliases of other commands
breakpoints -- Making program stop at certain points
data -- Examining data
files -- Specifying and examining files
internals -- Maintenance commands
obscure -- Obscure features
running -- Running the program
...

Clearly, an improvement would be for the output to be:

(gdb) c&
Continuing.
(gdb) help    << "enter" not pressed yet.

(wait for breakpoint hit)

(gdb) c&
Continuing.
Breakpoint 3, thread1_really () at thread.cpp:9
9       in thread.cpp
(gdb) help

I suppose it may be possible to add some smartness to printf_*filtered
and friends to do some dance with readline for this.

There's one case where prompting may get in the way
of what the user was typing.  That's when due to *_filtered,
pagination kicks in.  We use a prompt for the query.

Hey, nobody's been claiming CLI is perfect in async mode yet :-)

-- 
Pedro Alves

Re: continuations and breakpoint commands

[Daniel Jacobowitz]

On Tue, Apr 29, 2008 at 10:21:52PM +0100, Pedro Alves wrote:
> 2008-04-29  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* inf-loop.c (inferior_event_handler): Run all continuations and
> 	print any language change before running the breakpoint commands.

Looks OK, unless it needs to be reworked after the discussion we're
currently having about what happens to other continuations.

-- 
Daniel Jacobowitz
CodeSourcery

Re: continuations and breakpoint commands

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

A Friday 02 May 2008 16:38:26, Daniel Jacobowitz wrote:
> On Tue, Apr 29, 2008 at 10:21:52PM +0100, Pedro Alves wrote:
> > 2008-04-29  Pedro Alves  <pedro@codesourcery.com>
> >
> > 	* inf-loop.c (inferior_event_handler): Run all continuations and
> > 	print any language change before running the breakpoint commands.
>
> Looks OK, unless it needs to be reworked after the discussion we're
> currently having about what happens to other continuations.

Since we are not going to add per-thread continuations in all-stop mode
after all, I've checked this in, after another round of testing.

-- 
Pedro Alves

[-- Attachment #2: fix_commands.exp.diff --]
[-- Type: text/x-diff, Size: 2416 bytes --]

2008-05-06  Pedro Alves  <pedro@codesourcery.com>

	* inf-loop.c (inferior_event_handler): Run all continuations and
	print any language change before running the breakpoint commands.

---
 gdb/inf-loop.c |   33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

Index: src/gdb/inf-loop.c
===================================================================
--- src.orig/gdb/inf-loop.c	2008-05-06 19:23:15.000000000 +0100
+++ src/gdb/inf-loop.c	2008-05-06 19:23:52.000000000 +0100
@@ -92,30 +92,35 @@ inferior_event_handler (enum inferior_ev
       was_sync = sync_execution;
       async_enable_stdin ();
 
-      /* If there's an error doing breakpoint commands, we don't
-	 want to throw -- continuation might still do something.  */
-      TRY_CATCH (e, RETURN_MASK_ALL)
-	{
-	  bpstat_do_actions (&stop_bpstat);
-	}
       /* If we were doing a multi-step (eg: step n, next n), but it
 	 got interrupted by a breakpoint, still do the pending
 	 continuations.  The continuation itself is responsible for
-	 distinguishing the cases.  */
+	 distinguishing the cases.  The continuations are allowed to
+	 touch the inferior memory, e.g. to remove breakpoints, so run
+	 them before running breakpoint commands, which may resume the
+	 target.  */
       do_all_intermediate_continuations (0);
 
+      /* Always finish the previous command before running any
+	 breakpoint commands.  Any stop cancels the previous command.
+	 E.g. a "finish" or "step-n" command interrupted by an
+	 unrelated breakpoint is canceled.  */
       do_all_continuations (0);
 
-      if (current_language != expected_language)
+      if (current_language != expected_language
+	  && language_mode == language_mode_auto)
+	language_info (1);	/* Print what changed.  */
+
+      /* Don't propagate breakpoint commands errors.  Either we're
+	 stopping or some command resumes the inferior.  The user will
+	 be informed.  */
+      TRY_CATCH (e, RETURN_MASK_ALL)
 	{
-	  if (language_mode == language_mode_auto)
-	    {
-	      language_info (1);	/* Print what changed.  */
-	    }
+	  bpstat_do_actions (&stop_bpstat);
 	}
 
-      /* If the continuation did not start the target again,
-	 prepare for interation with the user.  */
+      /* If no breakpoint command resumed the inferior, prepare for
+	 interaction with the user.  */
       if (!target_executing)
 	{              
 	  if (was_sync)