Skip to content

Commit b226c23

Browse files
committed
x64: Fix panic compiling 16-bit multiply-with-immediate
This commit fixes a minor regression from bytecodealliance#10782 found via fuzzing. The regression is 16-bit immediates were forced to fit from an `i32` value into a `u16` for 16-bit multiplication. This meant though that negative numbers failed this conversion which meant that ISLE would panic due to the value not being matched. This fixes the logic to first fit the i32 into an i16 and then cast that to a u16 where the first phase should hit all the constants that are possible in Cranelift.
1 parent 2473041 commit b226c23

File tree

5 files changed

+50
-7
lines changed

5 files changed

+50
-7
lines changed

cranelift/codegen/src/isa/x64/inst.isle

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2884,9 +2884,9 @@
28842884
(rule 2 (x64_imul_imm $I16 src1 (i8_try_from_i32 src2)) (x64_imulw_rmi_sxb src1 src2))
28852885
(rule 2 (x64_imul_imm $I32 src1 (i8_try_from_i32 src2)) (x64_imull_rmi_sxb src1 src2))
28862886
(rule 2 (x64_imul_imm $I64 src1 (i8_try_from_i32 src2)) (x64_imulq_rmi_sxb src1 src2))
2887-
(rule 1 (x64_imul_imm $I16 src1 (u16_try_from_i32 src2)) (x64_imulw_rmi src1 src2))
2888-
(rule 1 (x64_imul_imm $I32 src1 (i32_as_u32 src2)) (x64_imull_rmi src1 src2))
2889-
(rule 1 (x64_imul_imm $I64 src1 src2) (x64_imulq_rmi_sxl src1 src2))
2887+
(rule 1 (x64_imul_imm $I16 src1 (i16_try_from_i32 src2)) (x64_imulw_rmi src1 (i16_as_u16 src2)))
2888+
(rule 1 (x64_imul_imm $I32 src1 src2) (x64_imull_rmi src1 (i32_as_u32 src2)))
2889+
(rule 1 (x64_imul_imm $I64 src1 src2) (x64_imulq_rmi_sxl src1 src2))
28902890

28912891
;; Helper for creating `mul` instructions or `imul` instructions (depending
28922892
;; on `signed`) for 8-bit operands.

cranelift/codegen/src/isle_prelude.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ macro_rules! isle_common_prelude_methods {
5959
}
6060

6161
#[inline]
62-
fn i32_as_u32(&mut self, x: i32) -> Option<u32> {
63-
Some(x as u32)
62+
fn i32_as_u32(&mut self, x: i32) -> u32 {
63+
x as u32
6464
}
6565

6666
#[inline]
@@ -958,6 +958,14 @@ macro_rules! isle_common_prelude_methods {
958958
i8::try_from(val).ok()
959959
}
960960

961+
fn i16_as_u16(&mut self, val: i16) -> u16 {
962+
val as u16
963+
}
964+
965+
fn i16_try_from_i32(&mut self, val: i32) -> Option<i16> {
966+
i16::try_from(val).ok()
967+
}
968+
961969
fn i16_try_from_u64(&mut self, val: u64) -> Option<i16> {
962970
i16::try_from(val).ok()
963971
}

cranelift/codegen/src/prelude.isle

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,18 +142,24 @@
142142
(decl pure partial i8_try_from_u64 (u64) i8)
143143
(extern constructor i8_try_from_u64 i8_try_from_u64)
144144

145+
(decl pure i16_as_u16 (i16) u16)
146+
(extern constructor i16_as_u16 i16_as_u16)
147+
145148
(decl pure partial i16_try_from_u64 (u64) i16)
146149
(extern constructor i16_try_from_u64 i16_try_from_u64)
147150

151+
(decl pure partial i16_try_from_i32 (i16) i32)
152+
(extern extractor i16_try_from_i32 i16_try_from_i32)
153+
148154
(decl pure partial i32_try_from_u64 (u64) i32)
149155
(extern constructor i32_try_from_u64 i32_try_from_u64)
150156

151157
(decl pure u32_as_u64 (u32) u64)
152158
(extern constructor u32_as_u64 u32_as_u64)
153159
(convert u32 u64 u32_as_u64)
154160

155-
(decl i32_as_u32 (u32) i32)
156-
(extern extractor i32_as_u32 i32_as_u32)
161+
(decl i32_as_u32 (i32) u32)
162+
(extern constructor i32_as_u32 i32_as_u32)
157163

158164
(decl pure i32_as_i64 (i32) i64)
159165
(extern constructor i32_as_i64 i32_as_i64)

tests/disas/x64-mul16-negative.wat

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
;;! target = 'x86_64'
2+
;;! test = 'compile'
3+
4+
(module
5+
(func (export "mul16") (param i32) (result i32)
6+
local.get 0
7+
i32.const -7937
8+
i32.mul
9+
i32.extend16_s
10+
)
11+
)
12+
;; wasm[0]::function[0]:
13+
;; pushq %rbp
14+
;; movq %rsp, %rbp
15+
;; imulw $0xe0ff, %dx, %dx
16+
;; movswl %dx, %eax
17+
;; movq %rbp, %rsp
18+
;; popq %rbp
19+
;; retq
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
(module
2+
(func (export "mul16") (param i32) (result i32)
3+
local.get 0
4+
i32.const -7937
5+
i32.mul
6+
i32.extend16_s
7+
)
8+
)
9+
10+
(assert_return (invoke "mul16" (i32.const 100)) (i32.const -7268))

0 commit comments

Comments
 (0)