From: Simon Marchi Date: Thu, 21 Mar 2024 19:17:16 +0000 (-0400) Subject: lib/graph/iterator: use clock correlation validator util X-Git-Url: https://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=78d07b86b198c6d34673d6016e5926b4e50f21a4 lib/graph/iterator: use clock correlation validator util Make `graph/iterator.{c,h}` use the clock correlation validator convenience library. - Replace the fields of `struct bt_message_iterator` that track the clock expectation state with a single `struct bt_clock_correlation_validator` field, only present if babeltrace is built in dev mode. - Initialize `bt_message_iterator::correlation_validator` in create_self_component_input_port_message_iterator. Finalize it in `bt_message_iterator_destroy`. - Replace the bulk of `assert_post_dev_clock_classes_are_compatible_one` with one call to `bt_clock_correlation_validator_validate_message`. If the latter returns an error, trigger an assertion failure based on the return error type and associated information. The modifications in the Makefiles (adding `dummy.cpp`) are to force linking with the C++ compiler driver. This is necessary, since the `libbabeltrace2.so` library now requires some symbols from the C++ standard library (e.g. `libstdc++.so`). The `dummy.cpp` does not actually need to exist. This trick is described in the automake doc [1]. [1] https://www.gnu.org/software/automake/manual/automake.html#Libtool-Convenience-Libraries Change-Id: I46a3a553d5e18eb6860d65d928ef33d941e9e951 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/11990 Tested-by: jenkins Reviewed-by: Philippe Proulx --- diff --git a/src/Makefile.am b/src/Makefile.am index d42b5e15..bdd43fc0 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -497,7 +497,10 @@ lib_libbabeltrace2_la_CPPFLAGS = \ lib_libbabeltrace2_la_LIBADD = \ logging/liblogging.la \ common/libcommon.la \ - compat/libcompat.la + compat/libcompat.la \ + clock-correlation-validator/libclock-correlation-validator.la + +nodist_EXTRA_lib_libbabeltrace2_la_SOURCES = dummy.cpp ctf_writer_libbabeltrace2_ctf_writer_la_SOURCES = \ ctf-writer/assert-pre.h \ diff --git a/src/lib/graph/iterator.c b/src/lib/graph/iterator.c index 1cb1e6a0..c3cd9fec 100644 --- a/src/lib/graph/iterator.c +++ b/src/lib/graph/iterator.c @@ -50,6 +50,7 @@ #include "message/stream.h" #include "message/packet.h" #include "lib/func-status.h" +#include "clock-correlation-validator/clock-correlation-validator.h" /* * TODO: Use graph's state (number of active iterators, etc.) and @@ -119,12 +120,6 @@ void bt_message_iterator_destroy(struct bt_object *obj) "%!+i", iterator); bt_message_iterator_try_finalize(iterator); - if (iterator->clock_expectation.type == - CLOCK_EXPECTATION_ORIGIN_OTHER_NO_UUID) { - BT_CLOCK_CLASS_PUT_REF_AND_RESET( - iterator->clock_expectation.clock_class); - } - if (iterator->connection) { /* * Remove ourself from the originating connection so @@ -160,6 +155,8 @@ void bt_message_iterator_destroy(struct bt_object *obj) iterator->msgs = NULL; } + BT_IF_DEV_MODE(bt_clock_correlation_validator_destroy( + iterator->correlation_validator)); BT_IF_DEV_MODE(g_hash_table_destroy(iterator->per_stream_state)); g_free(iterator); @@ -366,6 +363,17 @@ int create_self_component_input_port_message_iterator( goto error; } + BT_IF_DEV_MODE( + iterator->correlation_validator = + bt_clock_correlation_validator_create(); + if (!iterator->correlation_validator) { + BT_LIB_LOGE_APPEND_CAUSE( + "Failed to allocate a clock correlation validator."); + status = BT_FUNC_STATUS_MEMORY_ERROR; + goto error; + } + ); + g_ptr_array_set_size(iterator->msgs, MSG_BATCH_SIZE); iterator->last_ns_from_origin = INT64_MIN; @@ -677,113 +685,61 @@ void assert_post_dev_clock_classes_are_compatible_one( struct bt_message_iterator *iterator, const struct bt_message *msg) { - const struct bt_clock_class *clock_class = NULL; - bt_uuid clock_class_uuid = NULL; - - switch (bt_message_get_type(msg)) { - case BT_MESSAGE_TYPE_STREAM_BEGINNING: - { - const struct bt_message_stream *stream_msg = - (struct bt_message_stream *) msg; - - clock_class = stream_msg->stream->class->default_clock_class; - break; - } - case BT_MESSAGE_TYPE_MESSAGE_ITERATOR_INACTIVITY: - { - const struct bt_message_message_iterator_inactivity *mii_msg = - (struct bt_message_message_iterator_inactivity *) msg; - - clock_class = mii_msg->cs->clock_class; - break; - } - default: - return; - } - - if (clock_class) { - clock_class_uuid = bt_clock_class_get_uuid(clock_class); - } - - switch (iterator->clock_expectation.type) { - case CLOCK_EXPECTATION_UNSET: - /* - * This is the first time we see a message with a clock - * snapshot: record the properties of that clock, against - * which we'll compare the clock properties of the following - * messages. - */ + enum bt_clock_correlation_validator_error_type type; + bt_uuid expected_uuid; + const bt_clock_class *actual_clock_cls; + const bt_clock_class *expected_clock_cls; + + if (!bt_clock_correlation_validator_validate_message( + iterator->correlation_validator, msg, &type, + &expected_uuid, &actual_clock_cls, + &expected_clock_cls)) { + switch (type) { + case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_NO_CLOCK_CLASS_GOT_ONE: + BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, + "stream-class-has-no-clock-class", false, + "Expecting no clock class, got one."); + case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UNIX_GOT_NONE: + BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, + "stream-class-has-clock-class-with-unix-epoch-origin", false, + "Expecting a clock class, got none."); + case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UNIX_GOT_OTHER: + BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, + "clock-class-has-unix-epoch-origin", false, + "Expecting a clock class with Unix epoch origin: %![cc-]+K", + actual_clock_cls); + case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UUID_GOT_NONE: + BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, + "stream-class-has-clock-class-with-uuid", false, + "Expecting a clock class, got none."); + case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UUID_GOT_UNIX: + BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, + "clock-class-has-non-unix-epoch-origin", false, + "Expecting a clock class without Unix epoch origin: %![cc-]+K", + actual_clock_cls); + case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UUID_GOT_NO_UUID: + BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, + "clock-class-has-uuid", false, + "Expecting a clock class with UUID: %![cc-]+K", + actual_clock_cls); + case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UUID_GOT_OTHER_UUID: + BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, + "clock-class-has-expected-uuid", false, + "Expecting a clock class with UUID, got one with a different UUID: %![cc-]+K, expected-uuid=%!u", + actual_clock_cls, expected_uuid); - if (!clock_class) { - iterator->clock_expectation.type = CLOCK_EXPECTATION_NONE; - } else if (bt_clock_class_origin_is_unix_epoch(clock_class)) { - iterator->clock_expectation.type = CLOCK_EXPECTATION_ORIGIN_UNIX; - } else if (clock_class_uuid) { - iterator->clock_expectation.type = CLOCK_EXPECTATION_ORIGIN_OTHER_UUID; - bt_uuid_copy(iterator->clock_expectation.uuid, clock_class_uuid); - } else { - iterator->clock_expectation.type = CLOCK_EXPECTATION_ORIGIN_OTHER_NO_UUID; - iterator->clock_expectation.clock_class = clock_class; - bt_clock_class_get_ref(iterator->clock_expectation.clock_class); + case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_NO_UUID_GOT_NONE: + BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, + "stream-class-has-clock-class", false, + "Expecting a clock class, got none."); + case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_NO_UUID_GOT_OTHER: + BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, + "clock-class-is-expected", false, + "Unexpected clock class: %![expected-cc-]+K, %![actual-cc-]+K", + expected_clock_cls, actual_clock_cls); } - break; - - case CLOCK_EXPECTATION_NONE: - BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, - "stream-class-has-no-clock-class", !clock_class, - "Expecting no clock class, got one: %![cc-]+K", - clock_class); - break; - - case CLOCK_EXPECTATION_ORIGIN_UNIX: - BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, - "stream-class-has-clock-class-with-unix-epoch-origin", clock_class, - "Expecting a clock class with Unix epoch origin, got none."); - - BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, - "clock-class-has-unix-epoch-origin", - bt_clock_class_origin_is_unix_epoch(clock_class), - "Expecting a clock class with Unix epoch origin: %![cc-]+K", - clock_class); - break; - - case CLOCK_EXPECTATION_ORIGIN_OTHER_UUID: - BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, - "stream-class-has-clock-class-with-uuid", clock_class, - "Expecting a clock class with UUID, got none."); - - BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, - "clock-class-has-non-unix-epoch-origin", - !bt_clock_class_origin_is_unix_epoch(clock_class), - "Expecting a clock class without Unix epoch origin: %![cc-]+K", - clock_class); - - BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, - "clock-class-has-uuid", - clock_class_uuid, - "Expecting a clock class with UUID, got one without UUID: %![cc-]+K", - clock_class); - BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, - "clock-class-has-expected-uuid", - !bt_uuid_compare(iterator->clock_expectation.uuid, clock_class_uuid), - "Expecting a clock class with UUID, got one " - "with a different UUID: %![cc-]+K, expected-uuid=%!u", - clock_class, iterator->clock_expectation.uuid); - break; - - case CLOCK_EXPECTATION_ORIGIN_OTHER_NO_UUID: - BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, - "stream-class-has-clock-class", clock_class, - "Expecting a clock class, got none."); - - BT_ASSERT_POST_DEV(NEXT_METHOD_NAME, - "clock-class-is-expected", - clock_class == iterator->clock_expectation.clock_class, - "Expecting clock class %![cc-]+K, got %![cc-]+K.", - iterator->clock_expectation.clock_class, - clock_class); - break; + bt_common_abort(); } } diff --git a/src/lib/graph/iterator.h b/src/lib/graph/iterator.h index 83bf73c8..e508dc2b 100644 --- a/src/lib/graph/iterator.h +++ b/src/lib/graph/iterator.h @@ -123,55 +123,8 @@ struct bt_message_iterator { */ int64_t last_ns_from_origin; - struct { - enum { - /* We haven't recorded clock properties yet. */ - CLOCK_EXPECTATION_UNSET, - - /* Expect to have no clock. */ - CLOCK_EXPECTATION_NONE, - - /* Clock with origin_is_unix_epoch true.*/ - CLOCK_EXPECTATION_ORIGIN_UNIX, - - /* Clock with origin_is_unix_epoch false, with a UUID.*/ - CLOCK_EXPECTATION_ORIGIN_OTHER_UUID, - - /* Clock with origin_is_unix_epoch false, without a UUID.*/ - CLOCK_EXPECTATION_ORIGIN_OTHER_NO_UUID, - } type; - - - union { - /* - * Expected UUID of the clock, if `type`is - * CLOCK_EXPECTATION_ORIGIN_OTHER_UUID. - * - * If the clock's origin is the unix epoch, the UUID is - * irrelevant (as the clock will be correlatable with other - * clocks having the same origin). - */ - bt_uuid_t uuid; - - /* - * Expected clock class, if `type` is - * CLOCK_EXPECTATION_ORIGIN_OTHER_NO_UUID. - * - * If the first clock class seen has an unknown origin - * and no UUID, then all subsequent clock classes seen - * must be the same instance. - * - * To make sure that the clock class pointed by this - * field doesn't get freed and another one reallocated - * at the same address (which could potentially bypass - * the clock expectation check), we keep a strong - * reference, ensuring that the clock class lives at - * least as long as the iterator. - */ - const bt_clock_class *clock_class; - }; - } clock_expectation; - + BT_IF_DEV_MODE( + struct bt_clock_correlation_validator *correlation_validator); BT_IF_DEV_MODE(GHashTable *per_stream_state); /* diff --git a/tests/lib/Makefile.am b/tests/lib/Makefile.am index 229e53c0..ccc7519b 100644 --- a/tests/lib/Makefile.am +++ b/tests/lib/Makefile.am @@ -37,20 +37,24 @@ test_trace_ir_ref_SOURCES = test-trace-ir-ref.c test_trace_ir_ref_LDADD = $(COMMON_TEST_LDADD) \ $(top_builddir)/src/lib/libbabeltrace2.la \ $(top_builddir)/src/ctf-writer/libbabeltrace2-ctf-writer.la +nodist_EXTRA_test_trace_ir_ref_SOURCES = dummy.cpp test_graph_topo_SOURCES = test-graph-topo.c test_graph_topo_LDADD = $(COMMON_TEST_LDADD) \ $(top_builddir)/src/lib/libbabeltrace2.la +nodist_EXTRA_test_graph_topo_SOURCES = dummy.cpp test_simple_sink_SOURCES = test-simple-sink.c test_simple_sink_LDADD = $(COMMON_TEST_LDADD) \ $(top_builddir)/src/lib/libbabeltrace2.la +nodist_EXTRA_test_simple_sink_SOURCES = dummy.cpp test_remove_destruction_listener_in_destruction_listener_SOURCES = \ test-remove-destruction-listener-in-destruction-listener.c test_remove_destruction_listener_in_destruction_listener_LDADD = \ $(COMMON_TEST_LDADD) \ $(top_builddir)/src/lib/libbabeltrace2.la +nodist_EXTRA_test_remove_destruction_listener_in_destruction_listener_SOURCES = dummy.cpp noinst_PROGRAMS = \ test-bt-uuid \ diff --git a/tests/plugins/flt.lttng-utils.debug-info/Makefile.am b/tests/plugins/flt.lttng-utils.debug-info/Makefile.am index 664a99b5..1bd548dd 100644 --- a/tests/plugins/flt.lttng-utils.debug-info/Makefile.am +++ b/tests/plugins/flt.lttng-utils.debug-info/Makefile.am @@ -42,4 +42,6 @@ test_bin_info_LDADD = \ $(ELFUTILS_LIBS) \ $(LIBTAP) test_bin_info_SOURCES = test-bin-info.c +nodist_EXTRA_test_bin_info_SOURCES = dummy.cpp + endif # ENABLE_DEBUG_INFO