Fix: `ctf` plugin: use element FC's alignment as array/seq. FC alignment
authorPhilippe Proulx <eeppeliteloop@gmail.com>
Tue, 21 Jul 2020 17:57:10 +0000 (13:57 -0400)
committerPhilippe Proulx <eeppeliteloop@gmail.com>
Thu, 10 Sep 2020 15:26:21 +0000 (11:26 -0400)
Observed issue
==============
When reading some CTF trace with an empty array/sequence field,
Babeltrace 2 can fail.

More specifically, for the trace `tests/data/ctf-traces/succeed/array-align-elem`
(added by this patch):

The CLI's output is:

    07-21 14:00:24.636 73959 73959 E PLUGIN/CTF/MSG-ITER request_medium_bytes@msg-iter.c:535 [auto-disc-source-ctf-fs] User function returned EOF, but message iterator is in an unexpected state: state=DSCOPE_EVENT_PAYLOAD_CONTINUE, cur-packet-size=-1, cur=24, packet-cur=24, last-eh-at=16
    07-21 14:00:24.636 73959 73959 E PLUGIN/CTF/MSG-ITER read_dscope_continue_state@msg-iter.c:632 [auto-disc-source-ctf-fs] Cannot ensure that buffer has at least one byte: msg-addr=0x55d3b4e051f0, status=ERROR
    07-21 14:00:24.636 73959 73959 E PLUGIN/CTF/MSG-ITER ctf_msg_iter_get_next_message@msg-iter.c:2881 [auto-disc-source-ctf-fs] Cannot handle state: msg-it-addr=0x55d3b4e051f0, state=DSCOPE_EVENT_PAYLOAD_CONTINUE
    07-21 14:00:24.636 73959 73959 E PLUGIN/SRC.CTF.FS ctf_fs_iterator_next_one@fs.c:90 [auto-disc-source-ctf-fs] Failed to get next message from CTF message iterator.
    07-21 14:00:24.636 73959 73959 W LIB/MSG-ITER bt_message_iterator_next@iterator.c:865 Component input port message iterator's "next" method failed: iter-addr=0x55d3b4e05000, iter-upstream-comp-name="auto-disc-source-ctf-fs", iter-upstream-comp-log-level=WARNING, iter-upstream-comp-class-type=SOURCE, iter-upstream-comp-class-name="fs", iter-upstream-comp-class-partial-descr="Read CTF traces from the file sy", iter-upstream-port-type=OUTPUT, iter-upstream-port-name="array-align-elem | 0 | array-align-elem/stream", status=ERROR
    07-21 14:00:24.636 73959 73959 E PLUGIN/FLT.UTILS.MUXER muxer_upstream_msg_iter_next@muxer.c:446 [muxer] Upstream iterator's next method returned an error: status=ERROR
    07-21 14:00:24.636 73959 73959 E PLUGIN/FLT.UTILS.MUXER validate_muxer_upstream_msg_iters@muxer.c:989 [muxer] Cannot validate muxer's upstream message iterator wrapper: muxer-msg-iter-addr=0x55d3b4dcb830, muxer-upstream-msg-iter-wrap-addr=0x55d3b4e055c0
    ev: { a = 1, b = [ ], c = 2 }
    07-21 14:00:24.636 73959 73959 E PLUGIN/FLT.UTILS.MUXER muxer_msg_iter_next@muxer.c:1417 [muxer] Cannot get next message: comp-addr=0x55d3b4dcae70, muxer-comp-addr=0x55d3b4dcaef0, muxer-msg-iter-addr=0x55d3b4dcb830, msg-iter-addr=0x55d3b4dcb750, status=ERROR
    07-21 14:00:24.636 73959 73959 W LIB/MSG-ITER bt_message_iterator_next@iterator.c:865 Component input port message iterator's "next" method failed: iter-addr=0x55d3b4dcb750, iter-upstream-comp-name="muxer", iter-upstream-comp-log-level=WARNING, iter-upstream-comp-class-type=FILTER, iter-upstream-comp-class-name="muxer", iter-upstream-comp-class-partial-descr="Sort messages from multiple inpu", iter-upstream-port-type=OUTPUT, iter-upstream-port-name="out", status=ERROR
    07-21 14:00:24.636 73959 73959 W LIB/GRAPH consume_graph_sink@graph.c:462 Component's "consume" method failed: status=ERROR, comp-addr=0x55d3b4dcb070, comp-name="pretty", comp-log-level=WARNING, comp-class-type=SINK, comp-class-name="pretty", comp-class-partial-descr="Pretty-print messages (`text` fo", comp-class-is-frozen=1, comp-class-so-handle-addr=0x55d3b4dc7c50, comp-class-so-handle-path="/home/eepp/dev/babeltrace/install/lib/babeltrace2/plugins/babeltrace-plugin-text.la", comp-input-port-count=1, comp-output-port-count=0
    07-21 14:00:24.636 73959 73959 E CLI cmd_run@babeltrace2.c:2529 Graph failed to complete successfully

    ERROR:    [Babeltrace CLI] (babeltrace2.c:2529)
      Graph failed to complete successfully
    CAUSED BY [libbabeltrace2] (graph.c:462)
      Component's "consume" method failed: status=ERROR, comp-addr=0x55d3b4dcb070, comp-name="pretty", comp-log-level=WARNING, comp-class-type=SINK,
      comp-class-name="pretty", comp-class-partial-descr="Pretty-print messages (`text` fo", comp-class-is-frozen=1, comp-class-so-handle-addr=0x55d3b4dc7c50,
      comp-class-so-handle-path="/home/eepp/dev/babeltrace/install/lib/babeltrace2/plugins/babeltrace-plugin-text.la", comp-input-port-count=1,
      comp-output-port-count=0
    CAUSED BY [libbabeltrace2] (iterator.c:865)
      Component input port message iterator's "next" method failed: iter-addr=0x55d3b4dcb750, iter-upstream-comp-name="muxer",
      iter-upstream-comp-log-level=WARNING, iter-upstream-comp-class-type=FILTER, iter-upstream-comp-class-name="muxer",
      iter-upstream-comp-class-partial-descr="Sort messages from multiple inpu", iter-upstream-port-type=OUTPUT, iter-upstream-port-name="out", status=ERROR
    CAUSED BY [muxer: 'filter.utils.muxer'] (muxer.c:1417)
      Cannot get next message: comp-addr=0x55d3b4dcae70, muxer-comp-addr=0x55d3b4dcaef0, muxer-msg-iter-addr=0x55d3b4dcb830, msg-iter-addr=0x55d3b4dcb750,
      status=ERROR
    CAUSED BY [muxer: 'filter.utils.muxer'] (muxer.c:989)
      Cannot validate muxer's upstream message iterator wrapper: muxer-msg-iter-addr=0x55d3b4dcb830, muxer-upstream-msg-iter-wrap-addr=0x55d3b4e055c0
    CAUSED BY [muxer: 'filter.utils.muxer'] (muxer.c:446)
      Upstream iterator's next method returned an error: status=ERROR
    CAUSED BY [libbabeltrace2] (iterator.c:865)
      Component input port message iterator's "next" method failed: iter-addr=0x55d3b4e05000, iter-upstream-c

