Re: [PATCH bpf 2/2] selftests/bpf: Exercise stack argument precision on state reuse

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] selftests/bpf: multi-line comments violate BPF comment style
- [Low] selftests/bpf: test_stack_arg_precision_reuse() skips supported RV64
--

commit 46da3caeb85de86ad2e22a94e1a61ab12b6a1cfc
Author: Xu Yunxiang <xyx2021@mail.ustc.edu.cn>

selftests/bpf: Exercise stack argument precision on state reuse

This commit adds load-only BPF verifier selftests in
tools/testing/selftests/bpf/prog_tests/stack_arg_precision_reuse.c to
exercise stack argument precision tracking across state reuse and
conservative precision fallback. It constructs BPF instructions and BTF
directly so the tests can run even when the compiler cannot emit stack
arguments.

> diff --git a/tools/testing/selftests/bpf/prog_tests/stack_arg_precision_reuse.c b/tools/testing/selftests/bpf/prog_tests/stack_arg_precision_reuse.c
> new file mode 100644
> index 0000000000000..4497d959247d7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/stack_arg_precision_reuse.c
> @@ -0,0 +1,300 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include <bpf/btf.h>
> +
> +#define BPF_REG_PARAMS 11
> +
> +/* Construct BTF and instructions directly so these tests also run with
> + * compilers which cannot emit stack arguments yet.
> + */
> +struct test_program {

[ ... ]

> +static bool build_reuse(struct test_program *p, int map_fd,
> +			const struct reuse_test *test)
> +{

[ ... ]

> +	/* M: paths 2 and 3 meet with their offsets still in r6. Path 2
> +	 * must acquire precision here when it reuses path 1's state at P.
> +	 */
> +	jump_to(p, path2, p->insn_cnt);

[ ... ]

> +	/* Different precise r2 values prevent reuse before the callee.
> +	 * They converge here while the offset is still in the caller's slot.
> +	 */
> +	if (test->callee_join) {

[ ... ]

> +static bool build_fallback(struct test_program *p, int map_fd, bool callee)
> +{

[ ... ]

> +	/* Subtraction turns r1 into a scalar. Precision backtracking across
> +	 * the store through its old pointer value needs conservative fallback.
> +	 * The resulting offset is bounded, so this program is always safe.
> +	 */
> +	emit(p, BPF_MOV64_REG(BPF_REG_1, BPF_REG_7));

[Severity: Low]
This isn't a bug, but in
tools/testing/selftests/bpf/prog_tests/stack_arg_precision_reuse.c, the four
multi-line comments above struct test_program, inside build_reuse(), and
inside build_fallback() place text on the opening /* line. Could these use
the standard BPF multi-line comment style with /* on its own line?

[ ... ]

> +void test_stack_arg_precision_reuse(void)
> +{
> +	const struct reuse_test tests[] = {
> +		{ .name = "arg6", .slot = 0, .offset = 32 },
> +		{ .name = "arg7", .slot = 1, .offset = 32 },
> +		{ .name = "caller_frame", .slot = 0, .offset = 32, .callee_join = true },
> +		{ .name = "nested_caller_frame", .slot = 1, .offset = 32,
> +		  .callee_join = true, .nested = true },
> +		{ .name = "in_bounds", .slot = 1, .offset = 8 },
> +		{ .name = "discarded", .slot = 0, .offset = 32, .discard = true },
> +		{ .name = "register_control", .slot = -1, .offset = 32 },
> +	};
> +	struct test_program p;
> +	int map_fd, i;
> +
> +	/* Stack argument JIT support currently exists on x86-64 and arm64. */
> +#if !defined(__x86_64__) && !defined(__aarch64__)
> +	test__skip();
> +	return;
> +#endif

[Severity: Low]
In test_stack_arg_precision_reuse() in
tools/testing/selftests/bpf/prog_tests/stack_arg_precision_reuse.c, when
compiled and run on 64-bit RISC-V, this guard unconditionally calls
test__skip().

However, arch/riscv/net/bpf_jit_comp64.c implements
bpf_jit_supports_stack_args() returning true, and other stack argument
selftests such as tools/testing/selftests/bpf/progs/stack_arg.c enable stack
argument testing on 64-bit RISC-V.

Should this condition and its comment also include 64-bit RISC-V (for
example, defined(__riscv) && __riscv_xlen == 64) so these verifier tests are
not skipped on RV64?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027061303.1754535-1-xyx2021@mail.ustc.edu.cn?part=2