Re: [PATCH bpf-next v2 2/3] libbpf: Support symbol versioning for uprobe

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Sep 05, 2023 at 03:12:56PM +0000, Hengqi Chen wrote:
> In current implementation, we assume that symbol found in .dynsym section
> would have a version suffix and use it to compare with symbol user supplied.
> According to the spec ([0]), this assumption is incorrect, the version info
> of dynamic symbols are stored in .gnu.version and .gnu.version_d sections
> of ELF objects. For example:
> 
>     $ nm -D /lib/x86_64-linux-gnu/libc.so.6 | grep rwlock_wrlock
>     000000000009b1a0 T __pthread_rwlock_wrlock@GLIBC_2.2.5
>     000000000009b1a0 T pthread_rwlock_wrlock@@GLIBC_2.34
>     000000000009b1a0 T pthread_rwlock_wrlock@GLIBC_2.2.5
> 
>     $ readelf -W --dyn-syms /lib/x86_64-linux-gnu/libc.so.6 | grep rwlock_wrlock
>       706: 000000000009b1a0   878 FUNC    GLOBAL DEFAULT   15 __pthread_rwlock_wrlock@GLIBC_2.2.5
>       2568: 000000000009b1a0   878 FUNC    GLOBAL DEFAULT   15 pthread_rwlock_wrlock@@GLIBC_2.34
>       2571: 000000000009b1a0   878 FUNC    GLOBAL DEFAULT   15 pthread_rwlock_wrlock@GLIBC_2.2.5
> 
> In this case, specify pthread_rwlock_wrlock@@GLIBC_2.34 or
> pthread_rwlock_wrlock@GLIBC_2.2.5 in bpf_uprobe_opts::func_name won't work.
> Because the qualified name does NOT match `pthread_rwlock_wrlock` (without
> version suffix) in .dynsym sections.
> 
> This commit implements the symbol versioning for dynsym and allows user to
> specify symbol in the following forms:
>   - func
>   - func@LIB_VERSION
>   - func@@LIB_VERSION
> 
> In case of symbol conflicts, error out and users should resolve it by
> specifying a qualified name.
> 
>   [0]: https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/symversion.html
> 
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>

I have a question below, but other than that

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka


SNIP

> @@ -119,6 +148,7 @@ static struct elf_sym *elf_sym_iter_next(struct elf_sym_iter *iter)
>  	struct elf_sym *ret = &iter->sym;
>  	GElf_Sym *sym = &ret->sym;
>  	const char *name = NULL;
> +	GElf_Versym versym;
>  	Elf_Scn *sym_scn;
>  	size_t idx;
> 
> @@ -138,12 +168,112 @@ static struct elf_sym *elf_sym_iter_next(struct elf_sym_iter *iter)
> 
>  		iter->next_sym_idx = idx + 1;
>  		ret->name = name;
> +		ret->ver = 0;
> +		ret->hidden = false;
> +
> +		if (iter->versyms) {
> +			if (!gelf_getversym(iter->versyms, idx, &versym))
> +				continue;
> +			ret->ver = versym & VERSYM_VERSION;
> +			ret->hidden = versym & VERSYM_HIDDEN;

the doc mentions value 1 being special, also I can see readelf
code checking on that.. is that taken into account?

> +		}
>  		return ret;
>  	}
> 
>  	return NULL;
>  }
> 

SNIP