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 <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/11990
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

diff --git a/src/Makefile.am b/src/Makefile.am
index d42b5e155a3f4ae346588257b31496a8292166c9..bdd43fc0dd66b667e9bab0bd92a57f281891c9f7 100644 (file)
--- 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 \
 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 \
 
 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 1cb1e6a074f19af053650d58acdbe049987eda8b..c3cd9fecaf1ad4865131603300531f50f4e9a656 100644 (file)
--- 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 "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
 
 /*
  * 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);
 
                "%!+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
        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;
        }
 
                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);
        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;
        }
 
                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;
 
        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)
 {
                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 83bf73c8af198373805b799a2bda5edc080187fe..e508dc2bf66b7108886e4d74f0b47729c48ec58e 100644 (file)
--- 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;
 
         */
        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);
 
        /*
        BT_IF_DEV_MODE(GHashTable *per_stream_state);
 
        /*

diff --git a/tests/lib/Makefile.am b/tests/lib/Makefile.am
index 229e53c0f4ce363fa96d13b0097333b5f7291fa3..ccc7519b847941d10548e550f5380bdf750fe2ea 100644 (file)
--- 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
 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
 
 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
 
 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
 
 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 \
 
 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 664a99b50970c801531e6689ba0710782721b67a..1bd548ddc80f8c211e4a1f8b2dc90cc55641ec83 100644 (file)
--- 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
        $(ELFUTILS_LIBS) \
        $(LIBTAP)
 test_bin_info_SOURCES = test-bin-info.c

+nodist_EXTRA_test_bin_info_SOURCES = dummy.cpp
+

 endif # ENABLE_DEBUG_INFO
 endif # ENABLE_DEBUG_INFO