-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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.
- Loading branch information
Showing
4 changed files
with
33 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
test compile | ||
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} |