Changeset 216279 in webkit for trunk/Source/JavaScriptCore/jit

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/jit/JITOperations.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/jit/JITOperations.cpp

@@ -585 +585 @@
         uint32_t index = subscript.asUInt32();
         ASSERT(isIndex(index));
-        if (baseObject->canSetIndexQuicklyForPutDirect(index)) {
-            baseObject->setIndexQuickly(callFrame->vm(), index, value);
-            return;
-        }
-
-        // 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();
+
+        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;
+        }
+
         baseObject->putDirectIndex(callFrame, index, value, 0, isStrictMode ? PutDirectIndexShouldThrow : PutDirectIndexShouldNotThrow);
         return;
@@ -733 +738 @@
             // 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);