Re: [PATCH bpf-next v4 4/8] selftests/bpf: spell the addr_space_cast address spaces in decimal

[bot+bpf-ci]

> selftests/bpf: spell the addr_space_cast address spaces in decimal

TL;DR: Commit message only: the last sentence about GCC and "capable binutils"
is unclear, since the edited code is still behind
__BPF_FEATURE_ADDR_SPACE_CAST. Suggests saying it prepares for the later
guard-dropping patch.

>     selftests/bpf: spell the addr_space_cast address spaces in decimal
>
>     The inline asm in the arena tests writes the two address spaces of
>     addr_space_cast() in hex which trips up a GNU assembler bug:
>
>       progs/jit_insns_arena.c:49: Error: unrecognized instruction `r7 =addr_space_cast(r7,0x0,0x1)'
>       progs/jit_insns_arena.c:49: Error: expected ',', got 'x'
>
>     This has been fixed and landed binutils commit 835aee81fa93 ("bpf: accept
>     any spelling of the addr_space_cast address spaces"), but will only land
>     in the 2.47 release, so add the workarounds for now.
>     A GCC that defines __BPF_FEATURE_ADDR_SPACE_CAST would be paired with
>     a capable binutils.
>
>     No functional change.
>
>     Signed-off-by: Vineet Gupta <vineet.gupta@xxxxxxxxx>

This isn't a bug, but the last sentence of the changelog is unclear:

  "A GCC that defines __BPF_FEATURE_ADDR_SPACE_CAST would be paired with
  a capable binutils."

Every addr_space_cast() edited in this patch sits inside an
#if defined(__BPF_FEATURE_ADDR_SPACE_CAST) block (jit_insns_arena.c,
verifier_arena.c, verifier_hmul_arena.c, and the guards in fetch_r0(),
uaf(), load_acquire() and store_release() in arena_atomics.c).

Read literally, that means none of the edited code can reach the buggy
assembler at this point.  The gas failure quoted for jit_insns_arena.c
only becomes reachable once the later commit 05738ae8e1ca ("selftests/bpf:
drop the arena guards and tag the tests that need arena") removes those
guards.

Since every addr_space_cast changed here is still behind
__BPF_FEATURE_ADDR_SPACE_CAST, which bpf-gcc does not define yet, could
the changelog say this prepares for the following patch that drops those
guards, and say what "capable binutils" means here (one that knows the
addr_space_cast mnemonic, or one that also accepts the hex spelling)?


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