Log Message
Cherry-pick r260180. rdar://problem/61943707
[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):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@260180 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-609-branch/Source/_javascript_Core/ChangeLog (260285 => 260286)
--- branches/safari-609-branch/Source/_javascript_Core/ChangeLog 2020-04-17 21:34:32 UTC (rev 260285)
+++ branches/safari-609-branch/Source/_javascript_Core/ChangeLog 2020-04-17 21:34:36 UTC (rev 260286)
@@ -1,5 +1,57 @@
2020-04-17 Alan Coon <[email protected]>
+ Cherry-pick r260180. rdar://problem/61943707
+
+ [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):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@260180 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 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-17 Alan Coon <[email protected]>
+
Cherry-pick r259572. rdar://problem/61943713
[JSC] Put ensureStillAliveHere for Integer TypedArrays in GetByVal
Modified: branches/safari-609-branch/Source/_javascript_Core/ftl/FTLJITCode.cpp (260285 => 260286)
--- branches/safari-609-branch/Source/_javascript_Core/ftl/FTLJITCode.cpp 2020-04-17 21:34:32 UTC (rev 260285)
+++ branches/safari-609-branch/Source/_javascript_Core/ftl/FTLJITCode.cpp 2020-04-17 21:34:36 UTC (rev 260286)
@@ -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: branches/safari-609-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (260285 => 260286)
--- branches/safari-609-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-04-17 21:34:32 UTC (rev 260285)
+++ branches/safari-609-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-04-17 21:34:36 UTC (rev 260286)
@@ -4288,6 +4288,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;
}
@@ -4313,6 +4316,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;
}
@@ -4591,8 +4597,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();
@@ -4616,6 +4624,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;
}
@@ -5433,6 +5443,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);
@@ -5535,33 +5546,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(vmCall(Int32, operationArrayIndexOfString, weakPointer(globalObject), storage, lowString(searchElementEdge), startIndex));
- break;
+ return;
case UntypedUse:
switch (m_node->arrayMode().type()) {
case Array::Double:
setInt32(vmCall(Int32, 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(vmCall(Int32, 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;
}
}
@@ -5595,6 +5613,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(
@@ -5640,6 +5661,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);
@@ -7558,6 +7581,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));
}
@@ -7604,6 +7629,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));
}
@@ -7662,6 +7689,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));
}
@@ -7714,8 +7743,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()
@@ -7797,6 +7829,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
