Diff
Modified: releases/WebKitGTK/webkit-2.20/JSTests/ChangeLog (230432 => 230433)
--- releases/WebKitGTK/webkit-2.20/JSTests/ChangeLog 2018-04-09 15:46:50 UTC (rev 230432)
+++ releases/WebKitGTK/webkit-2.20/JSTests/ChangeLog 2018-04-09 15:47:01 UTC (rev 230433)
@@ -1,3 +1,17 @@
+2018-04-04 Filip Pizlo <fpi...@apple.com>
+
+ REGRESSION(r222563): removed DoubleReal type check causes tons of crashes because CSE has never known how to handle SaneChain
+ https://bugs.webkit.org/show_bug.cgi?id=184319
+
+ Reviewed by Saam Barati.
+
+ * stress/array-push-nan-to-double-array-cse-sane-and-insane-chain.js: Added.
+ (foo):
+ (bar):
+ * stress/array-push-nan-to-double-array.js: Added.
+ (foo):
+ (bar):
+
2018-03-31 Filip Pizlo <fpi...@apple.com>
JSC crash in JIT code with for-of loop and Array/Set iterators
Added: releases/WebKitGTK/webkit-2.20/JSTests/stress/array-push-nan-to-double-array-cse-sane-and-insane-chain.js (0 => 230433)
--- releases/WebKitGTK/webkit-2.20/JSTests/stress/array-push-nan-to-double-array-cse-sane-and-insane-chain.js (rev 0)
+++ releases/WebKitGTK/webkit-2.20/JSTests/stress/array-push-nan-to-double-array-cse-sane-and-insane-chain.js 2018-04-09 15:47:01 UTC (rev 230433)
@@ -0,0 +1,23 @@
+function foo(array, otherArray, i)
+{
+ var x = otherArray[i];
+ var y = otherArray[i];
+ array.push(y);
+ return x / 42;
+}
+
+function bar()
+{
+ return [];
+}
+
+noInline(foo);
+noInline(bar);
+
+for (var i = 0; i < 10000; ++i)
+ foo(bar(), [42.5], 0);
+
+var result = bar();
+foo(result, [,42.5], 0);
+if (result[0] !== void 0)
+ throw "Bad result: " + result;
Added: releases/WebKitGTK/webkit-2.20/JSTests/stress/array-push-nan-to-double-array.js (0 => 230433)
--- releases/WebKitGTK/webkit-2.20/JSTests/stress/array-push-nan-to-double-array.js (rev 0)
+++ releases/WebKitGTK/webkit-2.20/JSTests/stress/array-push-nan-to-double-array.js 2018-04-09 15:47:01 UTC (rev 230433)
@@ -0,0 +1,20 @@
+function foo(array, num, denum)
+{
+ array.push(num/denum);
+}
+
+function bar()
+{
+ return [];
+}
+
+noInline(foo);
+noInline(bar);
+
+for (var i = 0; i < 10000; ++i)
+ foo(bar(), 42.5, 3.1);
+
+var result = bar();
+foo(result, 0, 0);
+if ("" + result[0] !== "NaN")
+ throw "Bad result: " + result;
Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog (230432 => 230433)
--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog 2018-04-09 15:46:50 UTC (rev 230432)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog 2018-04-09 15:47:01 UTC (rev 230433)
@@ -1,3 +1,32 @@
+2018-04-04 Filip Pizlo <fpi...@apple.com>
+
+ REGRESSION(r222563): removed DoubleReal type check causes tons of crashes because CSE has never known how to handle SaneChain
+ https://bugs.webkit.org/show_bug.cgi?id=184319
+
+ Reviewed by Saam Barati.
+
+ In r222581, we replaced type checks about DoubleReal in ArrayPush in the DFG/FTL backends with
+ assertions. That's correct because FixupPhase was emitting those checks as Check(DoubleRealRep:) before
+ the ArrayPush.
+
+ But this revealed a longstanding CSE bug: CSE will happily match a SaneChain GetByVal with a InBounds
+ GetByVal. SaneChain can return NaN while InBounds cannot. This means that if we first use AI to
+ eliminate the Check(DoubleRealRep:) based on the input being a GetByVal(InBounds) but then replace that
+ with a GetByVal(SaneChain), then we will hit the assertion.
+
+ This teaches CSE to not replace GetByVal(InBounds) with GetByVal(SaneChain) and vice versa. That gets
+ tricky because PutByVal can match either. So, we use the fact that it's legal for a store to def() more
+ than once: PutByVal now defs() a HeapLocation for InBounds and a HeapLocation for SaneChain.
+
+ * dfg/DFGCSEPhase.cpp:
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGHeapLocation.cpp:
+ (WTF::printInternal):
+ * dfg/DFGHeapLocation.h:
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileArrayPush):
+
2018-04-03 Filip Pizlo <fpi...@apple.com>
JSArray::appendMemcpy seems to be missing a barrier
Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGCSEPhase.cpp (230432 => 230433)
--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2018-04-09 15:46:50 UTC (rev 230432)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2018-04-09 15:47:01 UTC (rev 230433)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -469,22 +469,21 @@
case Array::Int32:
if (!mode.isInBounds())
break;
- heap = HeapLocation(
- indexedPropertyLoc, IndexedInt32Properties, base, index);
+ heap = HeapLocation(indexedPropertyLoc, IndexedInt32Properties, base, index);
break;
- case Array::Double:
+ case Array::Double: {
if (!mode.isInBounds())
break;
- heap = HeapLocation(
- indexedPropertyLoc, IndexedDoubleProperties, base, index);
+ LocationKind kind = mode.isSaneChain() ? IndexedPropertyDoubleSaneChainLoc : IndexedPropertyDoubleLoc;
+ heap = HeapLocation(kind, IndexedDoubleProperties, base, index);
break;
+ }
case Array::Contiguous:
if (!mode.isInBounds())
break;
- heap = HeapLocation(
- indexedPropertyLoc, IndexedContiguousProperties, base, index);
+ heap = HeapLocation(indexedPropertyLoc, IndexedContiguousProperties, base, index);
break;
case Array::Int8Array:
Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGClobberize.h (230432 => 230433)
--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGClobberize.h 2018-04-09 15:46:50 UTC (rev 230432)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGClobberize.h 2018-04-09 15:47:01 UTC (rev 230433)
@@ -825,7 +825,8 @@
if (mode.isInBounds()) {
read(Butterfly_publicLength);
read(IndexedDoubleProperties);
- def(HeapLocation(indexedPropertyLoc, IndexedDoubleProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
+ LocationKind kind = mode.isSaneChain() ? IndexedPropertyDoubleSaneChainLoc : IndexedPropertyDoubleLoc;
+ def(HeapLocation(kind, IndexedDoubleProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
return;
}
read(World);
@@ -944,7 +945,8 @@
write(IndexedDoubleProperties);
if (node->arrayMode().mayStoreToHole())
write(Butterfly_publicLength);
- def(HeapLocation(indexedPropertyLoc, IndexedDoubleProperties, base, index), LazyNode(value));
+ def(HeapLocation(IndexedPropertyDoubleLoc, IndexedDoubleProperties, base, index), LazyNode(value));
+ def(HeapLocation(IndexedPropertyDoubleSaneChainLoc, IndexedDoubleProperties, base, index), LazyNode(value));
return;
case Array::Contiguous:
Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGHeapLocation.cpp (230432 => 230433)
--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGHeapLocation.cpp 2018-04-09 15:46:50 UTC (rev 230432)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGHeapLocation.cpp 2018-04-09 15:47:01 UTC (rev 230433)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2014-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -128,6 +128,10 @@
out.print("IndexedPropertyDoubleLoc");
return;
+ case IndexedPropertyDoubleSaneChainLoc:
+ out.print("IndexedPropertyDoubleSaneChainLoc");
+ return;
+
case IndexedPropertyInt52Loc:
out.print("IndexedPropertyInt52Loc");
return;
Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGHeapLocation.h (230432 => 230433)
--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGHeapLocation.h 2018-04-09 15:46:50 UTC (rev 230432)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGHeapLocation.h 2018-04-09 15:47:01 UTC (rev 230433)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2014-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -48,6 +48,7 @@
GlobalVariableLoc,
HasIndexedPropertyLoc,
IndexedPropertyDoubleLoc,
+ IndexedPropertyDoubleSaneChainLoc,
IndexedPropertyInt52Loc,
IndexedPropertyJSLoc,
IndexedPropertyStorageLoc,
Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (230432 => 230433)
--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-04-09 15:46:50 UTC (rev 230432)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-04-09 15:47:01 UTC (rev 230433)
@@ -7965,7 +7965,7 @@
JSValueRegs valueRegs = value.jsValueRegs();
if (node->arrayMode().type() == Array::Int32)
- RELEASE_ASSERT(!needsTypeCheck(element, SpecInt32Only));
+ 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()));
@@ -8010,7 +8010,7 @@
JSValueRegs valueRegs = value.jsValueRegs();
if (node->arrayMode().type() == Array::Int32)
- RELEASE_ASSERT(!needsTypeCheck(element, SpecInt32Only));
+ DFG_ASSERT(m_jit.graph(), node, !needsTypeCheck(element, SpecInt32Only));
m_jit.storeValue(valueRegs, MacroAssembler::Address(bufferGPR, sizeof(EncodedJSValue) * elementIndex));
value.use();
@@ -8037,7 +8037,7 @@
SpeculateDoubleOperand value(this, element);
FPRReg valueFPR = value.fpr();
- RELEASE_ASSERT(!needsTypeCheck(element, SpecDoubleReal));
+ 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()));
@@ -8081,7 +8081,7 @@
SpeculateDoubleOperand value(this, element);
FPRReg valueFPR = value.fpr();
- RELEASE_ASSERT(!needsTypeCheck(element, SpecDoubleReal));
+ DFG_ASSERT(m_jit.graph(), node, !needsTypeCheck(element, SpecDoubleReal));
m_jit.storeDouble(valueFPR, MacroAssembler::Address(bufferGPR, sizeof(double) * elementIndex));
value.use();
Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (230432 => 230433)
--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-04-09 15:46:50 UTC (rev 230432)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-04-09 15:47:01 UTC (rev 230433)
@@ -4447,11 +4447,11 @@
if (m_node->arrayMode().type() != Array::Double) {
value = lowJSValue(element, ManualOperandSpeculation);
if (m_node->arrayMode().type() == Array::Int32)
- RELEASE_ASSERT(!m_interpreter.needsTypeCheck(element, SpecInt32Only));
+ DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecInt32Only));
storeType = Output::Store64;
} else {
value = lowDouble(element);
- RELEASE_ASSERT(!m_interpreter.needsTypeCheck(element, SpecDoubleReal));
+ DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecDoubleReal));
storeType = Output::StoreDouble;
}
@@ -4527,11 +4527,11 @@
if (m_node->arrayMode().type() != Array::Double) {
value = lowJSValue(element, ManualOperandSpeculation);
if (m_node->arrayMode().type() == Array::Int32)
- RELEASE_ASSERT(!m_interpreter.needsTypeCheck(element, SpecInt32Only));
+ DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecInt32Only));
storeType = Output::Store64;
} else {
value = lowDouble(element);
- RELEASE_ASSERT(!m_interpreter.needsTypeCheck(element, SpecDoubleReal));
+ DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecDoubleReal));
storeType = Output::StoreDouble;
}
