Title: [178432] trunk
Revision
178432
Author
msab...@apple.com
Date
2015-01-14 11:38:21 -0800 (Wed, 14 Jan 2015)

Log Message

REGRESSION (r174226): Header on huffingtonpost.com is too large
https://bugs.webkit.org/show_bug.cgi?id=140306

Reviewed by Geoffrey Garen.

Source/_javascript_Core:

BytecodeGenerator::willResolveToArguments() is used to check to see if we can use the
arguments register or whether we need to resolve "arguments".  If the arguments have
been captured, then they are stored in the lexical environment and the arguments
register is not used.
Changed BytecodeGenerator::willResolveToArguments() to also check to see if the arguments
register is captured.  Renamed the function to willResolveToArgumentsRegister() to
better indicate what we are checking.

* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::willResolveToArgumentsRegister):
(JSC::BytecodeGenerator::uncheckedLocalArgumentsRegister):
(JSC::BytecodeGenerator::emitCall):
(JSC::BytecodeGenerator::emitConstruct):
(JSC::BytecodeGenerator::emitEnumeration):
(JSC::BytecodeGenerator::willResolveToArguments): Deleted.
* bytecompiler/BytecodeGenerator.h:
* bytecompiler/NodesCodegen.cpp:
(JSC::BracketAccessorNode::emitBytecode):
(JSC::DotAccessorNode::emitBytecode):
(JSC::getArgumentByVal):
(JSC::ApplyFunctionCallDotNode::emitBytecode):
(JSC::ArrayPatternNode::emitDirectBinding):

LayoutTests:

Updated js/arguments-iterator to test changing argument to array values.
Removed tests that changed arguments to a string and an object as they were
bogus and didn't test what the appeared to test. 
for .. of works on iterable objects only.

Added new regression test, js/regress-140306.

* js/arguments-iterator-expected.txt:
* js/regress-140306-expected.txt: Added.
* js/regress-140306.html: Added.
* js/script-tests/arguments-iterator.js:
(testEmptyArrayArguments):
(testArrayArguments):
(testOverwrittenArguments): Deleted.
(testNullArguments): Deleted.
(testNonArrayLikeArguments): Deleted.
* js/script-tests/regress-140306.js: Added.
(checkArgs):
(applyToArgs):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (178431 => 178432)


--- trunk/LayoutTests/ChangeLog	2015-01-14 19:08:29 UTC (rev 178431)
+++ trunk/LayoutTests/ChangeLog	2015-01-14 19:38:21 UTC (rev 178432)
@@ -1,5 +1,32 @@
 2015-01-14  Michael Saboff  <msab...@apple.com>
 
+        REGRESSION (r174226): Header on huffingtonpost.com is too large
+        https://bugs.webkit.org/show_bug.cgi?id=140306
+
+        Reviewed by Geoffrey Garen.
+
+        Updated js/arguments-iterator to test changing argument to array values.
+        Removed tests that changed arguments to a string and an object as they were
+        bogus and didn't test what the appeared to test. 
+        for .. of works on iterable objects only.
+
+        Added new regression test, js/regress-140306.
+
+        * js/arguments-iterator-expected.txt:
+        * js/regress-140306-expected.txt: Added.
+        * js/regress-140306.html: Added.
+        * js/script-tests/arguments-iterator.js:
+        (testEmptyArrayArguments):
+        (testArrayArguments):
+        (testOverwrittenArguments): Deleted.
+        (testNullArguments): Deleted.
+        (testNonArrayLikeArguments): Deleted.
+        * js/script-tests/regress-140306.js: Added.
+        (checkArgs):
+        (applyToArgs):
+
+2015-01-14  Michael Saboff  <msab...@apple.com>
+
         _javascript_ identifier incorrectly parsed if the prefix before an escape sequence is a keyword
         https://bugs.webkit.org/show_bug.cgi?id=140420
 

Modified: trunk/LayoutTests/js/arguments-iterator-expected.txt (178431 => 178432)


