Eager FTL failure for strict comparison of NaN with number check

https://bugs.webkit.org/show_bug.cgi?id=158368 Patch by Benjamin Poulain <[email protected]> on 2016-06-03 Reviewed by Darin Adler. DoupleRep with a RealNumberUse starts by handling double then falls back to Int32 if the unboxed double is NaN. Before handling integers, the code is checking if the input is indeed an int32. The problem was that this check failed to account for NaN as an original input of the DoubleRep. The call to isNotInt32() filter the doubles checks because that was handled by the previous block. The problem is the previous block handles any double except NaN. If the original input was NaN, the masking by "~SpecFullDouble" filter that possibility and isNotInt32() fails to test that case. This patch fixes the issue by changing the filter to SpecDoubleReal. The type SpecDoubleReal does not include the NaN types. * ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileDoubleRep): * tests/stress/double-rep-real-number-use-on-nan.js: Added. To ensure the isNotInt32() does not test anything, we want proven numbers as input. The (+value) are there to enforce a ToNumber() which in turn give us a proven Number type. Canonical link: https://commits.webkit.org/176464@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@201678 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jun 4, 2016 · 46870df · 46870df
1 parent 5fa0bb3
commit 46870df
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 2 deletions.
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,33 @@
+2016-06-03 Benjamin Poulain <[email protected]>
+
+ Eager FTL failure for strict comparison of NaN with number check
+ https://bugs.webkit.org/show_bug.cgi?id=158368
+
+ Reviewed by Darin Adler.
+
+ DoupleRep with a RealNumberUse starts by handling double
+ then falls back to Int32 if the unboxed double is NaN.
+
+ Before handling integers, the code is checking if the input
+ is indeed an int32. The problem was that this check failed
+ to account for NaN as an original input of the DoubleRep.
+
+ The call to isNotInt32() filter the doubles checks because
+ that was handled by the previous block.
+ The problem is the previous block handles any double except NaN.
+ If the original input was NaN, the masking by "~SpecFullDouble"
+ filter that possibility and isNotInt32() fails to test that case.
+
+ This patch fixes the issue by changing the filter to SpecDoubleReal.
+ The type SpecDoubleReal does not include the NaN types.
+
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileDoubleRep):
+ * tests/stress/double-rep-real-number-use-on-nan.js: Added.
+ To ensure the isNotInt32() does not test anything, we want
+ proven numbers as input. The (+value) are there to enforce
+ a ToNumber() which in turn give us a proven Number type.
+
 2016-06-03 Benjamin Poulain <[email protected]>
 
  JSON.stringify replacer function calls with numeric array indices

diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
@@ -1138,10 +1138,10 @@ class LowerDFGToB3 {
  usually(continuation), rarely(intCase));
 
  LBasicBlock lastNext = m_out.appendTo(intCase, continuation);
- 
+
  FTL_TYPE_CHECK(
  jsValueValue(value), m_node->child1(), SpecBytecodeRealNumber,
- isNotInt32(value, provenType(m_node->child1()) & ~SpecFullDouble));
+ isNotInt32(value, provenType(m_node->child1()) & ~SpecDoubleReal));
  ValueFromBlock slowResult = m_out.anchor(m_out.intToDouble(unboxInt32(value)));
  m_out.jump(continuation);
 

diff --git a/Source/JavaScriptCore/tests/stress/double-rep-real-number-use-on-nan.js b/Source/JavaScriptCore/tests/stress/double-rep-real-number-use-on-nan.js
@@ -0,0 +1,46 @@
+// Original test case.
+function isNaNOnDouble(value)
+{
+ return (+value) !== value;
+}
+noInline(isNaNOnDouble);
+
+function testIsNaNOnDoubles()
+{
+ var value = isNaNOnDouble(-0);
+ if (value)
+ throw "isNaNOnDouble(-0) = " + value;
+
+ var value = isNaNOnDouble(NaN);
+ if (!value)
+ throw "isNaNOnDouble(NaN) = " + value;
+
+ var value = isNaNOnDouble(Number.POSITIVE_INFINITY);
+ if (value)
+ throw "isNaNOnDouble(Number.POSITIVE_INFINITY) = " + value;
+}
+noInline(testIsNaNOnDoubles);
+
+for (let i = 0; i < 1e6; ++i) {
+ testIsNaNOnDoubles();
+}
+
+// Simplified test case.
+function isNaNOnDouble2(value)
+{
+ let valueToNumber = (+value);
+ return valueToNumber !== valueToNumber;
+}
+noInline(isNaNOnDouble2);
+
+// Warm up without NaN.
+for (let i = 0; i < 1e6; ++i) {
+ if (isNaNOnDouble2(1.5))
+ throw "Failed isNaNOnDouble(1.5)";
+}
+
+// Then pass some NaNs.
+for (let i = 0; i < 1e6; ++i) {
+ if (!isNaNOnDouble2(NaN))
+ throw "Failed isNaNOnDouble(NaN)";
+}