Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

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

On 6/12/25 12:18 PM, Alexei Starovoitov wrote:
> On Thu, Jun 12, 2025 at 12:10 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>> On Thu, 2025-06-12 at 10:19 -0700, Yonghong Song wrote:
>>> In one of upstream thread ([1]), there is a discussion about
>>> the below inline asm code:
>>>
>>>    if r1 == 0xdeadbeef goto +2;
>>>    ...
>>>
>>> In actual llvm backend, the above 0xdeadbeef will actually do
>>> sign extension to 64bit value and then compare to register r1.
>>>
>>> But the code itself does not imply the above semantics. It looks
>>> like the comparision is between r1 and 0xdeadbeef. For example,
>>> let us at a simple C code:
>>>    $ cat t1.c
>>>    int foo(long a) { return a == 0xdeadbeef ? 2 : 3; }
>>>    $ clang --target=bpf -O2 -c t1.c && llvm-objdump -d t1.o
>>>      ...
>>>      w0 = 0x2
>>>      r2 = 0xdeadbeef ll
>>>      if r1 == r2 goto +0x1
>>>      w0 = 0x3
>>>      exit
>>> It does try to compare r1 and 0xdeadbeef.
>>>
>>> To address the above confusing inline asm issue, llvm backend ([2])
>>> added some range checking for such insns and beyond. For the above
>>> insn asm, the warning like below
>>>    warning: immediate out of range, shall fit in int range
>>> will be issued. If -Werror is in the compilation flags, the
>>> error will be issued.
>>>
>>> To avoid the above warning/error, the afore-mentioned inline asm
>>> should be rewritten to
>>>
>>>    if r1 == -559038737 goto +2;
>>>    ...
>>>
>>> Fix a few selftest cases like the above based on insn range checking
>>> requirement in [2].
>>>
>>>    [1] https://lore.kernel.org/bpf/70affb12-327b-4882-bd1d-afda8b8c6f56@linux.dev/
>>>    [2] https://github.com/llvm/llvm-project/pull/142989
>>>
>>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>>> ---
>> Changes like 0xffffffff -> -1 and 0xfffffffe -> -2 look fine,
>> but changes like 0xffff1234 -> -60876 are an unnecessary obfuscation,
>> maybe we need to reconsider.
> I have to agree.
> I didn't expect there to be so many warnings.
> I thought it would be good to warn for
> r3 = 0xdeadbeef
>
> since r3 will have 0xffffFFFFdeadbeef value after assignment,
> but warn on
> r0 &= 0xFFFF1234
> and replacement with -60876 is taking the warning too far.

Agree this -60876 is bad.

>
> Also considering Jose's point.
>
> Warning in llvm/gcc on imm32 > UINT_MAX is not correct either.
> llvm should probably accept 0xffffFFFFdeadbeef as imm32.

In llvm, the value is represented as an int64, we probably
can just check the upper 32bit must be 0 or 0xffffFFFF.
Otherwise, the value is out of range.

> But that is a separate discussion.