Changeset 271034 in webkit

Timestamp:

Dec 21, 2020, 11:54:14 AM (6 years ago)

Author:

keith_miller@apple.com

Message:

DFG should make sure replacement watchpoint is fired before folding to PutByOffset
​https://bugs.webkit.org/show_bug.cgi?id=220031
<rdar://72045350>

Reviewed by Saam Barati.

• dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::presenceConditionIfConsistent):
(JSC::DFG::ByteCodeParser::checkPresence):
(JSC::DFG::ByteCodeParser::checkPresenceForReplace):
(JSC::DFG::ByteCodeParser::load):
(JSC::DFG::ByteCodeParser::store):
(JSC::DFG::ByteCodeParser::presenceLike): Deleted.
(JSC::DFG::ByteCodeParser::checkPresenceLike): Deleted.

• dfg/DFGConstantFoldingPhase.cpp:

(JSC::DFG::ConstantFoldingPhase::tryFoldAsPutByOffset):

• runtime/Structure.cpp:

(JSC::Structure::dump const):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGByteCodeParser.cpp (modified) (6 diffs)
• dfg/DFGConstantFoldingPhase.cpp (modified) (1 diff)
• runtime/Structure.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-12-21  Keith Miller  <keith_miller@apple.com>
+
+        DFG should make sure replacement watchpoint is fired before folding to PutByOffset
+        https://bugs.webkit.org/show_bug.cgi?id=220031
+        <rdar://72045350>
+
+        Reviewed by Saam Barati.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::presenceConditionIfConsistent):
+        (JSC::DFG::ByteCodeParser::checkPresence):
+        (JSC::DFG::ByteCodeParser::checkPresenceForReplace):
+        (JSC::DFG::ByteCodeParser::load):
+        (JSC::DFG::ByteCodeParser::store):
+        (JSC::DFG::ByteCodeParser::presenceLike): Deleted.
+        (JSC::DFG::ByteCodeParser::checkPresenceLike): Deleted.
+        * dfg/DFGConstantFoldingPhase.cpp:
+        (JSC::DFG::ConstantFoldingPhase::tryFoldAsPutByOffset):
+        * runtime/Structure.cpp:
+        (JSC::Structure::dump const):
+
 2020-12-18  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

@@ -228 +228 @@
     // Create a presence ObjectPropertyCondition based on some known offset and structure set. Does not
     // check the validity of the condition, but it may return a null one if it encounters a contradiction.
-    ObjectPropertyCondition presenceLike(
+    ObjectPropertyCondition presenceConditionIfConsistent(
         JSObject* knownBase, UniquedStringImpl*, PropertyOffset, const StructureSet&);
     
@@ -234 +234 @@
     // way as in all of the structures in the set. It may emit code instead of just setting a watchpoint.
     // Returns true if this all works out.
-    bool checkPresenceLike(JSObject* knownBase, UniquedStringImpl*, PropertyOffset, const StructureSet&);
-    void checkPresenceLike(Node* base, UniquedStringImpl*, PropertyOffset, const StructureSet&);
+    bool checkPresence(JSObject* knownBase, UniquedStringImpl*, PropertyOffset, const StructureSet&);
+    void checkPresenceForReplace(Node* base, UniquedStringImpl*, PropertyOffset, const StructureSet&);
     
     // Works with both GetByIdVariant and the setter form of PutByIdVariant.
@@ -4387 +4387 @@
 }
 
-ObjectPropertyCondition ByteCodeParser::presenceLike(
+ObjectPropertyCondition ByteCodeParser::presenceConditionIfConsistent(
     JSObject* knownBase, UniquedStringImpl* uid, PropertyOffset offset, const StructureSet& set)
 {
@@ -4405 +4405 @@
 }
 
-bool ByteCodeParser::checkPresenceLike(
+bool ByteCodeParser::checkPresence(
     JSObject* knownBase, UniquedStringImpl* uid, PropertyOffset offset, const StructureSet& set)
 {
-    return check(presenceLike(knownBase, uid, offset, set));
+    return check(presenceConditionIfConsistent(knownBase, uid, offset, set));
 }
 
-void ByteCodeParser::checkPresenceLike(
+void ByteCodeParser::checkPresenceForReplace(
     Node* base, UniquedStringImpl* uid, PropertyOffset offset, const StructureSet& set)
 {
     if (JSObject* knownBase = base->dynamicCastConstant<JSObject*>(*m_vm)) {
-        if (checkPresenceLike(knownBase, uid, offset, set))
+        auto condition = presenceConditionIfConsistent(knownBase, uid, offset, set);
+        if (check(condition)) {
+            auto* watchpointSet = knownBase->structure(*m_vm)->propertyReplacementWatchpointSet(condition.offset());
+            // This means that we probably have a stale cache and we should gather more information.
+            if (!watchpointSet || watchpointSet->isStillValid())
+                addToGraph(ForceOSRExit);
             return;
+        }
     }
 
@@ -4469 +4475 @@
 
                 ObjectPropertyCondition presenceCondition =
-                    presenceLike(knownBase, uid, variant.offset(), variant.structureSet());
+                    presenceConditionIfConsistent(knownBase, uid, variant.offset(), variant.structureSet());
                 if (presenceCondition) {
                     ObjectPropertyCondition equivalenceCondition =
@@ -4526 +4532 @@
     RELEASE_ASSERT(variant.kind() == PutByIdVariant::Replace);
 
-    checkPresenceLike(base, m_graph.identifiers()[identifier], variant.offset(), variant.structure());
+    checkPresenceForReplace(base, m_graph.identifiers()[identifier], variant.offset(), variant.structure());
     return handlePutByOffset(base, identifier, variant.offset(), value);
 }

trunk/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp

@@ -1428 +1428 @@
                     return;
 
+                if (variant.kind() == PutByIdVariant::Replace) {
+                    auto* watchpoints = structure->propertyReplacementWatchpointSet(condition.offset());
+                    if (!watchpoints || watchpoints->isStillValid())
+                        return;
+                }
+
                 m_insertionSet.insertNode(
                     indexInBlock, SpecNone, CheckStructure, node->origin,

trunk/Source/JavaScriptCore/runtime/Structure.cpp

@@ -1409 +1409 @@
         out.print(", Shady leaf");
     
+    if (transitionWatchpointSet().isBeingWatched())
+        out.print(" (Watched)");
+
     out.print("]");
 }