diff mbox series

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

Message ID 87e9970b63dede4a19ec62ec572e224eecc26fa3.1725016029.git.vmalik@redhat.com (mailing list archive)
State RFC
Delegated to: BPF
Headers show
Series libbpf: Add support for aliased BPF programs | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-17 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 7 this patch: 7
netdev/build_tools success Errors and warnings before: 4 this patch: 4
netdev/cc_maintainers warning 5 maintainers not CCed: justinstitt@google.com llvm@lists.linux.dev morbo@google.com ndesaulniers@google.com nathan@kernel.org
netdev/build_clang success Errors and warnings before: 7 this patch: 7
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 7 this patch: 7
netdev/checkpatch warning WARNING: line length of 83 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns WARNING: line length of 89 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Viktor Malik Sept. 2, 2024, 6:58 a.m. UTC
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 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(-)

Comments

Jiri Olsa Sept. 3, 2024, 11:16 a.m. UTC | #1
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?

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
>
Viktor Malik Sept. 3, 2024, 1:08 p.m. UTC | #2
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.

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
>>
>
Arnaldo Carvalho de Melo Sept. 3, 2024, 2:08 p.m. UTC | #3
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.

- 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
> >>
> >
Jiri Olsa Sept. 3, 2024, 2:53 p.m. UTC | #4
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.

ah ok, sry I misunderstood the purpose.. it's about the bpf code
generated in bpftrace

jirka
Viktor Malik Sept. 4, 2024, 5:46 a.m. UTC | #5
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
>>>>
>>>
>
diff mbox series

Patch

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;