Skip to content

Commit

Permalink
[typedarray] Remove per-comparator call detach check in TypedArray.pr…
Browse files Browse the repository at this point in the history
…ototype.sort

For the normative change, see tc39/ecma262#2723

Bug: v8:12750, v8:11111
Change-Id: I8e8a2e9b443622b20bb5a4c2d453f782dfbd2ed6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3570865
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79789}
  • Loading branch information
syg authored and kangwoosukeq committed Apr 28, 2022
1 parent 5046d7d commit 547be8e
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 32 deletions.
41 changes: 13 additions & 28 deletions src/builtins/typed-array-sort.tq
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,10 @@ transitioning macro CallCompare(
// a. Let v be ? ToNumber(? Call(comparefn, undefined, x, y)).
const v: Number = ToNumber_Inline(Call(context, comparefn, Undefined, a, b));

// b. If IsDetachedBuffer(buffer) is true, throw a TypeError exception.
// c. Let getBufferByteLength be
// MakeIdempotentArrayBufferByteLengthGetter(SeqCst).
// d. If IsIntegerIndexedObjectOutOfBounds(obj, getBufferByteLength) is true,
// throw a TypeError exception.
// TODO(v8:11111): Update this, depending on how
// https://github.com/tc39/ecma262/pull/2646#issuecomment-1067456576 gets
// resolved.
try {
LoadJSTypedArrayLengthAndCheckDetached(array)
otherwise DetachedOrOutOfBounds;
} label DetachedOrOutOfBounds {
ThrowTypeError(MessageTemplate::kDetachedOperation, kBuiltinNameSort);
}

// e. If v is NaN, return +0.
// b. If v is NaN, return +0.
if (NumberIsNaN(v)) return 0;

// f. return v.
// c. return v.
return v;
}

Expand Down Expand Up @@ -149,17 +134,17 @@ transitioning javascript builtin TypedArrayPrototypeSort(

TypedArrayMergeSort(work2, 0, len, work1, array, comparefn);

// Reload the length; it's possible the backing ArrayBuffer has been resized.
// It cannot be OOB here though, since we've checked it as part of the
// comparison function.

// TODO(v8:11111): Update this, depending on how
// https://github.com/tc39/ecma262/pull/2646#issuecomment-1067456576 gets
// resolved.
const newLen =
LoadJSTypedArrayLengthAndCheckDetached(array) otherwise unreachable;
if (newLen < len) {
len = newLen;
// Reload the length; it's possible the backing ArrayBuffer has been resized
// to be OOB or detached, in which case treat it as length 0.

try {
const newLen = LoadJSTypedArrayLengthAndCheckDetached(array)
otherwise DetachedOrOutOfBounds;
if (newLen < len) {
len = newLen;
}
} label DetachedOrOutOfBounds {
len = 0;
}

// work1 contains the sorted numbers. Write them back.
Expand Down
12 changes: 10 additions & 2 deletions test/mjsunit/typedarray-resizablearraybuffer-detach.js
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,12 @@ d8.file.execute('test/mjsunit/typedarray-helpers.js');
return 0;
}

function AssertIsDetached(ta) {
assertEquals(0, ta.byteLength);
assertEquals(0, ta.byteOffset);
assertEquals(0, ta.length);
}

// Fixed length TA.
for (let ctor of ctors) {
rab = CreateResizableArrayBuffer(4 * ctor.BYTES_PER_ELEMENT,
Expand All @@ -1466,7 +1472,8 @@ d8.file.execute('test/mjsunit/typedarray-helpers.js');
const taFull = new ctor(rab, 0);
WriteUnsortedData(taFull);

assertThrows(() => { fixedLength.sort(CustomComparison); });
fixedLength.sort(CustomComparison);
AssertIsDetached(fixedLength);
}

// Length-tracking TA.
Expand All @@ -1477,7 +1484,8 @@ d8.file.execute('test/mjsunit/typedarray-helpers.js');
const taFull = new ctor(rab, 0);
WriteUnsortedData(taFull);

assertThrows(() => { lengthTracking.sort(CustomComparison); });
lengthTracking.sort(CustomComparison);
AssertIsDetached(lengthTracking);
}
})();

Expand Down
2 changes: 1 addition & 1 deletion test/mjsunit/typedarray-resizablearraybuffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6597,7 +6597,7 @@ function TestIterationAndResize(ta, expected, rab, resize_after,
const taFull = new ctor(rab, 0);
WriteUnsortedData(taFull);

assertThrows(() => { fixedLength.sort(CustomComparison); });
fixedLength.sort(CustomComparison);

// The data is unchanged.
assertEquals([10, 9], ToNumbers(taFull));
Expand Down
1 change: 0 additions & 1 deletion test/test262/test262.status
Original file line number Diff line number Diff line change
Expand Up @@ -2907,7 +2907,6 @@

# https://bugs.chromium.org/p/v8/issues/detail?id=12750
'built-ins/TypedArray/prototype/set/array-arg-targetbuffer-detached-on-get-src-value-no-throw': [FAIL],
'built-ins/TypedArray/prototype/sort/sort-tonumber': [FAIL],

######################## NEEDS INVESTIGATION ###########################

Expand Down

0 comments on commit 547be8e

Please sign in to comment.