[MI tracepoints 4/9] -trace-start/end/status

[MI tracepoints 4/9] -trace-start/end/status

[Vladimir Prus]

[-- Attachment #1: Type: Text/Plain, Size: 222 bytes --]


This patch implement a few of tracing commands. The changes
in non-MI codepaths are nothing more than "extract function"
refactoring.

Thanks,

-- 
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722

[-- Attachment #2: 4-start-end-status.diff --]
[-- Type: text/x-patch, Size: 8275 bytes --]

commit 8c35f248ff803a25555554c01efdedb459434477
Author: vladimir <vladimir@e7755896-6108-0410-9592-8049d3e74e28>
Date:   Thu Aug 6 07:03:19 2009 +0000

    -trace-start/-trace-end/-trace-status.
    
    	* mi/mi-cmds.c (mi_cmds): Register -trace-start, -trace-status
    	and -trace-stop.
    	* mi/mi-cmds.h (mi_cmd_trace_start, mi_cmd_trace_status)
    	(mi_cmd_trace_stop): Declare.
    	* mi/mi-main.c (mi_cmd_trace_start, mi_cmd_trace_status)
    	(mi_cmd_trace_stop): New.
    	* tracepoint.c (start_tracing): New, extracted from...
    	(trace_start_command): ...this.
    	(trace_status_mi): New.
    	* tracepoint.h (struct trace_status): Document
    	stopping_tracepoint.
    	(start_tracing, stop_traceing, trace_status_mi): Declare.

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 7f18d94..c2183fb 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -105,6 +105,9 @@ struct mi_cmd mi_cmds[] =
   { "thread-info", { NULL, 0 }, mi_cmd_thread_info },
   { "thread-list-ids", { NULL, 0 }, mi_cmd_thread_list_ids},
   { "thread-select", { NULL, 0 }, mi_cmd_thread_select},
+  { "trace-start", { NULL, 0 }, mi_cmd_trace_start },
+  { "trace-status", { NULL, 0 }, mi_cmd_trace_status },
+  { "trace-stop", { NULL, 0 }, mi_cmd_trace_stop },
   { "var-assign", { NULL, 0 }, mi_cmd_var_assign},
   { "var-create", { NULL, 0 }, mi_cmd_var_create},
   { "var-delete", { NULL, 0 }, mi_cmd_var_delete},
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 20bdbfd..702dd9e 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -88,6 +88,9 @@ extern mi_cmd_argv_ftype mi_cmd_target_file_delete;
 extern mi_cmd_argv_ftype mi_cmd_thread_info;
 extern mi_cmd_argv_ftype mi_cmd_thread_list_ids;
 extern mi_cmd_argv_ftype mi_cmd_thread_select;
+extern mi_cmd_argv_ftype mi_cmd_trace_start;
+extern mi_cmd_argv_ftype mi_cmd_trace_status;
+extern mi_cmd_argv_ftype mi_cmd_trace_stop;
 extern mi_cmd_argv_ftype mi_cmd_var_assign;
 extern mi_cmd_argv_ftype mi_cmd_var_create;
 extern mi_cmd_argv_ftype mi_cmd_var_delete;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index f0b2f32..a422ae3 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -51,6 +51,7 @@
 #include "inferior.h"
 #include "osdata.h"
 #include "splay-tree.h"
+#include "tracepoint.h"
 
 #include <ctype.h>
 #include <sys/time.h>
@@ -2079,3 +2080,23 @@ print_diff (struct mi_timestamp *start, struct mi_timestamp *end)
        timeval_diff (start->utime, end->utime) / 1000000.0, 
        timeval_diff (start->stime, end->stime) / 1000000.0);
   }
+
+void
+mi_cmd_trace_start (char *command, char **argv, int argc)
+{
+  start_tracing ();
+}
+
+void
+mi_cmd_trace_status (char *command, char **argv, int argc)
+{
+  trace_status_mi (0);
+}
+
+void
+mi_cmd_trace_stop (char *command, char **argv, int argc)
+{
+  stop_tracing ();
+  trace_status_mi (1);
+}
+
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 38f41b7..1c17f03 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1386,15 +1386,9 @@ add_aexpr (struct collection_list *collect, struct agent_expr *aexpr)
   collect->next_aexpr_elt++;
 }
 
