From 5b4c6da4aa59719b613c612afc05aee91f4343a4 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Fri, 26 Mar 2021 11:13:25 -0400 Subject: [PATCH] tracepoint probe refactoring: Move provider name to provider descriptor The provider name is currently repeated for each event name as: : This repetition requires that each event embeds the provider name within its own name. Use this ABI break to clean this up: instead, each event descriptor has a pointer to its probe descriptor, and each probe descriptor has a provider name. Implement name size validation and event name formatting internal helpers, and use them across the tracer. Signed-off-by: Mathieu Desnoyers Change-Id: Ief8a4696a2030bf3b9c785b8422f382079c6bf06 --- include/lttng/ust-events.h | 5 +- include/lttng/ust-tracepoint-event.h | 21 ++++++-- liblttng-ust/lttng-events.c | 71 +++++++++++++++++------- liblttng-ust/lttng-probes.c | 80 ++++++++++++++-------------- liblttng-ust/ust-events-internal.h | 7 +++ 5 files changed, 121 insertions(+), 63 deletions(-) diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h index 96504940..1f55c00a 100644 --- a/include/lttng/ust-events.h +++ b/include/lttng/ust-events.h @@ -258,7 +258,8 @@ struct lttng_ust_event_field { struct lttng_ust_event_desc { uint32_t struct_size; /* Size of this structure. */ - const char *name; + const char *event_name; + struct lttng_ust_probe_desc *probe_desc; void (*probe_callback)(void); struct lttng_event_ctx *ctx; /* context */ struct lttng_ust_event_field **fields; /* event payload */ @@ -282,7 +283,7 @@ struct lttng_ust_event_desc { struct lttng_ust_probe_desc { uint32_t struct_size; /* Size of this structure. */ - const char *provider; + const char *provider_name; struct lttng_ust_event_desc **event_desc; unsigned int nr_events; struct cds_list_head head; /* chain registered probes */ diff --git a/include/lttng/ust-tracepoint-event.h b/include/lttng/ust-tracepoint-event.h index 18f91c5f..fcd53031 100644 --- a/include/lttng/ust-tracepoint-event.h +++ b/include/lttng/ust-tracepoint-event.h @@ -973,6 +973,19 @@ LTTNG_TP_EXTERN_C const char *_model_emf_uri___##__provider##___##__name \ #undef LTTNG_TP_EXTERN_C +/* + * Stage 7.0 of tracepoint event generation. + * + * Declare toplevel descriptor for the whole probe. + * Unlike C, C++ does not allow tentative definitions. Therefore, we + * need to explicitly declare the variable with "extern", using hidden + * visibility to keep this symbol from being exported to the global + * symbol table. + */ + +extern struct lttng_ust_probe_desc _TP_COMBINE_TOKENS(__probe_desc___, TRACEPOINT_PROVIDER) + __attribute__((visibility("hidden"))); + /* * Stage 7.1 of tracepoint event generation. * @@ -994,7 +1007,8 @@ static const char * \ __attribute__((weakref ("_model_emf_uri___" #_provider "___" #_name)));\ static struct lttng_ust_event_desc __event_desc___##_provider##_##_name = { \ .struct_size = sizeof(struct lttng_ust_event_desc), \ - .name = #_provider ":" #_name, \ + .event_name = #_name, \ + .probe_desc = &__probe_desc___##_provider, \ .probe_callback = (void (*)(void)) &__event_probe__##_provider##___##_template, \ .ctx = NULL, \ .fields = __event_fields___##_provider##___##_template, \ @@ -1031,10 +1045,9 @@ static struct lttng_ust_event_desc *_TP_COMBINE_TOKENS(__event_desc___, TRACEPOI * Create a toplevel descriptor for the whole probe. */ -/* non-const because list head will be modified when registered. */ -static struct lttng_ust_probe_desc _TP_COMBINE_TOKENS(__probe_desc___, TRACEPOINT_PROVIDER) = { +struct lttng_ust_probe_desc _TP_COMBINE_TOKENS(__probe_desc___, TRACEPOINT_PROVIDER) = { .struct_size = sizeof(struct lttng_ust_probe_desc), - .provider = __tp_stringify(TRACEPOINT_PROVIDER), + .provider_name = __tp_stringify(TRACEPOINT_PROVIDER), .event_desc = _TP_COMBINE_TOKENS(__event_desc___, TRACEPOINT_PROVIDER), .nr_events = _TP_ARRAY_SIZE(_TP_COMBINE_TOKENS(__event_desc___, TRACEPOINT_PROVIDER)) - 1, .head = { NULL, NULL }, diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c index c81581b2..140e5cae 100644 --- a/liblttng-ust/lttng-events.c +++ b/liblttng-ust/lttng-events.c @@ -84,6 +84,22 @@ void lttng_event_notifier_group_sync_enablers( static void lttng_enabler_destroy(struct lttng_enabler *enabler); +bool lttng_ust_validate_event_name(const struct lttng_ust_event_desc *desc) +{ + if (strlen(desc->probe_desc->provider_name) + 1 + + strlen(desc->event_name) >= LTTNG_UST_ABI_SYM_NAME_LEN) + return false; + return true; +} + +void lttng_ust_format_event_name(const struct lttng_ust_event_desc *desc, + char *name) +{ + strcpy(name, desc->probe_desc->provider_name); + strcat(name, ":"); + strcat(name, desc->event_name); +} + /* * Called with ust lock held. */ @@ -251,10 +267,12 @@ void register_event(struct lttng_ust_event_common *event) { int ret; struct lttng_ust_event_desc *desc; + char name[LTTNG_UST_ABI_SYM_NAME_LEN]; assert(event->priv->registered == 0); desc = event->priv->desc; - ret = lttng_ust_tp_probe_register_queue_release(desc->name, + lttng_ust_format_event_name(desc, name); + ret = lttng_ust_tp_probe_register_queue_release(name, desc->probe_callback, event, desc->signature); WARN_ON_ONCE(ret); @@ -267,10 +285,12 @@ void unregister_event(struct lttng_ust_event_common *event) { int ret; struct lttng_ust_event_desc *desc; + char name[LTTNG_UST_ABI_SYM_NAME_LEN]; assert(event->priv->registered == 1); desc = event->priv->desc; - ret = lttng_ust_tp_probe_unregister_queue_release(desc->name, + lttng_ust_format_event_name(desc, name); + ret = lttng_ust_tp_probe_unregister_queue_release(name, desc->probe_callback, event); WARN_ON_ONCE(ret); @@ -677,14 +697,14 @@ struct cds_hlist_head *borrow_hash_table_bucket( unsigned int hash_table_size, struct lttng_ust_event_desc *desc) { - const char *event_name; + char name[LTTNG_UST_ABI_SYM_NAME_LEN]; size_t name_len; uint32_t hash; - event_name = desc->name; - name_len = strlen(event_name); + lttng_ust_format_event_name(desc, name); + name_len = strlen(name); - hash = jhash(event_name, name_len, 0); + hash = jhash(name, name_len, 0); return &hash_table[hash & (hash_table_size - 1)]; } @@ -695,6 +715,7 @@ static int lttng_event_recorder_create(struct lttng_ust_event_desc *desc, struct lttng_ust_channel_buffer *chan) { + char name[LTTNG_UST_ABI_SYM_NAME_LEN]; struct lttng_ust_event_recorder *event_recorder; struct lttng_ust_event_recorder_private *event_recorder_priv; struct lttng_ust_session *session = chan->parent->session; @@ -767,12 +788,14 @@ int lttng_event_recorder_create(struct lttng_ust_event_desc *desc, else uri = NULL; + lttng_ust_format_event_name(desc, name); + /* Fetch event ID from sessiond */ ret = ustcomm_register_event(notify_socket, session, session->priv->objd, chan->priv->parent.objd, - desc->name, + name, loglevel, desc->signature, desc->nr_fields, @@ -877,12 +900,14 @@ static int lttng_desc_match_star_glob_enabler(struct lttng_ust_event_desc *desc, struct lttng_enabler *enabler) { + char name[LTTNG_UST_ABI_SYM_NAME_LEN]; int loglevel = 0; unsigned int has_loglevel = 0; + lttng_ust_format_event_name(desc, name); assert(enabler->format_type == LTTNG_ENABLER_FORMAT_STAR_GLOB); if (!strutils_star_glob_match(enabler->event_param.name, SIZE_MAX, - desc->name, SIZE_MAX)) + name, SIZE_MAX)) return 0; if (desc->loglevel) { loglevel = *(*desc->loglevel); @@ -900,11 +925,13 @@ static int lttng_desc_match_event_enabler(struct lttng_ust_event_desc *desc, struct lttng_enabler *enabler) { + char name[LTTNG_UST_ABI_SYM_NAME_LEN]; int loglevel = 0; unsigned int has_loglevel = 0; + lttng_ust_format_event_name(desc, name); assert(enabler->format_type == LTTNG_ENABLER_FORMAT_EVENT); - if (strcmp(desc->name, enabler->event_param.name)) + if (strcmp(name, enabler->event_param.name)) return 0; if (desc->loglevel) { loglevel = *(*desc->loglevel); @@ -945,8 +972,14 @@ int lttng_desc_match_enabler(struct lttng_ust_event_desc *desc, excluder_name = (char *) (excluder->excluder.names) + count * LTTNG_UST_ABI_SYM_NAME_LEN; len = strnlen(excluder_name, LTTNG_UST_ABI_SYM_NAME_LEN); - if (len > 0 && strutils_star_glob_match(excluder_name, len, desc->name, SIZE_MAX)) - return 0; + if (len > 0) { + char name[LTTNG_UST_ABI_SYM_NAME_LEN]; + + lttng_ust_format_event_name(desc, name); + if (strutils_star_glob_match(excluder_name, len, name, SIZE_MAX)) { + return 0; + } + } } } return 1; @@ -1052,8 +1085,9 @@ void lttng_create_event_recorder_if_missing(struct lttng_event_enabler *event_en ret = lttng_event_recorder_create(probe_desc->event_desc[i], event_enabler->chan); if (ret) { - DBG("Unable to create event %s, error %d\n", - probe_desc->event_desc[i]->name, ret); + DBG("Unable to create event \"%s:%s\", error %d\n", + probe_desc->provider_name, + probe_desc->event_desc[i]->event_name, ret); } } } @@ -1772,12 +1806,12 @@ void lttng_create_event_notifier_if_missing( /* Check that the probe supports event notifiers, else report the error. */ if (!lttng_ust_probe_supports_event_notifier(probe_desc)) { - ERR("Probe \"%s\" contains event \"%s\" which matches an enabled event notifier, " + ERR("Probe \"%s\" contains event \"%s:%s\" which matches an enabled event notifier, " "but its version (%u.%u) is too old and does not implement event notifiers. " "It needs to be recompiled against a newer version of LTTng-UST, otherwise " "this event will not generate any notification.", - probe_desc->provider, - desc->name, + probe_desc->provider_name, + probe_desc->provider_name, desc->event_name, probe_desc->major, probe_desc->minor); continue; @@ -1790,8 +1824,9 @@ void lttng_create_event_notifier_if_missing( event_notifier_enabler->error_counter_index, event_notifier_group); if (ret) { - DBG("Unable to create event_notifier %s, error %d\n", - probe_desc->event_desc[i]->name, ret); + DBG("Unable to create event_notifier \"%s:%s\", error %d\n", + probe_desc->provider_name, + probe_desc->event_desc[i]->event_name, ret); } } } diff --git a/liblttng-ust/lttng-probes.c b/liblttng-ust/lttng-probes.c index 00ce54cb..95a1c12b 100644 --- a/liblttng-ust/lttng-probes.c +++ b/liblttng-ust/lttng-probes.c @@ -41,23 +41,29 @@ static CDS_LIST_HEAD(lazy_probe_init); static int lazy_nesting; /* - * Called under ust lock. + * Validate that each event within the probe provider refers to the + * right probe, and that the resulting name is not too long. */ static -int check_event_provider(struct lttng_ust_probe_desc *desc) +bool check_event_provider(struct lttng_ust_probe_desc *probe_desc) { int i; - size_t provider_name_len; - - provider_name_len = strnlen(desc->provider, - LTTNG_UST_ABI_SYM_NAME_LEN - 1); - for (i = 0; i < desc->nr_events; i++) { - if (strncmp(desc->event_desc[i]->name, - desc->provider, - provider_name_len)) - return 0; /* provider mismatch */ + + for (i = 0; i < probe_desc->nr_events; i++) { + const struct lttng_ust_event_desc *event_desc = probe_desc->event_desc[i]; + + if (event_desc->probe_desc != probe_desc) { + ERR("Error registering probe provider '%s'. Event '%s:%s' refers to the wrong provider descriptor.", + probe_desc->provider_name, probe_desc->provider_name, event_desc->event_name); + return false; /* provider mismatch */ + } + if (!lttng_ust_validate_event_name(event_desc)) { + ERR("Error registering probe provider '%s'. Event '%s:%s' name is too long.", + probe_desc->provider_name, probe_desc->provider_name, event_desc->event_name); + return false; /* provider mismatch */ + } } - return 1; + return true; } /* @@ -69,14 +75,6 @@ void lttng_lazy_probe_register(struct lttng_ust_probe_desc *desc) struct lttng_ust_probe_desc *iter; struct cds_list_head *probe_list; - /* - * Each provider enforce that every event name begins with the - * provider name. Check this in an assertion for extra - * carefulness. This ensures we cannot have duplicate event - * names across providers. - */ - assert(check_event_provider(desc)); - /* * The provider ensures there are no duplicate event names. * Duplicated TRACEPOINT_EVENT event names would generate a @@ -100,7 +98,7 @@ void lttng_lazy_probe_register(struct lttng_ust_probe_desc *desc) cds_list_add(&desc->head, probe_list); desc_added: DBG("just registered probe %s containing %u events", - desc->provider, desc->nr_events); + desc->provider_name, desc->nr_events); } /* @@ -143,7 +141,7 @@ int check_provider_version(struct lttng_ust_probe_desc *desc) if (desc->major <= LTTNG_UST_PROVIDER_MAJOR) { DBG("Provider \"%s\" accepted, version %u.%u is compatible " "with LTTng UST provider version %u.%u.", - desc->provider, desc->major, desc->minor, + desc->provider_name, desc->major, desc->minor, LTTNG_UST_PROVIDER_MAJOR, LTTNG_UST_PROVIDER_MINOR); if (desc->major < LTTNG_UST_PROVIDER_MAJOR) { @@ -156,7 +154,7 @@ int check_provider_version(struct lttng_ust_probe_desc *desc) ERR("Provider \"%s\" rejected, version %u.%u is incompatible " "with LTTng UST provider version %u.%u. Please upgrade " "LTTng UST.", - desc->provider, desc->major, desc->minor, + desc->provider_name, desc->major, desc->minor, LTTNG_UST_PROVIDER_MAJOR, LTTNG_UST_PROVIDER_MINOR); return 0; /* reject */ @@ -176,13 +174,15 @@ int lttng_ust_probe_register(struct lttng_ust_probe_desc *desc) */ if (!check_provider_version(desc)) return 0; + if (!check_event_provider(desc)) + return 0; ust_lock_nocheck(); cds_list_add(&desc->lazy_init_head, &lazy_probe_init); desc->lazy = 1; DBG("adding probe %s containing %u events to lazy registration list", - desc->provider, desc->nr_events); + desc->provider_name, desc->nr_events); /* * If there is at least one active session, we need to register * the probe immediately, since we cannot delay event @@ -211,7 +211,7 @@ void lttng_ust_probe_unregister(struct lttng_ust_probe_desc *desc) cds_list_del(&desc->lazy_init_head); lttng_probe_provider_unregister_events(desc); - DBG("just unregistered probes of provider %s", desc->provider); + DBG("just unregistered probes of provider %s", desc->provider_name); ust_unlock(); } @@ -239,20 +239,22 @@ int lttng_probes_get_event_list(struct lttng_ust_tracepoint_list *list) CDS_INIT_LIST_HEAD(&list->head); cds_list_for_each_entry(probe_desc, probe_list, head) { for (i = 0; i < probe_desc->nr_events; i++) { + const struct lttng_ust_event_desc *event_desc = + probe_desc->event_desc[i]; struct tp_list_entry *list_entry; + /* Skip event if name is too long. */ + if (!lttng_ust_validate_event_name(event_desc)) + continue; list_entry = zmalloc(sizeof(*list_entry)); if (!list_entry) goto err_nomem; cds_list_add(&list_entry->head, &list->head); - strncpy(list_entry->tp.name, - probe_desc->event_desc[i]->name, - LTTNG_UST_ABI_SYM_NAME_LEN); - list_entry->tp.name[LTTNG_UST_ABI_SYM_NAME_LEN - 1] = '\0'; - if (!probe_desc->event_desc[i]->loglevel) { + lttng_ust_format_event_name(event_desc, list_entry->tp.name); + if (!event_desc->loglevel) { list_entry->tp.loglevel = TRACE_DEFAULT; } else { - list_entry->tp.loglevel = *(*probe_desc->event_desc[i]->loglevel); + list_entry->tp.loglevel = *(*event_desc->loglevel); } } } @@ -319,14 +321,14 @@ int lttng_probes_get_field_list(struct lttng_ust_field_list *list) /* Events without fields. */ struct tp_field_list_entry *list_entry; + /* Skip event if name is too long. */ + if (!lttng_ust_validate_event_name(event_desc)) + continue; list_entry = zmalloc(sizeof(*list_entry)); if (!list_entry) goto err_nomem; cds_list_add(&list_entry->head, &list->head); - strncpy(list_entry->field.event_name, - event_desc->name, - LTTNG_UST_ABI_SYM_NAME_LEN); - list_entry->field.event_name[LTTNG_UST_ABI_SYM_NAME_LEN - 1] = '\0'; + lttng_ust_format_event_name(event_desc, list_entry->field.event_name); list_entry->field.field_name[0] = '\0'; list_entry->field.type = LTTNG_UST_ABI_FIELD_OTHER; if (!event_desc->loglevel) { @@ -342,14 +344,14 @@ int lttng_probes_get_field_list(struct lttng_ust_field_list *list) event_desc->fields[j]; struct tp_field_list_entry *list_entry; + /* Skip event if name is too long. */ + if (!lttng_ust_validate_event_name(event_desc)) + continue; list_entry = zmalloc(sizeof(*list_entry)); if (!list_entry) goto err_nomem; cds_list_add(&list_entry->head, &list->head); - strncpy(list_entry->field.event_name, - event_desc->name, - LTTNG_UST_ABI_SYM_NAME_LEN); - list_entry->field.event_name[LTTNG_UST_ABI_SYM_NAME_LEN - 1] = '\0'; + lttng_ust_format_event_name(event_desc, list_entry->field.event_name); strncpy(list_entry->field.field_name, event_field->name, LTTNG_UST_ABI_SYM_NAME_LEN); diff --git a/liblttng-ust/ust-events-internal.h b/liblttng-ust/ust-events-internal.h index 0a5d213c..e6320504 100644 --- a/liblttng-ust/ust-events-internal.h +++ b/liblttng-ust/ust-events-internal.h @@ -899,4 +899,11 @@ __attribute__((visibility("hidden"))) int lttng_ust_session_uuid_validate(struct lttng_ust_session *session, unsigned char *uuid); +__attribute__((visibility("hidden"))) +bool lttng_ust_validate_event_name(const struct lttng_ust_event_desc *desc); + +__attribute__((visibility("hidden"))) +void lttng_ust_format_event_name(const struct lttng_ust_event_desc *desc, + char *name); + #endif /* _LTTNG_UST_EVENTS_INTERNAL_H */ -- 2.34.1