Fix: ctf: notif-iter: do not call request_medium_bytes() when not needed

Do not systematically call request_medium_bytes(), that is, at the
beginning of read_dscope_begin_state(), because the scope field which is
about to be decoded might be empty, an already aligned structure
field or the equivalent (through a variant field for example, of which
the tag is outside this scope).

To do this we also need to make bt_btr_start() accept a size of 0. In
this case, if the field to decode is an empty structure which is already
aligned, then bt_btr_start() does not need to consume any bit and
returns BT_BTR_STATUS_OK with 0 consumed bits. In this case, the next
notif-iter state is not a continuing state, thus request_medium_bytes()
is not called yet.

Checking for a bad state when request_medium_bytes() receives
BT_NOTIF_ITER_MEDIUM_STATUS_EOF from the medium is easier. Good states
are:

* If the current packet size is set:
** Current packet offset is current packet size.
* Otherwise:
** Current packet offset is 0.
** Last event header offset is set and current offset is equal to it.

All other states are invalid.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/plugins/ctf/common/btr/btr.c b/plugins/ctf/common/btr/btr.c
index e393f301d776c4b928fa5a46f79daed9a2a123e5..b6ccc5d49099977357084d019c020191926d7d66 100644 (file)
--- a/plugins/ctf/common/btr/btr.c
+++ b/plugins/ctf/common/btr/btr.c
@@ -482,6 +482,7 @@ void stitch_append_from_buf(struct bt_btr *btr, size_t sz)
        buf_byte_at = BITS_TO_BYTES_FLOOR(buf_at_from_addr(btr));
        nb_bytes = BITS_TO_BYTES_CEIL(sz);
        assert(nb_bytes > 0);
+       assert(btr->buf.addr);
        memcpy(&btr->stitch.buf[stitch_byte_at], &btr->buf.addr[buf_byte_at],
                nb_bytes);
        btr->stitch.at += sz;
