| Summary: | [JSC][wasm] Truncating slightly less than INT32_MIN is incorrect | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alon Zakai <alonzakai> | ||||
| Component: | WebAssembly | Assignee: | Yusuke Suzuki <ysuzuki> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | cryze92, darin, ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
Created attachment 405338 [details]
Patch
Comment on attachment 405338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405338&action=review > Source/JavaScriptCore/llint/WebAssembly.asm:1105 > + move 0xcf000000, t0 # INT32_MIN (Note that INT32_MIN - 1.0 in float is the same to INT32_MIN in float). "same to" -> "same as" > Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:2815 > + auto min = addConstant(Type::F64, bitwise_cast<uint64_t>(static_cast<double>(std::numeric_limits<int32_t>::min()) - 1.0)); No need for the static_cast<double> now since "- 1.0" causes conversion to double, but I suppose we can keep it for clarity. > Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:2273 > + Value* min = constant(Double, bitwise_cast<uint64_t>(static_cast<double>(std::numeric_limits<int32_t>::min()) - 1.0)); Ditto. Comment on attachment 405338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405338&action=review Thanks! >> Source/JavaScriptCore/llint/WebAssembly.asm:1105 >> + move 0xcf000000, t0 # INT32_MIN (Note that INT32_MIN - 1.0 in float is the same to INT32_MIN in float). > > "same to" -> "same as" Fixed! >> Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:2815 >> + auto min = addConstant(Type::F64, bitwise_cast<uint64_t>(static_cast<double>(std::numeric_limits<int32_t>::min()) - 1.0)); > > No need for the static_cast<double> now since "- 1.0" causes conversion to double, but I suppose we can keep it for clarity. For clarity, keeping this looks good :) >> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:2273 >> + Value* min = constant(Double, bitwise_cast<uint64_t>(static_cast<double>(std::numeric_limits<int32_t>::min()) - 1.0)); > > Ditto. Ditto. Committed r264995: <https://trac.webkit.org/changeset/264995> |
(module (func (export "trunc") (drop (i32.trunc_f64_s (f64.const -2147483648.1) ) ) ) ) This should not trap - while the number is smaller than INT32_MIN, it rounds to a valid value. See https://github.com/WebAssembly/spec/issues/1224 for details and https://github.com/WebAssembly/spec/pull/1225 for the spec + spec suite update (that will now test this).