<!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>[207226] 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/207226">207226</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2016-10-12 11:27:50 -0700 (Wed, 12 Oct 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Array.prototype.slice should not modify frozen objects.
https://bugs.webkit.org/show_bug.cgi?id=163338
Reviewed by Filip Pizlo.
JSTests:
* stress/array-slice-on-frozen-object.js: Added.
Source/JavaScriptCore:
1. The ES6 spec for Array.prototype.slice
(https://tc39.github.io/ecma262/#sec-array.prototype.slice) states that it uses
the CreateDataPropertyOrThrow()
(https://tc39.github.io/ecma262/#sec-createdatapropertyorthrow) to add items to
the result array. The spec for CreateDataPropertyOrThrow states:
"This abstract operation creates a property whose attributes are set to the
same defaults used for properties created by the ECMAScript language assignment
operator. Normally, the property will not already exist. If it does exist and
is not configurable or if O is not extensible, [[DefineOwnProperty]] will
return false causing this operation to throw a TypeError exception."
2. Array.prototype.slice also uses a Set function
(https://tc39.github.io/ecma262/#sec-set-o-p-v-throw) to set the "length"
property and passes true for the Throw argument. Ultimately, it ends up
calling the OrdinarySet function
(https://tc39.github.io/ecma262/#sec-ordinaryset) that will fail if the
property is not writable. This failure should result in a TypeError being
thrown in Set.
Since the properties of frozen objects are not extensible, not configurable,
and not writeable, Array.prototype.slice should fail to write to the result
array if it is frozen.
If the source array being sliced has 1 or more elements, (1) will take effect
when we try to set the element in the non-writeable result obj.
If the source array being sliced has 0 elements, we will not set any elements and
(1) will not trigger. Subsequently, (2) will take effect when we will try to
set the length of the result obj.
* runtime/ArrayPrototype.cpp:
(JSC::putLength):
(JSC::setLength):
(JSC::arrayProtoFuncSlice):
(JSC::arrayProtoFuncSplice):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkJSTestsChangeLog">trunk/JSTests/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="#trunkJSTestsstressarraysliceonfrozenobjectjs">trunk/JSTests/stress/array-slice-on-frozen-object.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkJSTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/JSTests/ChangeLog (207225 => 207226)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/ChangeLog 2016-10-12 17:41:00 UTC (rev 207225)
+++ trunk/JSTests/ChangeLog 2016-10-12 18:27:50 UTC (rev 207226)
</span><span class="lines">@@ -1,3 +1,12 @@
</span><ins>+2016-10-12 Mark Lam <mark.lam@apple.com>
+
+ Array.prototype.slice should not modify frozen objects.
+ https://bugs.webkit.org/show_bug.cgi?id=163338
+
+ Reviewed by Filip Pizlo.
+
+ * stress/array-slice-on-frozen-object.js: Added.
+
</ins><span class="cx"> 2016-10-11 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><span class="cx"> Array.prototype.concat should not modify frozen objects.
</span></span></pre></div>
<a id="trunkJSTestsstressarraysliceonfrozenobjectjs"></a>
<div class="addfile"><h4>Added: trunk/JSTests/stress/array-slice-on-frozen-object.js (0 => 207226)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/stress/array-slice-on-frozen-object.js (rev 0)
+++ trunk/JSTests/stress/array-slice-on-frozen-object.js 2016-10-12 18:27:50 UTC (rev 207226)
</span><span class="lines">@@ -0,0 +1,64 @@
</span><ins>+//@ runFTLNoCJIT
+
+let totalFailed = 0;
+
+function shouldEqual(testId, actual, expected) {
+ if (actual != expected) {
+ throw testId + ": ERROR: expect " + expected + ", actual " + actual;
+ }
+}
+
+function makeArray() {
+ return ['unmodifiable'];
+}
+
+function makeArrayLikeObject() {
+ var obj = {};
+ obj[0] = 'unmodifiable';
+ obj.length = 1;
+ return obj;
+}
+
+function emptyArraySourceMaker() {
+ return [];
+}
+
+function singleElementArraySourceMaker() {
+ return ['modified_1'];
+}
+
+// Make test functions with unique codeblocks.
+function makeSliceTest(testId) {
+ return new Function("arr", "arr.slice(0); return " + testId + ";");
+}
+
+let numIterations = 10000;
+
+function runTest(testId, testMaker, targetMaker, sourceMaker, expectedValue, expectedException) {
+ var test = testMaker(testId);
+ noInline(test);
+
+ for (var i = 0; i < numIterations; i++) {
+ var exception = undefined;
+
+ var obj = targetMaker();
+ obj = Object.freeze(obj);
+
+ var arr = sourceMaker();
+ arr.constructor = { [Symbol.species]: function() { return obj; } };
+
+ try {
+ test(arr);
+ } catch (e) {
+ exception = "" + e;
+ exception = exception.substr(0, 10); // Search for "TypeError:".
+ }
+ shouldEqual(testId, exception, expectedException);
+ shouldEqual(testId, obj[0], expectedValue);
+ }
+}
+
+runTest(10010, makeSliceTest, makeArray, emptyArraySourceMaker, "unmodifiable", "TypeError:");
+runTest(10011, makeSliceTest, makeArray, singleElementArraySourceMaker, "unmodifiable", "TypeError:");
+runTest(10020, makeSliceTest, makeArrayLikeObject, emptyArraySourceMaker, "unmodifiable", "TypeError:");
+runTest(10021, makeSliceTest, makeArrayLikeObject, singleElementArraySourceMaker, "unmodifiable", "TypeError:");
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (207225 => 207226)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-10-12 17:41:00 UTC (rev 207225)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-10-12 18:27:50 UTC (rev 207226)
</span><span class="lines">@@ -1,3 +1,46 @@
</span><ins>+2016-10-12 Mark Lam <mark.lam@apple.com>
+
+ Array.prototype.slice should not modify frozen objects.
+ https://bugs.webkit.org/show_bug.cgi?id=163338
+
+ Reviewed by Filip Pizlo.
+
+ 1. The ES6 spec for Array.prototype.slice
+ (https://tc39.github.io/ecma262/#sec-array.prototype.slice) states that it uses
+ the CreateDataPropertyOrThrow()
+ (https://tc39.github.io/ecma262/#sec-createdatapropertyorthrow) to add items to
+ the result array. The spec for CreateDataPropertyOrThrow states:
+
+ "This abstract operation creates a property whose attributes are set to the
+ same defaults used for properties created by the ECMAScript language assignment
+ operator. Normally, the property will not already exist. If it does exist and
+ is not configurable or if O is not extensible, [[DefineOwnProperty]] will
+ return false causing this operation to throw a TypeError exception."
+
+ 2. Array.prototype.slice also uses a Set function
+ (https://tc39.github.io/ecma262/#sec-set-o-p-v-throw) to set the "length"
+ property and passes true for the Throw argument. Ultimately, it ends up
+ calling the OrdinarySet function
+ (https://tc39.github.io/ecma262/#sec-ordinaryset) that will fail if the
+ property is not writable. This failure should result in a TypeError being
+ thrown in Set.
+
+ Since the properties of frozen objects are not extensible, not configurable,
+ and not writeable, Array.prototype.slice should fail to write to the result
+ array if it is frozen.
+
+ If the source array being sliced has 1 or more elements, (1) will take effect
+ when we try to set the element in the non-writeable result obj.
+ If the source array being sliced has 0 elements, we will not set any elements and
+ (1) will not trigger. Subsequently, (2) will take effect when we will try to
+ set the length of the result obj.
+
+ * runtime/ArrayPrototype.cpp:
+ (JSC::putLength):
+ (JSC::setLength):
+ (JSC::arrayProtoFuncSlice):
+ (JSC::arrayProtoFuncSplice):
+
</ins><span class="cx"> 2016-10-12 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> Remove JITWriteBarrier.h
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeArrayPrototypecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp (207225 => 207226)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp 2016-10-12 17:41:00 UTC (rev 207225)
+++ trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp 2016-10-12 18:27:50 UTC (rev 207226)
</span><span class="lines">@@ -163,17 +163,24 @@
</span><span class="cx"> return slot.getValue(exec, index);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static ALWAYS_INLINE void putLength(ExecState* exec, VM& vm, JSObject* obj, JSValue value)
</del><ins>+static ALWAYS_INLINE bool putLength(ExecState* exec, VM& vm, JSObject* obj, JSValue value)
</ins><span class="cx"> {
</span><span class="cx"> PutPropertySlot slot(obj);
</span><del>- obj->methodTable()->put(obj, exec, vm.propertyNames->length, value, slot);
</del><ins>+ return obj->methodTable()->put(obj, exec, vm.propertyNames->length, value, slot);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> static ALWAYS_INLINE void setLength(ExecState* exec, VM& vm, JSObject* obj, unsigned value)
</span><span class="cx"> {
</span><del>- if (isJSArray(obj))
- jsCast<JSArray*>(obj)->setLength(exec, value);
- putLength(exec, vm, obj, jsNumber(value));
</del><ins>+ auto scope = DECLARE_THROW_SCOPE(vm);
+ static const bool throwException = true;
+ if (isJSArray(obj)) {
+ jsCast<JSArray*>(obj)->setLength(exec, value, throwException);
+ RETURN_IF_EXCEPTION(scope, void());
+ }
+ bool success = putLength(exec, vm, obj, jsNumber(value));
+ RETURN_IF_EXCEPTION(scope, void());
+ if (UNLIKELY(!success))
+ throwTypeError(exec, scope, ASCIILiteral(ReadonlyPropertyWriteError));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> inline bool speciesWatchpointsValid(ExecState* exec, JSObject* thisObject)
</span><span class="lines">@@ -874,8 +881,9 @@
</span><span class="cx"> JSValue v = getProperty(exec, thisObj, k);
</span><span class="cx"> RETURN_IF_EXCEPTION(scope, encodedJSValue());
</span><span class="cx"> if (v)
</span><del>- result->putDirectIndex(exec, n, v);
</del><ins>+ result->putDirectIndex(exec, n, v, 0, PutDirectIndexShouldThrow);
</ins><span class="cx"> }
</span><ins>+ scope.release();
</ins><span class="cx"> setLength(exec, vm, result, n);
</span><span class="cx"> return JSValue::encode(result);
</span><span class="cx"> }
</span><span class="lines">@@ -907,6 +915,8 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> setLength(exec, vm, result, 0);
</span><ins>+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ scope.release();
</ins><span class="cx"> setLength(exec, vm, thisObj, length);
</span><span class="cx"> return JSValue::encode(result);
</span><span class="cx"> }
</span><span class="lines">@@ -972,6 +982,7 @@
</span><span class="cx"> RETURN_IF_EXCEPTION(scope, encodedJSValue());
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ scope.release();
</ins><span class="cx"> setLength(exec, vm, thisObj, length - deleteCount + additionalArgs);
</span><span class="cx"> return JSValue::encode(result);
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>