-/* tstart command:
-
-   Tell target to clear any previous trace experiment.
-   Walk the list of tracepoints, and send them (and their actions)
-   to the target.  If no errors, 
-   Tell target to start a new trace experiment.  */
 
-static void
-trace_start_command (char *args, int from_tty)
+void
+start_tracing (void)
 {
   char buf[2048];
   VEC(breakpoint_p) *tp_vec = NULL;
@@ -1403,8 +1397,6 @@ trace_start_command (char *args, int from_tty)
   struct trace_state_variable *tsv;
   int any_downloaded = 0;
 
-  dont_repeat ();	/* Like "run", dangerous to repeat accidentally.  */
-
   target_trace_init ();
   
   tp_vec = all_tracepoints ();
@@ -1440,6 +1432,21 @@ trace_start_command (char *args, int from_tty)
   current_trace_status()->running = 1;
 }
 
+/* tstart command:
+
+   Tell target to clear any previous trace experiment.
+   Walk the list of tracepoints, and send them (and their actions)
+   to the target.  If no errors,
+   Tell target to start a new trace experiment.  */
+
+static void
+trace_start_command (char *args, int from_tty)
+{
+  dont_repeat ();	/* Like "run", dangerous to repeat accidentally.  */
+
+  start_tracing ();
+}
+
 /* tstop command */
 static void
 trace_stop_command (char *args, int from_tty)
@@ -1448,7 +1455,7 @@ trace_stop_command (char *args, int from_tty)
 }
 
 void
-stop_tracing ()
+stop_tracing (void)
 {
   target_trace_stop ();
   /* should change in response to reply? */
@@ -1504,7 +1511,6 @@ trace_status_command (char *args, int from_tty)
 	  printf_filtered (_("Trace stopped because of disconnection.\n"));
 	  break;
 	case tracepoint_passcount:
-	  /* FIXME account for number on target */
 	  printf_filtered (_("Trace stopped by tracepoint %d.\n"),
 			   ts->stopping_tracepoint);
 	  break;
@@ -1538,6 +1544,91 @@ trace_status_command (char *args, int from_tty)
     printf_filtered (_("Not looking at any trace frame.\n"));
 }
 
