<!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>[195387] trunk/Source/JavaScriptCore</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/195387">195387</a></dd>
<dt>Author</dt> <dd>benjamin@webkit.org</dd>
<dt>Date</dt> <dd>2016-01-20 15:11:32 -0800 (Wed, 20 Jan 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[JSC] The register allocator can use a dangling pointer when selecting a spill candidate
https://bugs.webkit.org/show_bug.cgi?id=153287
Reviewed by Mark Lam.
A tricky bug I discovered while experimenting with live range breaking.
We have the following initial conditions:
-UseCounts is slow, so we only compute it once for all the iterations
of the allocator.
-The only new Tmps we create are for spills and refills. They are unspillable
by definition so it is fine to not update UseCounts accordingly.
But, in selectSpill(), we go over all the spill candidates and select the best
one based on its score. The score() lambda uses useCounts, it cannot be used
with a new Tmps created for something we already spilled.
The first time we use score is correct, we started by skipping all the unspillable
Tmps from the candidate. The next use was incorrect: we were checking unspillableTmps
*after* calling score().
The existing tests did not catch this due to back luck. I added an assertion
to find similar problems in the future.
* b3/air/AirIteratedRegisterCoalescing.cpp:
* b3/air/AirUseCounts.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3airAirIteratedRegisterCoalescingcpp">trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3airAirUseCountsh">trunk/Source/JavaScriptCore/b3/air/AirUseCounts.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (195386 => 195387)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-01-20 23:00:19 UTC (rev 195386)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-01-20 23:11:32 UTC (rev 195387)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2016-01-20 Benjamin Poulain <benjamin@webkit.org>
+
+ [JSC] The register allocator can use a dangling pointer when selecting a spill candidate
+ https://bugs.webkit.org/show_bug.cgi?id=153287
+
+ Reviewed by Mark Lam.
+
+ A tricky bug I discovered while experimenting with live range breaking.
+
+ We have the following initial conditions:
+ -UseCounts is slow, so we only compute it once for all the iterations
+ of the allocator.
+ -The only new Tmps we create are for spills and refills. They are unspillable
+ by definition so it is fine to not update UseCounts accordingly.
+
+ But, in selectSpill(), we go over all the spill candidates and select the best
+ one based on its score. The score() lambda uses useCounts, it cannot be used
+ with a new Tmps created for something we already spilled.
+
+ The first time we use score is correct, we started by skipping all the unspillable
+ Tmps from the candidate. The next use was incorrect: we were checking unspillableTmps
+ *after* calling score().
+
+ The existing tests did not catch this due to back luck. I added an assertion
+ to find similar problems in the future.
+
+ * b3/air/AirIteratedRegisterCoalescing.cpp:
+ * b3/air/AirUseCounts.h:
+
</ins><span class="cx"> 2016-01-20 Saam barati <sbarati@apple.com>
</span><span class="cx">
</span><span class="cx"> Fix CLoop build after bug https://bugs.webkit.org/show_bug.cgi?id=152766
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3airAirIteratedRegisterCoalescingcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp (195386 => 195387)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp 2016-01-20 23:00:19 UTC (rev 195386)
+++ trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp 2016-01-20 23:11:32 UTC (rev 195387)
</span><span class="lines">@@ -961,11 +961,11 @@
</span><span class="cx">
</span><span class="cx"> ++iterator;
</span><span class="cx"> for (;iterator != m_spillWorklist.end(); ++iterator) {
</span><ins>+ if (m_unspillableTmps.contains(*iterator))
+ continue;
+
</ins><span class="cx"> double tmpScore = score(AbsoluteTmpMapper<type>::tmpFromAbsoluteIndex(*iterator));
</span><span class="cx"> if (tmpScore > maxScore) {
</span><del>- if (m_unspillableTmps.contains(*iterator))
- continue;
-
</del><span class="cx"> victimIterator = iterator;
</span><span class="cx"> maxScore = tmpScore;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3airAirUseCountsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/air/AirUseCounts.h (195386 => 195387)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/air/AirUseCounts.h 2016-01-20 23:00:19 UTC (rev 195386)
+++ trunk/Source/JavaScriptCore/b3/air/AirUseCounts.h 2016-01-20 23:11:32 UTC (rev 195387)
</span><span class="lines">@@ -97,7 +97,12 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>- const Counts& operator[](const Thing& arg) const { return m_counts.find(arg)->value; }
</del><ins>+ const Counts& operator[](const Thing& arg) const
+ {
+ auto iterator = m_counts.find(arg);
+ ASSERT(iterator != m_counts.end());
+ return iterator->value;
+ }
</ins><span class="cx">
</span><span class="cx"> void dump(PrintStream& out) const
</span><span class="cx"> {
</span></span></pre>
</div>
</div>
</body>
</html>