diff --git a/tests/sdiv-by-zero-imm.data b/tests/sdiv-by-zero-imm.data new file mode 100644 index 00000000..fa50c224 --- /dev/null +++ b/tests/sdiv-by-zero-imm.data @@ -0,0 +1,6 @@ +-- asm +mov32 %r0, 1 +sdiv32 %r0, 0 +exit +-- result +0x0 diff --git a/tests/sdiv-by-zero-reg.data b/tests/sdiv-by-zero-reg.data new file mode 100644 index 00000000..731be296 --- /dev/null +++ b/tests/sdiv-by-zero-reg.data @@ -0,0 +1,7 @@ +-- asm +mov32 %r0, 1 +mov32 %r1, 0 +sdiv32 %r0, %r1 +exit +-- result +0x0 diff --git a/tests/sdiv32-by-zero-imm.data b/tests/sdiv32-by-zero-imm.data new file mode 100644 index 00000000..fa50c224 --- /dev/null +++ b/tests/sdiv32-by-zero-imm.data @@ -0,0 +1,6 @@ +-- asm +mov32 %r0, 1 +sdiv32 %r0, 0 +exit +-- result +0x0 diff --git a/tests/sdiv32-by-zero-reg.data b/tests/sdiv32-by-zero-reg.data new file mode 100644 index 00000000..731be296 --- /dev/null +++ b/tests/sdiv32-by-zero-reg.data @@ -0,0 +1,7 @@ +-- asm +mov32 %r0, 1 +mov32 %r1, 0 +sdiv32 %r0, %r1 +exit +-- result +0x0 diff --git a/tests/sdiv32-high-divisor.data b/tests/sdiv32-high-divisor.data new file mode 100644 index 00000000..d14072a0 --- /dev/null +++ b/tests/sdiv32-high-divisor.data @@ -0,0 +1,7 @@ +-- asm +mov %r0, 12 +lddw %r1, 0xfffffffff +sdiv32 %r0, %r1 +exit +-- result +0xfffffff4 diff --git a/tests/sdiv32-imm.data b/tests/sdiv32-imm.data new file mode 100644 index 00000000..16c2e66b --- /dev/null +++ b/tests/sdiv32-imm.data @@ -0,0 +1,6 @@ +-- asm +lddw %r0, 0x1000000c +sdiv32 %r0, 0xffffffff +exit +-- result +0xeffffff4 diff --git a/tests/sdiv32-reg.data b/tests/sdiv32-reg.data new file mode 100644 index 00000000..862a070c --- /dev/null +++ b/tests/sdiv32-reg.data @@ -0,0 +1,7 @@ +-- asm +lddw %r0, 0x1000000c +mov32 %r1, 0xffffffff +sdiv32 %r0, %r1 +exit +-- result +0xeffffff4 diff --git a/tests/sdiv64-by-zero-imm.data b/tests/sdiv64-by-zero-imm.data new file mode 100644 index 00000000..54bdc61d --- /dev/null +++ b/tests/sdiv64-by-zero-imm.data @@ -0,0 +1,6 @@ +-- asm +mov32 %r0, 1 +sdiv %r0, 0 +exit +-- result +0x0 diff --git a/tests/sdiv64-by-zero-reg.data b/tests/sdiv64-by-zero-reg.data new file mode 100644 index 00000000..7743bea1 --- /dev/null +++ b/tests/sdiv64-by-zero-reg.data @@ -0,0 +1,7 @@ +-- asm +mov32 %r0, 1 +mov32 %r1, 0 +sdiv %r0, %r1 +exit +-- result +0x0 diff --git a/tests/sdiv64-imm.data b/tests/sdiv64-imm.data new file mode 100644 index 00000000..9f7e186d --- /dev/null +++ b/tests/sdiv64-imm.data @@ -0,0 +1,7 @@ +-- asm +mov %r0, 0xc +lsh %r0, 32 +sdiv %r0, 0xffffffffc +exit +-- result +0xfffffffd00000000 diff --git a/tests/sdiv64-negative-imm.data b/tests/sdiv64-negative-imm.data new file mode 100644 index 00000000..fe85f24c --- /dev/null +++ b/tests/sdiv64-negative-imm.data @@ -0,0 +1,6 @@ +-- asm +lddw %r0, 0xFFFFFFFFFFFFFF0F +sdiv %r0, -5 +exit +-- result +0x30 diff --git a/tests/sdiv64-negative-reg.data b/tests/sdiv64-negative-reg.data new file mode 100644 index 00000000..4968fd7b --- /dev/null +++ b/tests/sdiv64-negative-reg.data @@ -0,0 +1,7 @@ +-- asm +lddw %r0, 0xFFFFFFFFFFFFFF0F +mov %r1, 0xFFFFFFFFFFFFFFFF +sdiv %r0, %r1 +exit +-- result +0xf1 diff --git a/tests/sdiv64-reg.data b/tests/sdiv64-reg.data new file mode 100644 index 00000000..907041c7 --- /dev/null +++ b/tests/sdiv64-reg.data @@ -0,0 +1,8 @@ +-- asm +mov %r0, 0xc +lsh %r0, 32 +mov %r1, 0xfffffffc +sdiv %r0, %r1 +exit +-- result +0xfffffffd00000000 diff --git a/vm/ubpf_jit_arm64.c b/vm/ubpf_jit_arm64.c index 6aa66537..63621da4 100644 --- a/vm/ubpf_jit_arm64.c +++ b/vm/ubpf_jit_arm64.c @@ -155,7 +155,7 @@ map_register(int r) static void emit_movewide_immediate(struct jit_state* state, bool sixty_four, enum Registers rd, uint64_t imm); static void -divmod(struct jit_state* state, uint8_t opcode, int rd, int rn, int rm); +divmod(struct jit_state* state, uint16_t opcode, bool signedness, int rd, int rn, int rm); static uint32_t inline align_to(uint32_t amount, uint64_t boundary) { return (amount + (boundary - 1 )) & ~(boundary - 1); } @@ -424,7 +424,8 @@ emit_dataprocessing_onesource( enum DP2Opcode { - // S opcode| + // Remember: These literals are written in big-endian (i.e., bit 31 + // is on the left)! DP2_UDIV = 0x1ac00800U, // 0001_1010_1100_0000_0000_1000_0000_0000 DP2_SDIV = 0x1ac00c00U, // 0001_1010_1100_0000_0000_1100_0000_0000 DP2_LSLV = 0x1ac02000U, // 0001_1010_1100_0000_0010_0000_0000_0000 @@ -920,7 +921,16 @@ translate(struct ubpf_vm* vm, struct jit_state* state, char** errmsg) int sixty_four = is_alu64_op(&inst); + /* + * If this instruction calls for an immediate, the first + * thing that we will do is put that immediate in to a register. + * From then on, we will consider the operation as if it + * were register-based. Sneaky. + */ if (is_imm_op(&inst) && !is_simple_imm(&inst)) { + // Sign extension happens here + // (as it should, per the ISA). + // \/ emit_movewide_immediate(state, sixty_four, temp_register, (int64_t)inst.imm); src = temp_register; opcode = to_reg_op(opcode); @@ -956,7 +966,7 @@ translate(struct ubpf_vm* vm, struct jit_state* state, char** errmsg) case EBPF_OP_MOD_REG: case EBPF_OP_DIV64_REG: case EBPF_OP_MOD64_REG: - divmod(state, opcode, dst, dst, src); + divmod(state, opcode, inst.offset != 0, dst, dst, src); break; case EBPF_OP_OR_REG: case EBPF_OP_AND_REG: @@ -1126,6 +1136,7 @@ translate(struct ubpf_vm* vm, struct jit_state* state, char** errmsg) case EBPF_OP_LSH64_IMM: case EBPF_OP_RSH64_IMM: case EBPF_OP_ARSH64_IMM: + // The immediate operations should have been converted ... see above. *errmsg = ubpf_error("Unexpected instruction at PC %d: opcode %02x, immediate %08x", i, opcode, inst.imm); return -1; default: @@ -1140,16 +1151,17 @@ translate(struct ubpf_vm* vm, struct jit_state* state, char** errmsg) } static void -divmod(struct jit_state* state, uint8_t opcode, int rd, int rn, int rm) +divmod(struct jit_state* state, uint16_t opcode, bool signedness, int rd, int rn, int rm) { bool mod = (opcode & EBPF_ALU_OP_MASK) == (EBPF_OP_MOD_IMM & EBPF_ALU_OP_MASK); bool sixty_four = (opcode & EBPF_CLS_MASK) == EBPF_CLS_ALU64; enum Registers div_dest = mod ? temp_div_register : rd; + enum DP2Opcode s_or_udiv_operation = signedness ? DP2_SDIV : DP2_UDIV; - /* Do not need to treet divide by zero as special because the UDIV instruction already + /* Do not need to treat divide by zero as special because the UDIV instruction already * returns 0 when dividing by zero. */ - emit_dataprocessing_twosource(state, sixty_four, DP2_UDIV, div_dest, rn, rm); + emit_dataprocessing_twosource(state, sixty_four, s_or_udiv_operation, div_dest, rn, rm); if (mod) { emit_dataprocessing_threesource(state, sixty_four, DP3_MSUB, rd, rm, div_dest, rn); } diff --git a/vm/ubpf_jit_x86_64.c b/vm/ubpf_jit_x86_64.c index a728e0d1..87128419 100644 --- a/vm/ubpf_jit_x86_64.c +++ b/vm/ubpf_jit_x86_64.c @@ -37,7 +37,7 @@ #endif static void -muldivmod(struct jit_state* state, uint8_t opcode, int src, int dst, int32_t imm); +muldivmod(struct jit_state* state, uint8_t opcode, uint8_t offset, int src, int dst, int32_t imm); #define REGISTER_MAP_SIZE 11 @@ -285,7 +285,7 @@ translate(struct ubpf_vm* vm, struct jit_state* state, char** errmsg) case EBPF_OP_DIV_REG: case EBPF_OP_MOD_IMM: case EBPF_OP_MOD_REG: - muldivmod(state, inst.opcode, src, dst, inst.imm); + muldivmod(state, inst.opcode, inst.offset, src, dst, inst.imm); break; case EBPF_OP_OR_IMM: emit_alu32_imm32(state, 0x81, 1, dst, inst.imm); @@ -372,7 +372,7 @@ translate(struct ubpf_vm* vm, struct jit_state* state, char** errmsg) case EBPF_OP_DIV64_REG: case EBPF_OP_MOD64_IMM: case EBPF_OP_MOD64_REG: - muldivmod(state, inst.opcode, src, dst, inst.imm); + muldivmod(state, inst.opcode, inst.offset, src, dst, inst.imm); break; case EBPF_OP_OR64_IMM: emit_alu64_imm32(state, 0x81, 1, dst, inst.imm); @@ -705,13 +705,14 @@ translate(struct ubpf_vm* vm, struct jit_state* state, char** errmsg) } static void -muldivmod(struct jit_state* state, uint8_t opcode, int src, int dst, int32_t imm) +muldivmod(struct jit_state* state, uint8_t opcode, uint8_t offset, int src, int dst, int32_t imm) { bool mul = (opcode & EBPF_ALU_OP_MASK) == (EBPF_OP_MUL_IMM & EBPF_ALU_OP_MASK); bool div = (opcode & EBPF_ALU_OP_MASK) == (EBPF_OP_DIV_IMM & EBPF_ALU_OP_MASK); bool mod = (opcode & EBPF_ALU_OP_MASK) == (EBPF_OP_MOD_IMM & EBPF_ALU_OP_MASK); bool is64 = (opcode & EBPF_CLS_MASK) == EBPF_CLS_ALU64; bool reg = (opcode & EBPF_SRC_REG) == EBPF_SRC_REG; + bool isSigned = offset != 0; // Short circuit for imm == 0. if (!reg && imm == 0) { @@ -777,12 +778,24 @@ muldivmod(struct jit_state* state, uint8_t opcode, int src, int dst, int32_t imm emit_alu32(state, 0x31, RDX, RDX); } + // If we are doing a signed operation (of either 64 or 32 bit width), + // we are responsible for appropriately setting _x_dx because _x_dx:_x_cx + // is the divisor. For an unsigned operation, we know that _x_dx is 0, but + // we need to do better when it is a signed operation. + // See, e.g., https://www.felixcloutier.com/x86/div. + if (isSigned) { + if (is64) { + emit_rex(state, 1, 0, 0, 0); + } + emit1(state, 0x99); + } + if (is64) { emit_rex(state, 1, 0, 0, 0); } - // Multiply or divide. - emit_alu32(state, 0xf7, mul ? 4 : 6, RCX); + // (signed) multiply or divide. + emit_alu32(state, 0xf7, isSigned + (mul ? 4 : 6), RCX); // Division operation stores the remainder in RDX and the quotient in RAX. if (div || mod) { diff --git a/vm/ubpf_vm.c b/vm/ubpf_vm.c index db166b10..ae50dccc 100644 --- a/vm/ubpf_vm.c +++ b/vm/ubpf_vm.c @@ -30,6 +30,7 @@ #include #include "ubpf_int.h" #include +#include #define MAX_EXT_FUNCS 64 #define SHIFT_MASK_32_BIT(X) ((X)&0x1f) @@ -230,6 +231,18 @@ i32(uint64_t x) return x; } +static uint64_t +u64(uint64_t x) +{ + return x; +} + +static int64_t +i64(uint64_t x) +{ + return x; +} + #define IS_ALIGNED(x, a) (((uintptr_t)(x) & ((a)-1)) == 0) inline static uint64_t @@ -364,11 +377,19 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret reg[inst.dst] &= UINT32_MAX; break; case EBPF_OP_DIV_IMM: - reg[inst.dst] = u32(inst.imm) ? u32(reg[inst.dst]) / u32(inst.imm) : 0; + if (inst.offset != 0) { + reg[inst.dst] = i32(inst.imm) ? i32(reg[inst.dst]) / i32(inst.imm) : 0; + } else { + reg[inst.dst] = u32(inst.imm) ? u32(reg[inst.dst]) / u32(inst.imm) : 0; + } reg[inst.dst] &= UINT32_MAX; break; case EBPF_OP_DIV_REG: - reg[inst.dst] = reg[inst.src] ? u32(reg[inst.dst]) / u32(reg[inst.src]) : 0; + if (inst.offset != 0) { + reg[inst.dst] = reg[inst.src] ? i32(reg[inst.dst]) / i32(reg[inst.src]) : 0; + } else { + reg[inst.dst] = reg[inst.src] ? u32(reg[inst.dst]) / u32(reg[inst.src]) : 0; + } reg[inst.dst] &= UINT32_MAX; break; case EBPF_OP_OR_IMM: @@ -475,10 +496,18 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret reg[inst.dst] *= reg[inst.src]; break; case EBPF_OP_DIV64_IMM: - reg[inst.dst] = inst.imm ? reg[inst.dst] / inst.imm : 0; + if (inst.offset != 0) { + reg[inst.dst] = inst.imm ? i64(reg[inst.dst]) / i64(inst.imm) : 0; + } else { + reg[inst.dst] = inst.imm ? u64(reg[inst.dst]) / u64(inst.imm) : 0; + } break; case EBPF_OP_DIV64_REG: - reg[inst.dst] = reg[inst.src] ? reg[inst.dst] / reg[inst.src] : 0; + if (inst.offset != 0) { + reg[inst.dst] = reg[inst.src] ? i64(reg[inst.dst]) / i64(reg[inst.src]) : 0; + } else { + reg[inst.dst] = reg[inst.src] ? u64(reg[inst.dst]) / u64(reg[inst.src]) : 0; + } break; case EBPF_OP_OR64_IMM: reg[inst.dst] |= inst.imm;