Skip to content

Commit 877adec

Browse files
sygsyg
committed
[JSC] Account for user code to arguments in Array.from
https://bugs.webkit.org/show_bug.cgi?id=306000 rdar://168635038 Reviewed by Sosuke Suzuki and Yusuke Suzuki. Arguments objects in JS are array-like, so there is no guarantee that .length implies a backing store of that size, and that indexed getters won't be called, even when the iterator protocol isn't user modified. This PR makes the optimized version of Array.from for arguments robust to user modification to fall back to generic get(). Test: JSTests/stress/array-from-arguments-overwritten-length.js Canonical link: https://commits.webkit.org/306050@main
1 parent 9e6af35 commit 877adec

2 files changed

Lines changed: 68 additions & 13 deletions

File tree

‎JSTests/stress/array-from-arguments-overwritten-length.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
function testArgumentsWriteLength(...xs) {
2+
arguments.length = 65535;
3+
let a = Array.from(arguments);
4+
if (a.length !== 65535)
5+
throw new Error("wrong array length");
6+
}
7+
8+
function testArgumentsCursedIndexedGetter(...xs) {
9+
let getterCalled = false;
10+
// This test should come after all others because it modifies Object.prototype
11+
Object.defineProperty(Object.prototype, 3, { get() {
12+
getterCalled = true;
13+
return -42;
14+
}, enumerable: true });
15+
arguments.length = 65535;
16+
let a = Array.from(arguments);
17+
if (a.length !== 65535)
18+
throw new Error("wrong array length");
19+
if (!getterCalled || a[3] !== -42)
20+
throw new Error("wrong array value");
21+
}
22+
23+
function testArgumentsCursedIndexedGetterThrows(...xs) {
24+
Object.defineProperty(Object.prototype, 4, { get() {
25+
throw -43;
26+
}, enumerable: true });
27+
arguments.length = 65535;
28+
let caughtException;
29+
try {
30+
Array.from(arguments);
31+
} catch (e) {
32+
caughtException = e;
33+
}
34+
if (caughtException !== -43)
35+
throw new Error("didn't throw");
36+
}
37+
38+
39+
testArgumentsWriteLength({});
40+
testArgumentsCursedIndexedGetter({});
41+
testArgumentsCursedIndexedGetterThrows({});

‎Source/JavaScriptCore/runtime/ArrayConstructor.cpp‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,24 @@ static ALWAYS_INLINE unsigned getArgumentsLength(ClonedArguments* arguments)
267267
return lengthValue.asInt32();
268268
}
269269

270+
template<typename Arguments, typename Functor>
271+
static ALWAYS_INLINE void forEachArgumentsElement(JSGlobalObject* globalObject, Arguments* arguments, unsigned length, const Functor& func)
272+
{
273+
auto scope = DECLARE_THROW_SCOPE(globalObject->vm());
274+
unsigned i;
275+
for (i = 0; i < length; ++i) {
276+
JSValue value = arguments->tryGetIndexQuickly(i);
277+
if (!value)
278+
break;
279+
func(value, i);
280+
}
281+
for (; i < length; ++i) {
282+
JSValue value = arguments->get(globalObject, i);
283+
RETURN_IF_EXCEPTION(scope, void());
284+
func(value, i);
285+
}
286+
}
287+
270288
template<typename Arguments>
271289
static ALWAYS_INLINE JSArray* tryCreateArrayFromArguments(JSGlobalObject* globalObject, Arguments* arguments)
272290
{
@@ -279,12 +297,10 @@ static ALWAYS_INLINE JSArray* tryCreateArrayFromArguments(JSGlobalObject* global
279297
RELEASE_AND_RETURN(scope, constructEmptyArray(globalObject, nullptr));
280298

281299
IndexingType indexingType = IsArray;
282-
for (unsigned i = 0; i < length; ++i) {
283-
JSValue value = arguments->getIndexQuickly(i);
284-
if (!value)
285-
value = jsUndefined();
300+
forEachArgumentsElement(globalObject, arguments, length, [&](JSValue value, unsigned) {
286301
indexingType = leastUpperBoundOfIndexingTypeAndValue(indexingType, value);
287-
}
302+
});
303+
RETURN_IF_EXCEPTION(scope, nullptr);
288304

289305
Structure* resultStructure = globalObject->arrayStructureForIndexingTypeDuringAllocation(indexingType);
290306
IndexingType resultIndexingType = resultStructure->indexingType();
@@ -306,18 +322,16 @@ static ALWAYS_INLINE JSArray* tryCreateArrayFromArguments(JSGlobalObject* global
306322
resultButterfly->setPublicLength(length);
307323

308324
if (hasDouble(resultIndexingType)) {
309-
for (uint64_t i = 0; i < length; ++i) {
310-
JSValue value = arguments->getIndexQuickly(i);
325+
forEachArgumentsElement(globalObject, arguments, length, [&](JSValue value, unsigned i) {
311326
ASSERT(value.isNumber());
312327
resultButterfly->contiguousDouble().atUnsafe(i) = value.asNumber();
313-
}
328+
});
329+
RETURN_IF_EXCEPTION(scope, nullptr);
314330
} else if (hasInt32(resultIndexingType) || hasContiguous(resultIndexingType)) {
315-
for (size_t i = 0; i < length; ++i) {
316-
JSValue value = arguments->getIndexQuickly(i);
317-
if (!value)
318-
value = jsUndefined();
331+
forEachArgumentsElement(globalObject, arguments, length, [&](JSValue value, unsigned i) {
319332
resultButterfly->contiguous().atUnsafe(i).setWithoutWriteBarrier(value);
320-
}
333+
});
334+
RETURN_IF_EXCEPTION(scope, nullptr);
321335
} else
322336
RELEASE_ASSERT_NOT_REACHED();
323337

0 commit comments

Comments
 (0)