On Tue, 21 May 2019 at 16:02, Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote: > > On 05/21/2019 03:46 PM, Björn Töpel wrote: > > When using 32-bit subregisters (ALU32), the RISC-V JIT would not clear > > the high 32-bits of the target register and therefore generate > > incorrect code. > > > > E.g., in the following code: > > > > $ cat test.c > > unsigned int f(unsigned long long a, > > unsigned int b) > > { > > return (unsigned int)a & b; > > } > > > > $ clang-9 -target bpf -O2 -emit-llvm -S test.c -o - | \ > > llc-9 -mattr=+alu32 -mcpu=v3 > > .text > > .file "test.c" > > .globl f > > .p2align 3 > > .type f,@function > > f: > > r0 = r1 > > w0 &= w2 > > exit > > .Lfunc_end0: > > .size f, .Lfunc_end0-f > > > > The JIT would not clear the high 32-bits of r0 after the > > and-operation, which in this case might give an incorrect return > > value. > > > > After this patch, that is not the case, and the upper 32-bits are > > cleared. > > > > Reported-by: Jiong Wang <jiong.wang@xxxxxxxxxxxxx> > > Fixes: 2353ecc6f91f ("bpf, riscv: add BPF JIT for RV64G") > > Signed-off-by: Björn Töpel <bjorn.topel@xxxxxxxxx> > > Was this missed because test_verifier did not have test coverage? Yup, and Jiong noted it. > If so, could you follow-up with alu32 test cases for it, so other > JITs can be tracked for these kind of issue as well. We should > probably have one for every alu32 alu op to make sure it's not > forgotten anywhere. > I'll hack a test_verifier test right away. Thanks, Björn > Thanks, > Daniel