Re: [PATCH v3 06/15] Write status to CTF and read.

[Yao Qi <yao at codesourcery dot com>]

On 03/15/2013 02:06 AM, Doug Evans wrote:
>   > +#define SET_INT32_FIELD(EVENT, SCOPE, VAR, FIELD)			\
>   > +  VAR->FIELD = (int) bt_ctf_get_int64 (bt_ctf_get_field (EVENT,	\
>   > +							SCOPE,		\
>   > +							#FIELD))
> 
> Macros like this should be in a do { } while (0).
>    do { \
>      ...; \
>    } while (0)
> 

Do we really need do/while loop for this single-line macro?

> Plus wrap params in parens (except #FIELD of course).
> 

Of course.

>   >  static void
>   >  ctf_open (char *dirname, int from_tty)
>   >  {
>   > +  struct bt_ctf_event *event;
>   > +  uint32_t event_id;
>   > +  const struct bt_definition *scope;
>   > +
>   >    target_preopen (from_tty);
>   >    if (!dirname)
>   >      error (_("No CTF directory specified."));
>   >
>   >    ctf_open_dir (dirname);
>   >
>   > +  /* Skip the first packet which about the trace status.  The first
>   > +     event is "frame".  */
>   > +  event = bt_ctf_iter_read_event (ctf_iter);
>   > +  scope = bt_ctf_get_top_level_scope (event, BT_STREAM_EVENT_HEADER);
>   > +  event_id = bt_ctf_get_uint64 (bt_ctf_get_field (event, scope, "id"));
>   > +  gdb_assert (event_id == CTF_EVENT_ID_FRAME);
> 
> Question: Will these asserts trigger on bad input data?
> [If so, you need to use something else besides gdb_assert.]

Yes, bad data will trigger assert.  I change them to error.

-- 
Yao (éå)

gdb:

2013-03-29  Yao Qi  <yao@codesourcery.com>

	* ctf.c (CTF_EVENT_ID_STATUS, ctf_save_write_int32): New
	macros.
	(ctf_save_metadata_header): Define new type alias in
	metadata.
	(ctf_write_header): Start a new faked packet for trace status.
	(ctf_write_status): Write trace status to CTF.
	(ctf_write_definition_end): End the faked packet.
	(start_pos): New variable.
	(SET_INT32_FIELD): New macro.
	(ctf_read_status): New.
	(ctf_open): Skip the first faked packet and assert on some
	event types.
	(ctf_trace_find): Set the iterator to the beginning of packet
	including trace frames, instead of the first packet.
	(ctf_get_trace_status): New.
	(init_ctf_ops): Install ctf_get_trace_status to field
	'to_get_trace_status'.
---
 gdb/ctf.c |  130 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 123 insertions(+), 7 deletions(-)

diff --git a/gdb/ctf.c b/gdb/ctf.c
index 14482f4..35b779d 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -35,7 +35,8 @@
    1. The length (in bytes) of register cache.  Event "register" will
    be defined in metadata, which includes the length.
 
-   2. Trace status.  Not implemented yet in CTF writer.
+   2. Trace status.  Event "status" is defined in metadata, which
+   includes all aspects of trace status.
 
    3. Uploaded trace variables and tracepoints.  Not implemented yet
    in CTF writer.
@@ -66,6 +67,7 @@
 #define CTF_EVENT_ID_TSV 1
 #define CTF_EVENT_ID_MEMORY 2
 #define CTF_EVENT_ID_FRAME 3
+#define CTF_EVENT_ID_STATUS 4
 
 /* The state kept while writing the CTF datastream file.  */
 
@@ -120,6 +122,12 @@ ctf_save_write (struct trace_write_handler *handler,
 #define ctf_save_write_uint32(HANDLER, U32) \
   ctf_save_write (HANDLER, (gdb_byte *) &U32, 4)
 
+/* Write a signed 32-bit integer to datastream file represented by
+   HANDLER.  */
+
+#define ctf_save_write_int32(HANDLER, INT32) \
+  ctf_save_write ((HANDLER), (gdb_byte *) &(INT32), 4)
+
 /* Set datastream file position.  Update HANDLER->content_size
    if WHENCE is SEEK_CUR.  */
 
@@ -218,6 +226,9 @@ ctf_save_metadata_header (struct trace_write_handler *handler)
 			   "typealias integer { size = 64; align = 64;"
 			   "signed = false; base = hex;}"
 			   " := uint64_t;\n");
+  ctf_save_write_metadata (handler,
+			   "typealias integer { size = 32; align = 32;"
+			   "signed = true; } := int32_t;\n");
   ctf_save_write_metadata (handler, "\n");
 
   /* Get the byte order of the host and write CTF data in this byte
@@ -368,6 +379,9 @@ ctf_write_header (struct trace_file_writer *self)
 
   gdb_assert (writer->tcs.content_size == 0);
   gdb_assert (writer->tcs.packet_start == 0);
+
+  /* Create a new packet to contain this event.  */
+  self->ops->frame_ops->start (self, 0);
 }
 
 /* This is the implementation of trace_file_write_ops method
@@ -398,8 +412,39 @@ static void
 ctf_write_status (struct trace_file_writer *self,
 		  struct trace_status *ts)
 {
-  /* It is not supported yet to write trace status into CTF trace
-     data.  */
+  struct ctf_trace_file_writer *writer
+    = (struct ctf_trace_file_writer *) self;
+  uint32_t id;
+  int32_t int32;
+
+  ctf_save_write_metadata (&writer->tcs, "\n");
+  ctf_save_write_metadata (&writer->tcs,
+			   "event {\n\tname = \"status\";\n\tid = %u;\n"
+			   "\tfields := struct { \n"
+			   "\t\tint32_t stop_reason;\n"
+			   "\t\tint32_t stopping_tracepoint;\n"
+			   "\t\tint32_t traceframe_count;\n"
+			   "\t\tint32_t traceframes_created;\n"
+			   "\t\tint32_t buffer_free;\n"
+			   "\t\tint32_t buffer_size;\n"
+			   "\t\tint32_t disconnected_tracing;\n"
+			   "\t\tint32_t circular_buffer;\n"
+			   "\t};\n"
+			   "};\n",
+			   CTF_EVENT_ID_STATUS);
+
+  id = CTF_EVENT_ID_STATUS;
+  /* Event Id.  */
+  ctf_save_align_write (&writer->tcs, (gdb_byte *) &id, 4, 4);
+
+  ctf_save_write_int32 (&writer->tcs, ts->stop_reason);
+  ctf_save_write_int32 (&writer->tcs, ts->stopping_tracepoint);
+  ctf_save_write_int32 (&writer->tcs, ts->traceframe_count);
+  ctf_save_write_int32 (&writer->tcs, ts->traceframes_created);
+  ctf_save_write_int32 (&writer->tcs, ts->buffer_free);
+  ctf_save_write_int32 (&writer->tcs, ts->buffer_size);
+  ctf_save_write_int32 (&writer->tcs, ts->disconnected_tracing);
+  ctf_save_write_int32 (&writer->tcs, ts->circular_buffer);
 }
 
 /* This is the implementation of trace_file_write_ops method
@@ -430,7 +475,10 @@ ctf_write_uploaded_tp (struct trace_file_writer *self,
 static void
 ctf_write_definition_end (struct trace_file_writer *self)
 {
-  /* Nothing to do for CTF.  */
+  struct ctf_trace_file_writer *writer
+    = (struct ctf_trace_file_writer *) self;
+
+  self->ops->frame_ops->end (self);
 }
 
 /* The minimal file size of data stream.  It is required by
@@ -685,6 +733,9 @@ ctf_trace_file_writer_new (void)
 /* The struct pointer for current CTF directory.  */
 static struct bt_context *ctx = NULL;
 static struct bt_ctf_iter *ctf_iter = NULL;
+/* The position of the first packet containing trace frame.  */
+static struct bt_iter_pos *start_pos;
+
 /* The name of CTF directory.  */
 static char *trace_dirname;
 
@@ -773,6 +824,29 @@ ctf_open_dir (char *dirname)
 
 }
 
