linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] gendwarfksyms: Add more kABI rules
@ 2025-04-30 21:40 Sami Tolvanen
  2025-04-30 21:40 ` [PATCH 1/4] gendwarfksyms: Clean up kABI rule look-ups Sami Tolvanen
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Sami Tolvanen @ 2025-04-30 21:40 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, linux-modules,
	linux-kbuild, linux-kernel, Sami Tolvanen

Hi,

While looking deeper into the genksyms hacks that have been
needed during Android's 5.10 and 5.15 GKI lifecycles so far,
we found a few instances that are not covered by the existing
gendwarfksyms kABI stability rules. The first case involved
appending new members to a partially opaque data structure, and
the second case completely changing opaque types due to a large
backport that was necessary for both stability and security.

These patches add rules that allow distribution maintainers
to deal with structure size changes, and as a last resort, to
completely override a type string used for version calculation.

Sami


---

Sami Tolvanen (4):
  gendwarfksyms: Clean up kABI rule look-ups
  gendwarfksyms: Add a kABI rule to override byte_size attributes
  gendwarfksyms: Add a kABI rule to override type strings
  Documentation/kbuild: Add new gendwarfksyms kABI rules

 Documentation/kbuild/gendwarfksyms.rst   | 103 ++++++++++++++--
 scripts/gendwarfksyms/dwarf.c            |  14 ++-
 scripts/gendwarfksyms/examples/kabi.h    |  21 +++-
 scripts/gendwarfksyms/examples/kabi_ex.c |   7 ++
 scripts/gendwarfksyms/examples/kabi_ex.h | 101 +++++++++++++++-
 scripts/gendwarfksyms/gendwarfksyms.h    |   2 +
 scripts/gendwarfksyms/kabi.c             | 143 ++++++++++++++---------
 scripts/gendwarfksyms/types.c            | 141 +++++++++++++++++++---
 8 files changed, 452 insertions(+), 80 deletions(-)


base-commit: b4432656b36e5cc1d50a1f2dc15357543add530e
-- 
2.49.0.906.g1f30a19c02-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] gendwarfksyms: Clean up kABI rule look-ups
  2025-04-30 21:40 [PATCH 0/4] gendwarfksyms: Add more kABI rules Sami Tolvanen
@ 2025-04-30 21:40 ` Sami Tolvanen
  2025-05-05 11:56   ` Petr Pavlu
  2025-04-30 21:40 ` [PATCH 2/4] gendwarfksyms: Add a kABI rule to override byte_size attributes Sami Tolvanen
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Sami Tolvanen @ 2025-04-30 21:40 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, linux-modules,
	linux-kbuild, linux-kernel, Sami Tolvanen

Reduce code duplication by moving kABI rule look-ups to separate
functions.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 scripts/gendwarfksyms/kabi.c | 101 +++++++++++++++--------------------
 1 file changed, 44 insertions(+), 57 deletions(-)

diff --git a/scripts/gendwarfksyms/kabi.c b/scripts/gendwarfksyms/kabi.c
index 66f01fcd1607..badf8d46b154 100644
--- a/scripts/gendwarfksyms/kabi.c
+++ b/scripts/gendwarfksyms/kabi.c
@@ -222,33 +222,55 @@ void kabi_read_rules(int fd)
 	check(elf_end(elf));
 }
 
-bool kabi_is_declonly(const char *fqn)
+static char *get_enumerator_target(const char *fqn, const char *field)
+{
+	char *target = NULL;
+
+	if (asprintf(&target, "%s %s", fqn, field) < 0)
+		error("asprintf failed for '%s %s'", fqn, field);
+
+	return target;
+}
+
+static struct rule *find_rule(enum kabi_rule_type type, const char *target)
 {
 	struct rule *rule;
 
 	if (!stable)
-		return false;
-	if (!fqn || !*fqn)
-		return false;
+		return NULL;
+	if (!target || !*target)
+		return NULL;
 
 	hash_for_each_possible(rules, rule, hash,
-			       rule_values_hash(KABI_RULE_TYPE_DECLONLY, fqn)) {
-		if (rule->type == KABI_RULE_TYPE_DECLONLY &&
-		    !strcmp(fqn, rule->target))
-			return true;
+			       rule_values_hash(type, target)) {
+		if (rule->type == type && !strcmp(target, rule->target))
+			return rule;
 	}
 
-	return false;
+	return NULL;
 }
 
-static char *get_enumerator_target(const char *fqn, const char *field)
+static struct rule *find_enumerator_rule(enum kabi_rule_type type,
+					 const char *fqn, const char *field)
 {
-	char *target = NULL;
+	struct rule *rule;
+	char *target;
 
-	if (asprintf(&target, "%s %s", fqn, field) < 0)
-		error("asprintf failed for '%s %s'", fqn, field);
+	if (!stable)
+		return NULL;
+	if (!fqn || !*fqn || !field || !*field)
+		return NULL;
 
-	return target;
+	target = get_enumerator_target(fqn, field);
+	rule = find_rule(type, target);
+
+	free(target);
+	return rule;
+}
+
+bool kabi_is_declonly(const char *fqn)
+{
+	return !!find_rule(KABI_RULE_TYPE_DECLONLY, fqn);
 }
 
 static unsigned long get_ulong_value(const char *value)
