<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[199935] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/199935">199935</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2016-04-22 16:48:44 -0700 (Fri, 22 Apr 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>javascript jit bug affecting Google Maps.
https://bugs.webkit.org/show_bug.cgi?id=153431
Reviewed by Filip Pizlo.
Source/JavaScriptCore:
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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeSpeculatedTypeh">trunk/Source/JavaScriptCore/bytecode/SpeculatedType.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGAbstractInterpreterInlinesh">trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsjsregressbug153431expectedtxt">trunk/LayoutTests/js/regress/bug-153431-expected.txt</a></li>
<li><a href="#trunkLayoutTestsjsregressbug153431html">trunk/LayoutTests/js/regress/bug-153431.html</a></li>
<li><a href="#trunkLayoutTestsjsregressscripttestsbug153431js">trunk/LayoutTests/js/regress/script-tests/bug-153431.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (199934 => 199935)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-04-22 23:25:54 UTC (rev 199934)
+++ trunk/LayoutTests/ChangeLog 2016-04-22 23:48:44 UTC (rev 199935)
</span><span class="lines">@@ -1,3 +1,14 @@
</span><ins>+2016-04-22 Mark Lam <mark.lam@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.
+
</ins><span class="cx"> 2016-04-22 Geoffrey Garen <ggaren@apple.com>
</span><span class="cx">
</span><span class="cx"> super should be available in object literals
</span></span></pre></div>
<a id="trunkLayoutTestsjsregressbug153431expectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/regress/bug-153431-expected.txt (0 => 199935)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+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
+
</ins></span></pre></div>
<a id="trunkLayoutTestsjsregressbug153431html"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/regress/bug-153431.html (0 => 199935)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,12 @@
</span><ins>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="../../resources/regress-pre.js"></script>
+<script src="script-tests/bug-153431.js"></script>
+<script src="../../resources/regress-post.js"></script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsjsregressscripttestsbug153431js"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/regress/script-tests/bug-153431.js (0 => 199935)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,36 @@
</span><ins>+//@ 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();
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (199934 => 199935)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-04-22 23:25:54 UTC (rev 199934)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-04-22 23:48:44 UTC (rev 199935)
</span><span class="lines">@@ -1,3 +1,22 @@
</span><ins>+2016-04-22 Mark Lam <mark.lam@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):
+
</ins><span class="cx"> 2016-04-22 Benjamin Poulain <bpoulain@apple.com>
</span><span class="cx">
</span><span class="cx"> [JSC] PredictionPropagation should not be in the top 5 heaviest phases
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeSpeculatedTypeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/SpeculatedType.h (199934 => 199935)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/SpeculatedType.h 2016-04-22 23:25:54 UTC (rev 199934)
+++ trunk/Source/JavaScriptCore/bytecode/SpeculatedType.h 2016-04-22 23:48:44 UTC (rev 199935)
</span><span class="lines">@@ -67,7 +67,7 @@
</span><span class="cx"> static const SpeculatedType SpecBoolInt32 = 1u << 21; // It's definitely an Int32 with value 0 or 1.
</span><span class="cx"> static const SpeculatedType SpecNonBoolInt32 = 1u << 22; // It's definitely an Int32 with value other than 0 or 1.
</span><span class="cx"> static const SpeculatedType SpecInt32 = SpecBoolInt32 | SpecNonBoolInt32; // It's definitely an Int32.
</span><del>-static const SpeculatedType SpecInt52 = 1u << 23; // It's definitely an Int52 and we intend it to unbox it.
</del><ins>+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.
</ins><span class="cx"> static const SpeculatedType SpecMachineInt = SpecInt32 | SpecInt52; // It's something that we can do machine int arithmetic on.
</span><span class="cx"> static const SpeculatedType SpecInt52AsDouble = 1u << 24; // It's definitely an Int52 and it's inside a double.
</span><span class="cx"> static const SpeculatedType SpecInteger = SpecMachineInt | SpecInt52AsDouble; // It's definitely some kind of integer.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGAbstractInterpreterInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h (199934 => 199935)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h 2016-04-22 23:25:54 UTC (rev 199934)
+++ trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h 2016-04-22 23:48:44 UTC (rev 199935)
</span><span class="lines">@@ -1562,7 +1562,7 @@
</span><span class="cx"> if (node->shouldSpeculateInt32())
</span><span class="cx"> forNode(node).setType(SpecInt32);
</span><span class="cx"> else if (enableInt52() && node->shouldSpeculateMachineInt())
</span><del>- forNode(node).setType(SpecInt52);
</del><ins>+ forNode(node).setType(SpecMachineInt);
</ins><span class="cx"> else
</span><span class="cx"> forNode(node).setType(SpecInt52AsDouble);
</span><span class="cx"> break;
</span></span></pre>
</div>
</div>
</body>
</html>