Re: [PATCH bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests

[Yonghong Song <yonghong.song@linux.dev>]

On 4/4/24 1:09 PM, Andrii Nakryiko wrote:
> On Thu, Apr 4, 2024 at 11:48 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>> On 4/2/24 4:26 PM, Andrii Nakryiko wrote:
>>> On Tue, Apr 2, 2024 at 3:50 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>>>> Add selftests validating that BPF verifier handles precision marking
>>>> for SCALAR registers derived from r10 (fp) register correctly.
>>>>
>>>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>>>> ---
>>>>    .../bpf/progs/verifier_subprog_precision.c    | 86 +++++++++++++++++++
>>>>    1 file changed, 86 insertions(+)
>>>>
>>>> diff --git a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
>>>> index 6f5d19665cf6..e1a8f107f0a7 100644
>>>> --- a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
>>>> +++ b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
>>>> @@ -76,6 +76,92 @@ __naked int subprog_result_precise(void)
>>>>           );
>>>>    }
>>>>
>>>> +__naked __noinline __used
>>>> +static unsigned long fp_leaking_subprog()
>>>> +{
>>>> +       asm volatile (
>>>> +               "r0 = (s8)r10;"
>>> Our CI's clang doesn't like this instruction. I guess I'll have to
>>> encode it in binary form :(
>> This patch disappeared from CI so I am not able to check the result.
>>
>> But I tried with the following small example.
>>
>> $ cat t.c
>> __attribute__((naked)) unsigned long t(void)
>> {
>>           asm volatile("r0 = (s8)r10;"
>>                        "exit;"
>>                       );
>> }
>>
>> $ clang --target=bpf -O2 -mcpu=v2 -g -c t.c && llvm-objdump -d t.o
>>
> You are using local clang built from source code, right? I think our
> BPF CI still is on Clang 17 or something, so it doesn't yet understand
> "(s8)r10" syntax, unfortunately.

Yes, it makes sense. Indeed in that case, either using bytes or guarding
with >= llvm18 is needed.

>
>
>> t.o:    file format elf64-bpf
>>
>> Disassembly of section .text:
>>
>> 0000000000000000 <t>:
>>          0:       bf a0 08 00 00 00 00 00 r0 = (s8)r10
>>          1:       95 00 00 00 00 00 00 00 exit
>>
>>
>> -mcpu=v3/v4 has the same result.
>> Not sure what clang complains.
>>
>>>> +               "exit;"
>>>> +       );
>>>> +}
>>>> +
>>> [...]
>>>