Skip to content

Commit

Permalink
Bug 1763605 - Part 2: Don't throw for detached array buffers in Typed…
Browse files Browse the repository at this point in the history 
…Array.prototype.set. r=tcampbell

Per <tc39/ecma262#2646>.

Depends on D143568

Differential Revision: https://phabricator.services.mozilla.com/D143569
  • Loading branch information
@anba
anba committed Apr 19, 2022
1 parent ae1e3f4 commit d568af8
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 65 deletions.
3 changes: 0 additions & 3 deletions js/src/tests/jstests.list
View file
Original file line number Diff line number Diff line change
Expand Up @@ -609,9 +609,6 @@ skip script test262/built-ins/RegExp/named-groups/non-unicode-property-names-val
# https://bugzilla.mozilla.org/show_bug.cgi?id=1761989
skip script test262/built-ins/TypedArrayConstructors/ctors/no-species.js

# https://bugzilla.mozilla.org/show_bug.cgi?id=1763605
skip script test262/built-ins/TypedArray/prototype/set/array-arg-targetbuffer-detached-on-get-src-value-no-throw.js

# https://bugzilla.mozilla.org/show_bug.cgi?id=1763606
skip script test262/built-ins/TypedArray/prototype/sort/sort-tonumber.js

Expand Down
55 changes: 40 additions & 15 deletions js/src/tests/non262/TypedArray/set-detached.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ if (typeof detachArrayBuffer === "function") {
}
}

// Test a TypeError is thrown when the typed array is detached and
// Test no TypeError is thrown when the typed array is detached and
// srcLength > 0.
for (let {typedArray, buffer} of createTypedArrays()) {
let source = {
Expand All @@ -130,8 +130,11 @@ if (typeof detachArrayBuffer === "function") {
}
}
};
let err = typedArray.length === 0 ? RangeError : TypeError;
assertThrowsInstanceOf(() => typedArray.set(source), err);
if (typedArray.length === 0) {
assertThrowsInstanceOf(() => typedArray.set(source), RangeError);
} else {
typedArray.set(source);
}
}

// Same as above, but with side-effect when executing Get(src, "0").
Expand Down Expand Up @@ -182,13 +185,17 @@ if (typeof detachArrayBuffer === "function") {
}
}
});
let err = typedArray.length === 0 ? RangeError : TypeError;
assertThrowsInstanceOf(() => typedArray.set(source), err);
if (typedArray.length === 0) {
assertThrowsInstanceOf(() => typedArray.set(source), RangeError);
} else {
typedArray.set(source);
}
}

// Side-effects when getting the source elements detach the buffer. Also
// ensure other elements aren't accessed.
// ensure other elements are accessed.
for (let {typedArray, buffer} of createTypedArrays()) {
let accessed = false;
let source = Object.defineProperties([], {
0: {
get() {
Expand All @@ -198,12 +205,19 @@ if (typeof detachArrayBuffer === "function") {
},
1: {
get() {
throw new Error("Unexpected access");
assertEq(accessed, false);
accessed = true;
return 2;
}
}
});
let err = typedArray.length <= 1 ? RangeError : TypeError;
assertThrowsInstanceOf(() => typedArray.set(source), err);
if (typedArray.length <= 1) {
assertThrowsInstanceOf(() => typedArray.set(source), RangeError);
} else {
assertEq(accessed, false);
typedArray.set(source);
assertEq(accessed, true);
}
}

// Side-effects when converting the source elements detach the buffer.
Expand All @@ -214,25 +228,36 @@ if (typeof detachArrayBuffer === "function") {
return 1;
}
}];
let err = typedArray.length === 0 ? RangeError : TypeError;
assertThrowsInstanceOf(() => typedArray.set(source), err);
if (typedArray.length === 0) {
assertThrowsInstanceOf(() => typedArray.set(source), RangeError);
} else {
typedArray.set(source);
}
}