@@ -903,6 +904,7 @@ enum bt_btr_status read_basic_type_and_call_begin(struct bt_btr *btr,
 
        if (field_size <= available) {
                /* We have all the bits; decode and set now */
+               assert(btr->buf.addr);
                status = read_basic_and_call_cb(btr, btr->buf.addr,
                        buf_at_from_addr(btr));
                if (status != BT_BTR_STATUS_OK) {
@@ -1004,6 +1006,7 @@ enum bt_btr_status read_basic_string_type_and_call(
        assert(buf_at_from_addr(btr) % 8 == 0);
        available_bytes = BITS_TO_BYTES_FLOOR(available_bits(btr));
        buf_at_bytes = BITS_TO_BYTES_FLOOR(buf_at_from_addr(btr));
+       assert(btr->buf.addr);
        first_chr = &btr->buf.addr[buf_at_bytes];
        result = memchr(first_chr, '\0', available_bytes);
 
@@ -1463,9 +1466,7 @@ size_t bt_btr_start(struct bt_btr *btr,
        enum bt_btr_status *status)
 {
        assert(btr);
-       assert(buf);
-       assert(sz > 0);
-       assert(BYTES_TO_BITS(sz) > offset);
+       assert(BYTES_TO_BITS(sz) >= offset);
        reset(btr);
        btr->buf.addr = buf;
        btr->buf.offset = offset;

diff --git a/plugins/ctf/common/notif-iter/notif-iter.c b/plugins/ctf/common/notif-iter/notif-iter.c
index d7164952b5990a84a93bdeebad5e143af4aa0341..527147e572a12b30f92e3f4bc7077b12f90b5096 100644 (file)
--- a/plugins/ctf/common/notif-iter/notif-iter.c
+++ b/plugins/ctf/common/notif-iter/notif-iter.c
@@ -489,118 +489,42 @@ enum bt_notif_iter_status request_medium_bytes(
                BT_LOGV_MEM(buffer_addr, buffer_sz, "Returned bytes at %p:",
                        buffer_addr);
        } else if (m_status == BT_NOTIF_ITER_MEDIUM_STATUS_EOF) {
-               struct bt_field_type *ph_ft =
-                       bt_trace_get_packet_header_type(notit->meta.trace);
-               struct bt_field_type *eh_ft = NULL;
-               struct bt_field_type *sec_ft = NULL;
-               struct bt_field_type *ec_ft = NULL;
-
                /*
                 * User returned end of stream: validate that we're not
                 * in the middle of a packet header, packet context, or
                 * event.
                 */
-               if (notit->state == STATE_DSCOPE_TRACE_PACKET_HEADER_BEGIN) {
-                       /* Beginning of packet: always valid */
-                       goto good_state;
-               }
-
-               if (!notit->meta.stream_class) {
-                       goto bad_state;
-               }
-
-               if (notit->state == STATE_DSCOPE_STREAM_PACKET_CONTEXT_BEGIN) {
-                       /*
-                        * Beginning of packet context context is only
-                        * valid if there's no packet header.
-                        */
-                       if (!ph_ft) {
-                               goto good_state;
-                       }
-               }
-
-               eh_ft = bt_stream_class_get_event_header_type(
-                       notit->meta.stream_class);
-               sec_ft = bt_stream_class_get_event_context_type(
-                       notit->meta.stream_class);
-
-               if (notit->state == STATE_DSCOPE_STREAM_EVENT_HEADER_BEGIN) {
-                       /*
-                        * Beginning of event's header is only valid if
-                        * the packet is not supposed to have a specific
-                        * size (whole packet sequence has in fact only
-                        * one packet).
-                        */
-                       if (notit->cur_packet_size == -1) {
-                               goto good_state;
+               if (notit->cur_packet_size >= 0) {
+                       if (packet_at(notit) == notit->cur_packet_size) {
+                               goto end;
                        }
-               }
-
-               if (notit->state == STATE_DSCOPE_STREAM_EVENT_CONTEXT_BEGIN) {
-                       /*
-                        * Beginning of event's stream event context is
-                        * only valid if the packet is not supposed to
-                        * have a specific size (whole packet sequence
-                        * has in fact only one packet), and there's no
-                        * event header.
-                        */
-                       if (notit->cur_packet_size == -1 && !eh_ft) {
-                               goto good_state;
-                       }
-               }
-
-               if (!notit->meta.event_class) {
-                       goto bad_state;
-               }
-
-               ec_ft = bt_event_class_get_context_type(
-                       notit->meta.event_class);
-
-               if (notit->state == STATE_DSCOPE_EVENT_CONTEXT_BEGIN) {
-                       /*
-                        * Beginning of event's context is only valid if
-                        * the packet is not supposed to have a specific
-                        * size (whole packet sequence has in fact only
-                        * one packet), and there's no event header and
-                        * no stream event context.
-                        */
-                       if (notit->cur_packet_size == -1 && !eh_ft && !sec_ft) {
-                               goto good_state;
+               } else {
+                       if (packet_at(notit) == 0) {
+                               goto end;
                        }
-               }
 
-               if (notit->state == STATE_DSCOPE_EVENT_PAYLOAD_BEGIN) {
-                       /*
-                        * Beginning of event's context is only valid if
-                        * the packet is not supposed to have a specific
-                        * size (whole packet sequence has in fact only
-                        * one packet), and there's no event header, no
-                        * stream event context, and no event context.
-                        */
-                       if (notit->cur_packet_size == -1 && !eh_ft && !sec_ft &&
-                                       !ec_ft) {
-                               goto good_state;
+                       if (notit->buf.last_eh_at != SIZE_MAX &&
+                                       notit->buf.at == notit->buf.last_eh_at) {
+                               goto end;
                        }
                }
 
-bad_state:
                /* All other states are invalid */
                BT_LOGW("User function returned %s, but notification iterator is in an unexpected state: "
-                       "state=%s",
+                       "state=%s, cur-packet-size=%" PRId64 ", cur=%zu, "
+                       "packet-cur=%zu, last-eh-at=%zu",
                        bt_notif_iter_medium_status_string(m_status),
-                       state_string(notit->state));
+                       state_string(notit->state),
+                       notit->cur_packet_size,
+                       notit->buf.at, packet_at(notit),
+                       notit->buf.last_eh_at);
                m_status = BT_NOTIF_ITER_MEDIUM_STATUS_ERROR;
-
-good_state:
-               bt_put(ph_ft);
-               bt_put(eh_ft);
-               bt_put(sec_ft);
-               bt_put(ec_ft);
        } else if (m_status < 0) {
                BT_LOGW("User function failed: status=%s",
                        bt_notif_iter_medium_status_string(m_status));
        }
 
+end:
        return notif_iter_status_from_m_status(m_status);
 }
 
@@ -632,21 +556,6 @@ enum bt_notif_iter_status read_dscope_begin_state(
        enum bt_btr_status btr_status;
        size_t consumed_bits;
 
-       status = buf_ensure_available_bits(notit);
-       if (status != BT_NOTIF_ITER_STATUS_OK) {
-               if (status < 0) {
-                       BT_LOGW("Cannot ensure that buffer has at least one byte: "
-                               "notif-addr=%p, status=%s",
-                               notit, bt_notif_iter_status_string(status));
-               } else {
-                       BT_LOGV("Cannot ensure that buffer has at least one byte: "
-                               "notif-addr=%p, status=%s",
-                               notit, bt_notif_iter_status_string(status));
-               }
-
-               goto end;
-       }
-
        bt_put(*dscope_field);
        notit->cur_dscope_field = dscope_field;
        BT_LOGV("Starting BTR: notit-addr=%p, btr-addr=%p, ft-addr=%p",
@@ -689,6 +598,9 @@ enum bt_notif_iter_status read_dscope_continue_state(
        enum bt_btr_status btr_status;
        size_t consumed_bits;
 
+       BT_LOGV("Continuing BTR: notit-addr=%p, btr-addr=%p",
+               notit, notit->btr);
+
        status = buf_ensure_available_bits(notit);
        if (status != BT_NOTIF_ITER_STATUS_OK) {
                if (status < 0) {
@@ -704,8 +616,7 @@ enum bt_notif_iter_status read_dscope_continue_state(
                goto end;
        }
 
-       BT_LOGV("Continuing BTR: notit-addr=%p, btr-addr=%p",
-               notit, notit->btr);
+
        consumed_bits = bt_btr_continue(notit->btr, notit->buf.addr,
                notit->buf.sz, &btr_status);
        BT_LOGV("BTR consumed bits: size=%zu", consumed_bits);