- Revision
- 219187
- Author
- sbar...@apple.com
- Date
- 2017-07-05 22:01:33 -0700 (Wed, 05 Jul 2017)
Log Message
NewArray in FTLLowerDFGToB3 does not handle speculating on doubles when having a bad time
https://bugs.webkit.org/show_bug.cgi?id=174188
<rdar://problem/30581423>
Reviewed by Mark Lam.
JSTests:
* stress/new-array-having-a-bad-time-double.js: Added.
(assert):
(foo):
Source/_javascript_Core:
We were calling lowJSValue(edge) when we were speculating the
edge as double. This isn't allowed. We should have been using
lowDouble.
This patch also adds a new option, called useArrayAllocationProfiling,
which defaults to true. When false, it will make the array allocation
profile not actually sample seen arrays. It'll force the allocation
profile's predicted indexing type to be ArrayWithUndecided. Adding
this option made it trivial to write a test for this bug.
* bytecode/ArrayAllocationProfile.cpp:
(JSC::ArrayAllocationProfile::updateIndexingType):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNewArray):
* runtime/Options.h:
Tools:
* Scripts/run-jsc-stress-tests:
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (219186 => 219187)
--- trunk/JSTests/ChangeLog 2017-07-06 04:51:10 UTC (rev 219186)
+++ trunk/JSTests/ChangeLog 2017-07-06 05:01:33 UTC (rev 219187)
@@ -1,3 +1,15 @@
+2017-07-05 Saam Barati <sbar...@apple.com>
+
+ NewArray in FTLLowerDFGToB3 does not handle speculating on doubles when having a bad time
+ https://bugs.webkit.org/show_bug.cgi?id=174188
+ <rdar://problem/30581423>
+
+ Reviewed by Mark Lam.
+
+ * stress/new-array-having-a-bad-time-double.js: Added.
+ (assert):
+ (foo):
+
2017-07-05 Yusuke Suzuki <utatane....@gmail.com>
WTF::StringImpl::copyChars segfaults when built with GCC 7
Added: trunk/JSTests/stress/new-array-having-a-bad-time-double.js (0 => 219187)
--- trunk/JSTests/stress/new-array-having-a-bad-time-double.js (rev 0)
+++ trunk/JSTests/stress/new-array-having-a-bad-time-double.js 2017-07-06 05:01:33 UTC (rev 219187)
@@ -0,0 +1,25 @@
+function assert(b) {
+ if (!b)
+ throw new Error("Bad");
+}
+
+function foo(a, b, c) {
+ let x = a + b + c;
+ return [a, b, c, x];
+}
+noInline(foo);
+
+Object.defineProperty(Object.prototype, "10000", {get() { return 20; }});
+
+for (let i = 0; i < 10000; ++i) {
+ let a = 10.5;
+ let b = 1.1;
+ let c = 1.2;
+ let x = a + b + c;
+ let result = foo(a, b, c);
+ assert(result.length === 4);
+ assert(result[0] === a);
+ assert(result[1] === b);
+ assert(result[2] === c);
+ assert(result[3] === x);
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (219186 => 219187)
--- trunk/Source/_javascript_Core/ChangeLog 2017-07-06 04:51:10 UTC (rev 219186)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-07-06 05:01:33 UTC (rev 219187)
@@ -1,3 +1,27 @@
+2017-07-05 Saam Barati <sbar...@apple.com>
+
+ NewArray in FTLLowerDFGToB3 does not handle speculating on doubles when having a bad time
+ https://bugs.webkit.org/show_bug.cgi?id=174188
+ <rdar://problem/30581423>
+
+ Reviewed by Mark Lam.
+
+ We were calling lowJSValue(edge) when we were speculating the
+ edge as double. This isn't allowed. We should have been using
+ lowDouble.
+
+ This patch also adds a new option, called useArrayAllocationProfiling,
+ which defaults to true. When false, it will make the array allocation
+ profile not actually sample seen arrays. It'll force the allocation
+ profile's predicted indexing type to be ArrayWithUndecided. Adding
+ this option made it trivial to write a test for this bug.
+
+ * bytecode/ArrayAllocationProfile.cpp:
+ (JSC::ArrayAllocationProfile::updateIndexingType):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileNewArray):
+ * runtime/Options.h:
+
2017-07-05 Yusuke Suzuki <utatane....@gmail.com>
WTF::Thread should have the threads stack bounds.
Modified: trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.cpp (219186 => 219187)
--- trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.cpp 2017-07-06 04:51:10 UTC (rev 219186)
+++ trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.cpp 2017-07-06 05:01:33 UTC (rev 219187)
@@ -49,7 +49,8 @@
JSArray* lastArray = m_lastArray;
if (!lastArray)
return;
- m_currentIndexingType = leastUpperBoundOfIndexingTypes(m_currentIndexingType, lastArray->indexingType());
+ if (LIKELY(Options::useArrayAllocationProfiling()))
+ m_currentIndexingType = leastUpperBoundOfIndexingTypes(m_currentIndexingType, lastArray->indexingType());
m_lastArray = 0;
}
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (219186 => 219187)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-07-06 04:51:10 UTC (rev 219186)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-07-06 05:01:33 UTC (rev 219187)
@@ -4648,9 +4648,16 @@
for (unsigned operandIndex = 0; operandIndex < m_node->numChildren(); ++operandIndex) {
Edge edge = m_graph.varArgChild(m_node, operandIndex);
- m_out.store64(
- lowJSValue(edge, ManualOperandSpeculation),
- m_out.absolute(buffer + operandIndex));
+ LValue valueToStore;
+ switch (m_node->indexingType()) {
+ case ALL_DOUBLE_INDEXING_TYPES:
+ valueToStore = boxDouble(lowDouble(edge));
+ break;
+ default:
+ valueToStore = lowJSValue(edge, ManualOperandSpeculation);
+ break;
+ }
+ m_out.store64(valueToStore, m_out.absolute(buffer + operandIndex));
}
m_out.storePtr(
Modified: trunk/Source/_javascript_Core/runtime/Options.h (219186 => 219187)
--- trunk/Source/_javascript_Core/runtime/Options.h 2017-07-06 04:51:10 UTC (rev 219186)
+++ trunk/Source/_javascript_Core/runtime/Options.h 2017-07-06 05:01:33 UTC (rev 219187)
@@ -464,7 +464,8 @@
v(bool, useWebAssemblyFastTLS, true, Normal, "If true, we will try to use fast thread-local storage if available on the current platform.") \
v(bool, useFastTLSForWasmContext, true, Normal, "If true (and fast TLS is enabled), we will store context in fast TLS. If false, we will pin it to a register.") \
v(bool, useCallICsForWebAssemblyToJSCalls, true, Normal, "If true, we will use CallLinkInfo to inline cache Wasm to JS calls.") \
- v(bool, useObjectRestSpread, true, Normal, "If true, we will enable Object Rest/Spread feature.")
+ v(bool, useObjectRestSpread, true, Normal, "If true, we will enable Object Rest/Spread feature.") \
+ v(bool, useArrayAllocationProfiling, true, Normal, "If true, we will use our normal array allocation profiling. If false, the allocation profile will always claim to be undecided.")
enum OptionEquivalence {
Modified: trunk/Tools/ChangeLog (219186 => 219187)
--- trunk/Tools/ChangeLog 2017-07-06 04:51:10 UTC (rev 219186)
+++ trunk/Tools/ChangeLog 2017-07-06 05:01:33 UTC (rev 219187)
@@ -1,3 +1,13 @@
+2017-07-05 Saam Barati <sbar...@apple.com>
+
+ NewArray in FTLLowerDFGToB3 does not handle speculating on doubles when having a bad time
+ https://bugs.webkit.org/show_bug.cgi?id=174188
+ <rdar://problem/30581423>
+
+ Reviewed by Mark Lam.
+
+ * Scripts/run-jsc-stress-tests:
+
2017-07-05 Yusuke Suzuki <utatane....@gmail.com>
[WTF] Clean up StringStatics.cpp by using LazyNeverDestroyed<> for Atoms
Modified: trunk/Tools/Scripts/run-jsc-stress-tests (219186 => 219187)
--- trunk/Tools/Scripts/run-jsc-stress-tests 2017-07-06 04:51:10 UTC (rev 219186)
+++ trunk/Tools/Scripts/run-jsc-stress-tests 2017-07-06 05:01:33 UTC (rev 219187)
@@ -859,7 +859,7 @@
end
def runFTLNoCJITB3O1(*optionalTestSpecificOptions)
- run("ftl-no-cjit-b3o1", *(FTL_OPTIONS + NO_CJIT_OPTIONS + B3O1_OPTIONS + optionalTestSpecificOptions))
+ run("ftl-no-cjit-b3o1", "--useArrayAllocationProfiling=false", *(FTL_OPTIONS + NO_CJIT_OPTIONS + B3O1_OPTIONS + optionalTestSpecificOptions))
end
def runFTLNoCJITValidate(*optionalTestSpecificOptions)
