Message-ID: <20240815173903.4172139-37-samitolvanen@google.com>
Date: Thu, 15 Aug 2024 17:39:20 +0000
From: Sami Tolvanen <samitolvanen@...gle.com>
To: Masahiro Yamada <masahiroy@...nel.org>, Luis Chamberlain <mcgrof@...nel.org>, 
	Miguel Ojeda <ojeda@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Matthew Maurer <mmaurer@...gle.com>, Alex Gaynor <alex.gaynor@...il.com>, 
	Wedson Almeida Filho <wedsonaf@...il.com>, Gary Guo <gary@...yguo.net>, Petr Pavlu <petr.pavlu@...e.com>, 
	Neal Gompa <neal@...pa.dev>, Hector Martin <marcan@...can.st>, Janne Grunau <j@...nau.net>, 
	Asahi Linux <asahi@...ts.linux.dev>, linux-kbuild@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-modules@...r.kernel.org, 
	rust-for-linux@...r.kernel.org, Sami Tolvanen <samitolvanen@...gle.com>
Subject: [PATCH v2 16/19] gendwarfksyms: Add support for reserved structure fields

Distributions that want to maintain a stable kABI need the ability to
add reserved fields to kernel data structures that they anticipate
will be modified during the ABI support timeframe, either by LTS
updates or backports.

With genksyms, developers would typically hide changes to the reserved
fields from version calculation with #ifndef __GENKSYMS__, which would
result in the symbol version not changing even though the actual type
of the reserved field changes. When we process precompiled object
files, this is again not an option.

To support stable symbol versions for reserved fields, change the
union type processing to recognize field name prefixes, and if the
union contains a field name that starts with __kabi_reserved, only use
the type of that field for computing symbol versions. In other words,
let's assume we have a structure where we want to reserve space for
future changes:

  struct struct1 {
    long a;
    long __kabi_reserved_0; /* reserved for future use */
  };
  struct struct1 exported;

gendwarfksyms --debug produces the following output:

  variable structure_type struct1 {
    member base_type long int byte_size(8) encoding(5) data_member_location(0),
    member base_type long int byte_size(8) encoding(5) data_member_location(8),
  } byte_size(16);
  #SYMVER exported 0x67997f89

