On Thu, Apr 27, 2023 at 6:24 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Wed, Apr 26, 2023 at 12:27:31PM -0700, Andrii Nakryiko wrote:
> > On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> > >
> > > Currently we have elf_find_func_offset function that looks up
> > > symbol in the binary and returns its offset to be used for uprobe
> > > attachment.
> > >
> > > For attaching multiple uprobes we will need interface that allows
> > > us to get offsets for multiple symbols specified either by name or
> > > regular expression.
> > >
> > > Factoring out elf_for_each_symbol helper function that iterates
> > > all symbols in binary and calls following callbacks:
> > >
> > > fn_match - on each symbol
> > > if it returns error < 0, we bail out with that error
> > > fn_done - when we finish iterating symbol section,
> > > if it returns true, we don't iterate next section
> > >
> > > It will be used in following changes to lookup multiple symbols
> > > and their offsets.
> > >
> > > Changing elf_find_func_offset to use elf_for_each_symbol with
> > > single_match callback that's looking to match single function.
> > >
> >
> > Given we have multiple uses for this for_each_elf_symbol, would it
> > make sense to implement it as an iterator (following essentially the
> > same pattern that BPF open-coded iterator is doing, where state is in
> > a small struct, and then we call next() until we get back NULL?)
> >
> > This will lead to cleaner code overall, I think. And it does seem func
> > to implement it this (composable) way.
>
> ok, I'll check the open-coded iterator for this
Do check it, as it's a useful thing on BPF side. But tl;dr for libbpf
internal use we could do something like:
struct elf_iter {
Elf *elf;
size_t next_sym_idx;
};
and in the code the use will be something like
struct elf_iter;
elf_iter_init(*iter, elf); /* sets next_sym_idx to 0 */
while ((sym = elf_iter_next(&iter))) {
/* use sym */
}
And we can tune the returned result to have symbol index, etc, of course.
>
> >
> > Also, I think we are at the point where libbpf.c is becoming pretty
> > bloated, so we should try to split out coherent subsets of
> > functionality into separate files. ELF helpers seem like a good group
> > of functionality to move to a separate file? Maybe as a separate
> > patch set and/or follow up, but think about whether you can do part of
> > that during refactoring?
>
> right, sounds good, will check
>
> thanks,
> jirka
>
> >
> > > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > > ---
> > > tools/lib/bpf/libbpf.c | 185 +++++++++++++++++++++++++----------------
> > > 1 file changed, 114 insertions(+), 71 deletions(-)
> > >
> >
> > [...]
