From 7a6e8f361f071f8fc002bc905caeed41e09caecf Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 1 Dec 2014 18:18:13 -0500 Subject: [PATCH] Fix: context alignment not properly handled This issue affects only architectures without efficient unaligned accesses, only when a context field with larger alignment follows a context field with smaller alignment. It generates unreadable traces when such context fields are enabled in this configuration. Fixes #867 Signed-off-by: Mathieu Desnoyers --- include/lttng/ust-events.h | 4 +- liblttng-ust/lttng-context-ip.c | 1 + liblttng-ust/lttng-context-perf-counters.c | 1 + liblttng-ust/lttng-context-procname.c | 1 + liblttng-ust/lttng-context-pthread-id.c | 1 + liblttng-ust/lttng-context-vpid.c | 1 + liblttng-ust/lttng-context-vtid.c | 1 + liblttng-ust/lttng-context.c | 89 ++++++++++++++++++++++ liblttng-ust/lttng-ring-buffer-client.h | 2 + 9 files changed, 100 insertions(+), 1 deletion(-) diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h index 01f611b2..9a46e678 100644 --- a/include/lttng/ust-events.h +++ b/include/lttng/ust-events.h @@ -258,11 +258,12 @@ struct lttng_ctx_field { void (*destroy)(struct lttng_ctx_field *field); }; -#define LTTNG_UST_CTX_PADDING 24 +#define LTTNG_UST_CTX_PADDING 20 struct lttng_ctx { struct lttng_ctx_field *fields; unsigned int nr_fields; unsigned int allocated_fields; + unsigned int largest_align; char padding[LTTNG_UST_CTX_PADDING]; }; @@ -590,6 +591,7 @@ void lttng_probes_exit(void); int lttng_find_context(struct lttng_ctx *ctx, const char *name); int lttng_get_context_index(struct lttng_ctx *ctx, const char *name); struct lttng_ctx_field *lttng_append_context(struct lttng_ctx **ctx_p); +void lttng_context_update(struct lttng_ctx *ctx); void lttng_remove_context_field(struct lttng_ctx **ctx_p, struct lttng_ctx_field *field); void lttng_destroy_context(struct lttng_ctx *ctx); diff --git a/liblttng-ust/lttng-context-ip.c b/liblttng-ust/lttng-context-ip.c index 6f3edf83..31283f17 100644 --- a/liblttng-ust/lttng-context-ip.c +++ b/liblttng-ust/lttng-context-ip.c @@ -69,5 +69,6 @@ int lttng_add_ip_to_ctx(struct lttng_ctx **ctx) field->event_field.type.u.basic.integer.encoding = lttng_encode_none; field->get_size = ip_get_size; field->record = ip_record; + lttng_context_update(*ctx); return 0; } diff --git a/liblttng-ust/lttng-context-perf-counters.c b/liblttng-ust/lttng-context-perf-counters.c index 99691c06..83b371c5 100644 --- a/liblttng-ust/lttng-context-perf-counters.c +++ b/liblttng-ust/lttng-context-perf-counters.c @@ -402,6 +402,7 @@ int lttng_add_perf_counter_to_ctx(uint32_t type, * the field here. */ + lttng_context_update(*ctx); return 0; setup_error: diff --git a/liblttng-ust/lttng-context-procname.c b/liblttng-ust/lttng-context-procname.c index c76d8ba3..4d41593e 100644 --- a/liblttng-ust/lttng-context-procname.c +++ b/liblttng-ust/lttng-context-procname.c @@ -108,6 +108,7 @@ int lttng_add_procname_to_ctx(struct lttng_ctx **ctx) field->get_size = procname_get_size; field->record = procname_record; field->get_value = procname_get_value; + lttng_context_update(*ctx); return 0; } diff --git a/liblttng-ust/lttng-context-pthread-id.c b/liblttng-ust/lttng-context-pthread-id.c index bf20c695..2b90e7bc 100644 --- a/liblttng-ust/lttng-context-pthread-id.c +++ b/liblttng-ust/lttng-context-pthread-id.c @@ -79,5 +79,6 @@ int lttng_add_pthread_id_to_ctx(struct lttng_ctx **ctx) field->get_size = pthread_id_get_size; field->record = pthread_id_record; field->get_value = pthread_id_get_value; + lttng_context_update(*ctx); return 0; } diff --git a/liblttng-ust/lttng-context-vpid.c b/liblttng-ust/lttng-context-vpid.c index 949529c6..b54ada1d 100644 --- a/liblttng-ust/lttng-context-vpid.c +++ b/liblttng-ust/lttng-context-vpid.c @@ -115,5 +115,6 @@ int lttng_add_vpid_to_ctx(struct lttng_ctx **ctx) field->get_size = vpid_get_size; field->record = vpid_record; field->get_value = vpid_get_value; + lttng_context_update(*ctx); return 0; } diff --git a/liblttng-ust/lttng-context-vtid.c b/liblttng-ust/lttng-context-vtid.c index 7d83b87c..f9abadbb 100644 --- a/liblttng-ust/lttng-context-vtid.c +++ b/liblttng-ust/lttng-context-vtid.c @@ -98,6 +98,7 @@ int lttng_add_vtid_to_ctx(struct lttng_ctx **ctx) field->get_size = vtid_get_size; field->record = vtid_record; field->get_value = vtid_get_value; + lttng_context_update(*ctx); return 0; } diff --git a/liblttng-ust/lttng-context.c b/liblttng-ust/lttng-context.c index ecee2344..6cab7dc1 100644 --- a/liblttng-ust/lttng-context.c +++ b/liblttng-ust/lttng-context.c @@ -80,6 +80,7 @@ struct lttng_ctx_field *lttng_append_context(struct lttng_ctx **ctx_p) *ctx_p = zmalloc(sizeof(struct lttng_ctx)); if (!*ctx_p) return NULL; + (*ctx_p)->largest_align = 1; } ctx = *ctx_p; if (ctx->nr_fields + 1 > ctx->allocated_fields) { @@ -99,6 +100,94 @@ struct lttng_ctx_field *lttng_append_context(struct lttng_ctx **ctx_p) return field; } +/* + * lttng_context_update() should be called at least once between context + * modification and trace start. + */ +void lttng_context_update(struct lttng_ctx *ctx) +{ + int i; + size_t largest_align = 8; /* in bits */ + + for (i = 0; i < ctx->nr_fields; i++) { + struct lttng_type *type; + size_t field_align = 8; + + type = &ctx->fields[i].event_field.type; + switch (type->atype) { + case atype_integer: + field_align = type->u.basic.integer.alignment; + break; + case atype_array: + { + struct lttng_basic_type *btype; + + btype = &type->u.array.elem_type; + switch (btype->atype) { + case atype_integer: + field_align = btype->u.basic.integer.alignment; + break; + case atype_string: + break; + + case atype_array: + case atype_sequence: + default: + WARN_ON_ONCE(1); + break; + } + break; + } + case atype_sequence: + { + struct lttng_basic_type *btype; + + btype = &type->u.sequence.length_type; + switch (btype->atype) { + case atype_integer: + field_align = btype->u.basic.integer.alignment; + break; + + case atype_string: + case atype_array: + case atype_sequence: + default: + WARN_ON_ONCE(1); + break; + } + + btype = &type->u.sequence.elem_type; + switch (btype->atype) { + case atype_integer: + field_align = max_t(size_t, + field_align, + btype->u.basic.integer.alignment); + break; + + case atype_string: + break; + + case atype_array: + case atype_sequence: + default: + WARN_ON_ONCE(1); + break; + } + break; + } + case atype_string: + break; + + case atype_enum: + default: + WARN_ON_ONCE(1); + break; + } + largest_align = max_t(size_t, largest_align, field_align); + } + ctx->largest_align = largest_align >> 3; /* bits to bytes */ +} + /* * Remove last context field. */ diff --git a/liblttng-ust/lttng-ring-buffer-client.h b/liblttng-ust/lttng-ring-buffer-client.h index e734632f..96aeb1e5 100644 --- a/liblttng-ust/lttng-ring-buffer-client.h +++ b/liblttng-ust/lttng-ring-buffer-client.h @@ -76,6 +76,7 @@ size_t ctx_get_size(size_t offset, struct lttng_ctx *ctx) if (caa_likely(!ctx)) return 0; + offset += lib_ring_buffer_align(offset, ctx->largest_align); for (i = 0; i < ctx->nr_fields; i++) offset += ctx->fields[i].get_size(offset); return offset - orig_offset; @@ -90,6 +91,7 @@ void ctx_record(struct lttng_ust_lib_ring_buffer_ctx *bufctx, if (caa_likely(!ctx)) return; + lib_ring_buffer_align_ctx(bufctx, ctx->largest_align); for (i = 0; i < ctx->nr_fields; i++) ctx->fields[i].record(&ctx->fields[i], bufctx, chan); } -- 2.34.1