public inbox for dwarves@vger.kernel.org
 help / color / mirror / Atom feed
From: Ihor Solodrai <ihor.solodrai@linux.dev>
To: "Alexis Lothoré (eBPF Foundation)" <alexis.lothore@bootlin.com>,
	dwarves@vger.kernel.org
Cc: bpf@vger.kernel.org, Alan Maguire <alan.maguire@oracle.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Alexei Starovoitov <ast@fb.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Bastien Curutchet <bastien.curutchet@bootlin.com>,
	ebpf@linuxfoundation.org
Subject: Re: [PATCH v2 2/3] tests: add some tests validating skipped functions due to uncertain arg location
Date: Thu, 3 Jul 2025 11:31:03 -0700	[thread overview]
Message-ID: <f696f834-bca6-4f9e-a81e-f7e45126e2eb@linux.dev> (raw)
In-Reply-To: <20250703-btf_skip_structs_on_stack-v2-2-4767e3ba10c9@bootlin.com>

On 7/3/25 2:02 AM, Alexis Lothoré (eBPF Foundation) wrote:
> Add a small kernel module representing specific cases likely absent from
> standard vmlinux files. As a starter, the introduced module exposes a
> few functions consuming structs passed by value, some passed by
> register, some passed on the stack:
> 
>    int kmod_test_init(void);
>    int test_kmod_func_ok(int, void *, char, short int);
>    int test_kmod_func_struct_ok(int, void *, char, struct kmod_struct);
>    int test_kmod_func_struct_on_stack_ok(int, void *, char, short int, int, \
>      void *, char, short int, struct kmod_struct);
>    int test_kmod_func_struct_on_stack_ko(int, void *, char, short int, int, \
>      void *, char, short int, struct kmod_struct_packed);
> 
> Then enrich btf_functions.sh to make it perform the following steps:
> - build the module
> - 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
> 
> Those new tests are executed only if a kernel directory is provided as
> script's second argument, they are otherwise skipped.

While this shouldn't be a problem for CI, since it checks out a kernel
tree to test vmlinux as input, I wonder if there is a way to do the
same test without this dependency.

We need to generate a binary with DWARF, containing function
prototypes with packed/aligned attributes. Give it to pahole and see
that those functions were skipped.

Any reason it must be a kernel module? Am I missing something?


> 
> 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...
>    Found 1 legitimately skipped function due to uncertain loc
>    Ok
> 
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
> ---
> Changes in v2:
> - new patch
> ---
>   tests/btf_functions.sh | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   tests/kmod/Makefile    |  1 +
>   tests/kmod/kmod.c      | 69 +++++++++++++++++++++++++++++++++++
>   3 files changed, 169 insertions(+)
> 
> diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
> index c92e5ae906f90badfede86eb530108894fbc8c93..64810b7eb51e7f2693929fbf66e0641a9d4e0277 100755
> --- a/tests/btf_functions.sh
> +++ b/tests/btf_functions.sh
> @@ -193,4 +193,103 @@ if [[ -n "$VERBOSE" ]]; then
>   fi
>   echo "Ok"
>   
> +# Some specific cases can not  be tested directly with a standard kernel.
> +# We can use the kernel module in kmod/ to test those cases, like packed
> +# structs passed on the stack. Run this test only if we have the needed
> +# dependencies (eg: some kernel sources directory passed as argument; those
> +# must match the used vmlinux file)
> +
> +KDIR=${KDIR:-$2}
> +if [ -z "$KDIR" ] ; then
> +	echo "Skipping kmod tests"
> +	exit 0
> +fi
> +
> +echo -n "Validation of BTF encoding corner cases with kmod functions; this may take some time: "
> +
> +test -n "$VERBOSE" && printf "\nBuilding kmod..."
> +tests_dir=$(dirname $0)
> +make -C ${KDIR} M=${tests_dir}/kmod

This part fails for me:

