Python and target-async

Python and target-async

[Kevin Pouget]

Hello,

I'm facing a problem with Python not behaving correctly when
"target-async" is on. I'll report it in the bugtracker if someone else
confirms the problem:

(replace PID by any process ID to which you can attach)

> (gdb) set target-async on
> (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> #0  0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> #1  0x000000000043ede2 in ?? ()
> #2  0x000000000044005f in wait_for ()
> #3  0x0000000000430c45 in execute_command_internal ()
> #4  0x0000000000430e0e in execute_command ()
> #5  0x000000000041d526 in reader_loop ()
> #6  0x000000000041ccde in main ()
> (gdb) detach
> Detaching from program ...
> (gdb) set target-async on
> (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> #0  0x0000003cbd0aa47e in ?? ()
> #1  0x0000000000000000 in ?? ()
> 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6

--> the inferior can't be access normally

the problem lies in the fact that GDB doesn't return from Python
between the `attach' and the `where', (the situation occurs during a
Python function calls as well).

> (gdb) set target-async on
> (gdb) py gdb.execute("attach PID")
> (gdb) py gdb.execute("where")

works correctly.


I'm not sure there is something I can do to solve it, any idea what I
should look for?


thanks,

Kevin

Re: Python and target-async

[Pedro Alves]

I've read this about 5 times, and I still have trouble
identifying exactly what it is you're saying is broken.
Is is the second example showing a couple of frames
before the "waitpid" line?  In your example you
have "set target-async on" twice. Was that a pasto?
What exactly do you think is wrong, and what did you
expect was the right output?

On Tuesday 31 May 2011 14:51:45, Kevin Pouget wrote:
> Hello,
> 
> I'm facing a problem with Python not behaving correctly when
> "target-async" is on. I'll report it in the bugtracker if someone else
> confirms the problem:
> 
> (replace PID by any process ID to which you can attach)
> 
> > (gdb) set target-async on
> > (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> > 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> > #0  0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> > #1  0x000000000043ede2 in ?? ()
> > #2  0x000000000044005f in wait_for ()
> > #3  0x0000000000430c45 in execute_command_internal ()
> > #4  0x0000000000430e0e in execute_command ()
> > #5  0x000000000041d526 in reader_loop ()
> > #6  0x000000000041ccde in main ()
> > (gdb) detach
> > Detaching from program ...
> > (gdb) set target-async on
> > (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> > #0  0x0000003cbd0aa47e in ?? ()
> > #1  0x0000000000000000 in ?? ()
> > 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> 
> --> the inferior can't be access normally
> 
> the problem lies in the fact that GDB doesn't return from Python
> between the `attach' and the `where', (the situation occurs during a
> Python function calls as well).
> 
> > (gdb) set target-async on
> > (gdb) py gdb.execute("attach PID")
> > (gdb) py gdb.execute("where")
> 
> works correctly.
> 
> 
> I'm not sure there is something I can do to solve it, any idea what I
> should look for?
> 
> 
> thanks,
> 
> Kevin
> 

-- 
Pedro Alves

Re: Python and target-async

[Kevin Pouget]

> you have "set target-async on" twice. Was that a pasto?

sorry, yes the first one was wrong

here is the correct behaviour:

> (gdb) set target-async off
> (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> #0  0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> #1  0x000000000043ede2 in ?? ()
> #2  0x000000000044005f in wait_for ()
> #3  0x0000000000430c45 in execute_command_internal ()
> #4  0x0000000000430e0e in execute_command ()
> #5  0x000000000041d526 in reader_loop ()
> #6  0x000000000041ccde in main ()

and this one is wrong because the backtrace is not correctly computed:

> (gdb) set target-async on
> (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> #0  0x0000003cbd0aa47e in ?? ()
> #1  0x0000000000000000 in ?? ()
> 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6

I expected to read:

> (gdb) set target-async on
> (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> #0  0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> #1  0x000000000043ede2 in ?? ()
> #2  0x000000000044005f in wait_for ()
> #3  0x0000000000430c45 in execute_command_internal ()
> #4  0x0000000000430e0e in execute_command ()
> #5  0x000000000041d526 in reader_loop ()
> #6  0x000000000041ccde in main ()

is it clearer this way ?

thanks,

Kevin

> On Tuesday 31 May 2011 14:51:45, Kevin Pouget wrote:
>> Hello,
>>
>> I'm facing a problem with Python not behaving correctly when
>> "target-async" is on. I'll report it in the bugtracker if someone else
>> confirms the problem:
>>
>> (replace PID by any process ID to which you can attach)
>>
>> > (gdb) set target-async on
>> > (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
>> > 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
>> > #0  0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
>> > #1  0x000000000043ede2 in ?? ()
>> > #2  0x000000000044005f in wait_for ()
>> > #3  0x0000000000430c45 in execute_command_internal ()
>> > #4  0x0000000000430e0e in execute_command ()
>> > #5  0x000000000041d526 in reader_loop ()
>> > #6  0x000000000041ccde in main ()
>> > (gdb) detach
>> > Detaching from program ...
>> > (gdb) set target-async on
>> > (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
>> > #0  0x0000003cbd0aa47e in ?? ()
>> > #1  0x0000000000000000 in ?? ()
>> > 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
>>
>> --> the inferior can't be access normally
>>
>> the problem lies in the fact that GDB doesn't return from Python
>> between the `attach' and the `where', (the situation occurs during a
>> Python function calls as well).
>>
>> > (gdb) set target-async on
>> > (gdb) py gdb.execute("attach PID")
>> > (gdb) py gdb.execute("where")
>>
>> works correctly.
>>
>>
>> I'm not sure there is something I can do to solve it, any idea what I
>> should look for?
>>
>>
>> thanks,
>>
>> Kevin
>>
>
> --
> Pedro Alves
>

Re: Python and target-async

[Pedro Alves]

On Tuesday 31 May 2011 18:54:10, Kevin Pouget wrote:
> > you have "set target-async on" twice. Was that a pasto?
> 
> sorry, yes the first one was wrong
> 
> here is the correct behaviour:
> 
> > (gdb) set target-async off
> > (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> > 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> > #0  0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> > #1  0x000000000043ede2 in ?? ()
> > #2  0x000000000044005f in wait_for ()
> > #3  0x0000000000430c45 in execute_command_internal ()
> > #4  0x0000000000430e0e in execute_command ()
> > #5  0x000000000041d526 in reader_loop ()
> > #6  0x000000000041ccde in main ()
> 
> and this one is wrong because the backtrace is not correctly computed:
> 
> > (gdb) set target-async on
> > (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> > #0  0x0000003cbd0aa47e in ?? ()
> > #1  0x0000000000000000 in ?? ()
> > 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> 
> I expected to read:
> 
> > (gdb) set target-async on
> > (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> > 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> > #0  0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> > #1  0x000000000043ede2 in ?? ()
> > #2  0x000000000044005f in wait_for ()
> > #3  0x0000000000430c45 in execute_command_internal ()
> > #4  0x0000000000430e0e in execute_command ()
> > #5  0x000000000041d526 in reader_loop ()
> > #6  0x000000000041ccde in main ()
> 
> is it clearer this way ?

Very much.  Thanks.  This sounds similar to the
define.exp:nextwhere testcase failing in async mode.
That is, the attach command installing a continuation
and returning before the attach is complete, and 
the "where" command executing before the inferior reports
a stop to the attach command continuation.
I've got a fix for the "nextwhere" issue, that I'll
need some cleaning up before pushing, though
I'll bet that the python code will need fixing
of its own.  :-/

-- 
Pedro Alves

Re: Python and target-async

[Kevin Pouget]

On Thu, Jun 2, 2011 at 3:28 PM, Pedro Alves <pedro@codesourcery.com> wrote:
>
> On Tuesday 31 May 2011 18:54:10, Kevin Pouget wrote:
> > > you have "set target-async on" twice. Was that a pasto?
> >
> > sorry, yes the first one was wrong
> >
> > here is the correct behaviour:
> >
> > > (gdb) set target-async off
> > > (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> > > 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> > > #0  0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> > > #1  0x000000000043ede2 in ?? ()
> > > #2  0x000000000044005f in wait_for ()
> > > #3  0x0000000000430c45 in execute_command_internal ()
> > > #4  0x0000000000430e0e in execute_command ()
> > > #5  0x000000000041d526 in reader_loop ()
> > > #6  0x000000000041ccde in main ()
> >
> > and this one is wrong because the backtrace is not correctly computed:
> >
> > > (gdb) set target-async on
> > > (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> > > #0  0x0000003cbd0aa47e in ?? ()
> > > #1  0x0000000000000000 in ?? ()
> > > 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> >
> > I expected to read:
> >
> > > (gdb) set target-async on
> > > (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> > > 0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> > > #0  0x0000003cbd0aa47e in waitpid () from /lib64/libc.so.6
> > > #1  0x000000000043ede2 in ?? ()
> > > #2  0x000000000044005f in wait_for ()
> > > #3  0x0000000000430c45 in execute_command_internal ()
> > > #4  0x0000000000430e0e in execute_command ()
> > > #5  0x000000000041d526 in reader_loop ()
> > > #6  0x000000000041ccde in main ()
> >
> > is it clearer this way ?
>
> Very much.  Thanks.  This sounds similar to the
> define.exp:nextwhere testcase failing in async mode.
> That is, the attach command installing a continuation
> and returning before the attach is complete, and
> the "where" command executing before the inferior reports
> a stop to the attach command continuation.
> I've got a fix for the "nextwhere" issue, that I'll
> need some cleaning up before pushing, though
> I'll bet that the python code will need fixing
> of its own.  :-/
>
> --
> Pedro Alves

Hi Pedro,

any update on this issue? you said that only "some cleaning up" where
required, but it's still failing on the last gdb-CVS ... anything I
could do to help you? I really this bit to be solved to keep going ...
(my copyright assignment is not ready yet, but maybe some
beta-testing?)

Thanks,

Kevin

( http://sourceware.org/bugzilla/show_bug.cgi?id=12929 )

Re: Python and target-async

[Pedro Alves]

On Wednesday 17 August 2011 12:36:28, Kevin Pouget wrote:
> On Thu, Jun 2, 2011 at 3:28 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> > Very much.  Thanks.  This sounds similar to the
> > define.exp:nextwhere testcase failing in async mode.
> > That is, the attach command installing a continuation
> > and returning before the attach is complete, and
> > the "where" command executing before the inferior reports
> > a stop to the attach command continuation.
> > I've got a fix for the "nextwhere" issue, that I'll
> > need some cleaning up before pushing, though
> > I'll bet that the python code will need fixing
> > of its own.  :-/

> Hi Pedro,
> 
> any update on this issue? you said that only "some cleaning up" where
> required, but it's still failing on the last gdb-CVS ... anything I
> could do to help you? I really this bit to be solved to keep going ...
> (my copyright assignment is not ready yet, but maybe some
> beta-testing?)

The patch I was talking about this one:
<http://sourceware.org/ml/gdb-patches/2011-06/msg00158.html>

I was pulled away for a while from the project, and I've now come back to
it.  Unfortunately, "Some cleaning up" turned into "large and endless
cleaning up", and I haven't reached python yet even.  So I came up with
a plan B, as I need to make progress on other fronts for itsets/ptsets
support which is really what I want to get at.  Instead of converting everything and
its mother to an asynchronous state machine, I've come up with the patch
below, which flips the _interpreter_ to synchronous mode when we want to
run some command list, and similars, like several python command in one
gdb command.

This makes the testsuite run regression free in async mode,
compared to sync mode.

I'd like to push something like this in, as first step, then suggest
flipping async mode on by default (need to discuss what happens to
MI then), so that user don't need to do "set target-async on"
anymore to enable new functionality.

Can you give it a try?

(Note, I haven't yet checked if/where MI will need flipping
the interpreter to sync mode).

-- 
Pedro Alves

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

	gdb/
	* top.c: Include interps.h.
	(execute_command): If the target can async, but the interpreter is
	in sync mode, synchronously wait for the command to finish before
	returning.
	* infrun.c: Include interps.h.
	(fetch_inferior_event): Don't restore the prompt yet if the
	interpreter is in sync mode.
	* interps.c (interpreter_async): New global.
	* interps.h (interpreter_async): Declare.
	* inf-loop.c: Include interps.h.
	(inferior_event_handler): Don't print the language change or run
	breakpoint commands yet if the interpreter in is sync mode.
	* cli/cli-script.c: Include interps.h.
	(execute_user_command, while_command, if_command): Force the
	interpreter to sync mode.
	* python/python.c: Include interps.h.
	(python_command): Force the interpreter to sync mode.

---
 gdb/cli/cli-script.c |   14 ++++++++++++++
 gdb/inf-loop.c       |   26 ++++++++++++++++----------
 gdb/infrun.c         |    3 ++-
 gdb/interps.c        |    4 ++++
 gdb/interps.h        |    9 +++++++++
 gdb/python/python.c  |    5 +++++
 gdb/top.c            |   14 +++++++++++++-
 7 files changed, 63 insertions(+), 12 deletions(-)

Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c	2011-08-16 13:19:30.228094364 +0100
+++ src/gdb/top.c	2011-08-16 15:33:59.467224810 +0100
@@ -48,6 +48,7 @@
 #include "event-loop.h"
 #include "gdbthread.h"
 #include "python/python.h"
+#include "interps.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
@@ -440,7 +441,18 @@ execute_command (char *p, int from_tty)
 	deprecated_call_command_hook (c, arg, from_tty & caution);
       else
 	cmd_func (c, arg, from_tty & caution);
-       
+
+      /* If the interpreter is in sync mode (we're running a user
+	 command's list, running command hooks or similars), and we
+	 just ran a synchronous command that started the target, wait
+	 for that command to end.  */
+      if (!interpreter_async && sync_execution)
+	{
+	  while (gdb_do_one_event () >= 0)
+	    if (!sync_execution)
+	      break;
+	}
+
       /* If this command has been post-hooked, run the hook last.  */
       execute_cmd_post_hook (c);
 
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2011-08-16 13:19:30.228094364 +0100
+++ src/gdb/infrun.c	2011-08-16 13:19:49.148094357 +0100
@@ -55,6 +55,7 @@
 #include "jit.h"
 #include "tracepoint.h"
 #include "continuations.h"
+#include "interps.h"
 
 /* Prototypes for local functions */
 
@@ -2810,7 +2811,7 @@ fetch_inferior_event (void *client_data)
 
   /* If the inferior was in sync execution mode, and now isn't,
      restore the prompt.  */
-  if (was_sync && !sync_execution)
+  if (interpreter_async && was_sync && !sync_execution)
     display_gdb_prompt (0);
 }
 
Index: src/gdb/cli/cli-script.c
===================================================================
--- src.orig/gdb/cli/cli-script.c	2011-08-16 13:19:30.178094364 +0100
+++ src/gdb/cli/cli-script.c	2011-08-16 13:19:49.148094357 +0100
@@ -35,6 +35,7 @@
 #include "gdb_assert.h"
 
 #include "python/python.h"
+#include "interps.h"
 
 /* Prototypes for local functions.  */
 
@@ -338,6 +339,9 @@ execute_user_command (struct cmd_list_el
      not confused with Insight.  */
   in_user_command = 1;
 
+  make_cleanup_restore_integer (&interpreter_async);
+  interpreter_async = 0;
+
   command_nest_depth++;
   while (cmdlines)
     {
@@ -598,6 +602,7 @@ void
 while_command (char *arg, int from_tty)
 {
   struct command_line *command = NULL;
+  struct cleanup *old_chain;
 
   control_level = 1;
   command = get_command_line (while_control, arg);
@@ -605,8 +610,13 @@ while_command (char *arg, int from_tty)
   if (command == NULL)
     return;
 
+  old_chain = make_cleanup_restore_integer (&interpreter_async);
+  interpreter_async = 0;
+
   execute_control_command_untraced (command);
   free_command_lines (&command);
+
+  do_cleanups (old_chain);
 }
 
 /* "if" command support.  Execute either the true or false arm depending
@@ -616,6 +626,7 @@ void
 if_command (char *arg, int from_tty)
 {
   struct command_line *command = NULL;
+  struct cleanup *old_chain;
 
   control_level = 1;
   command = get_command_line (if_control, arg);
@@ -623,6 +634,9 @@ if_command (char *arg, int from_tty)
   if (command == NULL)
     return;
 
+  old_chain = make_cleanup_restore_integer (&interpreter_async);
+  interpreter_async = 0;
+
   execute_control_command_untraced (command);
   free_command_lines (&command);
 }
Index: src/gdb/interps.c
===================================================================
--- src.orig/gdb/interps.c	2011-08-16 13:19:30.178094364 +0100
+++ src/gdb/interps.c	2011-08-16 13:19:49.148094357 +0100
@@ -43,6 +43,10 @@
 #include "exceptions.h"
 #include "continuations.h"
 
+/* True if the current interpreter in is async mode.  See interps.h
+   for more details.  */
+int interpreter_async = 1;
+
 struct interp
 {
   /* This is the name in "-i=" and set interpreter.  */
Index: src/gdb/interps.h
===================================================================
--- src.orig/gdb/interps.h	2011-08-16 13:19:30.118094364 +0100
+++ src/gdb/interps.h	2011-08-16 13:19:49.158094357 +0100
@@ -69,6 +69,15 @@ extern void current_interp_command_loop
 extern void *top_level_interpreter_data (void);
 extern struct interp *top_level_interpreter (void);
 
+/* True if the current interpreter is in async mode, false if in sync
+   mode.  If in sync mode, running a synchronous execution command
+   (with execute_command, e.g, "next") will not return until the
+   command is finished.  If in async mode, then running a synchronous
+   command returns right after resuming the target.  Waiting for the
+   command's completion is later done on the top event loop (using
+   continuations).  */
+extern int interpreter_async;
+
 extern void clear_interpreter_hooks (void);
 
 /* well-known interpreters */
Index: src/gdb/python/python.c
===================================================================
--- src.orig/gdb/python/python.c	2011-08-16 15:32:30.000000000 +0100
+++ src/gdb/python/python.c	2011-08-16 15:32:53.187224834 +0100
@@ -52,6 +52,7 @@ static int gdbpy_should_print_stack = 0;
 #include "target.h"
 #include "gdbthread.h"
 #include "observer.h"
+#include "interps.h"
 
 static PyMethodDef GdbMethods[];
 
@@ -199,6 +200,10 @@ python_command (char *arg, int from_tty)
   struct cleanup *cleanup;
 
   cleanup = ensure_python_env (get_current_arch (), current_language);
+
+  make_cleanup_restore_integer (&interpreter_async);
+  interpreter_async = 0;
+
   while (arg && *arg && isspace (*arg))
     ++arg;
   if (arg && *arg)
Index: src/gdb/inf-loop.c
===================================================================
--- src.orig/gdb/inf-loop.c	2011-08-16 15:33:30.000000000 +0100
+++ src/gdb/inf-loop.c	2011-08-16 15:42:00.677224644 +0100
@@ -29,6 +29,7 @@
 #include "language.h"
 #include "gdbthread.h"
 #include "continuations.h"
+#include "interps.h"
 
 static int fetch_inferior_event_wrapper (gdb_client_data client_data);
 
@@ -107,17 +108,22 @@ inferior_event_handler (enum inferior_ev
       else
 	do_all_continuations (0);
 
-      if (info_verbose
-	  && 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)
+      /* When running a command list (from a user command, say), these
+	 are only run when the command list is all done.  */
+      if (interpreter_async)
 	{
-	  bpstat_do_actions ();
+	  if (info_verbose
+	      && 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)
+	    {
+	      bpstat_do_actions ();
+	    }
 	}
 
       if (!was_sync

Re: Python and target-async

[Kevin Pouget]

On Wed, Aug 17, 2011 at 2:25 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 17 August 2011 12:36:28, Kevin Pouget wrote:
>> On Thu, Jun 2, 2011 at 3:28 PM, Pedro Alves <pedro@codesourcery.com> wrote:
>> > Very much.  Thanks.  This sounds similar to the
>> > define.exp:nextwhere testcase failing in async mode.
>> > That is, the attach command installing a continuation
>> > and returning before the attach is complete, and
>> > the "where" command executing before the inferior reports
>> > a stop to the attach command continuation.
>> > I've got a fix for the "nextwhere" issue, that I'll
>> > need some cleaning up before pushing, though
>> > I'll bet that the python code will need fixing
>> > of its own.  :-/
>
>> Hi Pedro,
>>
>> any update on this issue? you said that only "some cleaning up" where
>> required, but it's still failing on the last gdb-CVS ... anything I
>> could do to help you? I really this bit to be solved to keep going ...
>> (my copyright assignment is not ready yet, but maybe some
>> beta-testing?)
>
> The patch I was talking about this one:
> <http://sourceware.org/ml/gdb-patches/2011-06/msg00158.html>
>
> I was pulled away for a while from the project, and I've now come back to
> it.  Unfortunately, "Some cleaning up" turned into "large and endless
> cleaning up", and I haven't reached python yet even.  So I came up with
> a plan B, as I need to make progress on other fronts for itsets/ptsets
> support which is really what I want to get at.  Instead of converting everything and
> its mother to an asynchronous state machine, I've come up with the patch
> below, which flips the _interpreter_ to synchronous mode when we want to
> run some command list, and similars, like several python command in one
> gdb command.
>
> This makes the testsuite run regression free in async mode,
> compared to sync mode.
>
> I'd like to push something like this in, as first step, then suggest
> flipping async mode on by default (need to discuss what happens to
> MI then), so that user don't need to do "set target-async on"
> anymore to enable new functionality.
>
> Can you give it a try?
>
> (Note, I haven't yet checked if/where MI will need flipping
> the interpreter to sync mode).

thanks a lot, this patch solves the problem described in
http://sourceware.org/bugzilla/show_bug.cgi?id=12929, Python included,
so I'll use it for my further development, and I'll let you know if I
notice anything wrong

Cordially,

Kevin

NB: I don't use MI at all, only CLI and Python

Re: Python and target-async

[Kevin Pouget]

> On Wed, Aug 17, 2011 at 2:25 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> > On Wednesday 17 August 2011 12:36:28, Kevin Pouget wrote:
> >> On Thu, Jun 2, 2011 at 3:28 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> >> > Very much.  Thanks.  This sounds similar to the
> >> > define.exp:nextwhere testcase failing in async mode.
> >> > That is, the attach command installing a continuation
> >> > and returning before the attach is complete, and
> >> > the "where" command executing before the inferior reports
> >> > a stop to the attach command continuation.
> >> > I've got a fix for the "nextwhere" issue, that I'll
> >> > need some cleaning up before pushing, though
> >> > I'll bet that the python code will need fixing
> >> > of its own.  :-/
> >
> >> Hi Pedro,
> >>
> >> any update on this issue? you said that only "some cleaning up" where
> >> required, but it's still failing on the last gdb-CVS ... anything I
> >> could do to help you? I really this bit to be solved to keep going ...
> >> (my copyright assignment is not ready yet, but maybe some
> >> beta-testing?)
> >
> > The patch I was talking about this one:
> > <http://sourceware.org/ml/gdb-patches/2011-06/msg00158.html>
> >
> > I was pulled away for a while from the project, and I've now come back to
> > it.  Unfortunately, "Some cleaning up" turned into "large and endless
> > cleaning up", and I haven't reached python yet even.  So I came up with
> > a plan B, as I need to make progress on other fronts for itsets/ptsets
> > support which is really what I want to get at.  Instead of converting everything and
> > its mother to an asynchronous state machine, I've come up with the patch
> > below, which flips the _interpreter_ to synchronous mode when we want to
> > run some command list, and similars, like several python command in one
> > gdb command.
> >
> > This makes the testsuite run regression free in async mode,
> > compared to sync mode.
> >
> > I'd like to push something like this in, as first step, then suggest
> > flipping async mode on by default (need to discuss what happens to
> > MI then), so that user don't need to do "set target-async on"
> > anymore to enable new functionality.
> >
> > Can you give it a try?
> >
> > (Note, I haven't yet checked if/where MI will need flipping
> > the interpreter to sync mode).
>
> thanks a lot, this patch solves the problem described in
> http://sourceware.org/bugzilla/show_bug.cgi?id=12929, Python included,
> so I'll use it for my further development, and I'll let you know if I
> notice anything wrong

Hello Pedro,

it seems that Python doesn't entirely come for free with your patch:
although
> (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
gives the expected output,

class cmd_test(gdb.Command):
    def __init__(self):
        gdb.Command.__init__ (self, "test", gdb.COMMAND_NONE)

    def invoke (self, args, from_tty):
        gdb.execute("attach %s" % args)
	gdb.execute("where")
	#gdb.execute("detach")
cmd_test()

always returns something like

> (gdb) test PID
> #0  0x47eab690 in ?? ()
> Backtrace stopped: Not enough registers or memory available to unwind further
> 0x0000003c47eab690 in __nanosleep_nocancel () from /lib64/libc.so.6

furthermore, the `gdb.execute("detach")' which I commented out above,
leads to the internal_error that I reported in
http://sourceware.org/bugzilla/show_bug.cgi?id=13110 :

> ...gdb/thread.c:623: internal-error: is_thread_state: Assertion `tp' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.

Any thought on what to do for that?

Thanks,

Kevin

Re: Python and target-async

[Pedro Alves]

Hello Kevin,

On Friday 19 August 2011 14:35:07, Kevin Pouget wrote:

> Hello Pedro,
> 
> it seems that Python doesn't entirely come for free with your patch:
> although
> > (gdb) py gdb.execute("attach PID") ; gdb.execute("where")
> gives the expected output,
> 
> class cmd_test(gdb.Command):
>     def __init__(self):
>         gdb.Command.__init__ (self, "test", gdb.COMMAND_NONE)
> 
>     def invoke (self, args, from_tty):
>         gdb.execute("attach %s" % args)
> 	gdb.execute("where")
> 	#gdb.execute("detach")
> cmd_test()
> 
> always returns something like
> 
> > (gdb) test PID
> > #0  0x47eab690 in ?? ()
> > Backtrace stopped: Not enough registers or memory available to unwind further
> > 0x0000003c47eab690 in __nanosleep_nocancel () from /lib64/libc.so.6

Thanks.  There was another path in the python code that needed
the same treatment.  Please try this new patch.  This regtested this
one, and it fixes all async fails compared to sync mode, except a few
failures in default.exp that introduced very recently (related to prompt
stuff).

> furthermore, the `gdb.execute("detach")' which I commented out above,
> leads to the internal_error that I reported in
> http://sourceware.org/bugzilla/show_bug.cgi?id=13110 :
> 
> > ...gdb/thread.c:623: internal-error: is_thread_state: Assertion `tp' failed.
> > A problem internal to GDB has been detected,
> > further debugging may prove unreliable.
> 
> Any thought on what to do for that?

I'll fix it.

-- 
Pedro Alves

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

	gdb/
	* top.c: Include interps.h.
	(execute_command): If the target can async, but the interpreter is
	in sync mode, synchronously wait for the command to finish before
	returning.
	(execute_command_to_string): Force the interpreter to sync mode.
	* infrun.c: Include interps.h.
	(fetch_inferior_event): Don't restore the prompt yet if the
	interpreter is in sync mode.
	* interps.c (interpreter_async): New global.
	* interps.h (interpreter_async): Declare.
	* inf-loop.c: Include interps.h.
	(inferior_event_handler): Don't print the language change or run
	breakpoint commands yet if the interpreter in is sync mode.
	* main.c (captured_command_loop): Flip the interpreter to async
	mode.
	* cli/cli-script.c: Include interps.h.
	(execute_user_command, while_command, if_command): Force the
	interpreter to sync mode.
	* python/python.c: Include interps.h.
	(python_command, execute_gdb_command): Force the interpreter to
	sync mode.

---
 gdb/cli/cli-script.c |   16 ++++++++++++++++
 gdb/inf-loop.c       |   26 ++++++++++++++++----------
 gdb/infrun.c         |    3 ++-
 gdb/interps.c        |    6 ++++++
 gdb/interps.h        |    9 +++++++++
 gdb/main.c           |    4 ++++
 gdb/python/python.c  |    8 ++++++++
 gdb/top.c            |   17 ++++++++++++++++-
 8 files changed, 77 insertions(+), 12 deletions(-)

Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c	2011-08-30 12:17:19.000000000 +0100
+++ src/gdb/top.c	2011-08-30 13:57:17.145339665 +0100
@@ -48,6 +48,7 @@
 #include "event-loop.h"
 #include "gdbthread.h"
 #include "python/python.h"
+#include "interps.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
@@ -441,7 +442,18 @@ execute_command (char *p, int from_tty)
 	deprecated_call_command_hook (c, arg, from_tty & caution);
       else
 	cmd_func (c, arg, from_tty & caution);
-       
+
+      /* If the interpreter is in sync mode (we're running a user
+	 command's list, running command hooks or similars), and we
+	 just ran a synchronous command that started the target, wait
+	 for that command to end.  */
+      if (!interpreter_async && sync_execution && is_running (inferior_ptid))
+	{
+	  while (gdb_do_one_event () >= 0)
+	    if (!sync_execution)
+	      break;
+	}
+
       /* If this command has been post-hooked, run the hook last.  */
       execute_cmd_post_hook (c);
 
@@ -497,6 +509,9 @@ execute_command_to_string (char *p, int
      restoration callbacks.  */
   cleanup = set_batch_flag_and_make_cleanup_restore_page_info ();
 
+  make_cleanup_restore_integer (&interpreter_async);
+  interpreter_async = 0;
+
   str_file = mem_fileopen ();
 
   make_cleanup_ui_file_delete (str_file);
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2011-08-30 11:29:09.000000000 +0100
+++ src/gdb/infrun.c	2011-08-30 13:46:43.265339554 +0100
@@ -55,6 +55,7 @@
 #include "jit.h"
 #include "tracepoint.h"
 #include "continuations.h"
+#include "interps.h"
 
 /* Prototypes for local functions */
 
@@ -2814,7 +2815,7 @@ fetch_inferior_event (void *client_data)
 
   /* If the inferior was in sync execution mode, and now isn't,
      restore the prompt.  */
-  if (was_sync && !sync_execution)
+  if (interpreter_async && was_sync && !sync_execution)
     display_gdb_prompt (0);
 }
 
Index: src/gdb/interps.c
===================================================================
--- src.orig/gdb/interps.c	2011-08-30 11:29:09.000000000 +0100
+++ src/gdb/interps.c	2011-08-30 13:46:43.315339554 +0100
@@ -43,6 +43,12 @@
 #include "exceptions.h"
 #include "continuations.h"
 
+/* True if the current interpreter in is async mode.  See interps.h
+   for more details.  This starts out disabled, until all the explicit
+   command line arguments (e.g., `gdb -ex "start" -ex "next"') are
+   processed.  */
+int interpreter_async = 0;
+
 struct interp
 {
   /* This is the name in "-i=" and set interpreter.  */
Index: src/gdb/interps.h
===================================================================
--- src.orig/gdb/interps.h	2011-08-30 11:29:09.000000000 +0100
+++ src/gdb/interps.h	2011-08-30 13:46:43.315339554 +0100
@@ -69,6 +69,15 @@ extern void current_interp_command_loop
 extern void *top_level_interpreter_data (void);
 extern struct interp *top_level_interpreter (void);
 
+/* True if the current interpreter is in async mode, false if in sync
+   mode.  If in sync mode, running a synchronous execution command
+   (with execute_command, e.g, "next") will not return until the
+   command is finished.  If in async mode, then running a synchronous
+   command returns right after resuming the target.  Waiting for the
+   command's completion is later done on the top event loop (using
+   continuations).  */
+extern int interpreter_async;
+
 extern void clear_interpreter_hooks (void);
 
 /* well-known interpreters */
Index: src/gdb/inf-loop.c
===================================================================
--- src.orig/gdb/inf-loop.c	2011-08-30 11:29:09.000000000 +0100
+++ src/gdb/inf-loop.c	2011-08-30 13:46:43.375339554 +0100
@@ -29,6 +29,7 @@
 #include "language.h"
 #include "gdbthread.h"
 #include "continuations.h"
+#include "interps.h"
 
 static int fetch_inferior_event_wrapper (gdb_client_data client_data);
 
@@ -109,17 +110,22 @@ inferior_event_handler (enum inferior_ev
       else
 	do_all_continuations (0);
 
-      if (info_verbose
-	  && 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)
+      /* When running a command list (from a user command, say), these
+	 are only run when the command list is all done.  */
+      if (interpreter_async)
 	{
-	  bpstat_do_actions ();
+	  if (info_verbose
+	      && 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)
+	    {
+	      bpstat_do_actions ();
+	    }
 	}
       exception_print (gdb_stderr, e);
 
Index: src/gdb/main.c
===================================================================
--- src.orig/gdb/main.c	2011-08-30 11:29:09.000000000 +0100
+++ src/gdb/main.c	2011-08-30 13:46:43.375339554 +0100
@@ -227,6 +227,10 @@ get_init_files (char **system_gdbinit,
 static int
 captured_command_loop (void *data)
 {
+  /* Top-level execution commands can be run on the background from
+     here on.  */
+  interpreter_async = 1;
+
   current_interp_command_loop ();
   /* FIXME: cagney/1999-11-05: A correct command_loop() implementaton
      would clean things up (restoring the cleanup chain) to the state
Index: src/gdb/cli/cli-script.c
===================================================================
--- src.orig/gdb/cli/cli-script.c	2011-08-30 11:29:09.000000000 +0100
+++ src/gdb/cli/cli-script.c	2011-08-30 13:46:43.295339554 +0100
@@ -35,6 +35,7 @@
 #include "gdb_assert.h"
 
 #include "python/python.h"
+#include "interps.h"
 
 /* Prototypes for local functions.  */
 
@@ -338,6 +339,9 @@ execute_user_command (struct cmd_list_el
      not confused with Insight.  */
   in_user_command = 1;
 
+  make_cleanup_restore_integer (&interpreter_async);
+  interpreter_async = 0;
+
   command_nest_depth++;
   while (cmdlines)
     {
@@ -598,6 +602,7 @@ void
 while_command (char *arg, int from_tty)
 {
   struct command_line *command = NULL;
+  struct cleanup *old_chain;
 
   control_level = 1;
   command = get_command_line (while_control, arg);
@@ -605,8 +610,13 @@ while_command (char *arg, int from_tty)
   if (command == NULL)
     return;
 
+  old_chain = make_cleanup_restore_integer (&interpreter_async);
+  interpreter_async = 0;
+
   execute_control_command_untraced (command);
   free_command_lines (&command);
+
+  do_cleanups (old_chain);
 }
 
 /* "if" command support.  Execute either the true or false arm depending
@@ -616,6 +626,7 @@ void
 if_command (char *arg, int from_tty)
 {
   struct command_line *command = NULL;
+  struct cleanup *old_chain;
 
   control_level = 1;
   command = get_command_line (if_control, arg);
@@ -623,8 +634,13 @@ if_command (char *arg, int from_tty)
   if (command == NULL)
     return;
 
+  old_chain = make_cleanup_restore_integer (&interpreter_async);
+  interpreter_async = 0;
+
   execute_control_command_untraced (command);
   free_command_lines (&command);
+
+  do_cleanups (old_chain);
 }
 
 /* Cleanup */
Index: src/gdb/python/python.c
===================================================================
--- src.orig/gdb/python/python.c	2011-08-30 11:29:09.000000000 +0100
+++ src/gdb/python/python.c	2011-08-30 13:46:43.365339554 +0100
@@ -52,6 +52,7 @@ static int gdbpy_should_print_stack = 0;
 #include "target.h"
 #include "gdbthread.h"
 #include "observer.h"
+#include "interps.h"
 
 static PyMethodDef GdbMethods[];
 
@@ -199,6 +200,10 @@ python_command (char *arg, int from_tty)
   struct cleanup *cleanup;
 
   cleanup = ensure_python_env (get_current_arch (), current_language);
+
+  make_cleanup_restore_integer (&interpreter_async);
+  interpreter_async = 0;
+
   while (arg && *arg && isspace (*arg))
     ++arg;
   if (arg && *arg)
@@ -378,6 +383,9 @@ execute_gdb_command (PyObject *self, PyO
       char *copy = xstrdup (arg);
       struct cleanup *cleanup = make_cleanup (xfree, copy);
 
+      make_cleanup_restore_integer (&interpreter_async);
+      interpreter_async = 0;
+
       prevent_dont_repeat ();
       if (to_string)
 	result = execute_command_to_string (copy, from_tty);

Re: Python and target-async

[Jan Kratochvil]

On Tue, 30 Aug 2011 15:13:38 +0200, Pedro Alves wrote:
> --- src.orig/gdb/inf-loop.c	2011-08-30 11:29:09.000000000 +0100
> +++ src/gdb/inf-loop.c	2011-08-30 13:46:43.375339554 +0100
[...]
> @@ -109,17 +110,22 @@ inferior_event_handler (enum inferior_ev
>        else
>  	do_all_continuations (0);
>  
> -      if (info_verbose
> -	  && 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)
> +      /* When running a command list (from a user command, say), these
> +	 are only run when the command list is all done.  */
> +      if (interpreter_async)
>  	{
> -	  bpstat_do_actions ();
> +	  if (info_verbose
> +	      && 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)
> +	    {
> +	      bpstat_do_actions ();
> +	    }
>  	}
>        exception_print (gdb_stderr, e);
>  

It works great except for this mis-merged part of the recent change by me.


Thanks,
Jan


--- a/gdb/inf-loop.c
+++ b/gdb/inf-loop.c
@@ -126,8 +126,8 @@ inferior_event_handler (enum inferior_event_type event_type,
 	    {
 	      bpstat_do_actions ();
 	    }
+	  exception_print (gdb_stderr, e);
 	}
-      exception_print (gdb_stderr, e);
 
       if (!was_sync
 	  && exec_done_display_p

Re: Python and target-async

[Pedro Alves]

On Tuesday 30 August 2011 16:54:04, Jan Kratochvil wrote:
> It works great except for this mis-merged part of the recent change by me.

Whoops, thanks!  I've applied the patch with that corrected.

-- 
Pedro Alves

Re: Python and target-async

[Pedro Alves]

On Friday 19 August 2011 14:35:07, Kevin Pouget wrote:
> furthermore, the `gdb.execute("detach")' which I commented out above,
> leads to the internal_error that I reported in
> http://sourceware.org/bugzilla/show_bug.cgi?id=13110 :
> 
> > ...gdb/thread.c:623: internal-error: is_thread_state: Assertion `tp' failed.
> > A problem internal to GDB has been detected,
> > further debugging may prove unreliable.
> 
> Any thought on what to do for that?
> 

This.  Tested on x86_64-linux and applied.

But I'm now wondering if we actually ever need the registers_changed
call here or in wait_for_inferior nowadays --- we flush threads'
register caches when we resume them (target_resume).

-- 
Pedro Alves

2011-09-05  Pedro Alves  <pedro@codesourcery.com>

	PR cli/13110

	gdb/
	* infrun.c (fetch_inferior_event): Check if there's a selected
	thread before checking if the selected thread is executing.

---
 gdb/infrun.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2011-09-05 15:50:49.000000000 +0100
+++ src/gdb/infrun.c	2011-09-05 15:57:10.693964411 +0100
@@ -2749,7 +2749,9 @@ fetch_inferior_event (void *client_data)
      switches threads anyway).  If we didn't do this, a spurious
      delayed event in all-stop mode would make the user lose the
      selected frame.  */
-  if (non_stop || is_executing (inferior_ptid))
+  if (non_stop
+      || (!ptid_equal (inferior_ptid, null_ptid)
+	  && is_executing (inferior_ptid)))
     registers_changed ();
 
   make_cleanup_restore_integer (&execution_direction);