From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Mon, 1 Dec 2014 23:12:53 +0000 (-0500)
Subject: Fix: context alignment not properly handled
X-Git-Tag: v2.5.2~2
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=9b9aff07ba39fba25ba8600fc60fa82fc392a0be;p=lttng-modules.git

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 4ecee635..15aabe63 100644
--- 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 ea4f2ac9..65117f8b 100644
--- 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 2c67ecf3..0c74234c 100644
--- 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 36712591..25cbcbb2 100644
--- 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 9cb31339..a22c9d83 100644
--- 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 133748e2..891a315d 100644
--- 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 2efd3cd6..5913ee6a 100644
--- 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 b4e66952..5ddc43c5 100644
--- 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 0b498b40..ccb2023d 100644
--- 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 f723a5c0..c18c5531 100644
--- 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 31f40392..8abba82b 100644
--- 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 17d819d9..b79b7f66 100644
--- 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 64a1ac56..69525512 100644
--- 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 9872ea4b..7055e770 100644
--- 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);
 }