Re: [PATCH 3/4] gendwarfksyms: Add a kABI rule to override type strings

[Petr Pavlu]

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@xxxxxxxxxx>
> Signed-off-by: Sami Tolvanen <samitolvanen@xxxxxxxxxx>
> [...]
> @@ -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