<!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>[210935] 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/210935">210935</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2017-01-19 12:53:42 -0800 (Thu, 19 Jan 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>The mutator needs to fire a barrier after memmoving stuff around in an object that the GC scans
https://bugs.webkit.org/show_bug.cgi?id=167208
Reviewed by Saam Barati.
It used to be that if you moved a value from one place to another in the same object
then there is no need for a barrier because the generational GC would have no need to
know that some old object still continues to refer to the same other old object.
But the concurrent GC might scan that object as the mutator moves pointers around in
it. If the ordering is right, this could mean that the collector never sees some of
those pointers. This can be fixed by adding a barrier.
This fixes the most obvious cases I found. There may be more and I'll continue to
audit. Most of the other memmove users seem to already use some kind of synchronization
to prevent this. For example, this can also be fixed by just holding the cell lock
around the memmove since we're dealing with indexing storage and the GC reads that
under the cell lock.
* runtime/JSArray.cpp:
(JSC::JSArray::shiftCountWithAnyIndexingType):
(JSC::JSArray::unshiftCountWithAnyIndexingType):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSArraycpp">trunk/Source/JavaScriptCore/runtime/JSArray.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (210934 => 210935)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2017-01-19 20:21:14 UTC (rev 210934)
+++ trunk/Source/JavaScriptCore/ChangeLog 2017-01-19 20:53:42 UTC (rev 210935)
</span><span class="lines">@@ -1,3 +1,28 @@
</span><ins>+2017-01-19 Filip Pizlo <fpizlo@apple.com>
+
+ The mutator needs to fire a barrier after memmoving stuff around in an object that the GC scans
+ https://bugs.webkit.org/show_bug.cgi?id=167208
+
+ Reviewed by Saam Barati.
+
+ It used to be that if you moved a value from one place to another in the same object
+ then there is no need for a barrier because the generational GC would have no need to
+ know that some old object still continues to refer to the same other old object.
+
+ But the concurrent GC might scan that object as the mutator moves pointers around in
+ it. If the ordering is right, this could mean that the collector never sees some of
+ those pointers. This can be fixed by adding a barrier.
+
+ This fixes the most obvious cases I found. There may be more and I'll continue to
+ audit. Most of the other memmove users seem to already use some kind of synchronization
+ to prevent this. For example, this can also be fixed by just holding the cell lock
+ around the memmove since we're dealing with indexing storage and the GC reads that
+ under the cell lock.
+
+ * runtime/JSArray.cpp:
+ (JSC::JSArray::shiftCountWithAnyIndexingType):
+ (JSC::JSArray::unshiftCountWithAnyIndexingType):
+
</ins><span class="cx"> 2017-01-19 Myles C. Maxfield <mmaxfield@apple.com>
</span><span class="cx">
</span><span class="cx"> [Cocoa] Variation fonts are erroneously disabled on iOS
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSArraycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSArray.cpp (210934 => 210935)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSArray.cpp 2017-01-19 20:21:14 UTC (rev 210934)
+++ trunk/Source/JavaScriptCore/runtime/JSArray.cpp 2017-01-19 20:53:42 UTC (rev 210935)
</span><span class="lines">@@ -1021,6 +1021,11 @@
</span><span class="cx"> butterfly->contiguous()[i].clear();
</span><span class="cx">
</span><span class="cx"> butterfly->setPublicLength(oldLength - count);
</span><ins>+
+ // Our memmoving of values around in the array could have concealed some of them from
+ // the collector. Let's make sure that the collector scans this object again.
+ vm.heap.writeBarrier(this);
+
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1169,6 +1174,10 @@
</span><span class="cx"> butterfly->contiguous()[i + count].setWithoutWriteBarrier(v);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ // Our memmoving of values around in the array could have concealed some of them from
+ // the collector. Let's make sure that the collector scans this object again.
+ vm.heap.writeBarrier(this);
+
</ins><span class="cx"> // NOTE: we're leaving being garbage in the part of the array that we shifted out
</span><span class="cx"> // of. This is fine because the caller is required to store over that area, and
</span><span class="cx"> // in contiguous mode storing into a hole is guaranteed to behave exactly the same
</span></span></pre>
</div>
</div>
</body>
</html>