Message-ID: <20240815173903.4172139-36-samitolvanen@google.com>
Date: Thu, 15 Aug 2024 17:39:19 +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 15/19] gendwarfksyms: Add support for declaration-only data structures

If a source file refers to an opaque data structure, the DWARF
debugging information in the resulting object file will only contain a
structure declation, which means the contents of the structure are not
included when computing symbol versions. For example:

Source code:

  struct struct0;
  int func(struct struct0 *);

gendwarfksyms --debug:

  subprogram(
    formal_parameter pointer_type <unnamed> {
      structure_type struct0 {
        <declaration>
      }
    } byte_size(8),
  )
  -> base_type int byte_size(4) encoding(5);
  #SYMVER func 0x7e8284f9

The declaration can change into a full definition when an additional
include is added to the TU, which changes the version CRC, even though
the ABI has not changed. If this happens during an LTS update, a
distribution that wants to maintain a stable ABI needs a way to ensure
symbol versions remain unchanged.

With genksyms, the usual solution is to use #ifndef __GENKSYMS__ to
skip the newly added header file when computing symbol versions, but
that's not an option when we're processing a precompiled object file.

To support this use case, add a --stable command line flag that gates
kABI stability features that are not needed in mainline, but can be
useful for distributions, and add support for symbol annotations that
allow structures to always be treated as declarations when versions
are computed. If a __gendwarfksyms_declonly_<structname> symbol
exists in the object file's symbol table, the "structname" structure
is treated as a declaration only, and not expanded when computing
versions. The symbol can be defined using the following macro, for
example, which discards it from the final kernel binary:

  #define GENDWARFKSYMS_DECLONLY(structname) \
      static void *__gendwarfksyms_declonly_##structname __used \
        __section(".discard.gendwarfksyms")

Now, if we include struct0 definition in our source code, and add a
declaration-only annotation, we have:

  struct struct0 { int a; };
  GENDWARFKSYMS_DECLONLY(struct0);
  int func(struct struct0 *);

gendwarfksyms --debug reflects the added definition:

  subprogram(
    formal_parameter pointer_type <unnamed> {
      structure_type struct0 {
        member base_type int byte_size(4) encoding(5) data_member_location(0),
      } byte_size(4)
    } byte_size(8),
  )
  -> base_type int byte_size(4) encoding(5);
  #SYMVER func 0xc0de983d

But with --debug --stable, the definition is ignored and we again
have the original symbol version:

  subprogram(
    formal_parameter pointer_type <unnamed> {
      structure_type struct0 {
        <declaration>
      }
    } byte_size(8),
  )
  -> base_type int byte_size(4) encoding(5);
  #SYMVER func 0x7e8284f9

Signed-off-by: Sami Tolvanen <samitolvanen@...gle.com>
---
 scripts/gendwarfksyms/dwarf.c             | 25 +++++++++-
 scripts/gendwarfksyms/examples/declonly.c | 31 ++++++++++++
 scripts/gendwarfksyms/gendwarfksyms.c     |  4 ++
 scripts/gendwarfksyms/gendwarfksyms.h     |  4 ++
 scripts/gendwarfksyms/symbols.c           | 61 +++++++++++++++++++++++
 5 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 scripts/gendwarfksyms/examples/declonly.c

