Re: [RFC bpf-next 1/3] libbpf: Support aliased symbols in linker

[Viktor Malik <vmalik@redhat.com>]

On 9/3/24 16:08, Arnaldo Carvalho de Melo wrote:
> On Tue, Sep 03, 2024 at 03:08:25PM +0200, Viktor Malik wrote:
>> On 9/3/24 13:16, Jiri Olsa wrote:
>>> On Mon, Sep 02, 2024 at 08:58:01AM +0200, Viktor Malik wrote:
>>>> It is possible to create multiple BPF programs sharing the same
>>>> instructions using the compiler `__attribute__((alias("...")))`:
>>>>
>>>>     int BPF_PROG(prog)
>>>>     {
>>>>         [...]
>>>>     }
>>>>     int prog_alias() __attribute__((alias("prog")));
>>>>
>>>> This may be convenient when creating multiple programs with the same
>>>> instruction set attached to different events (such as bpftrace does).
>>>>
>>>> One problem in this situation is that Clang doesn't generate a BTF entry
>>>> for `prog_alias` which makes libbpf linker fail when processing such a
>>>> BPF object.
>>>
>>> this might not solve all the issues, but could we change pahole to
>>> generate BTF FUNC for alias function symbols?
>>
>> I don't think that would work here. First, we don't usually run pahole
>> when building BPF objects, it's Clang which generates BTF for the "bpf"
>> target directly. Second, AFAIK, pahole converts DWARF to BTF and
>> compilers don't generate DWARF entries for alias function symbols either.
> 
> So, pahole adds BTF info that doesn't come from DWARF, and it could read
> the BTF generated by clang for the bpf target and ammend/augment/add to
> it if that would allow us to have something we need and that isn't
> currently (or planned) to be supported by clang.
> 
> I.e. we have a BTF loader, we could pair it with the BPF encoder, in
> addition to the usual DWARF Loader + BPF encoder as what the loaders do
> is to generate internal representation that is then consumed by the
> pretty printer or the BTF encoder.
> 
> In this case the BTF encoder would use what is the internal
> representation, that it doesn't even know came from a BPF object
> generated by clang's BPF target, and would add this extra aliases, if we
> can get it from somewhere else.

Yes, that would be an option. We should be able to get the info about
aliases from the symbol table.

The problem here is that it would require an extra step of running
pahole when compiling BPF objects. Which should be ok for bpftrace,
though, as we control how we create BPF objects and can easily add the
extra step.

Viktor

> 
> - Arnaldo
>  
>> Viktor
>>
>>>
>>> jirka
>>>
>>>>
>>>> This commits adds support for that by finding another symbol at the same
>>>> address for which a BTF entry exists and using that entry in the linker.
>>>> This allows to use the linker (e.g. via `bpftool gen object ...`) on BPF
>>>> objects containing aliases.
>>>>
>>>> Note that this won't be sufficient for most programs as we also need to
>>>> add support for handling relocations in the aliased programs. This will
>>>> be added by the following commit.
>>>>
>>>> Signed-off-by: Viktor Malik <vmalik@redhat.com>
>>>> ---
>>>>  tools/lib/bpf/linker.c | 68 +++++++++++++++++++++++-------------------
>>>>  1 file changed, 38 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
>>>> index 9cd3d4109788..5ebc9ff1246e 100644
>>>> --- a/tools/lib/bpf/linker.c
>>>> +++ b/tools/lib/bpf/linker.c
>>>> @@ -1688,6 +1688,34 @@ static bool btf_is_non_static(const struct btf_type *t)
>>>>  	       || (btf_is_func(t) && btf_func_linkage(t) != BTF_FUNC_STATIC);
>>>>  }
>>>>  
>>>> +static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
>>>> +				   int sym_type, const char *sym_name)
>>>> +{
>>>> +	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
>>>> +	Elf64_Sym *sym = symtab->data->d_buf;
>>>> +	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
>>>> +	int str_sec_idx = symtab->shdr->sh_link;
>>>> +	const char *name;
>>>> +
>>>> +	for (i = 0; i < n; i++, sym++) {
>>>> +		if (sym->st_shndx != sec_idx)
>>>> +			continue;
>>>> +		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
>>>> +			continue;
>>>> +
>>>> +		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
>>>> +		if (!name)
>>>> +			return NULL;
>>>> +
>>>> +		if (strcmp(sym_name, name) != 0)
>>>> +			continue;
>>>> +
>>>> +		return sym;
>>>> +	}
>>>> +
>>>> +	return NULL;
>>>> +}
>>>> +
>>>>  static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sym_name,
>>>>  			     int *out_btf_sec_id, int *out_btf_id)
>>>>  {
>>>> @@ -1695,6 +1723,7 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
>>>>  	const struct btf_type *t;
>>>>  	const struct btf_var_secinfo *vi;
>>>>  	const char *name;
>>>> +	Elf64_Sym *s;
>>>>  
>>>>  	if (!obj->btf) {
>>>>  		pr_warn("failed to find BTF info for object '%s'\n", obj->filename);
>>>> @@ -1710,8 +1739,15 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
>>>>  		 */
>>>>  		if (btf_is_non_static(t)) {
>>>>  			name = btf__str_by_offset(obj->btf, t->name_off);
>>>> -			if (strcmp(name, sym_name) != 0)
>>>> -				continue;
>>>> +			if (strcmp(name, sym_name) != 0) {
>>>> +				/* the symbol that we look for may not have BTF as it may
>>>> +				 * be an alias of another symbol; we check if this is
>>>> +				 * the original symbol and if so, we use its BTF id
>>>> +				 */
>>>> +				s = find_sym_by_name(obj, sym->st_shndx, STT_FUNC, name);
>>>> +				if (!s || s->st_value != sym->st_value)
>>>> +					continue;
>>>> +			}
>>>>  
>>>>  			/* remember and still try to find DATASEC */
>>>>  			btf_id = i;
>>>> @@ -2132,34 +2168,6 @@ static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *ob
>>>>  	return 0;
>>>>  }
>>>>  
>>>> -static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
>>>> -				   int sym_type, const char *sym_name)
>>>> -{
>>>> -	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
>>>> -	Elf64_Sym *sym = symtab->data->d_buf;
>>>> -	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
>>>> -	int str_sec_idx = symtab->shdr->sh_link;
>>>> -	const char *name;
>>>> -
>>>> -	for (i = 0; i < n; i++, sym++) {
>>>> -		if (sym->st_shndx != sec_idx)
>>>> -			continue;
>>>> -		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
>>>> -			continue;
>>>> -
>>>> -		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
>>>> -		if (!name)
>>>> -			return NULL;
>>>> -
>>>> -		if (strcmp(sym_name, name) != 0)
>>>> -			continue;
>>>> -
>>>> -		return sym;
>>>> -	}
>>>> -
>>>> -	return NULL;
>>>> -}
>>>> -
>>>>  static int linker_fixup_btf(struct src_obj *obj)
>>>>  {
>>>>  	const char *sec_name;
>>>> -- 
>>>> 2.46.0
>>>>
>>>
>