[DTrace-devel] [PATCH] btf: fix symbol BTF ID lookup
Kris Van Hees
kris.van.hees at oracle.com
Fri Jul 25 19:05:02 UTC 2025
On Fri, Jul 25, 2025 at 07:37:45PM +0100, Nick Alcock wrote:
> 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 at 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?
Yes, it does, in the sense that it forces the the data to be loaded if it
is not loaded yet.
> 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?)
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*).
> > + /* 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/
Thanks.
> > 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).
More information about the DTrace-devel
mailing list