Repository: bytecodealliance/wasmtime
Pull request #9146 (2024-08-19 22:14:29)

x64: Fix encoding of adc/sbb with memory operands
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.

<!--
Please make sure you include the following information:

- If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

- Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->



Patch:
diff --git a/cranelift/codegen/src/isa/x64/inst/emit.rs b/cranelift/codegen/src/isa/x64/inst/emit.rs
index 890268dd8..0cc127724 100644
--- a/cranelift/codegen/src/isa/x64/inst/emit.rs
+++ b/cranelift/codegen/src/isa/x64/inst/emit.rs
@@ -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),


Test patch:
diff --git a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs
index 954522354..cf2150fc3 100644
--- a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs
+++ b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs
@@ -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
diff --git a/cranelift/filetests/filetests/isa/x64/i128.clif b/cranelift/filetests/filetests/isa/x64/i128.clif
index ec3c9b7ed..62df9b43a 100644
--- a/cranelift/filetests/filetests/isa/x64/i128.clif
+++ b/cranelift/filetests/filetests/isa/x64/i128.clif
@@ -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
+
diff --git a/cranelift/filetests/filetests/runtests/i128-arithmetic.clif b/cranelift/filetests/filetests/runtests/i128-arithmetic.clif
index 0a1fc80a3..e0f0a246f 100644
--- a/cranelift/filetests/filetests/runtests/i128-arithmetic.clif
+++ b/cranelift/filetests/filetests/runtests/i128-arithmetic.clif
@@ -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