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 #858

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/lttng-context-hostname.c b/lttng-context-hostname.c
index 4ecee6355162292027ef3799f59ab55441a1c07e..15aabe63c3f18bb31800f9b7ed61fc14dd6a859d 100644 (file)
--- a/lttng-context-hostname.c
+++ b/lttng-context-hostname.c
@@ -89,6 +89,7 @@ int lttng_add_hostname_to_ctx(struct lttng_ctx **ctx)
 
        field->get_size = hostname_get_size;
        field->record = hostname_record;
+       lttng_context_update(*ctx);
        wrapper_vmalloc_sync_all();
        return 0;
 }

diff --git a/lttng-context-nice.c b/lttng-context-nice.c
index ea4f2ac98c4bcb06208a790dca662e48472bf29d..65117f8bce3ed843285d4fa8a4fb8ff24a37998e 100644 (file)
--- a/lttng-context-nice.c
+++ b/lttng-context-nice.c
@@ -71,6 +71,7 @@ int lttng_add_nice_to_ctx(struct lttng_ctx **ctx)
        field->event_field.type.u.basic.integer.encoding = lttng_encode_none;
        field->get_size = nice_get_size;
        field->record = nice_record;
+       lttng_context_update(*ctx);
        wrapper_vmalloc_sync_all();
        return 0;
 }

diff --git a/lttng-context-perf-counters.c b/lttng-context-perf-counters.c
index 2c67ecf3eebc3978e965571bacf10d26f76a7c8f..0c74234c281711e44f9782bd182577ac7c72e3d2 100644 (file)
--- a/lttng-context-perf-counters.c
+++ b/lttng-context-perf-counters.c
@@ -253,6 +253,7 @@ int lttng_add_perf_counter_to_ctx(uint32_t type,
        field->record = perf_counter_record;
        field->u.perf_counter = perf_field;
        perf_field->hp_enable = 1;
+       lttng_context_update(*ctx);
 
        wrapper_vmalloc_sync_all();
        return 0;

diff --git a/lttng-context-pid.c b/lttng-context-pid.c
index 36712591a1c08114f493de5a7a801efe3722877c..25cbcbb299d70f6cee9107c959c98aa4595f8f12 100644 (file)
--- a/lttng-context-pid.c
+++ b/lttng-context-pid.c
@@ -71,6 +71,7 @@ int lttng_add_pid_to_ctx(struct lttng_ctx **ctx)
        field->event_field.type.u.basic.integer.encoding = lttng_encode_none;
        field->get_size = pid_get_size;
        field->record = pid_record;
+       lttng_context_update(*ctx);
        wrapper_vmalloc_sync_all();
        return 0;
 }

diff --git a/lttng-context-ppid.c b/lttng-context-ppid.c
index 9cb3133996eada56a49ef2b24b088ad063f4ee02..a22c9d83ccd2671133db528ea8b02b64ef242620 100644 (file)
--- a/lttng-context-ppid.c
+++ b/lttng-context-ppid.c
@@ -83,6 +83,7 @@ int lttng_add_ppid_to_ctx(struct lttng_ctx **ctx)
        field->event_field.type.u.basic.integer.encoding = lttng_encode_none;
        field->get_size = ppid_get_size;
        field->record = ppid_record;
+       lttng_context_update(*ctx);
        wrapper_vmalloc_sync_all();
        return 0;
 }

diff --git a/lttng-context-prio.c b/lttng-context-prio.c
index 133748e28c9fff0b2abc7e4d4f5768f9ed6d6518..891a315d7b990cc1133204f7263034faa35be3b4 100644 (file)
--- a/lttng-context-prio.c
+++ b/lttng-context-prio.c
@@ -92,6 +92,7 @@ int lttng_add_prio_to_ctx(struct lttng_ctx **ctx)
        field->event_field.type.u.basic.integer.encoding = lttng_encode_none;
        field->get_size = prio_get_size;
        field->record = prio_record;
+       lttng_context_update(*ctx);
        wrapper_vmalloc_sync_all();
        return 0;
 }

diff --git a/lttng-context-procname.c b/lttng-context-procname.c
index 2efd3cd6e1fb9514884f93dc125d117013a515ff..5913ee6ac997fefb5a8f65c69cd45aa41a7cc68c 100644 (file)
--- a/lttng-context-procname.c
+++ b/lttng-context-procname.c
@@ -75,6 +75,7 @@ int lttng_add_procname_to_ctx(struct lttng_ctx **ctx)
 
        field->get_size = procname_get_size;
        field->record = procname_record;
+       lttng_context_update(*ctx);
        wrapper_vmalloc_sync_all();
        return 0;
 }

diff --git a/lttng-context-tid.c b/lttng-context-tid.c
index b4e6695246c6366a16874ef3636bc7551b0cd540..5ddc43c5cfdbcb94587ded42da4594a1c38af4b9 100644 (file)
--- a/lttng-context-tid.c
+++ b/lttng-context-tid.c
@@ -71,6 +71,7 @@ int lttng_add_tid_to_ctx(struct lttng_ctx **ctx)
        field->event_field.type.u.basic.integer.encoding = lttng_encode_none;
        field->get_size = tid_get_size;
        field->record = tid_record;