diff --git a/scripts/gendwarfksyms/dwarf.c b/scripts/gendwarfksyms/dwarf.c
index 677190ae18ef..bf28946c321e 100644
--- a/scripts/gendwarfksyms/dwarf.c
+++ b/scripts/gendwarfksyms/dwarf.c
@@ -74,7 +74,30 @@ static bool is_declaration(Dwarf_Die *die)
 {
 	bool value;
 
-	return get_flag_attr(die, DW_AT_declaration, &value) && value;
+	/*
+	 * If the object file has a __gendwarfksyms_declonly_<structname>
+	 * symbol, we treat structures named "structname" as declarations,
+	 * i.e. they won't be expanded when calculating symbol versions.
+	 * This helps distributions maintain a stable kABI e.g., if extra
+	 * includes change a declaration into a definition.
+	 *
+	 * A simple way to mark a structure declaration-only in the source
+	 * code is to define a discarded symbol:
+	 *
+	 * #define GENDWARFKSYMS_DECLONLY(structname) \
+	 *   static void *__gendwarfksyms_declonly_##structname __used \
+	 *     __section(".discard.gendwarfksyms")
+	 *
+	 *  For example:
+	 *
+	 *  struct struct0 { int a; }
+	 *  GENDWARFKSYMS_DECLONLY(struct0);
+	 *
+	 *  Here, struct0 would be always be considered a declaration even
+	 *  though the definition is visible.
+	 */
+	return (get_flag_attr(die, DW_AT_declaration, &value) && value) ||
+	       is_struct_declonly(get_name(die));
 }
 
 /*
diff --git a/scripts/gendwarfksyms/examples/declonly.c b/scripts/gendwarfksyms/examples/declonly.c
new file mode 100644
index 000000000000..b1b889e582da
--- /dev/null
+++ b/scripts/gendwarfksyms/examples/declonly.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Google LLC
+ *
+ * Declaration-only data structure example. See dwarf.c:is_declaration
+ * for details.
+ *
+ * $ gcc -g -c examples/declonly.c
+ * $ echo exported | ./gendwarfksyms --dump-dies declonly.o
+ * variable structure_type struct0 {
+ *   member base_type int byte_size(4) encoding(5) data_member_location(0),
+ * } byte_size(4)
+ *
+ * With --stable, struct0 is treated as a declaration:
+ *
+ * $ echo exported | ./gendwarfksyms --stable --dump-dies declonly.o
+ * variable structure_type struct0 {
+ * }
+ */
+
+#define GENDWARFKSYMS_DECLONLY(structname) \
+	static void *__gendwarfksyms_declonly_##structname \
+		__attribute__((__used__)) \
+		__attribute__((__section__(".discard.gendwarfksyms")));
+
+struct struct0 {
+	int a;
+};
+
+struct struct0 exported;
+GENDWARFKSYMS_DECLONLY(struct0);
diff --git a/scripts/gendwarfksyms/gendwarfksyms.c b/scripts/gendwarfksyms/gendwarfksyms.c
index 4a160d19d7df..10d1470383be 100644
--- a/scripts/gendwarfksyms/gendwarfksyms.c
+++ b/scripts/gendwarfksyms/gendwarfksyms.c
@@ -24,6 +24,8 @@ bool dump_die_map;
 bool dump_types;
 /* Print out expanded type strings used for version calculations */
 bool dump_versions;
+/* Support kABI stability features */
+bool stable;
 /* Produce a symtypes file */
 bool symtypes;
 static const char *symtypes_file;
@@ -38,6 +40,7 @@ static const struct {
 	{ "--dump-die-map", &dump_die_map, NULL },
 	{ "--dump-types", &dump_types, NULL },
 	{ "--dump-versions", &dump_versions, NULL },
+	{ "--stable", &stable, NULL },
 	{ "--symtypes", &symtypes, &symtypes_file },
 };
 
@@ -186,6 +189,7 @@ int main(int argc, const char **argv)
 
 		dwfl_end(dwfl);
 		close(fd);
+		symbol_free_declonly();
 	}
 
 	if (symfile)
diff --git a/scripts/gendwarfksyms/gendwarfksyms.h b/scripts/gendwarfksyms/gendwarfksyms.h
index f85e080a8634..05b5c01b1c2a 100644
--- a/scripts/gendwarfksyms/gendwarfksyms.h
+++ b/scripts/gendwarfksyms/gendwarfksyms.h
@@ -24,6 +24,7 @@ extern bool dump_dies;
 extern bool dump_die_map;
 extern bool dump_types;
 extern bool dump_versions;