isolodrai@isolodrai-fedora-PC2K40WQ:~/pahole/tests$ 
KDIR=/home/isolodrai/kernels/bpf-next 
vmlinux=/home/isolodrai/kernels/bpf-next/vmlinux ./btf_functions.sh
Validation of BTF encoding of functions; this may take some time: Ok
Validation of BTF encoding corner cases with kmod functions; this may 
take some time: make: Entering directory '/home/isolodrai/kernels/bpf-next'
Makefile:199: *** specified external module directory "./kmod" does not 
exist.  Stop.
make: Leaving directory '/home/isolodrai/kernels/bpf-next'
No skipped functions.  Done.

Maybe:

diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
index 64810b7..fcb1591 100755
--- a/tests/btf_functions.sh
+++ b/tests/btf_functions.sh
@@ -208,7 +208,7 @@ fi
  echo -n "Validation of BTF encoding corner cases with kmod functions; 
this may take some time: "

  test -n "$VERBOSE" && printf "\nBuilding kmod..."
-tests_dir=$(dirname $0)
+tests_dir=$(realpath $(dirname $0))
  make -C ${KDIR} M=${tests_dir}/kmod

  test -n "$VERBOSE" && printf "\nEncoding..."


Also, in case kernel is built with LLVM, one must set LLVM=1.
Not sure if this is detectable by the test.

