Re: [PATCH v3 2/3] tests: add some tests validating skipped functions due to uncertain arg location

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 7/7/25 7:14 AM, Alexis Lothoré wrote:
> On Mon Jul 7, 2025 at 4:02 PM CEST, Alexis Lothoré (eBPF Foundation) wrote:
>> Add a small binary representing specific cases likely absent from
>> standard vmlinux or kernel modules files. As a starter, the introduced
>> binary exposes a few functions consuming structs passed by value, some
>> passed by register, some passed on the stack:
>>
>>    int main(void);
>>    int test_bin_func_struct_on_stack_ko(int, void *, char, short int, int, \
>>      void *, char, short int, struct test_bin_struct_packed);
>>    int test_bin_func_struct_on_stack_ok(int, void *, char, short int, int, \
>>      void *, char, short int, struct test_bin_struct);
>>    int test_bin_func_struct_ok(int, void *, char, struct test_bin_struct);
>>    int test_bin_func_ok(int, void *, char, short int);
>>
>> Then enrich btf_functions.sh to make it perform the following steps:
>> - build the binary
>> - generate BTF info and pfunct listing, both with dwarf and the
>>    generated BTF
>> - check that any function encoded in BTF is found in DWARF
>> - check that any function announced as skipped is indeed absent from BTF
>> - check that any skipped function has been skipped due to uncertain
>>    parameter location
>>
>> Example of the new test execution:
>>    Encoding...Matched 4 functions exactly.
>>    Ok
>>    Validation of skipped function logic...
>>    Skipped encoding 1 functions in BTF.
>>    Ok
>>    Validating skipped functions have uncertain parameter location...
>>    pahole: /home/alexis/src/pahole/tests/bin/test_bin: Invalid argument
> 
> A word about this specific error: I may have missed it in the previous
> iteration, but I systematically get this error when running the following
> command:
>    $ pahole -C test_bin_struct_packed tests/bin/test_bin
> 
> I initially thought that it would be something related to the binary being
> a userspace program and not a kernel module, but I observe the following:
> - the issue is observed even on a .ko file (tested on the previous series
>    iteration with kmod.ko)
> - the issue does not appear if there is no class filtering (ie the `-C`
>    arg) provided to pahole
> - the issue occurs as well with the packaged pahole version on my host (v1.30)
> - the struct layout is still displayed correctly despite the error
> 
> A quick bisect shows that the error log has started appearing with
> 59f5409f1357 ("dwarf_loader: Fix termination on BTF encoding error"). This
> commit has "enforced" error propagation if dwfl_getmodules returns
> something different than 0 (before, it was propagating an error only if the
> error code was negative, but dwfl_getmodules seems to be able to return
> values > 0 as well). As is sound unrelated to this series, I pushed this
> new revision anyway. [1] seems to hint that the issue is known, but in my
> case I don't get any additional log about unhandled DWARF operation. The
> issue is pretty repeatable on my side, feel free to ask for any additional
> detail or manipulation that could help.

I looked into this...

pahole_stealer may return LSK__STOP_LOADING in normal case, for example
when a class filter is provided [1]:

	if (list_empty(&class_names)) {
dump_and_stop:
		ret = LSK__STOP_LOADING;
	}

And in the dwarf_loader we abort (as with error) in case of 
LSK__STOP_LOADING [2]:

	if (cus__steal_now(dcus->cus, job->cu, dcus->conf) == LSK__STOP_LOADING)
		goto out_abort;

This was not an issue before 59f5409f1357 because of how errors were
propagated to dwfl_getmodules(), as mentioned in the other thread.

I think a proper fix for this is differentiating two variants of 
LSK__STOP_LOADING: stop because of an error, and stop because there is
nothing else to do. That would require a bit of refactoring.

Alan, Arnaldo, what do you think?

[1] https://github.com/acmel/dwarves/blob/master/pahole.c#L3390-L3392
[2] https://github.com/acmel/dwarves/blob/master/dwarf_loader.c#L3678-L3679

