[lttng-dev] [PATCH lttng-tools 1/5] Fix: probes should not be compared by their names and callsite signatures
Events with different payloads but identical name and signatures could
lead to corrupted trace as the Session Daemon would consider them
identical and give them the same event ID.
Events should be compared using the name, loglevel, fields and
model_emf_uri to ensure that their serialized layout is the same.
Signed-off-by: Francis Deslauriers
---
src/bin/lttng-sessiond/Makefile.am | 3 +-
src/bin/lttng-sessiond/ust-field-utils.c | 261 +++
src/bin/lttng-sessiond/ust-field-utils.h | 29
src/bin/lttng-sessiond/ust-registry.c| 40 -
tests/unit/Makefile.am | 1 +
5 files changed, 325 insertions(+), 9 deletions(-)
create mode 100644 src/bin/lttng-sessiond/ust-field-utils.c
create mode 100644 src/bin/lttng-sessiond/ust-field-utils.h
diff --git a/src/bin/lttng-sessiond/Makefile.am
b/src/bin/lttng-sessiond/Makefile.am
index 413fe75..6fc1809 100644
--- a/src/bin/lttng-sessiond/Makefile.am
+++ b/src/bin/lttng-sessiond/Makefile.am
@@ -40,7 +40,8 @@ lttng_sessiond_SOURCES = utils.c utils.h \
if HAVE_LIBLTTNG_UST_CTL
lttng_sessiond_SOURCES += trace-ust.c ust-registry.c ust-app.c \
ust-consumer.c ust-consumer.h ust-thread.c \
- ust-metadata.c ust-clock.h agent-thread.c agent-thread.h
+ ust-metadata.c ust-clock.h agent-thread.c
agent-thread.h \
+ ust-field-utils.h ust-field-utils.c
endif
# Add main.c at the end for compile order
diff --git a/src/bin/lttng-sessiond/ust-field-utils.c
b/src/bin/lttng-sessiond/ust-field-utils.c
new file mode 100644
index 000..3c2da14
--- /dev/null
+++ b/src/bin/lttng-sessiond/ust-field-utils.c
@@ -0,0 +1,261 @@
+/*
+ * Copyright (C) 2018 - Francis Deslauriers
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License, version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 51
+ * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include
+#include
+
+#include "ust-field-utils.h"
+
+/*
+ * The ustctl_field is made of a combinaison of C basic types
+ * ustctl_basic_type and _ustctl_basic_type.
+ *
+ * ustctl_basic_type contains an enumeration describing the abstract type.
+ * _ustctl_basic_type does _NOT_ contain an enumeration describing the
+ * abstract type.
+ *
+ * A layer is needed to use the same code for both structures.
+ * When dealing with _ustctl_basic_type, we need to use the abstract type of
+ * the ustctl_type struct.
+ */
+
+/*
+ * Compare two ustctl_integer_type fields.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_integer(struct ustctl_integer_type *first,
+ struct ustctl_integer_type *second)
+{
+ bool match = true;
+ match &= first->size == second->size;
+ match &= first->alignment == second->alignment;
+ match &= first->signedness == second->signedness;
+ match &= first->encoding == second->encoding;
+ match &= first->base == second->base;
+ match &= first->reverse_byte_order == second->reverse_byte_order;
+
+ return match;
+}
+
+/*
+ * Compare two _ustctl_basic_type fields known to be of type integer.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_integer_from_raw_basic_type(
+ union _ustctl_basic_type *first, union
_ustctl_basic_type *second)
+{
+ return match_ustctl_field_integer(&first->integer, &second->integer);
+}
+
+/*
+ * Compare two _ustctl_basic_type fields known to be of type enum.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_enum_from_raw_basic_type(
+ union _ustctl_basic_type *first, union
_ustctl_basic_type *second)
+{
+ /*
+* Compare enumeration ID. Enumeration ID is provided to the
application by
+* the session daemon before event registration.
+*/
+ if (first->enumeration.id != second->enumeration.id) {
+ goto no_match;
+ }
+
+ /*
+* Sanity check of the name and container type. Those were already
checked
+* during enum registration.
+*/
+ if (strncmp(first->enumeration.name, second->enumeration.name,
+ LTTNG_UST_SYM_NAME_LEN)) {
+ goto no_match;
+ }
+ if (match_ustctl_field_integer(&first->enumeration.container_type,
+ &second->enumeration.container_type) == false) {
+ goto no_match;
+
Re: [lttng-dev] [PATCH lttng-tools 1/5] Fix: probes should not be compared by their names and callsite signatures
- On Feb 7, 2018, at 2:36 PM, Francis Deslauriers
francis.deslauri...@efficios.com wrote:
> Events with different payloads but identical name and signatures could
> lead to corrupted trace as the Session Daemon would consider them
> identical and give them the same event ID.
>
> Events should be compared using the name, loglevel, fields and
> model_emf_uri to ensure that their serialized layout is the same.
model_emf_uri has no impact on the serialized layout, but does have an
impact on what ends up in the metadata description for the event. The
changelog should be adapated to clarify this.
>
> Signed-off-by: Francis Deslauriers
> ---
> src/bin/lttng-sessiond/Makefile.am | 3 +-
> src/bin/lttng-sessiond/ust-field-utils.c | 261 +++
> src/bin/lttng-sessiond/ust-field-utils.h | 29
> src/bin/lttng-sessiond/ust-registry.c| 40 -
> tests/unit/Makefile.am | 1 +
> 5 files changed, 325 insertions(+), 9 deletions(-)
> create mode 100644 src/bin/lttng-sessiond/ust-field-utils.c
> create mode 100644 src/bin/lttng-sessiond/ust-field-utils.h
>
> diff --git a/src/bin/lttng-sessiond/Makefile.am
> b/src/bin/lttng-sessiond/Makefile.am
> index 413fe75..6fc1809 100644
> --- a/src/bin/lttng-sessiond/Makefile.am
> +++ b/src/bin/lttng-sessiond/Makefile.am
> @@ -40,7 +40,8 @@ lttng_sessiond_SOURCES = utils.c utils.h \
> if HAVE_LIBLTTNG_UST_CTL
> lttng_sessiond_SOURCES += trace-ust.c ust-registry.c ust-app.c \
> ust-consumer.c ust-consumer.h ust-thread.c \
> - ust-metadata.c ust-clock.h agent-thread.c agent-thread.h
> + ust-metadata.c ust-clock.h agent-thread.c
> agent-thread.h \
> + ust-field-utils.h ust-field-utils.c
> endif
>
> # Add main.c at the end for compile order
> diff --git a/src/bin/lttng-sessiond/ust-field-utils.c
> b/src/bin/lttng-sessiond/ust-field-utils.c
> new file mode 100644
> index 000..3c2da14
> --- /dev/null
> +++ b/src/bin/lttng-sessiond/ust-field-utils.c
> @@ -0,0 +1,261 @@
> +/*
> + * Copyright (C) 2018 - Francis Deslauriers
>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License, version 2 only, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program; if not, write to the Free Software Foundation, Inc., 51
> + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include
> +#include
> +
> +#include "ust-field-utils.h"
> +
> +/*
> + * The ustctl_field is made of a combinaison of C basic types
combination
> + * ustctl_basic_type and _ustctl_basic_type.
> + *
> + * ustctl_basic_type contains an enumeration describing the abstract type.
> + * _ustctl_basic_type does _NOT_ contain an enumeration describing the
> + * abstract type.
> + *
> + * A layer is needed to use the same code for both structures.
> + * When dealing with _ustctl_basic_type, we need to use the abstract type of
> + * the ustctl_type struct.
> + */
> +
> +/*
> + * Compare two ustctl_integer_type fields.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_integer(struct ustctl_integer_type *first,
> + struct ustctl_integer_type *second)
> +{
> + bool match = true;
I don't like bitwise operators on C bool type. Is there anything that
guarantees us that bool always uses the same bit to represent "true" ?
Also, missing whiteline after the variable declaration.
> + match &= first->size == second->size;
> + match &= first->alignment == second->alignment;
> + match &= first->signedness == second->signedness;
> + match &= first->encoding == second->encoding;
> + match &= first->base == second->base;
> + match &= first->reverse_byte_order == second->reverse_byte_order;
> +
> + return match;
I'd prefer simply:
if (first->size != second->size)
return false;
.
return true;
> +}
> +
> +/*
> + * Compare two _ustctl_basic_type fields known to be of type integer.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_integer_from_raw_basic_type(
> + union _ustctl_basic_type *first, union
> _ustctl_basic_type *second)
> +{
> + return match_ustctl_field_integer(&first->integer, &second->integer);
> +}
> +
> +/*
> + * Compare two _ustctl_basic_type fields known to be of type enum.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_enum_from_raw_basic_type(
> + union _ustctl_basic_type *first, union
> _ustctl_basic_type *second)
Re: [lttng-dev] [PATCH lttng-tools 1/5] Fix: probes should not be compared by their names and callsite signatures
2018-02-07 15:43 GMT-05:00 Mathieu Desnoyers :
> - On Feb 7, 2018, at 2:36 PM, Francis Deslauriers
> francis.deslauri...@efficios.com wrote:
>
>> Events with different payloads but identical name and signatures could
>> lead to corrupted trace as the Session Daemon would consider them
>> identical and give them the same event ID.
>>
>> Events should be compared using the name, loglevel, fields and
>> model_emf_uri to ensure that their serialized layout is the same.
>
> model_emf_uri has no impact on the serialized layout, but does have an
> impact on what ends up in the metadata description for the event. The
> changelog should be adapated to clarify this.
I understand, how about this:
Fix: probes should be compared strictly by events metadata
Currently, events are compared using names and signatures. Events
with different payloads but identical name and signatures could
lead to corrupted trace as the Session Daemon would consider them
identical and give them the same event ID.
Events should be compared using the name, loglevel, fields and
model_emf_uri to ensure that their respective metadata is the same.
>
>>
>> Signed-off-by: Francis Deslauriers
>> ---
>> src/bin/lttng-sessiond/Makefile.am | 3 +-
>> src/bin/lttng-sessiond/ust-field-utils.c | 261
>> +++
>> src/bin/lttng-sessiond/ust-field-utils.h | 29
>> src/bin/lttng-sessiond/ust-registry.c| 40 -
>> tests/unit/Makefile.am | 1 +
>> 5 files changed, 325 insertions(+), 9 deletions(-)
>> create mode 100644 src/bin/lttng-sessiond/ust-field-utils.c
>> create mode 100644 src/bin/lttng-sessiond/ust-field-utils.h
>>
>> diff --git a/src/bin/lttng-sessiond/Makefile.am
>> b/src/bin/lttng-sessiond/Makefile.am
>> index 413fe75..6fc1809 100644
>> --- a/src/bin/lttng-sessiond/Makefile.am
>> +++ b/src/bin/lttng-sessiond/Makefile.am
>> @@ -40,7 +40,8 @@ lttng_sessiond_SOURCES = utils.c utils.h \
>> if HAVE_LIBLTTNG_UST_CTL
>> lttng_sessiond_SOURCES += trace-ust.c ust-registry.c ust-app.c \
>> ust-consumer.c ust-consumer.h ust-thread.c \
>> - ust-metadata.c ust-clock.h agent-thread.c
>> agent-thread.h
>> + ust-metadata.c ust-clock.h agent-thread.c
>> agent-thread.h \
>> + ust-field-utils.h ust-field-utils.c
>> endif
>>
>> # Add main.c at the end for compile order
>> diff --git a/src/bin/lttng-sessiond/ust-field-utils.c
>> b/src/bin/lttng-sessiond/ust-field-utils.c
>> new file mode 100644
>> index 000..3c2da14
>> --- /dev/null
>> +++ b/src/bin/lttng-sessiond/ust-field-utils.c
>> @@ -0,0 +1,261 @@
>> +/*
>> + * Copyright (C) 2018 - Francis Deslauriers
>>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License, version 2 only, as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> with
>> + * this program; if not, write to the Free Software Foundation, Inc., 51
>> + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>> + */
>> +
>> +#include
>> +#include
>> +
>> +#include "ust-field-utils.h"
>> +
>> +/*
>> + * The ustctl_field is made of a combinaison of C basic types
>
> combination
Done.
>
>> + * ustctl_basic_type and _ustctl_basic_type.
>> + *
>> + * ustctl_basic_type contains an enumeration describing the abstract type.
>> + * _ustctl_basic_type does _NOT_ contain an enumeration describing the
>> + * abstract type.
>> + *
>> + * A layer is needed to use the same code for both structures.
>> + * When dealing with _ustctl_basic_type, we need to use the abstract type of
>> + * the ustctl_type struct.
>> + */
>> +
>> +/*
>> + * Compare two ustctl_integer_type fields.
>> + * Returns 1 if both are identical.
>> + */
>> +static bool match_ustctl_field_integer(struct ustctl_integer_type *first,
>> + struct ustctl_integer_type *second)
>> +{
>> + bool match = true;
>
> I don't like bitwise operators on C bool type. Is there anything that
> guarantees us that bool always uses the same bit to represent "true" ?
>
> Also, missing whiteline after the variable declaration.
Changed for simpler ifs.
>
>
>> + match &= first->size == second->size;
>> + match &= first->alignment == second->alignment;
>> + match &= first->signedness == second->signedness;
>> + match &= first->encoding == second->encoding;
>> + match &= first->base == second->base;
>> + match &= first->reverse_byte_order == second->reverse_byte_order;
>> +
>> + return match;
>
>
> I'd prefer simply:
>
> if (first->size != second->size)
>return false;
>
