Re: [PATCH bpf-next v2 1/8] selftests/bpf: Build callx tests with GCC that defines __BPF_FEATURE_CALLX

[bot+bpf-ci]

> selftests/bpf: Build callx tests with GCC that defines __BPF_FEATURE_CALLX

TL;DR:
- data_in_arena.c use_ops() isn't converted to CAN_USE_CALLX, so GCC with
  callx support may still call add1() directly; commit message claims all
  tests converted
- Commit message typo (nit)

> selftests/bpf: Build callx tests with GCC that defines __BPF_FEATURE_CALLX
>
> Commit 7cd1f73b37ea ("selftests/bpf: Fix build of callx tests with GCC")
> gated the callx progs on __clang__. GCC had indirect call support which was
> recently refurbished along with a __BPF_FEATURE_CALLX macro [1], so gate
> on that as well so once this GCC is released, the tests start passing.
>
> Put the arch and compiler conditions in a CAN_USE_CALLX macro in
> bpf_misc.h, and use it all the test programs.
>
> While at it, mark the placeholder programs left for unsupported
> toolchains and arches with __skip(), so they report SKIP rather than OK.
>
> [1] https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=ffe3ead4a8ad704b557b0cfa4f823df647c65a0d
>
> Measured on bpf-next with a stock bpf-gcc that defines
> __BPF_FEATURE_CALLX, test_progs before and after: 76 subtests run that
> previously did not exist in the report, 75 OK and one @unpriv SKIP,
> with no failures.  The two dummy placeholders retire and
> callx_rodata_lskel goes SKIP -> OK.
>
> Signed-off-by: Vineet Gupta <vineet.gupta@xxxxxxxxx>

The commit message says CAN_USE_CALLX is used in all the test programs, but
use_ops() in progs/data_in_arena.c is not converted.  It still gates its
indirect call on __clang__ and keeps the same comment this patch removes from
the other files:

progs/data_in_arena.c:use_ops() {
    ...
#ifdef __clang__
	return ops.fn(*ops.data) + ops.name[1];
#else
	/* gcc doesn't support indirect calls */
	return add1(*ops.data) + ops.name[1];
#endif
    ...
}

With a GCC that defines __BPF_FEATURE_CALLX, doesn't this still take the
#else branch and call add1() directly?  That means the GCC build never makes
the callx through a function pointer in .data.rel.ro, and the "gcc doesn't
support indirect calls" comment is now wrong for that compiler.

A grep for "indirect call" under progs/ and prog_tests/ shows this is the
only remaining gate.  None of the later patches in the series touch
data_in_arena.c.

Should use_ops() be converted to CAN_USE_CALLX as well?  That would need
data_in_arena.c to include bpf_misc.h, and it would add the x86/arm64/riscv64
arch gate that clang builds of this file don't have today.  Otherwise, could
the commit message be adjusted so it doesn't say all the test programs were
converted?

This isn't a bug, but there is a missing word in the commit message:

> bpf_misc.h, and use it all the test programs.

This should read "use it in all the test programs".


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40768193895