+       lttng_context_update(*ctx);
        wrapper_vmalloc_sync_all();
        return 0;
 }

diff --git a/lttng-context-vpid.c b/lttng-context-vpid.c
index 0b498b40265a2a337ba5799e6df2d0f76aa8d707..ccb2023d3329b1050d0e87f0068a586a4b7c0e00 100644 (file)
--- a/lttng-context-vpid.c
+++ b/lttng-context-vpid.c
@@ -77,6 +77,7 @@ int lttng_add_vpid_to_ctx(struct lttng_ctx **ctx)
        field->event_field.type.u.basic.integer.encoding = lttng_encode_none;
        field->get_size = vpid_get_size;
        field->record = vpid_record;
+       lttng_context_update(*ctx);
        wrapper_vmalloc_sync_all();
        return 0;
 }

diff --git a/lttng-context-vppid.c b/lttng-context-vppid.c
index f723a5c0cf78202dd2915a0217b10dcb647a4fdf..c18c553148d6e41a0339b9f77faf0dbe0fd5c6f9 100644 (file)
--- a/lttng-context-vppid.c
+++ b/lttng-context-vppid.c
@@ -92,6 +92,7 @@ int lttng_add_vppid_to_ctx(struct lttng_ctx **ctx)
        field->event_field.type.u.basic.integer.encoding = lttng_encode_none;
        field->get_size = vppid_get_size;
        field->record = vppid_record;
+       lttng_context_update(*ctx);
        wrapper_vmalloc_sync_all();
        return 0;
 }

diff --git a/lttng-context-vtid.c b/lttng-context-vtid.c
index 31f40392c495d58e9f7a87293114caaa303aa811..8abba82bfc93c14cf88e19000be7f545a55c98ff 100644 (file)
--- a/lttng-context-vtid.c
+++ b/lttng-context-vtid.c
@@ -77,6 +77,7 @@ int lttng_add_vtid_to_ctx(struct lttng_ctx **ctx)
        field->event_field.type.u.basic.integer.encoding = lttng_encode_none;
        field->get_size = vtid_get_size;
        field->record = vtid_record;
+       lttng_context_update(*ctx);
        wrapper_vmalloc_sync_all();
        return 0;
 }

diff --git a/lttng-context.c b/lttng-context.c
index 17d819d90121f7e0c7146361e115197939f6fce3..b79b7f667bf483ce6611cfae31190fb1c19f12b4 100644 (file)
--- a/lttng-context.c
+++ b/lttng-context.c
@@ -55,6 +55,7 @@ struct lttng_ctx_field *lttng_append_context(struct lttng_ctx **ctx_p)
                *ctx_p = kzalloc(sizeof(struct lttng_ctx), GFP_KERNEL);
                if (!*ctx_p)
                        return NULL;
+               (*ctx_p)->largest_align = 1;
        }
        ctx = *ctx_p;
        if (ctx->nr_fields + 1 > ctx->allocated_fields) {
@@ -75,6 +76,94 @@ struct lttng_ctx_field *lttng_append_context(struct lttng_ctx **ctx_p)
 }
 EXPORT_SYMBOL_GPL(lttng_append_context);
 
+/*
+ * 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/lttng-events.h b/lttng-events.h
index c84689cb3c96cd8ef703486ba3a6928ad263b226..e8c9afedffc3eb3ac94c1536e72d3943aaf330be 100644 (file)
--- a/lttng-events.h
+++ b/lttng-events.h
@@ -167,6 +167,7 @@ struct lttng_ctx {
        struct lttng_ctx_field *fields;
        unsigned int nr_fields;
        unsigned int allocated_fields;
+       size_t largest_align;   /* in bytes */
 };
 
 struct lttng_event_desc {
@@ -405,6 +406,7 @@ static inline int lttng_syscalls_unregister(struct lttng_channel *chan)
 #endif
 
 struct lttng_ctx_field *lttng_append_context(struct lttng_ctx **ctx);
+void lttng_context_update(struct lttng_ctx *ctx);
 int lttng_find_context(struct lttng_ctx *ctx, const char *name);
 void lttng_remove_context_field(struct lttng_ctx **ctx,
                                struct lttng_ctx_field *field);

diff --git a/lttng-ring-buffer-client.h b/lttng-ring-buffer-client.h
index 9872ea4bc5be56f35b22c00eac7aa6aeab2b324c..7055e770734e52a6f339f3c6b78e1c7913f07a1d 100644 (file)
--- a/lttng-ring-buffer-client.h
+++ b/lttng-ring-buffer-client.h
@@ -88,6 +88,7 @@ size_t ctx_get_size(size_t offset, struct lttng_ctx *ctx)
 
        if (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;
@@ -102,6 +103,7 @@ void ctx_record(struct lib_ring_buffer_ctx *bufctx,
 
        if (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);
 }