From 63c3462c3dbd028a08f7a9b504c45e178371248d Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Fri, 25 Nov 2022 16:18:42 -0500 Subject: [PATCH] Fix: sessiond: work-around mismatching variant type tag field and selector names MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Observed issue -------------- Applications fail to register to the session daemon when an application context is used. The following error is printed by the session daemon: Error: Failed to handle application context: Invalid variant choice: `none` does not match any mapping in `$app.mayo:ketchup_tag` enumeration [operator()() ust-field-convert.cpp:606] Cause ----- Application contexts are expressed as variants. LTTng-UST announces those by registering an enumeration named `..._tag`. It then registers a variant as part of the event context that contains the various possible types. Unfortunately, the names used in the enumeration and variant don't match: the enumeration names are all prefixed with an underscore while the variant type tag fields aren't. In preparation for adding the support for the CTF 2 format, the session daemon's handling of user space field class declarations was decoupled from the generation of TSDL. Trace layouts are now expressed in an internal intermediate representation that is used to generate TSDL or CTF 2 independently. As part of the conversion from liblttng-ust-ctl's communication structures to the internal IR (see ust-field-convert.cpp), a certain number of validations steps are taken. One of them ensures that variant fields match a mapping within its '_tag' enumeration. This step fails and produces the error message above. While the CTF 1.8.3 specification mentions that underscores *should* (not *must*) be removed by CTF readers. Babeltrace 1.x (and possibly others) expect a perfect match between the names used by tags and variants. The previous implementation of the TSDL producer always prepended '_' to identifiers in order to side-step the problem of escaping TSDL keywords and ensuring identifiers started with an alphabetic character. This is not the case for enumeration mappings. Hence, I presume that the underscores were added on the LTTng-UST side to coherce the session daemon into producing a trace that Babeltrace 1.x would accept. For reference, using LTTng 2.13 to trace the `$app:hey:you` application context results in the following declaration in the TSDL metadata: stream { id = 0; event.header := struct event_header_large; packet.context := struct packet_context; event.context := struct { enum : integer { size = 8; align = 8; signed = 1; encoding = none; base = 10; } { "_none" = 0, "_int8" = 1, "_int16" = 2, "_int32" = 3, "_int64" = 4, "_uint8" = 5, "_uint16" = 6, "_uint32" = 7, "_uint64" = 8, "_float" = 9, "_double" = 10, "_string" = 11, } __app_hey_you_tag; variant <__app_hey_you_tag> { struct {} _none; integer { size = 8; align = 8; signed = 1; encoding = none; base = 10; } _int8; integer { size = 16; align = 8; signed = 1; encoding = none; base = 10; } _int16; integer { size = 32; align = 8; signed = 1; encoding = none; base = 10; } _int32; integer { size = 64; align = 8; signed = 1; encoding = none; base = 10; } _int64; integer { size = 8; align = 8; signed = 0; encoding = none; base = 10; } _uint8; integer { size = 16; align = 8; signed = 0; encoding = none; base = 10; } _uint16; integer { size = 32; align = 8; signed = 0; encoding = none; base = 10; } _uint32; integer { size = 64; align = 8; signed = 0; encoding = none; base = 10; } _uint64; floating_point { exp_dig = 8; mant_dig = 24; align = 8; } _float; floating_point { exp_dig = 11; mant_dig = 53; align = 8; } _double; string _string; } __app_hey_you; }; }; Solution -------- During the creation of variant field types, the check for matching variant/tag names is made more permissive if the registering tracer has an affected ABI major version, which is always true for the moment. In that mode, the variant fields are renamed to match the mappings in the tag field when they differ by a leading underscore. For a reader, this results in the same apparent trace where mappings and variant fields are prefixed with an underscore. A change to LTTng-UST removing the underscores from the enumeration mapping names has been submitted. That fix bumps the LTTNG_UST_ABI_MAJOR_VERSION to 10. Once/if that fix is accepted, the check will return to its original strict-ness. This change ensures that CTF 2 traces don't carry the leading underscore baggage when tracing applications built against LTTng-UST 2.14+. Signed-off-by: Jérémie Galarneau Change-Id: I0e3ad6668baefaea011a878746e041bd17f6373c --- src/bin/lttng-sessiond/ust-app.cpp | 7 +- src/bin/lttng-sessiond/ust-field-convert.cpp | 109 ++++++++++++------- src/bin/lttng-sessiond/ust-field-convert.hpp | 26 ++++- 3 files changed, 100 insertions(+), 42 deletions(-) diff --git a/src/bin/lttng-sessiond/ust-app.cpp b/src/bin/lttng-sessiond/ust-app.cpp index 9690115d6..8addb5eda 100644 --- a/src/bin/lttng-sessiond/ust-app.cpp +++ b/src/bin/lttng-sessiond/ust-app.cpp @@ -6430,7 +6430,8 @@ static int handle_app_register_channel_notification(int sock, auto app_context_fields = lsu::create_trace_fields_from_ust_ctl_fields( *locked_registry_session, ust_ctl_context_fields.get(), context_field_count, - lst::field_location::root::EVENT_RECORD_COMMON_CONTEXT); + lst::field_location::root::EVENT_RECORD_COMMON_CONTEXT, + lsu::ctl_field_quirks::UNDERSCORE_PREFIXED_VARIANT_TAG_MAPPINGS); if (!ust_reg_chan.is_registered()) { lst::type::cuptr event_context = app_context_fields.size() ? @@ -6566,7 +6567,9 @@ static int add_event_ust_registry(int sock, int sobjd, int cobjd, const char *na *locked_registry, fields.get(), nr_fields, lst::field_location::root:: - EVENT_RECORD_PAYLOAD), + EVENT_RECORD_PAYLOAD, + lsu::ctl_field_quirks:: + UNDERSCORE_PREFIXED_VARIANT_TAG_MAPPINGS), loglevel_value, model_emf_uri.get() ? nonstd::optional( diff --git a/src/bin/lttng-sessiond/ust-field-convert.cpp b/src/bin/lttng-sessiond/ust-field-convert.cpp index 91ab45c1a..21fd3e0c9 100644 --- a/src/bin/lttng-sessiond/ust-field-convert.cpp +++ b/src/bin/lttng-sessiond/ust-field-convert.cpp @@ -50,7 +50,8 @@ lst::type::cuptr create_type_from_ust_ctl_fields(const lttng_ust_ctl_field *curr publish_field_fn publish_field, lookup_field_fn lookup_field, lst::field_location::root lookup_root, - lst::field_location::elements& current_field_location_elements); + lst::field_location::elements& current_field_location_elements, + lsu::ctl_field_quirks quirks); void create_field_from_ust_ctl_fields(const lttng_ust_ctl_field *current, const lttng_ust_ctl_field *end, @@ -59,7 +60,8 @@ void create_field_from_ust_ctl_fields(const lttng_ust_ctl_field *current, publish_field_fn publish_field, lookup_field_fn lookup_field, lst::field_location::root lookup_root, - lst::field_location::elements& current_field_location_elements); + lst::field_location::elements& current_field_location_elements, + lsu::ctl_field_quirks quirks); template enum lst::null_terminated_string_type::encoding ust_ctl_encoding_to_string_field_encoding(UstCtlEncodingType encoding) @@ -104,7 +106,8 @@ enum lst::integer_type::base ust_ctl_base_to_integer_field_base(UstCtlBaseType b lst::type::cuptr create_integer_type_from_ust_ctl_fields(const lttng_ust_ctl_field *current, const lttng_ust_ctl_field *end, const session_attributes& session_attributes, - const lttng_ust_ctl_field **next_ust_ctl_field) + const lttng_ust_ctl_field **next_ust_ctl_field, + lsu::ctl_field_quirks quirks __attribute__((unused))) { if (current >= end) { LTTNG_THROW_PROTOCOL_ERROR( @@ -129,7 +132,8 @@ lst::type::cuptr create_integer_type_from_ust_ctl_fields(const lttng_ust_ctl_fie lst::type::cuptr create_floating_point_type_from_ust_ctl_fields(const lttng_ust_ctl_field *current, const lttng_ust_ctl_field *end, const session_attributes& session_attributes, - const lttng_ust_ctl_field **next_ust_ctl_field) + const lttng_ust_ctl_field **next_ust_ctl_field, + lsu::ctl_field_quirks quirks __attribute__((unused))) { if (current >= end) { LTTNG_THROW_PROTOCOL_ERROR( @@ -156,7 +160,8 @@ lst::type::cuptr create_floating_point_type_from_ust_ctl_fields(const lttng_ust_ lst::type::cuptr create_enumeration_type_from_ust_ctl_fields(const lttng_ust_ctl_field *current, const lttng_ust_ctl_field *end, const session_attributes& session_attributes, - const lttng_ust_ctl_field **next_ust_ctl_field) + const lttng_ust_ctl_field **next_ust_ctl_field, + lsu::ctl_field_quirks quirks __attribute__((unused))) { if (current >= end) { LTTNG_THROW_PROTOCOL_ERROR( @@ -228,7 +233,8 @@ lst::type::cuptr create_enumeration_type_from_ust_ctl_fields(const lttng_ust_ctl lst::type::cuptr create_string_type_from_ust_ctl_fields(const lttng_ust_ctl_field *current, const lttng_ust_ctl_field *end, const session_attributes& session_attributes __attribute__((unused)), - const lttng_ust_ctl_field **next_ust_ctl_field) + const lttng_ust_ctl_field **next_ust_ctl_field, + lsu::ctl_field_quirks quirks __attribute__((unused))) { if (current >= end) { LTTNG_THROW_PROTOCOL_ERROR( @@ -269,7 +275,8 @@ lst::type::cuptr create_integer_type_from_ust_ctl_basic_type( lst::type::cuptr create_array_type_from_ust_ctl_fields(const lttng_ust_ctl_field *current, const lttng_ust_ctl_field *end, const session_attributes& session_attributes, - const lttng_ust_ctl_field **next_ust_ctl_field) + const lttng_ust_ctl_field **next_ust_ctl_field, + lsu::ctl_field_quirks quirks __attribute__((unused))) { if (current >= end) { LTTNG_THROW_PROTOCOL_ERROR( @@ -329,7 +336,8 @@ lst::type::cuptr create_array_nestable_type_from_ust_ctl_fields(const lttng_ust_ publish_field_fn publish_field, lookup_field_fn lookup_field, lst::field_location::root lookup_root, - lst::field_location::elements ¤t_field_location_elements) + lst::field_location::elements ¤t_field_location_elements, + lsu::ctl_field_quirks quirks) { if (current >= end) { LTTNG_THROW_PROTOCOL_ERROR( @@ -351,7 +359,7 @@ lst::type::cuptr create_array_nestable_type_from_ust_ctl_fields(const lttng_ust_ /* next_ust_ctl_field is updated as needed. */ element_type = create_type_from_ust_ctl_fields(&element_uctl_field, end, session_attributes, next_ust_ctl_field, publish_field, lookup_field, lookup_root, - current_field_location_elements); + current_field_location_elements, quirks); if (element_uctl_field.type.atype == lttng_ust_ctl_atype_integer && element_uctl_field.type.u.integer.encoding != lttng_ust_ctl_encode_none) { /* Element represents a text character. */ @@ -388,7 +396,8 @@ lst::type::cuptr create_sequence_type_from_ust_ctl_fields(const lttng_ust_ctl_fi const lttng_ust_ctl_field **next_ust_ctl_field, publish_field_fn publish_field, lst::field_location::root lookup_root, - lst::field_location::elements ¤t_field_location_elements) + lst::field_location::elements ¤t_field_location_elements, + lsu::ctl_field_quirks quirks __attribute__((unused))) { if (current >= end) { LTTNG_THROW_PROTOCOL_ERROR( @@ -466,7 +475,8 @@ lst::type::cuptr create_sequence_nestable_type_from_ust_ctl_fields( publish_field_fn publish_field, lookup_field_fn lookup_field, lst::field_location::root lookup_root, - lst::field_location::elements ¤t_field_location_elements) + lst::field_location::elements ¤t_field_location_elements, + lsu::ctl_field_quirks quirks) { if (current >= end) { LTTNG_THROW_PROTOCOL_ERROR( @@ -492,7 +502,7 @@ lst::type::cuptr create_sequence_nestable_type_from_ust_ctl_fields( /* next_ust_ctl_field is updated as needed. */ auto element_type = create_type_from_ust_ctl_fields(&element_uctl_field, end, session_attributes, next_ust_ctl_field, publish_field, lookup_field, - lookup_root, current_field_location_elements); + lookup_root, current_field_location_elements, quirks); if (lttng_strnlen(sequence_uctl_field.type.u.sequence_nestable.length_name, sizeof(sequence_uctl_field.type.u.sequence_nestable.length_name)) == @@ -537,7 +547,8 @@ lst::type::cuptr create_sequence_nestable_type_from_ust_ctl_fields( lst::type::cuptr create_structure_field_from_ust_ctl_fields(const lttng_ust_ctl_field *current, const lttng_ust_ctl_field *end, const session_attributes& session_attributes __attribute__((unused)), - const lttng_ust_ctl_field **next_ust_ctl_field) + const lttng_ust_ctl_field **next_ust_ctl_field, + lsu::ctl_field_quirks quirks __attribute__((unused))) { if (current >= end) { LTTNG_THROW_PROTOCOL_ERROR( @@ -577,7 +588,8 @@ typename lst::variant_type::choices create_ty lst::field_location::root lookup_root, lst::field_location::elements& current_field_location_elements, unsigned int choice_count, - const lst::field& selector_field) + const lst::field& selector_field, + lsu::ctl_field_quirks quirks) { typename lst::variant_type::choices choices; const auto& typed_enumeration = static_cast< @@ -587,7 +599,7 @@ typename lst::variant_type::choices create_ty for (unsigned int i = 0; i < choice_count; i++) { create_field_from_ust_ctl_fields( current, end, session_attributes, next_ust_ctl_field, - [&choices, typed_enumeration, &selector_field]( + [&choices, typed_enumeration, &selector_field, quirks]( lst::field::uptr field) { /* * Find the enumeration mapping that matches the @@ -596,9 +608,21 @@ typename lst::variant_type::choices create_ty const auto mapping_it = std::find_if( typed_enumeration.mappings_->begin(), typed_enumeration.mappings_->end(), - [&field](const typename std::remove_reference< + [&field, quirks](const typename std::remove_reference< decltype(typed_enumeration)>::type:: mapping& mapping) { + if (static_cast(quirks & lsu::ctl_field_quirks::UNDERSCORE_PREFIXED_VARIANT_TAG_MAPPINGS)) { + /* + * Check if they match with + * a prepended underscore + * and, if not, perform the + * regular check. + */ + if ((std::string("_") + field->name) == mapping.name) { + return true; + } + } + return mapping.name == field->name; }); @@ -610,7 +634,7 @@ typename lst::variant_type::choices create_ty choices.emplace_back(*mapping_it, field->move_type()); }, - lookup_field, lookup_root, current_field_location_elements); + lookup_field, lookup_root, current_field_location_elements, quirks); current = *next_ust_ctl_field; } @@ -624,7 +648,8 @@ lst::type::cuptr create_variant_field_from_ust_ctl_fields(const lttng_ust_ctl_fi const lttng_ust_ctl_field **next_ust_ctl_field, lookup_field_fn lookup_field, lst::field_location::root lookup_root, - lst::field_location::elements ¤t_field_location_elements) + lst::field_location::elements ¤t_field_location_elements, + lsu::ctl_field_quirks quirks) { if (current >= end) { LTTNG_THROW_PROTOCOL_ERROR( @@ -674,7 +699,7 @@ lst::type::cuptr create_variant_field_from_ust_ctl_fields(const lttng_ust_ctl_fi end, session_attributes, next_ust_ctl_field, lookup_field, lookup_root, current_field_location_elements, choice_count, - selector_field); + selector_field, quirks); return lttng::make_unique>( alignment, std::move(selector_field_location), std::move(choices)); @@ -684,7 +709,7 @@ lst::type::cuptr create_variant_field_from_ust_ctl_fields(const lttng_ust_ctl_fi end, session_attributes, next_ust_ctl_field, lookup_field, lookup_root, current_field_location_elements, choice_count, - selector_field); + selector_field, quirks); return lttng::make_unique>( alignment, std::move(selector_field_location), std::move(choices)); @@ -698,46 +723,47 @@ lst::type::cuptr create_type_from_ust_ctl_fields(const lttng_ust_ctl_field *curr publish_field_fn publish_field, lookup_field_fn lookup_field, lst::field_location::root lookup_root, - lst::field_location::elements ¤t_field_location_elements) + lst::field_location::elements& current_field_location_elements, + lsu::ctl_field_quirks quirks) { switch (current->type.atype) { case lttng_ust_ctl_atype_integer: return create_integer_type_from_ust_ctl_fields( - current, end, session_attributes, next_ust_ctl_field); + current, end, session_attributes, next_ust_ctl_field, quirks); case lttng_ust_ctl_atype_enum: case lttng_ust_ctl_atype_enum_nestable: return create_enumeration_type_from_ust_ctl_fields( - current, end, session_attributes, next_ust_ctl_field); + current, end, session_attributes, next_ust_ctl_field, quirks); case lttng_ust_ctl_atype_float: return create_floating_point_type_from_ust_ctl_fields( - current, end, session_attributes, next_ust_ctl_field); + current, end, session_attributes, next_ust_ctl_field, quirks); case lttng_ust_ctl_atype_string: return create_string_type_from_ust_ctl_fields( - current, end, session_attributes, next_ust_ctl_field); + current, end, session_attributes, next_ust_ctl_field, quirks); case lttng_ust_ctl_atype_array: - return create_array_type_from_ust_ctl_fields(current, end, session_attributes, - next_ust_ctl_field); + return create_array_type_from_ust_ctl_fields( + current, end, session_attributes, next_ust_ctl_field, quirks); case lttng_ust_ctl_atype_array_nestable: return create_array_nestable_type_from_ust_ctl_fields(current, end, session_attributes, next_ust_ctl_field, publish_field, lookup_field, - lookup_root, current_field_location_elements); + lookup_root, current_field_location_elements, quirks); case lttng_ust_ctl_atype_sequence: return create_sequence_type_from_ust_ctl_fields(current, end, session_attributes, next_ust_ctl_field, publish_field, lookup_root, - current_field_location_elements); + current_field_location_elements, quirks); case lttng_ust_ctl_atype_sequence_nestable: return create_sequence_nestable_type_from_ust_ctl_fields(current, end, session_attributes, next_ust_ctl_field, publish_field, lookup_field, - lookup_root, current_field_location_elements); + lookup_root, current_field_location_elements, quirks); case lttng_ust_ctl_atype_struct: case lttng_ust_ctl_atype_struct_nestable: return create_structure_field_from_ust_ctl_fields( - current, end, session_attributes, next_ust_ctl_field); + current, end, session_attributes, next_ust_ctl_field, quirks); case lttng_ust_ctl_atype_variant: case lttng_ust_ctl_atype_variant_nestable: return create_variant_field_from_ust_ctl_fields(current, end, session_attributes, next_ust_ctl_field, lookup_field, lookup_root, - current_field_location_elements); + current_field_location_elements, quirks); default: LTTNG_THROW_PROTOCOL_ERROR(fmt::format( "Unknown {} value `{}` encountered while converting {} to {}", @@ -753,7 +779,8 @@ void create_field_from_ust_ctl_fields(const lttng_ust_ctl_field *current, publish_field_fn publish_field, lookup_field_fn lookup_field, lst::field_location::root lookup_root, - lst::field_location::elements& current_field_location_elements) + lst::field_location::elements& current_field_location_elements, + lsu::ctl_field_quirks quirks) { LTTNG_ASSERT(current < end); @@ -765,7 +792,7 @@ void create_field_from_ust_ctl_fields(const lttng_ust_ctl_field *current, publish_field(lttng::make_unique(current->name, create_type_from_ust_ctl_fields(current, end, session_attributes, next_ust_ctl_field, publish_field, lookup_field, - lookup_root, current_field_location_elements))); + lookup_root, current_field_location_elements, quirks))); } std::vector::iterator lookup_field_in_vector( @@ -809,7 +836,8 @@ std::vector create_fields_from_ust_ctl_fields( const lsu::registry_session& session, const lttng_ust_ctl_field *current, const lttng_ust_ctl_field *end, - lst::field_location::root lookup_root) + lst::field_location::root lookup_root, + lsu::ctl_field_quirks quirks) { std::vector fields; const auto trace_native_byte_order = session.abi.byte_order; @@ -836,7 +864,8 @@ std::vector create_fields_from_ust_ctl_fields( create_field_from_ust_ctl_fields( current, end, session_attributes, &next_field, [&fields](lst::field::cuptr field) { - /* Publishing a field simply adds it to the converted fields. */ + /* Publishing a field simply adds it to the converted + * fields. */ fields.emplace_back(std::move(field)); }, [&fields](const lst::field_location& location) @@ -844,7 +873,7 @@ std::vector create_fields_from_ust_ctl_fields( /* Resolve location to a previously-constructed field. */ return **lookup_field_in_vector(fields, location); }, - lookup_root, current_field_location_elements); + lookup_root, current_field_location_elements, quirks); current = next_field; } @@ -857,7 +886,9 @@ std::vector lsu::create_trace_fields_from_ust_ctl_fields( const lsu::registry_session& session, const lttng_ust_ctl_field *fields, std::size_t field_count, - lst::field_location::root lookup_root) + lst::field_location::root lookup_root, + lsu::ctl_field_quirks quirks) { - return create_fields_from_ust_ctl_fields(session, fields, fields + field_count, lookup_root); + return create_fields_from_ust_ctl_fields( + session, fields, fields + field_count, lookup_root, quirks); } diff --git a/src/bin/lttng-sessiond/ust-field-convert.hpp b/src/bin/lttng-sessiond/ust-field-convert.hpp index e165cdf1d..c0e663983 100644 --- a/src/bin/lttng-sessiond/ust-field-convert.hpp +++ b/src/bin/lttng-sessiond/ust-field-convert.hpp @@ -14,16 +14,40 @@ #include #include +#include namespace lttng { namespace sessiond { namespace ust { +enum class ctl_field_quirks : unsigned int { + NONE = 0, + /* + * LTTng-UST with ABI major version <= 9 express variants with a tag + * enumeration that doesn't match the fields of the variant. The + * tag's mapping names are systematically prefixed with an underscore. + */ + UNDERSCORE_PREFIXED_VARIANT_TAG_MAPPINGS = 1 << 0, +}; + +inline ctl_field_quirks operator&(ctl_field_quirks lhs, ctl_field_quirks rhs) +{ + using enum_type = std::underlying_type::type; + return ctl_field_quirks(static_cast(lhs) & static_cast(rhs)); +} + +inline ctl_field_quirks operator|(ctl_field_quirks lhs, ctl_field_quirks rhs) +{ + using enum_type = std::underlying_type::type; + return ctl_field_quirks(static_cast(lhs) | static_cast(rhs)); +} + std::vector create_trace_fields_from_ust_ctl_fields( const lttng::sessiond::ust::registry_session& session, const lttng_ust_ctl_field *fields, std::size_t field_count, - trace::field_location::root lookup_root); + trace::field_location::root lookup_root, + ctl_field_quirks quirks = ctl_field_quirks::NONE); } /* namespace ust */ } /* namespace sessiond */ -- 2.34.1