> 
> [1] https://lore.kernel.org/dwarves/933e199997949c0ac8a71551830f1e6c98d8bff0@linux.dev/
>>    Found 1 legitimately skipped function due to uncertain loc
>>    Ok
>>
>> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
>> ---
>> Changes in v3:
>> - bring a userspace binary instead of an OoT kernel module
>> - remove test dependency to a kernel directory being provided
>> - improve test dir detection
>>
>> Changes in v2:
>> - new patch
>> ---
>>   tests/bin/Makefile     | 10 ++++++
>>   tests/bin/test_bin.c   | 66 ++++++++++++++++++++++++++++++++++++
>>   tests/btf_functions.sh | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 167 insertions(+)
>>
>> diff --git a/tests/bin/Makefile b/tests/bin/Makefile
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..70bcf57ac4744f30fe03ea12908e42c69390f14a
>> --- /dev/null
>> +++ b/tests/bin/Makefile
>> @@ -0,0 +1,10 @@
>> +CC=${CROSS_COMPILE}gcc
>> +
>> +test_bin: test_bin.c
>> +	${CC} $^ -Wall -Wextra -Werror -g -o $@
>> +
>> +clean:
>> +	rm -rf test_bin
>> +
>> +.PHONY: clean
>> +
>> diff --git a/tests/bin/test_bin.c b/tests/bin/test_bin.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..ca6a4852cc511243925db905e55e040519af9cfd
>> --- /dev/null
>> +++ b/tests/bin/test_bin.c
>> @@ -0,0 +1,66 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <stdio.h>
>> +
>> +#define noinline __attribute__((noinline))
>> +#define __packed __attribute__((__packed__))
>> +
>> +struct test_bin_struct {
>> +	char a;
>> +	short b;
>> +	int c;
>> +	unsigned long long d;
>> +};
>> +
>> +struct test_bin_struct_packed {
>> +	char a;
>> +	short b;
>> +	int c;
>> +	unsigned long long d;
>> +}__packed;
>> +
>> +int test_bin_func_ok(int a, void *b, char c, short d);
>> +int test_bin_func_struct_ok(int a, void *b, char c, struct test_bin_struct d);
>> +int test_bin_func_struct_on_stack_ok(int a, void *b, char c, short d, int e,
>> +                                      void *f, char g, short h,
>> +                                      struct test_bin_struct i);
>> +int test_bin_func_struct_on_stack_ko(int a, void *b, char c, short d, int e,
>> +                                      void *f, char g, short h,
>> +                                      struct test_bin_struct_packed i);
>> +
>> +noinline int test_bin_func_ok(int a, void *b, char c, short d)
>> +{
>> +	return a + (long)b + c + d;
>> +}
>> +
>> +noinline int test_bin_func_struct_ok(int a, void *b, char c,
>> +                                      struct test_bin_struct d)
>> +{
>> +	return a + (long)b + c + d.a + d.b + d.c + d.d;
>> +}
>> +
>> +noinline int test_bin_func_struct_on_stack_ok(int a, void *b, char c, short d,
>> +                                               int e, void *f, char g, short h,
>> +                                               struct test_bin_struct i)
>> +{
>> +	return a + (long)b + c + d + e + (long)f + g + h + i.a + i.b + i.c + i.d;
>> +}
>> +
>> +noinline int test_bin_func_struct_on_stack_ko(int a, void *b, char c, short d,
>> +                                               int e, void *f, char g, short h,
>> +                                               struct test_bin_struct_packed i)
>> +{
>> +	return a + (long)b + c + d + e + (long)f + g + h + i.a + i.b + i.c + i.d;
>> +}
>> +
>> +int main()
>> +{
>> +	struct test_bin_struct test;
>> +	struct test_bin_struct_packed test_bis;
>> +
>> +	test_bin_func_ok(0, NULL, 0, 0);
>> +	test_bin_func_struct_ok(0, NULL, 0, test);
>> +	test_bin_func_struct_on_stack_ok(0, NULL, 0, 0, 0, NULL, 0, 0, test);
>> +	test_bin_func_struct_on_stack_ko(0, NULL, 0, 0, 0, NULL, 0, 0, test_bis);
>> +	return 0;
>> +}
>> +
>> diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
>> index c92e5ae906f90badfede86eb530108894fbc8c93..fb62b0b56662bb2ae58f7adc0a022c400cba5e0f 100755
>> --- a/tests/btf_functions.sh
>> +++ b/tests/btf_functions.sh
>> @@ -193,4 +193,95 @@ if [[ -n "$VERBOSE" ]]; then
>>   fi
>>   echo "Ok"
>>   
>> +# Some specific cases can not  be tested directly with a standard kernel.
>> +# We can use the small binary in bin/ to test those cases, like packed
>> +# structs passed on the stack.
>> +
>> +echo -n "Validation of BTF encoding corner cases with test_bin functions; this may take some time: "
>> +
>> +test -n "$VERBOSE" && printf "\nBuilding test_bin..."
>> +tests_dir=$(realpath $(dirname $0))
>> +make -C ${tests_dir}/bin
>> +
>> +test -n "$VERBOSE" && printf "\nEncoding..."
>> +pahole --btf_features=default --lang_exclude=rust --btf_encode_detached=$outdir/test_bin.btf \
>> +	--verbose ${tests_dir}/bin/test_bin | grep "skipping BTF encoding of function" \
>> +	> ${outdir}/test_bin_skipped_fns
>> +
>> +funcs=$(pfunct --format_path=btf $outdir/test_bin.btd 2>/dev/null|sort)
>> +pfunct --all --no_parm_names --format_path=dwarf bin/test_bin | \
>> +	sort|uniq > $outdir/test_bin_dwarf.funcs
>> +pfunct --all --no_parm_names --format_path=btf $outdir/test_bin.btf 2>/dev/null|\
>> +	awk '{ gsub("^(bpf_kfunc |bpf_fastcall )+",""); print $0}'|sort|uniq > $outdir/test_bin_btf.funcs
>> +
>> +exact=0
>> +while IFS= read -r btf ; do
>> +	# Matching process can be kept simpler as the tested binary is
>> +	# specifically tailored for tests
>> +	dwarf=$(grep -F "$btf" $outdir/test_bin_dwarf.funcs)
>> +	if [[ "$btf" != "$dwarf" ]]; then
>> +		echo "ERROR: mismatch : BTF '$btf' not found; DWARF '$dwarf'"
>> +		fail
>> +	else
>> +		exact=$((exact+1))
>> +	fi
>> +done < $outdir/test_bin_btf.funcs
>> +
>> +if [[ -n "$VERBOSE" ]]; then
>> +	echo "Matched $exact functions exactly."
>> +	echo "Ok"
>> +	echo "Validation of skipped function logic..."
>> +fi
>> +
>> +skipped_cnt=$(wc -l ${outdir}/test_bin_skipped_fns | awk '{ print $1}')
>> +if [[ "$skipped_cnt" == "0" ]]; then
>> +	echo "No skipped functions.  Done."
>> +	exit 0
>> +fi
>> +
>> +skipped_fns=$(awk '{print $1}' $outdir/test_bin_skipped_fns)
>> +for s in $skipped_fns ; do
>> +	# Ensure the skipped function are not in BTF
>> +	inbtf=$(grep " $s(" $outdir/test_bin_btf.funcs)
>> +	if [[ -n "$inbtf" ]]; then
>> +		echo "ERROR: '${s}()' was added incorrectly to BTF: '$inbtf'"
>> +		fail
>> +	fi
>> +done
>> +
>> +if [[ -n "$VERBOSE" ]]; then
>> +	echo "Skipped encoding $skipped_cnt functions in BTF."
>> +	echo "Ok"
>> +	echo "Validating skipped functions have uncertain parameter location..."
>> +fi
>> +
>> +uncertain_loc=$(awk '/due to uncertain parameter location/ { print $1 }' $outdir/test_bin_skipped_fns)
>> +legitimate_skip=0
>> +
>> +for f in $uncertain_loc ; do
>> +	# Extract parameters types
>> +	raw_params=$(grep ${f} $outdir/test_bin_dwarf.funcs|sed -n 's/^[^(]*(\([^)]*\)).*/\1/p')
>> +	IFS=',' read -ra params <<< "${raw_params}"
>> +	for param in "${params[@]}"
>> +	do
>> +		# Search any param that could be a struct
>> +		struct_type=$(echo ${param}|grep -E '^struct [^*]' | sed -E 's/^struct //')
>> +		if [ -n "${struct_type}" ]; then
>> +			# Check with pahole if the struct is detected as
>> +			# packed
>> +			if pahole -F dwarf -C "${struct_type}" ${tests_dir}/bin/test_bin|tail -n 2|grep -q __packed__
>> +			then
>> +				legitimate_skip=$((legitimate_skip+1))
>> +				continue 2
>> +			fi
>> +		fi
>> +	done
>> +	echo "ERROR: '${f}()' should not have been skipped; it has no parameter with uncertain location"
>> +	fail
>> +done
>> +
>> +if [[ -n "$VERBOSE" ]]; then
>> +	echo "Found ${legitimate_skip} legitimately skipped function due to uncertain loc"
>> +fi
>> +echo "Ok"
>>   exit 0
> 
> 
> 
>