REGRESSION(r172401): for-in optimization no longer works at all
https://bugs.webkit.org/show_bug.cgi?id=136056
Reviewed by Geoffrey Garen.
Source/JavaScriptCore:
Roll this back in, along with a fix to make proxies work. Previously, for-in over proxies
would instacrash every time.
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::emitGetByVal):
(JSC::BytecodeGenerator::pushIndexedForInScope):
(JSC::BytecodeGenerator::pushStructureForInScope):
* bytecompiler/BytecodeGenerator.h:
(JSC::ForInContext::ForInContext):
(JSC::StructureForInContext::StructureForInContext):
(JSC::IndexedForInContext::IndexedForInContext):
(JSC::ForInContext::base): Deleted.
* bytecompiler/NodesCodegen.cpp:
(JSC::ForInNode::emitMultiLoopBytecode):
* runtime/JSProxy.cpp:
(JSC::JSProxy::getStructurePropertyNames):
(JSC::JSProxy::getGenericPropertyNames):
* tests/stress/for-in-base-reassigned-later-and-change-structure.js: Added.
(foo):
* tests/stress/for-in-base-reassigned-later.js: Added.
(foo):
* tests/stress/for-in-base-reassigned.js: Added.
(foo):
* tests/stress/for-in-proxy-target-changed-structure.js: Added.
(deleteAll):
(foo):
* tests/stress/for-in-proxy.js: Added.
(foo):
LayoutTests:
This just needs a rebase because the number of calls into the DOM has changed and so the
number of console messages about security stuff has now changed.
* http/tests/security/cross-frame-access-enumeration-expected.txt:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@172794 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index d1104b1..6a16079 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,39 @@
+2014-08-19 Filip Pizlo <fpizlo@apple.com>
+
+ REGRESSION(r172401): for-in optimization no longer works at all
+ https://bugs.webkit.org/show_bug.cgi?id=136056
+
+ Reviewed by Geoffrey Garen.
+
+ Roll this back in, along with a fix to make proxies work. Previously, for-in over proxies
+ would instacrash every time.
+
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::emitGetByVal):
+ (JSC::BytecodeGenerator::pushIndexedForInScope):
+ (JSC::BytecodeGenerator::pushStructureForInScope):
+ * bytecompiler/BytecodeGenerator.h:
+ (JSC::ForInContext::ForInContext):
+ (JSC::StructureForInContext::StructureForInContext):
+ (JSC::IndexedForInContext::IndexedForInContext):
+ (JSC::ForInContext::base): Deleted.
+ * bytecompiler/NodesCodegen.cpp:
+ (JSC::ForInNode::emitMultiLoopBytecode):
+ * runtime/JSProxy.cpp:
+ (JSC::JSProxy::getStructurePropertyNames):
+ (JSC::JSProxy::getGenericPropertyNames):
+ * tests/stress/for-in-base-reassigned-later-and-change-structure.js: Added.
+ (foo):
+ * tests/stress/for-in-base-reassigned-later.js: Added.
+ (foo):
+ * tests/stress/for-in-base-reassigned.js: Added.
+ (foo):
+ * tests/stress/for-in-proxy-target-changed-structure.js: Added.
+ (deleteAll):
+ (foo):
+ * tests/stress/for-in-proxy.js: Added.
+ (foo):
+
2014-08-19 Jaehun Lim <ljaehun.lim@samsung.com>
Unreviewed, fix EFL build after r17275
diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
index 6345871..d65a2f2 100644
--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
@@ -1422,9 +1422,6 @@
{
for (size_t i = m_forInContextStack.size(); i > 0; i--) {
ForInContext* context = m_forInContextStack[i - 1].get();
- if (context->base() != base)
- continue;
-
if (context->local() != property)
continue;
@@ -2586,11 +2583,11 @@
return dst;
}
-void BytecodeGenerator::pushIndexedForInScope(RegisterID* baseRegister, RegisterID* localRegister, RegisterID* indexRegister)
+void BytecodeGenerator::pushIndexedForInScope(RegisterID* localRegister, RegisterID* indexRegister)
{
if (!localRegister)
return;
- m_forInContextStack.append(std::make_unique<IndexedForInContext>(baseRegister, localRegister, indexRegister));
+ m_forInContextStack.append(std::make_unique<IndexedForInContext>(localRegister, indexRegister));
}
void BytecodeGenerator::popIndexedForInScope(RegisterID* localRegister)
@@ -2600,11 +2597,11 @@
m_forInContextStack.removeLast();
}
-void BytecodeGenerator::pushStructureForInScope(RegisterID* baseRegister, RegisterID* localRegister, RegisterID* indexRegister, RegisterID* propertyRegister, RegisterID* enumeratorRegister)
+void BytecodeGenerator::pushStructureForInScope(RegisterID* localRegister, RegisterID* indexRegister, RegisterID* propertyRegister, RegisterID* enumeratorRegister)
{
if (!localRegister)
return;
- m_forInContextStack.append(std::make_unique<StructureForInContext>(baseRegister, localRegister, indexRegister, propertyRegister, enumeratorRegister));
+ m_forInContextStack.append(std::make_unique<StructureForInContext>(localRegister, indexRegister, propertyRegister, enumeratorRegister));
}
void BytecodeGenerator::popStructureForInScope(RegisterID* localRegister)
diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
index 7c9762d..3eba8ae 100644
--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
@@ -99,9 +99,8 @@
class ForInContext {
public:
- ForInContext(RegisterID* baseRegister, RegisterID* localRegister)
- : m_baseRegister(baseRegister)
- , m_localRegister(localRegister)
+ ForInContext(RegisterID* localRegister)
+ : m_localRegister(localRegister)
, m_isValid(true)
{
}
@@ -119,19 +118,17 @@
};
virtual ForInContextType type() const = 0;
- RegisterID* base() const { return m_baseRegister.get(); }
RegisterID* local() const { return m_localRegister.get(); }
private:
- RefPtr<RegisterID> m_baseRegister;
RefPtr<RegisterID> m_localRegister;
bool m_isValid;
};
class StructureForInContext : public ForInContext {
public:
- StructureForInContext(RegisterID* baseRegister, RegisterID* localRegister, RegisterID* indexRegister, RegisterID* propertyRegister, RegisterID* enumeratorRegister)
- : ForInContext(baseRegister, localRegister)
+ StructureForInContext(RegisterID* localRegister, RegisterID* indexRegister, RegisterID* propertyRegister, RegisterID* enumeratorRegister)
+ : ForInContext(localRegister)
, m_indexRegister(indexRegister)
, m_propertyRegister(propertyRegister)
, m_enumeratorRegister(enumeratorRegister)
@@ -155,8 +152,8 @@
class IndexedForInContext : public ForInContext {
public:
- IndexedForInContext(RegisterID* baseRegister, RegisterID* localRegister, RegisterID* indexRegister)
- : ForInContext(baseRegister, localRegister)
+ IndexedForInContext(RegisterID* localRegister, RegisterID* indexRegister)
+ : ForInContext(localRegister)
, m_indexRegister(indexRegister)
{
}
@@ -527,9 +524,9 @@
void pushFinallyContext(StatementNode* finallyBlock);
void popFinallyContext();
- void pushIndexedForInScope(RegisterID* base, RegisterID* local, RegisterID* index);
+ void pushIndexedForInScope(RegisterID* local, RegisterID* index);
void popIndexedForInScope(RegisterID* local);
- void pushStructureForInScope(RegisterID* base, RegisterID* local, RegisterID* index, RegisterID* property, RegisterID* enumerator);
+ void pushStructureForInScope(RegisterID* local, RegisterID* index, RegisterID* property, RegisterID* enumerator);
void popStructureForInScope(RegisterID* local);
void invalidateForInContextForLocal(RegisterID* local);
diff --git a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
index 6acfc01..573dbec 100644
--- a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
+++ b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
@@ -2070,7 +2070,7 @@
generator.emitToIndexString(propertyName.get(), i.get());
this->emitLoopHeader(generator, propertyName.get());
- generator.pushIndexedForInScope(base.get(), local.get(), i.get());
+ generator.pushIndexedForInScope(local.get(), i.get());
generator.emitNode(dst, m_statement);
generator.popIndexedForInScope(local.get());
@@ -2104,7 +2104,7 @@
this->emitLoopHeader(generator, propertyName.get());
- generator.pushStructureForInScope(base.get(), local.get(), i.get(), propertyName.get(), structureEnumerator.get());
+ generator.pushStructureForInScope(local.get(), i.get(), propertyName.get(), structureEnumerator.get());
generator.emitNode(dst, m_statement);
generator.popStructureForInScope(local.get());
diff --git a/Source/JavaScriptCore/runtime/JSProxy.cpp b/Source/JavaScriptCore/runtime/JSProxy.cpp
index 4070bf1..f4c6829 100644
--- a/Source/JavaScriptCore/runtime/JSProxy.cpp
+++ b/Source/JavaScriptCore/runtime/JSProxy.cpp
@@ -120,16 +120,17 @@
return thisObject->target()->methodTable(exec->vm())->getEnumerableLength(exec, thisObject->target());
}
-void JSProxy::getStructurePropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNames, EnumerationMode mode)
+void JSProxy::getStructurePropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode)
{
- JSProxy* thisObject = jsCast<JSProxy*>(object);
- thisObject->target()->methodTable(exec->vm())->getStructurePropertyNames(thisObject->target(), exec, propertyNames, mode);
+ // Skip the structure loop, since it is invalid for proxies.
}
void JSProxy::getGenericPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNames, EnumerationMode mode)
{
JSProxy* thisObject = jsCast<JSProxy*>(object);
- thisObject->target()->methodTable(exec->vm())->getGenericPropertyNames(thisObject->target(), exec, propertyNames, mode);
+ // Get *all* of the property names, not just the generic ones, since we skipped the structure
+ // ones above.
+ thisObject->target()->methodTable(exec->vm())->getPropertyNames(thisObject->target(), exec, propertyNames, mode);
}
void JSProxy::getOwnPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNames, EnumerationMode mode)
diff --git a/Source/JavaScriptCore/tests/stress/for-in-base-reassigned-later-and-change-structure.js b/Source/JavaScriptCore/tests/stress/for-in-base-reassigned-later-and-change-structure.js
new file mode 100644
index 0000000..1b087d5
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/for-in-base-reassigned-later-and-change-structure.js
@@ -0,0 +1,18 @@
+function foo(o_) {
+ var o = o_;
+ var result = 0;
+ for (var s in o) {
+ result += o[s];
+ if (result >= 3)
+ o = {0:1, 1:2, b:4, a:3};
+ }
+ return result;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var result = foo({0:0, 1:1, a:2, b:3});
+ if (result != 7)
+ throw "Error: bad result: " + result;
+}
diff --git a/Source/JavaScriptCore/tests/stress/for-in-base-reassigned-later.js b/Source/JavaScriptCore/tests/stress/for-in-base-reassigned-later.js
new file mode 100644
index 0000000..83115b4
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/for-in-base-reassigned-later.js
@@ -0,0 +1,18 @@
+function foo(o_) {
+ var o = o_;
+ var result = 0;
+ for (var s in o) {
+ result += o[s];
+ if (result >= 3)
+ o = {0:1, 1:2, a:3, b:4};
+ }
+ return result;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var result = foo({0:0, 1:1, a:2, b:3});
+ if (result != 7)
+ throw "Error: bad result: " + result;
+}
diff --git a/Source/JavaScriptCore/tests/stress/for-in-base-reassigned.js b/Source/JavaScriptCore/tests/stress/for-in-base-reassigned.js
new file mode 100644
index 0000000..09c8795
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/for-in-base-reassigned.js
@@ -0,0 +1,17 @@
+function foo(o_) {
+ var o = o_;
+ var result = 0;
+ for (var s in o) {
+ result += o[s];
+ o = {0:1, 1:2, a:3, b:4};
+ }
+ return result;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var result = foo({0:0, 1:1, a:2, b:3});
+ if (result != 9)
+ throw "Error: bad result: " + result;
+}
diff --git a/Source/JavaScriptCore/tests/stress/for-in-proxy-target-changed-structure.js b/Source/JavaScriptCore/tests/stress/for-in-proxy-target-changed-structure.js
new file mode 100644
index 0000000..5c9d76f
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/for-in-proxy-target-changed-structure.js
@@ -0,0 +1,32 @@
+var theO;
+
+function deleteAll() {
+ delete theO.a;
+ delete theO.b;
+ delete theO.c;
+ delete theO.d;
+ for (var i = 0; i < 10; ++i)
+ theO["i" + i] = 42;
+ theO.a = 11;
+ theO.b = 12;
+ theO.c = 13;
+ theO.d = 14;
+}
+
+function foo(o_) {
+ var o = o_;
+ var result = 0;
+ for (var s in o) {
+ result += o[s];
+ deleteAll();
+ }
+ return result;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var result = foo(createProxy(theO = {a:1, b:2, c:3, d:4}));
+ if (result != 1 + 12 + 13 + 14)
+ throw "Error: bad result: " + result;
+}
diff --git a/Source/JavaScriptCore/tests/stress/for-in-proxy.js b/Source/JavaScriptCore/tests/stress/for-in-proxy.js
new file mode 100644
index 0000000..44357db
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/for-in-proxy.js
@@ -0,0 +1,16 @@
+function foo(o_) {
+ var o = o_;
+ var result = 0;
+ for (var s in o) {
+ result += o[s];
+ }
+ return result;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var result = foo(createProxy({a:1, b:2, c:3, d:4}));
+ if (result != 1 + 2 + 3 + 4)
+ throw "Error: bad result: " + result;
+}
