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

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

On 25 Jul 2025, Kris Van Hees said:

> On Fri, Jul 25, 2025 at 07:37:45PM +0100, Nick Alcock wrote:
>> On 24 Jul 2025, Kris Van Hees said:
>> > -	/*
>> > -	 * 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?)
>
> dt_exec is the dt_module_t representing vmlinux, and thus its dm_btf *is*
> shared_btf.  And if you look at the load code, this *will* initialize the
> shared_btf (not *will probably*).

... yeah, I forgot that dt_exec was a module, not a CTF. It makes sense
given that.

>> >  	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!
>
> dt_exec == vmlinux, so this will trigger a lookup in shared_btf (which is the
> BTF of vmlinux aka dt_exec).

Aha. My brain was operating in CTF mode, in which of course vmlinux has
its own dict distinct from the shared parent. That's OK, then.

Reviewed-by: Nick Alcock <nick.alcock@oracle.com>

... and I shouldn't do these when I'm this tired, this was all obvious.

-- 
NULL && (void)