@@ -267,58 +289,23 @@ static unsigned long get_ulong_value(const char *value)
 
 bool kabi_is_enumerator_ignored(const char *fqn, const char *field)
 {
-	bool match = false;
-	struct rule *rule;
-	char *target;
-
-	if (!stable)
-		return false;
-	if (!fqn || !*fqn || !field || !*field)
-		return false;
-
-	target = get_enumerator_target(fqn, field);
-
-	hash_for_each_possible(
-		rules, rule, hash,
-		rule_values_hash(KABI_RULE_TYPE_ENUMERATOR_IGNORE, target)) {
-		if (rule->type == KABI_RULE_TYPE_ENUMERATOR_IGNORE &&
-		    !strcmp(target, rule->target)) {
-			match = true;
-			break;
-		}
-	}
-
-	free(target);
-	return match;
+	return !!find_enumerator_rule(KABI_RULE_TYPE_ENUMERATOR_IGNORE, fqn,
+				      field);
 }
 
 bool kabi_get_enumerator_value(const char *fqn, const char *field,
 			       unsigned long *value)
 {
-	bool match = false;
 	struct rule *rule;
-	char *target;
 
-	if (!stable)
-		return false;
-	if (!fqn || !*fqn || !field || !*field)
-		return false;
-
-	target = get_enumerator_target(fqn, field);
-
-	hash_for_each_possible(rules, rule, hash,
-			       rule_values_hash(KABI_RULE_TYPE_ENUMERATOR_VALUE,
-						target)) {
-		if (rule->type == KABI_RULE_TYPE_ENUMERATOR_VALUE &&
-		    !strcmp(target, rule->target)) {
-			*value = get_ulong_value(rule->value);
-			match = true;
-			break;
-		}
+	rule = find_enumerator_rule(KABI_RULE_TYPE_ENUMERATOR_VALUE, fqn,
+				    field);
+	if (rule) {
+		*value = get_ulong_value(rule->value);
+		return true;
 	}
 
-	free(target);
-	return match;
+	return false;
 }
 
 void kabi_free(void)
-- 
2.49.0.906.g1f30a19c02-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] gendwarfksyms: Add a kABI rule to override byte_size attributes
  2025-04-30 21:40 [PATCH 0/4] gendwarfksyms: Add more kABI rules Sami Tolvanen
  2025-04-30 21:40 ` [PATCH 1/4] gendwarfksyms: Clean up kABI rule look-ups Sami Tolvanen
@ 2025-04-30 21:40 ` Sami Tolvanen
  2025-05-05 11:57   ` Petr Pavlu
  2025-04-30 21:40 ` [PATCH 3/4] gendwarfksyms: Add a kABI rule to override type strings Sami Tolvanen
  2025-04-30 21:40 ` [PATCH 4/4] Documentation/kbuild: Add new gendwarfksyms kABI rules Sami Tolvanen
  3 siblings, 1 reply; 9+ messages in thread
From: Sami Tolvanen @ 2025-04-30 21:40 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, linux-modules,
	linux-kbuild, linux-kernel, Sami Tolvanen

A data structure can be partially opaque to modules if its
allocation is handled by the core kernel, and modules only need
to access some of its members. In this situation, it's possible
to append new members to the structure without breaking the ABI,
as long as the layout for the original members remains unchanged.
For example, consider the following struct:

  struct s {
          unsigned long a;
          void *p;
  };

gendwarfksyms --stable --dump-dies produces the following type
expansion:

  variable structure_type s {
    member base_type long unsigned int byte_size(8) encoding(7) a
      data_member_location(0) ,
    member pointer_type {
      base_type void
    } byte_size(8) p data_member_location(8)
  } byte_size(16)

To append new members, we can use the KABI_IGNORE() macro to
hide them from gendwarfksyms --stable:

  struct s {
          /* old members with unchanged layout */
          unsigned long a;
          void *p;

          /* new members not accessed by modules */
          KABI_IGNORE(0, unsigned long n);
  };

However, we can't hide the fact that adding new members changes
the struct size, as seen in the updated type string:

  variable structure_type s {
    member base_type long unsigned int byte_size(8) encoding(7) a
      data_member_location(0) ,
    member pointer_type {
      base_type void
    } byte_size(8) p data_member_location(8)
  } byte_size(24)

In order to support this use case, add a kABI rule that makes it
possible to override the byte_size attribute for types:

  /*
   * struct s allocation is handled by the kernel, so
   * appending new members without changing the original
   * layout won't break the ABI.
   */
  KABI_BYTE_SIZE(s, 16);

This results in a type string that's unchanged from the original
and therefore, won't change versions for symbols that reference
the changed structure.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 scripts/gendwarfksyms/dwarf.c            | 14 ++++++++++++-
 scripts/gendwarfksyms/examples/kabi.h    |  7 +++++++
 scripts/gendwarfksyms/examples/kabi_ex.c |  2 ++
 scripts/gendwarfksyms/examples/kabi_ex.h | 22 +++++++++++++++++++++
 scripts/gendwarfksyms/gendwarfksyms.h    |  1 +
 scripts/gendwarfksyms/kabi.c             | 25 ++++++++++++++++++++++++
 6 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/scripts/gendwarfksyms/dwarf.c b/scripts/gendwarfksyms/dwarf.c
index eed247d8abfc..13ea7bf1ae7d 100644
--- a/scripts/gendwarfksyms/dwarf.c
+++ b/scripts/gendwarfksyms/dwarf.c
@@ -228,12 +228,24 @@ static void process_fqn(struct die *cache, Dwarf_Die *die)
 DEFINE_PROCESS_UDATA_ATTRIBUTE(accessibility)
 DEFINE_PROCESS_UDATA_ATTRIBUTE(alignment)
 DEFINE_PROCESS_UDATA_ATTRIBUTE(bit_size)
-DEFINE_PROCESS_UDATA_ATTRIBUTE(byte_size)
 DEFINE_PROCESS_UDATA_ATTRIBUTE(encoding)
 DEFINE_PROCESS_UDATA_ATTRIBUTE(data_bit_offset)
 DEFINE_PROCESS_UDATA_ATTRIBUTE(data_member_location)
 DEFINE_PROCESS_UDATA_ATTRIBUTE(discr_value)
 
+static void process_byte_size_attr(struct die *cache, Dwarf_Die *die)
+{
+	Dwarf_Word value;
+	unsigned long override;
+
+	if (get_udata_attr(die, DW_AT_byte_size, &value)) {
+		if (stable && kabi_get_byte_size(cache->fqn, &override))
+			value = override;
+
+		process_fmt(cache, " byte_size(%" PRIu64 ")", value);
+	}
+}
+
 /* Match functions -- die_match_callback_t */
 #define DEFINE_MATCH(type)                                     \
 	static bool match_##type##_type(Dwarf_Die *die)        \
diff --git a/scripts/gendwarfksyms/examples/kabi.h b/scripts/gendwarfksyms/examples/kabi.h
index 97a5669b083d..86f4428e0479 100644
--- a/scripts/gendwarfksyms/examples/kabi.h
+++ b/scripts/gendwarfksyms/examples/kabi.h
@@ -89,6 +89,13 @@
 #define KABI_ENUMERATOR_VALUE(fqn, field, value) \
 	__KABI_RULE(enumerator_value, fqn field, value)
 
+/*
+ * KABI_BYTE_SIZE(fqn, value)
+ *   Set the byte_size attribute for the struct/union/enum fqn to
+ *   value bytes.
+ */
+#define KABI_BYTE_SIZE(fqn, value) __KABI_RULE(byte_size, fqn, value)
+
 /*
  * KABI_RESERVE
  *   Reserve some "padding" in a structure for use by LTS backports.
diff --git a/scripts/gendwarfksyms/examples/kabi_ex.c b/scripts/gendwarfksyms/examples/kabi_ex.c
index 0b7ffd830541..b73ee5399a59 100644
--- a/scripts/gendwarfksyms/examples/kabi_ex.c
+++ b/scripts/gendwarfksyms/examples/kabi_ex.c
@@ -28,3 +28,5 @@ struct ex2c ex2c;
 struct ex3a ex3a;
 struct ex3b ex3b;
 struct ex3c ex3c;
+
+struct ex4a ex4a;
diff --git a/scripts/gendwarfksyms/examples/kabi_ex.h b/scripts/gendwarfksyms/examples/kabi_ex.h
index 1736e0f65208..092c8cb7bcd7 100644
--- a/scripts/gendwarfksyms/examples/kabi_ex.h
+++ b/scripts/gendwarfksyms/examples/kabi_ex.h
@@ -260,4 +260,26 @@ _Static_assert(sizeof(struct ex3a) == sizeof(struct ex3c), "ex3a size doesn't ma
  * STABLE-NEXT: } byte_size(16)
  */
 
+/*
+ * Example: An ignored field added to an end of a partially opaque struct,
+ * while keeping the byte_size attribute unchanged.
+ */
+
+struct ex4a {
+	unsigned long a;
+	KABI_IGNORE(0, unsigned long b);
+};
+
+/*
+ * This may be safe if the structure allocation is managed by the core kernel
+ * and the layout remains unchanged except for appended new members.
+ */
+KABI_BYTE_SIZE(ex4a, 8);
+
+/*
+ * STABLE:      variable structure_type ex4a {
+ * STABLE-NEXT:   member base_type [[ULONG]] byte_size(8) encoding(7) a data_member_location(0)
+ * STABLE-NEXT: } byte_size(8)
+ */
+
 #endif /* __KABI_EX_H__ */
diff --git a/scripts/gendwarfksyms/gendwarfksyms.h b/scripts/gendwarfksyms/gendwarfksyms.h
index 2feec168bf73..2db49c2ad50e 100644
--- a/scripts/gendwarfksyms/gendwarfksyms.h
+++ b/scripts/gendwarfksyms/gendwarfksyms.h
@@ -287,6 +287,7 @@ void generate_symtypes_and_versions(FILE *file);
  * kabi.c
  */
 
+bool kabi_get_byte_size(const char *fqn, unsigned long *value);
 bool kabi_is_enumerator_ignored(const char *fqn, const char *field);
 bool kabi_get_enumerator_value(const char *fqn, const char *field,
 			       unsigned long *value);
diff --git a/scripts/gendwarfksyms/kabi.c b/scripts/gendwarfksyms/kabi.c
index badf8d46b154..61620ff647bd 100644
--- a/scripts/gendwarfksyms/kabi.c
+++ b/scripts/gendwarfksyms/kabi.c
@@ -54,11 +54,19 @@
  */
 #define KABI_RULE_TAG_ENUMERATOR_VALUE "enumerator_value"
 
+/*
+ * Rule: byte_size
+ * - For the fqn_field in the target field, set the byte_size
+ *   attribute to the value in the value field.
+ */
+#define KABI_RULE_TAG_BYTE_SIZE "byte_size"
+
 enum kabi_rule_type {
 	KABI_RULE_TYPE_UNKNOWN,
 	KABI_RULE_TYPE_DECLONLY,
 	KABI_RULE_TYPE_ENUMERATOR_IGNORE,
 	KABI_RULE_TYPE_ENUMERATOR_VALUE,
+	KABI_RULE_TYPE_BYTE_SIZE,
 };
 
 #define RULE_HASH_BITS 7
@@ -127,6 +135,10 @@ void kabi_read_rules(int fd)
 			.type = KABI_RULE_TYPE_ENUMERATOR_VALUE,
 			.tag = KABI_RULE_TAG_ENUMERATOR_VALUE,
 		},
+		{
+			.type = KABI_RULE_TYPE_BYTE_SIZE,
+			.tag = KABI_RULE_TAG_BYTE_SIZE,
+		},
 	};
 
 	if (!stable)
@@ -308,6 +320,19 @@ bool kabi_get_enumerator_value(const char *fqn, const char *field,
 	return false;
 }
 
+bool kabi_get_byte_size(const char *fqn, unsigned long *value)
+{
+	struct rule *rule;
+
+	rule = find_rule(KABI_RULE_TYPE_BYTE_SIZE, fqn);
+	if (rule) {
+		*value = get_ulong_value(rule->value);
+		return true;
+	}
+
+	return false;
+}
+
 void kabi_free(void)
 {
 	struct hlist_node *tmp;
-- 
2.49.0.906.g1f30a19c02-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] gendwarfksyms: Add a kABI rule to override type strings
  2025-04-30 21:40 [PATCH 0/4] gendwarfksyms: Add more kABI rules Sami Tolvanen
  2025-04-30 21:40 ` [PATCH 1/4] gendwarfksyms: Clean up kABI rule look-ups Sami Tolvanen
  2025-04-30 21:40 ` [PATCH 2/4] gendwarfksyms: Add a kABI rule to override byte_size attributes Sami Tolvanen
@ 2025-04-30 21:40 ` Sami Tolvanen
  2025-05-05 12:17   ` Petr Pavlu
  2025-04-30 21:40 ` [PATCH 4/4] Documentation/kbuild: Add new gendwarfksyms kABI rules Sami Tolvanen
  3 siblings, 1 reply; 9+ messages in thread
