Skip to content

Commit

Permalink
Remove revoked Proxy checks from ProxyCreate
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=210862

Reviewed by Ross Kirsling.

JSTests:

Removes expectations for 2 invalid ChakraCore tests.

* ChakraCore.yaml: Mark 2 test cases as passing.
* ChakraCore/test/es6/arraywithproxy.baseline: Removed.
* ChakraCore/test/es6/proxytest9.baseline: Removed.
* stress/proxy-revoke.js: Adjust test.
* test262/expectations.yaml: Mark 12 test cases as passing.

Source/JavaScriptCore:

This change removes revoked Proxy checks from ProxyCreate [1], implementing
tc39/ecma262#1814 and aligning JSC with SpiderMonkey.
Also cleans up ProxyObject creation by using isFunction() instead of
isCallable(), which are identical.

[1]: https://tc39.es/ecma262/#sec-proxycreate (steps 2, 4)

* runtime/ProxyObject.cpp:
(JSC::ProxyObject::structureForTarget):
(JSC::ProxyObject::finishCreation):


Canonical link: https://commits.webkit.org/223843@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@260621 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
shvaikalesh committed Apr 24, 2020
1 parent b76a4c7 commit 24aa49a
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 207 deletions.
12 changes: 4 additions & 8 deletions JSTests/ChakraCore.yaml
Expand Up @@ -1692,8 +1692,7 @@
- path: ChakraCore/test/es6/ReflectApiTests.js
cmd: runChakra :pass, "NoException", "", ["../UnitTestFramework/UnitTestFramework.js"]
- path: ChakraCore/test/es6/proxyTrapConsumeNewTarget.js
# Different behavior.
cmd: runChakra :skip, "NoException", "", ["../UnitTestFramework/UnitTestFramework.js"]
cmd: runChakra :pass, "NoException", "", ["../UnitTestFramework/UnitTestFramework.js"]
- path: ChakraCore/test/es6/CrossContextSpreadfunctionCall.js
# LoadScriptFile polyfill doesn't return an object.
cmd: runChakra :skip, "NoException", "", ["CrossContextSpreadFunction.js", "CrossContextSpreadArguments.js"]
Expand Down Expand Up @@ -1743,8 +1742,7 @@
- path: ChakraCore/test/es6/proxyenumbug.js
cmd: runChakra :skipDueToOutdatedOrBadTest, "NoException", "", []
- path: ChakraCore/test/es6/proxy-issue884.js
# Different behavior.
cmd: runChakra :skip, "NoException", "proxy-issue884.baseline", []
cmd: runChakra :pass, "NoException", "proxy-issue884.baseline", []
- path: ChakraCore/test/es6/nullPropertyDescriptor.js
cmd: runChakra :pass, "NoException", "", []
- path: ChakraCore/test/es6/object-is.js
Expand Down Expand Up @@ -1848,13 +1846,11 @@
- path: ChakraCore/test/es6/ProxySetPrototypeOf.js
cmd: runChakra :pass, "NoException", "", ["../UnitTestFramework/UnitTestFramework.js"]
- path: ChakraCore/test/es6/arraywithproxy.js
# Different behavior, cannot convert Symbol to string. Could be bad test.
cmd: runChakra :skip, "NoException", "arraywithproxy.baseline", []
cmd: runChakra :skipDueToOutdatedOrBadTest, "NoException", "", []
- path: ChakraCore/test/es6/proxytest8.js
cmd: runChakra :baseline, "NoException", "proxytest8.baseline", []
- path: ChakraCore/test/es6/proxytest9.js
# Different behavior.
cmd: runChakra :skip, "NoException", "proxytest9.baseline", []
cmd: runChakra :skipDueToOutdatedOrBadTest, "NoException", "", []
- path: ChakraCore/test/es6/ES6Function_bugs.js
cmd: runChakra :pass, "NoException", "", ["../UnitTestFramework/UnitTestFramework.js"]
- path: ChakraCore/test/es6/OS_2700778.js
Expand Down
33 changes: 0 additions & 33 deletions JSTests/ChakraCore/test/es6/arraywithproxy.baseline

