Re: [PATCH bpf-next v4 6/8] selftests/bpf: drop the arena guards and tag the tests that need arena

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 6824 bytes --]

> selftests/bpf: drop the arena guards and tag the tests that need arena

TL;DR: Removing the addr_space_cast guards may break the selftest build with
clang 17/18, whose assembler lacks that mnemonic, so test_progs may fail to
build; keep a guard or emit raw bytes.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena.c b/tools/testing/selftests/bpf/progs/verifier_arena.c
> --- a/tools/testing/selftests/bpf/progs/verifier_arena.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_arena.c

[ ... ]

> @@ -503,13 +482,10 @@ int arena_kfuncs_under_bpf_lock(void *ctx)
>  	bpf_arena_free_pages(&arena, page, 1);
>  
>  	bpf_spin_unlock(&arena_bpf_test_lock);
> -#endif
>  
>  	return 0;
>  }
>  
> -#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
> -
>  /*
>   * Test that scalar += PTR_TO_ARENA correctly upgrades the
>   * destination register to a PTR_TO_ARENA.
[ ... ]
> -#endif /* defined(__BPF_FEATURE_ADDR_SPACE_CAST) */
> -

Can this break the build with clang 17 and 18?  The removed guard also
kept the textual addr_space_cast mnemonic away from assemblers that do not
know it, since that mnemonic and __BPF_FEATURE_ADDR_SPACE_CAST both
arrived in LLVM 19.  Asm such as this is now always assembled:

    asm volatile (
        "%[arena_ptr] = 8192;"
        "%[arena_ptr] = addr_space_cast(%[arena_ptr], 0, 1);"
        ...

in scalar_add_arena_ptr(), arena_ptr_add_arena_ptr(),
mixed_arena_scalar_alu64_scalar_first(),
mixed_arena_scalar_alu64_arena_first(), scalar_xor_arena_ptr(),
arena_ptr_add_to_non_arena_ptr(), non_arena_ptr_add_to_arena_ptr() and
mixed_arena_stack_atomic().

verifier_arena.skel.h is included by prog_tests/verifier.c, so test_progs
and its no_alu32 and cpuv4 flavors would fail to build, not just these
subtests.  __skip_without_arena does not help since this fails at build
time.

Other places in the tree keep the guard for this reason, for example
arena_atomics.c and commit 9e35f216e5c5.  Would a guard like
"defined(__BPF_FEATURE_ADDR_SPACE_CAST) || !defined(__clang__)", or
emitting the cast as raw bytes (as bpf_addr_space_cast() in
bpf_experimental.h does), keep both bpf-gcc and older clang working?

> diff --git a/tools/testing/selftests/bpf/progs/verifier_hmul_arena.c b/tools/testing/selftests/bpf/progs/verifier_hmul_arena.c
> --- a/tools/testing/selftests/bpf/progs/verifier_hmul_arena.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_hmul_arena.c
> @@ -5,8 +5,6 @@
>  #include "bpf_misc.h"
>  #include "../../../include/linux/filter.h"
>  
> -#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
> -
>  #define BPF_UHMUL64_IMM(DST, IMM) \
>  	BPF_RAW_INSN(BPF_ALU64 | BPF_MUL | BPF_K, DST, 0, BPF_UHMUL, IMM)
>  
[ ... ]
> -#else
> -
> -SEC("socket")
> -__description("HMUL arena tests require compiler support for addr_space_cast")
> -__skip("compiler doesn't support addr_space_cast")
> -__success
> -int dummy_test(void)
> -{
> -	return 0;
> -}
> -
> -#endif
> -
>  char _license[] SEC("license") = "GPL";

Can this break the build with clang 17 and 18?  uhmul64_arena_ptr() and
shmul64_arena_ptr() contain:

    "r0 = addr_space_cast(r0, 0, 1);"
    "r1 = addr_space_cast(r1, 0, 1);"

The guard was about assembler support for that mnemonic (LLVM 19 and
later), not about compiler arena codegen.  With clang 17 or 18 the
integrated assembler rejects these lines, so verifier_hmul_arena.bpf.o
fails to build, and test_progs with it.  Previously those toolchains got
the dummy SKIP.

Commit ddd86412ac01 which added this file says "Retain
__BPF_FEATURE_ADDR_SPACE_CAST gating and a dummy skip program for
toolchains without addr_space_cast support."

Could the guard be kept in a form that both bpf-gcc and LLVM 19 or later
satisfy, for example "defined(__BPF_FEATURE_ADDR_SPACE_CAST) ||
!defined(__clang__)"?  Or could the cast be emitted as raw bytes with
.8byte/__imm_insn(), the way the HMUL instruction itself is emitted here?

> diff --git a/tools/testing/selftests/bpf/progs/verifier_ldsx.c b/tools/testing/selftests/bpf/progs/verifier_ldsx.c
> --- a/tools/testing/selftests/bpf/progs/verifier_ldsx.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_ldsx.c
> @@ -257,8 +257,6 @@ __naked void ldsx_ctx_8(void)
>  	: __clobber_all);
>  }
>  
> -#ifdef __BPF_FEATURE_ADDR_SPACE_CAST
> -
>  struct {
>  	__uint(type, BPF_MAP_TYPE_ARENA);
>  	__uint(map_flags, BPF_F_MMAPABLE);

Can this break the build with clang 18?  The outer guard of this file
accepts any clang 18 or later:

    (__clang_major__ >= 18 || defined(__BPF_FEATURE_LDSX))

but clang 18 does not have the textual addr_space_cast mnemonic.  The
inner __BPF_FEATURE_ADDR_SPACE_CAST check was what kept the Arena LDSX
programs away from it.  Now arena_ldsx_disasm(), arena_ldsx_exception()
and arena_ldsx_s8/s16/s32() are assembled, and lines such as

    "r0 = addr_space_cast(r0, 0, 1);"

are rejected, so verifier_ldsx.bpf.o fails to build.  Would an inner
guard like "defined(__BPF_FEATURE_ADDR_SPACE_CAST) || !defined(__clang__)"
keep both working?

> diff --git a/tools/testing/selftests/bpf/progs/verifier_zext.c b/tools/testing/selftests/bpf/progs/verifier_zext.c
> --- a/tools/testing/selftests/bpf/progs/verifier_zext.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_zext.c
> @@ -324,8 +324,6 @@ __naked void alu64_and_conditionals(void)
>  	: __clobber_all);
>  }
>  
> -#ifdef __BPF_FEATURE_ADDR_SPACE_CAST
> -
>  SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
>  __arch_s390x
>  __xlated("7: w1 = w0")
[ ... ]
> -#endif
> -
>  /* Check if probe mem loads keep their zero extension. */

Can this break the build with clang that lacks the addr_space_cast
mnemonic?  arena_ptr() and zext_arena_cmpxchg32() now always contain:

    r1 = addr_space_cast(r0, 0, 1);
    r2 = addr_space_cast(r1, 1, 0);
    r1 = addr_space_cast(r1, 0, 1);

With clang 18 or older the assembler rejects these, so verifier_zext.bpf.o
fails to build.  That also takes the roughly twenty non-arena zext
subtests in the same object with it, and since prog_tests/verifier.c
includes verifier_zext.skel.h, test_progs fails to build.
__skip_without_arena cannot help because it is a runtime test_loader tag.

zext_arena_cmpxchg32() already emits its cmpxchg as .8byte through
__imm_insn(BPF_ATOMIC_OP(...)).  Could the two casts be encoded the same
way, for example with __imm_insn(BPF_RAW_INSN(BPF_ALU64 | BPF_MOV |
BPF_X, dst, src, BPF_ADDR_SPACE_CAST, (dst_as << 16) | src_as))?  The
alternative is to keep a compile-time guard for compilers that lack the
mnemonic.

The same pattern appears in the other files unguarded here:
jit_insns_arena.c and compute_live_registers.c.


---
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/41694978246