+/* Report the trace status to uiout, in a way suitable for MI, and not
+   suitable for CLI. If ON_STOP is true, suppress a few fields that
+   are not meaningful in -trace-stop response.
+
+   The implementation is essentially parallel to trace_status_command, but
+   merging them will result in unreadable code.
+ */
+void
+trace_status_mi (int on_stop)
+{
+  struct trace_status *ts = current_trace_status ();
+  int status;
+  char *string_status;
+
+  status = target_get_trace_status (ts);
+
+  if (status == -1 && !ts->from_file)
+    {
+      ui_out_field_string (uiout, "supported", "0");
+      return;
+    }
+
+  if (ts->from_file)
+    ui_out_field_string (uiout, "supported", "file");
+  else if (!on_stop)
+    ui_out_field_string (uiout, "supported", "1");
+
+  gdb_assert (ts->running_known);
+
+  if (ts->running)
+    {
+      ui_out_field_string (uiout, "running", "1");
+
+      /** Unlike CLI, do not show the state of 'disconnected-tracing' variable.
+	  Given that frontend gets the status either on -trace-stop, or from
+	  -trace-status after re-connection, it does not seem like this
+	  information is necessary for anything. It is not necessary for either
+	  figuring the vital state of the target nor for navigation of trace
+	  frames. If the frontend wants to show the current state is some
+	  configure dialog, it can request the value when such dialog is
+	  invoked by the user.  */
+    }
+  else
+    {
+      char *stop_reason = NULL;
+      int stopping_tracepoint = -1;
+
+      if (!on_stop)
+	ui_out_field_string (uiout, "running", "0");
+
+      gdb_assert (ts->stop_reason != trace_stop_reason_unknown);
+
+      switch (ts->stop_reason)
+	{
+	case tstop_command:
+	  stop_reason = "request";
+	  break;
+	case trace_buffer_full:
+	  stop_reason = "overflow";
+	  break;
+	case trace_disconnected:
+	  stop_reason = "disconnection";
+	  break;
+	case tracepoint_passcount:
+	  stop_reason = "passcount";
+	  stopping_tracepoint = ts->stopping_tracepoint;
+	  break;
+	}
+
+      if (stop_reason)
+	{
+	  ui_out_field_string (uiout, "stop-reason", stop_reason);
+	  if (stopping_tracepoint != -1)
+	    ui_out_field_int (uiout, "stopping-tracepoint",
+			      stopping_tracepoint);
+	}
+    }
+
+  ui_out_field_int (uiout, "frames", ts->traceframe_count);
+
+  ui_out_field_int (uiout, "buffer-size",  (int) ts->buffer_size);
+  ui_out_field_int (uiout, "buffer-free",  (int) ts->buffer_free);
+}
+
+
 void
 disconnect_or_stop_tracing (int from_tty)
 {
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index 446b292..2e9193c 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -86,6 +86,8 @@ struct trace_status
 
   enum trace_stop_reason stop_reason;
 
+  /* If stop_reason == tracepoint_passcount, the on-target number
+     of the tracepoint which caused the stop.  */
   int stopping_tracepoint;
 
   int traceframe_count;
@@ -155,4 +157,9 @@ extern struct breakpoint *create_tracepoint_from_upload (struct uploaded_tp *utp
 extern void merge_uploaded_tracepoints (struct uploaded_tp **utpp);
 extern void merge_uploaded_trace_state_variables (struct uploaded_tsv **utsvp);
 
+extern void start_tracing (void);
+extern void stop_tracing (void);
+
+extern void trace_status_mi (int on_stop);
+
 #endif	/* TRACEPOINT_H */

Re: [MI tracepoints 4/9] -trace-start/end/status

[Pedro Alves]

On Sunday 14 March 2010 08:54:07, Vladimir Prus wrote:

>         (start_tracing, stop_traceing, trace_status_mi): Declare.

Typo: stop_traceing.

> +/* Report the trace status to uiout, in a way suitable for MI, and not
> +   suitable for CLI. If ON_STOP is true, suppress a few fields that

Double space after period.

> +   are not meaningful in -trace-stop response.

"the -trace-stop response"

> +
> +   The implementation is essentially parallel to trace_status_command, but
> +   merging them will result in unreadable code.
> + */

*/ at end of comment, not in new line:

 "... in unreadable code.  */" 

> +      /** Unlike CLI, do not show the state of 'disconnected-tracing' variable.

s,/**,/* :

    /* Unlike CLI,


> +         Given that frontend gets the status either on -trace-stop, or from

"the frontend gets"

> +         -trace-status after re-connection, it does not seem like this
> +         information is necessary for anything. It is not necessary for either
> +         figuring the vital state of the target nor for navigation of trace
> +         frames. If the frontend wants to show the current state is some
> +         configure dialog, it can request the value when such dialog is
> +         invoked by the user.  */

Double space after period everywhere please.

+  ui_out_field_int (uiout, "buffer-size",  (int) ts->buffer_size);
+  ui_out_field_int (uiout, "buffer-free",  (int) ts->buffer_free);

When these optional fields aren't reported by the target, this
will print -1.  I don't see that mentioned in the manual.  Is
this intended?  I would have expected optional fields to just
not be present, as is common in MI.  trace_status_command does
that too.

Hmmm, I don't see any of the qTStatus optional fields described
anywhere in manual.  Stan?  Am I just looking at the wrong
place?

-- 
Pedro Alves

Re: [MI tracepoints 4/9] -trace-start/end/status

[Pedro Alves]

I meant to also say:

"otherwise, looks good to me."

-- 
Pedro Alves

Re: [MI tracepoints 4/9] -trace-start/end/status

[Stan Shebs]

Pedro Alves wrote:
> Hmmm, I don't see any of the qTStatus optional fields described
> anywhere in manual.  Stan?  Am I just looking at the wrong
> place?
>   
I was holding off until things were "more done" - there was a period 
where it seemed like we were getting weekly requests for additions. :-)

I'm about to post the circular trace buffer patch, which is one of the 
ones that adds an additional status field.  I'll add docs to that one.

Stan

Re: [MI tracepoints 4/9] -trace-start/end/status

[Vladimir Prus]

[-- Attachment #1: Type: Text/Plain, Size: 719 bytes --]

On Monday 15 March 2010 21:46:05 Pedro Alves wrote:

> Double space after period everywhere please.
> 
> +  ui_out_field_int (uiout, "buffer-size",  (int) ts->buffer_size);
> +  ui_out_field_int (uiout, "buffer-free",  (int) ts->buffer_free);
> 
> When these optional fields aren't reported by the target, this
> will print -1.  I don't see that mentioned in the manual.  Is
> this intended?  I would have expected optional fields to just
> not be present, as is common in MI.  trace_status_command does
> that too.

I did not realize they are optional. Attached is the revised patch,
that I'll commit when prior patches are ready.

Thanks,

-- 
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722

[-- Attachment #2: 4-start-end-status-2.diff --]
[-- Type: text/x-patch, Size: 8350 bytes --]

commit 2ee525e212e0aefa844b5a3e68a383f711e6cbfa
Author: vladimir <vladimir@e7755896-6108-0410-9592-8049d3e74e28>
Date:   Thu Aug 6 07:03:19 2009 +0000

    -trace-start/-trace-end/-trace-status.
    
    	* mi/mi-cmds.c (mi_cmds): Register -trace-start, -trace-status
    	and -trace-stop.
    	* mi/mi-cmds.h (mi_cmd_trace_start, mi_cmd_trace_status)
    	(mi_cmd_trace_stop): Declare.
    	* mi/mi-main.c (mi_cmd_trace_start, mi_cmd_trace_status)
    	(mi_cmd_trace_stop): New.
    	* tracepoint.c (start_tracing): New, extracted from...
    	(trace_start_command): ...this.
    	(trace_status_mi): New.
    	* tracepoint.h (struct trace_status): Document
    	stopping_tracepoint.
    	(start_tracing, stop_tracing, trace_status_mi): Declare.

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 7f18d94..c2183fb 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -105,6 +105,9 @@ struct mi_cmd mi_cmds[] =
   { "thread-info", { NULL, 0 }, mi_cmd_thread_info },
   { "thread-list-ids", { NULL, 0 }, mi_cmd_thread_list_ids},
   { "thread-select", { NULL, 0 }, mi_cmd_thread_select},
+  { "trace-start", { NULL, 0 }, mi_cmd_trace_start },
+  { "trace-status", { NULL, 0 }, mi_cmd_trace_status },
+  { "trace-stop", { NULL, 0 }, mi_cmd_trace_stop },
   { "var-assign", { NULL, 0 }, mi_cmd_var_assign},
   { "var-create", { NULL, 0 }, mi_cmd_var_create},
   { "var-delete", { NULL, 0 }, mi_cmd_var_delete},
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 20bdbfd..702dd9e 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -88,6 +88,9 @@ extern mi_cmd_argv_ftype mi_cmd_target_file_delete;
 extern mi_cmd_argv_ftype mi_cmd_thread_info;
 extern mi_cmd_argv_ftype mi_cmd_thread_list_ids;
 extern mi_cmd_argv_ftype mi_cmd_thread_select;
+extern mi_cmd_argv_ftype mi_cmd_trace_start;
+extern mi_cmd_argv_ftype mi_cmd_trace_status;
+extern mi_cmd_argv_ftype mi_cmd_trace_stop;
 extern mi_cmd_argv_ftype mi_cmd_var_assign;
 extern mi_cmd_argv_ftype mi_cmd_var_create;
 extern mi_cmd_argv_ftype mi_cmd_var_delete;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index f0b2f32..5aebc50 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -51,6 +51,7 @@
 #include "inferior.h"
 #include "osdata.h"
 #include "splay-tree.h"
+#include "tracepoint.h"
 
 #include <ctype.h>
 #include <sys/time.h>
@@ -2079,3 +2080,22 @@ print_diff (struct mi_timestamp *start, struct mi_timestamp *end)
        timeval_diff (start->utime, end->utime) / 1000000.0, 
        timeval_diff (start->stime, end->stime) / 1000000.0);
   }
+
+void
+mi_cmd_trace_start (char *command, char **argv, int argc)
+{
+  start_tracing ();
+}
+
+void
+mi_cmd_trace_status (char *command, char **argv, int argc)
+{
+  trace_status_mi (0);
+}
+
+void
+mi_cmd_trace_stop (char *command, char **argv, int argc)
+{
+  stop_tracing ();
+  trace_status_mi (1);
+}
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index c713d7e..b7fae78 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1384,15 +1384,9 @@ add_aexpr (struct collection_list *collect, struct agent_expr *aexpr)
   collect->next_aexpr_elt++;
 }
 