> +
> +test -n "$VERBOSE" && printf "\nEncoding..."
> +pahole --btf_features=default --lang_exclude=rust --btf_encode_detached=$outdir/kmod.btf \
> +	--verbose ${tests_dir}/kmod/kmod.ko | grep "skipping BTF encoding of function" \
> +	> ${outdir}/kmod_skipped_fns
> +
> +funcs=$(pfunct --format_path=btf $outdir/kmod.btf 2>/dev/null|sort)
> +pfunct --all --no_parm_names --format_path=dwarf kmod/kmod.ko | \
> +	sort|uniq > $outdir/kmod_dwarf.funcs
> +pfunct --all --no_parm_names --format_path=btf $outdir/kmod.btf 2>/dev/null|\
> +	awk '{ gsub("^(bpf_kfunc |bpf_fastcall )+",""); print $0}'|sort|uniq > $outdir/kmod_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/kmod_dwarf.funcs)
> +	if [[ "$btf" != "$dwarf" ]]; then
> +		echo "ERROR: mismatch : BTF '$btf' not found; DWARF '$dwarf'"
> +		fail
> +	else
> +		exact=$((exact+1))
> +	fi
> +done < $outdir/kmod_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}/kmod_skipped_fns | awk '{ print $1}')
> +if [[ "$skipped_cnt" == "0" ]]; then
> +	echo "No skipped functions.  Done."
> +	exit 0
> +fi
> +
> +skipped_fns=$(awk '{print $1}' $outdir/kmod_skipped_fns)
> +for s in $skipped_fns ; do
> +	# Ensure the skipped function are not in BTF
> +	inbtf=$(grep " $s(" $outdir/kmod_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/kmod_skipped_fns)
> +legitimate_skip=0
> +
> +for f in $uncertain_loc ; do
> +	# Extract parameters types
> +	raw_params=$(grep ${f} $outdir/kmod_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 -C "${struct_type}" kmod/kmod.ko|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
> diff --git a/tests/kmod/Makefile b/tests/kmod/Makefile
> new file mode 100644
> index 0000000000000000000000000000000000000000..e7c2ed929eaf81e91429f744c3778156ed2be2d2
> --- /dev/null
> +++ b/tests/kmod/Makefile
> @@ -0,0 +1 @@
> +obj-m += kmod.o
> diff --git a/tests/kmod/kmod.c b/tests/kmod/kmod.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..5b93614b6156b05925a6cb48809ad63533ccba3e
> --- /dev/null
> +++ b/tests/kmod/kmod.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/module.h>
> +#include <linux/printk.h>
> +
> +struct kmod_struct {
> +	char a;
> +	short b;
> +	int c;
> +	unsigned long long d;
> +};
> +
> +struct kmod_struct_packed {
> +	char a;
> +	short b;
> +	int c;
> +	unsigned long long d;
> +}__packed;
> +
> +int test_kmod_func_ok(int a, void *b, char c, short d);
> +int test_kmod_func_struct_ok(int a, void *b, char c, struct kmod_struct d);
> +int test_kmod_func_struct_on_stack_ok(int a, void *b, char c, short d, int e,
> +                                      void *f, char g, short h,
> +                                      struct kmod_struct i);
> +int test_kmod_func_struct_on_stack_ko(int a, void *b, char c, short d, int e,
> +                                      void *f, char g, short h,
> +                                      struct kmod_struct_packed i);
> +
> +noinline int test_kmod_func_ok(int a, void *b, char c, short d)
> +{
> +	return a + (long)b + c + d;
> +}
> +
> +noinline int test_kmod_func_struct_ok(int a, void *b, char c,
> +                                      struct kmod_struct d)
> +{
> +	return a + (long)b + c + d.a + d.b + d.c + d.d;
> +}
> +
> +noinline int test_kmod_func_struct_on_stack_ok(int a, void *b, char c, short d,
> +                                               int e, void *f, char g, short h,
> +                                               struct kmod_struct i)
> +{
> +	return a + (long)b + c + d + e + (long)f + g + h + i.a + i.b + i.c + i.d;
> +}
> +
> +noinline int test_kmod_func_struct_on_stack_ko(int a, void *b, char c, short d,
> +                                               int e, void *f, char g, short h,
> +                                               struct kmod_struct_packed i)
> +{
> +	return a + (long)b + c + d + e + (long)f + g + h + i.a + i.b + i.c + i.d;
> +}
> +
> +static int kmod_test_init(void)
> +{
> +	struct kmod_struct test;
> +	struct kmod_struct_packed test_bis;
> +
> +	test_kmod_func_ok(0, NULL, 0, 0);
> +	test_kmod_func_struct_ok(0, NULL, 0, test);
> +	test_kmod_func_struct_on_stack_ok(0, NULL, 0, 0, 0, NULL, 0, 0, test);
> +	test_kmod_func_struct_on_stack_ko(0, NULL, 0, 0, 0, NULL, 0, 0, test_bis);
> +	return 0;
> +}
> +
> +module_init(kmod_test_init);
> +
> +MODULE_AUTHOR("Alexis Lothoré");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Pahole testing module");
> 


  reply	other threads:[~2025-07-03 18:31 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-03  9:02 [PATCH v2 0/3] btf_encoder: do not encode functions consuming packed structs on stack Alexis Lothoré (eBPF Foundation)
2025-07-03  9:02 ` [PATCH v2 1/3] btf_encoder: skip functions consuming packed structs passed by value " Alexis Lothoré (eBPF Foundation)
2025-07-03 18:17   ` Ihor Solodrai
2025-07-04  9:01     ` Alexis Lothoré
2025-07-04 19:59       ` Ihor Solodrai
2025-07-04 21:10         ` Alexis Lothoré
2025-07-04 20:05   ` Ihor Solodrai
2025-07-04 21:12     ` Alexis Lothoré
2025-07-03  9:02 ` [PATCH v2 2/3] tests: add some tests validating skipped functions due to uncertain arg location Alexis Lothoré (eBPF Foundation)
2025-07-03 18:31   ` Ihor Solodrai [this message]
2025-07-04  9:06     ` Alexis Lothoré
2025-07-03  9:02 ` [PATCH v2 3/3] gitignore: ignore all the test kmod build-related files Alexis Lothoré (eBPF Foundation)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f696f834-bca6-4f9e-a81e-f7e45126e2eb@linux.dev \
    --to=ihor.solodrai@linux.dev \
    --cc=acme@kernel.org \
    --cc=alan.maguire@oracle.com \
    --cc=alexis.lothore@bootlin.com \
    --cc=ast@fb.com \
    --cc=bastien.curutchet@bootlin.com \
    --cc=bpf@vger.kernel.org \
    --cc=dwarves@vger.kernel.org \
    --cc=ebpf@linuxfoundation.org \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox