On Tue, Oct 27, 2020 at 04:20:10PM -0700, Andrii Nakryiko wrote: > On Mon, Oct 26, 2020 at 5:07 PM Jiri Olsa <jolsa@xxxxxxxxxx> wrote: > > > > We need to generate just single BTF instance for the > > function, while DWARF data contains multiple instances > > of DW_TAG_subprogram tag. > > > > Unfortunately we can no longer rely on DW_AT_declaration > > tag (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060) > > > > Instead we apply following checks: > > - argument names are defined for the function > > - there's symbol and address defined for the function > > - function is generated only once > > > > They might be slightly superfluous together, but it's > > better to be ready for another DWARF mishap. > > > > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx> > > --- > > btf_encoder.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++- > > elf_symtab.h | 8 ++++ > > 2 files changed, 109 insertions(+), 1 deletion(-) > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > index 2dd26c904039..99b9abe36993 100644 > > --- a/btf_encoder.c > > +++ b/btf_encoder.c > > @@ -26,6 +26,62 @@ > > */ > > #define KSYM_NAME_LEN 128 > > > > +struct elf_function { > > + const char *name; > > + bool generated; > > +}; > > + > > +static struct elf_function *functions; > > +static int functions_alloc; > > +static int functions_cnt; > > + > > +static int functions_cmp(const void *_a, const void *_b) > > +{ > > + const struct elf_function *a = _a; > > + const struct elf_function *b = _b; > > + > > + return strcmp(a->name, b->name); > > +} > > + > > +static void delete_functions(void) > > +{ > > + free(functions); > > + functions_alloc = functions_cnt = 0; > > +} > > + > > +static int config_function(struct btf_elf *btfe, GElf_Sym *sym) > > +{ > > + if (!elf_sym__is_function(sym)) > > + return 0; > > + if (!elf_sym__value(sym)) > > + return 0; > > + > > + if (functions_cnt == functions_alloc) { > > + functions_alloc += 5000; > > maybe just do a conventional exponential size increase? Not > necessarily * 2, could be (* 3 / 2) or (* 4 / 3), libbpf uses such > approach. ok, will change > > > + functions = realloc(functions, functions_alloc * sizeof(*functions)); > > + if (!functions) > > + return -1; > > + } > > + > > + functions[functions_cnt].name = elf_sym__name(sym, btfe->symtab); > > + functions_cnt++; > > + return 0; > > +} > > + > > [...] > > > diff --git a/elf_symtab.h b/elf_symtab.h > > index 359add69c8ab..094ec4683d01 100644 > > --- a/elf_symtab.h > > +++ b/elf_symtab.h > > @@ -63,6 +63,14 @@ static inline uint64_t elf_sym__value(const GElf_Sym *sym) > > return sym->st_value; > > } > > > > +static inline int elf_sym__is_function(const GElf_Sym *sym) > > +{ > > + return (elf_sym__type(sym) == STT_FUNC || > > + elf_sym__type(sym) == STT_GNU_IFUNC) && > > Why do we need to collect STT_GNU_IFUNC? That is some PLT special > magic, does the kernel use that? Even if it does, are we even able to > attach to that? Could that remove some of the assembly functions? I missed that when I copied that function from perf ;-) I'll check jirka > > > + sym->st_name != 0 && > > + sym->st_shndx != SHN_UNDEF; > > +} > > + > > static inline bool elf_sym__is_local_function(const GElf_Sym *sym) > > { > > return elf_sym__type(sym) == STT_FUNC && > > -- > > 2.26.2 > > >