-/* tstart command:
-
-   Tell target to clear any previous trace experiment.
-   Walk the list of tracepoints, and send them (and their actions)
-   to the target.  If no errors, 
-   Tell target to start a new trace experiment.  */
 
-static void
-trace_start_command (char *args, int from_tty)
+void
+start_tracing (void)
 {
   char buf[2048];
   VEC(breakpoint_p) *tp_vec = NULL;
@@ -1401,8 +1395,6 @@ trace_start_command (char *args, int from_tty)
   struct trace_state_variable *tsv;
   int any_downloaded = 0;
 
-  dont_repeat ();	/* Like "run", dangerous to repeat accidentally.  */
-
   target_trace_init ();
   
   tp_vec = all_tracepoints ();
@@ -1438,6 +1430,21 @@ trace_start_command (char *args, int from_tty)
   current_trace_status()->running = 1;
 }
 
+/* tstart command:
+
+   Tell target to clear any previous trace experiment.
+   Walk the list of tracepoints, and send them (and their actions)
+   to the target.  If no errors,
+   Tell target to start a new trace experiment.  */
+
+static void
+trace_start_command (char *args, int from_tty)
+{
+  dont_repeat ();	/* Like "run", dangerous to repeat accidentally.  */
+
+  start_tracing ();
+}
+
 /* tstop command */
 static void
 trace_stop_command (char *args, int from_tty)
