Re: [PATCH] libbpf: fix symbol resolution without .symtab

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] selftests/bpf: missing uprobe_multi_stripped.so in RUNNER_EXTRA_FILES
- [Low] selftests/bpf: unfiltered -static flag breaks uprobe_multi_stripped.so build
--

commit 4a70746dbfb8f05e040808945c59a7d7fbba3b95
Author: Eduard Zateev <hackerowskiy@gmail.com>

libbpf: fix symbol resolution without .symtab

This commit updates elf_resolve_syms_offsets() in libbpf to clear -ENOENT
when one of the symbol tables (.symtab or .dynsym) is absent so symbol
resolution succeeds against the remaining table. It also adds a regression
selftest using a stripped shared object uprobe_multi_stripped.so.

Link: https://github.com/libbpf/libbpf/issues/953

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 1e4af62e8312e..4125133d4dab6 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -61,7 +61,8 @@ TEST_GEN_PROGS_EXTENDED = \
>  	xdp_synproxy \
>  	xskxceiver
>  
> -TEST_GEN_FILES += $(TEST_KMODS) liburandom_read.so urandom_read sign-file uprobe_multi
> +TEST_GEN_FILES += $(TEST_KMODS) liburandom_read.so urandom_read sign-file uprobe_multi \
> +	uprobe_multi_stripped.so

[Severity: Medium]
Should $(OUTPUT)/uprobe_multi_stripped.so also be added to
RUNNER_EXTRA_FILES in tools/testing/selftests/bpf/Makefile?

Currently RUNNER_EXTRA_FILES lists the other runtime fixtures:

    RUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read			\
                          $(OUTPUT)/liburandom_read.so			\
                          $(OUTPUT)/xdp_synproxy			\
                          $(OUTPUT)/sign-file				\
                          $(OUTPUT)/uprobe_multi			\

Because test_progs depends on $(RUNNER_EXTRA_FILES) (or
$(OUTPUT)/runner-extras) rather than $(TEST_GEN_FILES), running make
test_progs on a clean tree will not build uprobe_multi_stripped.so.

In addition, when flavored test runners (test_progs-no_alu32,
test_progs-cpuv4, test_progs-bpf_gcc) start, main() -> cd_flavor_subdir()
in tools/testing/selftests/bpf/test_progs.c changes the working directory
into the flavor subdirectory, where only $(RUNNER_EXTRA_FILES) are copied.
When test_attach_api_syms_stripped() in
tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c then calls:

    link = bpf_program__attach_uprobe_multi(skel->progs.uprobe, -1,
                                            "./uprobe_multi_stripped.so", NULL, &opts);

opening ./uprobe_multi_stripped.so fails with -ENOENT.

[ ... ]

> @@ -666,6 +667,10 @@ $(OUTPUT)/uprobe_multi: uprobe_multi.c uprobe_multi.ld
>  	$(Q)$(CC) $(CFLAGS) -Wl,-T,uprobe_multi.ld -O0 $(LDFLAGS) 	\
>  		$(filter-out %.ld,$^) $(LDLIBS) -o $@
>  
> +$(OUTPUT)/uprobe_multi_stripped.so: uprobe_multi_stripped.c
> +	$(call msg,BINARY,,$@)
> +	$(Q)$(CC) $(CFLAGS) -fPIC -shared -Wl,-s $(LDFLAGS) $< -o $@

[Severity: Low]
Will this target fail to link when building the BPF selftests with -static
in CFLAGS or LDFLAGS, since -static and -shared conflict when selecting
startup objects?

In tools/testing/selftests/bpf/Makefile, the $(OUTPUT)/liburandom_read.so
rule filters out -static:

    $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c liburandom_read.map
            $(call msg,LIB,,$@)
            $(Q)$(CLANG) $(CLANG_TARGET_ARCH) \
                         $(filter-out -static,$(COMMON_CFLAGS) $(LDFLAGS)) \

Would it make sense to use $(filter-out -static,$(CFLAGS) $(LDFLAGS)) for
$(OUTPUT)/uprobe_multi_stripped.so as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023141040.249597-1-hackerowskiy@gmail.com?part=1