// Side-effects when converting the source elements detach the buffer. Also
// ensure other elements aren't accessed.
// ensure other elements are accessed.
for (let {typedArray, buffer} of createTypedArrays()) {
let accessed = false;
let source = [{
valueOf() {
detachArrayBuffer(buffer);
return 1;
}
}, {
valueOf() {
throw new Error("Unexpected access");
assertEq(accessed, false);
accessed = true;
return 2;
}
}];
let err = typedArray.length <= 1 ? RangeError : TypeError;
assertThrowsInstanceOf(() => typedArray.set(source), err);
if (typedArray.length <= 1) {
assertThrowsInstanceOf(() => typedArray.set(source), RangeError);
} else {
assertEq(accessed, false);
typedArray.set(source);
assertEq(accessed, true);
}
}
}

Expand Down
12 changes: 1 addition & 11 deletions js/src/tests/non262/extensions/TypedArray-set-object-funky-length-detaches.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,7 @@ ctors.forEach(function(TypedArray) {
}
};

var passed = false;
try
{
ta.set(arraylike, 0x1234);
}
catch (e)
{
passed = true;
}

assertEq(passed, true);
ta.set(arraylike, 0x1234);
});

/******************************************************************************/
Expand Down
2 changes: 1 addition & 1 deletion js/src/tests/non262/extensions/typedarray-set-neutering.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ Object.defineProperty(src, 4, {
}
});

assertThrowsInstanceOf(() => a.set(src), TypeError);
a.set(src);

/******************************************************************************/

Expand Down
16 changes: 11 additions & 5 deletions js/src/vm/TypedArrayObject-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,12 +412,15 @@ class ElementSpecific {
size_t offset = 0) {
MOZ_ASSERT(target->type() == TypeIDOfType<T>::id,
"target type and NativeType must match");
MOZ_ASSERT(!target->hasDetachedBuffer(), "target isn't detached");
MOZ_ASSERT(!source->is<TypedArrayObject>(),
"use setFromTypedArray instead of this method");
MOZ_ASSERT_IF(target->hasDetachedBuffer(), target->length() == 0);
MOZ_ASSERT_IF(!target->hasDetachedBuffer(), offset <= target->length());
MOZ_ASSERT_IF(!target->hasDetachedBuffer(),
len <= target->length() - offset);

size_t i = 0;
if (source->is<NativeObject>()) {
if (source->is<NativeObject>() && !target->hasDetachedBuffer()) {
// Attempt fast-path infallible conversion of dense elements up to
// the first potentially side-effectful lookup or conversion.
size_t bound = std::min<size_t>(
Expand Down Expand Up @@ -459,11 +462,14 @@ class ElementSpecific {
return false;
}

len = std::min<size_t>(len, target->length());
if (i >= len) {
break;
// Ignore out-of-bounds writes, but still execute getElement/valueToNative
// because of observable side-effects.
if (offset + i >= target->length()) {
continue;
}

MOZ_ASSERT(!target->hasDetachedBuffer());

// Compute every iteration in case getElement/valueToNative
// detaches the underlying array buffer or GC moves the data.
SharedMem<T*> dest =
Expand Down
30 changes: 0 additions & 30 deletions js/src/vm/TypedArrayObject.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1729,27 +1729,6 @@ static bool SetTypedArrayFromArrayLike(JSContext* cx,

// Steps 8-9.
if (srcLength > 0) {
// GetLengthProperty in step 16 can lead to the execution of user code
// which may detach the buffer. Handle this case here to ensure
// SetFromNonTypedArray is never called with a detached buffer. We still
// need to execute steps 21.a-b for their possible side-effects.
if (target->hasDetachedBuffer()) {
// Steps 21.a-b.
RootedValue v(cx);
if (!GetElement(cx, src, src, 0, &v)) {
return false;
}

if (!target->convertForSideEffect(cx, v)) {
return false;
}

// Step 21.c.
JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
JSMSG_TYPED_ARRAY_DETACHED);
return false;
}

switch (target->type()) {
#define SET_FROM_NON_TYPED_ARRAY(_, T, N) \
case Scalar::N: \
Expand All @@ -1761,15 +1740,6 @@ static bool SetTypedArrayFromArrayLike(JSContext* cx,
default:
MOZ_CRASH("Unsupported TypedArray type");
}

// Step 21.c.
// SetFromNonTypedArray doesn't throw when the array buffer gets
// detached.
if (target->hasDetachedBuffer()) {
JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
JSMSG_TYPED_ARRAY_DETACHED);
return false;
}
}

// Step 10.
Expand Down

0 comments on commit d568af8

Please sign in to comment.