On Fri, Oct 23, 2020 at 11:22:05AM -0700, Andrii Nakryiko wrote: > On Thu, Oct 22, 2020 at 11:58 PM Jiri Olsa <jolsa@xxxxxxxxxx> wrote: > > > > On Fri, Oct 23, 2020 at 07:36:57AM +0200, Jiri Olsa wrote: > > > On Thu, Oct 22, 2020 at 01:00:19PM -0700, Andrii Nakryiko wrote: > > > > > > SNIP > > > > > > > > > > > > > hi, > > > > > FYI there's still no solution yet, so far the progress is: > > > > > > > > > > the proposed workaround was to use the negation -> we don't have > > > > > DW_AT_declaration tag, so let's find out instead which DW_TAG_subprogram > > > > > tags have attached code and skip them if they don't have any: > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060#c10 > > > > > > > > > > the attached patch is doing that, but the resulting BTF is missing > > > > > several functions due to another bug in dwarf: > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1890107 > > > > > > > > It seems fine if there are only few functions (especially if those are > > > > unlikely to be traced). Do you have an estimate of how many functions > > > > have this second DWARF bug? > > > > > > it wasn't that many, I'll recheck > > > > 127 functions missing if the workaround is applied, list attached > > > > some of those seem pretty useful... I guess the quick workaround in > pahole would be to just remember function names that were emitted > already. The problem with that is that we can pick a version without > parameter names, which is not the end of the world, but certainly > annoying. right, we can generate them in bpftrace, but it's a shame > > But otherwise, I don't really have a good feeling what's the perfect > solution here... I tried the check of dwarf record against function symbols with adresses mentioned earlier (attached) getting more functions of course ;-) $ bpftool btf dump file ./vmlinux | grep 'FUNC ' | wc -l 46606 compared to 22869 on the same .config with working gcc and current pahole and resolve_btfids is happy, because there are no duplications jirka --- diff --git a/btf_encoder.c b/btf_encoder.c index 2a6455be4c52..468781204cd9 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -326,6 +326,18 @@ static int find_all_percpu_vars(struct btf_elf *btfe) return 0; } +static bool btfe__generate_func(const struct btf_elf *btfe, const char *name) +{ + struct symbol *sym; + + sym = btfe__find_symbol(btfe, name); + if (!sym || sym->generated || !sym->addr) + return false; + + sym->generated = true; + return true; +} + int cu__encode_btf(struct cu *cu, int verbose, bool force, bool skip_encoding_vars) { @@ -354,6 +366,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, if (!skip_encoding_vars && find_all_percpu_vars(btfe)) goto out; + if (btfe__load_symbols(btfe, cu)) + goto out; + has_index_type = false; need_index_type = false; array_index_id = 0; @@ -401,7 +416,7 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, int btf_fnproto_id, btf_fn_id; const char *name; - if (fn->declaration || !fn->external) + if (!btfe__generate_func(btfe, function__name(fn, cu))) continue; btf_fnproto_id = btf_elf__add_func_proto(btfe, cu, &fn->proto, type_id_off); 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) && + 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 && diff --git a/libbtf.c b/libbtf.c index babf4fe8cd9e..6c4b8a069b85 100644 --- a/libbtf.c +++ b/libbtf.c @@ -30,6 +30,91 @@ uint8_t btf_elf__verbose; uint8_t btf_elf__force; +static int btfe__insert_symbol(struct btf_elf *btfe, const char *name, + uint64_t addr) +{ + struct rb_node **p = &btfe->symbols.rb_node; + struct rb_node *parent = NULL; + struct symbol *sym; + int err; + + while (*p != NULL) { + parent = *p; + sym = rb_entry(parent, struct symbol, rb_node); + + + err = strcmp(sym->name, name); + if (err < 0) + p = &(*p)->rb_left; + else if (err > 0) + p = &(*p)->rb_right; + else + return 0; + } + + sym = malloc(sizeof(*sym)); + if (!sym) + return -1; + sym->name = name; + sym->addr = addr; + sym->generated = false; + + rb_link_node(&sym->rb_node, parent, p); + rb_insert_color(&sym->rb_node, &btfe->symbols); + return 0; +} + +struct symbol *btfe__find_symbol(const struct btf_elf *btfe, const char *name) +{ + struct symbol *sym; + struct rb_node *n; + int err; + + n = btfe->symbols.rb_node; + + while (n) { + sym = rb_entry(n, struct symbol, rb_node); + err = strcmp(sym->name, name); + if (err < 0) + n = n->rb_left; + else if (err > 0) + n = n->rb_right; + else + return sym; + } + + return NULL; +} + +int btfe__load_symbols(struct btf_elf *btfe, struct cu *cu) +{ + const char *name; + GElf_Sym sym; + uint32_t id; + + cu__cache_symtab(cu); + + cu__for_each_cached_symtab_entry(cu, id, sym, name) { + if (!elf_sym__is_function(&sym)) + continue; + if (btfe__insert_symbol(btfe, name, elf_sym__value(&sym))) + return -1; + } + return 0; +} + +static void btfe__delete_symbols(struct btf_elf *btfe) +{ + struct rb_node *n = rb_first(&btfe->symbols); + struct symbol *sym; + + while (n) { + sym = rb_entry(n, struct symbol, rb_node); + n = rb_next(&sym->rb_node); + free(sym); + } +} + static int btf_var_secinfo_cmp(const void *a, const void *b) { const struct btf_var_secinfo *av = a; @@ -72,6 +157,7 @@ struct btf_elf *btf_elf__new(const char *filename, Elf *elf) if (!btfe) return NULL; + btfe->symbols = RB_ROOT; btfe->in_fd = -1; btfe->filename = strdup(filename); if (btfe->filename == NULL) @@ -177,6 +263,7 @@ void btf_elf__delete(struct btf_elf *btfe) elf_end(btfe->elf); } + btfe__delete_symbols(btfe); elf_symtab__delete(btfe->symtab); __gobuffer__delete(&btfe->percpu_secinfo); btf__free(btfe->btf); diff --git a/libbtf.h b/libbtf.h index 887b5bc55c8e..f4a58834c390 100644 --- a/libbtf.h +++ b/libbtf.h @@ -12,6 +12,14 @@ #include <stdbool.h> #include <stdint.h> #include "lib/bpf/src/btf.h" +#include "rbtree.h" + +struct symbol { + const char *name; + uint64_t addr; + bool generated; + struct rb_node rb_node; +}; struct btf_elf { void *priv; @@ -27,6 +35,7 @@ struct btf_elf { uint32_t percpu_shndx; uint64_t percpu_base_addr; struct btf *btf; + struct rb_root symbols; }; extern uint8_t btf_elf__verbose; @@ -66,4 +75,7 @@ int btf_elf__encode(struct btf_elf *btf, uint8_t flags); const char *btf_elf__string(struct btf_elf *btf, uint32_t ref); int btf_elf__load(struct btf_elf *btf); +struct symbol *btfe__find_symbol(const struct btf_elf *btfe, const char *name); +int btfe__load_symbols(struct btf_elf *btfe, struct cu *cu); + #endif /* _LIBBTF_H */