DFG tuples should not be queried for their state

rdar://107876378 https://bugs.webkit.org/show_bug.cgi?id=255279 Reviewed by Keith Miller. DFG tuples don't have a type themselves, they represent a collection of elements. We should only ask questions about the type of an element of a tuple, never the tuple directly. Edges to a tuple should always be Untyped. In this test case, we get garbage data when we ask for the type of EnumeratorNextUpdateIndexAndMode from ExtractFromTuple. We remove the assert for this case and add some extra assertions to make sure that nobody else is making the same mistake. * JSTests/stress/dfg-tuple-ai.js: Added. (f3.const.o7.set e): (f3): (const.v15.in.string_appeared_here.v16.v18.catch): * Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h: (JSC::DFG::AbstractInterpreter::forTupleNode): * Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h: (JSC::DFG::AbstractInterpreter<AbstractStateType>::verifyEdge): * Source/JavaScriptCore/dfg/DFGAtTailAbstractState.cpp: (JSC::DFG::AtTailAbstractState::forNode): * Source/JavaScriptCore/dfg/DFGAtTailAbstractState.h: (JSC::DFG::AtTailAbstractState::forNode): (JSC::DFG::AtTailAbstractState::forNodeWithoutFastForward): (JSC::DFG::AtTailAbstractState::clearForNode): (JSC::DFG::AtTailAbstractState::setForNode): (JSC::DFG::AtTailAbstractState::forTupleNodeWithoutFastForward): * Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h: (JSC::DFG::InPlaceAbstractState::forNodeWithoutFastForward): (JSC::DFG::InPlaceAbstractState::forNode): (JSC::DFG::InPlaceAbstractState::clearForNode): (JSC::DFG::InPlaceAbstractState::setForNode): (JSC::DFG::InPlaceAbstractState::setTypeForNode): (JSC::DFG::InPlaceAbstractState::setNonCellTypeForNode): (JSC::DFG::InPlaceAbstractState::makeBytecodeTopForNode): (JSC::DFG::InPlaceAbstractState::makeHeapTopForNode): (JSC::DFG::InPlaceAbstractState::forTupleNodeWithoutFastForward): Canonical link: https://commits.webkit.org/263433@main
WebKit · Apr 26, 2023 · 39dd6c8 · 39dd6c8
1 parent 7fc4afa
commit 39dd6c8
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 1 deletion.
diff --git a/JSTests/stress/dfg-tuple-ai.js b/JSTests/stress/dfg-tuple-ai.js
@@ -0,0 +1,26 @@
+//@ runDefault("--thresholdForOptimizeAfterWarmUp=0", "--thresholdForOptimizeSoon=0", "--thresholdForFTLOptimizeAfterWarmUp=0")
+function f3(a4) {
+    const o7 = {
+        ["forEach"]: "pCGSxWy10A",
+        set e(a6) {
+        },
+    };
+    return a4;
+}
+f3("forEach");
+f3("pCGSxWy10A");
+f3("function");
+const v12 = new Int8Array();
+const v14 = new Uint8ClampedArray(v12);
+for (const v15 in "pCGSxWy10A") {
+    for (let v16 = 0; v16 < 100; v16++) {
+        for (let v18 = 0; v18 < 10; v18++) {
+            try {
+                (2147483649).toString(v16);
+            } catch(e20) {
+            }
+        }
+    }
+}
+f3(v12);
+gc();
diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h
@@ -47,6 +47,11 @@ class AbstractInterpreter {
     {
         return m_state.forNode(node);
     }
+
+    ALWAYS_INLINE AbstractValue& forTupleNode(NodeFlowProjection node, unsigned index)
+    {
+        return m_state.forTupleNode(node, index);
+    }
 
     ALWAYS_INLINE AbstractValue& forNode(Edge edge)
     {

diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
@@ -25,6 +25,7 @@
 
 #pragma once
 
+#include "dfg/DFGNodeType.h"
 #if ENABLE(DFG_JIT)
 
 #include "ArrayConstructor.h"
@@ -172,6 +173,14 @@ ALWAYS_INLINE void AbstractInterpreter<AbstractStateType>::filterByType(Edge& ed
 template<typename AbstractStateType>
 void AbstractInterpreter<AbstractStateType>::verifyEdge(Node* node, Edge edge)
 {
+    if (edge.node()->isTuple()) {
+        if (edge.useKind() == UntypedUse && node->op() == ExtractFromTuple)
+            return;
+        DFG_CRASH(m_graph, node, toCString("Tuple edge verification error: ", node, "->", edge, " was expected to have Untyped use kind (had ", edge.useKind(),
+            "). Has type ", SpeculationDump(m_state.forTupleNodeWithoutFastForward(edge.node(), node->extractOffset()).m_type)).data(),
+            AbstractInterpreterInvalidType, node->op(), edge->op(), edge.useKind(), m_state.forNodeWithoutFastForward(node).m_type);
+    }
+
     if (!(m_state.forNodeWithoutFastForward(edge).m_type & ~typeFilterFor(edge.useKind())))
         return;
 

diff --git a/Source/JavaScriptCore/dfg/DFGAtTailAbstractState.cpp b/Source/JavaScriptCore/dfg/DFGAtTailAbstractState.cpp
@@ -56,6 +56,7 @@ void AtTailAbstractState::createValueForNode(NodeFlowProjection node)
 
 AbstractValue& AtTailAbstractState::forNode(NodeFlowProjection node)
 {
+    ASSERT(!node->isTuple());
     auto& valuesAtTail = m_valuesAtTailMap.at(m_block);
     HashMap<NodeFlowProjection, AbstractValue>::iterator iter = valuesAtTail.find(node);
     DFG_ASSERT(m_graph, node.node(), iter != valuesAtTail.end());

diff --git a/Source/JavaScriptCore/dfg/DFGAtTailAbstractState.h b/Source/JavaScriptCore/dfg/DFGAtTailAbstractState.h
@@ -54,10 +54,15 @@ class AtTailAbstractState {
     AbstractValue& fastForward(AbstractValue& value) { return value; }
 
     AbstractValue& forNode(NodeFlowProjection);
-    AbstractValue& forNode(Edge edge) { return forNode(edge.node()); }
+    AbstractValue& forNode(Edge edge)
+    {
+        ASSERT(!edge.node()->isTuple());
+        return forNode(edge.node());
+    }
 
     ALWAYS_INLINE AbstractValue& forNodeWithoutFastForward(NodeFlowProjection node)
     {
+        ASSERT(!node->isTuple());
         return forNode(node);
     }
 
@@ -73,6 +78,7 @@ class AtTailAbstractState {
 
     ALWAYS_INLINE void clearForNode(NodeFlowProjection node)
     {
+        ASSERT(!node->isTuple());
         forNode(node).clear();
     }
 
@@ -84,6 +90,7 @@ class AtTailAbstractState {
     template<typename... Arguments>
     ALWAYS_INLINE void setForNode(NodeFlowProjection node, Arguments&&... arguments)
     {
+        ASSERT(!node->isTuple());
         forNode(node).set(m_graph, std::forward<Arguments>(arguments)...);
     }
 
@@ -137,6 +144,11 @@ class AtTailAbstractState {
         makeHeapTopForNode(edge.node());
     }
 
+    ALWAYS_INLINE AbstractValue& forTupleNodeWithoutFastForward(NodeFlowProjection node, unsigned index)
+    {
+        return forTupleNode(node, index);
+    }
+
     ALWAYS_INLINE AbstractValue& forTupleNode(NodeFlowProjection node, unsigned index)
     {
         ASSERT(index < node->tupleSize());

diff --git a/Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h b/Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h
@@ -60,16 +60,19 @@ class InPlaceAbstractState {
 
     ALWAYS_INLINE AbstractValue& forNodeWithoutFastForward(NodeFlowProjection node)
     {
+        ASSERT(!node->isTuple());
         return m_abstractValues.at(node);
     }
 
     ALWAYS_INLINE AbstractValue& forNodeWithoutFastForward(Edge edge)
     {
+        ASSERT(!edge.node()->isTuple());
         return forNodeWithoutFastForward(edge.node());
     }
 
     ALWAYS_INLINE AbstractValue& forNode(NodeFlowProjection node)
     {
+        ASSERT(!node->isTuple());
         return fastForward(m_abstractValues.at(node));
     }
 
@@ -80,6 +83,7 @@ class InPlaceAbstractState {
 
     ALWAYS_INLINE void clearForNode(NodeFlowProjection node)
     {
+        ASSERT(!node->isTuple());
         AbstractValue& value = m_abstractValues.at(node);
         value.clear();
         value.m_effectEpoch = m_effectEpoch;
@@ -93,6 +97,7 @@ class InPlaceAbstractState {
     template<typename... Arguments>
     ALWAYS_INLINE void setForNode(NodeFlowProjection node, Arguments&&... arguments)
     {
+        ASSERT(!node->isTuple());
         AbstractValue& value = m_abstractValues.at(node);
         value.set(m_graph, std::forward<Arguments>(arguments)...);
         value.m_effectEpoch = m_effectEpoch;
@@ -107,6 +112,7 @@ class InPlaceAbstractState {
     template<typename... Arguments>
     ALWAYS_INLINE void setTypeForNode(NodeFlowProjection node, Arguments&&... arguments)
     {
+        ASSERT(!node->isTuple());
         AbstractValue& value = m_abstractValues.at(node);
         value.setType(m_graph, std::forward<Arguments>(arguments)...);
         value.m_effectEpoch = m_effectEpoch;
@@ -121,6 +127,7 @@ class InPlaceAbstractState {
     template<typename... Arguments>
     ALWAYS_INLINE void setNonCellTypeForNode(NodeFlowProjection node, Arguments&&... arguments)
     {
+        ASSERT(!node->isTuple());
         AbstractValue& value = m_abstractValues.at(node);
         value.setNonCellType(std::forward<Arguments>(arguments)...);
         value.m_effectEpoch = m_effectEpoch;
@@ -134,6 +141,7 @@ class InPlaceAbstractState {
 
     ALWAYS_INLINE void makeBytecodeTopForNode(NodeFlowProjection node)
     {
+        ASSERT(!node->isTuple());
         AbstractValue& value = m_abstractValues.at(node);
         value.makeBytecodeTop();
         value.m_effectEpoch = m_effectEpoch;
@@ -146,6 +154,7 @@ class InPlaceAbstractState {
 
     ALWAYS_INLINE void makeHeapTopForNode(NodeFlowProjection node)
     {
+        ASSERT(!node->isTuple());
         AbstractValue& value = m_abstractValues.at(node);
         value.makeHeapTop();
         value.m_effectEpoch = m_effectEpoch;
@@ -155,6 +164,13 @@ class InPlaceAbstractState {
     {
         makeHeapTopForNode(edge.node());
     }
+
+    ALWAYS_INLINE AbstractValue& forTupleNodeWithoutFastForward(NodeFlowProjection node, unsigned index)
+    {
+        ASSERT(node->isTuple());
+        ASSERT(index < node->tupleSize());
+        return m_tupleAbstractValues.at(node->tupleOffset() + index);
+    }
 
     ALWAYS_INLINE AbstractValue& forTupleNode(NodeFlowProjection node, unsigned index)
     {