Added: trunk/JSTests/stress/caller-and-arguments-properties-for-functions-that-dont-have-them.js (0 => 225845)
--- trunk/JSTests/stress/caller-and-arguments-properties-for-functions-that-dont-have-them.js (rev 0)
+++ trunk/JSTests/stress/caller-and-arguments-properties-for-functions-that-dont-have-them.js 2017-12-13 17:29:21 UTC (rev 225845)
@@ -0,0 +1,74 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+function test1(f) {
+ f.__proto__ = {};
+ Object.defineProperty(f, "caller", {value:42});
+ assert(f.caller === 42);
+ Object.defineProperty(f, "arguments", {value:32});
+ assert(f.arguments === 32);
+}
+for (let i = 0; i < 1000; ++i) {
+ test1(function () { "use strict"; });
+ test1(class C { });
+ test1(() => undefined);
+ test1(async function foo(){});
+ test1(function* foo() { });
+}
+
+function test2(f, p = {}) {
+ f.__proto__ = p;
+ f.caller = 42;
+ assert(f.caller === 42);
+ f.arguments = 44;
+ assert(f.arguments === 44);
+}
+
+{
+ let proxy = new Proxy({}, {
+ has(...args) {
+ throw new Error("Should not be called!");
+ }
+ });
+ for (let i = 0; i < 1000; ++i) {
+ test2(function () { "use strict"; }, proxy);
+ test2(class C { }, proxy);
+ test2(() => undefined, proxy);
+ test2(async function foo(){}, proxy);
+ test2(function* foo() { }, proxy);
+ }
+}
+
+for (let i = 0; i < 1000; ++i) {
+ test2(function () { "use strict"; });
+ test2(class C { });
+ test2(() => undefined);
+ test2(async function foo(){});
+ test2(function* foo() { });
+}
+
+function test3(f) {
+ f.__proto__ = {};
+ f.caller = 42;
+ assert(f.caller === 42);
+ assert(f.hasOwnProperty("caller"));
+ assert(delete f.caller === true);
+ assert(f.caller === undefined);
+ assert(!f.hasOwnProperty("caller"));
+
+ f.arguments = 44;
+ assert(f.arguments === 44);
+ assert(f.hasOwnProperty("arguments"));
+ assert(delete f.arguments === true);
+ assert(f.arguments === undefined);
+ assert(!f.hasOwnProperty("arguments"));
+}
+for (let i = 0; i < 1000; ++i) {
+ test3(function () { "use strict"; });
+ test3(class C { });
+ test3(() => undefined);
+ test3(async function foo(){});
+ test3(function* foo() { });
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (225844 => 225845)
--- trunk/Source/_javascript_Core/ChangeLog 2017-12-13 17:19:24 UTC (rev 225844)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-12-13 17:29:21 UTC (rev 225845)
@@ -1,5 +1,25 @@
2017-12-13 Saam Barati <[email protected]>
+ 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 <[email protected]>
+
TypeCheckHoistingPhase needs to emit a CheckStructureOrEmpty if it's doing it for |this|
https://bugs.webkit.org/show_bug.cgi?id=180734
<rdar://problem/35640547>
Modified: trunk/Source/_javascript_Core/runtime/JSFunction.cpp (225844 => 225845)
--- trunk/Source/_javascript_Core/runtime/JSFunction.cpp 2017-12-13 17:19:24 UTC (rev 225844)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.cpp 2017-12-13 17:29:21 UTC (rev 225845)
@@ -471,17 +471,11 @@
}
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));
}
PropertyStatus propertyType = thisObject->reifyLazyPropertyIfNeeded(vm, exec, propertyName);
@@ -501,8 +495,8 @@
// For non-host functions, don't let these properties by deleted - except by DefineOwnProperty.
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())
return false;
@@ -539,14 +533,6 @@
bool valueCheck;
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);
}
@@ -553,14 +539,6 @@
valueCheck = !descriptor.value() || sameValue(exec, descriptor.value(), retrieveArguments(exec, thisObject));
} 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);
}
