Closed Bug 1418195 Opened 2 years ago Closed 2 years ago

Baldr: make ToNonWrappingUint32 throw on NaN

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: luke, Assigned: luke)

Details

Attachments

(1 file)

This follows from the use of WebIDL's [EnforceRange] in https://github.com/WebAssembly/spec/pull/591 as opposed to the ad hoc ToNonWrappingUint32() in the informal JS.md (https://github.com/WebAssembly/design/blob/master/JS.md#tononwrappinguint32).
Priority: -- → P3
Attached patch enforce-rangeSplinter Review
Attachment #8930261 - Flags: review?(bbouvier)
Comment on attachment 8930261 [details] [diff] [review]
enforce-range

Review of attachment 8930261 [details] [diff] [review]:
-----------------------------------------------------------------

Nice tests, thanks

::: js/src/wasm/WasmJS.cpp
@@ +309,5 @@
> +    // Step 6.2.
> +    x = JS::ToInteger(x);
> +
> +    // Step 6.3, allowing caller to supply a more restrictive uint32_t max.
> +    if (x < 0 || x > max) {

nit: can you write it x < 0.0 || x > double(max) to make the implicit conversion explicit?
Attachment #8930261 - Flags: review?(bbouvier) → review+
Pushed by lwagner@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a91b80c925e
Baldr: use stricter checking for [EnforceRange] types (r=bbouvier)
https://hg.mozilla.org/mozilla-central/rev/5a91b80c925e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.