ctf: explicitly specify the sizes of structures and unions

Before now, we have allowed libdtrace-ctf to decide what size a
structure or union is, given the alignment and packing requirements of
the members.  However, this is flawed: the alignment and packing
requirements are platform-dependent, and libdtrace-ctf has no idea what
those requirements are: and for packed structures they are certain to be
wrong anyway.

The DWARF already encodes precisely how large all these things are: we
even collect and use this information to disambiguate otherwise-
identical structures (e.g. those modified by #ifdefs that vary by
translation unit).  So refactor this code out into a new
private_dwarf_size() function and use it both from the old place in
type_id() and from assemble_ctf_struct_union().  If no size is
specified, or if the compiler has elected to use a form that we cannot
handle (like, say, an exprloc, which we are *not* interpreting), let
libdtrace-ctf figure it out, but otherwise -- in the vast majority of
cases -- explicitly specify it.

Explicit specification of sizes requires libdtrace-ctf 1.1, which this
commit thus requires.

(This also adds some suitable structures to the dt_test module so that
the DTrace testsuite can verify that this change does not regress.)

Signed-off-by: Nick Alcock <[email protected]>
Reviewed-by: Kris Van Hees <[email protected]>
Orabug: 29054989

Author: nickalcock

dtrace/dt_test_dev.c

@@ -2,7 +2,7 @@
  * FILE:	dt_test_dev.c
  * DESCRIPTION:	DTrace - test provider device driver
  *
- * Copyright (c) 2011, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2018, Oracle and/or its affiliates. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -17,6 +17,7 @@
 
 #include <linux/fs.h>
 #include <linux/miscdevice.h>
+#include <linux/types.h>
 #include <trace/syscall.h>
 #include <asm/unistd.h>
 
@@ -27,6 +28,44 @@
 static dtrace_id_t	pid = DTRACE_IDNONE;
 static int		enabled = 0;
 
