Title: [172794] trunk
Revision
172794
Author
[email protected]
Date
2014-08-19 19:38:46 -0700 (Tue, 19 Aug 2014)

Log Message

REGRESSION(r172401): for-in optimization no longer works at all
https://bugs.webkit.org/show_bug.cgi?id=136056

Reviewed by Geoffrey Garen.
        
Source/_javascript_Core:

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:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (172793 => 172794)


--- trunk/LayoutTests/ChangeLog	2014-08-20 02:17:57 UTC (rev 172793)
+++ trunk/LayoutTests/ChangeLog	2014-08-20 02:38:46 UTC (rev 172794)
@@ -1,3 +1,15 @@
+2014-08-19  Filip Pizlo  <[email protected]>
+
+        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  <[email protected]>
 
         [CSS Shapes] A few calc() test failures in the shape-image-threshold parsing tests

Modified: trunk/LayoutTests/http/tests/security/cross-frame-access-enumeration-expected.txt (172793 => 172794)


--- trunk/LayoutTests/http/tests/security/cross-frame-access-enumeration-expected.txt	2014-08-20 02:17:57 UTC (rev 172793)
+++ trunk/LayoutTests/http/tests/security/cross-frame-access-enumeration-expected.txt	2014-08-20 02:38:46 UTC (rev 172794)
@@ -11,7 +11,6 @@
 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)
 
 

Modified: trunk/Source/_javascript_Core/ChangeLog (172793 => 172794)


--- trunk/Source/_javascript_Core/ChangeLog	2014-08-20 02:17:57 UTC (rev 172793)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-08-20 02:38:46 UTC (rev 172794)
@@ -1,3 +1,39 @@
+2014-08-19  Filip Pizlo  <[email protected]>
+
+        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  <[email protected]>
 
         Unreviewed, fix EFL build after r17275

Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (172793 => 172794)


--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2014-08-20 02:17:57 UTC (rev 172793)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2014-08-20 02:38:46 UTC (rev 172794)
@@ -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)

Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (172793 => 172794)


--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2014-08-20 02:17:57 UTC (rev 172793)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2014-08-20 02:38:46 UTC (rev 172794)
@@ -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);
 

Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (172793 => 172794)


--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2014-08-20 02:17:57 UTC (rev 172793)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2014-08-20 02:38:46 UTC (rev 172794)
@@ -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());
 

Modified: trunk/Source/_javascript_Core/runtime/JSProxy.cpp (172793 => 172794)


--- trunk/Source/_javascript_Core/runtime/JSProxy.cpp	2014-08-20 02:17:57 UTC (rev 172793)
+++ trunk/Source/_javascript_Core/runtime/JSProxy.cpp	2014-08-20 02:38:46 UTC (rev 172794)
@@ -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)

Added: trunk/Source/_javascript_Core/tests/stress/for-in-base-reassigned-later-and-change-structure.js (0 => 172794)


--- trunk/Source/_javascript_Core/tests/stress/for-in-base-reassigned-later-and-change-structure.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/for-in-base-reassigned-later-and-change-structure.js	2014-08-20 02:38:46 UTC (rev 172794)
@@ -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;
+}

Added: trunk/Source/_javascript_Core/tests/stress/for-in-base-reassigned-later.js (0 => 172794)


--- trunk/Source/_javascript_Core/tests/stress/for-in-base-reassigned-later.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/for-in-base-reassigned-later.js	2014-08-20 02:38:46 UTC (rev 172794)
@@ -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;
+}

Added: trunk/Source/_javascript_Core/tests/stress/for-in-base-reassigned.js (0 => 172794)


--- trunk/Source/_javascript_Core/tests/stress/for-in-base-reassigned.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/for-in-base-reassigned.js	2014-08-20 02:38:46 UTC (rev 172794)
@@ -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;
+}

Added: trunk/Source/_javascript_Core/tests/stress/for-in-proxy-target-changed-structure.js (0 => 172794)


--- trunk/Source/_javascript_Core/tests/stress/for-in-proxy-target-changed-structure.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/for-in-proxy-target-changed-structure.js	2014-08-20 02:38:46 UTC (rev 172794)
@@ -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;
+}

Added: trunk/Source/_javascript_Core/tests/stress/for-in-proxy.js (0 => 172794)


--- trunk/Source/_javascript_Core/tests/stress/for-in-proxy.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/for-in-proxy.js	2014-08-20 02:38:46 UTC (rev 172794)
@@ -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;
+}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to