Promise resolve and reject function should have length = 1
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 6 Aug 2017 17:57:26 +0000 (17:57 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 6 Aug 2017 17:57:26 +0000 (17:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175242

Reviewed by Saam Barati.

JSTests:

* stress/builtin-function-length.js: Added.
(shouldBe):
(shouldThrow):
(shouldBe.JSON.stringify.Object.getOwnPropertyDescriptor):
(shouldBe.JSON.stringify.Object.getOwnPropertyNames.Array.prototype.filter.sort):

Source/JavaScriptCore:

Previously we have separate system for "length" and "name" for builtin functions.
The builtin functions do not use lazy reifying system. Instead, they have direct
properties when instantiating it. While the function created for properties (like
Array.prototype.filter) is created by JSFunction::createBuiltin(), function inside
these builtin functions are just created by JSFunction::create(). Since it does
not set any values for "length", these functions do not have "length" property.
So, the resolve and reject functions passed to Promise's executor do not have
"length" property.

This patch make builtin functions use standard lazy reifying system for "length".
So, "length" property of the builtin function just works as if the normal functions
do.

* runtime/JSFunction.cpp:
(JSC::JSFunction::createBuiltinFunction):
(JSC::JSFunction::getOwnPropertySlot):
(JSC::JSFunction::getOwnNonIndexPropertyNames):
(JSC::JSFunction::put):
(JSC::JSFunction::deleteProperty):
(JSC::JSFunction::defineOwnProperty):
(JSC::JSFunction::reifyLazyPropertyIfNeeded):
(JSC::JSFunction::reifyLazyPropertyForHostOrBuiltinIfNeeded):
(JSC::JSFunction::reifyLazyLengthIfNeeded):
(JSC::JSFunction::reifyLazyBoundNameIfNeeded):
(JSC::JSFunction::reifyBoundNameIfNeeded): Deleted.
* runtime/JSFunction.h:

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

JSTests/ChangeLog
JSTests/stress/builtin-function-length.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSFunction.cpp
Source/JavaScriptCore/runtime/JSFunction.h

index 3ed359ed3b6304dfdbca0d4029b8f80dd9537083..ee32d139756f8f06414ba2b39aa77fc35731c29a 100644 (file)
@@ -1,3 +1,16 @@
+2017-08-06  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Promise resolve and reject function should have length = 1
+        https://bugs.webkit.org/show_bug.cgi?id=175242
+
+        Reviewed by Saam Barati.
+
+        * stress/builtin-function-length.js: Added.
+        (shouldBe):
+        (shouldThrow):
+        (shouldBe.JSON.stringify.Object.getOwnPropertyDescriptor):
+        (shouldBe.JSON.stringify.Object.getOwnPropertyNames.Array.prototype.filter.sort):
+
 2017-08-06  Oleksandr Skachkov  <gskachkov@gmail.com>
 
         [ESNext] Async iteration - Implement Async Generator - parser
diff --git a/JSTests/stress/builtin-function-length.js b/JSTests/stress/builtin-function-length.js
new file mode 100644 (file)
index 0000000..f2ea008
--- /dev/null
@@ -0,0 +1,47 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function shouldThrow(func, errorMessage) {
+    var errorThrown = false;
+    var error = null;
+    try {
+        func();
+    } catch (e) {
+        errorThrown = true;
+        error = e;
+    }
+    if (!errorThrown)
+        throw new Error('not thrown');
+    if (String(error) !== errorMessage)
+        throw new Error(`bad error: ${String(error)}`);
+}
+
+{
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(Array.prototype.filter).sort()), `["length","name"]`);
+    shouldBe(Array.prototype.filter.length, 1);
+    shouldBe(JSON.stringify(Object.getOwnPropertyDescriptor(Array.prototype.filter, 'length')), `{"value":1,"writable":false,"enumerable":false,"configurable":true}`);
+    shouldBe(delete Array.prototype.filter.length, true);
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(Array.prototype.filter).sort()), `["name"]`);
+}
+
+{
+    shouldThrow(function () {
+        "use strict";
+        Array.prototype.forEach.length = 42;
+    }, `TypeError: Attempted to assign to readonly property.`);
+}
+
+{
+    var resolve = null;
+    var reject = null;
+    new Promise(function (arg0, arg1) {
+        resolve = arg0;
+        reject = arg1;
+    });
+    shouldBe(resolve.length, 1);
+    shouldBe(JSON.stringify(Object.getOwnPropertyDescriptor(resolve, 'length')), `{"value":1,"writable":false,"enumerable":false,"configurable":true}`);
+    shouldBe(reject.length, 1);
+    shouldBe(JSON.stringify(Object.getOwnPropertyDescriptor(reject, 'length')), `{"value":1,"writable":false,"enumerable":false,"configurable":true}`);
+}
index 2ca8acef7f89f429893397fd6414bc05d815a37a..0e23ef2ec7c009c1c8c732482d2f9294211b7ffb 100644 (file)
@@ -1,3 +1,37 @@
+2017-08-06  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Promise resolve and reject function should have length = 1
+        https://bugs.webkit.org/show_bug.cgi?id=175242
+
+        Reviewed by Saam Barati.
+
+        Previously we have separate system for "length" and "name" for builtin functions.
+        The builtin functions do not use lazy reifying system. Instead, they have direct
+        properties when instantiating it. While the function created for properties (like
+        Array.prototype.filter) is created by JSFunction::createBuiltin(), function inside
+        these builtin functions are just created by JSFunction::create(). Since it does
+        not set any values for "length", these functions do not have "length" property.
+        So, the resolve and reject functions passed to Promise's executor do not have
+        "length" property.
+
+        This patch make builtin functions use standard lazy reifying system for "length".
+        So, "length" property of the builtin function just works as if the normal functions
+        do.
+
+        * runtime/JSFunction.cpp:
+        (JSC::JSFunction::createBuiltinFunction):
+        (JSC::JSFunction::getOwnPropertySlot):
+        (JSC::JSFunction::getOwnNonIndexPropertyNames):
+        (JSC::JSFunction::put):
+        (JSC::JSFunction::deleteProperty):
+        (JSC::JSFunction::defineOwnProperty):
+        (JSC::JSFunction::reifyLazyPropertyIfNeeded):
+        (JSC::JSFunction::reifyLazyPropertyForHostOrBuiltinIfNeeded):
+        (JSC::JSFunction::reifyLazyLengthIfNeeded):
+        (JSC::JSFunction::reifyLazyBoundNameIfNeeded):
+        (JSC::JSFunction::reifyBoundNameIfNeeded): Deleted.
+        * runtime/JSFunction.h:
+
 2017-08-06  Oleksandr Skachkov  <gskachkov@gmail.com>
 
         [ESNext] Async iteration - Implement Async Generator - parser