--- trunk/LayoutTests/js/arguments-iterator-expected.txt	2015-01-14 19:08:29 UTC (rev 178431)
+++ trunk/LayoutTests/js/arguments-iterator-expected.txt	2015-01-14 19:38:21 UTC (rev 178432)
@@ -36,36 +36,24 @@
 PASS actualArgumentsLength is iteratedArgumentsLength
 PASS arg === realArg is true
 PASS actualArgumentsLength is iteratedArgumentsLength
+PASS testEmptyArrayArguments('a') is true
+PASS testEmptyArrayArguments() is true
 PASS arg === realArg is true
 PASS arg === realArg is true
 PASS arg === realArg is true
-PASS arg === realArg is true
-PASS arg === realArg is true
-PASS arg === realArg is true
 PASS actualArgumentsLength is iteratedArgumentsLength
 PASS arg === realArg is true
 PASS arg === realArg is true
 PASS arg === realArg is true
-PASS arg === realArg is true
-PASS arg === realArg is true
-PASS arg === realArg is true
 PASS actualArgumentsLength is iteratedArgumentsLength
 PASS arg === realArg is true
 PASS arg === realArg is true
 PASS arg === realArg is true
-PASS arg === realArg is true
-PASS arg === realArg is true
-PASS arg === realArg is true
 PASS actualArgumentsLength is iteratedArgumentsLength
 PASS arg === realArg is true
 PASS arg === realArg is true
 PASS arg === realArg is true
-PASS arg === realArg is true
-PASS arg === realArg is true
-PASS arg === realArg is true
 PASS actualArgumentsLength is iteratedArgumentsLength
-PASS testNullArguments() threw exception TypeError: null is not an object (evaluating 'fail("nothing to iterate")').
-PASS testNonArrayLikeArguments() is true
 PASS successfullyParsed is true
 
 TEST COMPLETE

Added: trunk/LayoutTests/js/regress-140306-expected.txt (0 => 178432)


--- trunk/LayoutTests/js/regress-140306-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/js/regress-140306-expected.txt	2015-01-14 19:38:21 UTC (rev 178432)
@@ -0,0 +1,9 @@
+Regression test for https://webkit.org/b/140306. This test should run without any exceptions.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/js/regress-140306.html (0 => 178432)


--- trunk/LayoutTests/js/regress-140306.html	                        (rev 0)
+++ trunk/LayoutTests/js/regress-140306.html	2015-01-14 19:38:21 UTC (rev 178432)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>

Modified: trunk/LayoutTests/js/script-tests/arguments-iterator.js (178431 => 178432)


--- trunk/LayoutTests/js/script-tests/arguments-iterator.js	2015-01-14 19:08:29 UTC (rev 178431)
+++ trunk/LayoutTests/js/script-tests/arguments-iterator.js	2015-01-14 19:38:21 UTC (rev 178432)
@@ -76,43 +76,33 @@
 testReifiedArguments({})
 
 
