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


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

Re: [MI/RFC] Emit ^running via observer.


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

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

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