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 <mathieu.desnoyers@efficios.com>

diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
index 01f611b29c789909f7f933698c1e71b7be8d9cc6..9a46e678ad782afaaf13a3b97df3a0adf1ed5f89 100644 (file)
--- 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 6f3edf83b65903c4a9ff8bbac023fa681ae88c0c..31283f17199446d8542aed2c93d939effe620767 100644 (file)
--- 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 99691c067f23a421ceee6f5f07f883b68b543e03..83b371c5ba91431f47f44ce6880a4087d2e0244c 100644 (file)
--- 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 c76d8ba397b1c2abdb758ca5bbd8033b82d24bdc..4d41593e17cb503286e1355cbc3f03c0166b4c81 100644 (file)
--- 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 bf20c695f2d48f08f0787114de576ab44b152c0d..2b90e7bc21ae5ab35c3179619b09437c7be5804d 100644 (file)
--- 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 949529c606c253d6fdf272360658370ad818b2d7..b54ada1dd1a0ef953e9025f05f86239237bae582 100644 (file)
--- 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 7d83b87c90bcf804f796e497d5ecd35162668cb0..f9abadbb8f58eb1f05ee5bc22a3a49a509776b88 100644 (file)
--- 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 ecee23445787bce42397e96c200537b1385017ef..6cab7dc1721a57dbbd004cd84f5db4deed32072b 100644 (file)
--- 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 e734632ff05368cb298e23faeeb9fa8bfad0965a..96aeb1e5d7d2b5c5116ae884e9481e8e6b7370cd 100644 (file)
--- 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);
 }