+extern bool stable;
 extern bool symtypes;
 
 #define MAX_INPUT_FILES 128
@@ -134,6 +135,9 @@ extern int symbol_set_crc(struct symbol *sym, unsigned long crc);
 extern int symbol_for_each(symbol_callback_t func, void *arg);
 extern void symbol_print_versions(void);
 
+extern bool is_struct_declonly(const char *name);
+extern void symbol_free_declonly(void);
+
 /*
  * die.c
  */
diff --git a/scripts/gendwarfksyms/symbols.c b/scripts/gendwarfksyms/symbols.c
index c9889cfa89c4..03ba93ef9eac 100644
--- a/scripts/gendwarfksyms/symbols.c
+++ b/scripts/gendwarfksyms/symbols.c
@@ -5,12 +5,20 @@
 
 #include "gendwarfksyms.h"
 
+struct declonly {
+	const char *name;
+	struct hlist_node hash;
+};
+
 #define SYMBOL_HASH_BITS 15
+#define DECLONLY_HASH_BITS 10
 
 /* struct symbol_addr -> struct symbol */
 static DEFINE_HASHTABLE(symbol_addrs, SYMBOL_HASH_BITS);
 /* name -> struct symbol */
 static DEFINE_HASHTABLE(symbol_names, SYMBOL_HASH_BITS);
+/* name -> struct declonly */
+static DEFINE_HASHTABLE(declonly_structs, DECLONLY_HASH_BITS);
 
 static inline u32 symbol_addr_hash(const struct symbol_addr *addr)
 {
@@ -296,12 +304,36 @@ static int process_symbol(const char *name, GElf_Sym *sym, Elf32_Word xndx,
 			  void *arg)
 {
 	struct symbol_addr addr = { .section = xndx, .address = sym->st_value };
+	struct declonly *d;
 
 	/* Set addresses for exported symbols */
 	if (GELF_ST_BIND(sym->st_info) != STB_LOCAL &&
 	    addr.section != SHN_UNDEF)
 		checkp(for_each(name, true, set_symbol_addr, &addr));
 
+	if (!stable)
+		return 0;
+
+	/* Process declonly structs */
+	if (strncmp(name, SYMBOL_DECLONLY_PREFIX, SYMBOL_DECLONLY_PREFIX_LEN))
+		return 0;
+
+	d = malloc(sizeof(struct declonly));
+	if (!d) {
+		error("malloc failed");
+		return -1;
+	}
+
+	name += SYMBOL_DECLONLY_PREFIX_LEN;
+	d->name = strdup(name);
+	if (!d->name) {
+		error("strdup failed");
+		return -1;
+	}
+
+	hash_add(declonly_structs, &d->hash, name_hash(d->name));
+	debug("declaration-only: %s", d->name);
+
 	return 0;
 }
 
@@ -310,6 +342,35 @@ int symbol_read_symtab(int fd)
 	return elf_for_each_symbol(fd, process_symbol, NULL);
 }
 
+bool is_struct_declonly(const char *name)
+{
+	struct declonly *d;
+
+	if (!stable || !name)
+		return false;
+
+	hash_for_each_possible(declonly_structs, d, hash, name_hash(name)) {
+		if (!strcmp(name, d->name))
+			return true;
+	}
+
+	return false;
+}
+
+void symbol_free_declonly(void)
+{
+	struct hlist_node *tmp;
+	struct declonly *d;
+	int i;
+
+	hash_for_each_safe(declonly_structs, i, tmp, d, hash) {
+		free((void *)d->name);
+		free(d);
+	}
+
+	hash_init(declonly_structs);
+}
+
 void symbol_print_versions(void)
 {
 	struct hlist_node *tmp;
-- 
2.46.0.184.g6999bdac58-goog

Powered by blists - more mailing lists