On Mon, Jul 21, 2025 at 12:41:00PM +0100, Alan Maguire wrote:
> On 17/07/2025 16:25, Jiri Olsa wrote:
> > Menglong reported issue where we can have function in BTF which has
> > multiple addresses in kallsysm [1].
> >
> > Rather than filtering this in runtime, let's teach pahole to remove
> > such functions.
> >
> > Removing duplicate records from functions entries that have more
> > at least one different address. This way btf_encoder__find_function
> > won't find such functions and they won't be added in BTF.
> >
> > In my setup it removed 428 functions out of 77141.
> >
>
> Is such removal necessary? If the presence of an mcount annotation is
> the requirement, couldn't we just utilize
>
> /sys/kernel/tracing/available_filter_functions_addrs
>
> to map name to address safely? It identifies mcount-containing functions
> and some of these appear to be duplicates, for example there I see
>
> ffffffff8376e8b4 acpi_attr_is_visible
> ffffffff8379b7d4 acpi_attr_is_visible
for that we'd need new interface for loading fentry/fexit.. programs, right?
the current interface to get fentry/fexit.. attach address is:
- user specifies function name, that translates to btf_id
- in kernel that btf id translates back to function name
- kernel uses kallsyms_lookup_name or find_kallsyms_symbol_value
to get the address
so we don't really know which address user wanted in the first place
I think we discussed this issue some time ago, but I'm not sure what
the proposal was at the end (function address stored in BTF?)
this change is to make sure there are no duplicate functions in BTF
that could cause this confusion.. rather than bad result, let's deny
the attach for such function
jirka
>
> ?
>
> > [1] https://lore.kernel.org/bpf/20250710070835.260831-1-dongml2@xxxxxxxxxxxxxxx/
> > Reported-by: Menglong Dong <menglong8.dong@xxxxxxxxx>
> > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > ---
> >
> > Alan,
> > I'd like to test this in the pahole CI, is there a way to manualy trigger it?
> >
>
> Easiest way is to base from pahole's next branch and push to a github
> repo; the tests will run as actions there. I've just merged the function
> comparison work so that will be available if you base/sync a branch on
> next from git.kernel.org/pub/scm/devel/pahole/pahole.git/ . Thanks!
>
> Alan
>
>
> > thanks,
> > jirka
> >
> >
> > ---
> > btf_encoder.c | 37 +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 37 insertions(+)
> >
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index 16739066caae..a25fe2f8bfb1 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -99,6 +99,7 @@ struct elf_function {
> > size_t prefixlen;
> > bool kfunc;
> > uint32_t kfunc_flags;
> > + unsigned long addr;
> > };
> >
> > struct elf_secinfo {
> > @@ -1469,6 +1470,7 @@ static void elf_functions__collect_function(struct elf_functions *functions, GEl
> >
> > func = &functions->entries[functions->cnt];
> > func->name = name;
> > + func->addr = sym->st_value;
> > if (strchr(name, '.')) {
> > const char *suffix = strchr(name, '.');
> >
> > @@ -2143,6 +2145,40 @@ int btf_encoder__encode(struct btf_encoder *encoder, struct conf_load *conf)
> > return err;
> > }
> >
> > +/*
> > + * Remove name duplicates from functions->entries that have
> > + * at least 2 different addresses.
> > + */
> > +static void functions_remove_dups(struct elf_functions *functions)
> > +{
> > + struct elf_function *n = &functions->entries[0];
> > + bool matched = false, diff = false;
> > + int i, j;
> > +
> > + for (i = 0, j = 1; i < functions->cnt && j < functions->cnt; i++, j++) {
> > + struct elf_function *a = &functions->entries[i];
> > + struct elf_function *b = &functions->entries[j];
> > +
> > + if (!strcmp(a->name, b->name)) {
> > + matched = true;
> > + diff |= a->addr != b->addr;
> > + continue;
> > + }
> > +
> > + /*
> > + * Keep only not-matched entries and last one of the matched/duplicates
> > + * ones if all of the matched entries had the same address.
> > + **/
> > + if (!matched || !diff)
> > + *n++ = *a;
> > + matched = diff = false;
> > + }
> > +
> > + if (!matched || !diff)
> > + *n++ = functions->entries[functions->cnt - 1];
> > + functions->cnt = n - &functions->entries[0];
> > +}
> > +
> > static int elf_functions__collect(struct elf_functions *functions)
> > {
> > uint32_t nr_symbols = elf_symtab__nr_symbols(functions->symtab);
> > @@ -2168,6 +2204,7 @@ static int elf_functions__collect(struct elf_functions *functions)
> >
> > if (functions->cnt) {
> > qsort(functions->entries, functions->cnt, sizeof(*functions->entries), functions_cmp);
> > + functions_remove_dups(functions);
> > } else {
> > err = 0;
> > goto out_free;
>
