Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [MI/RFC] Emit ^running via observer.
@ 2008-06-13 22:01 Vladimir Prus
  2008-06-20  6:53 ` Nick Roberts
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Prus @ 2008-06-13 22:01 UTC (permalink / raw)
  To: gdb-patches


This patch fixes an issue in MI code that was present since at least 1999.
We output ^running before even trying to resume the target, not to mention
making sure the target is resumed. So, if resuming fails, we'd get ^running,
followed by ^error, and I don't really know if current frontends will like
it at all.

Now that we have observer for resume, and that observer is called after target
is resumed, we can emit ^running from that observer. The immediate bonus
is that ^running is now emitted for every command that resumes the inferior,
even for CLI commands. Another (unexpected) bonus, is that since now ^running
and *running is output in a single place, we can produce them in consistent order.
Previously, we would output:

    ^running
    (gdb)
    *running,...

in sync mode and:

    ^running
    *running,...
    (gdb)

in async mode, which makes testing for *running a bit hard.

I indend to double-check that we'll never output ^running except
immediately during processing of MI command, but otherwise I think
this is ready to go in. I'll commit in a few days if there are no
objections.

- Volodya

	[gdb]
	* mi/mi-interp.c (mi_cmd_interpreter_exec): Do no print
	^running here.
	(mi_on_resume): Print ^running if not previously output.
	* mi/mi-main.c (running_result_record_printed): New.
	(captured_mi_execute_command): Reset
	running_result_record_printed.  Use running_result_record_printed
	to decide if we should skip ^done.
	(mi_execute_async_cli_command): Don't print ^running here.
	* mi/mi-main.h (current_token, running_result_record_printed):
	Declare.

	[gdb/testsuite]
	* gdb.mi/mi-async.exp: No longer expect ^running followed by
	^done, expect just ^running.
	* gdb.mi/mi-support.exp (mi_run_cmd, mi_send_resuming_command):
	Demand that *running is output.
---
 gdb/mi/mi-interp.c                |   37 ++++++++++++++-----
 gdb/mi/mi-main.c                  |   71 ++++++++++++-------------------------
 gdb/mi/mi-main.h                  |    5 +++
 gdb/testsuite/gdb.mi/mi-async.exp |   12 +++---
 gdb/testsuite/lib/mi-support.exp  |    4 +-
 5 files changed, 63 insertions(+), 66 deletions(-)

diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index e31afc7..e329b8b 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -217,16 +217,6 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc)
 
   mi_remove_notify_hooks ();
 
-  /* Okay, now let's see if the command set the inferior going...
-     Tricky point - have to do this AFTER resetting the interpreter, since
-     changing the interpreter will clear out all the continuations for
-     that interpreter... */
-
-  if (target_can_async_p () && any_running ())
-    {
-      fputs_unfiltered ("^running\n", raw_stdout);
-    }
-
   if (mi_error_message != NULL)
     error ("%s", mi_error_message);
   do_cleanups (old_chain);
@@ -332,6 +322,21 @@ mi_on_normal_stop (struct bpstats *bs)
 static void
 mi_on_resume (ptid_t ptid)
 {
+  /* To cater for older frontends, emit ^running, but do it only once
+     per each command.  We do it here, since at this point we know
+     that the target was successfully resumed, and in non-async mode,
+     we won't return back to MI interpreter code until the target
+     is done running, so delaying the output of "^running" until then
+     will make it impossible for frontend to know what's going on.
+
+     In future (MI3), we'll be outputting "^done" here.  */
+  if (!running_result_record_printed)
+    {
+      if (current_token)
+	fputs_unfiltered (current_token, raw_stdout);
+      fputs_unfiltered ("^running\n", raw_stdout);
+    }
+
   if (PIDGET (ptid) == -1)
     fprintf_unfiltered (raw_stdout, "*running,thread-id=\"all\"\n");
   else
@@ -340,6 +345,18 @@ mi_on_resume (ptid_t ptid)
       gdb_assert (ti);
       fprintf_unfiltered (raw_stdout, "*running,thread-id=\"%d\"\n", ti->num);
     }
+
+  if (!running_result_record_printed)
+    {
+      running_result_record_printed = 1;
+      /* This is what gdb used to do historically -- printing prompt even if
+	 it cannot actually accept any input.  This will be surely removed
+	 for MI3, and may be removed even earler.  */
+      /* FIXME: review the use of target_is_async_p here -- is that
+	 what we want? */
+      if (!target_is_async_p ())
+	fputs_unfiltered ("(gdb) \n", raw_stdout);
+    }
 }
 
 extern initialize_file_ftype _initialize_mi_interp; /* -Wmissing-prototypes */
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 09ed6e3..0f7b8ed 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -94,7 +94,8 @@ static struct mi_timestamp *current_command_ts;
 
 static int do_timings = 0;
 
-static char *current_token;
+char *current_token;
+int running_result_record_printed = 1;
 
 extern void _initialize_mi_main (void);
 static enum mi_cmd_result mi_cmd_execute (struct mi_parse *parse);
@@ -1039,9 +1040,9 @@ captured_mi_execute_command (struct ui_out *uiout, void *data)
 
   struct mi_timestamp cmd_finished;
 
+  running_result_record_printed = 0;
   switch (context->op)
     {
-
     case MI_COMMAND:
       /* A MI command was read from the input stream.  */
       if (mi_debug_p)
@@ -1062,32 +1063,29 @@ captured_mi_execute_command (struct ui_out *uiout, void *data)
       if (do_timings)
 	timestamp (&cmd_finished);
 
-      if (!target_can_async_p () || !is_running (inferior_ptid))
-	{
-	  /* Print the result if there were no errors.
+      /* Print the result if there were no errors.
 
-	     Remember that on the way out of executing a command, you have
-	     to directly use the mi_interp's uiout, since the command could 
-	     have reset the interpreter, in which case the current uiout 
-	     will most likely crash in the mi_out_* routines.  */
-	  if (args->rc == MI_CMD_DONE)
-	    {
-	      fputs_unfiltered (context->token, raw_stdout);
-	      fputs_unfiltered ("^done", raw_stdout);
-	      mi_out_put (uiout, raw_stdout);
-	      mi_out_rewind (uiout);
-	      /* Have to check cmd_start, since the command could be
-		 -enable-timings.  */
-	      if (do_timings && context->cmd_start)
-		  print_diff (context->cmd_start, &cmd_finished);
-	      fputs_unfiltered ("\n", raw_stdout);
-	    }
-	  else
+	 Remember that on the way out of executing a command, you have
+	 to directly use the mi_interp's uiout, since the command could 
+	 have reset the interpreter, in which case the current uiout 
+	 will most likely crash in the mi_out_* routines.  */
+      if (args->rc == MI_CMD_DONE && !running_result_record_printed)
+	{
+	  fputs_unfiltered (context->token, raw_stdout);
+	  fputs_unfiltered ("^done", raw_stdout);
+	  mi_out_put (uiout, raw_stdout);
+	  mi_out_rewind (uiout);
+	  /* Have to check cmd_start, since the command could be
+	     -enable-timings.  */
+	  if (do_timings && context->cmd_start)
+	    print_diff (context->cmd_start, &cmd_finished);
+	  fputs_unfiltered ("\n", raw_stdout);
+	}
+      else
 	    /* The command does not want anything to be printed.  In that
 	       case, the command probably should not have written anything
 	       to uiout, but in case it has written something, discard it.  */
-	    mi_out_rewind (uiout);
-	}
+	mi_out_rewind (uiout);
       break;
 
     case CLI_COMMAND:
@@ -1109,7 +1107,7 @@ captured_mi_execute_command (struct ui_out *uiout, void *data)
 	    || current_interp_named_p (INTERP_MI2)
 	    || current_interp_named_p (INTERP_MI3))
 	  {
-	    if (args->rc == MI_CMD_DONE)
+	    if (args->rc == MI_CMD_DONE && !running_result_record_printed)
 	      {
 		fputs_unfiltered (context->token, raw_stdout);
 		fputs_unfiltered ("^done", raw_stdout);
@@ -1318,29 +1316,6 @@ mi_execute_async_cli_command (char *cli_command, char **argv, int argc)
     run = xstrprintf ("%s %s", cli_command, argc ? *argv : "");
   old_cleanups = make_cleanup (xfree, run);  
 
-  if (!target_can_async_p ())
-    {
-      /* NOTE: For synchronous targets asynchronous behavour is faked by
-         printing out the GDB prompt before we even try to execute the
-         command.  */
-      if (current_token)
-	fputs_unfiltered (current_token, raw_stdout);
-      fputs_unfiltered ("^running\n", raw_stdout);
-      fputs_unfiltered ("(gdb) \n", raw_stdout);
-      gdb_flush (raw_stdout);
-    }
-  else
-    {
-      /* FIXME: cagney/1999-11-29: Printing this message before
-         calling execute_command is wrong.  It should only be printed
-         once gdb has confirmed that it really has managed to send a
-         run command to the target.  */
-      if (current_token)
-	fputs_unfiltered (current_token, raw_stdout);
-      fputs_unfiltered ("^running\n", raw_stdout);
-
-    }
-
   execute_command ( /*ui */ run, 0 /*from_tty */ );
 
   if (target_can_async_p ())
diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h
index 1a2e93c..1aa5da2 100644
--- a/gdb/mi/mi-main.h
+++ b/gdb/mi/mi-main.h
@@ -25,5 +25,10 @@ extern void mi_load_progress (const char *section_name,
 			      unsigned long total_section,
 			      unsigned long total_sent,
 			      unsigned long grand_total);
+
+extern char *current_token;
+
+extern int running_result_record_printed;
+
 #endif
 
diff --git a/gdb/testsuite/gdb.mi/mi-async.exp b/gdb/testsuite/gdb.mi/mi-async.exp
index d2e3b47..a4efabb 100644
--- a/gdb/testsuite/gdb.mi/mi-async.exp
+++ b/gdb/testsuite/gdb.mi/mi-async.exp
@@ -61,9 +61,9 @@ proc linux_async_tests {} {
 
     send_gdb "start\n"
     gdb_expect {
-	-re ".*\\^running\r\n\\^done\r\n$mi_gdb_prompt" {
+	-re ".*\\^running\r\n(\\*running,thread-id=\"\[^\"\]+\"\r\n)$mi_gdb_prompt" {
 	    gdb_expect {
-		-re "\\*stopped,thread-id=\"0|1\",frame=\{addr=\"$hex\",func=\"main\",args=\\\[\\\],file=\".*basics.c\",line=\"$line_main_body\"\}\r\n$mi_gdb_prompt$" {
+		-re "\\*stopped,thread-id=\"0|1\",frame=\{addr=\"$hex\",func=\"main\",args=\\\[\\\],file=\".*basics.c\",line=\"$line_main_body\"\}\r\n" {
 		    pass "Asynchronous response after start command"
 		}
 		-re ".*$mi_gdb_prompt$" {
@@ -82,9 +82,9 @@ proc linux_async_tests {} {
 
     send_gdb "next\n"
     gdb_expect {
-	-re "\\^running\r\n\\^done\r\n$mi_gdb_prompt" {
+	-re ".*\\^running\r\n(\\*running,thread-id=\"\[^\"\]+\"\r\n)$mi_gdb_prompt" {
 	    gdb_expect {
-		-re "\\*stopped,reason=\"end-stepping-range\",thread-id=\"0|1\",frame=\{addr=\"$hex\",func=\"main\",args=\\\[\\\],file=\".*basics.c\",line=\"$line_main_next\"\}\r\n$mi_gdb_prompt$" {
+		-re "\\*stopped,reason=\"end-stepping-range\",thread-id=\"0|1\",frame=\{addr=\"$hex\",func=\"main\",args=\\\[\\\],file=\".*basics.c\",line=\"$line_main_next\"\}\r\n" {
 		    pass "Asynchronous response after next command"
 		}
 		-re ".*$mi_gdb_prompt$" {
@@ -107,9 +107,9 @@ proc linux_async_tests {} {
 
     send_gdb "start\n"
     gdb_expect {
-	-re ".*\\^running\r\n\\^done\r\n$mi_gdb_prompt" {
+	-re ".*\\^running\r\n(\\*running,thread-id=\"\[^\"\]+\"\r\n)$mi_gdb_prompt" {
 	    gdb_expect {
-		-re "\\*stopped,thread-id=\"0|1\",frame=\{addr=\"$hex\",func=\"main\",args=\\\[\\\],file=\".*basics.c\",line=\"$line_main_body\"\}\r\n$mi_gdb_prompt$" {
+		-re "\\*stopped,thread-id=\"0|1\",frame=\{addr=\"$hex\",func=\"main\",args=\\\[\\\],file=\".*basics.c\",line=\"$line_main_body\"\}\r\n" {
 		    pass "Asynchronous response after (re) start"
 		}
 		-re ".*$mi_gdb_prompt$" {
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 7a821b8..c8a488c 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -802,7 +802,7 @@ proc mi_run_cmd {args} {
 	if [target_info exists gdb,do_reload_on_run] {
 	    send_gdb "220-exec-continue\n";
 	    gdb_expect 60 {
-		-re "220\\^running\[\r\n\]+(\\*running,thread-id=\"\[^\"\]+\"\r\n)?$mi_gdb_prompt$" {}
+		-re "220\\^running\[\r\n\]+\\*running,thread-id=\"\[^\"\]+\"\r\n$mi_gdb_prompt$" {}
 		default {}
 	    }
 	    return;
@@ -1399,7 +1399,7 @@ proc mi_send_resuming_command {command test} {
 
     send_gdb "220-$command\n"
     gdb_expect {
-        -re "220\\^running\r\n(\\*running,thread-id=\"\[^\"\]+\"\r\n)?${mi_gdb_prompt}" {
+        -re "220\\^running\r\n\\*running,thread-id=\"\[^\"\]+\"\r\n${mi_gdb_prompt}" {
         }
         -re ".*${mi_gdb_prompt}" {
             fail "$test (failed to resume)"
-- 
1.5.3.5



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-13 22:01 [MI/RFC] Emit ^running via observer Vladimir Prus
@ 2008-06-20  6:53 ` Nick Roberts
  2008-06-25 15:04   ` Vladimir Prus
  0 siblings, 1 reply; 15+ messages in thread
From: Nick Roberts @ 2008-06-20  6:53 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > This patch fixes an issue in MI code that was present since at least 1999.
 > We output ^running before even trying to resume the target, not to mention
 > making sure the target is resumed. So, if resuming fails, we'd get ^running,
 > followed by ^error, and I don't really know if current frontends will like
 > it at all.
 > 
 > Now that we have observer for resume, and that observer is called after
 > target is resumed, we can emit ^running from that observer. The immediate
 > bonus is that ^running is now emitted for every command that resumes the
 > inferior, even for CLI commands. Another (unexpected) bonus, is that since
             ^^^^^^^^^^^^^^^^^^^^^
 > now ^running and *running is output in a single place, we can produce them
 > in consistent order.

I would like to see this patch committed!  I've not tested it but I should have
Emacs working with MI shortly and then I can regularly test this and other
changes.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-20  6:53 ` Nick Roberts
@ 2008-06-25 15:04   ` Vladimir Prus
  2008-06-26 13:33     ` Nick Roberts
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Prus @ 2008-06-25 15:04 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

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

