Diff
Modified: trunk/JSTests/ChangeLog (242953 => 242954)
--- trunk/JSTests/ChangeLog 2019-03-14 19:22:06 UTC (rev 242953)
+++ trunk/JSTests/ChangeLog 2019-03-14 19:27:28 UTC (rev 242954)
@@ -1,3 +1,14 @@
+2019-03-14 Saam barati <[email protected]>
+
+ Fixup uses KnownInt32 incorrectly in some nodes
+ https://bugs.webkit.org/show_bug.cgi?id=195279
+ <rdar://problem/47915654>
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/known-int32-cant-be-used-across-bytecode-boundary.js: Added.
+ (foo):
+
2019-03-14 Keith Miller <[email protected]>
DFG liveness can't skip tail caller inline frames
Added: trunk/JSTests/stress/known-int32-cant-be-used-across-bytecode-boundary.js (0 => 242954)
--- trunk/JSTests/stress/known-int32-cant-be-used-across-bytecode-boundary.js (rev 0)
+++ trunk/JSTests/stress/known-int32-cant-be-used-across-bytecode-boundary.js 2019-03-14 19:27:28 UTC (rev 242954)
@@ -0,0 +1,26 @@
+//@ runDefault("--useConcurrentJIT=0", "--useMaximalFlushInsertionPhase=1")
+
+function foo() {
+ var x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12, x13;
+ var copy = [];
+ var value = this[1];
+
+ for (var p in this)
+ copy[copy.length] = value;
+
+ for (var i = 0; i < 1000; i++) {
+ for (var j = 0; j < 1; j++) {
+ }
+ Math.min(0 ** []);
+ }
+};
+
+noInline(foo);
+
+let array0 = new Array(3).fill(1);
+delete array0[0];
+
+([])[1000] = 0xFFFFF;
+
+for (var i = 0; i < 100; i++)
+ foo.call(array0);
Modified: trunk/Source/_javascript_Core/ChangeLog (242953 => 242954)
--- trunk/Source/_javascript_Core/ChangeLog 2019-03-14 19:22:06 UTC (rev 242953)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-03-14 19:27:28 UTC (rev 242954)
@@ -1,3 +1,27 @@
+2019-03-14 Saam barati <[email protected]>
+
+ Fixup uses KnownInt32 incorrectly in some nodes
+ https://bugs.webkit.org/show_bug.cgi?id=195279
+ <rdar://problem/47915654>
+
+ Reviewed by Yusuke Suzuki.
+
+ Fixup was sometimes using KnownInt32 edges when it knew some
+ incoming value is an Int32 based on what the bytecode would return.
+ However, because bytecode may result in Int32 for some node does
+ not mean we'll pick Int32 as the value format for that local. For example,
+ we may choose for a value to be represented as a double. This patch
+ corrects such uses of KnownInt32.
+
+ * dfg/DFGArgumentsEliminationPhase.cpp:
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileArrayPush):
+ (JSC::DFG::SpeculativeJIT::compileGetDirectPname):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileArrayPush):
+
2019-03-14 Keith Miller <[email protected]>
DFG liveness can't skip tail caller inline frames
Modified: trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp (242953 => 242954)
--- trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2019-03-14 19:22:06 UTC (rev 242953)
+++ trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2019-03-14 19:27:28 UTC (rev 242954)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2019 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -661,6 +661,12 @@
if (!m_candidates.contains(node))
break;
+ ASSERT(node->origin.exitOK);
+ ASSERT(node->child1().useKind() == Int32Use);
+ insertionSet.insertNode(
+ nodeIndex, SpecNone, Check, node->origin,
+ node->child1());
+
node->setOpAndDefaultFlags(PhantomCreateRest);
// We don't need this parameter for OSR exit, we can find out all the information
// we need via the static parameter count and the dynamic argument count.
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (242953 => 242954)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2019-03-14 19:22:06 UTC (rev 242953)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2019-03-14 19:27:28 UTC (rev 242954)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2019 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -1149,12 +1149,10 @@
Edge& element = m_graph.varArgChild(node, i + elementOffset);
switch (node->arrayMode().type()) {
case Array::Int32:
- insertCheck<Int32Use>(element.node());
- fixEdge<KnownInt32Use>(element);
+ fixEdge<Int32Use>(element);
break;
case Array::Double:
- insertCheck<DoubleRepRealUse>(element.node());
- fixEdge<DoubleRepUse>(element);
+ fixEdge<DoubleRepRealUse>(element);
break;
case Array::Contiguous:
case Array::ArrayStorage:
@@ -1163,7 +1161,6 @@
default:
break;
}
- ASSERT(shouldNotHaveTypeCheck(element.useKind()));
}
break;
}
@@ -1868,7 +1865,7 @@
blessArrayOperation(m_graph.varArgChild(node, 0), m_graph.varArgChild(node, 1), m_graph.varArgChild(node, 2));
fixEdge<CellUse>(m_graph.varArgChild(node, 0));
- fixEdge<KnownInt32Use>(m_graph.varArgChild(node, 1));
+ fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
break;
}
case GetDirectPname: {
@@ -1878,7 +1875,7 @@
Edge& enumerator = m_graph.varArgChild(node, 3);
fixEdge<CellUse>(base);
fixEdge<KnownCellUse>(property);
- fixEdge<KnownInt32Use>(index);
+ fixEdge<Int32Use>(index);
fixEdge<KnownCellUse>(enumerator);
break;
}
@@ -1889,16 +1886,16 @@
}
case GetEnumeratorStructurePname: {
fixEdge<KnownCellUse>(node->child1());
- fixEdge<KnownInt32Use>(node->child2());
+ fixEdge<Int32Use>(node->child2());
break;
}
case GetEnumeratorGenericPname: {
fixEdge<KnownCellUse>(node->child1());
- fixEdge<KnownInt32Use>(node->child2());
+ fixEdge<Int32Use>(node->child2());
break;
}
case ToIndexString: {
- fixEdge<KnownInt32Use>(node->child1());
+ fixEdge<Int32Use>(node->child1());
break;
}
case ProfileType: {
@@ -1990,7 +1987,7 @@
case CreateRest: {
watchHavingABadTime(node);
- fixEdge<KnownInt32Use>(node->child1());
+ fixEdge<Int32Use>(node->child1());
break;
}
@@ -2154,7 +2151,7 @@
else
fixEdge<UntypedUse>(propertyEdge);
fixEdge<UntypedUse>(m_graph.varArgChild(node, 2));
- fixEdge<KnownInt32Use>(m_graph.varArgChild(node, 3));
+ fixEdge<Int32Use>(m_graph.varArgChild(node, 3));
break;
}
@@ -2204,7 +2201,7 @@
fixEdge<UntypedUse>(propertyEdge);
fixEdge<CellUse>(m_graph.varArgChild(node, 2));
fixEdge<CellUse>(m_graph.varArgChild(node, 3));
- fixEdge<KnownInt32Use>(m_graph.varArgChild(node, 4));
+ fixEdge<Int32Use>(m_graph.varArgChild(node, 4));
break;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (242953 => 242954)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2019-03-14 19:22:06 UTC (rev 242953)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2019-03-14 19:27:28 UTC (rev 242954)
@@ -8570,12 +8570,13 @@
case Array::Contiguous: {
if (elementCount == 1) {
Edge& element = m_jit.graph().varArgChild(node, elementOffset);
+ if (node->arrayMode().type() == Array::Int32) {
+ ASSERT(element.useKind() == Int32Use);
+ speculateInt32(element);
+ }
JSValueOperand value(this, element, ManualOperandSpeculation);
JSValueRegs valueRegs = value.jsValueRegs();
- if (node->arrayMode().type() == Array::Int32)
- DFG_ASSERT(m_jit.graph(), node, !needsTypeCheck(element, SpecInt32Only));
-
m_jit.load32(MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()), storageLengthGPR);
MacroAssembler::Jump slowPath = m_jit.branch32(MacroAssembler::AboveOrEqual, storageLengthGPR, MacroAssembler::Address(storageGPR, Butterfly::offsetOfVectorLength()));
m_jit.storeValue(valueRegs, MacroAssembler::BaseIndex(storageGPR, storageLengthGPR, MacroAssembler::TimesEight));
@@ -8590,6 +8591,14 @@
return;
}
+ if (node->arrayMode().type() == Array::Int32) {
+ for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
+ Edge element = m_jit.graph().varArgChild(node, elementIndex + elementOffset);
+ ASSERT(element.useKind() == Int32Use);
+ speculateInt32(element);
+ }
+ }
+
GPRTemporary buffer(this);
GPRReg bufferGPR = buffer.gpr();
@@ -8615,12 +8624,9 @@
storageDone.link(&m_jit);
for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
Edge& element = m_jit.graph().varArgChild(node, elementIndex + elementOffset);
- JSValueOperand value(this, element, ManualOperandSpeculation);
+ JSValueOperand value(this, element, ManualOperandSpeculation); // We did type checks above.
JSValueRegs valueRegs = value.jsValueRegs();
- if (node->arrayMode().type() == Array::Int32)
- DFG_ASSERT(m_jit.graph(), node, !needsTypeCheck(element, SpecInt32Only));
-
m_jit.storeValue(valueRegs, MacroAssembler::Address(bufferGPR, sizeof(EncodedJSValue) * elementIndex));
value.use();
}
@@ -8643,11 +8649,10 @@
case Array::Double: {
if (elementCount == 1) {
Edge& element = m_jit.graph().varArgChild(node, elementOffset);
+ speculate(node, element);
SpeculateDoubleOperand value(this, element);
FPRReg valueFPR = value.fpr();
- DFG_ASSERT(m_jit.graph(), node, !needsTypeCheck(element, SpecDoubleReal));
-
m_jit.load32(MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()), storageLengthGPR);
MacroAssembler::Jump slowPath = m_jit.branch32(MacroAssembler::AboveOrEqual, storageLengthGPR, MacroAssembler::Address(storageGPR, Butterfly::offsetOfVectorLength()));
m_jit.storeDouble(valueFPR, MacroAssembler::BaseIndex(storageGPR, storageLengthGPR, MacroAssembler::TimesEight));
@@ -8662,6 +8667,12 @@
return;
}
+ for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
+ Edge element = m_jit.graph().varArgChild(node, elementIndex + elementOffset);
+ ASSERT(element.useKind() == DoubleRepRealUse);
+ speculate(node, element);
+ }
+
GPRTemporary buffer(this);
GPRReg bufferGPR = buffer.gpr();
@@ -8690,8 +8701,6 @@
SpeculateDoubleOperand value(this, element);
FPRReg valueFPR = value.fpr();
- DFG_ASSERT(m_jit.graph(), node, !needsTypeCheck(element, SpecDoubleReal));
-
m_jit.storeDouble(valueFPR, MacroAssembler::Address(bufferGPR, sizeof(double) * elementIndex));
value.use();
}
@@ -13086,6 +13095,7 @@
{
Edge& baseEdge = m_jit.graph().varArgChild(node, 0);
Edge& propertyEdge = m_jit.graph().varArgChild(node, 1);
+ Edge& indexEdge = m_jit.graph().varArgChild(node, 2);
SpeculateCellOperand base(this, baseEdge);
SpeculateCellOperand property(this, propertyEdge);
@@ -13094,6 +13104,7 @@
#if CPU(X86)
// Not enough registers on X86 for this code, so always use the slow path.
+ speculate(node, indexEdge);
flushRegisters();
JSValueRegsFlushedCallResult result(this);
JSValueRegs resultRegs = result.regs();
@@ -13101,7 +13112,6 @@
m_jit.exceptionCheck();
jsValueResult(resultRegs, node);
#else
- Edge& indexEdge = m_jit.graph().varArgChild(node, 2);
Edge& enumeratorEdge = m_jit.graph().varArgChild(node, 3);
SpeculateStrictInt32Operand index(this, indexEdge);
SpeculateCellOperand enumerator(this, enumeratorEdge);
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (242953 => 242954)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2019-03-14 19:22:06 UTC (rev 242953)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2019-03-14 19:27:28 UTC (rev 242954)
@@ -4674,14 +4674,12 @@
Output::StoreType storeType;
Edge& element = m_graph.varArgChild(m_node, elementOffset);
+ speculate(element);
if (m_node->arrayMode().type() != Array::Double) {
value = lowJSValue(element, ManualOperandSpeculation);
- if (m_node->arrayMode().type() == Array::Int32)
- DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecInt32Only));
storeType = Output::Store64;
} else {
value = lowDouble(element);
- DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecDoubleReal));
storeType = Output::StoreDouble;
}
@@ -4720,6 +4718,11 @@
return;
}
+ for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
+ Edge element = m_graph.varArgChild(m_node, elementIndex + elementOffset);
+ speculate(element);
+ }
+
LValue prevLength = m_out.load32(storage, m_heaps.Butterfly_publicLength);
LValue newLength = m_out.add(prevLength, m_out.constInt32(elementCount));
@@ -4756,12 +4759,9 @@
Output::StoreType storeType;
if (m_node->arrayMode().type() != Array::Double) {
value = lowJSValue(element, ManualOperandSpeculation);
- if (m_node->arrayMode().type() == Array::Int32)
- DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecInt32Only));
storeType = Output::Store64;
} else {
value = lowDouble(element);
- DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecDoubleReal));
storeType = Output::StoreDouble;
}
