Title: [258573] trunk/Source/_javascript_Core
Revision
258573
Author
tzaga...@apple.com
Date
2020-03-17 12:45:05 -0700 (Tue, 17 Mar 2020)

Log Message

AccessCase::canReplace should allow a Getter to replace an IntrinsicGetter
https://bugs.webkit.org/show_bug.cgi?id=209158
<rdar://problem/59222012>

Reviewed by Saam Barati.

When we override an intrinsic getter with a user defined getter, we might end up with the
same offset and attributes. In which case, an inline cache that contained an entry for the
intrisic getter will believe that it is still valid, and add a new getter access case,
leading to duplicate entries for the same structure.

* bytecode/AccessCase.cpp:
(JSC::AccessCase::canReplace const):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (258572 => 258573)


--- trunk/Source/_javascript_Core/ChangeLog	2020-03-17 19:39:47 UTC (rev 258572)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-03-17 19:45:05 UTC (rev 258573)
@@ -1,3 +1,19 @@
+2020-03-17  Tadeu Zagallo  <tzaga...@apple.com>
+
+        AccessCase::canReplace should allow a Getter to replace an IntrinsicGetter
+        https://bugs.webkit.org/show_bug.cgi?id=209158
+        <rdar://problem/59222012>
+
+        Reviewed by Saam Barati.
+
+        When we override an intrinsic getter with a user defined getter, we might end up with the
+        same offset and attributes. In which case, an inline cache that contained an entry for the
+        intrisic getter will believe that it is still valid, and add a new getter access case,
+        leading to duplicate entries for the same structure.
+
+        * bytecode/AccessCase.cpp:
+        (JSC::AccessCase::canReplace const):
+
 2020-03-16  Yusuke Suzuki  <ysuz...@apple.com>
 
         [JSC] JSMapIterator and JSSetIterator are CellType

Modified: trunk/Source/_javascript_Core/bytecode/AccessCase.cpp (258572 => 258573)


--- trunk/Source/_javascript_Core/bytecode/AccessCase.cpp	2020-03-17 19:39:47 UTC (rev 258572)
+++ trunk/Source/_javascript_Core/bytecode/AccessCase.cpp	2020-03-17 19:45:05 UTC (rev 258573)
@@ -587,6 +587,23 @@
 
     if (m_identifier != other.m_identifier)
         return false;
+
+    auto checkPolyProtoAndStructure = [&] {
+        if (m_polyProtoAccessChain) {
+            if (!other.m_polyProtoAccessChain)
+                return false;
+            // This is the only check we need since PolyProtoAccessChain contains the base structure.
+            // If we ever change it to contain only the prototype chain, we'll also need to change
+            // this to check the base structure.
+            return structure() == other.structure()
+                && *m_polyProtoAccessChain == *other.m_polyProtoAccessChain;
+        }
+
+        if (!guardedByStructureCheckSkippingConstantIdentifierCheck() || !other.guardedByStructureCheckSkippingConstantIdentifierCheck())
+            return false;
+
+        return structure() == other.structure();
+    };
     
     switch (type()) {
     case IndexedInt32Load:
@@ -648,32 +665,24 @@
     case Replace:
     case Miss:
     case GetGetter:
-    case Getter:
     case Setter:
     case CustomValueGetter:
     case CustomAccessorGetter:
     case CustomValueSetter:
     case CustomAccessorSetter:
-    case IntrinsicGetter:
     case InHit:
     case InMiss:
         if (other.type() != type())
             return false;
 
-        if (m_polyProtoAccessChain) {
-            if (!other.m_polyProtoAccessChain)
-                return false;
-            // This is the only check we need since PolyProtoAccessChain contains the base structure.
-            // If we ever change it to contain only the prototype chain, we'll also need to change
-            // this to check the base structure.
-            return structure() == other.structure()
-                && *m_polyProtoAccessChain == *other.m_polyProtoAccessChain;
-        }
+        return checkPolyProtoAndStructure();
 
-        if (!guardedByStructureCheckSkippingConstantIdentifierCheck() || !other.guardedByStructureCheckSkippingConstantIdentifierCheck())
+    case IntrinsicGetter:
+    case Getter:
+        if (other.type() != Getter && other.type() != IntrinsicGetter)
             return false;
 
-        return structure() == other.structure();
+        return checkPolyProtoAndStructure();
     }
     RELEASE_ASSERT_NOT_REACHED();
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to