Changeset 257605 in webkit

Timestamp:

Feb 27, 2020, 5:34:05 PM (7 years ago)

Author:

Justin Michaud

Message:

Poly proto should work with property delete transitions
​https://bugs.webkit.org/show_bug.cgi?id=208261

Reviewed by Saam Barati.

JSTests:

• stress/delete-property-poly-proto.js: Added.

(A.prototype.set x):
(A):
(B):

Source/JavaScriptCore:

This patch fixes a bug where the combination of inline caching
and poly proto cause us to cache a setter call along a prototype chain that
is no longer the correct setter to call. This is exposed as a result of
​https://bugs.webkit.org/show_bug.cgi?id=206430 since DefineOwnProperty used
to transition to uncacheable dictionary.

The case looks like this:
A - setter for x redefines x
|
B
|
C

We set (new C).x

Right now, we first call A's setter, then we try to figure out what the state of things
were before it was called in order to cache it. We just assume that A's setter still exists, and we cache it
without ever checking, In this patch, we ensure that the property exists and the attributes match in order to prevent crashing.

In the code, A = target, C = base.

Get is correct because it collects caching information before any calls.

The bug ​https://bugs.webkit.org/show_bug.cgi?id=208337 tracks the remaining semantic bugs around this code.

• jit/Repatch.cpp:

(JSC::tryCachePutByID):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/delete-property-poly-proto.js (added)
• JSTests/stress/poly-proto-setter-adds-setter-in-middle.js (added)
• JSTests/stress/poly-proto-setter-changes-setter-2.js (added)
• JSTests/stress/poly-proto-setter-changes-setter.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/jit/Repatch.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2020-02-27  Justin Michaud  <justin_michaud@apple.com>
+
+        Poly proto should work with property delete transitions
+        https://bugs.webkit.org/show_bug.cgi?id=208261
+
+        Reviewed by Saam Barati.
+
+        * stress/delete-property-poly-proto.js: Added.
+        (A.prototype.set x):
+        (A):
+        (B):
+
 2020-02-27  Ross Kirsling  <ross.kirsling@sony.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-02-27  Justin Michaud  <justin_michaud@apple.com>
+
+        Poly proto should work with property delete transitions
+        https://bugs.webkit.org/show_bug.cgi?id=208261
+
+        Reviewed by Saam Barati.
+
+        This patch fixes a bug where the combination of inline caching
+        and poly proto cause us to cache a setter call along a prototype chain that 
+        is no longer the correct setter to call. This is exposed as a result of
+        https://bugs.webkit.org/show_bug.cgi?id=206430 since DefineOwnProperty used 
+        to transition to uncacheable dictionary.
+
+        The case looks like this:
+        A - setter for x redefines x
+        |
+        B
+        |
+        C
+
+        We set (new C).x
+
+        Right now, we first call A's setter, then we try to figure out what the state of things
+        were before it was called in order to cache it. We just assume that A's setter still exists, and we cache it
+        without ever checking, In this patch, we ensure that the property exists and the attributes match in order to prevent crashing. 
+
+        In the code, A = target, C = base.
+
+        Get is correct because it collects caching information before any calls.
+
+        The bug https://bugs.webkit.org/show_bug.cgi?id=208337 tracks the remaining semantic bugs around this code.
+
+        * jit/Repatch.cpp:
+        (JSC::tryCachePutByID):
+
 2020-02-27  Basuke Suzuki  <basuke.suzuki@sony.com>
 

trunk/Source/JavaScriptCore/jit/Repatch.cpp

@@ -689 +689 @@
                         if (!prototypeAccessChain)
                             return GiveUpOnCache;
-                        offset = prototypeAccessChain->slotBaseStructure(vm, baseCell->structure(vm))->get(vm, ident.impl());
+                        unsigned attributes;
+                        offset = prototypeAccessChain->slotBaseStructure(vm, baseCell->structure(vm))->get(vm, ident.impl(), attributes);
+                        if (!isValidOffset(offset) || !(attributes & PropertyAttribute::Accessor))
+                            return RetryCacheLater;
                     } else {
                         prototypeAccessChain = nullptr;