[webkit-changes] [260180] trunk/Source/JavaScriptCore

Thu, 16 Apr 2020 05:08:02 -0700

Title: [260180] trunk/Source/_javascript_Core
Revision
260180
Author
[email protected]
Date
2020-04-16 05:06:49 -0700 (Thu, 16 Apr 2020)

Log Message

[JSC] Use ensureStillAliveHere in FTL when content of storage should be kept alive
https://bugs.webkit.org/show_bug.cgi?id=210583
<rdar://problem/61831515>

Reviewed by Mark Lam.

The content of Butterfly / ArrayStorage is kept alive only when the owner JSCell is alive.
This means that we should keep the owner JSCell alive if we are loading content of storage
which includes JSCells. This patch inserts ensureStillAliveHere in FTL to ensure this invariant.

* ftl/FTLJITCode.cpp:
(JSC::FTL::JITCode::~JITCode): Found that we get crash with `dumpDisassembly` if FTL::JITCode is destroyed while it fails to generate code while testing this.
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
(JSC::FTL::DFG::LowerDFGToB3::compileArrayIndexOf):
(JSC::FTL::DFG::LowerDFGToB3::compileArrayPop):
(JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
(JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
(JSC::FTL::DFG::LowerDFGToB3::compileStringCodePointAt):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByOffset):
(JSC::FTL::DFG::LowerDFGToB3::compileMultiGetByOffset):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (260179 => 260180)


--- trunk/Source/_javascript_Core/ChangeLog	2020-04-16 11:45:15 UTC (rev 260179)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-04-16 12:06:49 UTC (rev 260180)
@@ -1,3 +1,27 @@
+2020-04-15  Yusuke Suzuki  <[email protected]>
+
+        [JSC] Use ensureStillAliveHere in FTL when content of storage should be kept alive
+        https://bugs.webkit.org/show_bug.cgi?id=210583
+        <rdar://problem/61831515>
+
+        Reviewed by Mark Lam.
+
+        The content of Butterfly / ArrayStorage is kept alive only when the owner JSCell is alive.
+        This means that we should keep the owner JSCell alive if we are loading content of storage
+        which includes JSCells. This patch inserts ensureStillAliveHere in FTL to ensure this invariant.
+
+        * ftl/FTLJITCode.cpp:
+        (JSC::FTL::JITCode::~JITCode): Found that we get crash with `dumpDisassembly` if FTL::JITCode is destroyed while it fails to generate code while testing this.
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
+        (JSC::FTL::DFG::LowerDFGToB3::compileArrayIndexOf):
+        (JSC::FTL::DFG::LowerDFGToB3::compileArrayPop):
+        (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
+        (JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
+        (JSC::FTL::DFG::LowerDFGToB3::compileStringCodePointAt):
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetByOffset):
+        (JSC::FTL::DFG::LowerDFGToB3::compileMultiGetByOffset):
+
 2020-04-15  Keith Miller  <[email protected]>
 
         Disable Store-load pair auto-vectorization for JSC

Modified: trunk/Source/_javascript_Core/ftl/FTLJITCode.cpp (260179 => 260180)


--- trunk/Source/_javascript_Core/ftl/FTLJITCode.cpp	2020-04-16 11:45:15 UTC (rev 260179)
+++ trunk/Source/_javascript_Core/ftl/FTLJITCode.cpp	2020-04-16 12:06:49 UTC (rev 260180)
@@ -43,11 +43,15 @@
 JITCode::~JITCode()
 {
     if (FTL::shouldDumpDisassembly()) {
-        dataLog("Destroying FTL JIT code at ");
-        CommaPrinter comma;
-        dataLog(comma, m_b3Code);
-        dataLog(comma, m_arityCheckEntrypoint);
-        dataLog("\n");
+        if (m_b3Code || m_arityCheckEntrypoint) {
+            dataLog("Destroying FTL JIT code at ");
+            CommaPrinter comma;
+            if (m_b3Code)
+                dataLog(comma, m_b3Code);
+            if (m_arityCheckEntrypoint)
+                dataLog(comma, m_arityCheckEntrypoint);
+            dataLog("\n");
+        }
     }
 }

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (260179 => 260180)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2020-04-16 11:45:15 UTC (rev 260179)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2020-04-16 12:06:49 UTC (rev 260180)
@@ -4353,6 +4353,9 @@
                         isHole, m_out.constInt64(JSValue::encode(jsUndefined())), result);
                 } else
                     speculate(LoadFromHole, noValue(), 0, isHole);
+                // We have to keep base alive to keep content in storage alive.
+                if (m_node->arrayMode().type() == Array::Contiguous)
+                    ensureStillAliveHere(base);
                 setJSValue(result);
                 return;
             }
@@ -4378,6 +4381,9 @@
             m_out.jump(continuation);
             
             m_out.appendTo(continuation, lastNext);
+            // We have to keep base alive to keep content in storage alive.
+            if (m_node->arrayMode().type() == Array::Contiguous)
+                ensureStillAliveHere(base);
             setJSValue(m_out.phi(Int64, fastResult, slowResult));
             return;
         }
@@ -4656,8 +4662,10 @@
             if (m_node->arrayMode().isInBounds()) {
                 LValue result = m_out.load64(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1)));
                 speculate(LoadFromHole, noValue(), 0, m_out.isZero64(result));
+                // We have to keep base alive to keep content in storage alive.
+                ensureStillAliveHere(base);
                 setJSValue(result);
-                break;
+                return;
             }
 
             LBasicBlock inBounds = m_out.newBlock();
