Skip to content

Commit 6abe3c4

Browse files
authored
x64: Fix panic compiling 16-bit multiply-with-immediate (#10817)
* x64: Fix panic compiling 16-bit multiply-with-immediate This commit fixes a minor regression from #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. * Add some more tests
1 parent 7bf9be5 commit 6abe3c4

File tree

7 files changed

+156
-7
lines changed

7 files changed

+156
-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)
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
test compile precise-output
2+
set unwind_info=false
3+
set opt_level=speed
4+
target x86_64
5+
6+
function %imul_i16_const_unsigned_but_big(i32) -> i16 {
7+
block0(v0: i32):
8+
v3 = imul_imm v0, 0x81111
9+
v4 = ireduce.i16 v3
10+
return v4
11+
}
12+
13+
; VCode:
14+
; pushq %rbp
15+
; movq %rsp, %rbp
16+
; block0:
17+
; imulw $0x1111, %di, %ax
18+
; movq %rbp, %rsp
19+
; popq %rbp
20+
; ret
21+
;
22+
; Disassembled:
23+
; block0: ; offset 0x0
24+
; pushq %rbp
25+
; movq %rsp, %rbp
26+
; block1: ; offset 0x4
27+
; imulw $0x1111, %di, %ax
28+
; movq %rbp, %rsp
29+
; popq %rbp
30+
; retq
31+

cranelift/filetests/filetests/isa/x64/mul.clif

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,81 @@ block0(v0: i16):
284284
; popq %rbp
285285
; retq
286286

287+
function %imul_i16_const_negative(i16) -> i16 {
288+
block0(v0: i16):
289+
v3 = imul_imm v0, -97
290+
return v3
291+
}
292+
293+
; VCode:
294+
; pushq %rbp
295+
; movq %rsp, %rbp
296+
; block0:
297+
; imulw $0xff9f, %di, %ax
298+
; movq %rbp, %rsp
299+
; popq %rbp
300+
; ret
301+
;
302+
; Disassembled:
303+
; block0: ; offset 0x0
304+
; pushq %rbp
305+
; movq %rsp, %rbp
306+
; block1: ; offset 0x4
307+
; imulw $-0x61, %di, %ax
308+
; movq %rbp, %rsp
309+
; popq %rbp
310+
; retq
311+
312+
function %imul_i16_const_unsigned_but_big(i16) -> i16 {
313+
block0(v0: i16):
314+
v3 = imul_imm v0, 0x8000
315+
return v3
316+
}
317+
318+
; VCode:
319+
; pushq %rbp
320+
; movq %rsp, %rbp
321+
; block0:
322+
; imulw $0x8000, %di, %ax
323+
; movq %rbp, %rsp
324+
; popq %rbp
325+
; ret
326+
;
327+
; Disassembled:
328+
; block0: ; offset 0x0
329+
; pushq %rbp
330+
; movq %rsp, %rbp
331+
; block1: ; offset 0x4
332+
; imulw $0x8000, %di, %ax
333+
; movq %rbp, %rsp
334+
; popq %rbp
335+
; retq
336+
337+
function %imul_i16_const_out_of_bounds(i16) -> i16 {
338+
block0(v0: i16):
339+
v3 = imul_imm v0, 0x80000
340+
return v3
341+
}
342+
343+
; VCode:
344+
; pushq %rbp
345+
; movq %rsp, %rbp
346+
; block0:
347+
; imulw $0x0, %di, %ax
348+
; movq %rbp, %rsp
349+
; popq %rbp
350+
; ret
351+
;
352+
; Disassembled:
353+
; block0: ; offset 0x0
354+
; pushq %rbp
355+
; movq %rsp, %rbp
356+
; block1: ; offset 0x4
357+
; imulw $0, %di, %ax
358+
; movq %rbp, %rsp
359+
; popq %rbp
360+
; retq
361+
287362
function %imul_i32_const(i32) -> i32{
288363
block0(v0: i32):
289364
v3 = imul_imm v0, 97

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)