putDirectIndex does not properly do defineOwnProperty
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 May 2017 22:35:31 +0000 (22:35 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 May 2017 22:35:31 +0000 (22:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171591
<rdar://problem/31735695>

Reviewed by Geoffrey Garen.

JSTests:

* stress/array-prototype-splice-making-typed-array.js:
(test):
* stress/array-species-config-array-constructor.js:
(shouldThrow):
(test):
* stress/put-direct-index-broken-2.js: Added.
(assert):
(test):
(makeLengthWritable):
(set get restoreOldDesc):
* stress/put-direct-index-broken.js: Added.
(whatToTest):
(tryRunning):
(tryItOut):
* stress/put-indexed-getter-setter.js: Added.
(foo.X.prototype.set 7):
(foo.X.prototype.get 7):
(foo.X):
(foo):

Source/JavaScriptCore:

This patch fixes putDirectIndex and its JIT implementations to be
compatible with the ES6 spec. I think our code became out of date
when we implemented ArraySpeciesCreate since ArraySpeciesCreate may
return arbitrary objects. We perform putDirectIndex on that arbitrary
object. The behavior we want is as if we performed defineProperty({configurable:true, enumerable:true, writable:true}).
However, we weren't doing this. putDirectIndex assumed it could just splat
data into any descendent of JSObject's butterfly. For example, this means
we'd just splat into the butterfly of a typed array, even though a typed
array doesn't use its butterfly to store its indexed properties in the usual
way. Also, typed array properties are non-configurable, so this operation
should throw. This also means if we saw a ProxyObject, we'd just splat
into its butterfly, but this is obviously wrong because ProxyObject should
intercept the defineProperty operation.

This patch fixes this issue by adding a whitelist of cell types that can
go down putDirectIndex's fast path. Anything not in that whitelist will
simply call into defineOwnProperty.

* bytecode/ByValInfo.h:
(JSC::jitArrayModePermitsPutDirect):
* dfg/DFGArrayMode.cpp:
(JSC::DFG::ArrayMode::refine):
* jit/JITOperations.cpp:
* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncSplice):
* runtime/ClonedArguments.cpp:
(JSC::ClonedArguments::createStructure):
* runtime/JSGenericTypedArrayViewInlines.h:
(JSC::JSGenericTypedArrayView<Adaptor>::defineOwnProperty):
* runtime/JSObject.cpp:
(JSC::canDoFastPutDirectIndex):
(JSC::JSObject::defineOwnIndexedProperty):
(JSC::JSObject::putDirectIndexSlowOrBeyondVectorLength):
(JSC::JSObject::putDirectIndexBeyondVectorLength): Deleted.
* runtime/JSObject.h:
(JSC::JSObject::putDirectIndex):
(JSC::JSObject::canSetIndexQuicklyForPutDirect): Deleted.
* runtime/JSType.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@216279 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
JSTests/ChangeLog
JSTests/stress/array-prototype-splice-making-typed-array.js
JSTests/stress/array-species-config-array-constructor.js
JSTests/stress/put-direct-index-broken-2.js [new file with mode: 0644]
JSTests/stress/put-direct-index-broken.js [new file with mode: 0644]
JSTests/stress/put-indexed-getter-setter.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/ByValInfo.h
Source/JavaScriptCore/dfg/DFGArrayMode.cpp
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/runtime/ArrayPrototype.cpp
Source/JavaScriptCore/runtime/ClonedArguments.cpp
Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/JSObject.h
Source/JavaScriptCore/runtime/JSType.h

index a4dd092..54e2446 100644 (file)
@@ -1,3 +1,31 @@
+2017-05-05  Saam Barati  <sbarati@apple.com>
+
+        putDirectIndex does not properly do defineOwnProperty
+        https://bugs.webkit.org/show_bug.cgi?id=171591
+        <rdar://problem/31735695>
+
+        Reviewed by Geoffrey Garen.
+
+        * stress/array-prototype-splice-making-typed-array.js:
+        (test):
+        * stress/array-species-config-array-constructor.js:
+        (shouldThrow):
+        (test):
+        * stress/put-direct-index-broken-2.js: Added.
+        (assert):
+        (test):
+        (makeLengthWritable):
+        (set get restoreOldDesc):
+        * stress/put-direct-index-broken.js: Added.
+        (whatToTest):
+        (tryRunning):
+        (tryItOut):
+        * stress/put-indexed-getter-setter.js: Added.
+        (foo.X.prototype.set 7):
+        (foo.X.prototype.get 7):
+        (foo.X):
+        (foo):
+
 2017-05-04  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [JSC] Math unary functions should be handled by DFG
index bc7ac27..7017d6f 100644 (file)
@@ -10,24 +10,30 @@ function test(f, n = 4) {
 
 test(function() {
     // This should not crash.
-
-    // FIXME: this might need to be updated as we make our splice implementation
-    // more ES6 compliant: https://bugs.webkit.org/show_bug.cgi?id=159645
     let x = [1,2,3,4,5];
     x.constructor = Uint8Array;
     delete x[2];
     assert(!(2 in x));
-    let removed = x.splice(1,3);
-    assert(removed instanceof Uint8Array);
-    assert(removed.length === 3);
-    assert(removed[0] === 2);
-    assert(removed[1] === 0);
-    assert(removed[2] === 4);
+    let err = null;
+    try {
+        let removed = x.splice(1,3);
+        assert(removed instanceof Uint8Array);
+        assert(removed.length === 3);
+        assert(removed[0] === 2);
+        assert(removed[1] === 0);
+        assert(removed[2] === 4);
+    } catch(e) {
+        err = e;
+    }
+    assert(err.toString() === "TypeError: Attempting to configure non-configurable property on a typed array at index: 0");
 
     assert(x instanceof Array);
-    assert(x.length === 2);
+    assert(x.length === 5);
     assert(x[0] === 1);
-    assert(x[1] === 5);
+    assert(x[1] === 2);
+    assert(x[2] === undefined);
+    assert(x[3] === 4);
+    assert(x[4] === 5);
 });
 
 test(function() {
index 8477d5b..ddc3ff2 100644 (file)
@@ -19,14 +19,31 @@ Object.defineProperty(Array, Symbol.species, { value: Int32Array, configurable:
 // We can't write to the length property on a typed array by default.
 Object.defineProperty(Int32Array.prototype, "length", { value: 0, writable: true });
 
-result = foo.concat([1]);
-if (!(result instanceof Int32Array))
-    throw "concat failed";
-
-result = foo.splice();
-if (!(result instanceof Int32Array))
-    throw "splice failed";
-
-result = foo.slice();
-if (!(result instanceof Int32Array))
-    throw "slice failed";
+function shouldThrow(f, m) {
+    let err;
+    try {
+        f();
+    } catch(e) {
+        err = e;
+    }
+    if (err.toString() !== m)
+        throw new Error("Wrong error: " + err);
+}
+
+function test() {
+    const message = "TypeError: Attempting to configure non-configurable property on a typed array at index: 0";
+    shouldThrow(() => foo.concat([1]), message);
+    foo = [1,2,3,4];
+    shouldThrow(() => foo.slice(0), message);
+    foo = [1,2,3,4];
+    let r = foo.splice();
+    if (!(r instanceof Int32Array))
+        throw "Bad";
+    if (r.length !== 0)
+        throw "Bad";
+    foo = [1,2,3,4];
+    shouldThrow(() => foo.splice(0), message);
+}
+noInline(test);
+for (let i = 0; i < 3000; ++i)
+    test();
diff --git a/JSTests/stress/put-direct-index-broken-2.js b/JSTests/stress/put-direct-index-broken-2.js
new file mode 100644 (file)
index 0000000..1f47a91
--- /dev/null
@@ -0,0 +1,247 @@
+function assert(b) {
+    if (!b)
+        throw new Error("Bad assertion");
+}
+
+function test(f) {
+    for (let i = 0; i < 1000; ++i) {
+        f();
+    }
+}
+
+let __oldDesc = null;
+let __localLength;
+function makeLengthWritable() {
+    assert(__oldDesc === null);
+    __oldDesc = Object.getOwnPropertyDescriptor(Uint8ClampedArray.prototype.__proto__, "length");
+    assert(typeof __oldDesc.get === "function");
+    Reflect.defineProperty(Uint8ClampedArray.prototype.__proto__, "length", {configurable:true, get() { return __localLength; }, set(x) { __localLength = x; }});
+}
+
+function restoreOldDesc() {
+    assert(__oldDesc !== null);
+    Reflect.defineProperty(Uint8ClampedArray.prototype.__proto__, "length", __oldDesc);
+    __oldDesc = null;
+}
+
+test(function() {
+    "use strict";
+    let a = [];
+    a.push(300);
+    a.length = 4277;
+
+    let x = new Uint8ClampedArray;
+    a.__proto__ = x;
+    let err = null;
+    try {
+        let y = Array.prototype.map.call(a, x => x);
+    } catch(e) {
+        err = e;
+    }
+    assert(!!err);
+    assert(err.toString() === "TypeError: Attempting to configure non-configurable property on a typed array at index: 0");
+});
+
+test(function() {
+    let a = [];
+    for (let i = 0; i < 100; i++) {
+        a.push(i);
+    }
+    a.length = 4277;
+    let x = new Uint8ClampedArray;
+    a.__proto__ = x;
+    let err = null;
+    try {
+        let y = Array.prototype.filter.call(a, x => true);
+    } catch(e) {
+        err = e;
+    }
+    assert(err.toString() ===  "TypeError: Attempting to configure non-configurable property on a typed array at index: 0");
+});
+
+test(function() {
+    let a = [];
+    for (let i = 0; i < 100; i++) {
+        a.push(i);
+    }
+    a.length = 4277;
+    let x = new Uint8ClampedArray;
+    a.__proto__ = x;
+    let err = null;
+    let y = Array.prototype.filter.call(a, x => false);
+    assert(y instanceof Uint8ClampedArray);
+});
+
+test(function() {
+    let a = [];
+    for (let i = 0; i < 100; i++) {
+        a.push(i);
+    }
+    a.length = 4277;
+    let x = new Uint8ClampedArray;
+    a.__proto__ = x;
+
+    let err = null;
+    try {
+        let y = Array.prototype.slice.call(a, 0);
+    } catch(e) {
+        err = e;
+    }
+    assert(err.toString() === "TypeError: Attempting to configure non-configurable property on a typed array at index: 0");
+});
+
+test(function() {
+    let a = [];
+    for (let i = 0; i < 100; i++) {
+        a.push(i);
+    }
+    a.length = 4277;
+    let x = new Uint8ClampedArray;
+    a.__proto__ = x;
+
+    makeLengthWritable();
+    let y = Array.prototype.slice.call(a, 100);
+    assert(y.length === 4277 - 100);
+    assert(y.length === __localLength);
+    assert(y instanceof Uint8ClampedArray);
+    restoreOldDesc();
+});
+
+test(function() {
+    let a = [];
+    for (let i = 0; i < 100; i++) {
+        a.push(i);
+    }
+    a.length = 4277;
+    let x = new Uint8ClampedArray;
+    a.__proto__ = x;
+
+    makeLengthWritable();
+    let y = Array.prototype.splice.call(a);
+    assert(y.length === __localLength);
+    assert(y.length === 0);
+    restoreOldDesc();
+});
+
+test(function() {
+    let a = [];
+    for (let i = 0; i < 100; i++) {
+        a.push(i);
+    }
+    a.length = 4277;
+    let x = new Uint8ClampedArray;
+    a.__proto__ = x;
+
+    let err = null;
+    try {
+        let y = Array.prototype.splice.call(a, 0);
+    } catch(e) {
+        err = e;
+    }
+    assert(err.toString() === "TypeError: Attempting to configure non-configurable property on a typed array at index: 0");
+});
+
+test(function() {
+    let a = [];
+    for (let i = 0; i < 100; i++) {
+        a.push(i);
+    }
+    a.length = 4277;
+    let x = new Uint8ClampedArray;
+    a.__proto__ = x;
+
+    makeLengthWritable();
+    let y = Array.prototype.slice.call(a, 100);
+    assert(y.length === 4277 - 100);
+    assert(y.length === __localLength);
+    assert(y instanceof Uint8ClampedArray);
+    restoreOldDesc();
+});
+
+test(function() {
+    let a = [];
+    for (let i = 0; i < 100; i++) {
+        a.push(i);
+    }
+    a.length = 4277;
+    let calls = 0;
+    let target = {};
+    a.__proto__ = {
+        constructor: {
+            [Symbol.species]: function(length) {
+                assert(length === 4277)
+                return new Proxy(target, {
+                    defineProperty(...args) {
+                        ++calls;
+                        return Reflect.defineProperty(...args);
+                    }
+                });
+            }
+        }
+    };
+    let y = Array.prototype.map.call(a, x => x);
+    assert(calls === 100);
+    for (let i = 0; i < 100; ++i)
+        assert(target[i] === i);
+});
+
+test(function() {
+    let a = [];
+    for (let i = 0; i < 100; i++) {
+        a.push(i);
+    }
+    a.length = 4277;
+    let calls = 0;
+    let target = {};
+    a.__proto__ = {
+        constructor: {
+            [Symbol.species]: function(length) {
+                assert(length === 0)
+                return new Proxy(target, {
+                    defineProperty(...args) {
+                        ++calls;
+                        return Reflect.defineProperty(...args);
+                    }
+                });
+            }
+        }
+    };
+    let y = Array.prototype.filter.call(a, x => true);
+    assert(calls === 100);
+    for (let i = 0; i < 100; ++i)
+        assert(target[i] === i);
+});
+
+test(function() {
+    let a = [];
+    for (let i = 0; i < 100; i++) {
+        a.push(i);
+    }
+    a.length = 4277;
+    let calls = 0;
+    let target = {};
+    let keys = [];
+    a.__proto__ = {
+        constructor: {
+            [Symbol.species]: function(length) {
+                assert(length === 4277)
+                return new Proxy(target, {
+                    defineProperty(...args) {
+                        keys.push(args[1])
+                        ++calls;
+                        return Reflect.defineProperty(...args);
+                    }
+                });
+            }
+        }
+    };
+    let y = Array.prototype.slice.call(a, 0);
+    assert(calls === 101); // length gets defined too.
+    assert(keys.length === 101);
+    for (let i = 0; i < 100; ++i) {
+        assert(parseInt(keys[i]) === i);
+        assert(target[i] === i);
+    }
+    assert(keys[keys.length - 1] === "length");
+    assert(target.length === 4277);
+});
diff --git a/JSTests/stress/put-direct-index-broken.js b/JSTests/stress/put-direct-index-broken.js
new file mode 100644 (file)
index 0000000..ce7d782
--- /dev/null
@@ -0,0 +1,25 @@
+function whatToTest(code){return {allowExec: true,};}
+function tryRunning(f, code, wtt)
+{
+    uneval = true
+    try {var rv = f();} catch(runError) {}
+    try {if ('__defineSetter__' in this) {delete this.uneval;} } catch(e) {}
+}
+function tryItOut(code)
+{
+    var wtt = true;
+    var f;
+    try {f = new Function(code);} catch(compileError) {}
+        tryRunning(f, code, wtt);
+}
+tryItOut(`a0 = []; 
+        r0 = /x/; 
+        t0 = new Uint8ClampedArray;
+        o1 = {};
+        g1 = this;
+        v2 = null;`);
+
+tryItOut("func = (function(x, y) {});");
+tryItOut("for (var p in g1) { this.a0[new func([].map(q => q, null), x)]; }");
+tryItOut("a0.push(o1.m1);a0.length = (4277);a0.__proto__ = this.t0;");
+tryItOut("\"use strict\"; a0 = Array.prototype.map.call(a0, (function() {}));");
diff --git a/JSTests/stress/put-indexed-getter-setter.js b/JSTests/stress/put-indexed-getter-setter.js
new file mode 100644 (file)
index 0000000..4375e1e
--- /dev/null
@@ -0,0 +1,18 @@
+function foo() {
+    let setterValue = 0;
+    class X {
+        static set 7(x) { setterValue = x; }
+        static get 7() { }
+    };
+    X[7] = 27;
+    if (setterValue !== 27)
+        throw new Error("Bad")
+}
+noInline(foo);
+
+for (let i = 0; i < 10000; ++i)
+    foo();
+
+Object.defineProperty(Object.prototype, "7", {get() { return 500; }, set(x) { }}); // this shouldn't change the test at all, it should be doing defineOwnProperty.
+for (let i = 0; i < 10000; ++i)
+    foo();
index 285fa86..f367c98 100644 (file)
@@ -1,3 +1,50 @@
+2017-05-05  Saam Barati  <sbarati@apple.com>
+
+        putDirectIndex does not properly do defineOwnProperty
+        https://bugs.webkit.org/show_bug.cgi?id=171591
+        <rdar://problem/31735695>
+
+        Reviewed by Geoffrey Garen.
+
+        This patch fixes putDirectIndex and its JIT implementations to be
+        compatible with the ES6 spec. I think our code became out of date
+        when we implemented ArraySpeciesCreate since ArraySpeciesCreate may
+        return arbitrary objects. We perform putDirectIndex on that arbitrary
+        object. The behavior we want is as if we performed defineProperty({configurable:true, enumerable:true, writable:true}).
+        However, we weren't doing this. putDirectIndex assumed it could just splat
+        data into any descendent of JSObject's butterfly. For example, this means
+        we'd just splat into the butterfly of a typed array, even though a typed
+        array doesn't use its butterfly to store its indexed properties in the usual
+        way. Also, typed array properties are non-configurable, so this operation
+        should throw. This also means if we saw a ProxyObject, we'd just splat
+        into its butterfly, but this is obviously wrong because ProxyObject should
+        intercept the defineProperty operation.
+        
+        This patch fixes this issue by adding a whitelist of cell types that can
+        go down putDirectIndex's fast path. Anything not in that whitelist will
+        simply call into defineOwnProperty.
+
+        * bytecode/ByValInfo.h:
+        (JSC::jitArrayModePermitsPutDirect):
+        * dfg/DFGArrayMode.cpp:
+        (JSC::DFG::ArrayMode::refine):
+        * jit/JITOperations.cpp:
+        * runtime/ArrayPrototype.cpp:
+        (JSC::arrayProtoFuncSplice):
+        * runtime/ClonedArguments.cpp:
+        (JSC::ClonedArguments::createStructure):
+        * runtime/JSGenericTypedArrayViewInlines.h:
+        (JSC::JSGenericTypedArrayView<Adaptor>::defineOwnProperty):
+        * runtime/JSObject.cpp:
+        (JSC::canDoFastPutDirectIndex):
+        (JSC::JSObject::defineOwnIndexedProperty):
+        (JSC::JSObject::putDirectIndexSlowOrBeyondVectorLength):
+        (JSC::JSObject::putDirectIndexBeyondVectorLength): Deleted.
+        * runtime/JSObject.h:
+        (JSC::JSObject::putDirectIndex):
+        (JSC::JSObject::canSetIndexQuicklyForPutDirect): Deleted.
+        * runtime/JSType.h:
+
 2017-05-05  Guillaume Emont  <guijemont@igalia.com>
 
         [JSC] include JSCInlines.h in ObjectInitializationScope.cpp
index e5fa708..661675f 100644 (file)
@@ -164,6 +164,26 @@ inline bool jitArrayModePermitsPut(JITArrayMode mode)
     }
 }
 
+inline bool jitArrayModePermitsPutDirect(JITArrayMode mode)
+{
+    // We don't allow typed array putDirect here since putDirect has
+    // defineOwnProperty({configurable: true, writable:true, enumerable:true})
+    // semantics. Typed array indexed properties are non-configurable by
+    // default, so we can't simply store to a typed array for putDirect.
+    //
+    // We could model putDirect on ScopedArguments and DirectArguments, but we
+    // haven't found any performance incentive to do it yet.
+    switch (mode) {
+    case JITInt32:
+    case JITDouble:
+    case JITContiguous:
+    case JITArrayStorage:
+        return true;
+    default:
+        return false;
+    }
+}
+
 inline TypedArrayType typedArrayTypeForJITArrayMode(JITArrayMode mode)
 {
     switch (mode) {
index 431061d..c888738 100644 (file)
@@ -186,6 +186,16 @@ ArrayMode ArrayMode::refine(
     // don't want to). So, for now, we assume that if the base is not a cell according
     // to value profiling, but the array profile tells us something else, then we
     // should just trust the array profile.
+
+    auto typedArrayResult = [&] (ArrayMode result) -> ArrayMode {
+        if (node->op() == PutByValDirect) {
+            // This is semantically identical to defineOwnProperty({configurable: true, writable:true, enumerable:true}),
+            // which we can't model as a simple store to the typed array since typed array indexed properties
+            // are non-configurable.
+            return ArrayMode(Array::Generic);
+        }
+        return result;
+    };
     
     switch (type()) {
     case Array::SelectUsingArguments:
@@ -235,15 +245,11 @@ ArrayMode ArrayMode::refine(
     case Array::Uint32Array:
     case Array::Float32Array:
     case Array::Float64Array:
-        switch (node->op()) {
-        case PutByVal:
+        if (node->op() == PutByVal) {
             if (graph.hasExitSite(node->origin.semantic, OutOfBounds) || !isInBounds())
-                return withSpeculation(Array::OutOfBounds);
-            return withSpeculation(Array::InBounds);
-        default:
-            return withSpeculation(Array::InBounds);
+                return typedArrayResult(withSpeculation(Array::OutOfBounds));
         }
-        return *this;
+        return typedArrayResult(withSpeculation(Array::InBounds));
     case Array::Unprofiled:
     case Array::SelectUsingPredictions: {
         base &= ~SpecOther;
@@ -274,33 +280,33 @@ ArrayMode ArrayMode::refine(
             result = withSpeculation(Array::InBounds);
             break;
         }
-        
+
         if (isInt8ArraySpeculation(base))
-            return result.withType(Array::Int8Array);
+            return typedArrayResult(result.withType(Array::Int8Array));
         
         if (isInt16ArraySpeculation(base))
-            return result.withType(Array::Int16Array);
+            return typedArrayResult(result.withType(Array::Int16Array));
         
         if (isInt32ArraySpeculation(base))
-            return result.withType(Array::Int32Array);
+            return typedArrayResult(result.withType(Array::Int32Array));
         
         if (isUint8ArraySpeculation(base))
-            return result.withType(Array::Uint8Array);
+            return typedArrayResult(result.withType(Array::Uint8Array));
         
         if (isUint8ClampedArraySpeculation(base))
-            return result.withType(Array::Uint8ClampedArray);
+            return typedArrayResult(result.withType(Array::Uint8ClampedArray));
         
         if (isUint16ArraySpeculation(base))
-            return result.withType(Array::Uint16Array);
+            return typedArrayResult(result.withType(Array::Uint16Array));
         
         if (isUint32ArraySpeculation(base))
-            return result.withType(Array::Uint32Array);
+            return typedArrayResult(result.withType(Array::Uint32Array));
         
         if (isFloat32ArraySpeculation(base))
-            return result.withType(Array::Float32Array);
+            return typedArrayResult(result.withType(Array::Float32Array));
         
         if (isFloat64ArraySpeculation(base))
-            return result.withType(Array::Float64Array);
+            return typedArrayResult(result.withType(Array::Float64Array));
 
         if (type() == Array::Unprofiled)
             return ArrayMode(Array::ForceExit);
index 921069e..4a4a2eb 100644 (file)
@@ -584,15 +584,20 @@ static void directPutByVal(CallFrame* callFrame, JSObject* baseObject, JSValue s
         byValInfo->tookSlowPath = true;
         uint32_t index = subscript.asUInt32();
         ASSERT(isIndex(index));
-        if (baseObject->canSetIndexQuicklyForPutDirect(index)) {
-            baseObject->setIndexQuickly(callFrame->vm(), index, value);
-            return;
+
+        switch (baseObject->indexingType()) {
+        case ALL_INT32_INDEXING_TYPES:
+        case ALL_DOUBLE_INDEXING_TYPES:
+        case ALL_CONTIGUOUS_INDEXING_TYPES:
+        case ALL_ARRAY_STORAGE_INDEXING_TYPES:
+            if (index < baseObject->butterfly()->vectorLength())
+                break;
+            FALLTHROUGH;
+        default:
+            byValInfo->arrayProfile->setOutOfBounds();
+            break;
         }
 
-        // FIXME: This will make us think that in-bounds typed array accesses are actually
-        // out-of-bounds.
-        // https://bugs.webkit.org/show_bug.cgi?id=149886
-        byValInfo->arrayProfile->setOutOfBounds();
         baseObject->putDirectIndex(callFrame, index, value, 0, isStrictMode ? PutDirectIndexShouldThrow : PutDirectIndexShouldNotThrow);
         return;
     }
@@ -732,7 +737,7 @@ static OptimizationResult tryDirectPutByValOptimize(ExecState* exec, JSObject* o
         if (hasOptimizableIndexing(structure)) {
             // Attempt to optimize.
             JITArrayMode arrayMode = jitArrayModeForStructure(structure);
-            if (jitArrayModePermitsPut(arrayMode) && arrayMode != byValInfo->arrayMode) {
+            if (jitArrayModePermitsPutDirect(arrayMode) && arrayMode != byValInfo->arrayMode) {
                 CodeBlock* codeBlock = exec->codeBlock();
                 ConcurrentJSLocker locker(codeBlock->m_lock);
                 byValInfo->arrayProfile->computeUpdatedPrediction(locker, codeBlock, structure);
index 71aab88..a185d97 100644 (file)
@@ -1031,32 +1031,22 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncSplice(ExecState* exec)
         result = asArray(thisObj)->fastSlice(*exec, actualStart, actualDeleteCount);
 
     if (!result) {
-        if (speciesResult.first == SpeciesConstructResult::CreatedObject) {
+        if (speciesResult.first == SpeciesConstructResult::CreatedObject)
             result = speciesResult.second;
-            
-            for (unsigned k = 0; k < actualDeleteCount; ++k) {
-                JSValue v = getProperty(exec, thisObj, k + actualStart);
-                RETURN_IF_EXCEPTION(scope, encodedJSValue());
-                if (UNLIKELY(!v))
-                    continue;
-                result->putByIndexInline(exec, k, v, true);
-                RETURN_IF_EXCEPTION(scope, encodedJSValue());
-            }
-        } else {
+        else {
             result = JSArray::tryCreate(vm, exec->lexicalGlobalObject()->arrayStructureForIndexingTypeDuringAllocation(ArrayWithUndecided), actualDeleteCount);
             if (UNLIKELY(!result)) {
                 throwOutOfMemoryError(exec, scope);
                 return encodedJSValue();
             }
-
-            for (unsigned k = 0; k < actualDeleteCount; ++k) {
-                JSValue v = getProperty(exec, thisObj, k + actualStart);
-                RETURN_IF_EXCEPTION(scope, encodedJSValue());
-                if (UNLIKELY(!v))
-                    continue;
-                result->putDirectIndex(exec, k, v);
-                RETURN_IF_EXCEPTION(scope, encodedJSValue());
-            }
+        }
+        for (unsigned k = 0; k < actualDeleteCount; ++k) {
+            JSValue v = getProperty(exec, thisObj, k + actualStart);
+            RETURN_IF_EXCEPTION(scope, encodedJSValue());
+            if (UNLIKELY(!v))
+                continue;
+            result->putDirectIndex(exec, k, v, 0, PutDirectIndexShouldThrow);
+            RETURN_IF_EXCEPTION(scope, encodedJSValue());
         }
     }
 
index 19aab8e..1e12cf9 100644 (file)
@@ -151,7 +151,7 @@ ClonedArguments* ClonedArguments::createByCopyingFrom(
 
 Structure* ClonedArguments::createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype, IndexingType indexingType)
 {
-    Structure* structure = Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info(), indexingType);
+    Structure* structure = Structure::create(vm, globalObject, prototype, TypeInfo(ClonedArgumentsType, StructureFlags), info(), indexingType);
     PropertyOffset offset;
     structure = structure->addPropertyTransition(vm, structure, vm.propertyNames->length, DontEnum, offset);
     ASSERT(offset == clonedArgumentsLengthPropertyOffset);
index f019659..1fd7318 100644 (file)
@@ -394,15 +394,21 @@ bool JSGenericTypedArrayView<Adaptor>::defineOwnProperty(
     auto scope = DECLARE_THROW_SCOPE(vm);
     JSGenericTypedArrayView* thisObject = jsCast<JSGenericTypedArrayView*>(object);
 
-    if (parseIndex(propertyName)) {
+    if (std::optional<uint32_t> index = parseIndex(propertyName)) {
+        auto throwTypeErrorIfNeeded = [&] (const char* errorMessage) -> bool {
+            if (shouldThrow)
+                throwTypeError(exec, scope, makeString(errorMessage, String::number(*index)));
+            return false;
+        };
+
         if (descriptor.isAccessorDescriptor())
-            return typeError(exec, scope, shouldThrow, ASCIILiteral("Attempting to store accessor indexed property on a typed array."));
+            return throwTypeErrorIfNeeded("Attempting to store accessor property on a typed array at index: ");
 
         if (descriptor.configurable())
-            return typeError(exec, scope, shouldThrow, ASCIILiteral("Attempting to configure non-configurable property."));
+            return throwTypeErrorIfNeeded("Attempting to configure non-configurable property on a typed array at index: ");
 
         if (!descriptor.enumerable() || !descriptor.writable())
-            return typeError(exec, scope, shouldThrow, ASCIILiteral("Attempting to store non-enumerable or non-writable indexed property on a typed array."));
+            return throwTypeErrorIfNeeded("Attempting to store non-enumerable or non-writable property on a typed array at index: ");
 
         if (descriptor.value()) {
             PutPropertySlot unused(JSValue(thisObject), shouldThrow);
index 514cb83..016d07b 100644 (file)
@@ -2356,6 +2356,15 @@ bool JSObject::putIndexedDescriptor(ExecState* exec, SparseArrayEntry* entryInMa
     return true;
 }
 
+ALWAYS_INLINE static bool canDoFastPutDirectIndex(JSObject* object)
+{
+    return isJSArray(object)
+        || isJSFinalObject(object)
+        || object->type() == DirectArgumentsType
+        || object->type() == ScopedArgumentsType
+        || object->type() == ClonedArgumentsType;
+}
+
 // Defined in ES5.1 8.12.9
 bool JSObject::defineOwnIndexedProperty(ExecState* exec, unsigned index, const PropertyDescriptor& descriptor, bool throwException)
 {
@@ -2369,7 +2378,7 @@ bool JSObject::defineOwnIndexedProperty(ExecState* exec, unsigned index, const P
         // FIXME: this will pessimistically assume that if attributes are missing then they'll default to false
         // however if the property currently exists missing attributes will override from their current 'true'
         // state (i.e. defineOwnProperty could be used to set a value without needing to entering 'SparseMode').
-        if (!descriptor.attributes() && descriptor.value()) {
+        if (!descriptor.attributes() && descriptor.value() && canDoFastPutDirectIndex(this)) {
             ASSERT(!descriptor.isAccessorDescriptor());
             return putDirectIndex(exec, index, descriptor.value(), 0, throwException ? PutDirectIndexShouldThrow : PutDirectIndexShouldNotThrow);
         }
@@ -2808,9 +2817,15 @@ bool JSObject::putDirectIndexBeyondVectorLengthWithArrayStorage(ExecState* exec,
     return true;
 }
 
-bool JSObject::putDirectIndexBeyondVectorLength(ExecState* exec, unsigned i, JSValue value, unsigned attributes, PutDirectIndexMode mode)
+bool JSObject::putDirectIndexSlowOrBeyondVectorLength(ExecState* exec, unsigned i, JSValue value, unsigned attributes, PutDirectIndexMode mode)
 {
     VM& vm = exec->vm();
+    
+    if (!canDoFastPutDirectIndex(this)) {
+        PropertyDescriptor descriptor;
+        descriptor.setDescriptor(value, attributes);
+        return methodTable(vm)->defineOwnProperty(this, exec, Identifier::from(exec, i), descriptor, mode == PutDirectIndexShouldThrow);
+    }
 
     // i should be a valid array index that is outside of the current vector.
     ASSERT(i <= MAX_ARRAY_INDEX);
@@ -2854,7 +2869,7 @@ bool JSObject::putDirectIndexBeyondVectorLength(ExecState* exec, unsigned i, JSV
         }
         if (!value.isInt32()) {
             convertInt32ForValue(vm, value);
-            return putDirectIndexBeyondVectorLength(exec, i, value, attributes, mode);
+            return putDirectIndexSlowOrBeyondVectorLength(exec, i, value, attributes, mode);
         }
         putByIndexBeyondVectorLengthWithoutAttributes<Int32Shape>(exec, i, value);
         return true;
@@ -2868,12 +2883,12 @@ bool JSObject::putDirectIndexBeyondVectorLength(ExecState* exec, unsigned i, JSV
         }
         if (!value.isNumber()) {
             convertDoubleToContiguous(vm);
-            return putDirectIndexBeyondVectorLength(exec, i, value, attributes, mode);
+            return putDirectIndexSlowOrBeyondVectorLength(exec, i, value, attributes, mode);
         }
         double valueAsDouble = value.asNumber();
         if (valueAsDouble != valueAsDouble) {
             convertDoubleToContiguous(vm);
-            return putDirectIndexBeyondVectorLength(exec, i, value, attributes, mode);
+            return putDirectIndexSlowOrBeyondVectorLength(exec, i, value, attributes, mode);
         }
         putByIndexBeyondVectorLengthWithoutAttributes<DoubleShape>(exec, i, value);
         return true;
index 2c16fc2..d35419a 100644 (file)
@@ -196,6 +196,7 @@ public:
     // putByIndex assumes that the receiver is this JSCell object.
     JS_EXPORT_PRIVATE static bool putByIndex(JSCell*, ExecState*, unsigned propertyName, JSValue, bool shouldThrow);
         
+    // This performs the ECMAScript Set() operation.
     ALWAYS_INLINE bool putByIndexInline(ExecState* exec, unsigned propertyName, JSValue value, bool shouldThrow)
     {
         if (canSetIndexQuickly(propertyName)) {
@@ -209,21 +210,44 @@ public:
     //  - the prototype chain is not consulted
     //  - accessors are not called.
     //  - it will ignore extensibility and read-only properties if PutDirectIndexLikePutDirect is passed as the mode (the default).
-    // This method creates a property with attributes writable, enumerable and configurable all set to true.
+    // This method creates a property with attributes writable, enumerable and configurable all set to true if attributes is zero,
+    // otherwise, it creates a property with the provided attributes. Semantically, this is performing defineOwnProperty.
     bool putDirectIndex(ExecState* exec, unsigned propertyName, JSValue value, unsigned attributes, PutDirectIndexMode mode)
     {
-        if (!attributes && canSetIndexQuicklyForPutDirect(propertyName)) {
+        auto canSetIndexQuicklyForPutDirect = [&] () -> bool {
+            switch (indexingType()) {
+            case ALL_BLANK_INDEXING_TYPES:
+            case ALL_UNDECIDED_INDEXING_TYPES:
+                return false;
+            case ALL_INT32_INDEXING_TYPES:
+            case ALL_DOUBLE_INDEXING_TYPES:
+            case ALL_CONTIGUOUS_INDEXING_TYPES:
+            case ALL_ARRAY_STORAGE_INDEXING_TYPES:
+                return propertyName < m_butterfly.get()->vectorLength();
+            default:
+                RELEASE_ASSERT_NOT_REACHED();
+                return false;
+            }
+        };
+        
+        if (!attributes && canSetIndexQuicklyForPutDirect()) {
             setIndexQuickly(exec->vm(), propertyName, value);
             return true;
         }
-        return putDirectIndexBeyondVectorLength(exec, propertyName, value, attributes, mode);
+        return putDirectIndexSlowOrBeyondVectorLength(exec, propertyName, value, attributes, mode);
     }
+    // This is semantically equivalent to performing defineOwnProperty(propertyName, {configurable:true, writable:true, enumerable:true, value:value}).
     bool putDirectIndex(ExecState* exec, unsigned propertyName, JSValue value)
     {
         return putDirectIndex(exec, propertyName, value, 0, PutDirectIndexLikePutDirect);
     }
 
-    // A non-throwing version of putDirect and putDirectIndex.
+    // A generally non-throwing version of putDirect and putDirectIndex.
+    // However, it's only guaranteed to not throw based on what the receiver is.
+    // For example, if the receiver is a ProxyObject, this is not guaranteed, since
+    // it may call into arbitrary JS code. It's the responsibility of the user of
+    // this API to ensure that the receiver object is a well known type if they
+    // want to ensure that this won't throw an exception.
     JS_EXPORT_PRIVATE bool putDirectMayBeIndex(ExecState*, PropertyName, JSValue);
         
     bool hasIndexingHeader() const
@@ -352,23 +376,6 @@ public:
         }
     }
         
-    bool canSetIndexQuicklyForPutDirect(unsigned i)
-    {
-        switch (indexingType()) {
-        case ALL_BLANK_INDEXING_TYPES:
-        case ALL_UNDECIDED_INDEXING_TYPES:
-            return false;
-        case ALL_INT32_INDEXING_TYPES:
-        case ALL_DOUBLE_INDEXING_TYPES:
-        case ALL_CONTIGUOUS_INDEXING_TYPES:
-        case ALL_ARRAY_STORAGE_INDEXING_TYPES:
-            return i < m_butterfly.get()->vectorLength();
-        default:
-            RELEASE_ASSERT_NOT_REACHED();
-            return false;
-        }
-    }
-        
     void setIndexQuickly(VM& vm, unsigned i, JSValue v)
     {
         Butterfly* butterfly = m_butterfly.get();
@@ -1016,7 +1023,7 @@ private:
         
     bool putByIndexBeyondVectorLength(ExecState*, unsigned propertyName, JSValue, bool shouldThrow);
     bool putDirectIndexBeyondVectorLengthWithArrayStorage(ExecState*, unsigned propertyName, JSValue, unsigned attributes, PutDirectIndexMode, ArrayStorage*);
-    JS_EXPORT_PRIVATE bool putDirectIndexBeyondVectorLength(ExecState*, unsigned propertyName, JSValue, unsigned attributes, PutDirectIndexMode);
+    JS_EXPORT_PRIVATE bool putDirectIndexSlowOrBeyondVectorLength(ExecState*, unsigned propertyName, JSValue, unsigned attributes, PutDirectIndexMode);
         
     unsigned getNewVectorLength(unsigned indexBias, unsigned currentVectorLength, unsigned currentLength, unsigned desiredLength);
     unsigned getNewVectorLength(unsigned desiredLength);
index 62e3772..ca5aa63 100644 (file)
@@ -99,7 +99,9 @@ enum JSType : uint8_t {
 
     WebAssemblyFunctionType,
 
-    LastJSCObjectType = WebAssemblyFunctionType,
+    ClonedArgumentsType,
+
+    LastJSCObjectType = ClonedArgumentsType,
     MaxJSType = 0b11111111,
 };