[lttng-dev] [PATCH lttng-tools 1/5] Fix: probes should not be compared by their names and callsite signatures
Francis Deslauriers
francis.deslauriers at efficios.com
Wed Feb 7 14:36:55 EST 2018
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 <francis.deslauriers at efficios.com>
---
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 0000000..3c2da14
--- /dev/null
+++ b/src/bin/lttng-sessiond/ust-field-utils.c
@@ -0,0 +1,261 @@
+/*
+ * Copyright (C) 2018 - Francis Deslauriers <francis.deslauriers at efficios.com>
+ *
+ * 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 <string.h>
+#include <stdbool.h>
+
+#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;
+ }
+
+ return true;
+
+no_match:
+ return false;
+}
+
+/*
+ * Compare two _ustctl_basic_type fields known to be of type string.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_string_from_raw_basic_type(
+ union _ustctl_basic_type *first, union _ustctl_basic_type *second)
+{
+ return first->string.encoding == second->string.encoding;
+}
+
+/*
+ * Compare two _ustctl_basic_type fields known to be of type float.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_float_from_raw_basic_type(
+ union _ustctl_basic_type *first, union _ustctl_basic_type *second)
+{
+ bool match = true;
+ match &= first->_float.exp_dig == second->_float.exp_dig;
+ match &= first->_float.mant_dig == second->_float.mant_dig;
+ match &= first->_float.reverse_byte_order == second->_float.reverse_byte_order;
+ match &= first->_float.alignment == second->_float.alignment;
+
+ return match;
+}
+
+/*
+ * Compare two _ustctl_basic_type fields given their respective abstract types.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_raw_basic_type(
+ enum ustctl_abstract_types first_atype,
+ union _ustctl_basic_type *first,
+ enum ustctl_abstract_types second_atype,
+ union _ustctl_basic_type *second)
+{
+ if (first_atype != second_atype) {
+ goto no_match;
+ }
+
+ switch (first_atype) {
+ case ustctl_atype_integer:
+ if (match_ustctl_field_integer_from_raw_basic_type(first, second) == false) {
+ goto no_match;
+ }
+ break;
+ case ustctl_atype_enum:
+ if (match_ustctl_field_enum_from_raw_basic_type(first, second) == false) {
+ goto no_match;
+ }
+ break;
+ case ustctl_atype_string:
+ if (match_ustctl_field_string_from_raw_basic_type(first, second) == false) {
+ goto no_match;
+ }
+ break;
+ case ustctl_atype_float:
+ if (match_ustctl_field_float_from_raw_basic_type(first, second) == false) {
+ goto no_match;
+ }
+ break;
+ default:
+ goto no_match;
+ }
+
+ return true;
+
+no_match:
+ return false;
+}
+
+/*
+ * Compatibility layer between the ustctl_basic_type struct and
+ * _ustctl_basic_type union.
+ */
+static bool match_ustctl_field_basic_type(struct ustctl_basic_type *first,
+ struct ustctl_basic_type *second)
+{
+ return match_ustctl_field_raw_basic_type(first->atype, &first->u.basic,
+ second->atype, &second->u.basic);
+}
+
+int match_ustctl_field(struct ustctl_field *first, struct ustctl_field *second)
+{
+ /* Check the name of the field is identical. */
+ if (strncmp(first->name, second->name, LTTNG_UST_SYM_NAME_LEN)) {
+ goto no_match;
+ }
+
+ /* Check the field type is identical. */
+ if (first->type.atype != second->type.atype) {
+ goto no_match;
+ }
+
+ /* Check the field layout. */
+ switch (first->type.atype) {
+ case ustctl_atype_integer:
+ case ustctl_atype_enum:
+ case ustctl_atype_string:
+ case ustctl_atype_float:
+ if (match_ustctl_field_raw_basic_type(first->type.atype,
+ &first->type.u.basic, second->type.atype,
+ &second->type.u.basic) == false) {
+ goto no_match;
+ }
+ break;
+ case ustctl_atype_sequence:
+ /* Match element type of the sequence. */
+ if (match_ustctl_field_basic_type(&first->type.u.sequence.elem_type,
+ &second->type.u.sequence.elem_type) == false) {
+ goto no_match;
+ }
+
+ /* Match length type of the sequence. */
+ if (match_ustctl_field_basic_type(&first->type.u.sequence.length_type,
+ &second->type.u.sequence.length_type) == false) {
+ goto no_match;
+ }
+
+ break;
+ case ustctl_atype_array:
+ /* Match element type of the array. */
+ if (match_ustctl_field_basic_type(&first->type.u.array.elem_type,
+ &second->type.u.array.elem_type) == false) {
+ goto no_match;
+ }
+
+ /* Match length of the array. */
+ if (first->type.u.array.length != second->type.u.array.length) {
+ goto no_match;
+ }
+
+ break;
+ case ustctl_atype_variant:
+ /* Compare number of choice of the variants. */
+ if (first->type.u.variant.nr_choices !=
+ second->type.u.variant.nr_choices) {
+ goto no_match;
+ }
+
+ /* Compare tag name of the variants. */
+ if (strncmp(first->type.u.variant.tag_name,
+ second->type.u.variant.tag_name,
+ LTTNG_UST_SYM_NAME_LEN)) {
+ goto no_match;
+ }
+
+ break;
+ case ustctl_atype_struct:
+ /* Compare number of fields of the structs. */
+ if (first->type.u._struct.nr_fields != second->type.u._struct.nr_fields) {
+ goto no_match;
+ }
+
+ break;
+ default:
+ goto no_match;
+ }
+
+ return true;
+
+no_match:
+ return false;
+}
diff --git a/src/bin/lttng-sessiond/ust-field-utils.h b/src/bin/lttng-sessiond/ust-field-utils.h
new file mode 100644
index 0000000..13a1433
--- /dev/null
+++ b/src/bin/lttng-sessiond/ust-field-utils.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2018 - Francis Deslauriers francis.deslauriers at efficios.com>
+ *
+ * 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.
+ */
+
+#ifndef LTTNG_UST_FIELD_UTILS_H
+#define LTTNG_UST_FIELD_UTILS_H
+
+#include "ust-ctl.h"
+
+/*
+ * Compare two UST fields.
+ * Return 1 if both fields have identical definition, 0 otherwise.
+ */
+int match_ustctl_field(struct ustctl_field *first, struct ustctl_field *second);
+
+#endif /* LTTNG_UST_FIELD_UTILS_H */
diff --git a/src/bin/lttng-sessiond/ust-registry.c b/src/bin/lttng-sessiond/ust-registry.c
index d63c750..e336b9e 100644
--- a/src/bin/lttng-sessiond/ust-registry.c
+++ b/src/bin/lttng-sessiond/ust-registry.c
@@ -25,15 +25,18 @@
#include "ust-registry.h"
#include "ust-app.h"
+#include "ust-field-utils.h"
#include "utils.h"
#include "lttng-sessiond.h"
#include "notification-thread-commands.h"
+
/*
* Hash table match function for event in the registry.
*/
static int ht_match_event(struct cds_lfht_node *node, const void *_key)
{
+ int i;
struct ust_registry_event *event;
const struct ust_registry_event *key;
@@ -44,15 +47,37 @@ static int ht_match_event(struct cds_lfht_node *node, const void *_key)
assert(event);
key = _key;
- /* It has to be a perfect match. */
+ /* It has to be a perfect match. First, compare the event names. */
if (strncmp(event->name, key->name, sizeof(event->name))) {
goto no_match;
}
- /* It has to be a perfect match. */
- if (strncmp(event->signature, key->signature,
- strlen(event->signature))) {
+ /* Compare log levels. */
+ if (event->loglevel_value != key->loglevel_value) {
+ goto no_match;
+ }
+
+ /* Compare the number of fields. */
+ if (event->nr_fields != key->nr_fields) {
+ goto no_match;
+ }
+
+ /* Compare each field individually. */
+ for (i = 0; i < event->nr_fields; i++) {
+ if (match_ustctl_field(&event->fields[i], &key->fields[i]) == 0) {
+ goto no_match;
+ }
+ }
+
+ /* Compare model URI. */
+ if (event->model_emf_uri != NULL && key->model_emf_uri == NULL) {
+ goto no_match;
+ } else if(event->model_emf_uri == NULL && key->model_emf_uri != NULL) {
goto no_match;
+ } else if (event->model_emf_uri != NULL && key->model_emf_uri != NULL) {
+ if (strcmp(event->model_emf_uri, key->model_emf_uri)) {
+ goto no_match;
+ }
}
/* Match */
@@ -64,15 +89,14 @@ no_match:
static unsigned long ht_hash_event(void *_key, unsigned long seed)
{
- uint64_t xored_key;
+ uint64_t hashed_key;
struct ust_registry_event *key = _key;
assert(key);
- xored_key = (uint64_t) (hash_key_str(key->name, seed) ^
- hash_key_str(key->signature, seed));
+ hashed_key = (uint64_t) hash_key_str(key->name, seed);
- return hash_key_u64(&xored_key, seed);
+ return hash_key_u64(&hashed_key, seed);
}
static int compare_enums(const struct ust_registry_enum *reg_enum_a,
diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
index 340dd93..2fb581b 100644
--- a/tests/unit/Makefile.am
+++ b/tests/unit/Makefile.am
@@ -63,6 +63,7 @@ UST_DATA_TRACE=$(top_builddir)/src/bin/lttng-sessiond/trace-ust.$(OBJEXT) \
$(top_builddir)/src/bin/lttng-sessiond/utils.$(OBJEXT) \
$(top_builddir)/src/bin/lttng-sessiond/buffer-registry.$(OBJEXT) \
$(top_builddir)/src/bin/lttng-sessiond/ust-registry.$(OBJEXT) \
+ $(top_builddir)/src/bin/lttng-sessiond/ust-field-utils.$(OBJEXT) \
$(top_builddir)/src/bin/lttng-sessiond/ust-metadata.$(OBJEXT) \
$(top_builddir)/src/bin/lttng-sessiond/ust-app.$(OBJEXT) \
$(top_builddir)/src/bin/lttng-sessiond/ust-consumer.$(OBJEXT) \
--
2.7.4
More information about the lttng-dev
mailing list