@@ -1446,7 +1453,7 @@ trace_stop_command (char *args, int from_tty)
 }
 
 void
-stop_tracing ()
+stop_tracing (void)
 {
   target_trace_stop ();
   /* should change in response to reply? */
@@ -1502,7 +1509,6 @@ trace_status_command (char *args, int from_tty)
 	  printf_filtered (_("Trace stopped because of disconnection.\n"));
 	  break;
 	case tracepoint_passcount:
-	  /* FIXME account for number on target */
 	  printf_filtered (_("Trace stopped by tracepoint %d.\n"),
 			   ts->stopping_tracepoint);
 	  break;
@@ -1536,6 +1542,92 @@ trace_status_command (char *args, int from_tty)
     printf_filtered (_("Not looking at any trace frame.\n"));
 }
 
+/* Report the trace status to uiout, in a way suitable for MI, and not
+   suitable for CLI.  If ON_STOP is true, suppress a few fields that
+   are not meaningful in the -trace-stop response.
+
+   The implementation is essentially parallel to trace_status_command, but
+   merging them will result in unreadable code.  */
+void
+trace_status_mi (int on_stop)
+{
+  struct trace_status *ts = current_trace_status ();
+  int status;
+  char *string_status;
+
+  status = target_get_trace_status (ts);
+
+  if (status == -1 && !ts->from_file)
+    {
+      ui_out_field_string (uiout, "supported", "0");
+      return;
+    }
+
+  if (ts->from_file)
+    ui_out_field_string (uiout, "supported", "file");
+  else if (!on_stop)
+    ui_out_field_string (uiout, "supported", "1");
+
+  gdb_assert (ts->running_known);
+
+  if (ts->running)
+    {
+      ui_out_field_string (uiout, "running", "1");
+
+      /* Unlike CLI, do not show the state of 'disconnected-tracing' variable.
+	 Given that the frontend gets the status either on -trace-stop, or from
+	 -trace-status after re-connection, it does not seem like this
+	 information is necessary for anything.  It is not necessary for either
+	 figuring the vital state of the target nor for navigation of trace
+	 frames.  If the frontend wants to show the current state is some
+	 configure dialog, it can request the value when such dialog is
+	 invoked by the user.  */
+    }
+  else
+    {
+      char *stop_reason = NULL;
+      int stopping_tracepoint = -1;
+
+      if (!on_stop)
+	ui_out_field_string (uiout, "running", "0");
+
+      gdb_assert (ts->stop_reason != trace_stop_reason_unknown);
+
+      switch (ts->stop_reason)
+	{
+	case tstop_command:
+	  stop_reason = "request";
+	  break;
+	case trace_buffer_full:
+	  stop_reason = "overflow";
+	  break;
+	case trace_disconnected:
+	  stop_reason = "disconnection";
+	  break;
+	case tracepoint_passcount:
+	  stop_reason = "passcount";
+	  stopping_tracepoint = ts->stopping_tracepoint;
+	  break;
+	}
+
+      if (stop_reason)
+	{
+	  ui_out_field_string (uiout, "stop-reason", stop_reason);
+	  if (stopping_tracepoint != -1)
+	    ui_out_field_int (uiout, "stopping-tracepoint",
+			      stopping_tracepoint);
+	}
+    }
+
+  ui_out_field_int (uiout, "frames", ts->traceframe_count);
+
+  if ((int) ts->buffer_size != -1)
+    ui_out_field_int (uiout, "buffer-size",  (int) ts->buffer_size);
+  if ((int) ts->buffer_free != -1)
+    ui_out_field_int (uiout, "buffer-free",  (int) ts->buffer_free);
+}
+
+
 void
 disconnect_or_stop_tracing (int from_tty)
 {
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index 446b292..2e9193c 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -86,6 +86,8 @@ struct trace_status
 
   enum trace_stop_reason stop_reason;
 
+  /* If stop_reason == tracepoint_passcount, the on-target number
+     of the tracepoint which caused the stop.  */
   int stopping_tracepoint;
 
   int traceframe_count;
@@ -155,4 +157,9 @@ extern struct breakpoint *create_tracepoint_from_upload (struct uploaded_tp *utp
 extern void merge_uploaded_tracepoints (struct uploaded_tp **utpp);
 extern void merge_uploaded_trace_state_variables (struct uploaded_tsv **utsvp);
 
+extern void start_tracing (void);
+extern void stop_tracing (void);
+
+extern void trace_status_mi (int on_stop);
+
 #endif	/* TRACEPOINT_H */

Re: [MI tracepoints 4/9] -trace-start/end/status

[Pedro Alves]

On Tuesday 16 March 2010 12:12:33, Vladimir Prus wrote:
> On Monday 15 March 2010 21:46:05 Pedro Alves wrote:
> 
> > Double space after period everywhere please.
> > 
> > +  ui_out_field_int (uiout, "buffer-size",  (int) ts->buffer_size);
> > +  ui_out_field_int (uiout, "buffer-free",  (int) ts->buffer_free);
> > 
> > When these optional fields aren't reported by the target, this
> > will print -1.  I don't see that mentioned in the manual.  Is
> > this intended?  I would have expected optional fields to just
> > not be present, as is common in MI.  trace_status_command does
> > that too.
> 
> I did not realize they are optional. 

All status fields are optional, except for "running", "not running".
The original packet was just:

 @item qTStatus
 Ask the stub if there is a trace experiment running right now.

 Replies:
 @table @samp
 @item T0
 There is no trace experiment running.
 @item T1
 There is a trace experiment running.
 @end table

We then added the support for passing a `:' list of
extra fields to that packet.  See tracepoint.c:parse_trace_status.

> Attached is the revised patch,

> +      gdb_assert (ts->stop_reason != trace_stop_reason_unknown);

Therefore, this assertion is too strong.  It should be a plain 

   if (ts->stop_reason != trace_stop_reason_unknown)
      {

> +
> +      switch (ts->stop_reason)
> +       {
> +       case tstop_command:
> +         stop_reason = "request";
> +         break;
> +       case trace_buffer_full:
> +         stop_reason = "overflow";
> +         break;
> +       case trace_disconnected:
> +         stop_reason = "disconnection";
> +         break;
> +       case tracepoint_passcount:
> +         stop_reason = "passcount";
> +         stopping_tracepoint = ts->stopping_tracepoint;
> +         break;
> +       }
> +
> +      if (stop_reason)
> +       {
> +         ui_out_field_string (uiout, "stop-reason", stop_reason);
> +         if (stopping_tracepoint != -1)
> +           ui_out_field_int (uiout, "stopping-tracepoint",
> +                             stopping_tracepoint);
> +       }
> +    }
> +

And this one should be conditionalized as well:

> +  ui_out_field_int (uiout, "frames", ts->traceframe_count);

And I think the manual patch should be updated to mention
any of these fields might be missing.

-- 
Pedro Alves