+#define SET_INT32_FIELD(EVENT, SCOPE, VAR, FIELD)			\
+  (VAR)->FIELD = (int) bt_ctf_get_int64 (bt_ctf_get_field ((EVENT),	\
+							   (SCOPE),	\
+							   #FIELD))
+
+/* EVENT is the "status" event and TS is filled in.  */
+
+static void
+ctf_read_status (struct bt_ctf_event *event, struct trace_status *ts)
+{
+  const struct bt_definition *scope
+    = bt_ctf_get_top_level_scope (event, BT_EVENT_FIELDS);
+
+  SET_INT32_FIELD (event, scope, ts, stop_reason);
+  SET_INT32_FIELD (event, scope, ts, stopping_tracepoint);
+  SET_INT32_FIELD (event, scope, ts, traceframe_count);
+  SET_INT32_FIELD (event, scope, ts, traceframes_created);
+  SET_INT32_FIELD (event, scope, ts, buffer_free);
+  SET_INT32_FIELD (event, scope, ts, buffer_size);
+  SET_INT32_FIELD (event, scope, ts, disconnected_tracing);
+  SET_INT32_FIELD (event, scope, ts, circular_buffer);
+}
+
 /* This is the implementation of target_ops method to_open.  Open CTF
    trace data, read trace status, trace state variables and tracepoint
    definitions from the first packet.  Set the start position at the
@@ -781,12 +855,42 @@ ctf_open_dir (char *dirname)
 static void
 ctf_open (char *dirname, int from_tty)
 {
+  struct bt_ctf_event *event;
+  uint32_t event_id;
+  const struct bt_definition *scope;
+
   if (!dirname)
     error (_("No CTF directory specified."));
 
   ctf_open_dir (dirname);
 
   target_preopen (from_tty);
+
+  /* Skip the first packet which about the trace status.  The first
+     event is "frame".  */
+  event = bt_ctf_iter_read_event (ctf_iter);
+  scope = bt_ctf_get_top_level_scope (event, BT_STREAM_EVENT_HEADER);
+  event_id = bt_ctf_get_uint64 (bt_ctf_get_field (event, scope, "id"));
+  if (event_id != CTF_EVENT_ID_FRAME)
+    error (_("Wrong event id of the first event"));
+  /* The second event is "status".  */
+  event = bt_ctf_iter_read_event (ctf_iter);
+  scope = bt_ctf_get_top_level_scope (event, BT_STREAM_EVENT_HEADER);
+  event_id = bt_ctf_get_uint64 (bt_ctf_get_field (event, scope, "id"));
+  if (event_id != CTF_EVENT_ID_STATUS)
+    error (_("Wrong event id of the second event"));
+  ctf_read_status (event, current_trace_status ());
+
+  /* The third event is "frame".  A new packet.  */
+  event = bt_ctf_iter_read_event (ctf_iter);
+  scope = bt_ctf_get_top_level_scope (event, BT_STREAM_EVENT_HEADER);
+  event_id = bt_ctf_get_uint64 (bt_ctf_get_field (event, scope, "id"));
+  if (event_id != CTF_EVENT_ID_FRAME)
+    error (_("Wrong event id of the first event of the second packet"));
+
+  start_pos = bt_iter_get_pos (bt_ctf_get_iter (ctf_iter));
+  gdb_assert (start_pos->type == BT_SEEK_RESTORE);
+
   trace_dirname = xstrdup (dirname);
   push_target (&ctf_ops);
 }
@@ -1218,9 +1322,8 @@ ctf_trace_find (enum trace_find_type type, int num,
     }
 
   gdb_assert (ctf_iter != NULL);
-  /* Set iterator back to the beginning.  */
-  pos.type = BT_SEEK_BEGIN;
-  bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), &pos);
+  /* Set iterator back to the start.  */
+  bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), start_pos);
 
   while (1)
     {
@@ -1380,6 +1483,18 @@ ctf_traceframe_info (void)
   return info;
 }
 
+/* This is the implementation of target_ops method to_get_trace_status.
+   The trace status for a file is that tracing can never be run.  */
+
+static int
+ctf_get_trace_status (struct trace_status *ts)
+{
+  /* Other bits of trace status were collected as part of opening the
+     trace files, so nothing to do here.  */
+
+  return -1;
+}
+
 static void
 init_ctf_ops (void)
 {
@@ -1394,6 +1509,7 @@ Specify the filename of the CTF directory.";
   ctf_ops.to_fetch_registers = ctf_fetch_registers;
   ctf_ops.to_xfer_partial = ctf_xfer_partial;
   ctf_ops.to_files_info = ctf_files_info;
+  ctf_ops.to_get_trace_status = ctf_get_trace_status;
   ctf_ops.to_trace_find = ctf_trace_find;
   ctf_ops.to_get_trace_state_variable_value
     = ctf_get_trace_state_variable_value;
-- 
1.7.7.6