Cause
=====
The internal CTF IR array and sequence field classes do not have an
alignment which is equal to their element FC's alignment, although
CTF 1.8.3 requires it [1]:

> Arrays are always aligned on their element alignment requirement.

Solution
========
In `bfcr.c`, align_class_state() is already called for array/sequence
fields.

Add a CTF IR trace class visitor to set any array/sequence FC's
alignment to its element FC's alignment, recursively (postorder).

Also update, postorder, structure FC alignments since some of the
alignments of the field types of their members can change.

Note
====
This patch adds two minimalist CTF traces to test the fix as well as two
new corresponding tests in
`tests/plugins/src.ctf.fs/succeed/test_succeed`:

`array-align-elem`:
    Metadata:

        struct {
            integer { size = 8; } a;
            integer { size = 8; align = 16; } b[0];
            integer { size = 8; } c;
        };

    Taking the aligment of `b` into account, there's one byte of padding
    between `a` and `c`:

        a # c
          ^---- alignment of `b`

`struct-array-align-elem`:
    Metadata:

        struct {
            integer { size = 8; } x;
            struct {
                integer { size = 8; } a;
                integer { size = 8; align = 32; } b[0];
            } y;
            integer { size = 8; } z;
        };

    Taking the alignment of `y.b` into account, the `y` structure itself
    is 32-bit-aligned, making the outer structure also 32-bit-aligned.
    Therefore, there are three bytes of padding between `x` and `a`, and
    three other bytes of padding between `a` and `z`:

        x # # # a # # # z
          ^       ^-------- alignment of `b`
          '---------------- alignment of `y`

References
==========
[1]: https://diamon.org/ctf/v1.8.3/#spec4.2.3

Fixes: https://bugs.lttng.org/issues/1276
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I06b4fbeda9ffd5b87b4964dd567027d0bfd5e9c7
Reviewed-on: https://review.lttng.org/c/babeltrace/+/4056

