From e6938018975e45d35dab5fef795fe7344eef7d62 Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Tue, 21 Jul 2020 13:57:10 -0400 Subject: [PATCH] Fix: `ctf` plugin: use element FC's alignment as array/seq. FC alignment 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 Change-Id: If1ccfe374976e9faef590f2fe7ff6bac2f335df8 Reviewed-on: https://review.lttng.org/c/babeltrace/+/3805 --- src/plugins/ctf/common/metadata/Makefile.am | 1 + .../metadata/ctf-meta-update-alignments.c | 173 ++++++++++++++++++ .../ctf/common/metadata/ctf-meta-visitors.h | 3 + .../ctf/common/metadata/visitor-generate-ir.c | 7 + .../succeed/array-align-elem/metadata | 16 ++ .../succeed/array-align-elem/stream | 1 + .../succeed/struct-array-align-elem/metadata | 19 ++ .../succeed/struct-array-align-elem/stream | 1 + .../succeed/trace-array-align-elem.expect | 34 ++++ .../trace-struct-array-align-elem.expect | 38 ++++ tests/plugins/src.ctf.fs/succeed/test_succeed | 4 +- 11 files changed, 296 insertions(+), 1 deletion(-) create mode 100644 src/plugins/ctf/common/metadata/ctf-meta-update-alignments.c create mode 100644 tests/data/ctf-traces/succeed/array-align-elem/metadata create mode 100644 tests/data/ctf-traces/succeed/array-align-elem/stream create mode 100644 tests/data/ctf-traces/succeed/struct-array-align-elem/metadata create mode 100644 tests/data/ctf-traces/succeed/struct-array-align-elem/stream create mode 100644 tests/data/plugins/src.ctf.fs/succeed/trace-array-align-elem.expect create mode 100644 tests/data/plugins/src.ctf.fs/succeed/trace-struct-array-align-elem.expect diff --git a/src/plugins/ctf/common/metadata/Makefile.am b/src/plugins/ctf/common/metadata/Makefile.am index 5d2db270..7fe4ce77 100644 --- a/src/plugins/ctf/common/metadata/Makefile.am +++ b/src/plugins/ctf/common/metadata/Makefile.am @@ -42,6 +42,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 index 00000000..8cea669a --- /dev/null +++ b/src/plugins/ctf/common/metadata/ctf-meta-update-alignments.c @@ -0,0 +1,173 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright 2020 Philippe Proulx + */ + +#include +#include "common/macros.h" +#include "common/assert.h" +#include +#include +#include +#include + +#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; +} diff --git a/src/plugins/ctf/common/metadata/ctf-meta-visitors.h b/src/plugins/ctf/common/metadata/ctf-meta-visitors.h index 6152e17d..34edb37e 100644 --- a/src/plugins/ctf/common/metadata/ctf-meta-visitors.h +++ b/src/plugins/ctf/common/metadata/ctf-meta-visitors.h @@ -36,6 +36,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); diff --git a/src/plugins/ctf/common/metadata/visitor-generate-ir.c b/src/plugins/ctf/common/metadata/visitor-generate-ir.c index cfb57705..810f3ed1 100644 --- a/src/plugins/ctf/common/metadata/visitor-generate-ir.c +++ b/src/plugins/ctf/common/metadata/visitor-generate-ir.c @@ -4867,6 +4867,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 index 00000000..bf2fe258 --- /dev/null +++ b/tests/data/ctf-traces/succeed/array-align-elem/metadata @@ -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 index 00000000..aed2973e --- /dev/null +++ b/tests/data/ctf-traces/succeed/array-align-elem/stream @@ -0,0 +1 @@ + \ 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 index 00000000..4b668b6b --- /dev/null +++ b/tests/data/ctf-traces/succeed/struct-array-align-elem/metadata @@ -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 index 00000000..158cb140 --- /dev/null +++ b/tests/data/ctf-traces/succeed/struct-array-align-elem/stream @@ -0,0 +1 @@ + \ 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 index 00000000..1bff6baa --- /dev/null +++ b/tests/data/plugins/src.ctf.fs/succeed/trace-array-align-elem.expect @@ -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 index 00000000..222a00d4 --- /dev/null +++ b/tests/data/plugins/src.ctf.fs/succeed/trace-struct-array-align-elem.expect @@ -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 diff --git a/tests/plugins/src.ctf.fs/succeed/test_succeed b/tests/plugins/src.ctf.fs/succeed/test_succeed index 68b70b73..b73b41b7 100755 --- a/tests/plugins/src.ctf.fs/succeed/test_succeed +++ b/tests/plugins/src.ctf.fs/succeed/test_succeed @@ -125,7 +125,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 @@ -134,5 +134,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 -- 2.34.1