Skip to content

Commit

Permalink
Fix: ctf plugin: use element FC's alignment as array/seq. FC alignment
Browse files Browse the repository at this point in the history
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 [email protected]: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 [email protected]: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 [email protected]: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 [email protected]: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 [email protected]: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 [email protected]: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 [email protected]: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 [email protected]: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 [email protected]: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 [email protected]: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 [email protected]: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 <[email protected]>
Change-Id: I06b4fbeda9ffd5b87b4964dd567027d0bfd5e9c7
Reviewed-on: https://review.lttng.org/c/babeltrace/+/4056
  • Loading branch information
eepp committed Sep 10, 2020
1 parent 6b1463e commit 0e32706
Show file tree
Hide file tree
Showing 11 changed files with 296 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/plugins/ctf/common/metadata/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
173 changes: 173 additions & 0 deletions src/plugins/ctf/common/metadata/ctf-meta-update-alignments.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
/*
* SPDX-License-Identifier: MIT
*
* Copyright 2020 Philippe Proulx <[email protected]>
*/

#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;
}
3 changes: 3 additions & 0 deletions src/plugins/ctf/common/metadata/ctf-meta-visitors.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
7 changes: 7 additions & 0 deletions src/plugins/ctf/common/metadata/visitor-generate-ir.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
16 changes: 16 additions & 0 deletions tests/data/ctf-traces/succeed/array-align-elem/metadata
Original file line number Diff line number Diff line change
@@ -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;
};
};
1 change: 1 addition & 0 deletions tests/data/ctf-traces/succeed/array-align-elem/stream
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

19 changes: 19 additions & 0 deletions tests/data/ctf-traces/succeed/struct-array-align-elem/metadata
Original file line number Diff line number Diff line change
@@ -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;
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
4 changes: 3 additions & 1 deletion tests/plugins/src.ctf.fs/succeed/test_succeed
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

0 comments on commit 0e32706

Please sign in to comment.