Title: [212021] trunk
Revision
212021
Author
bfulg...@apple.com
Date
2017-02-09 18:02:30 -0800 (Thu, 09 Feb 2017)

Log Message

Sloppy mode: We don't properly hoist functions names "arguments" when we have a non-simple parameter list
https://bugs.webkit.org/show_bug.cgi?id=167319
<rdar://problem/30149432>

Patch by Saam Barati <sbar...@apple.com> on 2017-02-09
Reviewed by Mark Lam.

JSTests:

* stress/sloppy-mode-hoist-arguments-function-non-simple-parameter-list.js: Added.
(assert):
(assert.arguments):
(assert.b):
(x.arguments):
(x.b):
(x):

Source/_javascript_Core:

When hoisting a function inside sloppy mode, we were assuming all "var"s are inside
what we call the "var" SymbolTableEntry. This was almost true, execpt for "arguments",
which has sufficiently weird behavior. "arguments" can be visible to the default
parameter expressions inside a function, therefore can't go inside the "var"
SymbolTableEntry since the parameter SymbolTableEntry comes before the "var"
SymbolTableEntry in the scope chain.  Therefore, if we hoist a function named
"arguments", then we must also look for that variable inside the parameter scope
stack entry.

* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::hoistSloppyModeFunctionIfNecessary):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (212020 => 212021)


--- trunk/JSTests/ChangeLog	2017-02-10 01:55:30 UTC (rev 212020)
+++ trunk/JSTests/ChangeLog	2017-02-10 02:02:30 UTC (rev 212021)
@@ -1,3 +1,19 @@
+2017-02-09  Saam Barati  <sbar...@apple.com>
+
+        Sloppy mode: We don't properly hoist functions names "arguments" when we have a non-simple parameter list
+        https://bugs.webkit.org/show_bug.cgi?id=167319
+        <rdar://problem/30149432>
+
+        Reviewed by Mark Lam.
+
+        * stress/sloppy-mode-hoist-arguments-function-non-simple-parameter-list.js: Added.
+        (assert):
+        (assert.arguments):
+        (assert.b):
+        (x.arguments):
+        (x.b):
+        (x):
+
 2017-02-09  Mark Lam  <mark....@apple.com>
 
         Fix max length check in ArrayPrototype.js' concatSlowPath().

Added: trunk/JSTests/stress/sloppy-mode-hoist-arguments-function-non-simple-parameter-list.js (0 => 212021)


--- trunk/JSTests/stress/sloppy-mode-hoist-arguments-function-non-simple-parameter-list.js	                        (rev 0)
+++ trunk/JSTests/stress/sloppy-mode-hoist-arguments-function-non-simple-parameter-list.js	2017-02-10 02:02:30 UTC (rev 212021)
@@ -0,0 +1,158 @@
+function assert(b) {
+    if (!b)
+        throw new Error("Bad assertion")
+}
+
+(function (x = 20) {
+    var a;
+    assert(arguments.length === 0);
+    assert(typeof arguments !== "function");
+    {
+        function arguments() {
+        }
+
+        function b() {
+            var g = 1;
+            a[5];
+        }
+    }
+    assert(typeof arguments === "function");
+}());
+
+(function (x = () => arguments) {
+    var a;
+    let originalArguments = x();
+    assert(originalArguments === arguments);
+    let z;
+    {
+        function arguments() { return 25; }
+        z = arguments;
+
+        function b() {
+            var g = 1;
+            a[5];
+        }
+    }
+    assert(z !== originalArguments);
+    assert(x() === z);
+    assert(typeof z === "function");
+    assert(z() === 25);
+}());
+
+(function ({arguments}) {
+    assert(arguments === 20);
+
+    var a;
+    {
+        function arguments() { return 25; }
+        assert(arguments() === 25);
+
+        function b() {
+            var g = 1;
+            a[5];
+        }
+    }
+
+    assert(arguments === 20);
+}({arguments: 20}));
+
+(function (y = () => arguments, {arguments}) {
+    assert(y() === arguments);
+    var a;
+    {
+        function arguments() { return 25; }
+        assert(arguments() === 25);
+        assert(y() !== arguments);
+
+        function b() {
+            var g = 1;
+            a[5];
+        }
+    }
+
+    assert(y() === arguments);
+}(undefined, {arguments: {}}));
+
+(function (y = () => arguments, z = y(), {arguments}) {
+    assert(typeof z === "object");
+    assert(z.length === 3);
+    assert(z[0] === undefined);
+    assert(z[1] === undefined);
+    assert(typeof z[2] === "object");
+    assert(z[2].arguments === arguments);
+    assert(y() === arguments);
+
+    var a;
+    {
+        function arguments() { return 25; }
+        assert(arguments() === 25);
+        assert(y() !== arguments);
+
+        function b() {
+            var g = 1;
+            a[5];
+        }
+    }
+
+    assert(y() === arguments);
+}(undefined, undefined, {arguments: {}}));
+
+(function (arguments) {
+    assert(arguments === 25);
+
+    var a;
+    {
+        function arguments() { return 30; }
+        assert(arguments() === 30);
+
+        function b() {
+            var g = 1;
+            a[5];
+        }
+    }
+
+    assert(arguments === 25);
+}(25));
+
+(function (arguments) {
+    assert(arguments === 25);
+    let foo = () => arguments;
+    assert(foo() === arguments);
+
+    var a;
+    {
+        function arguments() { return 30; }
+        assert(arguments() === 30);
+        assert(foo() === 25);
+
+        function b() {
+            var g = 1;
+            a[5];
+        }
+    }
+
+    assert(arguments === 25);
+    assert(foo() === arguments);
+}(25));
+
+(function () {
+    assert(arguments.length === 1);
+    assert(arguments[0] === 25);
+
+    let outer = () => arguments;
+    var a;
+    {
+        assert(outer()[0] === 25);
+        function arguments() { return 30; }
+        assert(outer() === arguments);
+        assert(outer()() === 30);
+        assert(arguments() === 30);
+
+        function b() {
+            var g = 1;
+            a[5];
+        }
+    }
+
+    assert(arguments() === 30);
+}(25));

