Changeset 252191 in webkit

Timestamp:

Nov 7, 2019 10:31:31 AM (4 years ago)

Author:

commit-queue@webkit.org

Message:

HasProperty? result of Proxy in prototype chain is ignored
​https://bugs.webkit.org/show_bug.cgi?id=203560

Patch by Alexey Shvayka <Alexey Shvayka> on 2019-11-07
Reviewed by Ross Kirsling.

JSTests:

• stress/proxy-get-prototype-of.js: Correct Proxy "has" trap test.
• test262/expectations.yaml: Mark 6 test cases as passing.

Source/JavaScriptCore:

Before this change, when HasProperty? was called on ordinary object with Proxy in prototype chain,
falsy result of Proxy's "has" trap was ignored and prototype chain was inspected further.

According to spec, OrdinaryHasProperty unconditionally returns result of parent's HasProperty? call.
(step 5.a of ​https://tc39.es/ecma262/#sec-ordinaryhasproperty)

• runtime/JSObjectInlines.h:

(JSC::JSObject::getPropertySlot):
(JSC::JSObject::getNonIndexPropertySlot):

• runtime/ProxyObject.cpp:

(JSC::ProxyObject::performHasProperty): Walk the prototype chain in performDefaultHasProperty.

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/proxy-get-prototype-of.js (modified) (2 diffs)
• JSTests/test262/expectations.yaml (modified) (4 diffs)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSObjectInlines.h (modified) (2 diffs)
• Source/JavaScriptCore/runtime/ProxyObject.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-11-07  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        [[HasProperty]] result of Proxy in prototype chain is ignored
+        https://bugs.webkit.org/show_bug.cgi?id=203560
+
+        Reviewed by Ross Kirsling.
+
+        * stress/proxy-get-prototype-of.js: Correct Proxy "has" trap test.
+        * test262/expectations.yaml: Mark 6 test cases as passing.
+
 2019-11-06  Mark Lam  <mark.lam@apple.com>
 

trunk/JSTests/stress/proxy-get-prototype-of.js

@@ -390 +390 @@
     let handler = {
         getPrototypeOf: function(theTarget) {
-            assert(theTarget === target);
             called = true;
             return proto;
@@ -401 +400 @@
     let proxy = new Proxy(target, handler);
     for (let i = 0; i < 500; i++) {
-        let result = "x" in proxy;
-        assert(called);
-        called = false;
+        let result = 1 in proxy;
+        assert(!called);
     }
 }

trunk/JSTests/test262/expectations.yaml

@@ -634 +634 @@
   default: 'TypeError: Attempted to assign to readonly property.'
   strict mode: 'TypeError: Attempted to assign to readonly property.'
-test/built-ins/Array/prototype/indexOf/calls-only-has-on-prototype-after-length-zeroed.js:
-  default: 'Test262Error: [[GetPrototypeOf]] trap called'
-  strict mode: 'Test262Error: [[GetPrototypeOf]] trap called'
-test/built-ins/Array/prototype/lastIndexOf/calls-only-has-on-prototype-after-length-zeroed.js:
-  default: 'Test262Error: [[GetPrototypeOf]] trap called'
-  strict mode: 'Test262Error: [[GetPrototypeOf]] trap called'
 test/built-ins/Array/prototype/map/target-array-with-non-writable-property.js:
   default: 'TypeError: Attempted to assign to readonly property.'
@@ -665 +659 @@
   strict mode: 'Test262Error: Length is 2**53 - 1 Expected a TypeError to be thrown but no exception was thrown at all'
 test/built-ins/Array/prototype/reverse/length-exceeding-integer-limit-with-proxy.js:
-  default: 'Test262Error: Expected a StopReverse but got a Test262Error'
-  strict mode: 'Test262Error: Expected a StopReverse but got a Test262Error'
+  default: 'Test262Error: Expected [Get:length, Has:0, Get:0, Has:4294967294, Delete:0, Set:4294967294, GetOwnPropertyDescriptor:4294967294, DefineProperty:4294967294, Has:1, Has:4294967293, Delete:1, Delete:4294967293, Has:2, Get:2, Has:4294967292, Delete:2, Set:4294967292, GetOwnPropertyDescriptor:4294967292, DefineProperty:4294967292, Has:3, Has:4294967291, Delete:3, Delete:4294967291, Has:4, Get:4] and [Get:length, Has:0, Get:0, Has:9007199254740990, Get:9007199254740990, Set:0, GetOwnPropertyDescriptor:0, DefineProperty:0, Set:9007199254740990, GetOwnPropertyDescriptor:9007199254740990, DefineProperty:9007199254740990, Has:1, Has:9007199254740989, Has:2, Get:2, Has:9007199254740988, Delete:2, Set:9007199254740988, GetOwnPropertyDescriptor:9007199254740988, DefineProperty:9007199254740988, Has:3, Has:9007199254740987, Get:9007199254740987, Set:3, GetOwnPropertyDescriptor:3, DefineProperty:3, Delete:9007199254740987, Has:4, Get:4] to have the same contents. '
+  strict mode: 'Test262Error: Expected [Get:length, Has:0, Get:0, Has:4294967294, Delete:0, Set:4294967294, GetOwnPropertyDescriptor:4294967294, DefineProperty:4294967294, Has:1, Has:4294967293, Delete:1, Delete:4294967293, Has:2, Get:2, Has:4294967292, Delete:2, Set:4294967292, GetOwnPropertyDescriptor:4294967292, DefineProperty:4294967292, Has:3, Has:4294967291, Delete:3, Delete:4294967291, Has:4, Get:4] and [Get:length, Has:0, Get:0, Has:9007199254740990, Get:9007199254740990, Set:0, GetOwnPropertyDescriptor:0, DefineProperty:0, Set:9007199254740990, GetOwnPropertyDescriptor:9007199254740990, DefineProperty:9007199254740990, Has:1, Has:9007199254740989, Has:2, Get:2, Has:9007199254740988, Delete:2, Set:9007199254740988, GetOwnPropertyDescriptor:9007199254740988, DefineProperty:9007199254740988, Has:3, Has:9007199254740987, Get:9007199254740987, Set:3, GetOwnPropertyDescriptor:3, DefineProperty:3, Delete:9007199254740987, Has:4, Get:4] to have the same contents. '
 test/built-ins/Array/prototype/slice/length-exceeding-integer-limit-proxied-array.js:
   default: 'Test262Error: Expected [] and [9007199254740989, 9007199254740990] to have the same contents. slice(9007199254740989)'
@@ -686 +680 @@
   strict mode: 'TypeError: Attempted to assign to readonly property.'
 test/built-ins/Array/prototype/splice/create-species-length-exceeding-integer-limit.js:
-  default: 'Test262Error: Expected a StopSplice but got a Test262Error'
-  strict mode: 'Test262Error: Expected a StopSplice but got a Test262Error'
+  default: 'Test262Error: length and deleteCount were correctly clamped to 2^53-1 Expected SameValue(«4294967295», «9007199254740991») to be true'
+  strict mode: 'Test262Error: length and deleteCount were correctly clamped to 2^53-1 Expected SameValue(«4294967295», «9007199254740991») to be true'
 test/built-ins/Array/prototype/splice/length-and-deleteCount-exceeding-integer-limit.js:
   default: "Test262Error: Expected [] and [9007199254740989, 9007199254740990] to have the same contents. arrayLike['9007199254740989'] and arrayLike['9007199254740990'] are removed"
@@ -1154 +1148 @@
   default: 'Test262Error: Expected a TypeError but got a TypeError'
   strict mode: 'Test262Error: Expected a TypeError but got a TypeError'
-test/built-ins/Proxy/has/call-in-prototype.js:
-  default: 'Test262Error: [[GetPrototypeOf]] trap called'
-  strict mode: 'Test262Error: [[GetPrototypeOf]] trap called'
 test/built-ins/Proxy/revocable/revocation-function-name.js:
   default: 'Test262Error: obj should have an own property name'

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-11-07  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        [[HasProperty]] result of Proxy in prototype chain is ignored
+        https://bugs.webkit.org/show_bug.cgi?id=203560
+
+        Reviewed by Ross Kirsling.
+
+        Before this change, when [[HasProperty]] was called on ordinary object with Proxy in prototype chain,
+        falsy result of Proxy's "has" trap was ignored and prototype chain was inspected further.
+
+        According to spec, OrdinaryHasProperty unconditionally returns result of parent's [[HasProperty]] call.
+        (step 5.a of https://tc39.es/ecma262/#sec-ordinaryhasproperty)
+
+        * runtime/JSObjectInlines.h:
+        (JSC::JSObject::getPropertySlot):
+        (JSC::JSObject::getNonIndexPropertySlot):
+        * runtime/ProxyObject.cpp:
+        (JSC::ProxyObject::performHasProperty): Walk the prototype chain in performDefaultHasProperty.
+
 2019-11-06  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSObjectInlines.h

@@ -127 +127 @@
         if (hasSlot)
             return true;
+        if (object->type() == ProxyObjectType && slot.internalMethodType() == PropertySlot::InternalMethodType::HasProperty)
+            return false;
         JSValue prototype;
         if (LIKELY(structure->classInfo()->methodTable.getPrototype == defaultGetPrototype || slot.internalMethodType() == PropertySlot::InternalMethodType::VMInquiry))
@@ -160 +162 @@
             if (hasSlot)
                 return true;
+            if (object->type() == ProxyObjectType && slot.internalMethodType() == PropertySlot::InternalMethodType::HasProperty)
+                return false;
         }
         JSValue prototype;

trunk/Source/JavaScriptCore/runtime/ProxyObject.cpp

@@ -323 +323 @@
 
     auto performDefaultHasProperty = [&] {
-        return target->methodTable(vm)->getOwnPropertySlot(target, globalObject, propertyName, slot);
+        return target->getPropertySlot(globalObject, propertyName, slot);
     };