On Friday 20 June 2008 06:48:52 Nick Roberts wrote:
>  > This patch fixes an issue in MI code that was present since at least 1999.
>  > We output ^running before even trying to resume the target, not to mention
>  > making sure the target is resumed. So, if resuming fails, we'd get ^running,
>  > followed by ^error, and I don't really know if current frontends will like
>  > it at all.
>  > 
>  > Now that we have observer for resume, and that observer is called after
>  > target is resumed, we can emit ^running from that observer. The immediate
>  > bonus is that ^running is now emitted for every command that resumes the
>  > inferior, even for CLI commands. Another (unexpected) bonus, is that since
>              ^^^^^^^^^^^^^^^^^^^^^
>  > now ^running and *running is output in a single place, we can produce them
>  > in consistent order.
> 
> I would like to see this patch committed!  I've not tested it but I should have
> Emacs working with MI shortly and then I can regularly test this and other
> changes.

I've checked in the following, which differs from original by extra test strictness.
Further, I've converted mi-async.exp to use the helper functions. Nick,
as it stands now it does not seem that mi-async.exp tests async behaviour
at all -- it merely changes that we get ^running for CLI commands, and we
get that in both sync and async mode. Do you think it worthwhile to rename
the test or move its content somewhere else?

- Volodya


[-- Attachment #2: commit.diff --]
[-- Type: text/x-diff, Size: 16693 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.9485
diff -u -p -r1.9485 ChangeLog
--- gdb/ChangeLog	24 Jun 2008 19:30:16 -0000	1.9485
+++ gdb/ChangeLog	25 Jun 2008 14:28:33 -0000
@@ -1,3 +1,17 @@
+2008-06-25  Vladimir Prus  <vladimir@codesourcery.com>
+
+	Emit ^running via observer.
+	* mi/mi-interp.c (mi_cmd_interpreter_exec): Do no print
+        ^running here.
+        (mi_on_resume): Print ^running if not previously output.
+        * mi/mi-main.c (running_result_record_printed): New.
+        (captured_mi_execute_command): Reset
+        running_result_record_printed.  Use running_result_record_printed
+        to decide if we should skip ^done.
+        (mi_execute_async_cli_command): Don't print ^running here.
+        * mi/mi-main.h (current_token, running_result_record_printed):
+        Declare.
+
 2008-06-24  Michael Snyder  <msnyder@specifix.com>
 
 	* infrun.c (_initialize_infrun): White space and typo fix.
Index: gdb/mi/mi-interp.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-interp.c,v
retrieving revision 1.32
diff -u -p -r1.32 mi-interp.c
--- gdb/mi/mi-interp.c	10 Jun 2008 10:23:54 -0000	1.32
+++ gdb/mi/mi-interp.c	25 Jun 2008 14:28:33 -0000
@@ -217,16 +217,6 @@ mi_cmd_interpreter_exec (char *command, 
 
   mi_remove_notify_hooks ();
 
-  /* Okay, now let's see if the command set the inferior going...
-     Tricky point - have to do this AFTER resetting the interpreter, since
-     changing the interpreter will clear out all the continuations for
-     that interpreter... */
-
-  if (target_can_async_p () && target_executing)
-    {
-      fputs_unfiltered ("^running\n", raw_stdout);
-    }
-
   if (mi_error_message != NULL)
     error ("%s", mi_error_message);
   do_cleanups (old_chain);
@@ -332,6 +322,21 @@ mi_on_normal_stop (struct bpstats *bs)
 static void
 mi_on_resume (ptid_t ptid)
 {
+  /* To cater for older frontends, emit ^running, but do it only once
+     per each command.  We do it here, since at this point we know
+     that the target was successfully resumed, and in non-async mode,
+     we won't return back to MI interpreter code until the target
+     is done running, so delaying the output of "^running" until then
+     will make it impossible for frontend to know what's going on.
+
+     In future (MI3), we'll be outputting "^done" here.  */
+  if (!running_result_record_printed)
+    {
+      if (current_token)
+	fputs_unfiltered (current_token, raw_stdout);
+      fputs_unfiltered ("^running\n", raw_stdout);
+    }
+
   if (PIDGET (ptid) == -1)
     fprintf_unfiltered (raw_stdout, "*running,thread-id=\"all\"\n");
   else
@@ -340,6 +345,18 @@ mi_on_resume (ptid_t ptid)
       gdb_assert (ti);
       fprintf_unfiltered (raw_stdout, "*running,thread-id=\"%d\"\n", ti->num);
     }
+
+  if (!running_result_record_printed)
+    {
+      running_result_record_printed = 1;
+      /* This is what gdb used to do historically -- printing prompt even if
+	 it cannot actually accept any input.  This will be surely removed
+	 for MI3, and may be removed even earler.  */
+      /* FIXME: review the use of target_is_async_p here -- is that
+	 what we want? */
+      if (!target_is_async_p ())
+	fputs_unfiltered ("(gdb) \n", raw_stdout);
+    }
 }
 
 extern initialize_file_ftype _initialize_mi_interp; /* -Wmissing-prototypes */
Index: gdb/mi/mi-main.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-main.c,v
retrieving revision 1.116
diff -u -p -r1.116 mi-main.c
--- gdb/mi/mi-main.c	10 Jun 2008 09:35:08 -0000	1.116
+++ gdb/mi/mi-main.c	25 Jun 2008 14:28:33 -0000
@@ -94,7 +94,8 @@ static struct mi_timestamp *current_comm
 
 static int do_timings = 0;
 
-static char *current_token;
+char *current_token;
+int running_result_record_printed = 1;
 
 extern void _initialize_mi_main (void);
 static enum mi_cmd_result mi_cmd_execute (struct mi_parse *parse);
@@ -1039,9 +1040,9 @@ captured_mi_execute_command (struct ui_o
 
   struct mi_timestamp cmd_finished;
 
+  running_result_record_printed = 0;
   switch (context->op)
     {
-
     case MI_COMMAND:
       /* A MI command was read from the input stream.  */
       if (mi_debug_p)
@@ -1062,32 +1063,29 @@ captured_mi_execute_command (struct ui_o
       if (do_timings)
 	timestamp (&cmd_finished);
 
-      if (!target_can_async_p () || !target_executing)
-	{
-	  /* Print the result if there were no errors.
+      /* Print the result if there were no errors.
 
-	     Remember that on the way out of executing a command, you have
-	     to directly use the mi_interp's uiout, since the command could 
-	     have reset the interpreter, in which case the current uiout 
-	     will most likely crash in the mi_out_* routines.  */
-	  if (args->rc == MI_CMD_DONE)
-	    {
-	      fputs_unfiltered (context->token, raw_stdout);
-	      fputs_unfiltered ("^done", raw_stdout);
-	      mi_out_put (uiout, raw_stdout);
-	      mi_out_rewind (uiout);
-	      /* Have to check cmd_start, since the command could be
-		 -enable-timings.  */
-	      if (do_timings && context->cmd_start)
-		  print_diff (context->cmd_start, &cmd_finished);
-	      fputs_unfiltered ("\n", raw_stdout);
-	    }
-	  else
+	 Remember that on the way out of executing a command, you have
+	 to directly use the mi_interp's uiout, since the command could 
+	 have reset the interpreter, in which case the current uiout 
+	 will most likely crash in the mi_out_* routines.  */
+      if (args->rc == MI_CMD_DONE && !running_result_record_printed)
+	{
+	  fputs_unfiltered (context->token, raw_stdout);
+	  fputs_unfiltered ("^done", raw_stdout);
+	  mi_out_put (uiout, raw_stdout);
+	  mi_out_rewind (uiout);
+	  /* Have to check cmd_start, since the command could be
+	     -enable-timings.  */
+	  if (do_timings && context->cmd_start)
+	    print_diff (context->cmd_start, &cmd_finished);
+	  fputs_unfiltered ("\n", raw_stdout);
+	}
+      else
 	    /* The command does not want anything to be printed.  In that
 	       case, the command probably should not have written anything
 	       to uiout, but in case it has written something, discard it.  */
-	    mi_out_rewind (uiout);
-	}
+	mi_out_rewind (uiout);
       break;
 
     case CLI_COMMAND:
@@ -1109,7 +1107,7 @@ captured_mi_execute_command (struct ui_o
 	    || current_interp_named_p (INTERP_MI2)
 	    || current_interp_named_p (INTERP_MI3))
 	  {
-	    if (args->rc == MI_CMD_DONE)
+	    if (args->rc == MI_CMD_DONE && !running_result_record_printed)
 	      {
 		fputs_unfiltered (context->token, raw_stdout);
 		fputs_unfiltered ("^done", raw_stdout);
@@ -1282,29 +1280,6 @@ mi_execute_async_cli_command (char *cli_
     run = xstrprintf ("%s %s", cli_command, argc ? *argv : "");
   old_cleanups = make_cleanup (xfree, run);  
 
-  if (!target_can_async_p ())
-    {
-      /* NOTE: For synchronous targets asynchronous behavour is faked by
-         printing out the GDB prompt before we even try to execute the
-         command.  */
-      if (current_token)
-	fputs_unfiltered (current_token, raw_stdout);
-      fputs_unfiltered ("^running\n", raw_stdout);
-      fputs_unfiltered ("(gdb) \n", raw_stdout);
-      gdb_flush (raw_stdout);
-    }
-  else
-    {
-      /* FIXME: cagney/1999-11-29: Printing this message before
-         calling execute_command is wrong.  It should only be printed
-         once gdb has confirmed that it really has managed to send a
-         run command to the target.  */
-      if (current_token)
-	fputs_unfiltered (current_token, raw_stdout);
-      fputs_unfiltered ("^running\n", raw_stdout);
-
-    }
-
   execute_command ( /*ui */ run, 0 /*from_tty */ );
 
   if (target_can_async_p ())
Index: gdb/mi/mi-main.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-main.h,v
retrieving revision 1.7
diff -u -p -r1.7 mi-main.h
--- gdb/mi/mi-main.h	1 Jan 2008 22:53:14 -0000	1.7
+++ gdb/mi/mi-main.h	25 Jun 2008 14:28:33 -0000
@@ -25,5 +25,10 @@ extern void mi_load_progress (const char
 			      unsigned long total_section,
 			      unsigned long total_sent,
 			      unsigned long grand_total);
+
+extern char *current_token;
+
+extern int running_result_record_printed;
+
 #endif
 
Index: gdb/testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.1664
diff -u -p -r1.1664 ChangeLog
--- gdb/testsuite/ChangeLog	13 Jun 2008 19:53:41 -0000	1.1664
+++ gdb/testsuite/ChangeLog	25 Jun 2008 14:28:41 -0000
@@ -1,3 +1,14 @@
+2008-06-25  Vladimir Prus  <vladimir@codesourcery.com>
+
+        * gdb.mi/mi-async.exp: Use mi_sending_resuming_command_raw and
+        mi_expect_stop.
+        * gdb.mi/mi-support.exp (mi_run_cmd, mi_send_resuming_command):
+        Demand that *running is output.
+        (detect_async): Perform checking every time.
+        (mi_send_resuming_command): Extract everything into...
+        (mi_send_resuming_command_raw): ...this.
+	(mi_expect_stop): Don't accept any output before *stopped.
+
 2008-06-13  Vladimir Prus  <vladimir@codesourcery.com>
 
 	Robustify mi-simplerun.
Index: gdb/testsuite/gdb.mi/mi-async.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-async.exp,v
retrieving revision 1.2
diff -u -p -r1.2 mi-async.exp
--- gdb/testsuite/gdb.mi/mi-async.exp	6 Apr 2008 22:41:19 -0000	1.2
+++ gdb/testsuite/gdb.mi/mi-async.exp	25 Jun 2008 14:28:41 -0000
@@ -59,72 +59,18 @@ proc linux_async_tests {} {
     set line_main_body     [expr $line_main_head + 2]
     set line_main_next     [expr $line_main_head + 3]
 
-    send_gdb "start\n"
-    gdb_expect {
-	-re ".*\\^running\r\n\\^done\r\n$mi_gdb_prompt" {
-	    gdb_expect {
-		-re "\\*stopped,thread-id=\"0|1\",frame=\{addr=\"$hex\",func=\"main\",args=\\\[\\\],file=\".*basics.c\",line=\"$line_main_body\"\}\r\n$mi_gdb_prompt$" {
-		    pass "Asynchronous response after start command"
-		}
-		-re ".*$mi_gdb_prompt$" {
-		    fail "Asynchronous response after start command (2)"
-		}
-		timeout	{
-		    fail "Asynchronous response after start command (timeout 2)"
-		}
-	    }
-	}
-	-re ".*$mi_gdb_prompt$" {
-	    fail "Asynchronous response after start command (1)"
-	}
-	timeout {fail "Asynchronous response after start command (timeout 1)"}
-    }
-
-    send_gdb "next\n"
-    gdb_expect {
-	-re "\\^running\r\n\\^done\r\n$mi_gdb_prompt" {
-	    gdb_expect {
-		-re "\\*stopped,reason=\"end-stepping-range\",thread-id=\"0|1\",frame=\{addr=\"$hex\",func=\"main\",args=\\\[\\\],file=\".*basics.c\",line=\"$line_main_next\"\}\r\n$mi_gdb_prompt$" {
-		    pass "Asynchronous response after next command"
-		}
-		-re ".*$mi_gdb_prompt$" {
-		    fail "Asynchronous response after next command (2)"
-		}
-		timeout {
-		    fail "Asynchronous response after next command (timeout 2)"
-		}
-	    }
-	}
-	-re ".*$mi_gdb_prompt$" {
-	    fail "Asynchronous response after next command (1)"
-	}
-	timeout {fail "Asynchronous response after next command (timeout 1)"}
-    }
+    mi_send_resuming_command_raw "start" "start: send"
+    mi_expect_stop "breakpoint-hit" "main" "" ".*basics.c" "$line_main_body" { "" "disp=\"del\"" } "start: stop"
+
+    mi_send_resuming_command_raw "next" "CLI next: send"
+    mi_expect_stop "end-stepping-range" "main" "" ".*basics.c" "$line_main_next" "" "CLI next: stop"
 
     mi_gdb_test "-exec-interrupt" \
 	"" \
 	""
 
-    send_gdb "start\n"
-    gdb_expect {
-	-re ".*\\^running\r\n\\^done\r\n$mi_gdb_prompt" {
-	    gdb_expect {
-		-re "\\*stopped,thread-id=\"0|1\",frame=\{addr=\"$hex\",func=\"main\",args=\\\[\\\],file=\".*basics.c\",line=\"$line_main_body\"\}\r\n$mi_gdb_prompt$" {
-		    pass "Asynchronous response after (re) start"
-		}
-		-re ".*$mi_gdb_prompt$" {
-		    fail "Asynchronous response after (re) start (2)"
-		}
-		timeout	{
-		    fail "Asynchronous response after (re) start (timeout 2)"
-		}
-	    }
-	}
-	-re ".*$mi_gdb_prompt$" {
-	    fail "Asynchronous response after (re) start (1)"
-	}
-	timeout {fail "Asynchronous response after (re) start (timeout 1)"}
-    }
+    mi_send_resuming_command_raw "start" "restart: send"
+    mi_expect_stop "breakpoint-hit" "main" "" ".*basics.c" "$line_main_body" { "" "disp=\"del\"" } "restart: stop"
 }
 
 
Index: gdb/testsuite/lib/mi-support.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/mi-support.exp,v
retrieving revision 1.62
diff -u -p -r1.62 mi-support.exp
--- gdb/testsuite/lib/mi-support.exp	10 Jun 2008 10:23:54 -0000	1.62
+++ gdb/testsuite/lib/mi-support.exp	25 Jun 2008 14:28:48 -0000
@@ -802,7 +802,7 @@ proc mi_run_cmd {args} {
 	if [target_info exists gdb,do_reload_on_run] {
 	    send_gdb "220-exec-continue\n";
 	    gdb_expect 60 {
-		-re "220\\^running\[\r\n\]+(\\*running,thread-id=\"\[^\"\]+\"\r\n)?$mi_gdb_prompt$" {}
+		-re "220\\^running\[\r\n\]+\\*running,thread-id=\"\[^\"\]+\"\r\n$mi_gdb_prompt$" {}
 		default {}
 	    }
 	    return;
@@ -919,19 +919,17 @@ proc detect_async {} {
     global async
     global mi_gdb_prompt
 
-    if { $async == "unknown" } {
-        send_gdb "maint show linux-async\n"
+    send_gdb "maint show linux-async\n"
         
-	gdb_expect {
-	    -re ".*Controlling the GNU/Linux inferior in asynchronous mode is on...*$mi_gdb_prompt$" {
-                set async 1
-	    }
-	    -re ".*$mi_gdb_prompt$" {
-                set async 0
-	    }
-            timeout {
-                set async 0
-            }
+    gdb_expect {
+        -re ".*Controlling the GNU/Linux inferior in asynchronous mode is on...*$mi_gdb_prompt$" {
+            set async 1
+        }
+        -re ".*$mi_gdb_prompt$" {
+            set async 0
+        }
+        timeout {
+            set async 0
         }
     }
     return $async
@@ -1018,13 +1016,13 @@ proc mi_expect_stop { reason func args f
 
     set a $after_reason
 
-    verbose -log "mi_expect_stop: expecting: .*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n$prompt_re$"
+    verbose -log "mi_expect_stop: expecting: \\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n$prompt_re$"
     gdb_expect {
-	-re ".*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$prompt_re$" {
+	-re "\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$prompt_re$" {
 	    pass "$test"
             return $expect_out(2,string)
 	}
-	-re ".*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\".*\",args=\[\\\[\{\].*\[\\\]\}\],file=\".*\",fullname=\"${fullname_syntax}.*\",line=\"\[0-9\]*\"\}.*\r\n$prompt_re$" {
+	-re "\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\".*\",args=\[\\\[\{\].*\[\\\]\}\],file=\".*\",fullname=\"${fullname_syntax}.*\",line=\"\[0-9\]*\"\}.*\r\n$prompt_re$" {
 	    fail "$test (stopped at wrong place)"
 	    return -1
 	}
@@ -1393,18 +1391,18 @@ proc mi_tbreak {location} {
 # Send COMMAND that must be a command that resumes
 # the inferiour (run/continue/next/etc) and consumes
 # the "^running" output from it.
-proc mi_send_resuming_command {command test} {
+proc mi_send_resuming_command_raw {command test} {
 
     global mi_gdb_prompt
 
-    send_gdb "220-$command\n"
+    send_gdb "$command\n"
     gdb_expect {
-        -re "220\\^running\r\n(\\*running,thread-id=\"\[^\"\]+\"\r\n)?${mi_gdb_prompt}" {
+        -re "\\^running\r\n\\*running,thread-id=\"\[^\"\]+\"\r\n${mi_gdb_prompt}" {
         }
         -re ".*${mi_gdb_prompt}" {
             fail "$test (failed to resume)"
         }
-        -re "220\\^error,msg=.*" {
+        -re "\\^error,msg=.*" {
             fail "$test (MI error)"
             return -1
         }
@@ -1415,6 +1413,10 @@ proc mi_send_resuming_command {command t
     }
 }
 
+proc mi_send_resuming_command {command test} {
+    mi_send_resuming_command_raw -$command $test
+}
+
 # Helper to mi_run_inline_test below.
 # Sets a temporary breakpoint at LOCATION and runs
 # the program using COMMAND.  When the program is stopped

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-25 15:04   ` Vladimir Prus
@ 2008-06-26 13:33     ` Nick Roberts
  2008-06-26 18:54       ` Vladimir Prus
  0 siblings, 1 reply; 15+ messages in thread
From: Nick Roberts @ 2008-06-26 13:33 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > I've checked in the following, which differs from original by extra test
 > strictness.  

Thanks.

 >              Further, I've converted mi-async.exp to use the helper
 > functions. Nick, as it stands now it does not seem that mi-async.exp tests
 > async behaviour at all -- it merely changes that we get ^running for CLI
 > commands, and we get that in both sync and async mode. Do you think it
 > worthwhile to rename the test or move its content somewhere else?

The tests appear to fail now.  However, there is still a difference in output
for sync and async behaviour and this is what the test is for.

ASYNC:

(gdb) 
r
&"r\n"
~"Starting program: /home/nickrob/myprog \n"
=thread-created,id="1"
^running
*running,thread-id="all"
(gdb) 
*stopped,reason="breakpoint-hit",disp="keep",bkptno="1",thread-id="1",frame={addr="0x08048706",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbfaf2db4"}],file="myprog.c",fullname="/home/nickrob/myprog.c",line="146"}


SYNC:

r
&"r\n"
~"Starting program: /home/nickrob/myprog \n"
=thread-created,id="1"
^running
*running,thread-id="all"
(gdb) 
~"\n"
~"Breakpoint 1, main (argc=1, argv=0xbf961424) at myprog.c:146\n"
~"146\tmain (int argc, char **argv) {\n"
*stopped
(gdb) 


i.e no reason, frame, file, etc fields.  It's important for the console in a
frontend that the CLI command generates the same MI output as the corresponding
MI command.

The async version now seems to be missing a final "(gdb) \n".  Maybe that's
why the tests are failing.


-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-26 13:33     ` Nick Roberts
@ 2008-06-26 18:54       ` Vladimir Prus
  2008-06-26 19:34         ` Daniel Jacobowitz
  2008-06-27  6:58         ` Nick Roberts
  0 siblings, 2 replies; 15+ messages in thread
From: Vladimir Prus @ 2008-06-26 18:54 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Thursday 26 June 2008 07:20:14 Nick Roberts wrote:
>  > I've checked in the following, which differs from original by extra test
>  > strictness.  
> 
> Thanks.
> 
>  >              Further, I've converted mi-async.exp to use the helper
>  > functions. Nick, as it stands now it does not seem that mi-async.exp tests
>  > async behaviour at all -- it merely changes that we get ^running for CLI
>  > commands, and we get that in both sync and async mode. Do you think it
>  > worthwhile to rename the test or move its content somewhere else?
> 
> The tests appear to fail now. 

Which tests? With unmodified CVS state, all MI tests pass for me both in sync
and async mode. But indeed, if I make mi-async.exp not force async mode, it
starts to fail....

> SYNC:
> 
> r
> &"r\n"
> ~"Starting program: /home/nickrob/myprog \n"
> =thread-created,id="1"
> ^running
> *running,thread-id="all"
> (gdb) 
> ~"\n"
> ~"Breakpoint 1, main (argc=1, argv=0xbf961424) at myprog.c:146\n"
> ~"146\tmain (int argc, char **argv) {\n"
> *stopped
> (gdb) 

... in this way. 

> i.e no reason, frame, file, etc fields.  It's important for the console in a
> frontend that the CLI command generates the same MI output as the corresponding
> MI command.

Of course. This is pre-existing problem, though, and was present in gdb 6.8 --
except that we did not output neither ^running nor *stopped at all -- so apparently
making mi-async.exp not actually enable async mode is a bit premature yet. The problem,
it appears, is that while the CLI command executes, 'uiout' is the CLI interpreter's
uiout, not MI uiout. I'll probably won't have time to work on this in next month,
so patches welcome.

- Volodya





^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-26 18:54       ` Vladimir Prus
@ 2008-06-26 19:34         ` Daniel Jacobowitz
  2008-06-28 11:34           ` Vladimir Prus
  2008-06-27  6:58         ` Nick Roberts
  1 sibling, 1 reply; 15+ messages in thread
From: Daniel Jacobowitz @ 2008-06-26 19:34 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: Nick Roberts, gdb-patches

On Thu, Jun 26, 2008 at 07:58:05PM +0400, Vladimir Prus wrote:
> On Thursday 26 June 2008 07:20:14 Nick Roberts wrote:
> >  > I've checked in the following, which differs from original by extra test
> >  > strictness.  
> > 
> > Thanks.
> > 
> >  >              Further, I've converted mi-async.exp to use the helper
> >  > functions. Nick, as it stands now it does not seem that mi-async.exp tests
> >  > async behaviour at all -- it merely changes that we get ^running for CLI
> >  > commands, and we get that in both sync and async mode. Do you think it
> >  > worthwhile to rename the test or move its content somewhere else?
> > 
> > The tests appear to fail now. 
> 
> Which tests? With unmodified CVS state, all MI tests pass for me both in sync
> and async mode. But indeed, if I make mi-async.exp not force async mode, it
> starts to fail....

Not for me.

(gdb)
start
&"start\n"
~"Temporary breakpoint 1 at 0x400635: file /space/fsf/commit/src/gdb/testsuite/gdb.mi/basics.c, line 62.\n"
~"Starting program: /space/fsf/x86-64/commit-gdb/gdb/testsuite/gdb.mi/basics \n"
=thread-created,id="1"
^running
*running,thread-id="all"
*stopped,reason="breakpoint-hit",disp="del",bkptno="1",thread-id="1",frame={addr="0x0000000000400635",func="main",args=[],file="/space/fsf/commit/src/gdb/testsuite/gdb.mi/basics.c",fullname="/space/fsf/commit/src/gdb/testsuite/gdb.mi/basics.c",line="62"}
FAIL: gdb.mi/mi-async.exp: start: send
mi_expect_stop: expecting: \*stopped,reason="breakpoint-hit",disp="del",bkptno="[0-9]+",thread-id="[0-9]+",frame={addr="0x[0-9A-Fa-f]+",func="main",args=\[\],file=".*.*basics.c",fullname="(/.*/|\\\\[^\\]+\\.+\\|\\[^\\].*\\|[a-zA-Z]:.*\\).*basics.c",line="62"}
$
PASS: gdb.mi/mi-async.exp: start: stop

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-26 18:54       ` Vladimir Prus
  2008-06-26 19:34         ` Daniel Jacobowitz
@ 2008-06-27  6:58         ` Nick Roberts
  2008-06-27 11:58           ` Vladimir Prus
  1 sibling, 1 reply; 15+ messages in thread
From: Nick Roberts @ 2008-06-27  6:58 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > > The tests appear to fail now. 
 > 
 > Which tests? 

All three in mi-async.exp, at the moment.  I'll look at them more closely to
see if they are due to local changes.

 >  With unmodified CVS state, all MI tests pass for me both in sync
 > and async mode. But indeed, if I make mi-async.exp not force async mode, it
 > starts to fail....

mi-async.exp is a test for async mode, so is not expected to pass in sync mode.

 > > SYNC:
 > > 
 > > r
 > > &"r\n"
 > > ~"Starting program: /home/nickrob/myprog \n"
 > > =thread-created,id="1"
 > > ^running
 > > *running,thread-id="all"
 > > (gdb) 
 > > ~"\n"
 > > ~"Breakpoint 1, main (argc=1, argv=0xbf961424) at myprog.c:146\n"
 > > ~"146\tmain (int argc, char **argv) {\n"
 > > *stopped
 > > (gdb) 
 > 
 > ... in this way. 
 > 
 > > i.e no reason, frame, file, etc fields.  It's important for the console in
 > > a frontend that the CLI command generates the same MI output as the
 > > corresponding MI command.
 > 
 > Of course. This is pre-existing problem, though, and was present in gdb 6.8
 > -- except that we did not output neither ^running nor *stopped at all -- so
 > apparently making mi-async.exp not actually enable async mode is a bit
 > premature yet. The problem, it appears, is that while the CLI command
 > executes, 'uiout' is the CLI interpreter's uiout, not MI uiout. I'll
 > probably won't have time to work on this in next month, so patches welcome.

It's not a problem if async mode becomes the default, which is my
understanding.  In sync mode, t may be possible to switch to the underlying MI
interpreter while in "proceed" but I think that using async mode is cleaner and
I have no plans to make sync mode work in this way.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-27  6:58         ` Nick Roberts
@ 2008-06-27 11:58           ` Vladimir Prus
  2008-06-28  5:49             ` Nick Roberts
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Prus @ 2008-06-27 11:58 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Friday 27 June 2008 04:14:18 Nick Roberts wrote:
>  > > The tests appear to fail now. 
>  > 
>  > Which tests? 
> 
> All three in mi-async.exp, at the moment.  I'll look at them more closely to
> see if they are due to local changes.
> 
>  >  With unmodified CVS state, all MI tests pass for me both in sync
>  > and async mode. But indeed, if I make mi-async.exp not force async mode, it
>  > starts to fail....
> 
> mi-async.exp is a test for async mode, so is not expected to pass in sync mode.

That's the question -- what about this test is specific to async mode?

>  > > SYNC:
>  > > 
>  > > r
>  > > &"r\n"
>  > > ~"Starting program: /home/nickrob/myprog \n"
>  > > =thread-created,id="1"
>  > > ^running
>  > > *running,thread-id="all"
>  > > (gdb) 
>  > > ~"\n"
>  > > ~"Breakpoint 1, main (argc=1, argv=0xbf961424) at myprog.c:146\n"
>  > > ~"146\tmain (int argc, char **argv) {\n"
>  > > *stopped
>  > > (gdb) 
>  > 
>  > ... in this way. 
>  > 
>  > > i.e no reason, frame, file, etc fields.  It's important for the console in
>  > > a frontend that the CLI command generates the same MI output as the
>  > > corresponding MI command.
>  > 
>  > Of course. This is pre-existing problem, though, and was present in gdb 6.8
>  > -- except that we did not output neither ^running nor *stopped at all -- so
>  > apparently making mi-async.exp not actually enable async mode is a bit
>  > premature yet. The problem, it appears, is that while the CLI command
>  > executes, 'uiout' is the CLI interpreter's uiout, not MI uiout. I'll
>  > probably won't have time to work on this in next month, so patches welcome.
> 
> It's not a problem if async mode becomes the default, which is my
> understanding.  

Not mine, unfortunately. We can't even make all target always have at least
one element in thread list -- which is much simpler change.

- Volodya


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-27 11:58           ` Vladimir Prus
@ 2008-06-28  5:49             ` Nick Roberts
  2008-06-28  9:41               ` Vladimir Prus
  0 siblings, 1 reply; 15+ messages in thread
From: Nick Roberts @ 2008-06-28  5:49 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > > mi-async.exp is a test for async mode, so is not expected to pass in sync
 > > mode.
 > 
 > That's the question -- what about this test is specific to async mode?

Async mode decouples the output from the input.  This allows a CLI command that
executes the inferior to (indirectly) generate MI output.  That's why I was
interested in async mode and that's what this test is for.  The work has
actually been done for more general reasons, such as non-stop mode.  I think
the MI output is just a fortunate side-effect.

 >...
 > > It's not a problem if async mode becomes the default, which is my
 > > understanding.  
 > 
 > Not mine, unfortunately. We can't even make all target always have at least
 > one element in thread list -- which is much simpler change.

Maybe that's a requirement of non-stop mode but I'm not sure that this is
relevant here, i.e. with just async mode.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-28  5:49             ` Nick Roberts
@ 2008-06-28  9:41               ` Vladimir Prus
  2008-06-28 10:16                 ` Nick Roberts
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Prus @ 2008-06-28  9:41 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Saturday 28 June 2008 04:14:27 Nick Roberts wrote:
>  > > mi-async.exp is a test for async mode, so is not expected to pass in sync
>  > > mode.
>  > 
>  > That's the question -- what about this test is specific to async mode?
> 
> Async mode decouples the output from the input.  This allows a CLI command that
> executes the inferior to (indirectly) generate MI output.  That's why I was
> interested in async mode and that's what this test is for.  The work has
> actually been done for more general reasons, such as non-stop mode.  I think
> the MI output is just a fortunate side-effect.

Err, async mode does one thing -- allows commands to be processed while inferiour
is still running. Unless you explicitly make use of this functionality, there's
no difference from sync mode. (Of course, there's a bunch of checks for 
target_can_async_p in GDB code, so some difference in output is possible, but
in theory there should be none). I don't know what you mean by decoupling output
from the input -- for example, then *running notification is emitted for CLI
just fine, and this does not require async mode.

>  >...
>  > > It's not a problem if async mode becomes the default, which is my
>  > > understanding.  
>  > 
>  > Not mine, unfortunately. We can't even make all target always have at least
>  > one element in thread list -- which is much simpler change.
> 
> Maybe that's a requirement of non-stop mode but I'm not sure that this is
> relevant here, i.e. with just async mode.

I meant relative complexity of things. Making the thread list always have a single thread 
is relatively straight-forward, but still, making this happen for 
*all* targets is going to take lots of time. Adding async support for a target
is considerably harder -- as linux-nat.c changes clearly demonstrate. Therefore, 
I don't expect that GDB will support async on a wide set of targets anytime soon.

- Volodya


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-28  9:41               ` Vladimir Prus
@ 2008-06-28 10:16                 ` Nick Roberts
  2008-06-28 10:44                   ` Vladimir Prus
  0 siblings, 1 reply; 15+ messages in thread
From: Nick Roberts @ 2008-06-28 10:16 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > > Async mode decouples the output from the input.  This allows a CLI command
 > > that executes the inferior to (indirectly) generate MI output.  That's why
 > > I was interested in async mode and that's what this test is for.  The work
 > > has actually been done for more general reasons, such as non-stop mode.  I
 > > think the MI output is just a fortunate side-effect.
 > 
 > Err, async mode does one thing -- allows commands to be processed while
 > inferiour is still running. Unless you explicitly make use of this
 > functionality, there's no difference from sync mode. (Of course, there's a
 > bunch of checks for target_can_async_p in GDB code, so some difference in
 > output is possible, but in theory there should be none). I don't know what
 > you mean by decoupling output from the input -- for example, then *running
 > notification is emitted for CLI just fine, and this does not require async
 > mode.

This is my understanding:

*running is presumably an observer that is only set in MI and fires whenever
execution (re)starts.  It doesn't depend on the (immediate) interpreter.  In
sync mode when a CLI command starts execution, GDB waits for it to stop before
switching back to MI and returning to the event loop.  In async mode it
doesn't, and when inferior_event_handler runs after execution has stopped,
GDB has already switched back to MI and so prints MI output.


-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-28 10:16                 ` Nick Roberts
@ 2008-06-28 10:44                   ` Vladimir Prus
  2008-06-29  2:40                     ` Nick Roberts
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Prus @ 2008-06-28 10:44 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Saturday 28 June 2008 13:40:29 Nick Roberts wrote:
>  > > Async mode decouples the output from the input.  This allows a CLI command
>  > > that executes the inferior to (indirectly) generate MI output.  That's why
>  > > I was interested in async mode and that's what this test is for.  The work
>  > > has actually been done for more general reasons, such as non-stop mode.  I
>  > > think the MI output is just a fortunate side-effect.
>  > 
>  > Err, async mode does one thing -- allows commands to be processed while
>  > inferiour is still running. Unless you explicitly make use of this
>  > functionality, there's no difference from sync mode. (Of course, there's a
>  > bunch of checks for target_can_async_p in GDB code, so some difference in
>  > output is possible, but in theory there should be none). I don't know what
>  > you mean by decoupling output from the input -- for example, then *running
>  > notification is emitted for CLI just fine, and this does not require async
>  > mode.
> 
> This is my understanding:
> 
> *running is presumably an observer that is only set in MI and fires whenever
> execution (re)starts.  It doesn't depend on the (immediate) interpreter.  In
> sync mode when a CLI command starts execution, GDB waits for it to stop before
> switching back to MI and returning to the event loop.  In async mode it
> doesn't, and when inferior_event_handler runs after execution has stopped,
> GDB has already switched back to MI and so prints MI output.

I still don't understand what you mean by "decouples the output from the input".
GDB, both in sync and async mode is in perfect position to print things as it sees
fit. And MI notifications don't actually care about 'current interpreter'. The
*only* fundamental limitation of sync mode is that no command will be processed
when the target is running. Am I missing something?

- Volodya


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-26 19:34         ` Daniel Jacobowitz
@ 2008-06-28 11:34           ` Vladimir Prus
  2008-06-28 16:35             ` Vladimir Prus
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Prus @ 2008-06-28 11:34 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Nick Roberts, gdb-patches

On Thursday 26 June 2008 20:03:09 Daniel Jacobowitz wrote:
> On Thu, Jun 26, 2008 at 07:58:05PM +0400, Vladimir Prus wrote:
> > On Thursday 26 June 2008 07:20:14 Nick Roberts wrote:
> > >  > I've checked in the following, which differs from original by extra test
> > >  > strictness.  
> > > 
> > > Thanks.
> > > 
> > >  >              Further, I've converted mi-async.exp to use the helper
> > >  > functions. Nick, as it stands now it does not seem that mi-async.exp tests
> > >  > async behaviour at all -- it merely changes that we get ^running for CLI
> > >  > commands, and we get that in both sync and async mode. Do you think it
> > >  > worthwhile to rename the test or move its content somewhere else?
> > > 
> > > The tests appear to fail now. 
> > 
> > Which tests? With unmodified CVS state, all MI tests pass for me both in sync
> > and async mode. But indeed, if I make mi-async.exp not force async mode, it
> > starts to fail....
> 
> Not for me.
> 
> (gdb)
> start
> &"start\n"
> ~"Temporary breakpoint 1 at 0x400635: file /space/fsf/commit/src/gdb/testsuite/gdb.mi/basics.c, line 62.\n"
> ~"Starting program: /space/fsf/x86-64/commit-gdb/gdb/testsuite/gdb.mi/basics \n"
> =thread-created,id="1"
> ^running
> *running,thread-id="all"
> *stopped,reason="breakpoint-hit",disp="del",bkptno="1",thread-id="1",frame={addr="0x0000000000400635",func="main",args=[],file="/space/fsf/commit/src/gdb/testsuite/gdb.mi/basics.c",fullname="/space/fsf/commit/src/gdb/testsuite/gdb.mi/basics.c",line="62"}
> FAIL: gdb.mi/mi-async.exp: start: send

The test expects (gdb) after *running. I don't actually understand how it can not be
output -- the command is handled by mi_execute_command, which prints prompts at the
end. Ah wait, except for this early exit path:

      if (args.action == EXECUTE_COMMAND_SUPPRESS_PROMPT)
	/* The command is executing synchronously.  Bail out early
	   suppressing the finished prompt.  */
	return;

and then:

==20541== Conditional jump or move depends on uninitialised value(s)
==20541==    at 0x80DCD38: mi_execute_command (mi-main.c:1138)

I'll fix this shortly -- one way or another.

- Volodya


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-28 11:34           ` Vladimir Prus
@ 2008-06-28 16:35             ` Vladimir Prus
  0 siblings, 0 replies; 15+ messages in thread
From: Vladimir Prus @ 2008-06-28 16:35 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Nick Roberts, gdb-patches

On Saturday 28 June 2008 14:43:56 Vladimir Prus wrote:

> > ^running
> > *running,thread-id="all"
> > *stopped,reason="breakpoint-hit",disp="del",bkptno="1",thread-id="1",frame={addr="0x0000000000400635",func="main",args=[],file="/space/fsf/commit/src/gdb/testsuite/gdb.mi/basics.c",fullname="/space/fsf/commit/src/gdb/testsuite/gdb.mi/basics.c",line="62"}
> > FAIL: gdb.mi/mi-async.exp: start: send
> 
> The test expects (gdb) after *running. I don't actually understand how it can not be
> output -- the command is handled by mi_execute_command, which prints prompts at the
> end. Ah wait, except for this early exit path:
> 
>       if (args.action == EXECUTE_COMMAND_SUPPRESS_PROMPT)
> 	/* The command is executing synchronously.  Bail out early
> 	   suppressing the finished prompt.  */
> 	return;
> 
> and then:
> 
> ==20541== Conditional jump or move depends on uninitialised value(s)
> ==20541==    at 0x80DCD38: mi_execute_command (mi-main.c:1138)
> 
> I'll fix this shortly -- one way or another.

Should be fixed now. Let me know if you continue to have test failures.

- Volodya


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [MI/RFC] Emit ^running via observer.
  2008-06-28 10:44                   ` Vladimir Prus
@ 2008-06-29  2:40                     ` Nick Roberts
  0 siblings, 0 replies; 15+ messages in thread
From: Nick Roberts @ 2008-06-29  2:40 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > I still don't understand what you mean by "decouples the output from the
 > input".  GDB, both in sync and async mode is in perfect position to print
 > things as it sees fit. And MI notifications don't actually care about
 > 'current interpreter'. The *only* fundamental limitation of sync mode is
 > that no command will be processed when the target is running. Am I missing
 > something?

I think we're saying similar things.  You're saying that sync mode should
be able to generate MI output from CLI commands like "run", "next" etc.  I'm
just saying that I don't know how to do it and it's probably quite messy.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2008-06-29  0:08 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-06-13 22:01 [MI/RFC] Emit ^running via observer Vladimir Prus
2008-06-20  6:53 ` Nick Roberts
2008-06-25 15:04   ` Vladimir Prus
2008-06-26 13:33     ` Nick Roberts
2008-06-26 18:54       ` Vladimir Prus
2008-06-26 19:34         ` Daniel Jacobowitz
2008-06-28 11:34           ` Vladimir Prus
2008-06-28 16:35             ` Vladimir Prus
2008-06-27  6:58         ` Nick Roberts
2008-06-27 11:58           ` Vladimir Prus
2008-06-28  5:49             ` Nick Roberts
2008-06-28  9:41               ` Vladimir Prus
2008-06-28 10:16                 ` Nick Roberts
2008-06-28 10:44                   ` Vladimir Prus
2008-06-29  2:40                     ` Nick Roberts

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox