Re: [PATCH bpf-next v2 19/20] selftests/bpf: Cover the exception cleanup shapes the chain does not reach

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: multi-line comment style violation
--

commit 59a07b8ba8f5c3976a5da9bea3b5d6359874813a
Author: Yonghong Song <yonghong.song@linux.dev>

selftests/bpf: Cover the exception cleanup shapes the chain does not reach

This commit adds end-to-end exception cleanup tests for various edge cases
and shapes that the primary call chain does not reach. It tests things like
tail calls, nounwind calls, subprogram pads, and landing pads that interact
with the stack.

> diff --git a/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c b/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c
> index d1e45b765af26..ffc0b95191681 100644
> --- a/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c
> +++ b/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c

[ ... ]

> @@ -35,6 +39,314 @@ static void run(struct exceptions_cleanup *skel, __u64 input, __u32 retval,
> +static void test_shapes(void)
> +{

[ ... ]

> +	/* The same call site either way: the subprogram's throw unwinds into
> +	 * this frame and runs its pad, an extension's stops at its own boundary.
> +	 */

[Severity: Low]
This isn't a bug, but does this comment follow the BPF subsystem coding style?
Multi-line comments should have their opening /* on its own line before the
text begins.

> +	if (test__start_subtest("freplace_subprog_throws"))
> +		run_shape(skel, skel->progs.entry_freplace, 7, THROW_COOKIE,
> +			  RAN_FREPLACE);
> +	if (test__start_subtest("freplace_extension_throws"))
> +		test_freplace(skel);
> +
> +	/* A tail call that is taken: the walk ends at the target, so the cookie
> +	 * comes back from there and this frame's pad does not run.
> +	 */

[Severity: Low]
This isn't a bug, but is this the preferred multi-line comment format? The BPF
subsystem coding style mandates that the opening /* should be on its own line.

> +	if (test__start_subtest("tail_call_taken")) {
> +		int key = 0, prog_fd = bpf_program__fd(skel->progs.tc_target);

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/exceptions_cleanup_shapes.c b/tools/testing/selftests/bpf/progs/exceptions_cleanup_shapes.c
> new file mode 100644
> index 0000000000000..f5eb2ff15c896
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/exceptions_cleanup_shapes.c

[ ... ]

> @@ -0,0 +1,863 @@
> +static __used __noinline __u64 tc_thrower(void *ctx)
> +{
> +	/* Never taken; its presence is what makes this frame, whose spill the
> +	 * walker reads, tail-call-reachable.
> +	 */

[Severity: Low]
This isn't a bug, but could this comment formatting be adjusted? Multi-line
comments in the BPF subsystem should place their opening /* on a separate line.

> +	bpf_tail_call_static(ctx, &jmp_table, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918044156.3283973-1-yonghong.song@linux.dev?part=19