Re: [PATCH] libbpf: Fix uprobe offset calculation

[Yonghong Song <yonghong.song@linux.dev>]

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@gmail.com>

Hengqi,

There are some test failures in the CI. For example,
   https://github.com/kernel-patches/bpf/actions/runs/13725803997/job/38392284640?pr=8631

Please take a look.
Your below elf_sym_offset change matches some bcc implementation, but
maybe maybe this is only under certain condition?

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.

> ---
>   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) {

[...]