From 0121936a17b5964dd23e8dc291b1371d59de399f Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 10 Nov 2023 20:26:15 +0000 Subject: [PATCH] lib: validate iterator message sequence Validate that the sequence of messages produced by iterators respects the sequence defined by the Message Interchange Protocol (MIP) [1]. Without packets SB (E | DE)* SE With packets SB ((PB (E | DE)* PE) | DE | DP)* SE Validate that when an iterator returns BT_MESSAGE_ITERATOR_CLASS_NEXT_METHOD_STATUS_END, all streams are properly ended (we have received a stream end message for all streams). Add an expected_msg_types field to the iterator's per_stream_state structure, which holds a bit mask of the message types the iterator is allowed to emit next. Use the cur_packet field that is maintained by message_packet_is_valid when processing a discarded events message. We need to know if we are in a packet or not to determine the following valid message types (DE appears twice in the "With packets" sequence above, once within a packet and once outside a packet). It doesn't matter in which order the two assertions are placed, since the "sequence is as expected" assertion doesn't use the cur_packet field when handling the "packet beginning" and "packet end" messages. However, I think that it is more useful to have the "sequence is as expected" first. Some mistakes, like the following sequence where a packet beginning message is forgotten: ... PB E E PE E E PE ... ... would cause both assertions to fail. But the "sequence is as expected" assertion points closer to the root cause of the problem than the "packet is expected" one, which points more to a symptom. So, if both assertions would fail, I prefer that we show the "sequence is as expected" one. Here's an example of the error message that is printed when an error is detected. In this case, a packet beginning message was forgotten. Babeltrace 2 library postcondition not satisfied. ------------------------------------------------------------------------ Condition ID: `post:message-iterator-class-next-method:mip-message-sequence-is-expected`. Function: bt_message_iterator_class_next_method(). ------------------------------------------------------------------------ Error is: MIP message sequence is not expected: stream-addr=0x60d000001d80, stream-id=0, iterator-addr=0x611000004c80, iterator-upstream-comp-name="source.gpx.GpxSource", iterator-upstream-comp-log-level=WARNING, iterator-upstream-comp-class-type=SOURCE, iterator-upstream-comp-class-name="GpxSource", iterator-upstream-comp-class-partial-descr="", message-addr=0x607000004540, message-type=EVENT, expected-msg-types=STREAM_END|PACKET_BEGINNING Aborting... [1] https://babeltrace.org/docs/v2.0/libbabeltrace2/group__api-msg.html#api-msg-seq Change-Id: I25d3ff9b87c551dcced1ba25e0a8525f316a8050 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/10450 Tested-by: jenkins Reviewed-by: Philippe Proulx --- src/lib/graph/iterator.c | 197 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 197 insertions(+) diff --git a/src/lib/graph/iterator.c b/src/lib/graph/iterator.c index dc1c778d..ff9f4a45 100644 --- a/src/lib/graph/iterator.c +++ b/src/lib/graph/iterator.c @@ -70,6 +70,9 @@ struct per_stream_state { bt_packet *cur_packet; + + /* Bit mask of expected message types. */ + guint expected_msg_types; }; #endif @@ -867,6 +870,145 @@ const bt_stream *get_stream_from_msg(const struct bt_message *msg) return stream; } +static +GString *message_types_to_string(guint msg_types) +{ + GString *str = g_string_new(""); + + for (int msg_type = 1; msg_type <= BT_MESSAGE_TYPE_MESSAGE_ITERATOR_INACTIVITY; + msg_type <<= 1) { + if (msg_type & msg_types) { + if (str->len > 0) { + g_string_append_c(str, '|'); + } + + g_string_append(str, + bt_common_message_type_string(msg_type)); + } + } + + return str; +} + +static +void update_expected_msg_type(const struct bt_stream *stream, + struct per_stream_state *state, + const struct bt_message *msg) +{ + switch (msg->type) { + case BT_MESSAGE_TYPE_STREAM_BEGINNING: + state->expected_msg_types = BT_MESSAGE_TYPE_STREAM_END; + + if (stream->class->supports_packets) { + state->expected_msg_types |= + BT_MESSAGE_TYPE_PACKET_BEGINNING; + + if (stream->class->supports_discarded_packets) { + state->expected_msg_types |= + BT_MESSAGE_TYPE_DISCARDED_PACKETS; + } + } else { + state->expected_msg_types |= BT_MESSAGE_TYPE_EVENT; + } + + if (stream->class->supports_discarded_events) { + state->expected_msg_types |= + BT_MESSAGE_TYPE_DISCARDED_EVENTS; + } + + break; + case BT_MESSAGE_TYPE_STREAM_END: + state->expected_msg_types = 0; + break; + case BT_MESSAGE_TYPE_EVENT: + { + state->expected_msg_types = BT_MESSAGE_TYPE_EVENT; + + if (stream->class->supports_packets) { + state->expected_msg_types |= BT_MESSAGE_TYPE_PACKET_END; + } else { + state->expected_msg_types |= BT_MESSAGE_TYPE_STREAM_END; + } + + if (stream->class->supports_discarded_events) { + state->expected_msg_types |= + BT_MESSAGE_TYPE_DISCARDED_EVENTS; + } + + break; + } + case BT_MESSAGE_TYPE_PACKET_BEGINNING: + { + state->expected_msg_types = BT_MESSAGE_TYPE_EVENT | + BT_MESSAGE_TYPE_PACKET_END; + + if (stream->class->supports_discarded_events) { + state->expected_msg_types |= + BT_MESSAGE_TYPE_DISCARDED_EVENTS; + } + + break; + } + case BT_MESSAGE_TYPE_PACKET_END: + { + state->expected_msg_types = BT_MESSAGE_TYPE_PACKET_BEGINNING | + BT_MESSAGE_TYPE_STREAM_END; + + if (stream->class->supports_discarded_events) { + state->expected_msg_types |= + BT_MESSAGE_TYPE_DISCARDED_EVENTS; + } + + if (stream->class->supports_discarded_packets) { + state->expected_msg_types |= + BT_MESSAGE_TYPE_DISCARDED_PACKETS; + } + + break; + } + case BT_MESSAGE_TYPE_DISCARDED_EVENTS: + state->expected_msg_types = BT_MESSAGE_TYPE_DISCARDED_EVENTS; + + if (state->cur_packet) { + state->expected_msg_types |= BT_MESSAGE_TYPE_EVENT | + BT_MESSAGE_TYPE_PACKET_END; + } else { + state->expected_msg_types |= BT_MESSAGE_TYPE_STREAM_END; + + if (stream->class->supports_packets) { + state->expected_msg_types |= + BT_MESSAGE_TYPE_PACKET_BEGINNING; + + if (stream->class->supports_discarded_packets) { + state->expected_msg_types |= + BT_MESSAGE_TYPE_DISCARDED_PACKETS; + } + } else { + state->expected_msg_types |= + BT_MESSAGE_TYPE_EVENT; + } + } + + break; + case BT_MESSAGE_TYPE_DISCARDED_PACKETS: + state->expected_msg_types = BT_MESSAGE_TYPE_DISCARDED_PACKETS | + BT_MESSAGE_TYPE_PACKET_BEGINNING | + BT_MESSAGE_TYPE_STREAM_END; + + if (stream->class->supports_discarded_events) { + state->expected_msg_types |= + BT_MESSAGE_TYPE_DISCARDED_EVENTS; + } + break; + default: + /* + * Other message types are not associated to a stream, so we + * should not get them here. + */ + bt_common_abort(); + } +} + static struct per_stream_state *get_per_stream_state( struct bt_message_iterator *iterator, @@ -877,6 +1019,7 @@ struct per_stream_state *get_per_stream_state( if (!state) { state = g_new0(struct per_stream_state, 1); + state->expected_msg_types = BT_MESSAGE_TYPE_STREAM_BEGINNING; g_hash_table_insert(iterator->per_stream_state, (gpointer) stream, state); } @@ -888,6 +1031,38 @@ struct per_stream_state *get_per_stream_state( #define NEXT_METHOD_NAME "bt_message_iterator_class_next_method" #ifdef BT_DEV_MODE +static +void assert_post_dev_expected_sequence(struct bt_message_iterator *iterator, + const struct bt_message *msg) +{ + const bt_stream *stream = get_stream_from_msg(msg); + struct per_stream_state *state; + + if (!stream) { + goto end; + } + + state = get_per_stream_state(iterator, stream); + + /* + * We don't free the return value of message_types_to_string(), but + * that's because we know the program is going to abort anyway, and + * we don't want to call it if the assertion holds. + */ + BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, + "message-type-is-expected", + msg->type & state->expected_msg_types, + "Unexpected message type: %![stream-]s, %![iterator-]i, " + "%![message-]n, expected-msg-types=%s", + stream, iterator, msg, + message_types_to_string(state->expected_msg_types)->str); + + update_expected_msg_type(stream, state, msg); + +end: + return; +} + static void assert_post_dev_expected_packet(struct bt_message_iterator *iterator, const struct bt_message *msg) @@ -958,8 +1133,30 @@ void assert_post_dev_next( uint64_t i; for (i = 0; i < msg_count; i++) { + assert_post_dev_expected_sequence(iterator, msgs[i]); assert_post_dev_expected_packet(iterator, msgs[i]); } + } else if (status == BT_MESSAGE_ITERATOR_CLASS_NEXT_METHOD_STATUS_END) { + GHashTableIter iter; + + gpointer stream_v, stream_state_v; + + g_hash_table_iter_init(&iter, iterator->per_stream_state); + while (g_hash_table_iter_next(&iter, &stream_v, + &stream_state_v)) { + struct bt_stream *stream = stream_v; + struct per_stream_state *stream_state = stream_state_v; + + BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, + "stream-is-ended", + stream_state->expected_msg_types == 0, + "Stream is not ended: %![stream-]s, " + "%![iterator-]i, expected-msg-types=%s", + stream, iterator, + message_types_to_string( + stream_state->expected_msg_types)->str); + } + } } #endif -- 2.34.1