On 1/6/23 7:11 PM, Dave Thaler wrote:
Daniel Borkmann wrote:
[...]
+Also note that the division and modulo operations are unsigned,
+where 'imm' is first sign extended to 64 bits and then converted to
+an unsigned 64-bit value. There are no instructions for signed
+division or modulo.
Less sure about this part, but it looks to be true at least by looking
at the interpreter which does:
DST = DST / IMM
where:
DST === (u64) regs[insn->dst_reg]
IMM === (s32) insn->imm
(and s32 is sign-expanded to u64 according to C rules)
Yeap, the actual operation is in the target width, so for 32 bit it's casted to
u32, e.g. for modulo (note that the verifier rewrites it into `(src != 0) ?
(dst % src) : dst` form, so here is just the extract of the plain mod insn and it's
similar for div):
ALU64_MOD_X:
div64_u64_rem(DST, SRC, &AX);
DST = AX;
CONT;
ALU_MOD_X:
AX = (u32) DST;
DST = do_div(AX, (u32) SRC);
CONT;
ALU64_MOD_K:
div64_u64_rem(DST, IMM, &AX);
DST = AX;
CONT;
ALU_MOD_K:
AX = (u32) DST;
DST = do_div(AX, (u32) IMM);
CONT;
So in above phrasing the middle part needs to be adapted or just removed.
The phrasing was based on the earlier discussion on this list (see
https://lore.kernel.org/bpf/CAADnVQJ387tWd7WgxqfoB44xMe17bY0RRp_Sng3xMnKsywFpxg@xxxxxxxxxxxxxx/) where
Alexei wrote "imm32 is _sign_ extended everywhere",
and I cited the div_k tests in lib/test_bpf.c that assume sign extension
not zero extension.
`Where 'imm' is first sign extended to 64 bits and then converted to an unsigned
64-bit value` is true for the 64 bit operation, for the 32-bit it's just converted
to an unsigned 32-bit value, see the (u32)IMM which is (u32)(s32) insn->imm. From
the phrasing, it was a bit less clear perhaps.
Thanks,
Daniel
