Changeset 196414 in webkit

Timestamp:

Feb 10, 2016 8:06:37 PM (8 years ago)

Author:

keith_miller@apple.com

Message:

Symbol.species accessors on builtin constructors should be configurable
​https://bugs.webkit.org/show_bug.cgi?id=154097

Reviewed by Benjamin Poulain.

We did not have the Symbol.species accessors on our builtin constructors
marked as configurable. This does not accurately follow the ES6 spec as
the ES6 spec states that all default accessors on builtins should be
configurable. This means that we need an additional watchpoint on
ArrayConstructor to make sure that no users re-configures Symbol.species.

• runtime/ArrayConstructor.cpp:

(JSC::ArrayConstructor::finishCreation):

• runtime/ArrayPrototype.cpp:

(JSC::speciesConstructArray):
(JSC::ArrayPrototype::setConstructor):
(JSC::ArrayPrototypeAdaptiveInferredPropertyWatchpoint::handleFire):

• runtime/ArrayPrototype.h:

(JSC::ArrayPrototype::didChangeConstructorOrSpeciesProperties):
(JSC::ArrayPrototype::didChangeConstructorProperty): Deleted.

• runtime/JSArrayBufferConstructor.cpp:

(JSC::JSArrayBufferConstructor::finishCreation):

• runtime/JSPromiseConstructor.cpp:

(JSC::JSPromiseConstructor::finishCreation):

• runtime/JSTypedArrayViewConstructor.cpp:

(JSC::JSTypedArrayViewConstructor::finishCreation):

• runtime/MapConstructor.cpp:

(JSC::MapConstructor::finishCreation):

• runtime/RegExpConstructor.cpp:

(JSC::RegExpConstructor::finishCreation):

• runtime/SetConstructor.cpp:

(JSC::SetConstructor::finishCreation):

• tests/stress/array-species-config-array-constructor.js: Added.

(A):

• tests/stress/symbol-species.js:

(testSymbolSpeciesOnConstructor):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/ArrayConstructor.cpp (modified) (1 diff)
• runtime/ArrayPrototype.cpp (modified) (4 diffs)
• runtime/ArrayPrototype.h (modified) (2 diffs)
• runtime/JSArrayBufferConstructor.cpp (modified) (1 diff)
• runtime/JSPromiseConstructor.cpp (modified) (1 diff)
• runtime/JSTypedArrayViewConstructor.cpp (modified) (1 diff)
• runtime/MapConstructor.cpp (modified) (1 diff)
• runtime/RegExpConstructor.cpp (modified) (1 diff)
• runtime/SetConstructor.cpp (modified) (1 diff)
• tests/stress/array-species-config-array-constructor.js (added)
• tests/stress/symbol-species.js (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-02-10  Keith Miller  <keith_miller@apple.com>
+
+        Symbol.species accessors on builtin constructors should be configurable
+        https://bugs.webkit.org/show_bug.cgi?id=154097
+
+        Reviewed by Benjamin Poulain.
+
+        We did not have the Symbol.species accessors on our builtin constructors
+        marked as configurable. This does not accurately follow the ES6 spec as
+        the ES6 spec states that all default accessors on builtins should be
+        configurable. This means that we need an additional watchpoint on
+        ArrayConstructor to make sure that no users re-configures Symbol.species.
+
+        * runtime/ArrayConstructor.cpp:
+        (JSC::ArrayConstructor::finishCreation):
+        * runtime/ArrayPrototype.cpp:
+        (JSC::speciesConstructArray):
+        (JSC::ArrayPrototype::setConstructor):
+        (JSC::ArrayPrototypeAdaptiveInferredPropertyWatchpoint::handleFire):
+        * runtime/ArrayPrototype.h:
+        (JSC::ArrayPrototype::didChangeConstructorOrSpeciesProperties):
+        (JSC::ArrayPrototype::didChangeConstructorProperty): Deleted.
+        * runtime/JSArrayBufferConstructor.cpp:
+        (JSC::JSArrayBufferConstructor::finishCreation):
+        * runtime/JSPromiseConstructor.cpp:
+        (JSC::JSPromiseConstructor::finishCreation):
+        * runtime/JSTypedArrayViewConstructor.cpp:
+        (JSC::JSTypedArrayViewConstructor::finishCreation):
+        * runtime/MapConstructor.cpp:
+        (JSC::MapConstructor::finishCreation):
+        * runtime/RegExpConstructor.cpp:
+        (JSC::RegExpConstructor::finishCreation):
+        * runtime/SetConstructor.cpp:
+        (JSC::SetConstructor::finishCreation):
+        * tests/stress/array-species-config-array-constructor.js: Added.
+        (A):
+        * tests/stress/symbol-species.js:
+        (testSymbolSpeciesOnConstructor):
+
 2016-02-10  Benjamin Poulain  <benjamin@webkit.org>
 

trunk/Source/JavaScriptCore/runtime/ArrayConstructor.cpp

@@ -68 +68 @@
     putDirectWithoutTransition(vm, vm.propertyNames->prototype, arrayPrototype, DontEnum | DontDelete | ReadOnly);
     putDirectWithoutTransition(vm, vm.propertyNames->length, jsNumber(1), ReadOnly | DontEnum | DontDelete);
-    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum | DontDelete);
+    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum);
 }
 

trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

@@ -193 +193 @@
         if (LIKELY(!thisObject->hasCustomProperties()
             && thisObject->globalObject()->arrayPrototype() == thisObject->prototype()
-            && !thisObject->globalObject()->arrayPrototype()->didChangeConstructorProperty()))
+            && !thisObject->globalObject()->arrayPrototype()->didChangeConstructorOrSpeciesProperties()))
             return std::make_pair(SpeciesConstructResult::FastPath, nullptr);
 
@@ -1061 +1061 @@
     putDirectWithoutTransition(vm, vm.propertyNames->constructor, constructorProperty, attributes);
 
+    // Do the watchpoint on our constructor property
     PropertyOffset offset = this->structure()->get(vm, vm.propertyNames->constructor);
     ASSERT(isValidOffset(offset));
@@ -1070 +1071 @@
     m_constructorWatchpoint = std::make_unique<ArrayPrototypeAdaptiveInferredPropertyWatchpoint>(condition, this);
     m_constructorWatchpoint->install();
+    
+    // Do the watchpoint on the constructor's Symbol.species property
+    offset = constructorProperty->structure()->get(vm, vm.propertyNames->speciesSymbol);
+    ASSERT(isValidOffset(offset));
+    constructorProperty->structure()->startWatchingPropertyForReplacements(vm, offset);
+
+    ASSERT(constructorProperty->getDirect(offset).isGetterSetter());
+    condition = ObjectPropertyCondition::equivalence(vm, this, constructorProperty, vm.propertyNames->speciesSymbol.impl(), constructorProperty->getDirect(offset));
+    ASSERT(condition.isWatchable());
+
+    m_constructorSpeciesWatchpoint = std::make_unique<ArrayPrototypeAdaptiveInferredPropertyWatchpoint>(condition, this);
+    m_constructorSpeciesWatchpoint->install();
 }
 
@@ -1085 +1098 @@
     StringFireDetail stringDetail(out.toCString().data());
 
-    m_arrayPrototype->m_didChangeConstructorProperty = true;
+    m_arrayPrototype->m_didChangeConstructorOrSpeciesProperties = true;
 }
 

trunk/Source/JavaScriptCore/runtime/ArrayPrototype.h

@@ -47 +47 @@
     void setConstructor(VM&, JSObject* constructorProperty, unsigned attributes);
 
-    bool didChangeConstructorProperty() const { return m_didChangeConstructorProperty; }
+    bool didChangeConstructorOrSpeciesProperties() const { return m_didChangeConstructorOrSpeciesProperties; }
 
     static const bool needsDestruction = false;
