<!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>[203033] 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/203033">203033</a></dd>
<dt>Author</dt> <dd>keith_miller@apple.com</dd>
<dt>Date</dt> <dd>2016-07-09 13:50:51 -0700 (Sat, 09 Jul 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>appendMemcpy might fail in concatAppendOne
https://bugs.webkit.org/show_bug.cgi?id=159601
Source/JavaScriptCore:

&lt;rdar://problem/27211300&gt;

Reviewed by Mark Lam.

There are multiple reasons why we might fail appendMemcpy. One
reason, which I suspect was the source of the crashes, is that one
of the Array prototypes has an indexed property. This patch
consolidates the two old cases by just creating an array then
attempting to memcpy append. If that fails, we fall back to
moveElements.

* runtime/ArrayPrototype.cpp:
(JSC::concatAppendOne):
* tests/stress/concat-with-holesMustForwardToPrototype.js: Added.
(arrayEq):

LayoutTests:

Reviewed by Mark Lam.

Add new microbenchmark testing the performance of concat
when appending one new element. This patch appears to be
about a 13% progression on this test.

* js/regress/concat-append-one-expected.txt: Added.
* js/regress/concat-append-one.html: Added.
* js/regress/script-tests/concat-append-one.js: Added.
(test):</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="#trunkSourceJavaScriptCoreruntimeArrayPrototypecpp">trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsjsregressconcatappendoneexpectedtxt">trunk/LayoutTests/js/regress/concat-append-one-expected.txt</a></li>
<li><a href="#trunkLayoutTestsjsregressconcatappendonehtml">trunk/LayoutTests/js/regress/concat-append-one.html</a></li>
<li><a href="#trunkLayoutTestsjsregressscripttestsconcatappendonejs">trunk/LayoutTests/js/regress/script-tests/concat-append-one.js</a></li>
<li><a href="#trunkSourceJavaScriptCoretestsstressconcatwithholesMustForwardToPrototypejs">trunk/Source/JavaScriptCore/tests/stress/concat-with-holesMustForwardToPrototype.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (203032 => 203033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2016-07-09 16:23:27 UTC (rev 203032)
+++ trunk/LayoutTests/ChangeLog        2016-07-09 20:50:51 UTC (rev 203033)
</span><span class="lines">@@ -1,3 +1,19 @@
</span><ins>+2016-07-09  Keith Miller  &lt;keith_miller@apple.com&gt;
+
+        appendMemcpy might fail in concatAppendOne
+        https://bugs.webkit.org/show_bug.cgi?id=159601
+
+        Reviewed by Mark Lam.
+
+        Add new microbenchmark testing the performance of concat
+        when appending one new element. This patch appears to be
+        about a 13% progression on this test.
+
+        * js/regress/concat-append-one-expected.txt: Added.
+        * js/regress/concat-append-one.html: Added.
+        * js/regress/script-tests/concat-append-one.js: Added.
+        (test):
+
</ins><span class="cx"> 2016-07-09  Youenn Fablet  &lt;youenn@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Make use of PrivateIdentifier to simplify Fetch Headers built-in checks
</span></span></pre></div>
<a id="trunkLayoutTestsjsregressconcatappendoneexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/regress/concat-append-one-expected.txt (0 => 203033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/regress/concat-append-one-expected.txt                                (rev 0)
+++ trunk/LayoutTests/js/regress/concat-append-one-expected.txt        2016-07-09 20:50:51 UTC (rev 203033)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+JSRegress/concat-append-one
+
+On success, you will see a series of &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsjsregressconcatappendonehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/regress/concat-append-one.html (0 => 203033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/regress/concat-append-one.html                                (rev 0)
+++ trunk/LayoutTests/js/regress/concat-append-one.html        2016-07-09 20:50:51 UTC (rev 203033)
</span><span class="lines">@@ -0,0 +1,12 @@
</span><ins>+&lt;!DOCTYPE HTML PUBLIC &quot;-//IETF//DTD HTML//EN&quot;&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;script src=&quot;../../resources/js-test-pre.js&quot;&gt;&lt;/script&gt;
+&lt;/head&gt;
+&lt;body&gt;
+&lt;script src=&quot;../../resources/regress-pre.js&quot;&gt;&lt;/script&gt;
+&lt;script src=&quot;script-tests/concat-append-one.js&quot;&gt;&lt;/script&gt;
+&lt;script src=&quot;../../resources/regress-post.js&quot;&gt;&lt;/script&gt;
+&lt;script src=&quot;../../resources/js-test-post.js&quot;&gt;&lt;/script&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkLayoutTestsjsregressscripttestsconcatappendonejs"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/regress/script-tests/concat-append-one.js (0 => 203033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/regress/script-tests/concat-append-one.js                                (rev 0)
+++ trunk/LayoutTests/js/regress/script-tests/concat-append-one.js        2016-07-09 20:50:51 UTC (rev 203033)
</span><span class="lines">@@ -0,0 +1,15 @@
</span><ins>+function test() {
+    let obj = {};
+    [1,2,3].concat(4);
+    [1,2,3].concat(1.34);
+    [1.35,2,3].concat(1.34);
+    [1.35,2,3].concat(obj);
+    [1,2,3].concat(obj);
+    [].concat(obj);
+    [].concat(1);
+    [].concat(1.224);
+}
+noInline(test);
+
+for (let i = 0; i &lt; 10000; i++)
+    test();
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (203032 => 203033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2016-07-09 16:23:27 UTC (rev 203032)
+++ trunk/Source/JavaScriptCore/ChangeLog        2016-07-09 20:50:51 UTC (rev 203033)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2016-07-09  Keith Miller  &lt;keith_miller@apple.com&gt;
+
+        appendMemcpy might fail in concatAppendOne
+        https://bugs.webkit.org/show_bug.cgi?id=159601
+        &lt;rdar://problem/27211300&gt;
+
+        Reviewed by Mark Lam.
+
+        There are multiple reasons why we might fail appendMemcpy. One
+        reason, which I suspect was the source of the crashes, is that one
+        of the Array prototypes has an indexed property. This patch
+        consolidates the two old cases by just creating an array then
+        attempting to memcpy append. If that fails, we fall back to
+        moveElements.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::concatAppendOne):
+        * tests/stress/concat-with-holesMustForwardToPrototype.js: Added.
+        (arrayEq):
+
</ins><span class="cx"> 2016-07-09  Benjamin Poulain  &lt;bpoulain@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [JSC] Fix the Template Raw Value of \ (escape) + LineTerminatorSequence
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeArrayPrototypecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp (203032 => 203033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp        2016-07-09 16:23:27 UTC (rev 203032)
+++ trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp        2016-07-09 20:50:51 UTC (rev 203033)
</span><span class="lines">@@ -1047,25 +1047,19 @@
</span><span class="cx">     unsigned firstArraySize = firstButterfly-&gt;publicLength();
</span><span class="cx"> 
</span><span class="cx">     IndexingType type = first-&gt;mergeIndexingTypeForCopying(indexingTypeForValue(second) | IsArray);
</span><del>-    JSArray* result;
-    if (type == NonArray) {
-        result = constructEmptyArray(exec, nullptr, firstArraySize + 1);
-        if (vm.exception())
-            return JSValue::encode(JSValue());
</del><ins>+    if (type == NonArray)
+        type = ArrayWithUndecided;
</ins><span class="cx"> 
</span><ins>+    Structure* resultStructure = exec-&gt;lexicalGlobalObject()-&gt;arrayStructureForIndexingTypeDuringAllocation(type);
+    JSArray* result = JSArray::create(vm, resultStructure, firstArraySize + 1);
+    if (!result)
+        return JSValue::encode(throwOutOfMemoryError(exec));
+
+    if (!result-&gt;appendMemcpy(exec, vm, 0, first)) {
</ins><span class="cx">         if (!moveElements(exec, vm, result, 0, first, firstArraySize)) {
</span><span class="cx">             ASSERT(vm.exception());
</span><span class="cx">             return JSValue::encode(JSValue());
</span><span class="cx">         }
</span><del>-
-    } else {
-        Structure* resultStructure = exec-&gt;lexicalGlobalObject()-&gt;arrayStructureForIndexingTypeDuringAllocation(type);
-        result = JSArray::tryCreateUninitialized(vm, resultStructure, firstArraySize + 1);
-        if (!result)
-            return JSValue::encode(throwOutOfMemoryError(exec));
-
-        bool memcpyResult = result-&gt;appendMemcpy(exec, vm, 0, first);
-        RELEASE_ASSERT(memcpyResult);
</del><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     result-&gt;putDirectIndex(exec, firstArraySize, second);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstressconcatwithholesMustForwardToPrototypejs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/concat-with-holesMustForwardToPrototype.js (0 => 203033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/concat-with-holesMustForwardToPrototype.js                                (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/concat-with-holesMustForwardToPrototype.js        2016-07-09 20:50:51 UTC (rev 203033)
</span><span class="lines">@@ -0,0 +1,19 @@
</span><ins>+Array.prototype[1] = 5;
+
+function arrayEq(a, b) {
+    if (a.length !== b.length)
+        throw new Error([a, &quot;\n\n&quot;,  b]);
+
+    for (let i = 0; i &lt; a.length; i++) {
+        if (a[i] !== b[i])
+            throw new Error([a, &quot;\n\n&quot;,  b]);
+    }
+}
+
+
+let obj = {};
+arrayEq([1,2,3].concat(4), [1,2,3,4]);
+arrayEq([1,2,3].concat(1.34), [1,2,3,1.34]);
+arrayEq([1.35,2,3].concat(1.34), [1.35,2,3,1.34]);
+arrayEq([1.35,2,3].concat(obj), [1.35,2,3,obj]);
+arrayEq([1,2,3].concat(obj), [1,2,3,obj]);
</ins></span></pre>
</div>
</div>

</body>
</html>