Title: [228842] branches/safari-605-branch

Diff

Modified: branches/safari-605-branch/JSTests/ChangeLog (228841 => 228842)


--- branches/safari-605-branch/JSTests/ChangeLog	2018-02-20 22:30:13 UTC (rev 228841)
+++ branches/safari-605-branch/JSTests/ChangeLog	2018-02-20 22:30:16 UTC (rev 228842)
@@ -1,5 +1,19 @@
 2018-02-20  Jason Marcell  <[email protected]>
 
+        Cherry-pick r228725. rdar://problem/37714027
+
+    2018-02-19  Saam Barati  <[email protected]>
+
+            Don't use JSFunction's allocation profile when getting the prototype can be effectful
+            https://bugs.webkit.org/show_bug.cgi?id=182942
+            <rdar://problem/37584764>
+
+            Reviewed by Mark Lam.
+
+            * stress/get-prototype-create-this-effectful.js: Added.
+
+2018-02-20  Jason Marcell  <[email protected]>
+
         Cherry-pick r228565. rdar://problem/37697682
 
     2018-02-16  Saam Barati  <[email protected]>

Added: branches/safari-605-branch/JSTests/stress/get-prototype-create-this-effectful.js (0 => 228842)


--- branches/safari-605-branch/JSTests/stress/get-prototype-create-this-effectful.js	                        (rev 0)
+++ branches/safari-605-branch/JSTests/stress/get-prototype-create-this-effectful.js	2018-02-20 22:30:16 UTC (rev 228842)
@@ -0,0 +1,40 @@
+function assert(b) {
+    if (!b)
+        throw new Error("Bad assertion")
+}
+
+function test1() {
+    let boundFunction = function () {}.bind();
+    Object.defineProperty(boundFunction, "prototype", {
+        get() {
+            throw Error("Hello");
+        }
+    });
+
+    let threw = false;
+    try {
+        Reflect.construct(function() {}, [], boundFunction);
+    } catch(e) {
+        threw = true;
+        assert(e.message === "Hello");
+    }
+    assert(threw);
+}
+test1();
+
+function test2() {
+    let boundFunction = function () {}.bind();
+    let counter = 0;
+    Object.defineProperty(boundFunction, "prototype", {
+        get() {
+            ++counter;
+            return {};
+        }
+    });
+
+    const iters = 1000;
+    for (let i = 0; i < iters; ++i)
+        Reflect.construct(function() {}, [], boundFunction);
+    assert(counter === iters);
+}
+test2();

Modified: branches/safari-605-branch/Source/_javascript_Core/ChangeLog (228841 => 228842)


--- branches/safari-605-branch/Source/_javascript_Core/ChangeLog	2018-02-20 22:30:13 UTC (rev 228841)
+++ branches/safari-605-branch/Source/_javascript_Core/ChangeLog	2018-02-20 22:30:16 UTC (rev 228842)
@@ -1,5 +1,48 @@
 2018-02-20  Jason Marcell  <[email protected]>
 
+        Cherry-pick r228725. rdar://problem/37714027
+
+    2018-02-19  Saam Barati  <[email protected]>
+
+            Don't use JSFunction's allocation profile when getting the prototype can be effectful
+            https://bugs.webkit.org/show_bug.cgi?id=182942
+            <rdar://problem/37584764>
+
+            Reviewed by Mark Lam.
+
+            Prior to this patch, the create_this implementation assumed that anything
+            that is a JSFunction can use the object allocation profile and go down the
+            fast path to allocate the |this| object. Implied by this approach is that
+            accessing the 'prototype' property of the incoming function is not an
+            effectful operation. This is inherent to the ObjectAllocationProfile
+            data structure: it caches the prototype field. However, getting the
+            'prototype' property might be an effectful operation, e.g, it could
+            be a getter. Many variants of functions in JS have the 'prototype' property
+            as non-configurable. However, some functions, like bound functions, do not
+            have the 'prototype' field with these attributes.
+
+            This patch adds the notion of 'canUseAllocationProfile' to JSFunction
+            and threads it through so that we only go down the fast path and use
+            the allocation profile when the prototype property is non-configurable.
+
+            * bytecompiler/NodesCodegen.cpp:
+            (JSC::ClassExprNode::emitBytecode):
+            * dfg/DFGOperations.cpp:
+            * runtime/CommonSlowPaths.cpp:
+            (JSC::SLOW_PATH_DECL):
+            * runtime/JSFunction.cpp:
+            (JSC::JSFunction::prototypeForConstruction):
+            (JSC::JSFunction::allocateAndInitializeRareData):
+            (JSC::JSFunction::initializeRareData):
+            (JSC::JSFunction::getOwnPropertySlot):
+            (JSC::JSFunction::canUseAllocationProfileNonInline):
+            * runtime/JSFunction.h:
+            (JSC::JSFunction::ensureRareDataAndAllocationProfile):
+            * runtime/JSFunctionInlines.h:
+            (JSC::JSFunction::canUseAllocationProfile):
+
+2018-02-20  Jason Marcell  <[email protected]>
+
         Cherry-pick r228720. rdar://problem/37714022
 
     2018-02-19  Saam Barati  <[email protected]>

