[JSC] Use storage node in ArrayPush for SlowPutArray

https://bugs.webkit.org/show_bug.cgi?id=246405
rdar://problem/101081844

Reviewed by Justin Michaud.

This patch fixes a bug that GetArrayLength gets nullptr crash when we convert
ArrayPush+SlowPutArray with empty arguments to GetArrayLength because we are discarding
butterfly storage for that case. But since SlowPutArray's ArrayPush is slow anyway, let's simplify
our code and always get butterfly storage even for SlowPutArray case.

* JSTests/stress/slow-put-array-empty-push.js: Added.
(runNearStackLimit):
(__f_6):
(__f_32):
* Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:
* Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileArrayPush):

Canonical link: https://commits.webkit.org/255454@main

JSTests/stress/slow-put-array-empty-push.js

@@ -0,0 +1,27 @@
+//@ runDefault("--useConcurrentGC=0", "--returnEarlyFromInfiniteLoopsForFuzzing=1", "--earlyReturnFromInfiniteLoopsLimit=1000000", "--verifyGC=true", "--forceGCSlowPaths=true", "--forceEagerCompilation=1", "--jitPolicyScale=0", "--useConcurrentJIT=0")
+function runNearStackLimit() {
+  __v_21 = []
+  try {
+    try {
+        __v_21.push()} catch {}
+  } catch {}
+}
+function __f_6() {
+  try {
+       runNearStackLimit()
+  } catch {}
+}
+try {
+  __f_6()
+  for (__v_19 = 0; __v_19 < 10; ++__v_19)
+    try {
+      Object.defineProperty(Array.prototype, __v_19, {})} catch {}
+} catch {}
+function __f_32() {
+  try {
+    __f_6()
+      } catch {}
+}
+try {
+  __f_32()
+  __f_32()} catch {}

Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

@@ -1437,16 +1437,7 @@ class FixupPhase : public Phase {
             if (!elementCount)
                 node->setArrayMode(node->arrayMode().refine(m_graph, node, arrayEdge->prediction() & SpecCell, SpecInt32Only));
 
-            switch (node->arrayMode().type()) {
-            case Array::SlowPutArrayStorage: {
-                Edge unusedEdge;
-                blessArrayOperation(arrayEdge, Edge(), unusedEdge, neverNeedsStorage);
-                break;
-            }
-            default:
-                blessArrayOperation(arrayEdge, Edge(), storageEdge);
-                break;
-            }
+            blessArrayOperation(arrayEdge, Edge(), storageEdge);
             fixEdge<KnownCellUse>(arrayEdge);
 
             // Convert `array.push()` to GetArrayLength.

Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -10320,44 +10320,13 @@ void SpeculativeJIT::compileArrayPush(Node* node)
     unsigned elementOffset = 2;
     unsigned elementCount = node->numChildren() - elementOffset;
 
-    if (node->arrayMode().type() == Array::SlowPutArrayStorage) {
-        SpeculateCellOperand base(this, arrayEdge);
-        GPRTemporary buffer(this);
-        ASSERT(!storageEdge.node());
-
-        GPRReg baseGPR = base.gpr();
-        GPRReg bufferGPR = buffer.gpr();
-
-        size_t scratchSize = sizeof(EncodedJSValue) * elementCount;
-        ScratchBuffer* scratchBuffer = vm().scratchBufferForSize(scratchSize);
-        m_jit.move(TrustedImmPtr(static_cast<EncodedJSValue*>(scratchBuffer->dataBuffer())), bufferGPR);
-
-        for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
-            Edge& element = m_graph.varArgChild(node, elementIndex + elementOffset);
-            JSValueOperand value(this, element);
-            JSValueRegs valueRegs = value.jsValueRegs();
-            m_jit.storeValue(valueRegs, MacroAssembler::Address(bufferGPR, sizeof(EncodedJSValue) * elementIndex));
-            value.use();
-        }
-        base.use();
-
-        flushRegisters();
-        JSValueRegsFlushedCallResult result(this);
-        JSValueRegs resultRegs = result.regs();
-        callOperation(operationArrayPushMultipleSlow, resultRegs, JITCompiler::LinkableConstant(m_jit, m_graph.globalObjectFor(node->origin.semantic)), baseGPR, bufferGPR, CCallHelpers::TrustedImm32(elementCount));
-
-        jsValueResult(resultRegs, node, DataFormatJS, UseChildrenCalledExplicitly);
-        return;
-    }
-
     SpeculateCellOperand base(this, arrayEdge);
+    StorageOperand storage(this, storageEdge);
     GPRTemporary storageLength(this);
 
     GPRReg baseGPR = base.gpr();
