Re: [PATCH bpf-next v2 3/3] selftests/bpf: Enable some of the blocked cpuv4 instruction tests for bpf-gcc

[Vineet Gupta <vineet.gupta@linux.dev>]

On 8/24/26 2:14 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added `#ifdef __BPF_FEATURE_ADDR_SPACE_CAST` guard unintentionally drops 5 arena tests from the default test_progs flavour for clang.
> --
>
> --- Patch [3]: [PATCH bpf-next v2 3/3] selftests/bpf: Enable some of the blocked cpuv4 instruction tests for bpf-gcc ---
> commit ad6608dfd47ddd6a7068cdc6e0e6d467ec3558b8
> Author: Vineet Gupta <vineet.gupta@linux.dev>
>
> selftests/bpf: Enable some of the blocked cpuv4 instruction tests for bpf-gcc
>
> This commit relaxes clang-only checks in BPF selftests by adding specific
> feature checks like __BPF_FEATURE_LDSX to allow building under bpf-gcc.
> It also wraps arena tests with __BPF_FEATURE_ADDR_SPACE_CAST to skip them
> on bpf-gcc.
>
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_ldsx.c b/tools/testing/selftests/bpf/progs/verifier_ldsx.c
>> index e58ea69e38549..75762e11b443f 100644
>> --- a/tools/testing/selftests/bpf/progs/verifier_ldsx.c
>> +++ b/tools/testing/selftests/bpf/progs/verifier_ldsx.c
> [ ... ]
>> @@ -263,6 +257,14 @@ __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);
>> +	__uint(max_entries, 1);
>> +} arena SEC(".maps");
>> +
> [Severity: Medium]
> Does this inner guard unintentionally drop the arena tests for clang under
> the default test_progs flavour?
>
> The commit message notes that clang only defines feature macros like this at
> -mcpu=v4, but successfully assembles the inline asm at -mcpu=v3. Because of
> this, you kept the clang version checks on the file's main guard to avoid
> removing tests where they run today.
>
> However, since this inner guard lacks a || __clang_major__ >= 18 fallback,
> clang at -mcpu=v3 evaluates this as false,

Is this just by review or was it actually tested - I'm guessing former ...

> silently dropping the arena map
> and the subsequent 5 arena tests from the default test flavour.

... since clang (at least recent ones, I testet recent and a local jan 
build) defines this for all -mcpus

So this is invalid claim.

Thx,
-Vineet