Re: [PATCH] btf: fix symbol BTF ID lookup

[Nick Alcock <nick.alcock@oracle.com>]

On 24 Jul 2025, Kris Van Hees said:

> The logic to perform a BTF ID lookup for a symbol did not guard against
> BTF data not having been loaded for the module that contains the symbol.

Oops!

> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>

This seems not to do what it says on the tin (though what it does
doesn't look bad).

It's not guarding against a module not being loaded: it's doing all
lookups in vmlinux rather than the shared repo if the module isn't
found. I guess this is desirable (it does increase the set of available
types), but it might be worth mentioning that we're doing this
somewhere, given that at no point does what is actually implemented
guard against BTF not having been loaded for the module that contains
the symbol :) maybe "push the code that decides which BTF to use down
next to the code that loads it" or something?

There are a few rather odd hunks in here, notably:

> -	/*
> -	 * Ensure the shared BTF is loaded, and if no BTF is given, use the
> -	 * shared one.
> -	 */
> -	 if (!dtp->dt_shared_btf) {
> -		  dt_btf_load_module(dtp, dtp->dt_exec);
> +	/* Ensure the shared BTF is loaded. */
> +	if (!dtp->dt_shared_btf)
> +		dt_btf_load_module(dtp, dtp->dt_exec);


If we don't have the dt_shared_btf... load the dt_exec? I mean yes this
will probably load dt_shared_btf as a side-effect, but why not check
dtp->dt_exec? (Hell, why not check both?)

> +	/* If the module does not have BTF data yet, try to load it. */
> +	if (!btf) {
> +		btf = dt_btf_load_module(dtp, dmp);

That looks good though.

> +		/* If no BTF momdule data was found, use the shared BTF. */
> +		if (!btf)
> +			btf = dtp->dt_shared_btf;

s/momdule/module/

>  	if (base > 0)
> -		return dt_btf_lookup_name_kind(dtp, dtp->dt_shared_btf,
> -					       name, kind);
> +		return dt_btf_lookup_name_kind(dtp, dtp->dt_exec, name, kind);

This definitely looks wrong. The intent here is seemingly to climb to
the parent (the shared BTF) if a lookup in the child fails, since you
don't have anything like libctf here to do that for you. Replacing it
with a lookup of dt_exec isn't going to do the same thing at all!