Re: [bpf-next PATCH 2/2] selftests/bpf: Enable stack argument tests for riscv

[email protected]
Newsgroups org.kernel.vger.bpf
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Stack argument selftests are incorrectly enabled for 32-bit RISC-V (RV32) targets, leading to test regressions.
--

--- Patch [2]: [bpf-next PATCH 2/2] selftests/bpf: Enable stack argument tests for riscv ---
commit 4c9f49e3a0c01bc5c673e83a3a3e5b7f61947d45
Author: Feng Jiang <[email protected]>

selftests/bpf: Enable stack argument tests for riscv

This commit adds __TARGET_ARCH_riscv to the architecture guard in all stack
argument selftests so the real test cases are compiled for RISC-V. It keeps
the __BPF_FEATURE_STACK_ARGUMENT gate defined by clang.

Link: https://github.com/llvm/llvm-project/pull/189060

> diff --git a/tools/testing/selftests/bpf/progs/btf__stack_arg_precision.c b/tools/testing/selftests/bpf/progs/btf__stack_arg_precision.c
> index 8d38aafe66a28..c8540d30e293d 100644
> --- a/tools/testing/selftests/bpf/progs/btf__stack_arg_precision.c
> +++ b/tools/testing/selftests/bpf/progs/btf__stack_arg_precision.c
> @@ -4,7 +4,7 @@
>  #include <bpf/bpf_helpers.h>
>  #include "../test_kmods/bpf_testmod_kfunc.h"
>  
> -#if (defined(__TARGET_ARCH_x86) || defined(__TARGET_ARCH_arm64)) && \
> +#if (defined(__TARGET_ARCH_x86) || defined(__TARGET_ARCH_arm64) || defined(__TARGET_ARCH_riscv)) && \
>  	defined(__BPF_FEATURE_STACK_ARGUMENT)

[Severity: Medium]
Does this incorrectly enable the tests for 32-bit RISC-V (RV32) as well?

The BPF selftests build system defines __TARGET_ARCH_riscv for both RV32
and RV64, but BPF stack arguments are only supported in the RV64 JIT.

If these tests are compiled and run on RV32, the verifier will likely reject
programs with more than 5 arguments (returning -ENOTSUPP or -EINVAL),
causing test regressions on that architecture.

Could this be restricted to RV64 by using
(defined(__TARGET_ARCH_riscv) && __riscv_xlen == 64) across all of the
modified selftest files?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.