To take the reserved field into use, a distribution would replace it
with a union, with one of the fields keeping the __kabi_reserved name
prefix for the original type:

  struct struct1 {
    long a;
    union {
      long __kabi_reserved_0;
      struct {
          int b;
          int v;
      };
    };

gendwarfksyms --debug now produces the following output:

  variable structure_type struct1 {
    member base_type long int byte_size(8) encoding(5) data_member_location(0),
    member union_type <unnamed> {
      member base_type long int byte_size(8) encoding(5),
      member structure_type <unnamed> {
        member base_type int byte_size(4) encoding(5) data_member_location(0),
        member base_type int byte_size(4) encoding(5) data_member_location(4),
      } byte_size(8),
    } byte_size(8) data_member_location(8),
  } byte_size(16);
  #SYMVER exported 0x66916c41

But with --stable, gendwarfksyms only uses the reserved field for the
version calculation, thus leaving the symbol version unchanged:

  variable structure_type struct1 {
    member base_type long int byte_size(8) encoding(5) data_member_location(0),
    member base_type long int byte_size(8) encoding(5) data_member_location(8),
  } byte_size(16);
  #SYMVER exported 0x67997f89

Signed-off-by: Sami Tolvanen <samitolvanen@...gle.com>
---
 scripts/gendwarfksyms/dwarf.c             | 148 +++++++++++++++++++++-
 scripts/gendwarfksyms/examples/reserved.c |  66 ++++++++++
 scripts/gendwarfksyms/gendwarfksyms.h     |  18 +++
 3 files changed, 229 insertions(+), 3 deletions(-)
 create mode 100644 scripts/gendwarfksyms/examples/reserved.c

diff --git a/scripts/gendwarfksyms/dwarf.c b/scripts/gendwarfksyms/dwarf.c
index bf28946c321e..d6252194692d 100644
--- a/scripts/gendwarfksyms/dwarf.c
+++ b/scripts/gendwarfksyms/dwarf.c
@@ -274,8 +274,12 @@ int process_die_container(struct state *state, struct die *cache,
 
 	res = checkp(dwarf_child(die, &current));
 	while (!res) {
-		if (match(&current))
-			check(func(state, cache, &current));
+		if (match(&current)) {
+			/* <0 = error, 0 = continue, >0 = stop */
+			res = checkp(func(state, cache, &current));
+			if (res)
+				return res;
+		}
 		res = checkp(dwarf_siblingof(&current, &current));
 	}
 
@@ -490,7 +494,145 @@ static int __process_structure_type(struct state *state, struct die *cache,
 
 DEFINE_PROCESS_STRUCTURE_TYPE(class)
 DEFINE_PROCESS_STRUCTURE_TYPE(structure)
-DEFINE_PROCESS_STRUCTURE_TYPE(union)
+
+static bool is_reserved_member(Dwarf_Die *die)
+{
+	const char *name = get_name(die);
+
+	return name && !strncmp(name, RESERVED_PREFIX, RESERVED_PREFIX_LEN);
+}
+
+static int __process_reserved_struct(struct state *state, struct die *cache,
+				     Dwarf_Die *die)
+{
+	Dwarf_Die type;
+
+	/*
+	 * If the union member is a struct, expect the placeholder type to
+	 * be the first member, i.e.:
+	 *
+	 * union {
+	 * 	type replaced_member;
+	 * 	struct {
+	 * 		type placeholder; // <- type
+	 * 	}
+	 * };
+	 *
+	 * Stop processing if this member isn't reserved.
+	 */
+	if (!is_reserved_member(die))
+		return NOT_RESERVED;
+
+	if (!get_ref_die_attr(die, DW_AT_type, &type)) {
+		error("structure member missing a type?");
+		return -1;
+	}
+
+	check(process_type(state, cache, &type));
+	return RESERVED;
+}
+
+static int __process_reserved_union(struct state *state, struct die *cache,
+				    Dwarf_Die *die)
+{
+	int res = NOT_RESERVED;
+	Dwarf_Die type;
+
+	if (!get_ref_die_attr(die, DW_AT_type, &type)) {
+		error("union member missing a type?");
+		return -1;
+	}
+
+	/*
+	 * We expect a union with two members. Check if either of them
+	 * has the reserved name prefix, i.e.:
+	 *
+	 * union {
+	 * 	...
+	 * 	type memberN; // <- type, N = {0,1}
+	 *	...
+	 * };
+	 *
+	 * The member can also be a structure type, in which case we'll
+	 * check the first structure member.
+	 *
+	 * Stop processing after we've seen two members.
+	 */
+	if (is_reserved_member(die)) {
+		check(process_type(state, cache, &type));
+		return RESERVED;
+	}
+
+	if (dwarf_tag(&type) == DW_TAG_structure_type)
+		res = checkp(process_die_container(state, cache, &type,
+						   __process_reserved_struct,
+						   match_member_type));
+	if (res != RESERVED && ++state->reserved.members < 2)
+		return 0; /* Continue */
+
+	return res;
+}
+
+static int process_reserved(struct state *state, struct die *cache,
+			    Dwarf_Die *die)
+{
+	if (!stable)
+		return NOT_RESERVED;
+
+	/*
+	 * To maintain a stable kABI, distributions may choose to reserve
+	 * space in structs for later use by adding placeholder members,
+	 * for example:
+	 *
+	 * struct s {
+	 * 	u64 a;
+	 *	// placeholder
+	 * 	u64 __kabi_reserved_0;
+	 * };
+	 *
+	 * When the reserved member is taken into use, the type change
+	 * would normally cause the symbol version to change as well, but
+	 * if the replacement uses the following convention, gendwarfksyms
+	 * continues to use the placeholder type for versioning instead,
+	 * thus maintaining the same symbol version:
+	 *
+	 * struct s {
+	 * 	u64 a;
+	 *	union {
+	 * 		// replaced member
+	 * 		struct t b;
+	 * 		struct {
+	 * 			// original placeholder
+	 * 			u64 __kabi_reserved_0;
+	 * 		};
+	 * 	};
+	 * };
+	 *
+	 * I.e., as long as the replaced member is in a union, and the
+	 * placeholder has a __kabi_reserved name prefix, we'll continue
+	 * to use the placeholder type (here u64) for version calculation
+	 * instead of the union type.
+	 *
+	 * Note that the user is responsible for ensuring that the
+	 * replacement type is ABI compatible with the placeholder type.
+	 */
+	state->reserved.members = 0;
+
+	return checkp(process_die_container(state, cache, die,
+					    __process_reserved_union,
+					    match_member_type));
+}
+
+static int process_union_type(struct state *state, struct die *cache,
+			      Dwarf_Die *die)
+{
+	if (checkp(process_reserved(state, cache, die)) == RESERVED)
+		return 0;
+
+	return check(__process_structure_type(state, cache, die, "union_type ",
+					      ___process_structure_type,
+					      match_all));
+}
 
 static int process_enumerator_type(struct state *state, struct die *cache,
 				   Dwarf_Die *die)
diff --git a/scripts/gendwarfksyms/examples/reserved.c b/scripts/gendwarfksyms/examples/reserved.c
new file mode 100644
index 000000000000..1e8de7ccd7d2
--- /dev/null
+++ b/scripts/gendwarfksyms/examples/reserved.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Google LLC
+ *
+ * Reserved data structure field example. See dwarf.c:process_reserved
+ * for details.
+ *
+ * $ gcc -g -c examples/reserved.c
+ *
+ * With --stable, only the reserved field placeholder is used for calculating
+ * symbol versions.
+ *
+ * $ echo exported0 | ./gendwarfksyms --stable --dump-dies reserved.o
+ * variable structure_type struct0 {
+ *   member base_type int byte_size(4) encoding(5) data_member_location(0),
+ *   member base_type long int byte_size(8) encoding(5) data_member_location(8),
+ * } byte_size(16)
+ *
+ * $ echo exported1 | ./gendwarfksyms --stable --dump-dies reserved.o
+ * variable structure_type struct1 {
+ *   member base_type int byte_size(4) encoding(5) data_member_location(0),
+ *   member base_type long int byte_size(8) encoding(5) data_member_location(8),
+ * } byte_size(16)
+ *
+ * $ echo exported2 | ./gendwarfksyms --stable --dump-dies reserved.o
+ * variable structure_type struct2 {
+ *   member base_type int byte_size(4) encoding(5) data_member_location(0),
+ *   member base_type long int byte_size(8) encoding(5) data_member_location(8),
+ * } byte_size(16)
+ */
+
+struct struct0 {
+	int a;
+	union {
+		long __kabi_reserved_0;
+		struct {
+			int b;
+			int c;
+		};
+	};
+};
+
+struct struct1 {
+	int a;
+	union {
+		struct {
+			int b;
+			int c;
+		};
+		long __kabi_reserved_1;
+	};
+};
+
+struct struct2 {
+	int a;
+	union {
+		unsigned long b;
+		struct {
+			long __kabi_reserved_1;
+		};
+	};
+};
+
+struct struct0 exported0;
+struct struct1 exported1;
+struct struct2 exported2;
diff --git a/scripts/gendwarfksyms/gendwarfksyms.h b/scripts/gendwarfksyms/gendwarfksyms.h
index 05b5c01b1c2a..963a07167892 100644
--- a/scripts/gendwarfksyms/gendwarfksyms.h
+++ b/scripts/gendwarfksyms/gendwarfksyms.h
@@ -220,12 +220,27 @@ extern void cache_clear_expanded(struct expansion_cache *ec);
 /*
  * dwarf.c
  */
+
+/* See dwarf.c:process_reserved */
+#define RESERVED_PREFIX "__kabi_reserved"
+#define RESERVED_PREFIX_LEN (sizeof(RESERVED_PREFIX) - 1)
+
 struct expansion_state {
 	bool expand;
 	bool in_pointer_type;
 	unsigned int ptr_expansion_depth;
 };
 
+enum reserved_status {
+	/* >0 to stop DIE processing */
+	NOT_RESERVED = 1,
+	RESERVED
+};
+
+struct reserved_state {
+	int members;
+};
+
 struct state {
 	Dwfl_Module *mod;
 	Dwarf *dbg;
@@ -235,6 +250,9 @@ struct state {
 	/* Structure expansion */
 	struct expansion_state expand;
 	struct expansion_cache expansion_cache;
+
+	/* Reserved members */
+	struct reserved_state reserved;
 };
 
 typedef int (*die_callback_t)(struct state *state, struct die *cache,
-- 
2.46.0.184.g6999bdac58-goog

Powered by blists - more mailing lists