From b2a84b9fd88e0e5abed91793b6667ce891ca0327 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Thu, 23 Nov 2023 17:36:09 -0500 Subject: [PATCH] Introduce event state ABI version Signed-off-by: Mathieu Desnoyers --- include/side/trace.h | 53 +++++++++++++++++++++++-------------- src/side.c | 62 ++++++++++++++++++++++++++++++-------------- src/tracer.c | 13 +++++++--- 3 files changed, 86 insertions(+), 42 deletions(-) diff --git a/include/side/trace.h b/include/side/trace.h index 15d74f2..4042431 100644 --- a/include/side/trace.h +++ b/include/side/trace.h @@ -75,9 +75,17 @@ * * Existing attribute types are never changed nor extended. Attribute * types can be added to the ABI by reserving a label within * enum side_attr_type. - * * Each union part of the ABI has an explicit side defined by a + * * Each union part of the ABI has an explicit size defined by a * side_padding() member. Each structure and union have a static * assert validating its size. + * * If the semantic of the existing event description or type fields + * change, the SIDE_EVENT_DESCRIPTION_ABI_VERSION should be increased. + * * If the semantic of the "struct side_event_state_N" fields change, + * the SIDE_EVENT_STATE_ABI_VERSION should be increased. The + * "struct side_event_state_N" is not extensible and must have its + * ABI version increased whenever it is changed. Note that increasing + * the version of SIDE_EVENT_DESCRIPTION_ABI_VERSION is not necessary + * when changing the layout of "struct side_event_state_N". * * Handling of unknown types by the tracers: * @@ -88,12 +96,12 @@ * receiving the side_call arguments. * * * Event descriptions can be extended by adding fields at the end of - * the structure. The "struct side_event_description" and "struct - * side_event_state" are therefore structures with flexible size and - * must not be used within arrays. + * the structure. The "struct side_event_description" is a structure + * with flexible size which must not be used within arrays. */ -#define SIDE_ABI_VERSION 0 +#define SIDE_EVENT_DESCRIPTION_ABI_VERSION 0 +#define SIDE_EVENT_STATE_ABI_VERSION 0 struct side_arg; struct side_arg_vec; @@ -108,7 +116,7 @@ struct side_event_description; struct side_arg_dynamic_struct; struct side_events_register_handle; struct side_arg_variant; -struct side_event_state; +struct side_event_state_0; struct side_callback; enum side_type_label { @@ -715,7 +723,7 @@ side_check_size(struct side_arg_dynamic_field, 16 + sizeof(const struct side_arg struct side_event_description { uint32_t struct_size; /* Size of this structure. */ - uint32_t version; /* ABI version. */ + uint32_t version; /* Event description ABI version. */ side_ptr_t(struct side_event_state) state; side_ptr_t(const char) provider_name; @@ -729,7 +737,7 @@ struct side_event_description { uint32_t nr_fields; uint32_t nr_attr; uint32_t nr_callbacks; -#define side_event_description_orig_abi_last nr_callbacks +#define side_event_description_orig_abi_last nr_callbacks /* End of fields supported in the original ABI. */ char end[]; /* End with a flexible array to account for extensibility. */ @@ -737,15 +745,20 @@ struct side_event_description { /* * This structure is _not_ packed to allow atomic operations on its - * fields. + * fields. Changes to this structure must bump the "Event state ABI + * version" and tracers _must_ learn how to deal with this ABI, + * otherwise they should reject the event. */ + struct side_event_state { - uint32_t struct_size; /* Size of this structure. */ + uint32_t version; /* Event state ABI version. */ +}; + +struct side_event_state_0 { + struct side_event_state p; /* Required first field. */ uint32_t enabled; side_ptr_t(const struct side_callback) callbacks; side_ptr_t(struct side_event_description) desc; - - char end[]; /* End with a flexible array to account for extensibility. */ }; /* Event and type attributes */ @@ -1849,7 +1862,7 @@ struct side_event_state { .sav = SIDE_PTR_INIT(side_sav), \ .len = SIDE_ARRAY_SIZE(side_sav), \ }; \ - side_call(&(side_event_state__##_identifier), &side_arg_vec); \ + side_call(&(side_event_state__##_identifier).p, &side_arg_vec); \ } #define side_event(_identifier, _sav) \ @@ -1870,7 +1883,7 @@ struct side_event_state { .len = SIDE_ARRAY_SIZE(side_fields), \ .nr_attr = SIDE_ARRAY_SIZE(SIDE_PARAM_SELECT_ARG1(_, ##_attr, side_attr_list())), \ }; \ - side_call_variadic(&(side_event_state__##_identifier), &side_arg_vec, &var_struct); \ + side_call_variadic(&(side_event_state__##_identifier.p), &side_arg_vec, &var_struct); \ } #define side_event_variadic(_identifier, _sav, _var, _attr...) \ @@ -1880,9 +1893,11 @@ struct side_event_state { #define _side_define_event(_linkage, _identifier, _provider, _event, _loglevel, _fields, _flags, _attr...) \ _linkage struct side_event_description __attribute__((section("side_event_description"))) \ _identifier; \ - _linkage struct side_event_state __attribute__((section("side_event_state"))) \ + _linkage struct side_event_state_0 __attribute__((section("side_event_state"))) \ side_event_state__##_identifier = { \ - .struct_size = offsetof(struct side_event_state, end), \ + .p = { \ + .version = SIDE_EVENT_STATE_ABI_VERSION, \ + }, \ .enabled = 0, \ .callbacks = SIDE_PTR_INIT(&side_empty_callback), \ .desc = SIDE_PTR_INIT(&(_identifier)), \ @@ -1890,8 +1905,8 @@ struct side_event_state { _linkage struct side_event_description __attribute__((section("side_event_description"))) \ _identifier = { \ .struct_size = offsetof(struct side_event_description, end), \ - .version = SIDE_ABI_VERSION, \ - .state = SIDE_PTR_INIT(&(side_event_state__##_identifier)), \ + .version = SIDE_EVENT_DESCRIPTION_ABI_VERSION, \ + .state = SIDE_PTR_INIT(&(side_event_state__##_identifier.p)), \ .provider_name = SIDE_PTR_INIT(_provider), \ .event_name = SIDE_PTR_INIT(_event), \ .fields = SIDE_PTR_INIT(_fields), \ @@ -1932,7 +1947,7 @@ struct side_event_state { _loglevel, SIDE_PARAM(_fields), SIDE_EVENT_FLAG_VARIADIC, SIDE_PARAM_SELECT_ARG1(_, ##_attr, side_attr_list())) #define side_declare_event(_identifier) \ - extern struct side_event_state side_event_state_##_identifier; \ + extern struct side_event_state_0 side_event_state_##_identifier; \ extern struct side_event_description _identifier #ifdef __cplusplus diff --git a/src/side.c b/src/side.c index d02a34f..b180e41 100644 --- a/src/side.c +++ b/src/side.c @@ -66,6 +66,7 @@ const struct side_callback side_empty_callback = { }; void side_call(const struct side_event_state *event_state, const struct side_arg_vec *side_arg_vec) { struct side_rcu_read_state rcu_read_state; + const struct side_event_state_0 *es0; const struct side_callback *side_cb; uintptr_t enabled; @@ -73,14 +74,17 @@ void side_call(const struct side_event_state *event_state, const struct side_arg return; if (side_unlikely(!initialized)) side_init(); - assert(!(side_ptr_get(event_state->desc)->flags & SIDE_EVENT_FLAG_VARIADIC)); - enabled = __atomic_load_n(&event_state->enabled, __ATOMIC_RELAXED); + if (side_unlikely(event_state->version != 0)) + abort(); + es0 = side_container_of(event_state, const struct side_event_state_0, p); + assert(!(side_ptr_get(es0->desc)->flags & SIDE_EVENT_FLAG_VARIADIC)); + enabled = __atomic_load_n(&es0->enabled, __ATOMIC_RELAXED); if (side_unlikely(enabled & SIDE_EVENT_ENABLED_KERNEL_USER_EVENT_MASK)) { // TODO: call kernel write. } side_rcu_read_begin(&rcu_gp, &rcu_read_state); - for (side_cb = side_rcu_dereference(side_ptr_get(event_state->callbacks)); side_cb->u.call != NULL; side_cb++) - side_cb->u.call(side_ptr_get(event_state->desc), side_arg_vec, side_cb->priv); + for (side_cb = side_rcu_dereference(side_ptr_get(es0->callbacks)); side_cb->u.call != NULL; side_cb++) + side_cb->u.call(side_ptr_get(es0->desc), side_arg_vec, side_cb->priv); side_rcu_read_end(&rcu_gp, &rcu_read_state); } @@ -89,6 +93,7 @@ void side_call_variadic(const struct side_event_state *event_state, const struct side_arg_dynamic_struct *var_struct) { struct side_rcu_read_state rcu_read_state; + const struct side_event_state_0 *es0; const struct side_callback *side_cb; uintptr_t enabled; @@ -96,14 +101,17 @@ void side_call_variadic(const struct side_event_state *event_state, return; if (side_unlikely(!initialized)) side_init(); - assert(side_ptr_get(event_state->desc)->flags & SIDE_EVENT_FLAG_VARIADIC); - enabled = __atomic_load_n(&event_state->enabled, __ATOMIC_RELAXED); + if (side_unlikely(event_state->version != 0)) + abort(); + es0 = side_container_of(event_state, const struct side_event_state_0, p); + assert(side_ptr_get(es0->desc)->flags & SIDE_EVENT_FLAG_VARIADIC); + enabled = __atomic_load_n(&es0->enabled, __ATOMIC_RELAXED); if (side_unlikely(enabled & SIDE_EVENT_ENABLED_KERNEL_USER_EVENT_MASK)) { // TODO: call kernel write. } side_rcu_read_begin(&rcu_gp, &rcu_read_state); - for (side_cb = side_rcu_dereference(side_ptr_get(event_state->callbacks)); side_cb->u.call_variadic != NULL; side_cb++) - side_cb->u.call_variadic(side_ptr_get(event_state->desc), side_arg_vec, var_struct, side_cb->priv); + for (side_cb = side_rcu_dereference(side_ptr_get(es0->callbacks)); side_cb->u.call_variadic != NULL; side_cb++) + side_cb->u.call_variadic(side_ptr_get(es0->desc), side_arg_vec, var_struct, side_cb->priv); side_rcu_read_end(&rcu_gp, &rcu_read_state); } @@ -113,9 +121,13 @@ const struct side_callback *side_tracer_callback_lookup( void *call, void *priv) { struct side_event_state *event_state = side_ptr_get(desc->state); + const struct side_event_state_0 *es0; const struct side_callback *cb; - for (cb = side_ptr_get(event_state->callbacks); cb->u.call != NULL; cb++) { + if (side_unlikely(event_state->version != 0)) + abort(); + es0 = side_container_of(event_state, const struct side_event_state_0, p); + for (cb = side_ptr_get(es0->callbacks); cb->u.call != NULL; cb++) { if ((void *) cb->u.call == call && cb->priv == priv) return cb; } @@ -128,6 +140,7 @@ int _side_tracer_callback_register(struct side_event_description *desc, { struct side_event_state *event_state; struct side_callback *old_cb, *new_cb; + struct side_event_state_0 *es0; int ret = SIDE_ERROR_OK; uint32_t old_nr_cb; @@ -139,6 +152,9 @@ int _side_tracer_callback_register(struct side_event_description *desc, side_init(); pthread_mutex_lock(&side_lock); event_state = side_ptr_get(desc->state); + if (side_unlikely(event_state->version != 0)) + abort(); + es0 = side_container_of(event_state, struct side_event_state_0, p); old_nr_cb = desc->nr_callbacks; if (old_nr_cb == UINT32_MAX) { ret = SIDE_ERROR_INVAL; @@ -149,7 +165,7 @@ int _side_tracer_callback_register(struct side_event_description *desc, ret = SIDE_ERROR_EXIST; goto unlock; } - old_cb = (struct side_callback *) side_ptr_get(event_state->callbacks); + old_cb = (struct side_callback *) side_ptr_get(es0->callbacks); /* old_nr_cb + 1 (new cb) + 1 (NULL) */ new_cb = (struct side_callback *) calloc(old_nr_cb + 2, sizeof(struct side_callback)); if (!new_cb) { @@ -165,14 +181,14 @@ int _side_tracer_callback_register(struct side_event_description *desc, (side_tracer_callback_func) call; new_cb[old_nr_cb].priv = priv; /* High order bits are already zeroed. */ - side_rcu_assign_pointer(side_ptr_get(event_state->callbacks), new_cb); + side_rcu_assign_pointer(side_ptr_get(es0->callbacks), new_cb); side_rcu_wait_grace_period(&rcu_gp); if (old_nr_cb) free(old_cb); desc->nr_callbacks++; /* Increment concurrently with kernel setting the top bits. */ if (!old_nr_cb) - (void) __atomic_add_fetch(&event_state->enabled, 1, __ATOMIC_RELAXED); + (void) __atomic_add_fetch(&es0->enabled, 1, __ATOMIC_RELAXED); unlock: pthread_mutex_unlock(&side_lock); return ret; @@ -202,6 +218,7 @@ static int _side_tracer_callback_unregister(struct side_event_description *desc, struct side_event_state *event_state; struct side_callback *old_cb, *new_cb; const struct side_callback *cb_pos; + struct side_event_state_0 *es0; uint32_t pos_idx; int ret = SIDE_ERROR_OK; uint32_t old_nr_cb; @@ -214,17 +231,20 @@ static int _side_tracer_callback_unregister(struct side_event_description *desc, side_init(); pthread_mutex_lock(&side_lock); event_state = side_ptr_get(desc->state); + if (side_unlikely(event_state->version != 0)) + abort(); + es0 = side_container_of(event_state, struct side_event_state_0, p); cb_pos = side_tracer_callback_lookup(desc, call, priv); if (!cb_pos) { ret = SIDE_ERROR_NOENT; goto unlock; } old_nr_cb = desc->nr_callbacks; - old_cb = (struct side_callback *) side_ptr_get(event_state->callbacks); + old_cb = (struct side_callback *) side_ptr_get(es0->callbacks); if (old_nr_cb == 1) { new_cb = (struct side_callback *) &side_empty_callback; } else { - pos_idx = cb_pos - side_ptr_get(event_state->callbacks); + pos_idx = cb_pos - side_ptr_get(es0->callbacks); /* Remove entry at pos_idx. */ /* old_nr_cb - 1 (removed cb) + 1 (NULL) */ new_cb = (struct side_callback *) calloc(old_nr_cb, sizeof(struct side_callback)); @@ -236,13 +256,13 @@ static int _side_tracer_callback_unregister(struct side_event_description *desc, memcpy(&new_cb[pos_idx], &old_cb[pos_idx + 1], old_nr_cb - pos_idx - 1); } /* High order bits are already zeroed. */ - side_rcu_assign_pointer(side_ptr_get(event_state->callbacks), new_cb); + side_rcu_assign_pointer(side_ptr_get(es0->callbacks), new_cb); side_rcu_wait_grace_period(&rcu_gp); free(old_cb); desc->nr_callbacks--; /* Decrement concurrently with kernel setting the top bits. */ if (old_nr_cb == 1) - (void) __atomic_add_fetch(&event_state->enabled, -1, __ATOMIC_RELAXED); + (void) __atomic_add_fetch(&es0->enabled, -1, __ATOMIC_RELAXED); unlock: pthread_mutex_unlock(&side_lock); return ret; @@ -298,19 +318,23 @@ void side_event_remove_callbacks(struct side_event_description *desc) { struct side_event_state *event_state = side_ptr_get(desc->state); uint32_t nr_cb = desc->nr_callbacks; + struct side_event_state_0 *es0; struct side_callback *old_cb; if (!nr_cb) return; - old_cb = (struct side_callback *) side_ptr_get(event_state->callbacks); - (void) __atomic_add_fetch(&event_state->enabled, -1, __ATOMIC_RELAXED); + if (side_unlikely(event_state->version != 0)) + abort(); + es0 = side_container_of(event_state, struct side_event_state_0, p); + old_cb = (struct side_callback *) side_ptr_get(es0->callbacks); + (void) __atomic_add_fetch(&es0->enabled, -1, __ATOMIC_RELAXED); /* * Setting the state back to 0 cb and empty callbacks out of * caution. This should not matter because instrumentation is * unreachable. */ desc->nr_callbacks = 0; - side_rcu_assign_pointer(side_ptr_get(event_state->callbacks), &side_empty_callback); + side_rcu_assign_pointer(side_ptr_get(es0->callbacks), &side_empty_callback); /* * No need to wait for grace period because instrumentation is * unreachable. diff --git a/src/tracer.c b/src/tracer.c index dc902aa..352cba5 100644 --- a/src/tracer.c +++ b/src/tracer.c @@ -1797,15 +1797,20 @@ void tracer_event_notification(enum side_tracer_notification notif, /* Skip NULL pointers */ if (!event) continue; - if (event->version != SIDE_ABI_VERSION) { - printf("Error: event SIDE ABI version (%u) does not match the SIDE ABI version supported by the tracer (%u)\n", - event->version, SIDE_ABI_VERSION); + if (event->version != SIDE_EVENT_DESCRIPTION_ABI_VERSION) { + printf("Error: event description ABI version (%u) does not match the version supported by the tracer (%u)\n", + event->version, SIDE_EVENT_DESCRIPTION_ABI_VERSION); + return; + } + if (side_ptr_get(event->state)->version != SIDE_EVENT_STATE_ABI_VERSION) { + printf("Error: event state ABI version (%u) does not match the version supported by the tracer (%u)\n", + side_ptr_get(event->state)->version, SIDE_EVENT_STATE_ABI_VERSION); return; } printf("provider: %s, event: %s\n", side_ptr_get(event->provider_name), side_ptr_get(event->event_name)); if (event->struct_size != side_offsetofend(struct side_event_description, side_event_description_orig_abi_last)) { - printf("Warning: Event %s.%s contains fields unknown to the tracer\n", + printf("Warning: Event %s.%s description contains fields unknown to the tracer\n", side_ptr_get(event->provider_name), side_ptr_get(event->event_name)); } if (notif == SIDE_TRACER_NOTIFICATION_INSERT_EVENTS) { -- 2.34.1