Changeset 225845 in webkit

Timestamp:

Dec 13, 2017 9:29:21 AM (6 years ago)

Author:

sbarati@apple.com

Message:

Fix how JSFunction handles "caller" and "arguments" for functions that don't have those properties
​https://bugs.webkit.org/show_bug.cgi?id=163579
<rdar://problem/35455798>

Reviewed by Mark Lam.

JSTests:

• stress/caller-and-arguments-properties-for-functions-that-dont-have-them.js: Added.

(assert):
(test1):
(i.test1):
(i.test1.C):
(i.test1.async.foo):
(i.test1.foo):
(test2):

Source/JavaScriptCore:

Some functions in JavaScript do not have the "caller" and "arguments" properties.
For example, strict functions do not. When reading our code that dealt with these
types of functions, it was simply all wrong. We were doing weird things depending
on the method table hook. This patch fixes this by doing what we should've been
doing all along: when the JSFunction does not own the "caller"/"arguments" property,
it should defer to its base class implementation for the various method table hooks.

• runtime/JSFunction.cpp:

(JSC::JSFunction::put):
(JSC::JSFunction::deleteProperty):
(JSC::JSFunction::defineOwnProperty):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/caller-and-arguments-properties-for-functions-that-dont-have-them.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSFunction.cpp (modified) (4 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2017-12-13  Saam Barati  <sbarati@apple.com>
+
+        Fix how JSFunction handles "caller" and "arguments" for functions that don't have those properties
+        https://bugs.webkit.org/show_bug.cgi?id=163579
+        <rdar://problem/35455798>
+
+        Reviewed by Mark Lam.
+
+        * stress/caller-and-arguments-properties-for-functions-that-dont-have-them.js: Added.
+        (assert):
+        (test1):
+        (i.test1):
+        (i.test1.C):
+        (i.test1.async.foo):
+        (i.test1.foo):
+        (test2):
+
 2017-12-13  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2017-12-13  Saam Barati  <sbarati@apple.com>
+
+        Fix how JSFunction handles "caller" and "arguments" for functions that don't have those properties
+        https://bugs.webkit.org/show_bug.cgi?id=163579
+        <rdar://problem/35455798>
+
+        Reviewed by Mark Lam.
+
+        Some functions in JavaScript do not have the "caller" and "arguments" properties.
+        For example, strict functions do not. When reading our code that dealt with these
+        types of functions, it was simply all wrong. We were doing weird things depending
+        on the method table hook. This patch fixes this by doing what we should've been
+        doing all along: when the JSFunction does not own the "caller"/"arguments" property,
+        it should defer to its base class implementation for the various method table hooks.
+
+        * runtime/JSFunction.cpp:
+        (JSC::JSFunction::put):
+        (JSC::JSFunction::deleteProperty):
+        (JSC::JSFunction::defineOwnProperty):
+
 2017-12-13  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSFunction.cpp

@@ -472 +472 @@
 
     if (propertyName == vm.propertyNames->arguments || propertyName == vm.propertyNames->caller) {
-        slot.disableCaching();
         if (!thisObject->jsExecutable()->hasCallerAndArgumentsProperties()) {
-            // This will trigger the property to be reified, if this is not already the case!
-            // FIXME: Investigate if the `hasProperty()` call is even needed, as in the `!hasCallerAndArgumentsProperties()` case,
-            // these properties are not lazy and should not need to be reified. (https://bugs.webkit.org/show_bug.cgi?id=163579)
-            bool okay = thisObject->hasProperty(exec, propertyName);
-            RETURN_IF_EXCEPTION(scope, false);
-            ASSERT_UNUSED(okay, okay);
             scope.release();
             return Base::put(thisObject, exec, propertyName, value, slot);
         }
+        slot.disableCaching();
         return typeError(exec, scope, slot.isStrictMode(), ASCIILiteral(ReadonlyPropertyWriteError));
     }
@@ -502 +496 @@
         FunctionExecutable* executable = thisObject->jsExecutable();
         
-        if (propertyName == vm.propertyNames->caller || propertyName == vm.propertyNames->arguments)
-            return !executable->hasCallerAndArgumentsProperties();
+        if ((propertyName == vm.propertyNames->caller || propertyName == vm.propertyNames->arguments) && executable->hasCallerAndArgumentsProperties())
+            return false;
 
         if (propertyName == vm.propertyNames->prototype && !executable->isArrowFunction())
@@ -540 +534 @@
     if (propertyName == vm.propertyNames->arguments) {
         if (!thisObject->jsExecutable()->hasCallerAndArgumentsProperties()) {
-            if (thisObject->jsExecutable()->isClass()) {
-                thisObject->reifyLazyPropertyIfNeeded(vm, exec, propertyName);
-                scope.release();
-                return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);
-            }
-            PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
-            if (!Base::getOwnPropertySlot(thisObject, exec, propertyName, slot))
-                thisObject->putDirectAccessor(exec, propertyName, thisObject->globalObject(vm)->throwTypeErrorArgumentsCalleeAndCallerGetterSetter(), PropertyAttribute::DontDelete | PropertyAttribute::DontEnum | PropertyAttribute::Accessor);
             scope.release();
             return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);
@@ -554 +540 @@
     } else if (propertyName == vm.propertyNames->caller) {
         if (!thisObject->jsExecutable()->hasCallerAndArgumentsProperties()) {
-            if (thisObject->jsExecutable()->isClass()) {
-                thisObject->reifyLazyPropertyIfNeeded(vm, exec, propertyName);
-                scope.release();
-                return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);
-            }
-            PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
-            if (!Base::getOwnPropertySlot(thisObject, exec, propertyName, slot))
-                thisObject->putDirectAccessor(exec, propertyName, thisObject->globalObject(vm)->throwTypeErrorArgumentsCalleeAndCallerGetterSetter(), PropertyAttribute::DontDelete | PropertyAttribute::DontEnum | PropertyAttribute::Accessor);
             scope.release();
             return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);