Modified: trunk/Source/_javascript_Core/ChangeLog (212020 => 212021)


--- trunk/Source/_javascript_Core/ChangeLog	2017-02-10 01:55:30 UTC (rev 212020)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-02-10 02:02:30 UTC (rev 212021)
@@ -1,3 +1,23 @@
+2017-02-09  Saam Barati  <sbar...@apple.com>
+
+        Sloppy mode: We don't properly hoist functions names "arguments" when we have a non-simple parameter list
+        https://bugs.webkit.org/show_bug.cgi?id=167319
+        <rdar://problem/30149432>
+
+        Reviewed by Mark Lam.
+
+        When hoisting a function inside sloppy mode, we were assuming all "var"s are inside
+        what we call the "var" SymbolTableEntry. This was almost true, execpt for "arguments",
+        which has sufficiently weird behavior. "arguments" can be visible to the default
+        parameter expressions inside a function, therefore can't go inside the "var"
+        SymbolTableEntry since the parameter SymbolTableEntry comes before the "var"
+        SymbolTableEntry in the scope chain.  Therefore, if we hoist a function named
+        "arguments", then we must also look for that variable inside the parameter scope
+        stack entry.
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::hoistSloppyModeFunctionIfNecessary):
+
 2017-02-09  Mark Lam  <mark....@apple.com>
 
         Fix max length check in ArrayPrototype.js' concatSlowPath().

Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (212020 => 212021)


--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2017-02-10 01:55:30 UTC (rev 212020)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2017-02-10 02:02:30 UTC (rev 212021)
@@ -2154,11 +2154,22 @@
         
         ASSERT(m_varScopeLexicalScopeStackIndex);
         ASSERT(*m_varScopeLexicalScopeStackIndex < m_lexicalScopeStack.size());
-        auto& varScope = m_lexicalScopeStack[*m_varScopeLexicalScopeStackIndex];
+        LexicalScopeStackEntry varScope = m_lexicalScopeStack[*m_varScopeLexicalScopeStackIndex];
         SymbolTable* varSymbolTable = varScope.m_symbolTable;
         ASSERT(varSymbolTable->scopeType() == SymbolTable::ScopeType::VarScope);
         SymbolTableEntry entry = varSymbolTable->get(NoLockingNecessary, functionName.impl());
-        ASSERT(!entry.isNull());
+        if (functionName == propertyNames().arguments && entry.isNull()) {
+            // "arguments" might be put in the parameter scope when we have a non-simple
+            // parameter list since "arguments" is visible to expressions inside the
+            // parameter evaluation list.
+            // e.g:
+            // function foo(x = arguments) { { function arguments() { } } }
+            RELEASE_ASSERT(*m_varScopeLexicalScopeStackIndex > 0);
+            varScope = m_lexicalScopeStack[*m_varScopeLexicalScopeStackIndex - 1];
+            SymbolTable* parameterSymbolTable = varScope.m_symbolTable;
+            entry = parameterSymbolTable->get(NoLockingNecessary, functionName.impl());
+        }
+        RELEASE_ASSERT(!entry.isNull());
         bool isLexicallyScoped = false;
         emitPutToScope(varScope.m_scope, variableForLocalEntry(functionName, entry, varScope.m_symbolTableConstantIndex, isLexicallyScoped), currentValue.get(), DoNotThrowIfNotFound, InitializationMode::NotInitialization);
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to