Bug 214834 – [JSC][wasm] Truncating slightly less than INT32_MIN is incorrect

• Status: RESOLVED
• Resolution: FIXED
• Component: WebAssembly

[Alon Zakai]

(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).

[Radar WebKit Bug Importer]

<rdar://problem/66193187>

[Yusuke Suzuki]

Created attachment 405338 [details]
Patch

[Darin Adler]

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.

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Committed r264995: <https://trac.webkit.org/changeset/264995>