Changeset 216279 in webkit for trunk/Source/JavaScriptCore/dfg/DFGArrayMode.cpp

Timestamp:

May 5, 2017, 3:35:31 PM (8 years ago)

Author:

[email protected]

Message:

putDirectIndex does not properly do defineOwnProperty
​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:

File:

• trunk/Source/JavaScriptCore/dfg/DFGArrayMode.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/dfg/DFGArrayMode.cpp

@@ -187 +187 @@
     // 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()) {
@@ -236 +246 @@
     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 *this;
+                return typedArrayResult(withSpeculation(Array::OutOfBounds));
+        }
+        return typedArrayResult(withSpeculation(Array::InBounds));
     case Array::Unprofiled:
     case Array::SelectUsingPredictions: {
@@ -275 +281 @@
             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)