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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/07/2025 15:02, 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
>   Found 1 legitimately skipped function due to uncertain loc
>   Ok
> 
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@xxxxxxxxxxx>

Thanks for the updated changes+test. I think it'd be good to have this
be less verbose in successful case. Currently I see:

  1: Validation of BTF encoding of functions; this may take some time: Ok
Validation of BTF encoding corner cases with test_bin functions; this
may take some time: make: Entering directory
'/home/almagui/src/github/dwarves/tests/bin'
gcc test_bin.c -Wall -Wextra -Werror -g -o test_bin
make: Leaving directory '/home/almagui/src/github/dwarves/tests/bin'
No skipped functions.  Done.

Ideally we just want the "Ok" for success in non-vebose mode. I'd
propose making the following changes in order to support that; if these
are okay by you there's no need to send another revision.

diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
index f97bdf5..a4ab67e 100755
--- a/tests/btf_functions.sh
+++ b/tests/btf_functions.sh
@@ -110,7 +110,6 @@ skipped_cnt=$(wc -l ${outdir}/skipped_fns | awk '{
print $1}')

 if [[ "$skipped_cnt" == "0" ]]; then
        echo "No skipped functions.  Done."
-       exit 0
 fi

 skipped_fns=$(awk '{print $1}' $outdir/skipped_fns)
@@ -191,17 +190,16 @@ if [[ -n "$VERBOSE" ]]; then
        echo "Found $optimized instances where the function name
suggests optimizations led to inconsistent parameters."
        echo "Found $warnings instances where pfunct did not notice
inconsistencies."
 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" && 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
+make -C ${tests_dir}/bin >/dev/null

 test -n "$VERBOSE" && printf "\nEncoding..."
 pahole --btf_features=default --lang_exclude=rust
--btf_encode_detached=$outdir/test_bin.btf \
@@ -234,10 +232,6 @@ if [[ -n "$VERBOSE" ]]; then
 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



> ---
> 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
> 





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux