Title: [249911] trunk
Revision
249911
Author
[email protected]
Date
2019-09-16 12:32:39 -0700 (Mon, 16 Sep 2019)

Log Message

JSObject::putInlineSlow should not ignore "__proto__" for Proxy
https://bugs.webkit.org/show_bug.cgi?id=200386
<rdar://problem/53854946>

Reviewed by Yusuke Suzuki.

JSTests:

* stress/proxy-__proto__-in-prototype-chain.js: Added.
* stress/proxy-property-replace-structure-transition.js: Added.

Source/_javascript_Core:

We used to ignore '__proto__' in putInlineSlow when the object in question
was Proxy. There is no reason for this, and it goes against the spec. So
I've removed that condition. This also has the effect that it fixes an
assertion firing inside our inline caching code which dictates that for a
property replace that the base value's structure must be equal to the
structure when we grabbed the structure prior to the put operation.
The old code caused a weird edge case where we broke this invariant.

* runtime/JSObject.cpp:
(JSC::JSObject::putInlineSlow):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (249910 => 249911)


--- trunk/JSTests/ChangeLog	2019-09-16 19:15:51 UTC (rev 249910)
+++ trunk/JSTests/ChangeLog	2019-09-16 19:32:39 UTC (rev 249911)
@@ -1,3 +1,14 @@
+2019-09-16  Saam Barati  <[email protected]>
+
+        JSObject::putInlineSlow should not ignore "__proto__" for Proxy
+        https://bugs.webkit.org/show_bug.cgi?id=200386
+        <rdar://problem/53854946>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/proxy-__proto__-in-prototype-chain.js: Added.
+        * stress/proxy-property-replace-structure-transition.js: Added.
+
 2019-09-13  Alexey Shvayka  <[email protected]>
 
         Date.prototype.toJSON does not execute steps 1-2

Added: trunk/JSTests/stress/proxy-__proto__-in-prototype-chain.js (0 => 249911)


--- trunk/JSTests/stress/proxy-__proto__-in-prototype-chain.js	                        (rev 0)
+++ trunk/JSTests/stress/proxy-__proto__-in-prototype-chain.js	2019-09-16 19:32:39 UTC (rev 249911)
@@ -0,0 +1,16 @@
+let called = false;
+let p = new Proxy({ }, {
+    set(obj, prop, value) {
+        called = prop === "__proto__";
+    }
+});
+let o = {__proto__: p};
+o.__proto__ = null;
+
+if (!called)
+    throw new Error;
+
+called = false;
+Reflect.set(o, "__proto__", null, {});
+if (!called)
+    throw new Error;

Added: trunk/JSTests/stress/proxy-property-replace-structure-transition.js (0 => 249911)


--- trunk/JSTests/stress/proxy-property-replace-structure-transition.js	                        (rev 0)
+++ trunk/JSTests/stress/proxy-property-replace-structure-transition.js	2019-09-16 19:32:39 UTC (rev 249911)
@@ -0,0 +1,23 @@
+let map = new Map();
+let count = 0;
+function outer() {
+    ++count;
+    if (count >= 5)
+        return;
+    function inner() {
+        function getPrototypeOf() {
+            const result = outer();
+            return null;
+        }
+
+        const handler = { getPrototypeOf: getPrototypeOf };
+        const p = new Proxy(map,handler);
+
+        map.__proto__ = p;
+        const result = inner();
+    }
+    try {
+        const result = inner();
+    } catch { }
+}
+const result = outer();

Modified: trunk/Source/_javascript_Core/ChangeLog (249910 => 249911)


--- trunk/Source/_javascript_Core/ChangeLog	2019-09-16 19:15:51 UTC (rev 249910)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-09-16 19:32:39 UTC (rev 249911)
@@ -1,3 +1,22 @@
+2019-09-16  Saam Barati  <[email protected]>
+
+        JSObject::putInlineSlow should not ignore "__proto__" for Proxy
+        https://bugs.webkit.org/show_bug.cgi?id=200386
+        <rdar://problem/53854946>
+
+        Reviewed by Yusuke Suzuki.
+
+        We used to ignore '__proto__' in putInlineSlow when the object in question
+        was Proxy. There is no reason for this, and it goes against the spec. So
+        I've removed that condition. This also has the effect that it fixes an
+        assertion firing inside our inline caching code which dictates that for a
+        property replace that the base value's structure must be equal to the
+        structure when we grabbed the structure prior to the put operation.
+        The old code caused a weird edge case where we broke this invariant.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::putInlineSlow):
+
 2019-09-15  David Kilzer  <[email protected]>
 
         Leak of NSMapTable in -[JSVirtualMachine addManagedReference:withOwner:]

Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (249910 => 249911)


--- trunk/Source/_javascript_Core/runtime/JSObject.cpp	2019-09-16 19:15:51 UTC (rev 249910)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp	2019-09-16 19:32:39 UTC (rev 249911)
@@ -684,7 +684,7 @@
     JSObject* current = object;
     PropertyDescriptor ownDescriptor;
     while (true) {
-        if (current->type() == ProxyObjectType && propertyName != vm.propertyNames->underscoreProto) {
+        if (current->type() == ProxyObjectType) {
             ProxyObject* proxy = jsCast<ProxyObject*>(current);
             PutPropertySlot slot(receiver, shouldThrow);
             RELEASE_AND_RETURN(scope, proxy->ProxyObject::put(proxy, exec, propertyName, value, slot));
@@ -828,8 +828,8 @@
             }
             ASSERT(!(attributes & PropertyAttribute::Accessor));
 
-            // If there's an existing property on the object or one of its 
-            // prototypes it should be replaced, so break here.
+            // If there's an existing property on the base object, or on one of its 
+            // prototypes, we should store the property on the *base* object.
             break;
         }
         if (!obj->staticPropertiesReified(vm)) {
@@ -838,7 +838,7 @@
                     RELEASE_AND_RETURN(scope, putEntry(exec, entry->table->classForThis, entry->value, obj, this, propertyName, value, slot));
             }
         }
-        if (obj->type() == ProxyObjectType && propertyName != vm.propertyNames->underscoreProto) {
+        if (obj->type() == ProxyObjectType) {
             // FIXME: We shouldn't unconditionally perform [[Set]] here.
             // We need to do more because this is observable behavior.
             // https://bugs.webkit.org/show_bug.cgi?id=155012
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to