@@ -60 +60 @@
     friend ArrayPrototypeAdaptiveInferredPropertyWatchpoint;
     std::unique_ptr<ArrayPrototypeAdaptiveInferredPropertyWatchpoint> m_constructorWatchpoint;
-    bool m_didChangeConstructorProperty = false;
+    std::unique_ptr<ArrayPrototypeAdaptiveInferredPropertyWatchpoint> m_constructorSpeciesWatchpoint;
+    bool m_didChangeConstructorOrSpeciesProperties = false;
 };
 

trunk/Source/JavaScriptCore/runtime/JSArrayBufferConstructor.cpp

@@ -54 +54 @@
     putDirectWithoutTransition(vm, vm.propertyNames->prototype, prototype, DontEnum | DontDelete | ReadOnly);
     putDirectWithoutTransition(vm, vm.propertyNames->length, jsNumber(1), DontEnum | DontDelete | ReadOnly);
-    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum | DontDelete);
+    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum);
 
     JSGlobalObject* globalObject = this->globalObject();

trunk/Source/JavaScriptCore/runtime/JSPromiseConstructor.cpp

@@ -86 +86 @@
     putDirectWithoutTransition(vm, vm.propertyNames->prototype, promisePrototype, DontEnum | DontDelete | ReadOnly);
     putDirectWithoutTransition(vm, vm.propertyNames->length, jsNumber(1), ReadOnly | DontEnum | DontDelete);
-    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum | DontDelete);
+    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum);
 }
 

trunk/Source/JavaScriptCore/runtime/JSTypedArrayViewConstructor.cpp

@@ -51 +51 @@
     putDirectWithoutTransition(vm, vm.propertyNames->prototype, prototype, DontEnum | DontDelete | ReadOnly);
     putDirectWithoutTransition(vm, vm.propertyNames->length, jsNumber(3), DontEnum | DontDelete | ReadOnly);
-    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum | DontDelete);
+    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum);
 
     JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->of, typedArrayConstructorOfCodeGenerator, DontEnum);

trunk/Source/JavaScriptCore/runtime/MapConstructor.cpp

@@ -46 +46 @@
     putDirectWithoutTransition(vm, vm.propertyNames->prototype, mapPrototype, DontEnum | DontDelete | ReadOnly);
     putDirectWithoutTransition(vm, vm.propertyNames->length, jsNumber(0), ReadOnly | DontEnum | DontDelete);
-    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum | DontDelete);
+    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum);
 }
 

trunk/Source/JavaScriptCore/runtime/RegExpConstructor.cpp

@@ -103 +103 @@
     putDirectWithoutTransition(vm, vm.propertyNames->length, jsNumber(2), ReadOnly | DontDelete | DontEnum);
 
-    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum | DontDelete);
+    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum);
 }
 

trunk/Source/JavaScriptCore/runtime/SetConstructor.cpp

@@ -47 +47 @@
     putDirectWithoutTransition(vm, vm.propertyNames->prototype, setPrototype, DontEnum | DontDelete | ReadOnly);
     putDirectWithoutTransition(vm, vm.propertyNames->length, jsNumber(0), ReadOnly | DontEnum | DontDelete);
-    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum | DontDelete);
+    putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, Accessor | ReadOnly | DontEnum);
 }
 

trunk/Source/JavaScriptCore/tests/stress/symbol-species.js

@@ -7 +7 @@
     if (constructor[Symbol.species] !== constructor)
         throw "Symbol.species was mutable " + constructor.name;
-    try {
-        Object.defineProperty(constructor, Symbol.species, { value: true });
-    } catch(e) {
-        return;
-    }
-    throw "Symbol.species was configurable " + constructor.name;
+
+    // Symbol.species should be configurable.
+    Object.defineProperty(constructor, Symbol.species, { value: true });
 }