wasmtime
wasmtime copied to clipboard
cranelift: ty_smin/smax/umax/mask panic on i128
OSS-Fuzz reported this as https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=69209. I've minimized the test case and identified several related panics, all with the message "unimplemented for > 64 bits".
This is not a security issue according to Wasmtime's security policy because it's a panic at compile time. It also shouldn't be reachable from Wasmtime today, only other Cranelift frontends.
Minimized CLIF
test compile
set opt_level=speed
target x86_64
function %slt() system_v {
block0:
v0 = iconst.i64 0
v1 = uextend.i128 v0
v2 = icmp slt v1, v1
return
}
function %ult() system_v {
block0:
v0 = iconst.i64 0
v1 = uextend.i128 v0
v2 = icmp ult v1, v1
return
}
The original fuzzbug, and my %slt
function above, hit this panic in ty_smin
: https://github.com/bytecodealliance/wasmtime/blob/f40aaa51a1df835498985acfc91ade33240d02b5/cranelift/codegen/src/isle_prelude.rs#L287-L289
I believe the optimization rule which invoked ty_smin
is here: https://github.com/bytecodealliance/wasmtime/blob/f40aaa51a1df835498985acfc91ade33240d02b5/cranelift/codegen/src/opts/icmp.isle#L105-L108
There are similar rules for ult
exercised by my %ult
function above. They call ty_umax
, which calls ty_mask
, which has essentially the same panic.
There are also similar rules for slt
which would hit an equivalent panic in ty_smax
, but I can't exercise them because ISLE is choosing to call ty_smin
first which then panics.
I believe these panics are only being exposed now because #8686 allowed these rules to match i128 constants when they couldn't before. The rules should still fail to match because of the fits_in_64
constraint on the other type in the pattern. However, ISLE does not guarantee anything about what order constructors and extractors will be evaluated in except for data dependencies. So that fits_in_64
guard isn't sufficient to ensure that ty_smin
won't be called.
One fix that I've verified works for the %slt
test case:
- add the
partial
keyword to(decl pure ty_smin ...)
and similarly forty_smax
, - make the corresponding Rust functions return
Option
, - use
?
instead of.expect()
when.checked_sub()
fails.
However I ran out of energy when I looked at doing the same for ty_umax
, which requires doing it for ty_mask
, which is called from Rust in a lot of places.
cc: @scottmcm