[PATCH] gendwarfksyms: Fix structure type overrides

[PATCH] gendwarfksyms: Fix structure type overrides

[Sami Tolvanen]

As we always iterate through the entire die_map when expanding
type strings, recursively processing referenced types in
type_expand_child() is not actually necessary. Furthermore,
the type_string kABI rule added in commit c9083467f7b9
("gendwarfksyms: Add a kABI rule to override type strings") can
fail to override type strings for structures due to a missing
kabi_get_type_string() check in this function.

Fix the issue by dropping the unnecessary recursion and moving
the override check to type_expand(). Note that symbol versions
are otherwise unchanged with this patch.

Fixes: c9083467f7b9 ("gendwarfksyms: Add a kABI rule to override type strings")
Reported-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 scripts/gendwarfksyms/types.c | 65 ++++++++++-------------------------
 1 file changed, 19 insertions(+), 46 deletions(-)

diff --git a/scripts/gendwarfksyms/types.c b/scripts/gendwarfksyms/types.c
index 39ce1770e463..7bd459ea6c59 100644
--- a/scripts/gendwarfksyms/types.c
+++ b/scripts/gendwarfksyms/types.c
@@ -333,37 +333,11 @@ static void calculate_version(struct version *version,
 	cache_free(&expansion_cache);
 }
 
