Skip to content

Commit f0d0278

Browse files
author
Suwei Chen
committed
TypedArray construction from array with user-defined %ArrayIteratorPrototype%.next
TypedArray constructor uses the fast path for array on an array even if %ArrayIteratorPrototype%.next is changed by user code. Correct behavior is to treat it as an iterable object and invoke TypedArray(object). Fix by adding extra conditions (essentially testing if %ArrayIteratorPrototype%.next is the built-in version) under which TypedArray(object) will be used. To avoid causing side effects while invoking GetProperty() for %ArrayIteratorPrototype%.next, implicit call is disabled before the call and restored after. Tweak unit test to cover this case and the case where %ArrayIteratorPrototype%.next is overridden by a getter.
1 parent 359bcf7 commit f0d0278

9 files changed

Lines changed: 113 additions & 35 deletions

File tree

‎lib/Runtime/Base/ThreadContext.h‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,25 @@ class ThreadContext sealed :
387387
void SetStackProber(StackProber * stackProber);
388388
PBYTE GetScriptStackLimit() const;
389389
static DWORD GetStackLimitForCurrentThreadOffset() { return offsetof(ThreadContext, stackLimitForCurrentThread); }
390+
391+
template <class Fn>
392+
Js::ImplicitCallFlags TryWithDisabledImplicitCall(Fn fn)
393+
{
394+
DisableImplicitFlags prevDisableImplicitFlags = this->GetDisableImplicitFlags();
395+
Js::ImplicitCallFlags savedImplicitCallFlags = this->GetImplicitCallFlags();
396+
397+
this->DisableImplicitCall();
398+
this->SetImplicitCallFlags(Js::ImplicitCallFlags::ImplicitCall_None);
399+
fn();
400+
401+
Js::ImplicitCallFlags curImplicitCallFlags = this->GetImplicitCallFlags();
402+
403+
this->SetDisableImplicitFlags(prevDisableImplicitFlags);
404+
this->SetImplicitCallFlags(savedImplicitCallFlags);
405+
406+
return curImplicitCallFlags;
407+
}
408+
390409
void * GetAddressOfStackLimitForCurrentThread()
391410
{
392411
FAULTINJECT_SCRIPT_TERMINATION

‎lib/Runtime/Library/JavascriptLibrary.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4434,7 +4434,7 @@ namespace Js
44344434
JavascriptLibrary* library = arrayIteratorPrototype->GetLibrary();
44354435
ScriptContext* scriptContext = library->GetScriptContext();
44364436

4437-
library->AddFunctionToLibraryObject(arrayIteratorPrototype, PropertyIds::next, &JavascriptArrayIterator::EntryInfo::Next, 0);
4437+
library->arrayIteratorPrototypeBuiltinNextFunction = library->AddFunctionToLibraryObject(arrayIteratorPrototype, PropertyIds::next, &JavascriptArrayIterator::EntryInfo::Next, 0);
44384438

44394439
if (scriptContext->GetConfig()->IsES6ToStringTagEnabled())
44404440
{

‎lib/Runtime/Library/JavascriptLibrary.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,7 @@ namespace Js
619619
JavascriptFunction* GetSymbolConstructor() const {return symbolConstructor; }
620620
JavascriptFunction* GetEvalFunctionObject() { return evalFunctionObject; }
621621
JavascriptFunction* GetArrayPrototypeValuesFunction() { return EnsureArrayPrototypeValuesFunction(); }
622+
JavascriptFunction* GetArrayIteratorPrototypeBuiltinNextFunction() { return arrayIteratorPrototypeBuiltinNextFunction; }
622623
DynamicObject* GetMathObject() const {return mathObject; }
623624
DynamicObject* GetJSONObject() const {return JSONObject; }
624625
DynamicObject* GetReflectObject() const { return reflectObject; }

‎lib/Runtime/Library/JavascriptLibraryBase.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ namespace Js
182182
JavascriptFunction* debugObjectDebugModeGetterFunction;
183183
JavascriptFunction* __proto__getterFunction;
184184
JavascriptFunction* __proto__setterFunction;
185+
JavascriptFunction* arrayIteratorPrototypeBuiltinNextFunction;
185186
DynamicObject* mathObject;
186187
// SIMD_JS
187188
DynamicObject* simdObject;

‎lib/Runtime/Library/TypedArray.cpp‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,15 @@ namespace Js
9494
return newArr;
9595
}
9696

97+
bool TypedArrayBase::ArrayIteratorPrototypeHasUserDefinedNext(ScriptContext *scriptContext)
98+
{
99+
Var arrayIteratorPrototypeNext = nullptr;
100+
ImplicitCallFlags flags = scriptContext->GetThreadContext()->TryWithDisabledImplicitCall(
101+
[&] () { arrayIteratorPrototypeNext = JavascriptOperators::GetProperty(scriptContext->GetLibrary()->GetArrayIteratorPrototype(), PropertyIds::next, scriptContext); });
102+
103+
return (flags != ImplicitCall_None) || arrayIteratorPrototypeNext != scriptContext->GetLibrary()->GetArrayIteratorPrototypeBuiltinNextFunction();
104+
}
105+
97106
Var TypedArrayBase::CreateNewInstance(Arguments& args, ScriptContext* scriptContext, uint32 elementSize, PFNCreateTypedArray pfnCreateTypedArray)
98107
{
99108
uint32 byteLength = 0;
@@ -147,7 +156,7 @@ namespace Js
147156

148157
if (!JavascriptOperators::IsUndefinedObject(iterator) &&
149158
(iterator != scriptContext->GetLibrary()->GetArrayPrototypeValuesFunction() ||
150-
!JavascriptArray::Is(firstArgument)))
159+
!JavascriptArray::Is(firstArgument) || ArrayIteratorPrototypeHasUserDefinedNext(scriptContext) ))
151160
{
152161
return CreateNewInstanceFromIterableObj(RecyclableObject::FromVar(firstArgument), scriptContext, elementSize, pfnCreateTypedArray);
153162
}

