Closed
Bug 1317384
Opened 8 years ago
Closed 7 years ago
%TypedArray%.prototype.set should use ToInteger instead of ToInt32
Categories
(Core :: JavaScript: Standard Library, defect)
Core
JavaScript: Standard Library
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: anba, Assigned: anba)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
25.32 KB,
patch
|
lth
:
review+
|
Details | Diff | Splinter Review |
For example `new Int8Array(10).set([], -Infinity)` should throw a RangeError. ES2017 spec: https://tc39.github.io/ecma262/#sec-%typedarray%.prototype.set-overloaded-offset The following test262 tests are currently failing because of this issue: built-ins/TypedArray/prototype/set/array-arg-negative-integer-offset-throws.js built-ins/TypedArray/prototype/set/typedarray-arg-negative-integer-offset-throws.js built-ins/TypedArray/prototype/set/typedarray-arg-src-range-greather-than-target-throws-rangeerror.js
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → andrebargull
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•7 years ago
|
||
This updates %TypedArray%.prototype.set() to be compliant with the latest ECMAScript draft (with the exception of bug 1314148). There are three notable changes: 1) The offset parameter for %TypedArray%.prototype.set() is now converted with ToInteger instead of ToInt32. 2) For the non-TypedArray case (ES2017, 22.2.3.23.1), the argument gets converted to an object with ToObject. That means |new Int32Array(0).set("")| is now allowed. 3) Additional checks for detached ArrayBuffers are now present. This change shouldn't lead to web-compatibility problems, because JavaScriptCore and Chakra also already check for detached buffers. I've added three new test files to cover each change. And I've also added new assertions to the various ElementSpecific methods to ensure they're never called with detached ArrayBuffers, so it's easier to reason about their behaviour. (And one comment was declassified, because the sec-issue was fixed three years ago, so we longer need to care about not 0-daying ourselves.)
Attachment #8834372 -
Flags: review?(lhansen)
Assignee | ||
Comment 2•7 years ago
|
||
I forgot to mention that the patch applies on top of bug 1225031.
Comment 3•7 years ago
|
||
Comment on attachment 8834372 [details] [diff] [review] bug1317384.patch Review of attachment 8834372 [details] [diff] [review]: ----------------------------------------------------------------- I bow down in awe for some of those tests.
Attachment #8834372 -
Flags: review?(lhansen) → review+
Assignee | ||
Comment 4•7 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e8264f2931eeaa1026f3da698e6265127e3b225
Keywords: checkin-needed
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Lars T Hansen [:lth] from comment #3) > I bow down in awe for some of those tests. It may make sense to contribute them to test262, but I'd have to check the existing coverage for %TypedArray%.prototype.set() first.
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4a930acd18f8 Update TypedArray.prototype.set to be compliant with latest ECMA2017. r=lth
Keywords: checkin-needed
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4a930acd18f8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•