On Thu, Jul 06, 2023 at 08:52:13PM -0700, Andrii Nakryiko wrote:
> On Fri, Jun 30, 2023 at 1:36 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> >
> > Adding elf_resolve_pattern_offsets function that looks up
> > offsets for symbols specified by pattern argument.
> >
> > The 'pattern' argument allows wildcards (*?' supported).
> >
> > Offsets are returned in allocated array together with its
> > size and needs to be released by the caller.
> >
> > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > ---
> > tools/lib/bpf/elf.c | 57 +++++++++++++++++++++++++++++++++
> > tools/lib/bpf/libbpf.c | 2 +-
> > tools/lib/bpf/libbpf_elf.h | 3 ++
> > tools/lib/bpf/libbpf_internal.h | 1 +
> > 4 files changed, 62 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/elf.c b/tools/lib/bpf/elf.c
> > index 7e2f3b2e1fb6..f2d1a8cc2f9d 100644
> > --- a/tools/lib/bpf/elf.c
> > +++ b/tools/lib/bpf/elf.c
> > @@ -376,3 +376,60 @@ int elf_resolve_syms_offsets(const char *binary_path, int cnt,
> > elf_close(&elf_fd);
> > return err;
> > }
> > +
>
> same, leave comment that caller should free offsets on success?
yes, will add
>
> > +int elf_resolve_pattern_offsets(const char *binary_path, const char *pattern,
> > + unsigned long **poffsets, size_t *pcnt)
> > +{
> > + int sh_types[2] = { SHT_DYNSYM, SHT_SYMTAB };
> > + unsigned long *offsets = NULL;
> > + size_t cap = 0, cnt = 0;
> > + struct elf_fd elf_fd;
> > + int err = 0, i;
> > +
> > + err = elf_open(binary_path, &elf_fd);
> > + if (err)
> > + return err;
> > +
> > + for (i = 0; i < ARRAY_SIZE(sh_types); i++) {
> > + struct elf_sym_iter iter;
> > + struct elf_sym *sym;
> > +
> > + err = elf_sym_iter_new(&iter, elf_fd.elf, binary_path, sh_types[i], STT_FUNC);
> > + if (err) {
> > + if (err == -ENOENT)
> > + continue;
> > + goto out;
> > + }
>
> ditto, minimize nesting, please
ok
>
> > +
> > + while ((sym = elf_sym_iter_next(&iter))) {
> > + if (!glob_match(sym->name, pattern))
> > + continue;
> > +
> > + err = libbpf_ensure_mem((void **) &offsets, &cap, sizeof(*offsets),
> > + cnt + 1);
> > + if (err)
> > + goto out;
> > +
> > + offsets[cnt++] = elf_sym_offset(sym);
> > + }
> > +
> > + /* If we found anything in the first symbol section,
> > + * do not search others to avoid duplicates.
>
> DYNSYM is going to have only exposed symbols, so for this pattern
> matching, maybe it's best to start with SYMTAB and only fallback to
> DYNSYM if we didn't find anything in SYMTAB (more realistically it
> would be that SYMTAB section is missing, so we fallback to DYNSYM;
> otherwise neither DYNSYM nor SYMTAB will have matching symbols, most
> probably, but that's minor)
makes sense, will switch
>
> other than that, LGTM
thanks,
jirka
