REGRESSION(r172401): for-in optimization no longer works at all
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Aug 2014 02:38:46 +0000 (02:38 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Aug 2014 02:38:46 +0000 (02:38 +0000)
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: https://svn.webkit.org/repository/webkit/trunk@172794 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/security/cross-frame-access-enumeration-expected.txt
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
Source/JavaScriptCore/runtime/JSProxy.cpp
Source/JavaScriptCore/tests/stress/for-in-base-reassigned-later-and-change-structure.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/for-in-base-reassigned-later.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/for-in-base-reassigned.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/for-in-proxy-target-changed-structure.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/for-in-proxy.js [new file with mode: 0644]

index 9dff584..d25e972 100644 (file)
@@ -1,3 +1,15 @@
+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.
+        
+        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:
+
 2014-08-19  Bem Jones-Bey  <bjonesbe@adobe.com>
 
         [CSS Shapes] A few calc() test failures in the shape-image-threshold parsing tests
index e78a3e6..af4b3ac 100644 (file)
@@ -11,7 +11,6 @@ CONSOLE MESSAGE: Blocked a frame with origin "http://127.0.0.1:8000" from access
 CONSOLE MESSAGE: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
 CONSOLE MESSAGE: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
 CONSOLE MESSAGE: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
-CONSOLE MESSAGE: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
 This tests that variable names can't be enumerated cross domain (see http://bugs.webkit.org/show_bug.cgi?id=16387)
 
 
index d1104b1..6a16079 100644 (file)
@@ -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
index 6345871..d65a2f2 100644 (file)
@@ -1422,9 +1422,6 @@ RegisterID* BytecodeGenerator::emitGetByVal(RegisterID* dst, RegisterID* base, R
 {
     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 @@ RegisterID* BytecodeGenerator::emitToIndexString(RegisterID* dst, RegisterID* in
     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 @@ void BytecodeGenerator::popIndexedForInScope(RegisterID* localRegister)
     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)
index 7c9762d..3eba8ae 100644 (file)
@@ -99,9 +99,8 @@ namespace JSC {
 
     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 @@ namespace JSC {
         };
         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 @@ namespace JSC {
 
     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 @@ namespace JSC {
         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);
 
index 6acfc01..573dbec 100644 (file)
@@ -2070,7 +2070,7 @@ void ForInNode::emitMultiLoopBytecode(BytecodeGenerator& generator, RegisterID*
         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 @@ void ForInNode::emitMultiLoopBytecode(BytecodeGenerator& generator, RegisterID*
 
         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());
 
index 4070bf1..f4c6829 100644 (file)
@@ -120,16 +120,17 @@ uint32_t JSProxy::getEnumerableLength(ExecState* exec, JSObject* object)
     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 (file)
index 0000000..1b087d5
--- /dev/null
@@ -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 (file)
index 0000000..83115b4
--- /dev/null
@@ -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 (file)
index 0000000..09c8795
--- /dev/null
@@ -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 (file)
index 0000000..5c9d76f
--- /dev/null
@@ -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 (file)
index 0000000..44357db
--- /dev/null
@@ -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;
+}