Closed
Bug 1264941
Opened 8 years ago
Closed 8 years ago
byteLength calculation for ArrayBuffer object in TypedArray(typedArray) differs between lazily allocated case and eagerly allocated case.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: arai, Assigned: arai)
Details
Attachments
(1 file)
7.77 KB,
patch
|
lth
:
review+
|
Details | Diff | Splinter Review |
Code: var abuf = new ArrayBuffer(16); var a1 = new Float64Array(abuf, 0, 0); var a2 = new Float64Array(a1); a2.buffer.byteLength Expected output 16 Actual output 0 a2.buffer is created lazily with a2.byteLength, but a2.byteLength can be different than expected a2.buffer.byteLength. https://tc39.github.io/ecma262/#sec-clonearraybuffer > 4. Let srcLength be the value of srcBuffer's [[ArrayBufferByteLength]] internal slot. > 5. Assert: srcByteOffset ≤ srcLength. > 6. Let cloneLength be srcLength - srcByteOffset. > 7. Let srcBlock be the value of srcBuffer's [[ArrayBufferData]] internal slot. > 8. Let targetBuffer be ? AllocateArrayBuffer(cloneConstructor, cloneLength). srcLength is 16, and srcByteOffset is 0, so cloneLength should be 16, even if that area is not used by owning TypedArray instance.
Comment 1•8 years ago
|
||
The computation of cloneLength is actually considered a bug (even Allen thinks so), if I read this correctly: https://github.com/tc39/ecma262/issues/447
Assignee | ||
Comment 2•8 years ago
|
||
Ah, in that case what we should track is the behavior difference between lazily allocated case and eagerly allocated case. // lazily allocated case var abuf1 = new ArrayBuffer(16); var a1 = new Float64Array(abuf1, 0, 0); var a2 = new Float64Array(a1); print(a2.buffer.byteLength); // 0 // eagerly allocated case var abuf2 = new ArrayBuffer(128); var a3 = new Float64Array(abuf2, 0, 0); var a4 = new Float64Array(a3); print(a4.buffer.byteLength); // 128 either one should be fixed to follow the fixed spec.
Assignee | ||
Comment 3•8 years ago
|
||
So, eagerly allocated case follows current spec (cloneLength = srcLength - srcByteOffset), and lazily allocated case doesn't, and maybe it's the expected behavior for the *fixed* spec in https://github.com/tc39/ecma262/issues/447 .
Summary: ArrayBuffer object is created with less capacity than expected in TypedArray(typedArray) → byteLength calculation for ArrayBuffer object in TypedArray(typedArray) differs between lazily allocated case and eagerly allocated case.
Comment 4•8 years ago
|
||
I don't suppose the patch in bug 1255128 fixes this, does it? I need to land it, but we have some browser tests that expect the nonstandard constructor behavior.
Assignee | ||
Comment 5•8 years ago
|
||
The code change in bug 1255128 have no effect on this behavior. https://github.com/tc39/ecma262/issues/447 is closed with following commit https://github.com/tc39/ecma262/commit/e1cecfa320879912701fddf7d317f4636bd10457 so we should fix the eagerly allocated case to use the byteLength of typedArray.
Assignee | ||
Comment 6•8 years ago
|
||
CloneArrayBuffer is used also from %TypedArray%.prototype.set, but it uses totally different codepath, and doesn't call our CloneArrayBufferNoCopy function. So I'd like to leave that part to another bug. Green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0d773b00cb8
Assignee: nobody → arai.unmht
Attachment #8742877 -
Flags: review?(lhansen)
Comment 7•8 years ago
|
||
Comment on attachment 8742877 [details] [diff] [review] Use byteLength of source typedArray in CloneArrayBuffer. Review of attachment 8742877 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/tests/ecma_6/ArrayBuffer/CloneArrayBuffer.js @@ +2,5 @@ > +var summary = 'CloneArrayBuffer should be called with byteLength of source typedArray'; > + > +print(BUGNUMBER + ": " + summary); > + > +function test(ctor, byteLength, offsets) { "offsets" is unused.
Attachment #8742877 -
Flags: review?(lhansen) → review+
Assignee | ||
Comment 8•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e92704fae381eb6b3363a87372663c5a6466e6fd Bug 1264941 - Use byteLength of source typedArray in CloneArrayBuffer. r=lth
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e92704fae381
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•