index 4e226c3b77be2ce88f1a59522c78036a0528b112..dfc9375429f4d2b85608af222db3dc8ac2662b5e 100644 (file)
@@ -105,7 +105,6 @@ JSFunction* JSFunction::createBuiltinFunction(VM& vm, FunctionExecutable* execut
 {
     JSFunction* function = create(vm, executable, globalObject);
     function->putDirect(vm, vm.propertyNames->name, jsString(&vm, executable->name().string()), ReadOnly | DontEnum);
-    function->putDirect(vm, vm.propertyNames->length, jsNumber(executable->parameterCount()), ReadOnly | DontEnum);
     return function;
 }
 
@@ -113,7 +112,6 @@ JSFunction* JSFunction::createBuiltinFunction(VM& vm, FunctionExecutable* execut
 {
     JSFunction* function = create(vm, executable, globalObject);
     function->putDirect(vm, vm.propertyNames->name, jsString(&vm, name), ReadOnly | DontEnum);
-    function->putDirect(vm, vm.propertyNames->length, jsNumber(executable->parameterCount()), ReadOnly | DontEnum);
     return function;
 }
 
@@ -350,7 +348,7 @@ bool JSFunction::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyN
     VM& vm = exec->vm();
     JSFunction* thisObject = jsCast<JSFunction*>(object);
     if (thisObject->isHostOrBuiltinFunction()) {
-        thisObject->reifyBoundNameIfNeeded(vm, exec, propertyName);
+        thisObject->reifyLazyPropertyForHostOrBuiltinIfNeeded(vm, exec, propertyName);
         return Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
     }
 
@@ -402,21 +400,27 @@ void JSFunction::getOwnNonIndexPropertyNames(JSObject* object, ExecState* exec,
 {
     JSFunction* thisObject = jsCast<JSFunction*>(object);
     VM& vm = exec->vm();
-    if (!thisObject->isHostOrBuiltinFunction() && mode.includeDontEnumProperties()) {
-        // Make sure prototype has been reified.
-        PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
-        thisObject->methodTable(vm)->getOwnPropertySlot(thisObject, exec, vm.propertyNames->prototype, slot);
+    if (mode.includeDontEnumProperties()) {
+        if (!thisObject->isHostOrBuiltinFunction()) {
+            // Make sure prototype has been reified.
+            PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
+            thisObject->methodTable(vm)->getOwnPropertySlot(thisObject, exec, vm.propertyNames->prototype, slot);
 
-        if (thisObject->jsExecutable()->hasCallerAndArgumentsProperties()) {
-            propertyNames.add(vm.propertyNames->arguments);
-            propertyNames.add(vm.propertyNames->caller);
+            if (thisObject->jsExecutable()->hasCallerAndArgumentsProperties()) {
+                propertyNames.add(vm.propertyNames->arguments);
+                propertyNames.add(vm.propertyNames->caller);
+            }
+            if (!thisObject->hasReifiedLength())
+                propertyNames.add(vm.propertyNames->length);
+            if (!thisObject->hasReifiedName())
+                propertyNames.add(vm.propertyNames->name);
+        } else {
+            if (thisObject->isBuiltinFunction() && !thisObject->hasReifiedLength())
+                propertyNames.add(vm.propertyNames->length);
+            if (thisObject->inherits(vm, JSBoundFunction::info()) && !thisObject->hasReifiedName())
+                propertyNames.add(vm.propertyNames->name);
         }
-        if (!thisObject->hasReifiedLength())
-            propertyNames.add(vm.propertyNames->length);
-        if (!thisObject->hasReifiedName())
-            propertyNames.add(vm.propertyNames->name);
-    } else if (thisObject->isHostOrBuiltinFunction() && mode.includeDontEnumProperties() && thisObject->inherits(vm, JSBoundFunction::info()) && !thisObject->hasReifiedName())
-        propertyNames.add(exec->vm().propertyNames->name);
+    }
     Base::getOwnNonIndexPropertyNames(thisObject, exec, propertyNames, mode);
 }
 
@@ -433,7 +437,7 @@ bool JSFunction::put(JSCell* cell, ExecState* exec, PropertyName propertyName, J
     }
 
     if (thisObject->isHostOrBuiltinFunction()) {
-        LazyPropertyType propType = thisObject->reifyBoundNameIfNeeded(vm, exec, propertyName);
+        LazyPropertyType propType = thisObject->reifyLazyPropertyForHostOrBuiltinIfNeeded(vm, exec, propertyName);
         if (propType == LazyPropertyType::IsLazyProperty)
             slot.disableCaching();
         scope.release();
@@ -475,12 +479,12 @@ bool JSFunction::put(JSCell* cell, ExecState* exec, PropertyName propertyName, J
 
 bool JSFunction::deleteProperty(JSCell* cell, ExecState* exec, PropertyName propertyName)
 {
+    VM& vm = exec->vm();
     JSFunction* thisObject = jsCast<JSFunction*>(cell);
     if (thisObject->isHostOrBuiltinFunction())
-        thisObject->reifyBoundNameIfNeeded(exec->vm(), exec, propertyName);
-    else if (exec->vm().deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable) {
+        thisObject->reifyLazyPropertyForHostOrBuiltinIfNeeded(vm, exec, propertyName);
+    else if (vm.deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable) {
         // For non-host functions, don't let these properties by deleted - except by DefineOwnProperty.
-        VM& vm = exec->vm();
         FunctionExecutable* executable = thisObject->jsExecutable();
         
         if (propertyName == exec->propertyNames().caller || propertyName == exec->propertyNames().arguments)
@@ -502,7 +506,7 @@ bool JSFunction::defineOwnProperty(JSObject* object, ExecState* exec, PropertyNa
 
     JSFunction* thisObject = jsCast<JSFunction*>(object);
     if (thisObject->isHostOrBuiltinFunction()) {
-        thisObject->reifyBoundNameIfNeeded(vm, exec, propertyName);
+        thisObject->reifyLazyPropertyForHostOrBuiltinIfNeeded(vm, exec, propertyName);
         scope.release();
         return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);
     }
@@ -680,11 +684,9 @@ void JSFunction::reifyName(VM& vm, ExecState* exec, String name)
 
 JSFunction::LazyPropertyType JSFunction::reifyLazyPropertyIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
 {
-    if (propertyName == vm.propertyNames->length) {
-        if (!hasReifiedLength())
-            reifyLength(vm);
+    if (reifyLazyLengthIfNeeded(vm, exec, propertyName) == LazyPropertyType::IsLazyProperty)
         return LazyPropertyType::IsLazyProperty;
-    } else if (propertyName == vm.propertyNames->name) {
+    if (propertyName == vm.propertyNames->name) {
         if (!hasReifiedName())
             reifyName(vm, exec);
         return LazyPropertyType::IsLazyProperty;
@@ -692,7 +694,27 @@ JSFunction::LazyPropertyType JSFunction::reifyLazyPropertyIfNeeded(VM& vm, ExecS
     return LazyPropertyType::NotLazyProperty;
 }
 
-JSFunction::LazyPropertyType JSFunction::reifyBoundNameIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
+JSFunction::LazyPropertyType JSFunction::reifyLazyPropertyForHostOrBuiltinIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
+{
+    ASSERT(isHostOrBuiltinFunction());
+    if (isBuiltinFunction()) {
+        if (reifyLazyLengthIfNeeded(vm, exec, propertyName) == LazyPropertyType::IsLazyProperty)
+            return LazyPropertyType::IsLazyProperty;
+    }
+    return reifyLazyBoundNameIfNeeded(vm, exec, propertyName);
+}
+
+JSFunction::LazyPropertyType JSFunction::reifyLazyLengthIfNeeded(VM& vm, ExecState*, PropertyName propertyName)
+{
+    if (propertyName == vm.propertyNames->length) {
+        if (!hasReifiedLength())
+            reifyLength(vm);
+        return LazyPropertyType::IsLazyProperty;
+    }
+    return LazyPropertyType::NotLazyProperty;
+}
+
+JSFunction::LazyPropertyType JSFunction::reifyLazyBoundNameIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
 {
     const Identifier& nameIdent = vm.propertyNames->name;
     if (propertyName != nameIdent)
index 7dc9b6acabeec7776b49b2369dd27a51f86e1c7b..8c85730dce54b421115ac01855991d1f37d78b65 100644 (file)
@@ -190,7 +190,9 @@ private:
 
     enum class LazyPropertyType { NotLazyProperty, IsLazyProperty };
     LazyPropertyType reifyLazyPropertyIfNeeded(VM&, ExecState*, PropertyName);
-    LazyPropertyType reifyBoundNameIfNeeded(VM&, ExecState*, PropertyName);
+    LazyPropertyType reifyLazyPropertyForHostOrBuiltinIfNeeded(VM&, ExecState*, PropertyName);
+    LazyPropertyType reifyLazyLengthIfNeeded(VM&, ExecState*, PropertyName);
+    LazyPropertyType reifyLazyBoundNameIfNeeded(VM&, ExecState*, PropertyName);
 
     friend class LLIntOffsetsExtractor;