src/plugins/ctf/common/metadata/Makefile.am
src/plugins/ctf/common/metadata/ctf-meta-update-alignments.c [new file with mode: 0644]
src/plugins/ctf/common/metadata/ctf-meta-visitors.h
src/plugins/ctf/common/metadata/visitor-generate-ir.c
tests/data/ctf-traces/succeed/array-align-elem/metadata [new file with mode: 0644]
tests/data/ctf-traces/succeed/array-align-elem/stream [new file with mode: 0644]
tests/data/ctf-traces/succeed/struct-array-align-elem/metadata [new file with mode: 0644]
tests/data/ctf-traces/succeed/struct-array-align-elem/stream [new file with mode: 0644]
tests/data/plugins/src.ctf.fs/succeed/trace-array-align-elem.expect [new file with mode: 0644]
tests/data/plugins/src.ctf.fs/succeed/trace-struct-array-align-elem.expect [new file with mode: 0644]
tests/plugins/src.ctf.fs/succeed/test_succeed

index 5a6e4deec75c021966ebcc67931c2194ee9a1dae..42a6d4138fa76908ba95bf0a982650facc01b618 100644 (file)
@@ -40,6 +40,7 @@ libctf_ast_la_SOURCES = \
        ctf-meta-update-in-ir.c \
        ctf-meta-update-default-clock-classes.c \
        ctf-meta-update-text-array-sequence.c \
+       ctf-meta-update-alignments.c \
        ctf-meta-update-value-storing-indexes.c \
        ctf-meta-update-stream-class-config.c \
        ctf-meta-warn-meaningless-header-fields.c \
diff --git a/src/plugins/ctf/common/metadata/ctf-meta-update-alignments.c b/src/plugins/ctf/common/metadata/ctf-meta-update-alignments.c
new file mode 100644 (file)
index 0000000..8cea669
--- /dev/null
@@ -0,0 +1,173 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright 2020 Philippe Proulx <pproulx@efficios.com>
+ */
+
+#include <babeltrace2/babeltrace.h>
+#include "common/macros.h"
+#include "common/assert.h"
+#include <glib.h>
+#include <stdint.h>
+#include <string.h>
+#include <inttypes.h>
+
+#include "ctf-meta-visitors.h"
+
+static inline
+int set_alignments(struct ctf_field_class *fc)
+{
+       int ret = 0;
+       uint64_t i;
+
+       if (!fc) {
+               goto end;
+       }
+
+       switch (fc->type) {
+       case CTF_FIELD_CLASS_TYPE_STRUCT:
+       {
+               struct ctf_field_class_struct *struct_fc = (void *) fc;
+
+               for (i = 0; i < struct_fc->members->len; i++) {
+                       struct ctf_named_field_class *named_fc =
+                               ctf_field_class_struct_borrow_member_by_index(
+                                       struct_fc, i);
+
+                       ret = set_alignments(named_fc->fc);
+                       if (ret) {
+                               goto end;
+                       }
+
+                       if (named_fc->fc->alignment > fc->alignment) {
+                               fc->alignment = named_fc->fc->alignment;
+                       }
+               }
+
+               break;
+       }
+       case CTF_FIELD_CLASS_TYPE_VARIANT:
+       {
+               struct ctf_field_class_variant *var_fc = (void *) fc;
+
+               for (i = 0; i < var_fc->options->len; i++) {
+                       struct ctf_named_field_class *named_fc =
+                               ctf_field_class_variant_borrow_option_by_index(
+                                       var_fc, i);
+
+                       ret = set_alignments(named_fc->fc);
+                       if (ret) {
+                               goto end;
+                       }
+               }
+
+               break;
+       }
+       case CTF_FIELD_CLASS_TYPE_ARRAY:
+       case CTF_FIELD_CLASS_TYPE_SEQUENCE:
+       {
+               struct ctf_field_class_array_base *array_fc = (void *) fc;
+
+               ret = set_alignments(array_fc->elem_fc);
+               if (ret) {
+                       goto end;
+               }
+
+               /*
+                * Use the alignment of the array/sequence field class's
+                * element FC as its own alignment.
+                *
+                * This is especially important when the array/sequence
+                * field's effective length is zero: as per CTF 1.8, the
+                * stream data decoding process still needs to align the
+                * cursor using the element's alignment [1]:
+                *
+                * > Arrays are always aligned on their element
+                * > alignment requirement.
+                *
+                * For example:
+                *
+                *     struct {
+                *         integer { size = 8; } a;
+                *         integer { size = 8; align = 16; } b[0];
+                *         integer { size = 8; } c;
+                *     };
+                *
+                * When using this to decode the bytes 1, 2, and 3, then
+                * the decoded values are:
+                *
+                * `a`: 1
+                * `b`: []
+                * `c`: 3
+                *
+                * [1]: https://diamon.org/ctf/#spec4.2.3
+                */
+               array_fc->base.alignment = array_fc->elem_fc->alignment;
+               break;
+       }
+       default:
+               break;
+       }
+
+end:
+       return ret;
+}
+
+BT_HIDDEN
+int ctf_trace_class_update_alignments(
+               struct ctf_trace_class *ctf_tc)
+{
+       int ret = 0;
+       uint64_t i;
+
+       if (!ctf_tc->is_translated) {
+               ret = set_alignments(ctf_tc->packet_header_fc);
+               if (ret) {
+                       goto end;
+               }
+       }
+
+       for (i = 0; i < ctf_tc->stream_classes->len; i++) {
+               struct ctf_stream_class *sc = ctf_tc->stream_classes->pdata[i];
+               uint64_t j;
+
+               if (!sc->is_translated) {
+                       ret = set_alignments(sc->packet_context_fc);
+                       if (ret) {
+                               goto end;
+                       }
+
+                       ret = set_alignments(sc->event_header_fc);
+                       if (ret) {
+                               goto end;
+                       }
+
+                       ret = set_alignments(sc->event_common_context_fc);
+                       if (ret) {
+                               goto end;
+                       }
+               }
+
+               for (j = 0; j < sc->event_classes->len; j++) {
+                       struct ctf_event_class *ec =
+                               sc->event_classes->pdata[j];
+
+                       if (ec->is_translated) {
+                               continue;
+                       }
+
+                       ret = set_alignments(ec->spec_context_fc);
+                       if (ret) {
+                               goto end;
+                       }
+
+                       ret = set_alignments(ec->payload_fc);
+                       if (ret) {
+                               goto end;
+                       }
+               }
+       }
+
+end:
+       return ret;
+}
index 98e794c0713366bb30b5262405df219dd63d73d5..d305d424a4570f21c5e2183a9f29670628a2f9bc 100644 (file)
@@ -44,6 +44,9 @@ int ctf_trace_class_update_meanings(struct ctf_trace_class *ctf_tc);
 BT_HIDDEN
 int ctf_trace_class_update_text_array_sequence(struct ctf_trace_class *ctf_tc);
 