‎lib/Runtime/Library/TypedArray.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ namespace Js
170170
static Var CreateNewInstanceFromIterableObj(RecyclableObject *object, ScriptContext *scriptContext, uint32 elementSize, PFNCreateTypedArray pfnCreateTypedArray);
171171
static Var CreateNewInstance(Arguments& args, ScriptContext* scriptContext, uint32 elementSize, PFNCreateTypedArray pfnCreateTypedArray );
172172
static int32 ToLengthChecked(Var lengthVar, uint32 elementSize, ScriptContext* scriptContext);
173+
static bool ArrayIteratorPrototypeHasUserDefinedNext(ScriptContext *scriptContext);
173174

174175
virtual void* GetCompareElementsFunction() = 0;
175176

‎test/typedarray/TypedArrayBuiltins.baseline‎

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎test/typedarray/TypedArrayBuiltins.js‎

Lines changed: 79 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,21 @@ var tests = [
6464
};
6565
}
6666

67+
function getIterableObjNextDesc (array)
68+
{
69+
return {
70+
get: function () {
71+
array.shift(); // side effect
72+
return function () {
73+
return {
74+
value: array.shift(),
75+
done: array.length == 0
76+
};
77+
};
78+
}
79+
};
80+
}
81+
6782
var TypedArray = [
6883
'Int8Array',
6984
'Uint8Array',
@@ -84,36 +99,74 @@ var tests = [
8499
assert.areEqual(3, arr[2], "TypedArray " + t + " created from iterable has element #2 == 3");
85100
}
86101

87-
for(var t of TypedArray) {
88-
var a = [1,2,3,4];
89-
a[Symbol.iterator] = getIterableObj([99,0])[Symbol.iterator];
90-
var arr = new this[t](a);
91-
assert.areEqual(1, arr.length, "TypedArray " + t + " created from array with user-defined iterator has length == 1");
92-
assert.areEqual(99, arr[0], "TypedArray " + t + " created from array with user-defined iterator has element #0 == 99");
93-
}
102+
// change array's iterator
103+
(function() {
104+
for(var t of TypedArray) {
105+
var a = [1,2,3,4];
106+
a[Symbol.iterator] = getIterableObj([99,0])[Symbol.iterator];
107+
var arr = new this[t](a);
108+
assert.areEqual(1, arr.length, "TypedArray " + t + " created from array with user-defined iterator has length == 1");
109+
assert.areEqual(99, arr[0], "TypedArray " + t + " created from array with user-defined iterator has element #0 == 99");
110+
}
111+
})();
94112

95-
function testTypedArrayConstructorWithIterableArray(t) {
96-
Array.prototype[Symbol.iterator] = getIterableObj([99,0])[Symbol.iterator];
97-
var arr = new this[t](a);
98-
assert.areEqual(1, arr.length, "TypedArray " + t + " created from array with Array.prototype overridden has length == 1");
99-
assert.areEqual(99, arr[0], "TypedArray " + t + " created from array with Array.prototype overriden has element #0 == 99");
113+
// helpers for testing all typed arrays when built-in array iterator is changed
114+
function testTypedArrayConstructorWithIterableArray(inputarray, t, func, text) {
115+
func();
116+
var arr = new this[t](inputarray);
117+
assert.areEqual(1, arr.length, "TypedArray " + t + " created from array with " + text + " has length == 1");
118+
assert.areEqual(99, arr[0], "TypedArray " + t + " created from array with "+ text + " has element #0 == 99");
100119
}
120+
function testAllTypedArrayConstructorsWithIterableArray(inputarray, func, text) {
121+
testTypedArrayConstructorWithIterableArray(inputarray, 'Int8Array', func, text);
122+
testTypedArrayConstructorWithIterableArray(inputarray, 'Uint8Array', func, text);
123+
testTypedArrayConstructorWithIterableArray(inputarray, 'Uint8ClampedArray', func, text);
124+
testTypedArrayConstructorWithIterableArray(inputarray, 'Int16Array', func, text);
125+
testTypedArrayConstructorWithIterableArray(inputarray, 'Uint16Array', func, text);
126+
testTypedArrayConstructorWithIterableArray(inputarray, 'Int32Array', func, text);
127+
testTypedArrayConstructorWithIterableArray(inputarray, 'Uint32Array', func, text);
128+
testTypedArrayConstructorWithIterableArray(inputarray, 'Float32Array', func, text);
129+
testTypedArrayConstructorWithIterableArray(inputarray, 'Float64Array', func, text);
130+
}
131+
132+
// change built-in Array prototype's iterator
133+
(function() {
134+
var builtinArrayPrototypeIteratorDesc = Object.getOwnPropertyDescriptor(Array.prototype, Symbol.iterator);
135+
var a = [1,2,3,4];
136+
Object.defineProperty(Array.prototype, Symbol.iterator, {enumerable: false, configurable: true, writable: true});
137+
138+
var overrideBuiltinArrayPrototypeIterator = function() {
139+
Array.prototype[Symbol.iterator] = getIterableObj([99,0])[Symbol.iterator];
140+
};
141+
testAllTypedArrayConstructorsWithIterableArray(a, overrideBuiltinArrayPrototypeIterator, "Array.prototype overriden");
142+
Object.defineProperty(Array.prototype, Symbol.iterator, builtinArrayPrototypeIteratorDesc);
143+
})();
144+
145+
// change built-in array iterator's next function
146+
(function() {
147+
var arrayIteratorProto = Object.getPrototypeOf([][Symbol.iterator]());
148+
var builtinArrayPrototypeIteratorNext = arrayIteratorProto.next;
149+
var overrideBuiltinArrayIteratorNext = function() {
150+
arrayIteratorProto.next = getIterableObj([99,0])[Symbol.iterator]().next;
151+
}
152+
var a = [1,2,3,4];
153+
testAllTypedArrayConstructorsWithIterableArray(a, overrideBuiltinArrayIteratorNext, "%ArrayIteratorPrototype%.next overriden");
154+
arrayIteratorProto.next = builtinArrayPrototypeIteratorNext;
155+
})();
101156

102-
var builtinArrayPrototypeIteratorDesc = Object.getOwnPropertyDescriptor(Array.prototype, Symbol.iterator);
103-
var a = [1,2,3,4];
104-
Object.defineProperty(Array.prototype, Symbol.iterator, {enumerable: false, configurable: true, writable: true});
105-
testTypedArrayConstructorWithIterableArray('Int8Array');
106-
testTypedArrayConstructorWithIterableArray('Uint8Array');
107-
testTypedArrayConstructorWithIterableArray('Uint8ClampedArray');
108-
testTypedArrayConstructorWithIterableArray('Int16Array');
109-
testTypedArrayConstructorWithIterableArray('Uint16Array');
110-
testTypedArrayConstructorWithIterableArray('Int32Array');
111-
testTypedArrayConstructorWithIterableArray('Uint32Array');
112-
testTypedArrayConstructorWithIterableArray('Float32Array');
113-
testTypedArrayConstructorWithIterableArray('Float64Array');
114-
Object.defineProperty(Array.prototype, Symbol.iterator, builtinArrayPrototypeIteratorDesc);
157+
// change built-in array iterator's next getter function
158+
(function() {
159+
var arrayIteratorProto = Object.getPrototypeOf([][Symbol.iterator]());
160+
var builtinArrayPrototypeIteratorNextDesc = Object.getOwnPropertyDescriptor(arrayIteratorProto, "next");
161+
var overrideBuiltinArrayIteratorNext = function() {
162+
Object.defineProperty(arrayIteratorProto, "next", getIterableObjNextDesc([0,99,0]));
163+
}
164+
var a = [1,2,3,4];
165+
testAllTypedArrayConstructorsWithIterableArray(a, overrideBuiltinArrayIteratorNext, "%ArrayIteratorPrototype%.next overriden by getter");
166+
Object.defineProperty(arrayIteratorProto, "next", builtinArrayPrototypeIteratorNextDesc);
167+
})();
115168
}
116169
}
117170
];
118171

119-
testRunner.runTests(tests);
172+
testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });

‎test/typedarray/rlexe.xml‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,8 @@
2626
<test>
2727
<default>
2828
<files>TypedArrayBuiltins.js</files>
29-
<baseline>TypedArrayBuiltins.baseline</baseline>
3029
<tags>typedarray</tags>
31-
<compile-flags>-ES6TypedArrayExtensions</compile-flags>
30+
<compile-flags>-ES6TypedArrayExtensions -args summary -endargs</compile-flags>
3231
</default>
3332
</test>
3433
<test>

0 commit comments

Comments
 (0)