@@ -4681,6 +4689,8 @@
             m_out.jump(continuation);
 
             m_out.appendTo(continuation, lastNext);
+            // We have to keep base alive to keep content in storage alive.
+            ensureStillAliveHere(base);
             setJSValue(m_out.phi(Int64, fastResult, slowResult));
             return;
         }
@@ -5651,6 +5661,7 @@
     void compileArrayIndexOf()
     {
         JSGlobalObject* globalObject = m_graph.globalObjectFor(m_node->origin.semantic);
+        LValue base = lowCell(m_graph.varArgChild(m_node, 0));
         LValue storage = lowStorage(m_node->numChildren() == 3 ? m_graph.varArgChild(m_node, 2) : m_graph.varArgChild(m_node, 3));
         LValue length = m_out.load32(storage, m_heaps.Butterfly_publicLength);
 
@@ -5753,33 +5764,40 @@
             m_out.jump(continuation);
 
             m_out.appendTo(continuation, lastNext);
+            // We have to keep base alive since that keeps content of storage alive.
+            ensureStillAliveHere(base);
             setInt32(m_out.castToInt32(m_out.phi(pointerType(), notFoundResult, foundResult)));
-            break;
+            return;
         }
 
         case StringUse:
             ASSERT(m_node->arrayMode().type() == Array::Contiguous);
+            // We have to keep base alive since that keeps storage alive.
+            ensureStillAliveHere(base);
             setInt32(m_out.castToInt32(vmCall(Int64, operationArrayIndexOfString, weakPointer(globalObject), storage, lowString(searchElementEdge), startIndex)));
-            break;
+            return;
 
         case UntypedUse:
             switch (m_node->arrayMode().type()) {
             case Array::Double:
                 setInt32(m_out.castToInt32(vmCall(Int64, operationArrayIndexOfValueDouble, weakPointer(globalObject), storage, lowJSValue(searchElementEdge), startIndex)));
-                break;
+                return;
+            case Array::Contiguous:
+                // We have to keep base alive since that keeps content of storage alive.
+                ensureStillAliveHere(base);
+                FALLTHROUGH;
             case Array::Int32:
-            case Array::Contiguous:
                 setInt32(m_out.castToInt32(vmCall(Int64, operationArrayIndexOfValueInt32OrContiguous, weakPointer(globalObject), storage, lowJSValue(searchElementEdge), startIndex)));
-                break;
+                return;
             default:
                 RELEASE_ASSERT_NOT_REACHED();
-                break;
+                return;
             }
-            break;
+            return;
 
         default:
             RELEASE_ASSERT_NOT_REACHED();
-            break;
+            return;
         }
     }
 
@@ -5813,6 +5831,9 @@
             TypedPointer pointer = m_out.baseIndex(heap, storage, m_out.zeroExtPtr(newLength));
             if (m_node->arrayMode().type() != Array::Double) {
                 LValue result = m_out.load64(pointer);
+                // We have to keep base alive to keep content in storage alive.
+                if (m_node->arrayMode().type() == Array::Contiguous)
+                    ensureStillAliveHere(base);
                 m_out.store64(m_out.int64Zero, pointer);
                 results.append(m_out.anchor(result));
                 m_out.branch(
@@ -5858,6 +5879,8 @@
             m_out.appendTo(popCheckCase, fastCase);
             TypedPointer pointer = m_out.baseIndex(m_heaps.ArrayStorage_vector, storage, m_out.zeroExtPtr(newLength));
             LValue result = m_out.load64(pointer);
+            // We have to keep base alive to keep content in storage alive.
+            ensureStillAliveHere(base);
             m_out.branch(m_out.notZero64(result), usually(fastCase), rarely(slowCase));
 
             m_out.appendTo(fastCase, slowCase);
@@ -7816,6 +7839,8 @@
         m_out.jump(continuation);
             
         m_out.appendTo(continuation, lastNext);
+        // We have to keep base alive since that keeps storage alive.
+        ensureStillAliveHere(base);
         setJSValue(m_out.phi(Int64, results));
     }
     
@@ -7862,6 +7887,8 @@
         
         m_out.appendTo(continuation, lastNext);
         
+        // We have to keep base alive since that keeps storage alive.
+        ensureStillAliveHere(base);
         setInt32(m_out.phi(Int32, char8Bit, char16Bit));
     }
 
@@ -7920,6 +7947,8 @@
         m_out.jump(continuation);
 
         m_out.appendTo(continuation, lastNext);
+        // We have to keep base alive since that keeps storage alive.
+        ensureStillAliveHere(base);
         setInt32(m_out.phi(Int32, char8Bit, char16Bit, charSurrogatePair));
     }
 
@@ -7972,8 +8001,11 @@
     {
         StorageAccessData& data = ""
         
-        setJSValue(loadProperty(
-            lowStorage(m_node->child1()), data.identifierNumber, data.offset));
+        LValue base = lowCell(m_node->child2());
+        LValue value = loadProperty(lowStorage(m_node->child1()), data.identifierNumber, data.offset);
+        // We have to keep base alive since that keeps content of storage alive.
+        ensureStillAliveHere(base);
+        setJSValue(value);
     }
     
     void compileGetGetter()
@@ -8055,6 +8087,8 @@
         m_out.unreachable();
         
         m_out.appendTo(continuation, lastNext);
+        // We have to keep base alive since that keeps storage alive.
+        ensureStillAliveHere(base);
         setJSValue(m_out.phi(Int64, results));
     }
     

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to