This file was deleted.

124 changes: 0 additions & 124 deletions JSTests/ChakraCore/test/es6/proxytest9.baseline

This file was deleted.

15 changes: 15 additions & 0 deletions JSTests/ChangeLog
@@ -1,3 +1,18 @@
2020-04-23 Alexey Shvayka <shvaikalesh@gmail.com>

Remove revoked Proxy checks from ProxyCreate
https://bugs.webkit.org/show_bug.cgi?id=210862

Reviewed by Ross Kirsling.

Removes expectations for 2 invalid ChakraCore tests.

* ChakraCore.yaml: Mark 2 test cases as passing.
* ChakraCore/test/es6/arraywithproxy.baseline: Removed.
* ChakraCore/test/es6/proxytest9.baseline: Removed.
* stress/proxy-revoke.js: Adjust test.
* test262/expectations.yaml: Mark 12 test cases as passing.

2020-04-23 Ross Kirsling <ross.kirsling@sony.com>

Unreviewed test262 gardening following r260591.
Expand Down
3 changes: 1 addition & 2 deletions JSTests/stress/proxy-revoke.js
Expand Up @@ -95,9 +95,8 @@ function assert(b) {
new Proxy(proxy, {});
} catch(e) {
threw = true;
assert(e.toString() === "TypeError: A Proxy's 'target' shouldn't be a revoked Proxy");
}
assert(threw);
assert(!threw);
}
foo();
}
Expand Down
18 changes: 0 additions & 18 deletions JSTests/test262/expectations.yaml
Expand Up @@ -1252,15 +1252,6 @@ test/built-ins/Proxy/construct/return-not-object-throws-undefined-realm.js:
test/built-ins/Proxy/construct/trap-is-not-callable-realm.js:
default: 'Test262Error: Expected a TypeError but got a TypeError'
strict mode: 'Test262Error: Expected a TypeError but got a TypeError'
test/built-ins/Proxy/create-handler-is-revoked-proxy.js:
default: "TypeError: A Proxy's 'handler' shouldn't be a revoked Proxy"
strict mode: "TypeError: A Proxy's 'handler' shouldn't be a revoked Proxy"
test/built-ins/Proxy/create-target-is-revoked-function-proxy.js:
default: "TypeError: A Proxy's 'target' shouldn't be a revoked Proxy"
strict mode: "TypeError: A Proxy's 'target' shouldn't be a revoked Proxy"
test/built-ins/Proxy/create-target-is-revoked-proxy.js:
default: "TypeError: A Proxy's 'target' shouldn't be a revoked Proxy"
strict mode: "TypeError: A Proxy's 'target' shouldn't be a revoked Proxy"
test/built-ins/Proxy/get-fn-realm-recursive.js:
default: 'Test262Error: Expected true but got false'
strict mode: 'Test262Error: Expected true but got false'
Expand All @@ -1273,15 +1264,6 @@ test/built-ins/Proxy/ownKeys/trap-is-undefined-target-is-proxy.js:
test/built-ins/Proxy/revocable/builtin.js:
default: 'Test262Error: Expected SameValue(«true», «false») to be true'
strict mode: 'Test262Error: Expected SameValue(«true», «false») to be true'
test/built-ins/Proxy/revocable/handler-is-revoked-proxy.js:
default: "TypeError: A Proxy's 'handler' shouldn't be a revoked Proxy"
strict mode: "TypeError: A Proxy's 'handler' shouldn't be a revoked Proxy"
test/built-ins/Proxy/revocable/target-is-revoked-function-proxy.js:
default: "TypeError: A Proxy's 'target' shouldn't be a revoked Proxy"
strict mode: "TypeError: A Proxy's 'target' shouldn't be a revoked Proxy"
test/built-ins/Proxy/revocable/target-is-revoked-proxy.js:
default: "TypeError: A Proxy's 'target' shouldn't be a revoked Proxy"
strict mode: "TypeError: A Proxy's 'target' shouldn't be a revoked Proxy"
test/built-ins/Reflect/ownKeys/order-after-define-property.js:
default: 'Test262Error: Expected [Symbol(b), Symbol(a)] and [Symbol(a), Symbol(b)] to have the same contents. '
strict mode: 'Test262Error: Expected [Symbol(b), Symbol(a)] and [Symbol(a), Symbol(b)] to have the same contents. '
Expand Down
18 changes: 18 additions & 0 deletions Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,21 @@
2020-04-23 Alexey Shvayka <shvaikalesh@gmail.com>