+BT_HIDDEN
+int ctf_trace_class_update_alignments(struct ctf_trace_class *ctf_tc);
+
 BT_HIDDEN
 int ctf_trace_class_update_value_storing_indexes(struct ctf_trace_class *ctf_tc);
 
index 604388b4cf2e17cf005a05ca9b9ab60911280f8f..5f071ff73ee655d528e7289bbe3f989d9d31995c 100644 (file)
@@ -4887,6 +4887,13 @@ int ctf_visitor_generate_ir_visit_node(struct ctf_visitor_generate_ir *visitor,
                goto end;
        }
 
+       /* Update structure/array/sequence alignments */
+       ret = ctf_trace_class_update_alignments(ctx->ctf_tc);
+       if (ret) {
+               ret = -EINVAL;
+               goto end;
+       }
+
        /* Resolve sequence lengths and variant tags */
        ret = ctf_trace_class_resolve_field_classes(ctx->ctf_tc, &ctx->log_cfg);
        if (ret) {
diff --git a/tests/data/ctf-traces/succeed/array-align-elem/metadata b/tests/data/ctf-traces/succeed/array-align-elem/metadata
new file mode 100644 (file)
index 0000000..bf2fe25
--- /dev/null
@@ -0,0 +1,16 @@
+/* CTF 1.8 */
+
+trace {
+       major = 1;
+       minor = 8;
+       byte_order = le;
+};
+
+event {
+       name = ev;
+       fields := struct {
+               integer { size = 8; } a;
+               integer { size = 8; align = 16; } b[0];
+               integer { size = 8; } c;
+       };
+};
diff --git a/tests/data/ctf-traces/succeed/array-align-elem/stream b/tests/data/ctf-traces/succeed/array-align-elem/stream
new file mode 100644 (file)
index 0000000..aed2973
--- /dev/null
@@ -0,0 +1 @@
+\ 1\ 2\ 3
\ No newline at end of file
diff --git a/tests/data/ctf-traces/succeed/struct-array-align-elem/metadata b/tests/data/ctf-traces/succeed/struct-array-align-elem/metadata
new file mode 100644 (file)
index 0000000..4b668b6
--- /dev/null
@@ -0,0 +1,19 @@
+/* CTF 1.8 */
+
+trace {
+       major = 1;
+       minor = 8;
+       byte_order = le;
+};
+
+event {
+       name = ev;
+       fields := struct {
+               integer { size = 8; } x;
+               struct {
+                       integer { size = 8; } a;
+                       integer { size = 8; align = 32; } b[0];
+               } y;
+               integer { size = 8; } z;
+       };
+};
diff --git a/tests/data/ctf-traces/succeed/struct-array-align-elem/stream b/tests/data/ctf-traces/succeed/struct-array-align-elem/stream
new file mode 100644 (file)
index 0000000..158cb14
--- /dev/null
@@ -0,0 +1 @@
+\ 1\ 2\ 3\ 4\ 5\ 6\a\b       
\ No newline at end of file
diff --git a/tests/data/plugins/src.ctf.fs/succeed/trace-array-align-elem.expect b/tests/data/plugins/src.ctf.fs/succeed/trace-array-align-elem.expect
new file mode 100644 (file)
index 0000000..1bff6ba
--- /dev/null
@@ -0,0 +1,34 @@
+Trace class:
+  Stream class (ID 0):
+    Supports packets: Yes
+    Packets have beginning default clock snapshot: No
+    Packets have end default clock snapshot: No
+    Supports discarded events: No
+    Supports discarded packets: No
+    Event class `ev` (ID 0):
+      Payload field class: Structure (3 members):
+        a: Unsigned integer (8-bit, Base 10)
+        b: Static array (Length 0):
+          Element: Unsigned integer (8-bit, Base 10)
+        c: Unsigned integer (8-bit, Base 10)
+
+{Trace 0, Stream class ID 0, Stream ID 0}
+Stream beginning:
+  Trace:
+    Stream (ID 0, Class ID 0)
+
+{Trace 0, Stream class ID 0, Stream ID 0}
+Packet beginning
+
+{Trace 0, Stream class ID 0, Stream ID 0}
+Event `ev` (Class ID 0):
+  Payload:
+    a: 1
+    b: Empty
+    c: 3
+
+{Trace 0, Stream class ID 0, Stream ID 0}
+Packet end
+
+{Trace 0, Stream class ID 0, Stream ID 0}
+Stream end
diff --git a/tests/data/plugins/src.ctf.fs/succeed/trace-struct-array-align-elem.expect b/tests/data/plugins/src.ctf.fs/succeed/trace-struct-array-align-elem.expect
new file mode 100644 (file)
index 0000000..222a00d
--- /dev/null
@@ -0,0 +1,38 @@
+Trace class:
+  Stream class (ID 0):
+    Supports packets: Yes
+    Packets have beginning default clock snapshot: No
+    Packets have end default clock snapshot: No
+    Supports discarded events: No
+    Supports discarded packets: No
+    Event class `ev` (ID 0):
+      Payload field class: Structure (3 members):
+        x: Unsigned integer (8-bit, Base 10)
+        y: Structure (2 members):
+          a: Unsigned integer (8-bit, Base 10)
+          b: Static array (Length 0):
+            Element: Unsigned integer (8-bit, Base 10)
+        z: Unsigned integer (8-bit, Base 10)
+
+{Trace 0, Stream class ID 0, Stream ID 0}
+Stream beginning:
+  Trace:
+    Stream (ID 0, Class ID 0)
+
+{Trace 0, Stream class ID 0, Stream ID 0}
+Packet beginning
+
+{Trace 0, Stream class ID 0, Stream ID 0}
+Event `ev` (Class ID 0):
+  Payload:
+    x: 1
+    y:
+      a: 5
+      b: Empty
+    z: 9
+
+{Trace 0, Stream class ID 0, Stream ID 0}
+Packet end
+
+{Trace 0, Stream class ID 0, Stream ID 0}
+Stream end
index 63ba2499b39197fe921793334110e57742be1f08..9932664badb45c87f8989632cde2a90161afb77a 100755 (executable)
@@ -129,7 +129,7 @@ test_force_origin_unix_epoch() {
        rm -f "$temp_stdout_output_file" "$temp_stderr_output_file"
 }
 
-plan_tests 10
+plan_tests 12
 
 test_force_origin_unix_epoch 2packets barectf-event-before-packet
 test_ctf_gen_single simple
@@ -138,5 +138,7 @@ test_ctf_single 2packets
 test_ctf_single barectf-event-before-packet
 test_ctf_single session-rotation
 test_ctf_single lttng-tracefile-rotation
+test_ctf_single array-align-elem
+test_ctf_single struct-array-align-elem
 test_packet_end lttng-event-after-packet
 test_packet_end lttng-crash
This page took 0.031873 seconds and 4 git commands to generate.