Diff
Modified: trunk/JSTests/ChangeLog (208310 => 208311)
--- trunk/JSTests/ChangeLog 2016-11-02 23:04:22 UTC (rev 208310)
+++ trunk/JSTests/ChangeLog 2016-11-02 23:15:33 UTC (rev 208311)
@@ -1,3 +1,29 @@
+2016-11-02 Saam Barati <sbar...@apple.com>
+
+ Allocation elimination of rest parameter doesn't take into account indexed properties on Array.prototype/Object.prototype
+ https://bugs.webkit.org/show_bug.cgi?id=164301
+
+ Reviewed by Geoffrey Garen.
+
+ * stress/rest-parameter-allocation-elimination-watchpoints-2.js: Added.
+ (assert):
+ (foo):
+ * stress/rest-parameter-allocation-elimination-watchpoints-3.js: Added.
+ (assert):
+ (foo):
+ * stress/rest-parameter-allocation-elimination-watchpoints-4.js: Added.
+ (assert):
+ (foo):
+ * stress/rest-parameter-allocation-elimination-watchpoints-5.js: Added.
+ (assert):
+ (foo):
+ * stress/rest-parameter-allocation-elimination-watchpoints-6.js: Added.
+ (assert):
+ (foo):
+ * stress/rest-parameter-allocation-elimination-watchpoints.js: Added.
+ (assert):
+ (foo):
+
2016-11-01 Saam Barati <sbar...@apple.com>
We should be able to eliminate rest parameter allocations
Added: trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-2.js (0 => 208311)
--- trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-2.js (rev 0)
+++ trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-2.js 2016-11-02 23:15:33 UTC (rev 208311)
@@ -0,0 +1,19 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+noInline(assert);
+
+function foo(...args) {
+ return args[0];
+}
+noInline(foo);
+
+for (let i = 0; i < 10000; i++) {
+ // Warm it up on in bound accesses.
+ assert(foo(i) === i);
+}
+
+Array.prototype[0] = 50;
+for (let i = 0; i < 10000; i++)
+ assert(foo() === 50);
Added: trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-3.js (0 => 208311)
--- trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-3.js (rev 0)
+++ trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-3.js 2016-11-02 23:15:33 UTC (rev 208311)
@@ -0,0 +1,22 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+noInline(assert);
+
+function foo(...args) {
+ return args[0];
+}
+noInline(foo);
+
+for (let i = 0; i < 10000; i++) {
+ // Warm it up on both in bound and out of bound accesses.
+ if (i % 2)
+ assert(foo(i) === i);
+ else
+ assert(foo() === undefined);
+}
+
+Object.prototype[0] = 50;
+for (let i = 0; i < 10000; i++)
+ assert(foo() === 50);
Added: trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-4.js (0 => 208311)
--- trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-4.js (rev 0)
+++ trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-4.js 2016-11-02 23:15:33 UTC (rev 208311)
@@ -0,0 +1,20 @@
+
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+noInline(assert);
+
+function foo(...args) {
+ return args[0];
+}
+noInline(foo);
+
+for (let i = 0; i < 10000; i++) {
+ // Warm it up on in bound accesses.
+ assert(foo(i) === i);
+}
+
+Object.prototype[0] = 50;
+for (let i = 0; i < 10000; i++)
+ assert(foo() === 50);
Added: trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-5.js (0 => 208311)
--- trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-5.js (rev 0)
+++ trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-5.js 2016-11-02 23:15:33 UTC (rev 208311)
@@ -0,0 +1,25 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+noInline(assert);
+
+function foo(...args) {
+ return args[0];
+}
+noInline(foo);
+
+for (let i = 0; i < 10000; i++) {
+ // Warm it up on both in bound and out of bound accesses.
+ if (i % 2)
+ assert(foo(i) === i);
+ else
+ assert(foo() === undefined);
+}
+
+let newProto = [50];
+newProto.__proto__ = null;
+
+Array.prototype.__proto__ = newProto;
+for (let i = 0; i < 10000; i++)
+ assert(foo() === 50);
Added: trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-6.js (0 => 208311)
--- trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-6.js (rev 0)
+++ trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints-6.js 2016-11-02 23:15:33 UTC (rev 208311)
@@ -0,0 +1,24 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+noInline(assert);
+
+function foo(...args) {
+ return args[0];
+}
+noInline(foo);
+
+for (let i = 0; i < 10000; i++) {
+ // Warm it up on both in bound and out of bound accesses.
+ if (i % 2)
+ assert(foo(i) === i);
+ else
+ assert(foo() === undefined);
+}
+
+let newProto = [50];
+newProto.__proto__ = null;
+Object.prototype.__proto__ = newProto;
+for (let i = 0; i < 10000; i++)
+ assert(foo() === 50);
Added: trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints.js (0 => 208311)
--- trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints.js (rev 0)
+++ trunk/JSTests/stress/rest-parameter-allocation-elimination-watchpoints.js 2016-11-02 23:15:33 UTC (rev 208311)
@@ -0,0 +1,22 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+noInline(assert);
+
+function foo(...args) {
+ return args[0];
+}
+noInline(foo);
+
+for (let i = 0; i < 10000; i++) {
+ // Warm it up on both in bound and out of bound accesses.
+ if (i % 2)
+ assert(foo(i) === i);
+ else
+ assert(foo() === undefined);
+}
+
+Array.prototype[0] = 50;
+for (let i = 0; i < 10000; i++)
+ assert(foo() === 50);
Modified: trunk/Source/_javascript_Core/ChangeLog (208310 => 208311)
--- trunk/Source/_javascript_Core/ChangeLog 2016-11-02 23:04:22 UTC (rev 208310)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-11-02 23:15:33 UTC (rev 208311)
@@ -1,3 +1,22 @@
+2016-11-02 Saam Barati <sbar...@apple.com>
+
+ Allocation elimination of rest parameter doesn't take into account indexed properties on Array.prototype/Object.prototype
+ https://bugs.webkit.org/show_bug.cgi?id=164301
+
+ Reviewed by Geoffrey Garen.
+
+ We weren't taking into account indexed properties on the __proto__
+ of the rest parameter. This made the code for doing out of bound
+ accesses incorrect since it just assumed it's safe for the result of
+ an out of bound access to be undefined. This broke the semantics
+ of JS code when there was an indexed property on the Array.prototype
+ or Object.prototype.
+
+ This patch makes sure we set up the proper watchpoints for making
+ sure out of bound accesses are safe to return undefined.
+
+ * dfg/DFGArgumentsEliminationPhase.cpp:
+
2016-11-02 Geoffrey Garen <gga...@apple.com>
One file per class for CodeBlock.h/.cpp
Modified: trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp (208310 => 208311)
--- trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2016-11-02 23:04:22 UTC (rev 208310)
+++ trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2016-11-02 23:15:33 UTC (rev 208311)
@@ -28,6 +28,7 @@
#if ENABLE(DFG_JIT)
+#include "ArrayPrototype.h"
#include "BytecodeLivenessAnalysisInlines.h"
#include "ClonedArguments.h"
#include "DFGArgumentsUtilities.h"
@@ -154,13 +155,25 @@
if (mode.isInBounds())
break;
- // If we're out-of-bounds then we proceed only if the object prototype is
- // sane (i.e. doesn't have indexed properties).
+ // If we're out-of-bounds then we proceed only if the prototype chain
+ // for the allocation is sane (i.e. doesn't have indexed properties).
JSGlobalObject* globalObject = m_graph.globalObjectFor(edge->origin.semantic);
- if (globalObject->objectPrototypeIsSane()) {
- m_graph.watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
- if (globalObject->objectPrototypeIsSane())
+ InlineWatchpointSet& objectPrototypeTransition = globalObject->objectPrototype()->structure()->transitionWatchpointSet();
+ if (edge->op() == CreateRest) {
+ InlineWatchpointSet& arrayPrototypeTransition = globalObject->arrayPrototype()->structure()->transitionWatchpointSet();
+ if (arrayPrototypeTransition.isStillValid()
+ && objectPrototypeTransition.isStillValid()
+ && globalObject->arrayPrototypeChainIsSane()) {
+ m_graph.watchpoints().addLazily(arrayPrototypeTransition);
+ m_graph.watchpoints().addLazily(objectPrototypeTransition);
break;
+ }
+ } else {
+ if (objectPrototypeTransition.isStillValid()
+ && globalObject->objectPrototypeIsSane()) {
+ m_graph.watchpoints().addLazily(objectPrototypeTransition);
+ break;
+ }
}
escape(edge, source);
break;