-    GPRReg storageLengthGPR = storageLength.gpr();
-
-    StorageOperand storage(this, storageEdge);
     GPRReg storageGPR = storage.gpr();
+    GPRReg storageLengthGPR = storageLength.gpr();
 
 #if USE(JSVALUE32_64)
     GPRTemporary tag(this);
@@ -10594,8 +10563,32 @@ void SpeculativeJIT::compileArrayPush(Node* node)
         return;
     }
 
+    case Array::SlowPutArrayStorage: {
+        GPRTemporary buffer(this);
+        GPRReg bufferGPR = buffer.gpr();
+
+        size_t scratchSize = sizeof(EncodedJSValue) * elementCount;
+        ScratchBuffer* scratchBuffer = vm().scratchBufferForSize(scratchSize);
+        m_jit.move(TrustedImmPtr(static_cast<EncodedJSValue*>(scratchBuffer->dataBuffer())), bufferGPR);
+
+        for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
+            Edge& element = m_graph.varArgChild(node, elementIndex + elementOffset);
+            JSValueOperand value(this, element);
+            JSValueRegs valueRegs = value.jsValueRegs();
+            m_jit.storeValue(valueRegs, MacroAssembler::Address(bufferGPR, sizeof(EncodedJSValue) * elementIndex));
+            value.use();
+        }
+        base.use();
+        storage.use();
+
+        flushRegisters();
+        callOperation(operationArrayPushMultipleSlow, resultRegs, JITCompiler::LinkableConstant(m_jit, m_graph.globalObjectFor(node->origin.semantic)), baseGPR, bufferGPR, CCallHelpers::TrustedImm32(elementCount));
+
+        jsValueResult(resultRegs, node, DataFormatJS, UseChildrenCalledExplicitly);
+        return;
+    }
+
     case Array::ForceExit:
-    case Array::SlowPutArrayStorage:
         DFG_CRASH(m_graph, node, "Bad array mode type");
         break;
 

Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -6493,25 +6493,11 @@ IGNORE_CLANG_WARNINGS_END
     {
         JSGlobalObject* globalObject = m_graph.globalObjectFor(m_origin.semantic);
         LValue base = lowCell(m_graph.varArgChild(m_node, 1));
+        LValue storage = lowStorage(m_graph.varArgChild(m_node, 0));
+
         unsigned elementOffset = 2;
         unsigned elementCount = m_node->numChildren() - elementOffset;
 
-        if (m_node->arrayMode().type() == Array::SlowPutArrayStorage) {
-            size_t scratchSize = sizeof(EncodedJSValue) * elementCount;
-            ASSERT(scratchSize);
-            ScratchBuffer* scratchBuffer = vm().scratchBufferForSize(scratchSize);
-            LValue buffer = m_out.constIntPtr(static_cast<EncodedJSValue*>(scratchBuffer->dataBuffer()));
-            for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
-                Edge& element = m_graph.varArgChild(m_node, elementIndex + elementOffset);
-                LValue value = lowJSValue(element);
-                m_out.store64(value, m_out.baseIndex(m_heaps.ArrayStorage_vector.atAnyIndex(), buffer, m_out.constIntPtr(elementIndex), ScaleEight));
-            }
-            setJSValue(vmCall(Int64, operationArrayPushMultipleSlow, weakPointer(globalObject), base, buffer, m_out.constInt32(elementCount)));
-            return;
-        }
-
-        LValue storage = lowStorage(m_graph.varArgChild(m_node, 0));
-
         switch (m_node->arrayMode().type()) {
         case Array::Int32:
         case Array::Contiguous:
@@ -6722,6 +6708,20 @@ IGNORE_CLANG_WARNINGS_END
             return;
         }
 
+        case Array::SlowPutArrayStorage: {
+            size_t scratchSize = sizeof(EncodedJSValue) * elementCount;
+            ASSERT(scratchSize);
+            ScratchBuffer* scratchBuffer = vm().scratchBufferForSize(scratchSize);
+            LValue buffer = m_out.constIntPtr(static_cast<EncodedJSValue*>(scratchBuffer->dataBuffer()));
+            for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
+                Edge& element = m_graph.varArgChild(m_node, elementIndex + elementOffset);
+                LValue value = lowJSValue(element);
+                m_out.store64(value, m_out.baseIndex(m_heaps.ArrayStorage_vector.atAnyIndex(), buffer, m_out.constIntPtr(elementIndex), ScaleEight));
+            }
+            setJSValue(vmCall(Int64, operationArrayPushMultipleSlow, weakPointer(globalObject), base, buffer, m_out.constInt32(elementCount)));
+            return;
+        }
+
         default:
             DFG_CRASH(m_graph, m_node, "Bad array type");
             return;