Re: [PATCH] selftests/bpf: improve test coverage for kfunc call

["Jose E. Marchesi" <jemarch@gnu.org>]

> On 3/11/26 11:03 AM, Hari Bathini wrote:
>>
>> On 11/03/26 9:32 pm, Alexei Starovoitov wrote:
>>> On Wed, Mar 11, 2026 at 8:10 AM Hari Bathini
>>> <hbathini@linux.ibm.com> wrote:
>>>>
>>>>>
>>>>>> +
>>>>>> +       /* Check zero-extension */
>>>>>> +       if (val != (unsigned long)a)
>>>>>> +               return 1;
>>>>>> +       /* Check no sign-extension */
>>>>>> +       if (val < 0)
>>>>>> +               return 2;
>>>>>> +
>>>>>> +       val = b;
>>>>>> +       if (val != (unsigned long)b)
>>>>>> +               return 3;
>>>>>> +       if (val < 0)
>>>>>> +               return 4;
>>>>>> +
>>>>>> +       val = c;
>>>>>> +       if (val != (unsigned long)c)
>>>>>> +               return 5;
>>>>>> +       if (val < 0)
>>>>>> +               return 6;
>>>>>> +
>>>>>> +       return 0;
>>>>>> +}
>>>>>
>>>>> Overall this looks very useful.
>>>>> I would expand with another test where a,b,c are s8,s16,s32.
>>>>
>>>> Slightly different approach but kfunc_call_test4/bpf_kfunc_call_test4
>>>> cover signed arguments already?
>>>
>>> Ahh. Then may be tweak it to adopt similar fine grained
>>> error reporting as your bpf_kfunc_call_test5()
>>
>> I Prefer this.
>>
>> Does the below change to bpf_kfunc_call_test4 look fine:
>>
>> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> index 48dcaf93bb9f..6237c2222633 100644
>> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> @@ -760,8 +760,30 @@ __bpf_kfunc struct sock
>> *bpf_kfunc_call_test3(struct sock *sk)
>>
>>  __bpf_kfunc long noinline bpf_kfunc_call_test4(signed char a, short
>> b, int c, long d)
>
> I might regret for bringing this up as it could be yet another ABI
> fiasco between gcc and llvm.

char is signed in BPF in both compilers, matching x86.

> As per C standard, sign of unadorned char (i.e. w/o explicit signed or
> unsigned prefix) is ABI defined.
> For gcc-bpf char is specified to be signed.
> So test4 has s8, while new test5 has u8. Would it make sense to have
> an additional test without signed/unsigned annotation for char ?
> This will flag any discrepancy between the two compilers.
>
> Thx,
> -Vineet
>
>>  {
>> +       /*
>> +        * Make val as volatile to avoid compiler optimizations.
>> +        * Verify that negative signed values remain negative after
>> +        * sign-extension (JIT must sign-extend, not zero-extend).
>> +        */
>> +       volatile long val;
>> +
>> +       /* val will be positive, if JIT does zero-extension instead
>> of sign-extension */
>> +       val = a;
>> +       if (val >= 0)
>> +               return 1;
>> +
>> +       val = b;
>> +       if (val >= 0)
>> +               return 2;
>> +
>> +       val = c;
>> +       if (val >= 0)
>> +               return 3;
>> +
>>         /* Provoke the compiler to assume that the caller has
>> sign-extended a,
>>          * b and c on platforms where this is required (e.g. s390x).
>> +        *
>> +        * Original behavior: return sum for backward compatibility
>>          */
>>         return (long)a + (long)b + (long)c + d;
>>  }
>>
>>
>> - Hari