JSObject::putInlineSlow should not ignore "__proto__" for Proxy
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Sep 2019 19:32:39 +0000 (19:32 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Sep 2019 19:32:39 +0000 (19:32 +0000)
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/JavaScriptCore:

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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249911 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/proxy-__proto__-in-prototype-chain.js [new file with mode: 0644]
JSTests/stress/proxy-property-replace-structure-transition.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObject.cpp

index 4f288e0df08739a2d1ea9499d41dc8c6fecdbf6f..688cd92b044fc66f2dc882cb7c51d8e07437e723 100644 (file)
@@ -1,3 +1,14 @@
+2019-09-16  Saam Barati  <sbarati@apple.com>
+
+        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  <shvaikalesh@gmail.com>
 
         Date.prototype.toJSON does not execute steps 1-2
diff --git a/JSTests/stress/proxy-__proto__-in-prototype-chain.js b/JSTests/stress/proxy-__proto__-in-prototype-chain.js
new file mode 100644 (file)
index 0000000..ad1432b
--- /dev/null
@@ -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;
diff --git a/JSTests/stress/proxy-property-replace-structure-transition.js b/JSTests/stress/proxy-property-replace-structure-transition.js
new file mode 100644 (file)
index 0000000..a11ff98
--- /dev/null
@@ -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();
index 604dbd4ae189f0620d67b7c9dfc2e88479da1e44..25b102838963f778b5b2d453ecc64d93fe324b22 100644 (file)
@@ -1,3 +1,22 @@
+2019-09-16  Saam Barati  <sbarati@apple.com>
+
+        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  <ddkilzer@apple.com>
 
         Leak of NSMapTable in -[JSVirtualMachine addManagedReference:withOwner:]
index 68bca0125e8ec1bc870a907b8c9c5652fbe3ff72..259fe72efab9cff3a217aaa627179ef28a451858 100644 (file)
@@ -684,7 +684,7 @@ bool ordinarySetSlow(ExecState* exec, JSObject* object, PropertyName propertyNam
     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 @@ bool JSObject::putInlineSlow(ExecState* exec, PropertyName propertyName, JSValue
             }
             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 @@ bool JSObject::putInlineSlow(ExecState* exec, PropertyName propertyName, JSValue
                     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