Skip to content

Commit

Permalink
x64: Fix encoding of adc/sbb with memory operands (#9146)
Browse files Browse the repository at this point in the history
This commit is similar to #8976 where it's fixing some typos in the
encoding of the `adc` and `sbb` instructions used in Cranelift. These
appear to have copy/paste typos where the non-register-based opcodes
weren't updated from the `add` and `sub` opcodes. This problem was
exposed from a fuzz test case after #9136 landed. The fuzz test case is
minimized and included here as a new runtest and new emit tests are
additionally added.
  • Loading branch information
alexcrichton authored Aug 19, 2024
1 parent d76df36 commit a2f0f2f
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 2 deletions.
4 changes: 2 additions & 2 deletions cranelift/codegen/src/isa/x64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ pub(crate) fn emit(
let mut rex = RexFlags::from(*size);
let (opcode_r, opcode_m, subopcode_i) = match op {
AluRmiROpcode::Add => (0x01, 0x03, 0),
AluRmiROpcode::Adc => (0x11, 0x03, 2),
AluRmiROpcode::Adc => (0x11, 0x13, 2),
AluRmiROpcode::Sub => (0x29, 0x2B, 5),
AluRmiROpcode::Sbb => (0x19, 0x2B, 3),
AluRmiROpcode::Sbb => (0x19, 0x1B, 3),
AluRmiROpcode::And => (0x21, 0x23, 4),
AluRmiROpcode::Or => (0x09, 0x0B, 1),
AluRmiROpcode::Xor => (0x31, 0x33, 6),
Expand Down
20 changes: 20 additions & 0 deletions cranelift/codegen/src/isa/x64/inst/emit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1607,6 +1607,26 @@ fn test_x64_emit() {
"4983D700",
"adcq %r15, $0, %r15",
));
insns.push((
Inst::alu_rmi_r(
OperandSize::Size64,
AluRmiROpcode::Adc,
RegMemImm::mem(Amode::imm_reg(99, rdi)),
w_r15,
),
"4C137F63",
"adcq %r15, 99(%rdi), %r15",
));
insns.push((
Inst::alu_rmi_r(
OperandSize::Size64,
AluRmiROpcode::Sbb,
RegMemImm::mem(Amode::imm_reg(99, rdi)),
w_r15,
),
"4C1B7F63",
"sbbq %r15, 99(%rdi), %r15",
));

// ========================================================
// AluRM
Expand Down
74 changes: 74 additions & 0 deletions cranelift/filetests/filetests/isa/x64/i128.clif
Original file line number Diff line number Diff line change
Expand Up @@ -2069,3 +2069,77 @@ block0(v0: i64, v1: i64):
; popq %rbp
; retq

function %sink_two_i128_loads_in_add(i64, i64) -> i128 {
block0(v0: i64, v1: i64):
v10 = load.i64 v0
v11 = load.i64 v0+8
v18 = uextend.i128 v1
v20 = iconcat v10, v11
v14 = iadd v18, v20
return v14
}

; VCode:
; pushq %rbp
; movq %rsp, %rbp
; block0:
; movq 0(%rdi), %r9
; xorq %rdx, %rdx, %rdx
; movq %rsi, %rax
; addq %rax, %r9, %rax
; adcq %rdx, 8(%rdi), %rdx
; movq %rbp, %rsp
; popq %rbp
; ret
;
; Disassembled:
; block0: ; offset 0x0
; pushq %rbp
; movq %rsp, %rbp
; block1: ; offset 0x4
; movq (%rdi), %r9 ; trap: heap_oob
; xorq %rdx, %rdx
; movq %rsi, %rax
; addq %r9, %rax
; adcq 8(%rdi), %rdx ; trap: heap_oob
; movq %rbp, %rsp
; popq %rbp
; retq

function %sink_two_i128_loads_in_sub(i64, i64) -> i128 {
block0(v0: i64, v1: i64):
v10 = load.i64 v0
v11 = load.i64 v0+8
v18 = uextend.i128 v1
v20 = iconcat v10, v11
v14 = isub v18, v20
return v14
}

; VCode:
; pushq %rbp
; movq %rsp, %rbp
; block0:
; movq 0(%rdi), %r9
; xorq %rdx, %rdx, %rdx
; movq %rsi, %rax
; subq %rax, %r9, %rax
; sbbq %rdx, 8(%rdi), %rdx
; movq %rbp, %rsp
; popq %rbp
; ret
;
; Disassembled:
; block0: ; offset 0x0
; pushq %rbp
; movq %rsp, %rbp
; block1: ; offset 0x4
; movq (%rdi), %r9 ; trap: heap_oob
; xorq %rdx, %rdx
; movq %rsi, %rax
; subq %r9, %rax
; sbbq 8(%rdi), %rdx ; trap: heap_oob
; movq %rbp, %rsp
; popq %rbp
; retq

11 changes: 11 additions & 0 deletions cranelift/filetests/filetests/runtests/i128-arithmetic.clif
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,14 @@ block0(v0: i64, v1: i64):
; run: %mul_sextend_i64(0x10000000_00000000, 0x10000000_00000000) == 0x1000000000000000000000000000000
; run: %mul_sextend_i64(-2, 200) == -400
; run: %mul_sextend_i64(300, -4) == -1200

function %fuzz_test_case_4625018733002752() -> i128 {
block0():
v21 = iconst.i64 0x00f5_f500_a7ff_b389
v22 = iconst.i64 -8557360460515480319
v23 = iconcat v22, v21 ; v22 = -8557360460515480319, v21 = 0x00f5_f500_a7ff_b389
v32 = iadd v23, v23
return v32
}

; run: %fuzz_test_case_4625018733002752() == 2554163945374095986207901943416805890

0 comments on commit a2f0f2f

Please sign in to comment.