<!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>[171092] 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/171092">171092</a></dd>
<dt>Author</dt> <dd>mhahnenberg@apple.com</dd>
<dt>Date</dt> <dd>2014-07-14 16:59:15 -0700 (Mon, 14 Jul 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>Flattening dictionaries with oversize backing stores can cause crashes
https://bugs.webkit.org/show_bug.cgi?id=134906
Reviewed by Filip Pizlo.
The collector expects any pointers into CopiedSpace passed to copyLater are within 32 KB
of the CopiedBlock header. This was always the case except for when flattening a dictionary
caused the size of the Butterfly to decrease. This was equivalent to moving the base of the
Butterfly to higher addresses. If the object was reduced sufficiently in size, the base
would no longer be within the first 32 KB of the CopiedBlock and the next collection would
choke on the Butterfly pointer.
This patch fixes this issue by detect this situation during flattening and memmove-ing
the Butterfly down to where the old base was.
* runtime/JSObject.cpp:
(JSC::JSObject::shiftButterflyAfterFlattening):
* runtime/JSObject.h:
(JSC::JSObject::butterflyPreCapacity):
(JSC::JSObject::butterflyTotalSize):
* runtime/Structure.cpp:
(JSC::Structure::flattenDictionaryStructure):
* tests/stress/flatten-oversize-dictionary-object.js: Added.
(foo):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSObjectcpp">trunk/Source/JavaScriptCore/runtime/JSObject.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSObjecth">trunk/Source/JavaScriptCore/runtime/JSObject.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeStructurecpp">trunk/Source/JavaScriptCore/runtime/Structure.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoretestsstressflattenoversizedictionaryobjectjs">trunk/Source/JavaScriptCore/tests/stress/flatten-oversize-dictionary-object.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (171091 => 171092)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2014-07-14 23:35:47 UTC (rev 171091)
+++ trunk/Source/JavaScriptCore/ChangeLog 2014-07-14 23:59:15 UTC (rev 171092)
</span><span class="lines">@@ -1,3 +1,30 @@
</span><ins>+2014-07-14 Mark Hahnenberg <mhahnenberg@apple.com>
+
+ Flattening dictionaries with oversize backing stores can cause crashes
+ https://bugs.webkit.org/show_bug.cgi?id=134906
+
+ Reviewed by Filip Pizlo.
+
+ The collector expects any pointers into CopiedSpace passed to copyLater are within 32 KB
+ of the CopiedBlock header. This was always the case except for when flattening a dictionary
+ caused the size of the Butterfly to decrease. This was equivalent to moving the base of the
+ Butterfly to higher addresses. If the object was reduced sufficiently in size, the base
+ would no longer be within the first 32 KB of the CopiedBlock and the next collection would
+ choke on the Butterfly pointer.
+
+ This patch fixes this issue by detect this situation during flattening and memmove-ing
+ the Butterfly down to where the old base was.
+
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::shiftButterflyAfterFlattening):
+ * runtime/JSObject.h:
+ (JSC::JSObject::butterflyPreCapacity):
+ (JSC::JSObject::butterflyTotalSize):
+ * runtime/Structure.cpp:
+ (JSC::Structure::flattenDictionaryStructure):
+ * tests/stress/flatten-oversize-dictionary-object.js: Added.
+ (foo):
+
</ins><span class="cx"> 2014-07-14 Benjamin Poulain <benjamin@webkit.org>
</span><span class="cx">
</span><span class="cx"> Remove some dead code from FTLJITFinalizer
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSObject.cpp (171091 => 171092)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSObject.cpp 2014-07-14 23:35:47 UTC (rev 171091)
+++ trunk/Source/JavaScriptCore/runtime/JSObject.cpp 2014-07-14 23:59:15 UTC (rev 171092)
</span><span class="lines">@@ -2696,4 +2696,15 @@
</span><span class="cx"> return exec->vm().throwException(exec, createTypeError(exec, message));
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void JSObject::shiftButterflyAfterFlattening(VM& vm, size_t outOfLineCapacityBefore, size_t outOfLineCapacityAfter)
+{
+ Butterfly* butterfly = this->butterfly();
+ size_t preCapacity = this->butterflyPreCapacity();
+ void* currentBase = butterfly->base(preCapacity, outOfLineCapacityAfter);
+ void* newBase = butterfly->base(preCapacity, outOfLineCapacityBefore);
+
+ memmove(newBase, currentBase, this->butterflyTotalSize());
+ setButterflyWithoutChangingStructure(vm, Butterfly::fromBase(newBase, preCapacity, outOfLineCapacityAfter));
+}
+
</ins><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSObjecth"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSObject.h (171091 => 171092)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSObject.h 2014-07-14 23:35:47 UTC (rev 171091)
+++ trunk/Source/JavaScriptCore/runtime/JSObject.h 2014-07-14 23:59:15 UTC (rev 171092)
</span><span class="lines">@@ -635,6 +635,7 @@
</span><span class="cx"> {
</span><span class="cx"> structure(vm)->flattenDictionaryStructure(vm, this);
</span><span class="cx"> }
</span><ins>+ void shiftButterflyAfterFlattening(VM&, size_t outOfLineCapacityBefore, size_t outOfLineCapacityAfter);
</ins><span class="cx">
</span><span class="cx"> JSGlobalObject* globalObject() const
</span><span class="cx"> {
</span><span class="lines">@@ -770,6 +771,9 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ size_t butterflyTotalSize();
+ size_t butterflyPreCapacity();
+
</ins><span class="cx"> Butterfly* createInitialUndecided(VM&, unsigned length);
</span><span class="cx"> ContiguousJSValues createInitialInt32(VM&, unsigned length);
</span><span class="cx"> ContiguousDoubles createInitialDouble(VM&, unsigned length);
</span><span class="lines">@@ -1527,6 +1531,32 @@
</span><span class="cx"> return offsetInOutOfLineStorage(offset) + Butterfly::indexOfPropertyStorage();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+inline size_t JSObject::butterflyPreCapacity()
+{
+ if (UNLIKELY(hasIndexingHeader()))
+ return butterfly()->indexingHeader()->preCapacity(structure());
+ return 0;
+}
+
+inline size_t JSObject::butterflyTotalSize()
+{
+ Structure* structure = this->structure();
+ Butterfly* butterfly = this->butterfly();
+ size_t preCapacity;
+ size_t indexingPayloadSizeInBytes;
+ bool hasIndexingHeader = this->hasIndexingHeader();
+
+ if (UNLIKELY(hasIndexingHeader)) {
+ preCapacity = butterfly->indexingHeader()->preCapacity(structure);
+ indexingPayloadSizeInBytes = butterfly->indexingHeader()->indexingPayloadSizeInBytes(structure);
+ } else {
+ preCapacity = 0;
+ indexingPayloadSizeInBytes = 0;
+ }
+
+ return Butterfly::totalSize(preCapacity, structure->outOfLineCapacity(), hasIndexingHeader, indexingPayloadSizeInBytes);
+}
+
</ins><span class="cx"> // Helpers for patching code where you want to emit a load or store and
</span><span class="cx"> // the base is:
</span><span class="cx"> // For inline offsets: a pointer to the out-of-line storage pointer.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeStructurecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/Structure.cpp (171091 => 171092)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/Structure.cpp 2014-07-14 23:35:47 UTC (rev 171091)
+++ trunk/Source/JavaScriptCore/runtime/Structure.cpp 2014-07-14 23:59:15 UTC (rev 171092)
</span><span class="lines">@@ -720,6 +720,8 @@
</span><span class="cx"> {
</span><span class="cx"> checkOffsetConsistency();
</span><span class="cx"> ASSERT(isDictionary());
</span><ins>+
+ size_t beforeOutOfLineCapacity = this->outOfLineCapacity();
</ins><span class="cx"> if (isUncacheableDictionary()) {
</span><span class="cx"> ASSERT(propertyTable());
</span><span class="cx">
</span><span class="lines">@@ -748,11 +750,21 @@
</span><span class="cx"> m_dictionaryKind = NoneDictionaryKind;
</span><span class="cx"> m_hasBeenFlattenedBefore = true;
</span><span class="cx">
</span><del>- // If the object had a Butterfly but after flattening/compacting we no longer have need of it,
- // we need to zero it out because the collector depends on the Structure to know the size for copying.
- if (object->butterfly() && !this->outOfLineCapacity() && !this->hasIndexingHeader(object))
- object->setStructureAndButterfly(vm, this, 0);
</del><ins>+ size_t afterOutOfLineCapacity = this->outOfLineCapacity();
</ins><span class="cx">
</span><ins>+ if (beforeOutOfLineCapacity != afterOutOfLineCapacity) {
+ ASSERT(beforeOutOfLineCapacity > afterOutOfLineCapacity);
+ // If the object had a Butterfly but after flattening/compacting we no longer have need of it,
+ // we need to zero it out because the collector depends on the Structure to know the size for copying.
+ if (object->butterfly() && !afterOutOfLineCapacity && !this->hasIndexingHeader(object))
+ object->setStructureAndButterfly(vm, this, 0);
+ // If the object was down-sized to the point where the base of the Butterfly is no longer within the
+ // first CopiedBlock::blockSize bytes, we'll get the wrong answer if we try to mask the base back to
+ // the CopiedBlock header. To prevent this case we need to memmove the Butterfly down.
+ else if (object->butterfly())
+ object->shiftButterflyAfterFlattening(vm, beforeOutOfLineCapacity, afterOutOfLineCapacity);
+ }
+
</ins><span class="cx"> return this;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstressflattenoversizedictionaryobjectjs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/flatten-oversize-dictionary-object.js (0 => 171092)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/flatten-oversize-dictionary-object.js (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/flatten-oversize-dictionary-object.js 2014-07-14 23:59:15 UTC (rev 171092)
</span><span class="lines">@@ -0,0 +1,37 @@
</span><ins>+var foo = function(o) {
+ return o.baa;
+};
+
+noInline(foo);
+
+(function() {
+ var letters = ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z"];
+ var properties = [];
+ var o = {};
+ for (var i = 0; i < letters.length; ++i) {
+ for (var j = 0; j < letters.length; ++j) {
+ for (var k = 0; k < letters.length; ++k) {
+ var property = letters[i] + letters[j] + letters[k];
+ o[property] = i;
+ }
+ }
+ }
+
+ var keys = Object.keys(o);
+ keys.sort();
+ for (var i = keys.length - 1; i >= keys.length - 8000; i--) {
+ delete o[keys[i]];
+ }
+
+ var sum = 0;
+ var iVal = letters.indexOf("b");
+ var niters = 1000;
+ for (var i = 0; i < niters; ++i) {
+ sum += foo(o);
+ }
+
+ if (sum != iVal * niters)
+ throw new Error("incorrect result: " + sum);
+
+ fullGC();
+})();
</ins></span></pre>
</div>
</div>
</body>
</html>