Hi Yonghong, On Sat, Mar 8, 2025 at 2:48 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote: > > > > On 3/7/25 6:01 AM, Hengqi Chen wrote: > > As reported on libbpf-rs issue([0]), the current implementation > > may resolve symbol to a wrong offset and thus missing uprobe > > event. Calculate the symbol offset from program header instead. > > See the BCC implementation (which in turn used by bpftrace) and > > the spec ([1]) for references. > > > > [0]: https://github.com/libbpf/libbpf-rs/issues/1110 > > [1]: https://refspecs.linuxfoundation.org/elf/ > > > > Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx> > > Hengqi, > > There are some test failures in the CI. For example, > https://github.com/kernel-patches/bpf/actions/runs/13725803997/job/38392284640?pr=8631 > Yes, I've received an email from BPF CI. It seems like the uprobe multi testcase is unhappy with this change. > Please take a look. > Your below elf_sym_offset change matches some bcc implementation, but > maybe maybe this is only under certain condition? > Remove the `phdr.p_flags & PF_X` check fix the issue. Need more investigation. > Also, it would be great if you can add detailed description in commit message > about what is the problem and why a different approach is necessary to > fix the issue. > Will do. Thanks. > > --- > > tools/lib/bpf/elf.c | 32 ++++++++++++++++++++++++-------- > > 1 file changed, 24 insertions(+), 8 deletions(-) > > > > diff --git a/tools/lib/bpf/elf.c b/tools/lib/bpf/elf.c > > index 823f83ad819c..9b561c8d1eec 100644 > > --- a/tools/lib/bpf/elf.c > > +++ b/tools/lib/bpf/elf.c > > @@ -260,13 +260,29 @@ static bool symbol_match(struct elf_sym_iter *iter, int sh_type, struct elf_sym > > * for shared libs) into file offset, which is what kernel is expecting > > * for uprobe/uretprobe attachment. > > * See Documentation/trace/uprobetracer.rst for more details. This is done > > - * by looking up symbol's containing section's header and using iter's virtual > > - * address (sh_addr) and corresponding file offset (sh_offset) to transform > > + * by looking up symbol's containing program header and using its virtual > > + * address (p_vaddr) and corresponding file offset (p_offset) to transform > > * sym.st_value (virtual address) into desired final file offset. > > */ > > -static unsigned long elf_sym_offset(struct elf_sym *sym) > > +static unsigned long elf_sym_offset(Elf *elf, struct elf_sym *sym) > > { > > - return sym->sym.st_value - sym->sh.sh_addr + sym->sh.sh_offset; > > + size_t nhdrs, i; > > + GElf_Phdr phdr; > > + > > + if (elf_getphdrnum(elf, &nhdrs)) > > + return -1; > > + > > + for (i = 0; i < nhdrs; i++) { > > + if (!gelf_getphdr(elf, (int)i, &phdr)) > > + continue; > > + if (phdr.p_type != PT_LOAD || !(phdr.p_flags & PF_X)) > > + continue; > > + if (sym->sym.st_value >= phdr.p_vaddr && > > + sym->sym.st_value < (phdr.p_vaddr + phdr.p_memsz)) > > + return sym->sym.st_value - phdr.p_vaddr + phdr.p_offset; > > + } > > + > > + return -1; > > } > > > > /* Find offset of function name in the provided ELF object. "binary_path" is > > @@ -329,7 +345,7 @@ long elf_find_func_offset(Elf *elf, const char *binary_path, const char *name) > > > > if (ret > 0) { > > /* handle multiple matches */ > > - if (elf_sym_offset(sym) == ret) { > > + if (elf_sym_offset(elf, sym) == ret) { > > /* same offset, no problem */ > > continue; > > } else if (last_bind != STB_WEAK && cur_bind != STB_WEAK) { > > [...] >
