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

Yao Qi
2013-03-29T07:24:42Z

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