From: Sami Tolvanen @ 2025-04-30 21:40 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, linux-modules,
	linux-kbuild, linux-kernel, Sami Tolvanen, Giuliano Procida

In rare situations where distributions must make significant
changes to otherwise opaque data structures that have
inadvertently been included in the published ABI, keeping
symbol versions stable using the existing kABI macros can
become tedious.

For example, Android decided to switch to a newer io_uring
implementation in the 5.10 GKI kernel "to resolve a huge number
of potential, and known, problems with the codebase," requiring
"horrible hacks" with genksyms:

  "A number of the io_uring structures get used in other core
  kernel structures, only as "opaque" pointers, so there is
  not any real ABI breakage.  But, due to the visibility of
  the structures going away, the CRC values of many scheduler
  variables and functions were changed."
    -- https://r.android.com/2425293

While these specific changes probably could have been hidden
from gendwarfksyms using the existing kABI macros, this may not
always be the case.

Add a last resort kABI rule that allows distribution
maintainers to fully override a type string for a symbol or a
type. Also add a more informative error message in case we find
a non-existent type references when calculating versions.

Suggested-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 scripts/gendwarfksyms/examples/kabi.h    |  14 ++-
 scripts/gendwarfksyms/examples/kabi_ex.c |   5 +
 scripts/gendwarfksyms/examples/kabi_ex.h |  79 ++++++++++++-
 scripts/gendwarfksyms/gendwarfksyms.h    |   1 +
 scripts/gendwarfksyms/kabi.c             |  25 ++++
 scripts/gendwarfksyms/types.c            | 141 ++++++++++++++++++++---
 6 files changed, 247 insertions(+), 18 deletions(-)

diff --git a/scripts/gendwarfksyms/examples/kabi.h b/scripts/gendwarfksyms/examples/kabi.h
index 86f4428e0479..170733a3fba4 100644
--- a/scripts/gendwarfksyms/examples/kabi.h
+++ b/scripts/gendwarfksyms/examples/kabi.h
@@ -37,11 +37,14 @@
 #define __stringify(x...) __stringify_1(x)
 #endif
 