-function testOverwrittenArguments() {
-    var i = 0;
-    arguments = "foobar";
+function testEmptyArrayArguments() {
+    arguments = [];
     for (arg of arguments) {
-        realArg = arguments[i++];
-        shouldBeTrue("arg === realArg");
+        fail("nothing to iterate");
+        return false;
     }
-    iteratedArgumentsLength = i;
-    actualArgumentsLength = arguments.length;
-    shouldBe("actualArgumentsLength", "iteratedArgumentsLength");
+
+    return true;
 }
 
+shouldBeTrue("testEmptyArrayArguments('a')");
+shouldBeTrue("testEmptyArrayArguments()");
 
-testOverwrittenArguments();
-testOverwrittenArguments("a");
-testOverwrittenArguments("a", "b");
-testOverwrittenArguments({})
 
-
-
-function testNullArguments() {
+function testArrayArguments() {
     var i = 0;
-    arguments = null;
+    arguments = [1, 2, 3];
     for (arg of arguments) {
-        fail("nothing to iterate");
+        realArg = arguments[i++];
+        shouldBeTrue("arg === realArg");
     }
+    iteratedArgumentsLength = i;
+    actualArgumentsLength = arguments.length;
+    shouldBe("actualArgumentsLength", "iteratedArgumentsLength");
 }
 
-shouldThrow("testNullArguments()");
-function testNonArrayLikeArguments() {
-    var i = 0;
-    arguments = {};
-    for (arg of arguments) {
-        fail("nothing to iterate");
-        return false;
-    }
-    return true;
-}
-shouldBeTrue("testNonArrayLikeArguments()");
-
+testArrayArguments();
+testArrayArguments("a");
+testArrayArguments("a", "b");
+testArrayArguments({});

Added: trunk/LayoutTests/js/script-tests/regress-140306.js (0 => 178432)


--- trunk/LayoutTests/js/script-tests/regress-140306.js	                        (rev 0)
+++ trunk/LayoutTests/js/script-tests/regress-140306.js	2015-01-14 19:38:21 UTC (rev 178432)
@@ -0,0 +1,34 @@
+description(
+"Regression test for https://webkit.org/b/140306. This test should run without any exceptions."
+);
+
+testArgs = [ 1, "Second", new Number(3) ];
+
+function checkArgs(a0, a1, a2) {
+    if (a0 !== testArgs[0])
+        throw "Value of declared arg a0 is wrong.  Should be: " + testArgs[0] + ", was: " + a0;
+
+    if (a1 !== testArgs[1])
+        throw "Value of declared arg a1 is wrong.  Should be: " + testArgs[1] + ", was: " + a1;
+
+    if (a2 !== testArgs[2])
+        throw "Value of declared arg a2 is wrong.  Should be: " + testArgs[2] + ", was: " + a2;
+
+    if (arguments.length != 3)
+        throw "Length of arguments is wrong.  Should be: 3, was: " + arguments.length;
+
+    for (var i = 0; i < arguments.length; i++) {
+        if (arguments[i] !== testArgs[i])
+            throw "Value of arguments[" + i + "] is wrong.  Should be: " + testArgs[i] + ", was: " + arguments[i];
+    }
+}
+
+function applyToArgs() {
+    arguments = testArgs;
+
+    checkArgs.apply(this, arguments)
+
+    try { } catch (e) { throw e; }  // To force the creation of an activation object
+}
+
+applyToArgs(42);

Modified: trunk/Source/_javascript_Core/ChangeLog (178431 => 178432)


--- trunk/Source/_javascript_Core/ChangeLog	2015-01-14 19:08:29 UTC (rev 178431)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-01-14 19:38:21 UTC (rev 178432)
@@ -1,5 +1,35 @@
 2015-01-14  Michael Saboff  <msab...@apple.com>
 
+        REGRESSION (r174226): Header on huffingtonpost.com is too large
+        https://bugs.webkit.org/show_bug.cgi?id=140306
+
+        Reviewed by Geoffrey Garen.
+
+        BytecodeGenerator::willResolveToArguments() is used to check to see if we can use the
+        arguments register or whether we need to resolve "arguments".  If the arguments have
+        been captured, then they are stored in the lexical environment and the arguments
+        register is not used.
+        Changed BytecodeGenerator::willResolveToArguments() to also check to see if the arguments
+        register is captured.  Renamed the function to willResolveToArgumentsRegister() to
+        better indicate what we are checking.
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::willResolveToArgumentsRegister):
+        (JSC::BytecodeGenerator::uncheckedLocalArgumentsRegister):
+        (JSC::BytecodeGenerator::emitCall):
+        (JSC::BytecodeGenerator::emitConstruct):
+        (JSC::BytecodeGenerator::emitEnumeration):
+        (JSC::BytecodeGenerator::willResolveToArguments): Deleted.
+        * bytecompiler/BytecodeGenerator.h:
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::BracketAccessorNode::emitBytecode):
+        (JSC::DotAccessorNode::emitBytecode):
+        (JSC::getArgumentByVal):
+        (JSC::ApplyFunctionCallDotNode::emitBytecode):
+        (JSC::ArrayPatternNode::emitDirectBinding):
+
+2015-01-14  Michael Saboff  <msab...@apple.com>
+
         _javascript_ identifier incorrectly parsed if the prefix before an escape sequence is a keyword
         https://bugs.webkit.org/show_bug.cgi?id=140420
 

Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (178431 => 178432)