Remove revoked Proxy checks from ProxyCreate
https://bugs.webkit.org/show_bug.cgi?id=210862

Reviewed by Ross Kirsling.

This change removes revoked Proxy checks from ProxyCreate [1], implementing
https://github.com/tc39/ecma262/pull/1814 and aligning JSC with SpiderMonkey.
Also cleans up ProxyObject creation by using isFunction() instead of
isCallable(), which are identical.

[1]: https://tc39.es/ecma262/#sec-proxycreate (steps 2, 4)

* runtime/ProxyObject.cpp:
(JSC::ProxyObject::structureForTarget):
(JSC::ProxyObject::finishCreation):

2020-04-22 Keith Miller <keith_miller@apple.com>

Fix OSR exiting/iterator object checks in for-of bytecodes
Expand Down
25 changes: 3 additions & 22 deletions Source/JavaScriptCore/runtime/ProxyObject.cpp
Expand Up @@ -71,14 +71,8 @@ String ProxyObject::toStringName(const JSObject* object, JSGlobalObject* globalO

Structure* ProxyObject::structureForTarget(JSGlobalObject* globalObject, JSValue target)
{
if (!target.isObject())
return globalObject->proxyObjectStructure();

JSObject* targetAsObject = jsCast<JSObject*>(target);
CallData ignoredCallData;
VM& vm = globalObject->vm();
bool isCallable = targetAsObject->methodTable(vm)->getCallData(targetAsObject, ignoredCallData) != CallType::None;
return isCallable ? globalObject->callableProxyObjectStructure() : globalObject->proxyObjectStructure();
return target.isFunction(vm) ? globalObject->callableProxyObjectStructure() : globalObject->proxyObjectStructure();
}

void ProxyObject::finishCreation(VM& vm, JSGlobalObject* globalObject, JSValue target, JSValue handler)
Expand All @@ -90,33 +84,20 @@ void ProxyObject::finishCreation(VM& vm, JSGlobalObject* globalObject, JSValue t
throwTypeError(globalObject, scope, "A Proxy's 'target' should be an Object"_s);
return;
}
if (ProxyObject* targetAsProxy = jsDynamicCast<ProxyObject*>(vm, target)) {
if (targetAsProxy->isRevoked()) {
throwTypeError(globalObject, scope, "A Proxy's 'target' shouldn't be a revoked Proxy"_s);
return;
}
}
if (!handler.isObject()) {
throwTypeError(globalObject, scope, "A Proxy's 'handler' should be an Object"_s);
return;
}
if (ProxyObject* handlerAsProxy = jsDynamicCast<ProxyObject*>(vm, handler)) {
if (handlerAsProxy->isRevoked()) {
throwTypeError(globalObject, scope, "A Proxy's 'handler' shouldn't be a revoked Proxy"_s);
return;
}
}

JSObject* targetAsObject = jsCast<JSObject*>(target);

CallData ignoredCallData;
m_isCallable = targetAsObject->methodTable(vm)->getCallData(targetAsObject, ignoredCallData) != CallType::None;
m_isCallable = targetAsObject->isFunction(vm);
if (m_isCallable) {
TypeInfo info = structure(vm)->typeInfo();
RELEASE_ASSERT(info.implementsHasInstance() && info.implementsDefaultHasInstance());
}

m_isConstructible = jsCast<JSObject*>(target)->isConstructor(vm);
m_isConstructible = targetAsObject->isConstructor(vm);

m_target.set(vm, this, targetAsObject);
m_handler.set(vm, this, handler);
Expand Down

0 comments on commit 24aa49a

Please sign in to comment.