Title: [227716] trunk
- Revision
- 227716
- Author
- [email protected]
- Date
- 2018-01-27 10:14:06 -0800 (Sat, 27 Jan 2018)
Log Message
DFG strength reduction fails to convert NumberToStringWithValidRadixConstant for 0 to constant '0'
https://bugs.webkit.org/show_bug.cgi?id=182213
Reviewed by Mark Lam.
JSTests:
* stress/int32-min-to-string.js: Added.
(shouldBe):
(test2):
(test4):
(test8):
(test16):
(test32):
* stress/zero-to-string.js: Added.
(shouldBe):
(test2):
(test4):
(test8):
(test16):
(test32):
Source/_javascript_Core:
toStringWithRadixInternal is originally used for the slow path if the given value is larger than radix or negative.
As a result, it does not accept 0 correctly, and produces an empty string. Since DFGStrengthReductionPhase uses
this function, it accidentally converts NumberToStringWithValidRadixConstant(0, radix) to an empty string.
This patch fixes toStringWithRadixInternal to accept 0. This change fixes twitch.tv's issue.
We also add a careful cast to avoid `-INT32_MIN`. It does not produce incorrect value in x86 in practice,
but it is UB, and a compiler may assume that the given value is never INT32_MIN and could do an incorrect optimization.
* runtime/NumberPrototype.cpp:
(JSC::toStringWithRadixInternal):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (227715 => 227716)
--- trunk/JSTests/ChangeLog 2018-01-27 17:50:55 UTC (rev 227715)
+++ trunk/JSTests/ChangeLog 2018-01-27 18:14:06 UTC (rev 227716)
@@ -1,3 +1,25 @@
+2018-01-27 Yusuke Suzuki <[email protected]>
+
+ DFG strength reduction fails to convert NumberToStringWithValidRadixConstant for 0 to constant '0'
+ https://bugs.webkit.org/show_bug.cgi?id=182213
+
+ Reviewed by Mark Lam.
+
+ * stress/int32-min-to-string.js: Added.
+ (shouldBe):
+ (test2):
+ (test4):
+ (test8):
+ (test16):
+ (test32):
+ * stress/zero-to-string.js: Added.
+ (shouldBe):
+ (test2):
+ (test4):
+ (test8):
+ (test16):
+ (test32):
+
2018-01-23 Yusuke Suzuki <[email protected]>
Add more module scope related tests with code evaluation by string
Added: trunk/JSTests/stress/int32-min-to-string.js (0 => 227716)
--- trunk/JSTests/stress/int32-min-to-string.js (rev 0)
+++ trunk/JSTests/stress/int32-min-to-string.js 2018-01-27 18:14:06 UTC (rev 227716)
@@ -0,0 +1,43 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+function test2()
+{
+ return (-2147483648).toString(2);
+}
+noInline(test2);
+
+function test4()
+{
+ return (-2147483648).toString(4);
+}
+noInline(test4);
+
+function test8()
+{
+ return (-2147483648).toString(8);
+}
+noInline(test8);
+
+function test16()
+{
+ return (-2147483648).toString(16);
+}
+noInline(test16);
+
+function test32()
+{
+ return (-2147483648).toString(32);
+}
+noInline(test32);
+
+for (var i = 0; i < 1e5; ++i) {
+ shouldBe(test2(), '-10000000000000000000000000000000');
+ shouldBe(test4(), '-2000000000000000');
+ shouldBe(test8(), '-20000000000');
+ shouldBe(test16(), '-80000000');
+ shouldBe(test32(), '-2000000');
+}
Added: trunk/JSTests/stress/zero-to-string.js (0 => 227716)
--- trunk/JSTests/stress/zero-to-string.js (rev 0)
+++ trunk/JSTests/stress/zero-to-string.js 2018-01-27 18:14:06 UTC (rev 227716)
@@ -0,0 +1,43 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+function test2()
+{
+ return 0..toString(2);
+}
+noInline(test2);
+
+function test4()
+{
+ return 0..toString(4);
+}
+noInline(test4);
+
+function test8()
+{
+ return 0..toString(8);
+}
+noInline(test8);
+
+function test16()
+{
+ return 0..toString(16);
+}
+noInline(test16);
+
+function test32()
+{
+ return 0..toString(32);
+}
+noInline(test32);
+
+for (var i = 0; i < 1e5; ++i) {
+ shouldBe(test2(), '0');
+ shouldBe(test4(), '0');
+ shouldBe(test8(), '0');
+ shouldBe(test16(), '0');
+ shouldBe(test32(), '0');
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (227715 => 227716)
--- trunk/Source/_javascript_Core/ChangeLog 2018-01-27 17:50:55 UTC (rev 227715)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-01-27 18:14:06 UTC (rev 227716)
@@ -1,3 +1,21 @@
+2018-01-27 Yusuke Suzuki <[email protected]>
+
+ DFG strength reduction fails to convert NumberToStringWithValidRadixConstant for 0 to constant '0'
+ https://bugs.webkit.org/show_bug.cgi?id=182213
+
+ Reviewed by Mark Lam.
+
+ toStringWithRadixInternal is originally used for the slow path if the given value is larger than radix or negative.
+ As a result, it does not accept 0 correctly, and produces an empty string. Since DFGStrengthReductionPhase uses
+ this function, it accidentally converts NumberToStringWithValidRadixConstant(0, radix) to an empty string.
+ This patch fixes toStringWithRadixInternal to accept 0. This change fixes twitch.tv's issue.
+
+ We also add a careful cast to avoid `-INT32_MIN`. It does not produce incorrect value in x86 in practice,
+ but it is UB, and a compiler may assume that the given value is never INT32_MIN and could do an incorrect optimization.
+
+ * runtime/NumberPrototype.cpp:
+ (JSC::toStringWithRadixInternal):
+
2018-01-26 Saam Barati <[email protected]>
Fix emitAllocateWithNonNullAllocator to work on arm
Modified: trunk/Source/_javascript_Core/runtime/NumberPrototype.cpp (227715 => 227716)
--- trunk/Source/_javascript_Core/runtime/NumberPrototype.cpp 2018-01-27 17:50:55 UTC (rev 227715)
+++ trunk/Source/_javascript_Core/runtime/NumberPrototype.cpp 2018-01-27 18:14:06 UTC (rev 227716)
@@ -365,15 +365,17 @@
uint32_t positiveNumber = number;
if (number < 0) {
negative = true;
- positiveNumber = -number;
+ positiveNumber = static_cast<uint32_t>(-static_cast<int64_t>(number));
}
- while (positiveNumber) {
+ // Always loop at least once, to emit at least '0'.
+ do {
uint32_t index = positiveNumber % radix;
ASSERT(index < sizeof(radixDigits));
*--p = static_cast<LChar>(radixDigits[index]);
positiveNumber /= radix;
- }
+ } while (positiveNumber);
+
if (negative)
*--p = '-';
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