-static void __type_expand(struct die *cache, struct type_expansion *type,
-			  bool recursive);
-
-static void type_expand_child(struct die *cache, struct type_expansion *type,
-			      bool recursive)
-{
-	struct type_expansion child;
-	char *name;
-
-	name = get_type_name(cache);
-	if (!name) {
-		__type_expand(cache, type, recursive);
-		return;
-	}
-
-	if (recursive && !__cache_was_expanded(&expansion_cache, cache->addr)) {
-		__cache_mark_expanded(&expansion_cache, cache->addr);
-		type_expansion_init(&child);
-		__type_expand(cache, &child, true);
-		type_map_add(name, &child);
-		type_expansion_free(&child);
-	}
-
-	type_expansion_append(type, name, name);
-}
-
-static void __type_expand(struct die *cache, struct type_expansion *type,
-			  bool recursive)
+static void __type_expand(struct die *cache, struct type_expansion *type)
 {
 	struct die_fragment *df;
 	struct die *child;
+	char *name;
 
 	list_for_each_entry(df, &cache->fragments, list) {
 		switch (df->type) {
@@ -379,7 +353,12 @@ static void __type_expand(struct die *cache, struct type_expansion *type,
 				error("unknown child: %" PRIxPTR,
 				      df->data.addr);
 
-			type_expand_child(child, type, recursive);
+			name = get_type_name(child);
+			if (name)
+				type_expansion_append(type, name, name);
+			else
+				__type_expand(child, type);
+
 			break;
 		case FRAGMENT_LINEBREAK:
 			/*
@@ -397,12 +376,17 @@ static void __type_expand(struct die *cache, struct type_expansion *type,
 	}
 }
 
-static void type_expand(struct die *cache, struct type_expansion *type,
-			bool recursive)
+static void type_expand(const char *name, struct die *cache,
+			struct type_expansion *type)
 {
+	const char *override;
+
 	type_expansion_init(type);
-	__type_expand(cache, type, recursive);
-	cache_free(&expansion_cache);
+
+	if (stable && kabi_get_type_string(name, &override))
+		type_parse(name, override, type);
+	else
+		__type_expand(cache, type);
 }
 
 static void type_parse(const char *name, const char *str,
@@ -416,8 +400,6 @@ static void type_parse(const char *name, const char *str,
 	if (!*str)
 		error("empty type string override for '%s'", name);
 
-	type_expansion_init(type);
-
 	for (pos = 0; str[pos]; ++pos) {
 		bool empty;
 		char marker = ' ';
@@ -478,7 +460,6 @@ static void type_parse(const char *name, const char *str,
 static void expand_type(struct die *cache, void *arg)
 {
 	struct type_expansion type;
-	const char *override;
 	char *name;
 
 	if (cache->mapped)
@@ -504,11 +485,7 @@ static void expand_type(struct die *cache, void *arg)
 
 	debug("%s", name);
 
-	if (stable && kabi_get_type_string(name, &override))
-		type_parse(name, override, &type);
-	else
-		type_expand(cache, &type, true);
-
+	type_expand(name, cache, &type);
 	type_map_add(name, &type);
 	type_expansion_free(&type);
 	free(name);
@@ -518,7 +495,6 @@ static void expand_symbol(struct symbol *sym, void *arg)
 {
 	struct type_expansion type;
 	struct version version;
-	const char *override;
 	struct die *cache;
 
 	/*
@@ -532,10 +508,7 @@ 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. */
 
-	if (stable && kabi_get_type_string(sym->name, &override))
-		type_parse(sym->name, override, &type);
-	else
-		type_expand(cache, &type, false);
+	type_expand(sym->name, cache, &type);
 
 	/* If the symbol already has a version, don't calculate it again. */
 	if (sym->state != SYMBOL_PROCESSED) {

base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog

Re: [PATCH] gendwarfksyms: Fix structure type overrides

[Petr Pavlu]

On 6/9/25 5:49 PM, Sami Tolvanen wrote:
> As we always iterate through the entire die_map when expanding
> type strings, recursively processing referenced types in
> type_expand_child() is not actually necessary. Furthermore,
> the type_string kABI rule added in commit c9083467f7b9
> ("gendwarfksyms: Add a kABI rule to override type strings") can
> fail to override type strings for structures due to a missing
> kabi_get_type_string() check in this function.
> 
> Fix the issue by dropping the unnecessary recursion and moving
> the override check to type_expand(). Note that symbol versions
> are otherwise unchanged with this patch.
> 
> Fixes: c9083467f7b9 ("gendwarfksyms: Add a kABI rule to override type strings")
> Reported-by: Giuliano Procida <gprocida@google.com>
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> ---
>  scripts/gendwarfksyms/types.c | 65 ++++++++++-------------------------
>  1 file changed, 19 insertions(+), 46 deletions(-)
> 
> diff --git a/scripts/gendwarfksyms/types.c b/scripts/gendwarfksyms/types.c
> index 39ce1770e463..7bd459ea6c59 100644
> --- a/scripts/gendwarfksyms/types.c
> +++ b/scripts/gendwarfksyms/types.c
> @@ -333,37 +333,11 @@ static void calculate_version(struct version *version,
>  	cache_free(&expansion_cache);
>  }
>  
> -static void __type_expand(struct die *cache, struct type_expansion *type,
> -			  bool recursive);
> -
> -static void type_expand_child(struct die *cache, struct type_expansion *type,
> -			      bool recursive)
> -{
> -	struct type_expansion child;
> -	char *name;
> -
> -	name = get_type_name(cache);
> -	if (!name) {
> -		__type_expand(cache, type, recursive);
> -		return;
> -	}
> -
> -	if (recursive && !__cache_was_expanded(&expansion_cache, cache->addr)) {
> -		__cache_mark_expanded(&expansion_cache, cache->addr);
> -		type_expansion_init(&child);
> -		__type_expand(cache, &child, true);
> -		type_map_add(name, &child);
> -		type_expansion_free(&child);
> -	}

Nit: This code was the only user of __cache_was_expanded() and
__cache_mark_expanded(). It is now possible to merge
__cache_was_expanded() into cache_was_expanded() and
__cache_mark_expanded() into cache_mark_expanded().

Looks ok to me otherwise, feel free to add:

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

-- 
Thanks,
Petr

Re: [PATCH] gendwarfksyms: Fix structure type overrides

[Sami Tolvanen]

Hi Petr,

On Fri, Jun 13, 2025 at 12:52 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> Nit: This code was the only user of __cache_was_expanded() and
> __cache_mark_expanded(). It is now possible to merge
> __cache_was_expanded() into cache_was_expanded() and
> __cache_mark_expanded() into cache_mark_expanded().

Nice catch. I'll send v2 with these dropped.

> Looks ok to me otherwise, feel free to add:
>
> Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

Thanks!

Sami