Handle srem properly when avoid_div_traps is false.
The codegen for div/rem ops has two modes, depending on the `avoid_div_traps` flag: it can either do all checks for trapping conditions explicitly, and use explicit trap instructions, then use a hardware divide instruction that will not trap (`avoid_div_traps == true`); or it can run in a mode where a hardware FP fault on the divide instruction implies a Wasm trap (`avoid_div_traps == false`). Wasmtime uses the former while Lucet (for example) uses the latter. It turns out that because we run all our spec tests run under Wasmtime, we missed a spec corner case that fails in the latter: INT_MIN % -1 == 0 per the spec, but causes a trap with the x86 signed divide/remainder instruction. Hence, in Lucet, this specific remainder computation would incorrectly result in a Wasm trap. This PR fixes the issue by just forcing use of the explicit-checks implementation for `srem` even when `avoid_div_traps` is false.
This commit is contained in:
@@ -887,7 +887,6 @@ pub(crate) fn emit(
|
||||
// idiv %divisor
|
||||
//
|
||||
// $done:
|
||||
debug_assert!(info.flags().avoid_div_traps());
|
||||
|
||||
// Check if the divisor is zero, first.
|
||||
let inst = Inst::cmp_rmi_r(*size, RegMemImm::imm(0), divisor.to_reg());
|
||||
|
||||
@@ -5181,7 +5181,8 @@ fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
|
||||
input_ty,
|
||||
));
|
||||
|
||||
if flags.avoid_div_traps() {
|
||||
// Always do explicit checks for `srem`: otherwise, INT_MIN % -1 is not handled properly.
|
||||
if flags.avoid_div_traps() || op == Opcode::Srem {
|
||||
// A vcode meta-instruction is used to lower the inline checks, since they embed
|
||||
// pc-relative offsets that must not change, thus requiring regalloc to not
|
||||
// interfere by introducing spills and reloads.
|
||||
|
||||
12
cranelift/filetests/filetests/isa/x64/div-checks-run.clif
Normal file
12
cranelift/filetests/filetests/isa/x64/div-checks-run.clif
Normal file
@@ -0,0 +1,12 @@
|
||||
test run
|
||||
set avoid_div_traps=false
|
||||
target x86_64
|
||||
feature "experimental_x64"
|
||||
|
||||
function %f0(i32, i32) -> i32 {
|
||||
block0(v0: i32, v1: i32):
|
||||
v2 = srem.i32 v0, v1
|
||||
return v2
|
||||
}
|
||||
|
||||
; run: %f0(0x80000000, 0xffffffff) == 0
|
||||
19
cranelift/filetests/filetests/isa/x64/div-checks.clif
Normal file
19
cranelift/filetests/filetests/isa/x64/div-checks.clif
Normal file
@@ -0,0 +1,19 @@
|
||||
test compile
|
||||
set avoid_div_traps=false
|
||||
target x86_64
|
||||
feature "experimental_x64"
|
||||
|
||||
;; We should get the checked-div/rem sequence (`srem` pseudoinst below) even
|
||||
;; when `avoid_div_traps` above is false (i.e. even when the host is normally
|
||||
;; willing to accept SIGFPEs as Wasm traps). The machine will SIGFPE in some
|
||||
;; cases when `srem` is valid (specifically -INT_MIN % -1).
|
||||
function %f0(i64, i64) -> i64 {
|
||||
block0(v0: i64, v1: i64):
|
||||
v2 = srem.i64 v0, v1
|
||||
; check: movq %rdi, %rax
|
||||
; nextln: movl $$0, %edx
|
||||
; nextln: srem $$rax:$$rdx, %rsi
|
||||
; nextln: movq %rdx, %rax
|
||||
|
||||
return v2
|
||||
}
|
||||
Reference in New Issue
Block a user