+/*
+ * Some arrays of structures of different sizes populated with
+ * unchanging randomly-chosen numbers, for padding tests.
+ */
+
+static struct dt_test_int_char
+{
+	int foo;
+	char bar;
+} intish[2] __attribute__((used)) = { { 47204473, 48 },
+				       { 18472, 62 } };
+
+static struct dt_test_long_int
+{
+	long foo;
+	int bar;
+} longish[2] __attribute__((used)) = { { 43737975, 240724 },
+					{ 24924709, 526 } };
+
+static struct dt_test_longlong_long
+{
+	long long foo;
+	long bar;
+} longlongish[2] __attribute__((used)) = { { 4294479287, 4395957 },
+					    { 5239637, 249750 } };
+
+static struct dt_test_like_a_scatterlist
+{
+	unsigned long	a;
+	unsigned int	b;
+	unsigned int	c;
+	u64	d;
+	unsigned int	e;
+} scatter_failure[2] __attribute__((used)) = { { .a = 1, .b = 2,
+						 .c = 3, .d = 4, .e = 5 },
+					       { .a = 6, .b = 7,
+						 .c = 8, .d = 9, .e = 10 } };
+
 void dt_test_provide(void *arg, const dtrace_probedesc_t *desc)
 {
 	if (dtrace_probe_lookup(dt_test_id, "dt_test", NULL, "test") != DTRACE_IDNONE)

scripts/dwarf2ctf/dwarf2ctf.c

@@ -773,6 +773,12 @@ static inline Dwarf_Attribute *private_dwarf_attr(Dwarf_Die *die,
 static inline Dwarf_Word private_dwarf_udata(Dwarf_Die *die, int attribute,
 					     die_override_t *overrides);
 
+/*
+ * Given a DIE, return its byte size, if known and interpretable, or -1
+ * otherwise.
+ */
+static inline long long private_dwarf_size(Dwarf_Die *die);
+
 /*
  * Find an override in an override list.
  */
@@ -1631,11 +1637,11 @@ static char *type_id(Dwarf_Die *die,
 	case DW_TAG_structure_type:
 	case DW_TAG_union_type: {
 		/*
-		 * Incorporate the sizeof() the structure, if statically known
-		 * (the offset of the last member in the DWARF) so that most
-		 * structures which are redefined on the fly by preprocessor
-		 * defines are disambiguated despite being defined in the same
-		 * place.
+		 * Incorporate the unaligned sizeof() the structure, if
+		 * statically known (the offset of the last member in the DWARF)
+		 * so that most structures which are redefined on the fly by
+		 * preprocessor defines are disambiguated despite being defined
+		 * in the same place.
 		 *
 		 * Only do this if this is a non-opaque structure/union
 		 * definition: opaque definitions cannot have a size, but if
@@ -1646,35 +1652,12 @@ static char *type_id(Dwarf_Die *die,
 		 */
 		const char *sou;
 
-		if (strncmp(id, "////", 4) != 0 &&
-		    private_dwarf_hasattr(die, DW_AT_byte_size)) {
-			Dwarf_Attribute size_attr;
+		if (strncmp(id, "////", 4) != 0) {
 			long long size;
 			char byte_size[24];
 
-			private_dwarf_attr(die, DW_AT_byte_size, &size_attr);
-
-			switch (dwarf_whatform(&size_attr)) {
-			case DW_FORM_data1:
-			case DW_FORM_data2:
-			case DW_FORM_data4:
-			case DW_FORM_data8:
-			case DW_FORM_udata:
-			case DW_FORM_sdata:
-
-				if (dwarf_whatform(&size_attr) ==
-				    DW_FORM_sdata) {
-					Dwarf_Sword dw_size;
-
-					dwarf_formsdata(&size_attr, &dw_size);
-					size = dw_size;
-				} else {
-					Dwarf_Word dw_size;
-
-					dwarf_formudata(&size_attr, &dw_size);
-					size = dw_size;
-				}
-
+			size = private_dwarf_size(die);
+			if (size > -1) {
 				sprintf(byte_size, "%lli", size);
 				id = str_appendn(id, byte_size, "//", NULL);
 			}
@@ -3669,19 +3652,33 @@ static ctf_id_t assemble_ctf_struct_union(const char *module_name,
 					  enum skip_type *skip,
 					  int *replace)
 {
-	ctf_id_t (*ctf_add_sou)(ctf_file_t *, uint_t, const char *);
+	ctf_id_t (*ctf_add_sou)(ctf_file_t *, uint_t, const char *, size_t);
 
 	const char *name = dwarf_diename(die);
 	int is_union = (dwarf_tag(die) == DW_TAG_union_type);
 	ctf_memb_count_t *member_count = NULL;
 	ctf_id_t id;
+	long long size;
 
 	/*
 	 * FIXME: these both need handling for DWARF4 support.
 	 */
 	CTF_DW_ENFORCE_NOT(specification);
 	CTF_DW_ENFORCE_NOT(signature);
 
+	/*
+	 * Figure out the size of the type (if possible) and force it into the
+	 * CTF to ensure that struct/union padding is added appropriately.
+	 *
+	 * If we don't know it, force a size of zero, which is interpreted as
+	 * being equivalent to a call to the unsized struct/union addition
+	 * function, letting libdtrace-ctf figure out a likely size as best it
+	 * can.
+	 */
+	size = private_dwarf_size(die);
+	if (size < 0)
+		size = 0;
+
 	/*
 	 * Possibly we should ignore this entire structure, if we already know
 	 * of one with the same name and at least as many members.  If we
@@ -3736,13 +3733,14 @@ static ctf_id_t assemble_ctf_struct_union(const char *module_name,
 	}
 
 	dw_ctf_trace("%s: adding structure %s\n", locerrstr, name);
+
 	if (is_union)
-		ctf_add_sou = ctf_add_union;
+		ctf_add_sou = ctf_add_union_sized;
 	else
-		ctf_add_sou = ctf_add_struct;
+		ctf_add_sou = ctf_add_struct_sized;
 
 	id = ctf_add_sou(ctf, top_level_type ? CTF_ADD_ROOT : CTF_ADD_NONROOT,
-			 name);
+			 name, size);
 
 	if (member_count != NULL)
 		member_count->ctf_id = id;
@@ -4413,6 +4411,43 @@ static inline Dwarf_Word private_dwarf_udata(Dwarf_Die *die, int attribute,
 	return value;
 }
 
+/*
+ * Given a DIE, return its byte size, if known and interpretable, or -1
+ * otherwise.
+ */
+static inline long long private_dwarf_size(Dwarf_Die *die)
+{
+	Dwarf_Attribute size_attr;
+
+	if (private_dwarf_hasattr(die, DW_AT_byte_size)) {
+		private_dwarf_attr(die, DW_AT_byte_size, &size_attr);
+
+		switch (dwarf_whatform(&size_attr)) {
+		case DW_FORM_data1:
+		case DW_FORM_data2:
+		case DW_FORM_data4:
+		case DW_FORM_data8:
+		case DW_FORM_udata: {
+			Dwarf_Word dw_size;
+
+			dwarf_formudata(&size_attr, &dw_size);
+			return dw_size;
+		}
+		case DW_FORM_sdata: {
+			Dwarf_Sword dw_size;
+
+			dwarf_formsdata(&size_attr, &dw_size);
+			return dw_size;
+		}
+		}
+	}
+
+	/*
+	 * exprloc or other type we don't know how to interpret yet.
+	 */
+	return -1;
+}
+
 /*
  * Find an override in an override list, walking up the chained overrides if
  * need be, until one is found.

uek-rpm/ol7/kernel-uek.spec

@@ -558,7 +558,7 @@ BuildRequires: pesign >= 0.10-4
 BuildRequires: hmaccalc
 %endif
 %if %{with_dtrace}
-BuildRequires: libdtrace-ctf-devel >= 0.8.0
+BuildRequires: libdtrace-ctf-devel >= 1.1.0
 %endif
 %if %{with_perf_tui}
 BuildRequires: slang-devel, slang-static
@@ -864,7 +864,7 @@ Requires(pre): /usr/bin/find\
 Requires: elfutils-libelf >= 0.160\
 Requires: elfutils-libs >= 0.160\
 %if %{with_dtrace}\
-Requires: libdtrace-ctf >= 0.8.0\
+Requires: libdtrace-ctf >= 1.1.0\
 %endif\
 %description -n kernel%{?variant}%{?1:-%{1}}-devel\
 This package provides kernel headers and makefiles sufficient to build modules\