On Wed, Jan 19, 2022 at 6:04 AM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote:
>
> On Fri, 14 Jan 2022, Andrii Nakryiko wrote:
>
> > > The one piece that seems to be missing from my perspective - and this may
> > > be in more recent versions - is uprobe function attachment by name. Most of
> > > the work is already done in libusdt so it's reasonably doable I think - at a
> > > minimum it would require an equivalent to the find_elf_func_offset()
> > > function in my patch 1. Now the name of the library libusdt suggests its
> > > focus is on USDT of course, but I think having userspace function attach
> > > by name too would be great. Is that part of your plans for this work?
> >
> > True, uprobes don't supprot attaching by function name, which is quite
> > annoying. It's certainly not a focus for libusdt (or whatever it will
> > end up being called when open-sources). But if it's not much code and
> > complexity we should probably just add that to libbpf directly for
> > uprobes.
> >
>
> I've been looking at this, and I've got the following cases working:
>
> - local symbols in a binary. This involves symbol table lookup and
> relative offset calcuation.
> - shared object symbols in a shared object. In this case, the symbol
> table values suffice, no adjustment needed.
>
> The former works using the program headers (instead of /proc/pid/maps for
> offset computation), so can be run for all processes, lifting the
> limitation in the RFC which only supported name lookup for a specific
> process. Around a hundred lines for this makes it worthwhile I think.
>
> There is one more case, which is a shared library function in a binary -
> where I specify "malloc" as the function and /usr/bin/foo as the binary
> path. In this case, for dynamic symbols we can't just look up the symbol
> table in the binary, since the associated values are 0. Ideally it would
> be nice if the user could just specify "malloc" and not need to use libc
> as the binary path argument, but getting this working is proving to be
> trickier. I've tried making use of PLT section information but no luck
> yet (the idea being we try to use the trampoline address of malloc@@PLT
> instead, but I'm still trying to figure out how to extract that).
>
> So I'm wondering if we just fail lookup for that case, assuming the user
> will specify the shared library path if they want to trace a shared library
> function. What do you think? Thanks!
I think it all makes sense (but let's see the code as well ;) ). For
the latter, can you please double-check what sort of functionality BCC
provides? Also make sure that you support specifying absolute address
instead of function name as well (func+0x123 probably as well, just
like for kprobes?).
The annoying bit is libbpf's convention to use '/' as a separator in
SEC() definitions. I think bpftrace/dtrace's ':' makes more sense, but
it seems to disruptive to switch it now. Because of this, specifying
absolute path to the binary would look weird:
SEC("uprobe//usr/bin/bash/readline")
or something like that would consistent with current convention, but
super weird.
Did you run into this issue during your experiments?
I can see two improvements, more and less radical (short of switching
from / to : completely):
1. less radical is to use "custom" format for uprobe after the "uprobe/" part:
SEC("uprobe//usr/bin/bash:readline")
2. a bit more radical (but probably better long term) is to support
'/' and ':' interchangeably (but only one of them in any given SEC()
definition). For existing definitions, we can say that both forms are
supported now:
SEC("kprobe/some_func") and SEC("kprobe:some_func")
For uprobe I'd probably combine #1 and #2 and say that these two forms
are supported:
SEC("uprobe//usr/bin/bash:readline") (so function separator is always ':')
and
SEC("uprobe:/usr/bin/bash:readline") (nicer and more consistent).
Thoughts?
BTW, as much as I like consistency, the proposal to switch to ':'
exclusively in libbpf 1.0 is a no-go, IMO, it's too much of a
disruption for tons of users.
>
> Alan
