Changeset 215986 in webkit

Timestamp:

Apr 30, 2017, 7:51:27 AM (8 years ago)

Author:

gskachkov@gmail.com

Message:

Initialize functions too early in an eval
​https://bugs.webkit.org/show_bug.cgi?id=161099

Reviewed by Saam Barati.

JSTests:

• stress/eval-func-decl-with-let-const-class.js: Added.

Source/JavaScriptCore:

Current patch allow to fix problem with scope in function that is
declared within eval. Before scope was set inside Interpretator.cpp and it
was scope where eval is executed, but in this case function would not
see let/const variables and classes declated in eval.
This patch devide declaration and binding in two operation, first just declare
variable with function name, and second bind variable to function with correct
scope

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::generate):
(JSC::BytecodeGenerator::BytecodeGenerator):

• bytecompiler/BytecodeGenerator.h:
• interpreter/Interpreter.cpp:

(JSC::Interpreter::execute):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/eval-func-decl-with-let-const-class.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp (modified) (4 diffs)
• Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h (modified) (1 diff)
• Source/JavaScriptCore/interpreter/Interpreter.cpp (modified) (2 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2017-04-30  Oleksandr Skachkov  <gskachkov@gmail.com>
+
+        We initialize functions too early in an eval
+        https://bugs.webkit.org/show_bug.cgi?id=161099
+
+        Reviewed by Saam Barati.
+
+        * stress/eval-func-decl-with-let-const-class.js: Added.
+
 2017-04-30  Oleksandr Skachkov  <gskachkov@gmail.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2017-04-30  Oleksandr Skachkov  <gskachkov@gmail.com>
+
+        We initialize functions too early in an eval
+        https://bugs.webkit.org/show_bug.cgi?id=161099
+
+        Reviewed by Saam Barati.
+
+        Current patch allow to fix problem with scope in function that is 
+        declared within eval. Before scope was set inside Interpretator.cpp and it
+        was scope where eval is executed, but in this case function would not 
+        see let/const variables and classes declated in eval.
+        This patch devide declaration and binding in two operation, first just declare
+        variable with function name, and second bind variable to function with correct 
+        scope
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::generate):
+        (JSC::BytecodeGenerator::BytecodeGenerator):
+        * bytecompiler/BytecodeGenerator.h:
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::execute):
+
 2017-04-30  Oleksandr Skachkov  <gskachkov@gmail.com>
 

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -107 +107 @@
     {
         RefPtr<RegisterID> temp = newTemporary();
-        RefPtr<RegisterID> globalScope;
+        RefPtr<RegisterID> tolLevelScope;
         for (auto functionPair : m_functionsToInitialize) {
             FunctionMetadataNode* metadata = functionPair.first;
@@ -114 +114 @@
             if (functionType == NormalFunctionVariable)
                 initializeVariable(variable(metadata->ident()), temp.get());
-            else if (functionType == GlobalFunctionVariable) {
-                if (!globalScope) {
-                    // We know this will resolve to the global object because our parser/global initialization code 
+            else if (functionType == TopLevelFunctionVariable) {
+                if (!tolLevelScope) {
+                    // We know this will resolve to the top level scope or global object because our parser/global initialization code 
                     // doesn't allow let/const/class variables to have the same names as functions.
-                    RefPtr<RegisterID> globalObjectScope = emitResolveScope(nullptr, Variable(metadata->ident())); 
-                    globalScope = newBlockScopeVariable(); 
-                    emitMove(globalScope.get(), globalObjectScope.get());
+                    // This is a top level function, and it's an error to ever create a top level function
+                    // name that would resolve to a lexical variable. E.g:
+                    // ```
+                    //     function f() {
+                    //         {
+                    //             let x;
+                    //             {
+                    //             //// error thrown here
+                    //                  eval("function x(){}");
+                    //             }
+                    //         }
+                    //     }
+                    // ```
+                    // Therefore, we're guaranteed to have this resolve to a top level variable.
+                    RefPtr<RegisterID> tolLevelObjectScope = emitResolveScope(nullptr, Variable(metadata->ident()));
+                    tolLevelScope = newBlockScopeVariable();
+                    emitMove(tolLevelScope.get(), tolLevelObjectScope.get());
                 }
-                emitPutToScope(globalScope.get(), Variable(metadata->ident()), temp.get(), ThrowIfNotFound, InitializationMode::NotInitialization);
+                emitPutToScope(tolLevelScope.get(), Variable(metadata->ident()), temp.get(), ThrowIfNotFound, InitializationMode::NotInitialization);
             } else
                 RELEASE_ASSERT_NOT_REACHED();
@@ -208 +222 @@
 
     for (auto* function : functionStack)
-        m_functionsToInitialize.append(std::make_pair(function, GlobalFunctionVariable));
+        m_functionsToInitialize.append(std::make_pair(function, TopLevelFunctionVariable));
 
     if (Options::validateBytecode()) {
@@ -765 +779 @@
     emitCheckTraps();
     
-    for (FunctionMetadataNode* function : evalNode->functionStack())
+    for (FunctionMetadataNode* function : evalNode->functionStack()) {
         m_codeBlock->addFunctionDecl(makeFunction(function));
+        m_functionsToInitialize.append(std::make_pair(function, TopLevelFunctionVariable));
+    }
 
     const VariableEnvironment& varDeclarations = evalNode->varDeclarations();

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

@@ -1112 +1112 @@
         int m_generatorFrameSymbolTableIndex { 0 };
 
-        enum FunctionVariableType : uint8_t { NormalFunctionVariable, GlobalFunctionVariable };
+        enum FunctionVariableType : uint8_t { NormalFunctionVariable, TopLevelFunctionVariable };
         Vector<std::pair<FunctionMetadataNode*, FunctionVariableType>> m_functionsToInitialize;
         bool m_needToInitializeArguments { false };

trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

@@ -1179 +1179 @@
                 FunctionExecutable* function = codeBlock->functionDecl(i);
                 PutPropertySlot slot(variableObject);
-                variableObject->methodTable()->put(variableObject, callFrame, function->name(), JSFunction::create(vm, function, scope), slot);
+                // We need create this variables because it will be used to emits code by bytecode generator
+                variableObject->methodTable()->put(variableObject, callFrame, function->name(), jsUndefined(), slot);
             }
         } else {
@@ -1188 +1189 @@
                     return checkedReturn(throwSyntaxError(callFrame, throwScope, makeString("Can't create duplicate variable in eval: '", String(function->name().impl()), "'")));
                 PutPropertySlot slot(variableObject);
-                variableObject->methodTable()->put(variableObject, callFrame, function->name(), JSFunction::create(vm, function, scope), slot);
+                // We need create this variables because it will be used to emits code by bytecode generator
+                variableObject->methodTable()->put(variableObject, callFrame, function->name(), jsUndefined(), slot);
                 RETURN_IF_EXCEPTION(throwScope, checkedReturn(throwScope.exception()));
             }