Title: [201678] trunk/Source/_javascript_Core
Revision
201678
Author
commit-qu...@webkit.org
Date
2016-06-03 20:28:57 -0700 (Fri, 03 Jun 2016)

Log Message

Eager FTL failure for strict comparison of NaN with number check
https://bugs.webkit.org/show_bug.cgi?id=158368

Patch by Benjamin Poulain <bpoul...@apple.com> 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.

Modified Paths

Added Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (201677 => 201678)


--- trunk/Source/_javascript_Core/ChangeLog	2016-06-04 02:04:58 UTC (rev 201677)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-06-04 03:28:57 UTC (rev 201678)
@@ -1,5 +1,35 @@
 2016-06-03  Benjamin Poulain  <bpoul...@apple.com>
 
+        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  <bpoul...@apple.com>
+
         JSON.stringify replacer function calls with numeric array indices
         https://bugs.webkit.org/show_bug.cgi?id=158262
         rdar://problem/26613876

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (201677 => 201678)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2016-06-04 02:04:58 UTC (rev 201677)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2016-06-04 03:28:57 UTC (rev 201678)
@@ -1138,10 +1138,10 @@
                 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);
             

Added: trunk/Source/_javascript_Core/tests/stress/double-rep-real-number-use-on-nan.js (0 => 201678)


--- trunk/Source/_javascript_Core/tests/stress/double-rep-real-number-use-on-nan.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/double-rep-real-number-use-on-nan.js	2016-06-04 03:28:57 UTC (rev 201678)
@@ -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)";
+}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to