From 36559d91ca480a61c5ef6d64ea414ccbfee481ac Mon Sep 17 00:00:00 2001 From: Shu-yu Guo Date: Fri, 9 Sep 2022 14:52:34 -0700 Subject: [PATCH] [rab/gsab] Fix length-tracking handling in TA#subarray MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The normative change in https://github.com/tc39/proposal-resizablearraybuffer/pull/93 changed the behavior of TypedArray.prototype.subarray(begin, end) such that if the receiver is a length-tracking TA and end is undefined, the result TypedArray is also length-tracking. This change reached consensus in the March 2022 TC39. Bug: v8:11111 Change-Id: If1a84cc3134f3ce8046196d6cc36683b6996dec0 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3888382 Commit-Queue: Marja Hölttä Auto-Submit: Shu-yu Guo Reviewed-by: Marja Hölttä Cr-Commit-Position: refs/heads/main@{#83147} --- src/builtins/typed-array-createtypedarray.tq | 4 +- src/builtins/typed-array-subarray.tq | 64 ++++++++++++------- .../typedarray-growablesharedarraybuffer.js | 13 ++-- .../typedarray-resizablearraybuffer.js | 17 +++-- 4 files changed, 60 insertions(+), 38 deletions(-) diff --git a/src/builtins/typed-array-createtypedarray.tq b/src/builtins/typed-array-createtypedarray.tq index acee937f75..91dd4404a7 100644 --- a/src/builtins/typed-array-createtypedarray.tq +++ b/src/builtins/typed-array-createtypedarray.tq @@ -478,12 +478,12 @@ transitioning macro TypedArraySpeciesCreateByLength(implicit context: Context)( transitioning macro TypedArraySpeciesCreateByBuffer(implicit context: Context)( methodName: constexpr string, exemplar: JSTypedArray, buffer: JSArrayBuffer, - beginByteOffset: uintptr, newLength: uintptr): JSTypedArray { + beginByteOffset: uintptr, newLength: NumberOrUndefined): JSTypedArray { const numArgs: constexpr int31 = 3; // TODO(v8:4153): pass length further as uintptr. const typedArray: JSTypedArray = TypedArraySpeciesCreate( methodName, numArgs, exemplar, buffer, Convert(beginByteOffset), - Convert(newLength)); + newLength); return typedArray; } diff --git a/src/builtins/typed-array-subarray.tq b/src/builtins/typed-array-subarray.tq index 1d815e84ea..d9506321b8 100644 --- a/src/builtins/typed-array-subarray.tq +++ b/src/builtins/typed-array-subarray.tq @@ -10,16 +10,18 @@ transitioning javascript builtin TypedArrayPrototypeSubArray( const methodName: constexpr string = '%TypedArray%.prototype.subarray'; // 1. Let O be the this value. - // 3. If O does not have a [[TypedArrayName]] internal slot, throw a - // TypeError exception. + // 2. Perform ? RequireInternalSlot(O, [[TypedArrayName]]). const source = Cast(receiver) otherwise ThrowTypeError( MessageTemplate::kIncompatibleMethodReceiver, methodName); - // 5. Let buffer be O.[[ViewedArrayBuffer]]. + // 3. Assert: O has a [[ViewedArrayBuffer]] internal slot. + // 4. Let buffer be O.[[ViewedArrayBuffer]]. const buffer = typed_array::GetTypedArrayBuffer(source); - // 6. Let srcLength be O.[[ArrayLength]]. + // 5. Let getSrcBufferByteLength be + // MakeIdempotentArrayBufferByteLengthGetter(SeqCst). + // 6. Let srcLength be IntegerIndexedObjectLength(O, getSrcBufferByteLength). let srcLength: uintptr; try { srcLength = LoadJSTypedArrayLengthAndCheckDetached(source) @@ -29,41 +31,57 @@ transitioning javascript builtin TypedArrayPrototypeSubArray( srcLength = 0; } - // 8. Let relativeBegin be ? ToInteger(begin). - // 9. If relativeBegin < 0, let beginIndex be max((srcLength + - // relativeBegin), 0); else let beginIndex be min(relativeBegin, - // srcLength). + // 8. Let relativeBegin be ? ToIntegerOrInfinity(begin). + // 9. If relativeBegin is -∞, let beginIndex be 0. + // 10. Else if relativeBegin < 0, let beginIndex be max(srcLength + + // relativeBegin, 0). + // 11. Else, let beginIndex be min(relativeBegin, srcLength). const arg0 = arguments[0]; const begin: uintptr = arg0 != Undefined ? ConvertAndClampRelativeIndex(arg0, srcLength) : 0; - // 10. If end is undefined, let relativeEnd be srcLength; - // else, let relativeEnd be ? ToInteger(end). - // 11. If relativeEnd < 0, let endIndex be max((srcLength + relativeEnd), - // 0); else let endIndex be min(relativeEnd, srcLength). + // 12. If O.[[ArrayLength]] is auto and end is undefined, then const arg1 = arguments[1]; - const end: uintptr = arg1 != Undefined ? - ConvertAndClampRelativeIndex(arg1, srcLength) : - srcLength; + const endIsDefined = arg1 != Undefined; - // 12. Let newLength be max(endIndex - beginIndex, 0). - const newLength: uintptr = Unsigned(IntPtrMax(Signed(end - begin), 0)); + let newLength: NumberOrUndefined; + if (IsLengthTrackingJSArrayBufferView(source) && !endIsDefined) { + // a. Let newLength be undefined. + newLength = Undefined; + } else { + // 13. Else, + // a. If end is undefined, let relativeEnd be srcLength; else let + // relativeEnd be ? ToIntegerOrInfinity(end). + // b. If relativeEnd is -∞, let endIndex be 0. + // c. Else if relativeEnd < 0, let endIndex be max(srcLength + + // relativeEnd, 0). + // d. Else, let endIndex be min(relativeEnd, srcLength). + const end: uintptr = endIsDefined ? + ConvertAndClampRelativeIndex(arg1, srcLength) : + srcLength; - // 13. Let constructorName be the String value of O.[[TypedArrayName]]. - // 14. Let elementSize be the Number value of the Element Size value + // e. Let newLength be max(endIndex - beginIndex, 0). + newLength = Convert(Unsigned(IntPtrMax(Signed(end - begin), 0))); + } + + // 14. Let constructorName be the String value of O.[[TypedArrayName]]. + // 15. Let elementSize be the Number value of the Element Size value // specified in Table 52 for constructorName. const elementsInfo = typed_array::GetTypedArrayElementsInfo(source); - // 15. Let srcByteOffset be O.[[ByteOffset]]. + // 16. Let srcByteOffset be O.[[ByteOffset]]. const srcByteOffset: uintptr = source.byte_offset; - // 16. Let beginByteOffset be srcByteOffset + beginIndex × elementSize. + // 17. Let beginByteOffset be srcByteOffset + beginIndex × elementSize. const beginByteOffset = srcByteOffset + elementsInfo.CalculateByteLength(begin) otherwise ThrowRangeError(MessageTemplate::kInvalidArrayBufferLength); - // 17. Let argumentsList be « buffer, beginByteOffset, newLength ». - // 18. Return ? TypedArraySpeciesCreate(O, argumentsList). + // 18. If newLength is undefined, then + // a. Let argumentsList be « buffer, 𝔽(beginByteOffset) ». + // 19. Else, + // a. Let argumentsList be « buffer, 𝔽(beginByteOffset), 𝔽(newLength) ». + // 20. Return ? TypedArraySpeciesCreate(O, argumentsList). return TypedArraySpeciesCreateByBuffer( methodName, source, buffer, beginByteOffset, newLength); } diff --git a/test/mjsunit/typedarray-growablesharedarraybuffer.js b/test/mjsunit/typedarray-growablesharedarraybuffer.js index 1e5679521a..6627e9ee3b 100644 --- a/test/mjsunit/typedarray-growablesharedarraybuffer.js +++ b/test/mjsunit/typedarray-growablesharedarraybuffer.js @@ -3437,11 +3437,11 @@ Reverse(ArrayReverseHelper); assertEquals(4, fixedLengthSubFull.length); assertEquals(2, fixedLengthWithOffsetSubFull.length); - // TODO(v8:11111): Are subarrays of length-tracking TAs also - // length-tracking? See - // https://github.com/tc39/proposal-resizablearraybuffer/issues/91 - assertEquals(4, lengthTrackingSubFull.length); - assertEquals(2, lengthTrackingWithOffsetSubFull.length); + // Subarrays of length-tracking TAs that don't pass an explicit end argument + // are also length-tracking. + assertEquals(lengthTracking.length, lengthTrackingSubFull.length); + assertEquals(lengthTrackingWithOffset.length, + lengthTrackingWithOffsetSubFull.length); } })(); @@ -3479,7 +3479,8 @@ Reverse(ArrayReverseHelper); const evil = { valueOf: () => { gsab.grow(6 * ctor.BYTES_PER_ELEMENT); return 0;}}; - assertEquals([0, 2, 4, 6], ToNumbers(lengthTracking.subarray(evil))); + assertEquals([0, 2, 4, 6], ToNumbers( + lengthTracking.subarray(evil, lengthTracking.length))); } })(); diff --git a/test/mjsunit/typedarray-resizablearraybuffer.js b/test/mjsunit/typedarray-resizablearraybuffer.js index 38ef983666..d72260f561 100644 --- a/test/mjsunit/typedarray-resizablearraybuffer.js +++ b/test/mjsunit/typedarray-resizablearraybuffer.js @@ -6682,11 +6682,11 @@ Reverse(ArrayReverseHelper, false); assertEquals(4, fixedLengthSubFull.length); assertEquals(2, fixedLengthWithOffsetSubFull.length); - // TODO(v8:11111): Are subarrays of length-tracking TAs also - // length-tracking? See - // https://github.com/tc39/proposal-resizablearraybuffer/issues/91 - assertEquals(4, lengthTrackingSubFull.length); - assertEquals(2, lengthTrackingWithOffsetSubFull.length); + // Subarrays of length-tracking TAs that don't pass an explicit end argument + // are also length-tracking. + assertEquals(lengthTracking.length, lengthTrackingSubFull.length); + assertEquals(lengthTrackingWithOffset.length, + lengthTrackingWithOffsetSubFull.length); } })(); @@ -6779,7 +6779,9 @@ Reverse(ArrayReverseHelper, false); rab.resize(2 * ctor.BYTES_PER_ELEMENT); return 0; }}; - assertThrows(() => { lengthTracking.subarray(evil); }); + assertThrows(() => { + lengthTracking.subarray(evil, lengthTracking.length); + }); } // Like the previous test, but now we construct a smaller subarray and it @@ -6874,7 +6876,8 @@ Reverse(ArrayReverseHelper, false); const evil = { valueOf: () => { rab.resize(6 * ctor.BYTES_PER_ELEMENT); return 0;}}; - assertEquals([0, 2, 4, 6], ToNumbers(lengthTracking.subarray(evil))); + assertEquals([0, 2, 4, 6], ToNumbers( + lengthTracking.subarray(evil, lengthTracking.length))); } })();