-#define __KABI_RULE(hint, target, value)                             \
+#define ___KABI_RULE(hint, target, value)                            \
 	static const char __PASTE(__gendwarfksyms_rule_,             \
 				  __COUNTER__)[] __used __aligned(1) \
 		__section(".discard.gendwarfksyms.kabi_rules") =     \
-			"1\0" #hint "\0" #target "\0" #value
+			"1\0" #hint "\0" target "\0" value
+
+#define __KABI_RULE(hint, target, value) \
+	___KABI_RULE(hint, #target, #value)
 
 #define __KABI_NORMAL_SIZE_ALIGN(_orig, _new)                                             \
 	union {                                                                           \
@@ -96,6 +99,13 @@
  */
 #define KABI_BYTE_SIZE(fqn, value) __KABI_RULE(byte_size, fqn, value)
 
+/*
+ * KABI_TYPE_STRING(type, str)
+ *   For the given type, override the type string used in symtypes
+ *   output and version calculation with str.
+ */
+#define KABI_TYPE_STRING(type, str) ___KABI_RULE(type_string, type, str)
+
 /*
  * KABI_RESERVE
  *   Reserve some "padding" in a structure for use by LTS backports.
diff --git a/scripts/gendwarfksyms/examples/kabi_ex.c b/scripts/gendwarfksyms/examples/kabi_ex.c
index b73ee5399a59..1f799eb7c756 100644
--- a/scripts/gendwarfksyms/examples/kabi_ex.c
+++ b/scripts/gendwarfksyms/examples/kabi_ex.c
@@ -30,3 +30,8 @@ struct ex3b ex3b;
 struct ex3c ex3c;
 
 struct ex4a ex4a;
+
+struct ex5a ex5a;
+struct ex5b ex5b;
+
+int ex6a;
diff --git a/scripts/gendwarfksyms/examples/kabi_ex.h b/scripts/gendwarfksyms/examples/kabi_ex.h
index 092c8cb7bcd7..785b211d9c58 100644
--- a/scripts/gendwarfksyms/examples/kabi_ex.h
+++ b/scripts/gendwarfksyms/examples/kabi_ex.h
@@ -21,6 +21,12 @@
  * 	./gendwarfksyms --stable --dump-dies \
  * 		examples/kabi_ex.o 2>&1 >/dev/null | \
  * 	FileCheck examples/kabi_ex.h --check-prefix=STABLE
+
+ * $ nm examples/kabi_ex.o | awk '{ print $NF }' | \
+ * 	./gendwarfksyms --stable --dump-versions \
+ * 		examples/kabi_ex.o 2>&1 >/dev/null | \
+ * 	sort | \
+ * 	FileCheck examples/kabi_ex.h --check-prefix=VERSIONS
  */
 
 #ifndef __KABI_EX_H__
@@ -170,7 +176,7 @@ struct ex2a {
 /*
  * STABLE:      variable structure_type ex2a {
  * STABLE-NEXT:   member base_type int byte_size(4) encoding(5) a data_member_location(0) ,
- * STABLE-NEXT:   member base_type [[ULONG:long unsigned int|unsigned long]] byte_size(8) encoding(7) b data_member_location(8)
+ * STABLE-NEXT:   member base_type [[ULONG]] byte_size(8) encoding(7) b data_member_location(8)
  * STABLE-NEXT:   member base_type int byte_size(4) encoding(5) c data_member_location(16) ,
  * STABLE-NEXT:   member base_type [[ULONG]] byte_size(8) encoding(7) d data_member_location(24)
  * STABLE-NEXT: } byte_size(32)
@@ -227,7 +233,7 @@ struct ex3a {
 
 /*
  * STABLE:      variable structure_type ex3a {
- * STABLE-NEXT:   member base_type [[ULONG:long unsigned int|unsigned long]] byte_size(8) encoding(7) a data_member_location(0)
+ * STABLE-NEXT:   member base_type [[ULONG]] byte_size(8) encoding(7) a data_member_location(0)
  * STABLE-NEXT:   member base_type [[ULONG]] byte_size(8) encoding(7) unused data_member_location(8)
  * STABLE-NEXT: } byte_size(16)
  */
@@ -282,4 +288,73 @@ KABI_BYTE_SIZE(ex4a, 8);
  * STABLE-NEXT: } byte_size(8)
  */
 
+/*
+ * Example: A type string override.
+ */
+
+struct ex5a {
+	unsigned long a;
+};
+
+/*
+ * This may be safe if the structure is fully opaque to modules, even though
+ * its definition has inadvertently become part of the ABI.
+ */
+KABI_TYPE_STRING(
+	"s#ex5a",
+	"structure_type ex5a { member pointer_type { s#ex4a } byte_size(8) p data_member_location(0) } byte_size(8)");
+
+/*
+ * Make sure the fully expanded type string includes ex4a.
+ *
+ * VERSIONS:      ex5a variable structure_type ex5a {
+ * VERSIONS-SAME:   member pointer_type {
+ * VERSIONS-SAME:     structure_type ex4a {
+ * VERSIONS-SAME:       member base_type [[ULONG:long unsigned int|unsigned long]] byte_size(8) encoding(7) a data_member_location(0)
+ * VERSIONS-SAME:     } byte_size(8)
+ * VERSIONS-SAME:   } byte_size(8) p data_member_location(0)
+ * VERSIONS-SAME: } byte_size(8)
+ */
+
+/*
+ * Example: A type string definition for a non-existent type.
+ */
+
+struct ex5b {
+	unsigned long a;
+};
+
+/* Replace the type string for struct ex5b */
+KABI_TYPE_STRING(
+	"s#ex5b",
+	"structure_type ex5b { member pointer_type { s#ex5c } byte_size(8) p data_member_location(0) } byte_size(8)");
+
+/* Define a type string for a non-existent struct ex5c */
+KABI_TYPE_STRING(
+	"s#ex5c",
+	"structure_type ex5c { member base_type int byte_size(4) encoding(5) n data_member_location(0) } byte_size(8)");
+
+/*
+ * Make sure the fully expanded type string includes the definition for ex5c.
+ *
+ * VERSIONS:      ex5b variable structure_type ex5b {
+ * VERSIONS-SAME:   member pointer_type {
+ * VERSIONS-SAME:     structure_type ex5c {
+ * VERSIONS-SAME:       member base_type int byte_size(4) encoding(5) n data_member_location(0)
+ * VERSIONS-SAME:     } byte_size(8)
+ * VERSIONS-SAME:   } byte_size(8) p data_member_location(0)
+ * VERSIONS-SAME: } byte_size(8)
+ */
+
+/*
+ * Example: A type string override for a symbol.
+ */
+
+KABI_TYPE_STRING("ex6a", "variable s#ex5c");
+
+/*
+ * VERSIONS:      ex6a variable structure_type ex5c {
+ * VERSIONS-SAME:   member base_type int byte_size(4) encoding(5) n data_member_location(0)
+ * VERSIONS-SAME: } byte_size(8)
+ */
 #endif /* __KABI_EX_H__ */
diff --git a/scripts/gendwarfksyms/gendwarfksyms.h b/scripts/gendwarfksyms/gendwarfksyms.h
index 2db49c2ad50e..7dd03ffe0c5c 100644
--- a/scripts/gendwarfksyms/gendwarfksyms.h
+++ b/scripts/gendwarfksyms/gendwarfksyms.h
@@ -292,6 +292,7 @@ bool kabi_is_enumerator_ignored(const char *fqn, const char *field);
 bool kabi_get_enumerator_value(const char *fqn, const char *field,
 			       unsigned long *value);
 bool kabi_is_declonly(const char *fqn);
+bool kabi_get_type_string(const char *type, const char **str);
 
 void kabi_read_rules(int fd);
 void kabi_free(void);
diff --git a/scripts/gendwarfksyms/kabi.c b/scripts/gendwarfksyms/kabi.c
index 61620ff647bd..b3ade713778f 100644
--- a/scripts/gendwarfksyms/kabi.c
+++ b/scripts/gendwarfksyms/kabi.c
@@ -61,12 +61,20 @@
  */
 #define KABI_RULE_TAG_BYTE_SIZE "byte_size"
 
+/*
+ * Rule: type_string
+ * - For the type reference in the fqn field, use the type string
+ *   in the value field.
+ */
+#define KABI_RULE_TAG_TYPE_STRING "type_string"
+
 enum kabi_rule_type {
 	KABI_RULE_TYPE_UNKNOWN,
 	KABI_RULE_TYPE_DECLONLY,
 	KABI_RULE_TYPE_ENUMERATOR_IGNORE,
 	KABI_RULE_TYPE_ENUMERATOR_VALUE,
 	KABI_RULE_TYPE_BYTE_SIZE,
+	KABI_RULE_TYPE_TYPE_STRING,
 };
 
 #define RULE_HASH_BITS 7
@@ -139,6 +147,10 @@ void kabi_read_rules(int fd)
 			.type = KABI_RULE_TYPE_BYTE_SIZE,
 			.tag = KABI_RULE_TAG_BYTE_SIZE,
 		},
+		{
+			.type = KABI_RULE_TYPE_TYPE_STRING,
+			.tag = KABI_RULE_TAG_TYPE_STRING,
+		},
 	};
 
 	if (!stable)
