<!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>[203229] 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/203229">203229</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2016-07-14 11:18:55 -0700 (Thu, 14 Jul 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>JSONObject Walker::walk must save array length before processing array elements.
https://bugs.webkit.org/show_bug.cgi?id=153485
Reviewed by Darin Adler and Michael Saboff.
Source/JavaScriptCore:
According to https://tc39.github.io/ecma262/#sec-internalizejsonproperty,
JSON.parse() should cache the length of an array and use the cached length when
iterating array elements (see section 24.3.1.1 2.b.iii).
* runtime/JSONObject.cpp:
(JSC::Walker::walk):
* tests/stress/JSON-parse-should-cache-array-lengths.js: Added.
(toString):
(shouldBe):
(test):
(test2):
LayoutTests:
* js/JSON-parse-reviver-expected.txt:
* js/script-tests/JSON-parse-reviver.js:
- Fixed a bug in arrayReviver() where it was setting the array length to 3,
but was immediately returning a value from the reviver for index 3. This
effectively forces array.length to 4. As a result, case 4 always failed
silently, and case 5 never executed.
- Added tracking of cases visited by the revivers so that they can be verified.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsjsJSONparsereviverexpectedtxt">trunk/LayoutTests/js/JSON-parse-reviver-expected.txt</a></li>
<li><a href="#trunkLayoutTestsjsscripttestsJSONparsereviverjs">trunk/LayoutTests/js/script-tests/JSON-parse-reviver.js</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSONObjectcpp">trunk/Source/JavaScriptCore/runtime/JSONObject.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoretestsstressJSONparseshouldcachearraylengthsjs">trunk/Source/JavaScriptCore/tests/stress/JSON-parse-should-cache-array-lengths.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (203228 => 203229)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-07-14 17:59:17 UTC (rev 203228)
+++ trunk/LayoutTests/ChangeLog 2016-07-14 18:18:55 UTC (rev 203229)
</span><span class="lines">@@ -1,3 +1,18 @@
</span><ins>+2016-07-14 Mark Lam <mark.lam@apple.com>
+
+ JSONObject Walker::walk must save array length before processing array elements.
+ https://bugs.webkit.org/show_bug.cgi?id=153485
+
+ Reviewed by Darin Adler and Michael Saboff.
+
+ * js/JSON-parse-reviver-expected.txt:
+ * js/script-tests/JSON-parse-reviver.js:
+ - Fixed a bug in arrayReviver() where it was setting the array length to 3,
+ but was immediately returning a value from the reviver for index 3. This
+ effectively forces array.length to 4. As a result, case 4 always failed
+ silently, and case 5 never executed.
+ - Added tracking of cases visited by the revivers so that they can be verified.
+
</ins><span class="cx"> 2016-07-14 Youenn Fablet <youenn@apple.com>
</span><span class="cx">
</span><span class="cx"> DOM value iterable interfaces should use Array prototype methods
</span></span></pre></div>
<a id="trunkLayoutTestsjsJSONparsereviverexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/js/JSON-parse-reviver-expected.txt (203228 => 203229)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/JSON-parse-reviver-expected.txt 2016-07-14 17:59:17 UTC (rev 203228)
+++ trunk/LayoutTests/js/JSON-parse-reviver-expected.txt 2016-07-14 18:18:55 UTC (rev 203229)
</span><span class="lines">@@ -51,6 +51,15 @@
</span><span class="cx"> Ensure that when visiting a deleted property value is undefined
</span><span class="cx"> PASS value is undefined.
</span><span class="cx">
</span><ins>+Ensure the holder for our array is indeed an array
+PASS Array.isArray(currentHolder) is true
+PASS currentHolder.length is 4
+
+Ensure that we always get the same holder
+PASS currentHolder is lastHolder
+PASS Ensured that property was visited despite Array length being reduced.
+PASS value is undefined.
+
</ins><span class="cx"> Ensure that we created the root holder as specified in ES5
</span><span class="cx"> PASS '' in lastHolder is true
</span><span class="cx"> PASS result is lastHolder['']
</span><span class="lines">@@ -57,6 +66,7 @@
</span><span class="cx">
</span><span class="cx"> Ensure that a deleted value is revived if the reviver function returns a value
</span><span class="cx"> PASS result.hasOwnProperty(3) is true
</span><ins>+PASS casesVisited is [0,1,2,3,4,5]
</ins><span class="cx">
</span><span class="cx"> Test behaviour of revivor used in conjunction with an object
</span><span class="cx"> PASS currentHolder != globalObject is true
</span><span class="lines">@@ -98,6 +108,7 @@
</span><span class="cx"> PASS result.hasOwnProperty('a property') is false
</span><span class="cx"> PASS result.hasOwnProperty('to delete') is true
</span><span class="cx"> PASS result is lastHolder['']
</span><ins>+PASS casesVisited is ['a property', 'another property', 'and another property', 'to delete']
</ins><span class="cx">
</span><span class="cx"> Test behaviour of revivor that introduces a cycle
</span><span class="cx"> PASS JSON.parse("[0,1]", reviveAddsCycle) threw exception RangeError: Maximum call stack size exceeded..
</span></span></pre></div>
<a id="trunkLayoutTestsjsscripttestsJSONparsereviverjs"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/js/script-tests/JSON-parse-reviver.js (203228 => 203229)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/script-tests/JSON-parse-reviver.js 2016-07-14 17:59:17 UTC (rev 203228)
+++ trunk/LayoutTests/js/script-tests/JSON-parse-reviver.js 2016-07-14 18:18:55 UTC (rev 203229)
</span><span class="lines">@@ -2,6 +2,7 @@
</span><span class="cx"> if (!Array.isArray)
</span><span class="cx"> Array.isArray = function(o) { return o.constructor === Array; }
</span><span class="cx">
</span><ins>+let casesVisited = [];
</ins><span class="cx"> function arrayReviver(i,v) {
</span><span class="cx"> if (i != "") {
</span><span class="cx"> currentHolder = this;
</span><span class="lines">@@ -14,6 +15,7 @@
</span><span class="cx"> debug("Ensure that we always get the same holder");
</span><span class="cx"> shouldBe("currentHolder", "lastHolder");
</span><span class="cx"> }
</span><ins>+ casesVisited.push(Number(i));
</ins><span class="cx"> switch (Number(i)) {
</span><span class="cx"> case 0:
</span><span class="cx"> v = undefined;
</span><span class="lines">@@ -53,17 +55,19 @@
</span><span class="cx"> debug("Ensure that when visiting a deleted property value is undefined");
</span><span class="cx"> shouldBeUndefined("value");
</span><span class="cx"> v = "undelete the property";
</span><del>- expectedLength = this.length = 3;
</del><ins>+ this.length = 3;
+ expectedLength = 4; // undeleting the value at index 3 should bump the length back to 4.
</ins><span class="cx"> break;
</span><span class="cx">
</span><span class="cx"> case 4:
</span><del>- if (this.length != 3) {
</del><ins>+ if (this.length != 4) {
</ins><span class="cx"> testFailed("Did not call reviver for deleted property");
</span><del>- expectedLength = this.length = 3;
</del><ins>+ expectedLength = this.length = 4;
</ins><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> case 5:
</span><ins>+ casesVisited.push(5);
</ins><span class="cx"> testPassed("Ensured that property was visited despite Array length being reduced.");
</span><span class="cx"> value = v;
</span><span class="cx"> shouldBeUndefined("value");
</span><span class="lines">@@ -86,7 +90,9 @@
</span><span class="cx"> debug("");
</span><span class="cx"> debug("Ensure that a deleted value is revived if the reviver function returns a value");
</span><span class="cx"> shouldBeTrue("result.hasOwnProperty(3)");
</span><ins>+shouldBe("casesVisited", "[0,1,2,3,4,5]");
</ins><span class="cx">
</span><ins>+casesVisited = [];
</ins><span class="cx"> function objectReviver(i,v) {
</span><span class="cx"> if (i != "") {
</span><span class="cx"> currentHolder = this;
</span><span class="lines">@@ -97,6 +103,7 @@
</span><span class="cx"> shouldBe("currentHolder", "lastHolder");
</span><span class="cx"> }
</span><span class="cx"> seen = true;
</span><ins>+ casesVisited.push(i);
</ins><span class="cx"> switch (i) {
</span><span class="cx"> case "a property":
</span><span class="cx"> v = undefined;
</span><span class="lines">@@ -155,6 +162,7 @@
</span><span class="cx"> shouldBeFalse("result.hasOwnProperty('a property')");
</span><span class="cx"> shouldBeTrue("result.hasOwnProperty('to delete')");
</span><span class="cx"> shouldBe("result", "lastHolder['']");
</span><ins>+shouldBe("casesVisited", "['a property', 'another property', 'and another property', 'to delete']");
</ins><span class="cx">
</span><span class="cx"> debug("");
</span><span class="cx"> debug("Test behaviour of revivor that introduces a cycle");
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (203228 => 203229)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-07-14 17:59:17 UTC (rev 203228)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-07-14 18:18:55 UTC (rev 203229)
</span><span class="lines">@@ -1,3 +1,22 @@
</span><ins>+2016-07-14 Mark Lam <mark.lam@apple.com>
+
+ JSONObject Walker::walk must save array length before processing array elements.
+ https://bugs.webkit.org/show_bug.cgi?id=153485
+
+ Reviewed by Darin Adler and Michael Saboff.
+
+ According to https://tc39.github.io/ecma262/#sec-internalizejsonproperty,
+ JSON.parse() should cache the length of an array and use the cached length when
+ iterating array elements (see section 24.3.1.1 2.b.iii).
+
+ * runtime/JSONObject.cpp:
+ (JSC::Walker::walk):
+ * tests/stress/JSON-parse-should-cache-array-lengths.js: Added.
+ (toString):
+ (shouldBe):
+ (test):
+ (test2):
+
</ins><span class="cx"> 2016-07-14 Julien Brianceau <jbriance@cisco.com>
</span><span class="cx">
</span><span class="cx"> [mips] Handle properly unaligned halfword load
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSONObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSONObject.cpp (203228 => 203229)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSONObject.cpp 2016-07-14 17:59:17 UTC (rev 203228)
+++ trunk/Source/JavaScriptCore/runtime/JSONObject.cpp 2016-07-14 18:18:55 UTC (rev 203229)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2009 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2009, 2016 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -593,6 +593,7 @@
</span><span class="cx"> Vector<uint32_t, 16, UnsafeVectorOverflow> indexStack;
</span><span class="cx"> LocalStack<JSObject, 16> objectStack(m_exec->vm());
</span><span class="cx"> LocalStack<JSArray, 16> arrayStack(m_exec->vm());
</span><ins>+ Vector<unsigned, 16, UnsafeVectorOverflow> arrayLengthStack;
</ins><span class="cx">
</span><span class="cx"> Vector<WalkerState, 16, UnsafeVectorOverflow> stateStack;
</span><span class="cx"> WalkerState state = StateUnknown;
</span><span class="lines">@@ -610,6 +611,7 @@
</span><span class="cx">
</span><span class="cx"> JSArray* array = asArray(inValue);
</span><span class="cx"> arrayStack.push(array);
</span><ins>+ arrayLengthStack.append(array->length());
</ins><span class="cx"> indexStack.append(0);
</span><span class="cx"> }
</span><span class="cx"> arrayStartVisitMember:
</span><span class="lines">@@ -617,9 +619,11 @@
</span><span class="cx"> case ArrayStartVisitMember: {
</span><span class="cx"> JSArray* array = arrayStack.peek();
</span><span class="cx"> uint32_t index = indexStack.last();
</span><del>- if (index == array->length()) {
</del><ins>+ unsigned arrayLength = arrayLengthStack.last();
+ if (index == arrayLength) {
</ins><span class="cx"> outValue = array;
</span><span class="cx"> arrayStack.pop();
</span><ins>+ arrayLengthStack.removeLast();
</ins><span class="cx"> indexStack.removeLast();
</span><span class="cx"> break;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstressJSONparseshouldcachearraylengthsjs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/JSON-parse-should-cache-array-lengths.js (0 => 203229)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/JSON-parse-should-cache-array-lengths.js (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/JSON-parse-should-cache-array-lengths.js 2016-07-14 18:18:55 UTC (rev 203229)
</span><span class="lines">@@ -0,0 +1,100 @@
</span><ins>+// This test should not hang, and should call the reviver function the expected number of times.
+
+function shouldBe(actualExpr, expectedExpr) {
+ function toString(x) {
+ return "" + x;
+ }
+
+ let actual = eval(actualExpr);
+ let expected = eval(expectedExpr);
+ if (typeof actual != typeof expected)
+ throw Error("expected type " + typeof expected + " actual type " + typeof actual);
+ if (toString(actual) != toString(expected))
+ throw Error("expected: " + expected + " actual: " + actual);
+}
+
+let result;
+let visited;
+
+function test(parseString, clearLength) {
+ visited = [];
+ var result = JSON.parse(parseString, function (key, value) {
+ visited.push('{' + key + ':' + value + '}');
+ if (clearLength)
+ this.length = 0;
+ return; // returning undefined deletes the value.
+ });
+ return result;
+}
+
+result = test('[10]', false);
+shouldBe("result", "undefined");
+shouldBe("visited", "['{0:10}','{:}']");
+shouldBe("visited.length", "2");
+
+result = test('[10]', true);
+shouldBe("result", "undefined");
+shouldBe("visited", "['{0:10}','{:}']");
+shouldBe("visited.length", "2");
+
+result = test('[ [ 10, 11 ], 12, [13, 14, 15], 16, 17]', false);
+shouldBe("result", "undefined");
+shouldBe("visited", "['{0:10}','{1:11}','{0:,}','{1:12}','{0:13}','{1:14}','{2:15}','{2:,,}','{3:16}','{4:17}','{:,,,,}']");
+shouldBe("visited.length", "11");
+
+result = test('[ [ 10, 11 ], 12, [13, 14, 15], 16, 17]', true);
+shouldBe("result", "undefined");
+shouldBe("visited", "['{0:10}','{1:undefined}','{0:}','{1:undefined}','{2:undefined}','{3:undefined}','{4:undefined}','{:}']");
+shouldBe("visited.length", "8");
+
+result = test('[ { "a": [ 10, 11 ], "b": 12 } , [ 13, { "c": 14 }, 15], 16, 17]', false);
+shouldBe("result", "undefined");
+shouldBe("visited", "['{0:10}','{1:11}','{a:,}','{b:12}','{0:[object Object]}','{0:13}','{c:14}','{1:[object Object]}','{2:15}','{1:,,}','{2:16}','{3:17}','{:,,,}']");
+shouldBe("visited.length", "13");
+
+result = test('[ { "a": [ 10, 11 ], "b": 12 } , [ 13, { "c": 14 }, 15], 16, 17]', true);
+shouldBe("result", "undefined");
+shouldBe("visited", "['{0:10}','{1:undefined}','{a:}','{b:12}','{0:[object Object]}','{1:undefined}','{2:undefined}','{3:undefined}','{:}']");
+shouldBe("visited.length", "9");
+
+
+function test2(parseString, clearLength) {
+ visited = [];
+ var result = JSON.parse(parseString, function (key, value) {
+ visited.push('{' + key + ':' + value + '}');
+ if (clearLength)
+ this.length = 0;
+ return (typeof value === "number") ? value * 2 : value;
+ });
+ return result;
+}
+
+result = test2('[10]', false);
+shouldBe("result", "[20]");
+shouldBe("visited", "['{0:10}','{:20}']");
+shouldBe("visited.length", "2");
+
+result = test2('[10]', true);
+shouldBe("result", "[20]");
+shouldBe("visited", "['{0:10}','{:20}']");
+shouldBe("visited.length", "2");
+
+result = test2('[ [ 10, 11 ], 12, [13, 14, 15], 16, 17]', false);
+shouldBe("result", "[20,22,24,26,28,30,32,34]");
+shouldBe("visited", "['{0:10}','{1:11}','{0:20,22}','{1:12}','{0:13}','{1:14}','{2:15}','{2:26,28,30}','{3:16}','{4:17}','{:20,22,24,26,28,30,32,34}']");
+shouldBe("visited.length", "11");
+
+result = test2('[ [ 10, 11 ], 12, [13, 14, 15], 16, 17]', true);
+shouldBe("result", "[]");
+shouldBe("visited", "['{0:10}','{1:undefined}','{0:}','{1:undefined}','{2:undefined}','{3:undefined}','{4:undefined}','{:}']");
+shouldBe("visited.length", "8");
+
+result = test2('[ { "a": [ 10, 11 ], "b": 12 } , [ 13, { "c": 14 }, 15], 16, 17]', false);
+shouldBe("result", "['[object Object]',26,'[object Object]',30,32,34]");
+shouldBe("visited", "['{0:10}','{1:11}','{a:20,22}','{b:12}','{0:[object Object]}','{0:13}','{c:14}','{1:[object Object]}','{2:15}','{1:26,[object Object],30}','{2:16}','{3:17}','{:[object Object],26,[object Object],30,32,34}']");
+shouldBe("visited.length", "13");
+
+result = test2('[ { "a": [ 10, 11 ], "b": 12 } , [ 13, { "c": 14 }, 15], 16, 17]', true);
+shouldBe("result", "[]");
+shouldBe("visited", "['{0:10}','{1:undefined}','{a:}','{b:12}','{0:[object Object]}','{1:undefined}','{2:undefined}','{3:undefined}','{:}']");
+shouldBe("visited.length", "9");
</ins></span></pre>
</div>
</div>
</body>
</html>