Re: Masks and overflow of signed immediates in BPF instructions

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

On 8/15/23 7:19 AM, Jose E. Marchesi wrote:
> 
> Hello.
> 
> The selftest progs/verifier_masking.c contains inline assembly code
> like:
> 
>    	w1 = 0xffffffff;
> 
> The 32-bit immediate of that instruction is signed.  Therefore, GAS
> complains that the above instruction overflows its field:
> 
>    /tmp/ccNOXFQy.s:46: Error: signed immediate out of range, shall fit in 32 bits
> 
> The llvm assembler is likely relying on signed overflow for the above to
> work.

Not really.

   def _ri_32 : ALU_RI<BPF_ALU, Opc, off,
                    (outs GPR32:$dst),
                    (ins GPR32:$src2, i32imm:$imm),
                    "$dst "#OpcodeStr#" $imm",
                    [(set GPR32:$dst, (OpNode GPR32:$src2, 
i32immSExt32:$imm))]>;


If generating from source, the pattern
    [(set GPR32:$dst, (OpNode GPR32:$src2, i32immSExt32:$imm))]
so value 0xffffffff is not SExt32 and it won't match and
eventually a LDimm_64 insn will be generated.

But for inline asm, we will have
   (outs GPR32:$dst)
   (ins GPR32:$src2, i32imm:$imm)

and i32imm is defined as
   def i32imm : Operand<i32>;
which is a unsigned 32bit value, so it is recognized properly
and the insn is encoded properly.

> 
> Using negative numbers to denote masks is ugly and obfuscating (for
> non-obvious cases like -1/0xffffffff) so I suggest we introduce a
> pseudo-op so we can do:
> 
>     w1 = %mask(0xffffffff)

I changed above
   w1 = 0xffffffff;
to
   w1 = %mask(0xffffffff)
and hit the following compilation failure.

progs/verifier_masking.c:54:9: error: invalid % escape in inline 
assembly string
    53 |         asm volatile ("                                 \
       |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    54 |         w1 = %mask(0xffffffff);                         \
       |                ^
1 error generated.

Do you have documentation what is '%mask' thing?

> 
> allowing the assembler to do the right thing (TM) converting and
> checking that the mask is valid and not relying on UB.
> 
> Thoughts?
>