--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2015-01-14 19:08:29 UTC (rev 178431)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2015-01-14 19:38:21 UTC (rev 178432)
@@ -563,7 +563,7 @@
     m_codeBlock->addParameter();
 }
 
-bool BytecodeGenerator::willResolveToArguments(const Identifier& ident)
+bool BytecodeGenerator::willResolveToArgumentsRegister(const Identifier& ident)
 {
     if (ident != propertyNames().arguments)
         return false;
@@ -575,6 +575,9 @@
     if (entry.isNull())
         return false;
 
+    if (m_localArgumentsRegister && isCaptured(m_localArgumentsRegister->index()))
+        return false;
+
     if (m_codeBlock->usesArguments() && m_codeType == FunctionCode && m_localArgumentsRegister)
         return true;
     
@@ -583,7 +586,7 @@
 
 RegisterID* BytecodeGenerator::uncheckedLocalArgumentsRegister()
 {
-    ASSERT(willResolveToArguments(propertyNames().arguments));
+    ASSERT(willResolveToArgumentsRegister(propertyNames().arguments));
     ASSERT(m_localArgumentsRegister);
     return m_localArgumentsRegister;
 }
@@ -1874,7 +1877,7 @@
             RELEASE_ASSERT(!n->m_next);
             auto _expression_ = static_cast<SpreadExpressionNode*>(n->m_expr)->_expression_();
             RefPtr<RegisterID> argumentRegister;
-            if (_expression_->isResolveNode() && willResolveToArguments(static_cast<ResolveNode*>(_expression_)->identifier()) && !symbolTable().slowArguments())
+            if (_expression_->isResolveNode() && willResolveToArgumentsRegister(static_cast<ResolveNode*>(_expression_)->identifier()) && !symbolTable().slowArguments())
                 argumentRegister = uncheckedLocalArgumentsRegister();
             else
                 argumentRegister = _expression_->emitBytecode(*this, callArguments.argumentRegister(0));
@@ -2016,7 +2019,7 @@
             RELEASE_ASSERT(!n->m_next);
             auto _expression_ = static_cast<SpreadExpressionNode*>(n->m_expr)->_expression_();
             RefPtr<RegisterID> argumentRegister;
-            if (_expression_->isResolveNode() && willResolveToArguments(static_cast<ResolveNode*>(_expression_)->identifier()) && !symbolTable().slowArguments())
+            if (_expression_->isResolveNode() && willResolveToArgumentsRegister(static_cast<ResolveNode*>(_expression_)->identifier()) && !symbolTable().slowArguments())
                 argumentRegister = uncheckedLocalArgumentsRegister();
             else
                 argumentRegister = _expression_->emitBytecode(*this, callArguments.argumentRegister(0));
@@ -2594,7 +2597,7 @@
 void BytecodeGenerator::emitEnumeration(ThrowableExpressionData* node, ExpressionNode* subjectNode, const std::function<void(BytecodeGenerator&, RegisterID*)>& callBack)
 {
     if (subjectNode->isResolveNode()
-        && willResolveToArguments(static_cast<ResolveNode*>(subjectNode)->identifier())
+        && willResolveToArgumentsRegister(static_cast<ResolveNode*>(subjectNode)->identifier())
         && !symbolTable().slowArguments()) {
         RefPtr<RegisterID> index = emitLoad(newTemporary(), jsNumber(0));
 

Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (178431 => 178432)


--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2015-01-14 19:08:29 UTC (rev 178431)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2015-01-14 19:38:21 UTC (rev 178432)
@@ -277,7 +277,7 @@
 
         void setIsNumericCompareFunction(bool isNumericCompareFunction);
 
-        bool willResolveToArguments(const Identifier&);
+        bool willResolveToArgumentsRegister(const Identifier&);
 
         bool hasSafeLocalArgumentsRegister() { return m_localArgumentsRegister; }
         RegisterID* uncheckedLocalArgumentsRegister();

Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (178431 => 178432)


--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2015-01-14 19:08:29 UTC (rev 178431)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2015-01-14 19:38:21 UTC (rev 178432)
@@ -382,7 +382,7 @@
 RegisterID* BracketAccessorNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
 {
     if (m_base->isResolveNode() 
-        && generator.willResolveToArguments(static_cast<ResolveNode*>(m_base)->identifier())
+        && generator.willResolveToArgumentsRegister(static_cast<ResolveNode*>(m_base)->identifier())
         && !generator.symbolTable().slowArguments()) {
         RefPtr<RegisterID> property = generator.emitNode(m_subscript);
         generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
@@ -418,7 +418,7 @@
         if (!m_base->isResolveNode())
             goto nonArgumentsPath;
         ResolveNode* resolveNode = static_cast<ResolveNode*>(m_base);
-        if (!generator.willResolveToArguments(resolveNode->identifier()))
+        if (!generator.willResolveToArgumentsRegister(resolveNode->identifier()))
             goto nonArgumentsPath;
         generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
         return generator.emitGetArgumentsLength(generator.finalDestination(dst), generator.uncheckedLocalArgumentsRegister());
@@ -599,7 +599,7 @@
 static RegisterID* getArgumentByVal(BytecodeGenerator& generator, ExpressionNode* base, RegisterID* property, RegisterID* dst, JSTextPosition divot, JSTextPosition divotStart, JSTextPosition divotEnd)
 {
     if (base->isResolveNode()
-        && generator.willResolveToArguments(static_cast<ResolveNode*>(base)->identifier())
+        && generator.willResolveToArgumentsRegister(static_cast<ResolveNode*>(base)->identifier())
         && !generator.symbolTable().slowArguments()) {
         generator.emitExpressionInfo(divot, divotStart, divotEnd);
         return generator.emitGetArgumentByVal(generator.finalDestination(dst), generator.uncheckedLocalArgumentsRegister(), property);
@@ -757,7 +757,7 @@
         RefPtr<RegisterID> thisRegister = generator.emitNode(m_args->m_listNode->m_expr);
         RefPtr<RegisterID> argsRegister;
         ArgumentListNode* args = m_args->m_listNode->m_next;
-        if (args->m_expr->isResolveNode() && generator.willResolveToArguments(static_cast<ResolveNode*>(args->m_expr)->identifier()) && !generator.symbolTable().slowArguments())
+        if (args->m_expr->isResolveNode() && generator.willResolveToArgumentsRegister(static_cast<ResolveNode*>(args->m_expr)->identifier()) && !generator.symbolTable().slowArguments())
             argsRegister = generator.uncheckedLocalArgumentsRegister();
         else
             argsRegister = generator.emitNode(args->m_expr);
@@ -2776,7 +2776,7 @@
 RegisterID* ArrayPatternNode::emitDirectBinding(BytecodeGenerator& generator, RegisterID* dst, ExpressionNode* rhs)
 {
     if (rhs->isResolveNode()
-        && generator.willResolveToArguments(static_cast<ResolveNode*>(rhs)->identifier())
+        && generator.willResolveToArgumentsRegister(static_cast<ResolveNode*>(rhs)->identifier())
         && generator.hasSafeLocalArgumentsRegister()&& !generator.symbolTable().slowArguments()) {
         for (size_t i = 0; i < m_targetPatterns.size(); i++) {
             auto target = m_targetPatterns[i];
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to