Title: [199935] trunk
- Revision
- 199935
- Author
- mark....@apple.com
- Date
- 2016-04-22 16:48:44 -0700 (Fri, 22 Apr 2016)
Log Message
_javascript_ jit bug affecting Google Maps.
https://bugs.webkit.org/show_bug.cgi?id=153431
Reviewed by Filip Pizlo.
Source/_javascript_Core:
The issue was due to the abstract interpreter wrongly marking the type of the
value read from the Uint3Array as SpecInt52, which precludes it from being an
Int32. This proves to be false, and the generated code failed to handle the case
where the read value is actually an Int32.
The fix is to have the abstract interpreter use SpecMachineInt instead of
SpecInt52.
* bytecode/SpeculatedType.h:
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
LayoutTests:
* js/regress/bug-153431-expected.txt: Added.
* js/regress/bug-153431.html: Added.
* js/regress/script-tests/bug-153431.js: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (199934 => 199935)
--- trunk/LayoutTests/ChangeLog 2016-04-22 23:25:54 UTC (rev 199934)
+++ trunk/LayoutTests/ChangeLog 2016-04-22 23:48:44 UTC (rev 199935)
@@ -1,3 +1,14 @@
+2016-04-22 Mark Lam <mark....@apple.com>
+
+ _javascript_ jit bug affecting Google Maps.
+ https://bugs.webkit.org/show_bug.cgi?id=153431
+
+ Reviewed by Filip Pizlo.
+
+ * js/regress/bug-153431-expected.txt: Added.
+ * js/regress/bug-153431.html: Added.
+ * js/regress/script-tests/bug-153431.js: Added.
+
2016-04-22 Geoffrey Garen <gga...@apple.com>
super should be available in object literals
Added: trunk/LayoutTests/js/regress/bug-153431-expected.txt (0 => 199935)
--- trunk/LayoutTests/js/regress/bug-153431-expected.txt (rev 0)
+++ trunk/LayoutTests/js/regress/bug-153431-expected.txt 2016-04-22 23:48:44 UTC (rev 199935)
@@ -0,0 +1,10 @@
+JSRegress/bug-153431
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/js/regress/bug-153431.html (0 => 199935)
--- trunk/LayoutTests/js/regress/bug-153431.html (rev 0)
+++ trunk/LayoutTests/js/regress/bug-153431.html 2016-04-22 23:48:44 UTC (rev 199935)
@@ -0,0 +1,12 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/js/regress/script-tests/bug-153431.js (0 => 199935)
--- trunk/LayoutTests/js/regress/script-tests/bug-153431.js (rev 0)
+++ trunk/LayoutTests/js/regress/script-tests/bug-153431.js 2016-04-22 23:48:44 UTC (rev 199935)
@@ -0,0 +1,36 @@
+//@ runDefault
+
+// Regression test for https://bugs.webkit.org/show_bug.cgi?id=153431.
+// Reduced version based on the reproduction case provided by Ryan Sturgell in the bug,
+// with some variable renames to read slightly better.
+
+function assert(testedValue) {
+ if (!testedValue)
+ throw Error("Failed assertion");
+}
+
+function badFunc(arr, operand, resultArr) {
+ // This re-use of variable "operand" is important - rename it and the bug goes away.
+ operand = arr[operand];
+ if (false) {
+ // If this unreachable block is removed, the bug goes away!!
+ } else
+ {
+ resultArr[0] = operand;
+ }
+}
+noInline(badFunc);
+
+function run() {
+ for (var i = 0; i < 10000; i++) {
+ var arr = new Uint32Array([0x80000000,1]); // Needs to be an Uint32Array.
+ var resultArr = [];
+
+ badFunc(arr, 0, resultArr);
+ assert(resultArr[0] == arr[0]);
+ badFunc(arr, 1, resultArr);
+ assert(resultArr[0] == arr[1]);
+ }
+}
+
+run();
Modified: trunk/Source/_javascript_Core/ChangeLog (199934 => 199935)
--- trunk/Source/_javascript_Core/ChangeLog 2016-04-22 23:25:54 UTC (rev 199934)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-04-22 23:48:44 UTC (rev 199935)
@@ -1,3 +1,22 @@
+2016-04-22 Mark Lam <mark....@apple.com>
+
+ _javascript_ jit bug affecting Google Maps.
+ https://bugs.webkit.org/show_bug.cgi?id=153431
+
+ Reviewed by Filip Pizlo.
+
+ The issue was due to the abstract interpreter wrongly marking the type of the
+ value read from the Uint3Array as SpecInt52, which precludes it from being an
+ Int32. This proves to be false, and the generated code failed to handle the case
+ where the read value is actually an Int32.
+
+ The fix is to have the abstract interpreter use SpecMachineInt instead of
+ SpecInt52.
+
+ * bytecode/SpeculatedType.h:
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+
2016-04-22 Benjamin Poulain <bpoul...@apple.com>
[JSC] PredictionPropagation should not be in the top 5 heaviest phases
Modified: trunk/Source/_javascript_Core/bytecode/SpeculatedType.h (199934 => 199935)
--- trunk/Source/_javascript_Core/bytecode/SpeculatedType.h 2016-04-22 23:25:54 UTC (rev 199934)
+++ trunk/Source/_javascript_Core/bytecode/SpeculatedType.h 2016-04-22 23:48:44 UTC (rev 199935)
@@ -67,7 +67,7 @@
static const SpeculatedType SpecBoolInt32 = 1u << 21; // It's definitely an Int32 with value 0 or 1.
static const SpeculatedType SpecNonBoolInt32 = 1u << 22; // It's definitely an Int32 with value other than 0 or 1.
static const SpeculatedType SpecInt32 = SpecBoolInt32 | SpecNonBoolInt32; // It's definitely an Int32.
-static const SpeculatedType SpecInt52 = 1u << 23; // It's definitely an Int52 and we intend it to unbox it.
+static const SpeculatedType SpecInt52 = 1u << 23; // It's definitely an Int52 and we intend it to unbox it. It's also definitely not an Int32.
static const SpeculatedType SpecMachineInt = SpecInt32 | SpecInt52; // It's something that we can do machine int arithmetic on.
static const SpeculatedType SpecInt52AsDouble = 1u << 24; // It's definitely an Int52 and it's inside a double.
static const SpeculatedType SpecInteger = SpecMachineInt | SpecInt52AsDouble; // It's definitely some kind of integer.
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (199934 => 199935)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2016-04-22 23:25:54 UTC (rev 199934)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2016-04-22 23:48:44 UTC (rev 199935)
@@ -1562,7 +1562,7 @@
if (node->shouldSpeculateInt32())
forNode(node).setType(SpecInt32);
else if (enableInt52() && node->shouldSpeculateMachineInt())
- forNode(node).setType(SpecInt52);
+ forNode(node).setType(SpecMachineInt);
else
forNode(node).setType(SpecInt52AsDouble);
break;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