@@ -333,6 +345,19 @@ bool kabi_get_byte_size(const char *fqn, unsigned long *value)
 	return false;
 }
 
+bool kabi_get_type_string(const char *type, const char **str)
+{
+	struct rule *rule;
+
+	rule = find_rule(KABI_RULE_TYPE_TYPE_STRING, type);
+	if (rule) {
+		*str = rule->value;
+		return true;
+	}
+
+	return false;
+}
+
 void kabi_free(void)
 {
 	struct hlist_node *tmp;
diff --git a/scripts/gendwarfksyms/types.c b/scripts/gendwarfksyms/types.c
index 6f37289104ff..0702569ff58c 100644
--- a/scripts/gendwarfksyms/types.c
+++ b/scripts/gendwarfksyms/types.c
@@ -100,7 +100,7 @@ static void type_expansion_append(struct type_expansion *type, const char *s,
 #define TYPE_HASH_BITS 12
 static HASHTABLE_DEFINE(type_map, 1 << TYPE_HASH_BITS);
 
-static int type_map_get(const char *name, struct type_expansion **res)
+static int __type_map_get(const char *name, struct type_expansion **res)
 {
 	struct type_expansion *e;
 
@@ -114,11 +114,12 @@ static int type_map_get(const char *name, struct type_expansion **res)
 	return -1;
 }
 
-static void type_map_add(const char *name, struct type_expansion *type)
+static struct type_expansion *type_map_add(const char *name,
+					   struct type_expansion *type)
 {
 	struct type_expansion *e;
 
-	if (type_map_get(name, &e)) {
+	if (__type_map_get(name, &e)) {
 		e = xmalloc(sizeof(struct type_expansion));
 		type_expansion_init(e);
 		e->name = xstrdup(name);
@@ -130,7 +131,7 @@ static void type_map_add(const char *name, struct type_expansion *type)
 	} else {
 		/* Use the longest available expansion */
 		if (type->len <= e->len)
-			return;
+			return e;
 
 		type_list_free(&e->expanded);
 
@@ -148,6 +149,34 @@ static void type_map_add(const char *name, struct type_expansion *type)
 		type_list_write(&e->expanded, stderr);
 		checkp(fputs("\n", stderr));
 	}
+
+	return e;
+}
+
+static void type_parse(const char *name, const char *str,
+		       struct type_expansion *type);
+
+static int type_map_get(const char *name, struct type_expansion **res)
+{
+	struct type_expansion type;
+	const char *override;
+
+	if (!__type_map_get(name, res))
+		return 0;
+
+	/*
+	 * If die_map didn't contain a type, we might still have
+	 * a type_string kABI rule that defines it.
+	 */
+	if (stable && kabi_get_type_string(name, &override)) {
+		type_expansion_init(&type);
+		type_parse(name, override, &type);
+		*res = type_map_add(name, &type);
+		type_expansion_free(&type);
+		return 0;
+	}
+
+	return -1;
 }
 
 static void type_map_write(FILE *file)
@@ -267,15 +296,18 @@ static char *get_type_name(struct die *cache)
 	return name;
 }
 
-static void __calculate_version(struct version *version, struct list_head *list)
+static void __calculate_version(struct version *version,
+				struct type_expansion *type)
 {
 	struct type_list_entry *entry;
 	struct type_expansion *e;
 
 	/* Calculate a CRC over an expanded type string */
-	list_for_each_entry(entry, list, list) {
+	list_for_each_entry(entry, &type->expanded, list) {
 		if (is_type_prefix(entry->str)) {
-			check(type_map_get(entry->str, &e));
+			if (type_map_get(entry->str, &e))
+				error("unknown type reference to '%s' when expanding '%s'",
+				      entry->str, type->name);
 
 			/*
 			 * It's sufficient to expand each type reference just
@@ -285,7 +317,7 @@ static void __calculate_version(struct version *version, struct list_head *list)
 				version_add(version, entry->str);
 			} else {
 				cache_mark_expanded(&expansion_cache, e);
-				__calculate_version(version, &e->expanded);
+				__calculate_version(version, e);
 			}
 		} else {
 			version_add(version, entry->str);
@@ -293,10 +325,11 @@ static void __calculate_version(struct version *version, struct list_head *list)
 	}
 }
 
-static void calculate_version(struct version *version, struct list_head *list)
+static void calculate_version(struct version *version,
+			      struct type_expansion *type)
 {
 	version_init(version);
-	__calculate_version(version, list);
+	__calculate_version(version, type);
 	cache_free(&expansion_cache);
 }
 
@@ -372,9 +405,81 @@ static void type_expand(struct die *cache, struct type_expansion *type,
 	cache_free(&expansion_cache);
 }
 
+static void type_parse(const char *name, const char *str,
+		       struct type_expansion *type)
+{
+	char *fragment;
+	size_t start = 0;
+	size_t end;
+	size_t pos;
+
+	if (!*str)
+		error("empty type string override for '%s'", name);
+
+	type_expansion_init(type);
+
+	for (pos = 1; str[pos]; ++pos) {
+		bool empty;
+		char marker = ' ';
+
+		if (!is_type_prefix(&str[pos - 1]))
+			continue;
+
+		end = pos + 1;
+
+		/*
+		 * Find the end of the type reference. If the type name contains
+		 * spaces, it must be in single quotes.
+		 */
+		if (str[end] == '\'') {
+			marker = '\'';
+			++end;
+		}
+		while (str[end] && str[end] != marker)
+			++end;
+
+		/* Check that we have a non-empty type name */
+		if (marker == '\'') {
+			if (str[end] != marker)
+				error("incomplete %c# type reference for '%s' (string : '%s')",
+				      str[pos - 1], name, str);
+			empty = end == pos + 2;
+			++end;
+		} else {
+			empty = end == pos + 1;
+		}
+		if (empty)
+			error("empty %c# type name for '%s' (string: '%s')",
+			      str[pos - 1], name, str);
+
+		/* Append the part of the string before the type reference */
+		if (pos > start + 1) {
+			fragment = xstrndup(&str[start], pos - start - 1);
+			type_expansion_append(type, fragment, fragment);
+		}
+
+		/*
+		 * Append the type reference -- note that if the reference
+		 * is invalid, i.e. points to a non-existent type, we will
+		 * print out an error when calculating versions.
+		 */
+		fragment = xstrndup(&str[pos - 1], end - pos + 1);
+		type_expansion_append(type, fragment, fragment);
+
+		pos = start = end;
+		if (!str[pos])
+			break;
+	}
+
+	/* Append the rest of the type string, if there's any left */
+	if (str[start])
+		type_expansion_append(type, &str[start], NULL);
+}
+
 static void expand_type(struct die *cache, void *arg)
 {
 	struct type_expansion type;
+	const char *override;
 	char *name;
 
 	if (cache->mapped)
@@ -399,9 +504,13 @@ static void expand_type(struct die *cache, void *arg)
 		return;
 
 	debug("%s", name);
-	type_expand(cache, &type, true);
-	type_map_add(name, &type);
 
+	if (stable && kabi_get_type_string(name, &override))
+		type_parse(name, override, &type);
+	else
+		type_expand(cache, &type, true);
+
+	type_map_add(name, &type);
 	type_expansion_free(&type);
 	free(name);
 }
@@ -410,6 +519,7 @@ static void expand_symbol(struct symbol *sym, void *arg)
 {
 	struct type_expansion type;
 	struct version version;
+	const char *override;
 	struct die *cache;
 
 	/*
@@ -423,11 +533,14 @@ static void expand_symbol(struct symbol *sym, void *arg)
 	if (__die_map_get(sym->die_addr, DIE_SYMBOL, &cache))
 		return; /* We'll warn about missing CRCs later. */
 
-	type_expand(cache, &type, false);
+	if (stable && kabi_get_type_string(sym->name, &override))
+		type_parse(sym->name, override, &type);
+	else
+		type_expand(cache, &type, false);
 
 	/* If the symbol already has a version, don't calculate it again. */
 	if (sym->state != SYMBOL_PROCESSED) {
-		calculate_version(&version, &type.expanded);
+		calculate_version(&version, &type);
 		symbol_set_crc(sym, version.crc);
 		debug("%s = %lx", sym->name, version.crc);
 
-- 
2.49.0.906.g1f30a19c02-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] Documentation/kbuild: Add new gendwarfksyms kABI rules
  2025-04-30 21:40 [PATCH 0/4] gendwarfksyms: Add more kABI rules Sami Tolvanen
                   ` (2 preceding siblings ...)
  2025-04-30 21:40 ` [PATCH 3/4] gendwarfksyms: Add a kABI rule to override type strings Sami Tolvanen
@ 2025-04-30 21:40 ` Sami Tolvanen
  3 siblings, 0 replies; 9+ messages in thread
From: Sami Tolvanen @ 2025-04-30 21:40 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, linux-modules,
	linux-kbuild, linux-kernel, Sami Tolvanen

Document byte_size and type_string kABI stability rules. Also fix
the section numbers while we're at it.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 Documentation/kbuild/gendwarfksyms.rst | 103 +++++++++++++++++++++++--
 1 file changed, 95 insertions(+), 8 deletions(-)

diff --git a/Documentation/kbuild/gendwarfksyms.rst b/Documentation/kbuild/gendwarfksyms.rst
index e4beaae7e456..8b0d7ebbb084 100644
--- a/Documentation/kbuild/gendwarfksyms.rst
+++ b/Documentation/kbuild/gendwarfksyms.rst
@@ -125,14 +125,17 @@ the rules. The fields are as follows:
   qualified name of the DWARF Debugging Information Entry (DIE).
 - `value`: Provides rule-specific data.
 
-The following helper macro, for example, can be used to specify rules
+The following helper macros, for example, can be used to specify rules
 in the source code::
 
-	#define __KABI_RULE(hint, target, value)                             \
-		static const char __PASTE(__gendwarfksyms_rule_,             \
+	#define ___KABI_RULE(hint, target, value)			    \
+		static const char __PASTE(__gendwarfksyms_rule_,	     \
 					  __COUNTER__)[] __used __aligned(1) \
 			__section(".discard.gendwarfksyms.kabi_rules") =     \
-				"1\0" #hint "\0" #target "\0" #value
+				"1\0" #hint "\0" target "\0" value
+
+	#define __KABI_RULE(hint, target, value) \
+		___KABI_RULE(hint, #target, #value)
 
 
 Currently, only the rules discussed in this section are supported, but
@@ -223,7 +226,88 @@ Example usage::
 	KABI_ENUMERATOR_IGNORE(e, C);
 	KABI_ENUMERATOR_VALUE(e, LAST, 2);
 
-4.3. Adding structure members
+4.1.3. Managing structure size changes
+======================================
+
+A data structure can be partially opaque to modules if its allocation is
+handled by the core kernel, and modules only need to access some of its
+members. In this situation, it's possible to append new members to the
+structure without breaking the ABI, as long as the layout for the original
+members remains unchanged.
+
+To append new members, we can hide them from symbol versioning as
+described in section :ref:`Hiding members <hiding_members>`, but we can't
+hide the increase in structure size. The `byte_size` rule allows us to
+override the structure size used for symbol versioning.
+
+The rule fields are expected to be as follows:
+
+- `type`: "byte_size"
+- `target`: The fully qualified name of the target data structure
+  (as shown in **--dump-dies** output).
+- `value`: A positive decimal number indicating the structure size
+  in bytes.
+
+Using the `__KABI_RULE` macro, this rule can be defined as::
+
+        #define KABI_BYTE_SIZE(fqn, value) \
+                __KABI_RULE(byte_size, fqn, value)
+
+Example usage::
+
+	struct s {
+                /* Unchanged original members */
+		unsigned long a;
+                void *p;
+
+                /* Appended new members */
+                KABI_IGNORE(0, unsigned long n);
+	};
+
+	KABI_BYTE_SIZE(s, 16);
+
+4.1.4. Overriding type strings
+==============================
+
+In rare situations where distributions must make significant changes to
+otherwise opaque data structures that have inadvertently been included
+in the published ABI, keeping symbol versions stable using the more
+targeted kABI rules can become tedious. The `type_string` rule allows us
+to override the full type string for a type or a symbol, and even add
+types for versioning that no longer exist in the kernel.
+
+The rule fields are expected to be as follows:
+
+- `type`: "type_string"
+- `target`: The fully qualified name of the target data structure
+  (as shown in **--dump-dies** output) or symbol.
+- `value`: A valid type string (as shown in **--symtypes**) output)
+  to use instead of the real type.
+
+Using the `__KABI_RULE` macro, this rule can be defined as::
+
+	#define KABI_TYPE_STRING(type, str) \
+		___KABI_RULE("type_string", type, str)
+
+Example usage::
+
+	/* Override type for a structure */
+	KABI_TYPE_STRING("s#s",
+		"structure_type s { "
+			"member base_type int byte_size(4) "
+				"encoding(5) n "
+			"data_member_location(0) "
+		"} byte_size(8)");
+
+	/* Override type for a symbol */
+	KABI_TYPE_STRING("my_symbol", "variable s#s");
+
+The `type_string` rule should be used only as a last resort if maintaining
+a stable symbol versions cannot be reasonably achieved using other
+means. Overriding a type string increases the risk of actual ABI breakages
+going unnoticed as it hides all changes to the type.
+
+4.2. Adding structure members
 =============================
 
 Perhaps the most common ABI compatible change is adding a member to a
@@ -237,7 +321,7 @@ natural method. This section describes gendwarfksyms support for using
 reserved space in data structures and hiding members that don't change
 the ABI when calculating symbol versions.
 
-4.3.1. Reserving space and replacing members
+4.2.1. Reserving space and replacing members
 ============================================
 
 Space is typically reserved for later use by appending integer types, or
@@ -276,7 +360,9 @@ The examples include `KABI_(RESERVE|USE|REPLACE)*` macros that help
 simplify the process and also ensure the replacement member is correctly
 aligned and its size won't exceed the reserved space.
 
-4.3.2. Hiding members
+.. _hiding_members:
+
+4.2.2. Hiding members
 =====================
 
 Predicting which structures will require changes during the support
@@ -305,4 +391,5 @@ member to a union where one of the fields has a name starting with
                 unsigned long b;
         };
 
-With **--stable**, both versions produce the same symbol version.
+With **--stable**, both versions produce the same symbol version. The
+examples include a `KABI_IGNORE` macro to simplify the code.
-- 
2.49.0.906.g1f30a19c02-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] gendwarfksyms: Clean up kABI rule look-ups
  2025-04-30 21:40 ` [PATCH 1/4] gendwarfksyms: Clean up kABI rule look-ups Sami Tolvanen
@ 2025-05-05 11:56   ` Petr Pavlu
  0 siblings, 0 replies; 9+ messages in thread
From: Petr Pavlu @ 2025-05-05 11:56 UTC (permalink / raw)
  To: Sami Tolvanen
  Cc: Masahiro Yamada, Luis Chamberlain, Daniel Gomez, linux-modules,
	linux-kbuild, linux-kernel

On 4/30/25 23:40, Sami Tolvanen wrote:
> Reduce code duplication by moving kABI rule look-ups to separate
> functions.
> 
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>

Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

-- Petr

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] gendwarfksyms: Add a kABI rule to override byte_size attributes
  2025-04-30 21:40 ` [PATCH 2/4] gendwarfksyms: Add a kABI rule to override byte_size attributes Sami Tolvanen
@ 2025-05-05 11:57   ` Petr Pavlu
  0 siblings, 0 replies; 9+ messages in thread
From: Petr Pavlu @ 2025-05-05 11:57 UTC (permalink / raw)
  To: Sami Tolvanen
  Cc: Masahiro Yamada, Luis Chamberlain, Daniel Gomez, linux-modules,
	linux-kbuild, linux-kernel

On 4/30/25 23:40, Sami Tolvanen wrote:
> A data structure can be partially opaque to modules if its
> allocation is handled by the core kernel, and modules only need
> to access some of its members. In this situation, it's possible
> to append new members to the structure without breaking the ABI,
> as long as the layout for the original members remains unchanged.
> For example, consider the following struct:
> 
>   struct s {
>           unsigned long a;
>           void *p;
>   };
> 
> gendwarfksyms --stable --dump-dies produces the following type
> expansion:
> 
>   variable structure_type s {
>     member base_type long unsigned int byte_size(8) encoding(7) a
>       data_member_location(0) ,
>     member pointer_type {
>       base_type void
>     } byte_size(8) p data_member_location(8)
>   } byte_size(16)
> 
> To append new members, we can use the KABI_IGNORE() macro to
> hide them from gendwarfksyms --stable:
> 
>   struct s {
>           /* old members with unchanged layout */
>           unsigned long a;
>           void *p;
> 
>           /* new members not accessed by modules */
>           KABI_IGNORE(0, unsigned long n);
>   };
> 
> However, we can't hide the fact that adding new members changes
> the struct size, as seen in the updated type string:
> 
>   variable structure_type s {
>     member base_type long unsigned int byte_size(8) encoding(7) a
>       data_member_location(0) ,
>     member pointer_type {
>       base_type void
>     } byte_size(8) p data_member_location(8)
>   } byte_size(24)
> 
> In order to support this use case, add a kABI rule that makes it
> possible to override the byte_size attribute for types:
> 
>   /*
>    * struct s allocation is handled by the kernel, so
>    * appending new members without changing the original
>    * layout won't break the ABI.
>    */
>   KABI_BYTE_SIZE(s, 16);
> 
> This results in a type string that's unchanged from the original
> and therefore, won't change versions for symbols that reference
> the changed structure.
> 
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>

Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

-- Petr

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] gendwarfksyms: Add a kABI rule to override type strings
  2025-04-30 21:40 ` [PATCH 3/4] gendwarfksyms: Add a kABI rule to override type strings Sami Tolvanen
@ 2025-05-05 12:17   ` Petr Pavlu
  2025-05-05 19:15     ` Sami Tolvanen
  0 siblings, 1 reply; 9+ messages in thread
From: Petr Pavlu @ 2025-05-05 12:17 UTC (permalink / raw)
  To: Sami Tolvanen
  Cc: Masahiro Yamada, Luis Chamberlain, Daniel Gomez, linux-modules,
	linux-kbuild, linux-kernel, Giuliano Procida

On 4/30/25 23:40, Sami Tolvanen wrote:
> In rare situations where distributions must make significant
> changes to otherwise opaque data structures that have
> inadvertently been included in the published ABI, keeping
> symbol versions stable using the existing kABI macros can
> become tedious.
> 
> For example, Android decided to switch to a newer io_uring
> implementation in the 5.10 GKI kernel "to resolve a huge number
> of potential, and known, problems with the codebase," requiring
> "horrible hacks" with genksyms:
> 
>   "A number of the io_uring structures get used in other core
>   kernel structures, only as "opaque" pointers, so there is
>   not any real ABI breakage.  But, due to the visibility of
>   the structures going away, the CRC values of many scheduler
>   variables and functions were changed."
>     -- https://r.android.com/2425293
> 
> While these specific changes probably could have been hidden
> from gendwarfksyms using the existing kABI macros, this may not
> always be the case.
> 
> Add a last resort kABI rule that allows distribution
> maintainers to fully override a type string for a symbol or a
> type. Also add a more informative error message in case we find
> a non-existent type references when calculating versions.
> 
> Suggested-by: Giuliano Procida <gprocida@google.com>
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> [...]
> @@ -372,9 +405,81 @@ static void type_expand(struct die *cache, struct type_expansion *type,
>  	cache_free(&expansion_cache);
>  }
>  
> +static void type_parse(const char *name, const char *str,
> +		       struct type_expansion *type)
> +{
> +	char *fragment;
> +	size_t start = 0;
> +	size_t end;
> +	size_t pos;
> +
> +	if (!*str)
> +		error("empty type string override for '%s'", name);
> +
> +	type_expansion_init(type);
> +
> +	for (pos = 1; str[pos]; ++pos) {
> +		bool empty;
> +		char marker = ' ';
> +
> +		if (!is_type_prefix(&str[pos - 1]))
> +			continue;
> +
> +		end = pos + 1;
> +
> +		/*
> +		 * Find the end of the type reference. If the type name contains
> +		 * spaces, it must be in single quotes.
> +		 */
> +		if (str[end] == '\'') {
> +			marker = '\'';
> +			++end;
> +		}
> +		while (str[end] && str[end] != marker)
> +			++end;
> +
> +		/* Check that we have a non-empty type name */
> +		if (marker == '\'') {
> +			if (str[end] != marker)
> +				error("incomplete %c# type reference for '%s' (string : '%s')",
> +				      str[pos - 1], name, str);
> +			empty = end == pos + 2;
> +			++end;
> +		} else {
> +			empty = end == pos + 1;
> +		}
> +		if (empty)
> +			error("empty %c# type name for '%s' (string: '%s')",
> +			      str[pos - 1], name, str);
> +
> +		/* Append the part of the string before the type reference */
> +		if (pos > start + 1) {
> +			fragment = xstrndup(&str[start], pos - start - 1);
> +			type_expansion_append(type, fragment, fragment);
> +		}
> +
> +		/*
> +		 * Append the type reference -- note that if the reference
> +		 * is invalid, i.e. points to a non-existent type, we will
> +		 * print out an error when calculating versions.
> +		 */
> +		fragment = xstrndup(&str[pos - 1], end - pos + 1);
> +		type_expansion_append(type, fragment, fragment);
> +
> +		pos = start = end;
> +		if (!str[pos])
> +			break;
> +	}
> +
> +	/* Append the rest of the type string, if there's any left */
> +	if (str[start])
> +		type_expansion_append(type, &str[start], NULL);
> +}
> +

I'd find this mini-parser more straightforward, if its main loop started at
pos=0 and looked ahead, instead of starting at pos=1 and looking behind.

The patch otherwise makes sense to me. It looks similar to the override keyword
supported by genksyms.

-- 
Thanks,
Petr

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] gendwarfksyms: Add a kABI rule to override type strings
  2025-05-05 12:17   ` Petr Pavlu
@ 2025-05-05 19:15     ` Sami Tolvanen
  0 siblings, 0 replies; 9+ messages in thread
From: Sami Tolvanen @ 2025-05-05 19:15 UTC (permalink / raw)
  To: Petr Pavlu
  Cc: Masahiro Yamada, Luis Chamberlain, Daniel Gomez, linux-modules,
	linux-kbuild, linux-kernel, Giuliano Procida

Hi Petr,

On Mon, May 5, 2025 at 5:17 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> On 4/30/25 23:40, Sami Tolvanen wrote:
> > +static void type_parse(const char *name, const char *str,
> > +                    struct type_expansion *type)
> > +{
> > +     char *fragment;
> > +     size_t start = 0;
> > +     size_t end;
> > +     size_t pos;
> > +
> > +     if (!*str)
> > +             error("empty type string override for '%s'", name);
> > +
> > +     type_expansion_init(type);
> > +
> > +     for (pos = 1; str[pos]; ++pos) {
> > +             bool empty;
> > +             char marker = ' ';
> > +
> > +             if (!is_type_prefix(&str[pos - 1]))
> > +                     continue;
[...]
>
> I'd find this mini-parser more straightforward, if its main loop started at
> pos=0 and looked ahead, instead of starting at pos=1 and looking behind.

True, I'll send v2 with this changed. Thanks for taking a look!

Sami

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-05-05 19:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-30 21:40 [PATCH 0/4] gendwarfksyms: Add more kABI rules Sami Tolvanen
2025-04-30 21:40 ` [PATCH 1/4] gendwarfksyms: Clean up kABI rule look-ups Sami Tolvanen
2025-05-05 11:56   ` Petr Pavlu
2025-04-30 21:40 ` [PATCH 2/4] gendwarfksyms: Add a kABI rule to override byte_size attributes Sami Tolvanen
2025-05-05 11:57   ` Petr Pavlu
2025-04-30 21:40 ` [PATCH 3/4] gendwarfksyms: Add a kABI rule to override type strings Sami Tolvanen
2025-05-05 12:17   ` Petr Pavlu
2025-05-05 19:15     ` Sami Tolvanen
2025-04-30 21:40 ` [PATCH 4/4] Documentation/kbuild: Add new gendwarfksyms kABI rules Sami Tolvanen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).