Modified: branches/safari-605-branch/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (228841 => 228842)


--- branches/safari-605-branch/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2018-02-20 22:30:13 UTC (rev 228841)
+++ branches/safari-605-branch/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2018-02-20 22:30:16 UTC (rev 228842)
@@ -3897,7 +3897,6 @@
     RefPtr<RegisterID> constructor;
     bool needsHomeObject = false;
 
-    // FIXME: Make the prototype non-configurable & non-writable.
     if (m_constructorExpression) {
         ASSERT(m_constructorExpression->isFuncExprNode());
         FunctionMetadataNode* metadata = static_cast<FuncExprNode*>(m_constructorExpression)->metadata();

Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGOperations.cpp (228841 => 228842)


--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGOperations.cpp	2018-02-20 22:30:13 UTC (rev 228841)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGOperations.cpp	2018-02-20 22:30:16 UTC (rev 228842)
@@ -242,8 +242,8 @@
     VM& vm = exec->vm();
     NativeCallFrameTracer tracer(&vm, exec);
     auto scope = DECLARE_THROW_SCOPE(vm);
-    if (constructor->type() == JSFunctionType) {
-        auto rareData = jsCast<JSFunction*>(constructor)->rareData(exec, inlineCapacity);
+    if (constructor->type() == JSFunctionType && jsCast<JSFunction*>(constructor)->canUseAllocationProfile()) {
+        auto rareData = jsCast<JSFunction*>(constructor)->ensureRareDataAndAllocationProfile(exec, inlineCapacity);
         RETURN_IF_EXCEPTION(scope, nullptr);
         Structure* structure = rareData->objectAllocationProfile()->structure();
         JSObject* result = constructEmptyObject(exec, structure);

Modified: branches/safari-605-branch/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (228841 => 228842)


--- branches/safari-605-branch/Source/_javascript_Core/runtime/CommonSlowPaths.cpp	2018-02-20 22:30:13 UTC (rev 228841)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/CommonSlowPaths.cpp	2018-02-20 22:30:16 UTC (rev 228842)
@@ -222,7 +222,7 @@
     auto& bytecode = *reinterpret_cast<OpCreateThis*>(pc);
     JSObject* result;
     JSObject* constructorAsObject = asObject(GET(bytecode.callee()).jsValue());
-    if (constructorAsObject->type() == JSFunctionType) {
+    if (constructorAsObject->type() == JSFunctionType && jsCast<JSFunction*>(constructorAsObject)->canUseAllocationProfile()) {
         JSFunction* constructor = jsCast<JSFunction*>(constructorAsObject);
         WriteBarrier<JSCell>& cachedCallee = bytecode.cachedCallee();
         if (!cachedCallee)
@@ -231,7 +231,7 @@
             cachedCallee.setWithoutWriteBarrier(JSCell::seenMultipleCalleeObjects());
 
         size_t inlineCapacity = bytecode.inlineCapacity();
-        Structure* structure = constructor->rareData(exec, inlineCapacity)->objectAllocationProfile()->structure();
+        Structure* structure = constructor->ensureRareDataAndAllocationProfile(exec, inlineCapacity)->objectAllocationProfile()->structure();
         result = constructEmptyObject(exec, structure);
         if (structure->hasPolyProto()) {
             JSObject* prototype = constructor->prototypeForConstruction(vm, exec);

Modified: branches/safari-605-branch/Source/_javascript_Core/runtime/JSFunction.cpp (228841 => 228842)


--- branches/safari-605-branch/Source/_javascript_Core/runtime/JSFunction.cpp	2018-02-20 22:30:13 UTC (rev 228841)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/JSFunction.cpp	2018-02-20 22:30:16 UTC (rev 228842)
@@ -139,9 +139,12 @@
 
 JSObject* JSFunction::prototypeForConstruction(VM& vm, ExecState* exec)
 {
+    // This code assumes getting the prototype is not effectful. That's only
+    // true when we can use the allocation profile.
+    ASSERT(canUseAllocationProfile()); 
     auto scope = DECLARE_CATCH_SCOPE(vm);
     JSValue prototype = get(exec, vm.propertyNames->prototype);
-    ASSERT_UNUSED(scope, !scope.exception());
+    scope.releaseAssertNoException();
     if (prototype.isObject())
         return asObject(prototype);
 
@@ -151,6 +154,7 @@
 FunctionRareData* JSFunction::allocateAndInitializeRareData(ExecState* exec, size_t inlineCapacity)
 {
     ASSERT(!m_rareData);
+    ASSERT(canUseAllocationProfile());
     VM& vm = exec->vm();
     JSObject* prototype = prototypeForConstruction(vm, exec);
     FunctionRareData* rareData = FunctionRareData::create(vm);
@@ -167,6 +171,7 @@
 FunctionRareData* JSFunction::initializeRareData(ExecState* exec, size_t inlineCapacity)
 {
     ASSERT(!!m_rareData);
+    ASSERT(canUseAllocationProfile());
     VM& vm = exec->vm();
     JSObject* prototype = prototypeForConstruction(vm, exec);
     m_rareData->initializeObjectAllocationProfile(vm, globalObject(vm), prototype, inlineCapacity, this);
@@ -374,6 +379,13 @@
     }
 
     if (propertyName == vm.propertyNames->prototype && thisObject->jsExecutable()->hasPrototypeProperty() && !thisObject->jsExecutable()->isClassConstructorFunction()) {
+        // NOTE: class constructors define the prototype property in bytecode using
+        // defineOwnProperty, which ends up calling into this code (see our defineOwnProperty
+        // implementation below). The bytecode will end up doing the proper definition
+        // with the property being non-writable/non-configurable. However, we must ignore
+        // the initial materialization of the property so that the defineOwnProperty call
+        // from bytecode succeeds. Otherwise, the materialization here would prevent the
+        // defineOwnProperty from being able to overwrite the property.
         unsigned attributes;
         PropertyOffset offset = thisObject->getDirectOffset(vm, propertyName, attributes);
         if (!isValidOffset(offset)) {

Modified: branches/safari-605-branch/Source/_javascript_Core/runtime/JSFunction.h (228841 => 228842)


--- branches/safari-605-branch/Source/_javascript_Core/runtime/JSFunction.h	2018-02-20 22:30:13 UTC (rev 228841)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/JSFunction.h	2018-02-20 22:30:16 UTC (rev 228842)
@@ -130,14 +130,7 @@
         return m_rareData.get();
     }
 
-    FunctionRareData* rareData(ExecState* exec, unsigned inlineCapacity)
-    {
-        if (UNLIKELY(!m_rareData))
-            return allocateAndInitializeRareData(exec, inlineCapacity);
-        if (UNLIKELY(!m_rareData->isObjectAllocationProfileInitialized()))
-            return initializeRareData(exec, inlineCapacity);
-        return m_rareData.get();
-    }
+    FunctionRareData* ensureRareDataAndAllocationProfile(ExecState*, unsigned inlineCapacity);
 
     FunctionRareData* rareData()
     {
@@ -160,6 +153,9 @@
     // Returns the __proto__ for the |this| value if this JSFunction were to be constructed.
     JSObject* prototypeForConstruction(VM&, ExecState*);
 
+    bool canUseAllocationProfile();
+    bool canUseAllocationProfileNonInline();
+
 protected:
     JS_EXPORT_PRIVATE JSFunction(VM&, JSGlobalObject*, Structure*);
     JSFunction(VM&, FunctionExecutable*, JSScope*, Structure*);
@@ -167,10 +163,6 @@
     void finishCreation(VM&, NativeExecutable*, int length, const String& name);
     void finishCreation(VM&);
 
-    FunctionRareData* allocateRareData(VM&);
-    FunctionRareData* allocateAndInitializeRareData(ExecState*, size_t inlineCapacity);
-    FunctionRareData* initializeRareData(ExecState*, size_t inlineCapacity);
-
     static bool getOwnPropertySlot(JSObject*, ExecState*, PropertyName, PropertySlot&);
     static void getOwnNonIndexPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode = EnumerationMode());
     static bool defineOwnProperty(JSObject*, ExecState*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
@@ -192,6 +184,10 @@
         return function;
     }
 
+    FunctionRareData* allocateRareData(VM&);
+    FunctionRareData* allocateAndInitializeRareData(ExecState*, size_t inlineCapacity);
+    FunctionRareData* initializeRareData(ExecState*, size_t inlineCapacity);
+
     bool hasReifiedLength() const;
     bool hasReifiedName() const;
     void reifyLength(VM&);

Modified: branches/safari-605-branch/Source/_javascript_Core/runtime/JSFunctionInlines.h (228841 => 228842)


--- branches/safari-605-branch/Source/_javascript_Core/runtime/JSFunctionInlines.h	2018-02-20 22:30:13 UTC (rev 228841)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/JSFunctionInlines.h	2018-02-20 22:30:16 UTC (rev 228842)
@@ -107,4 +107,26 @@
     return m_rareData ? m_rareData->hasReifiedName() : false;
 }
 
+inline bool JSFunction::canUseAllocationProfile()
+{
+    if (isHostFunction())
+        return false;
+
+    // If we don't have a prototype property, we're not guaranteed it's
+    // non-configurable. For example, user code can define the prototype
+    // as a getter. JS semantics require that the getter is called every
+    // time |construct| occurs with this function as new.target.
+    return jsExecutable()->hasPrototypeProperty();
+}
+
+inline FunctionRareData* JSFunction::ensureRareDataAndAllocationProfile(ExecState* exec, unsigned inlineCapacity)
+{
+    ASSERT(canUseAllocationProfile());
+    if (UNLIKELY(!m_rareData))
+        return allocateAndInitializeRareData(exec, inlineCapacity);
+    if (UNLIKELY(!m_rareData->isObjectAllocationProfileInitialized()))
+        return initializeRareData(exec, inlineCapacity);
+    return m_rareData.get();
+}
+
 } // namespace JSC
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to