Changeset 259835 in webkit

Timestamp:

Apr 9, 2020 3:49:01 PM (4 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] ModuleEnvironment do not have JSGlobalLexicalEnvironment as its upper scope
​https://bugs.webkit.org/show_bug.cgi?id=193347

Reviewed by Tadeu Zagallo.

JSTests:

• stress/global-lexical-environment-access-from-module.js: Added.

(shouldBe):
(import.string_appeared_here.then):

• stress/resources/global-lexical-environment-access-from-module-child.js: Added.

(export.read):
(export.write):

LayoutTests/imported/w3c:

• web-platform-tests/html/semantics/scripting-1/the-script-element/module/inline-async-execorder-expected.txt:

Source/JavaScriptCore:

The upper scope of module scope should be global lexical environment instead of global object.
This patch fixes it to allow modules to access global lexical environment's variables.

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::notifyLexicalBindingUpdate):

• dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::parseBlock):

• runtime/JSModuleEnvironment.h:
• runtime/JSModuleRecord.cpp:

(JSC::JSModuleRecord::instantiateDeclarations):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/global-lexical-environment-access-from-module.js (added)
• JSTests/stress/resources/global-lexical-environment-access-from-module-child.js (added)
• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/inline-async-execorder-expected.txt (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/bytecode/CodeBlock.cpp (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (modified) (3 diffs)
• Source/JavaScriptCore/runtime/JSModuleEnvironment.h (modified) (2 diffs)
• Source/JavaScriptCore/runtime/JSModuleRecord.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2020-04-09  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] ModuleEnvironment do not have JSGlobalLexicalEnvironment as its upper scope
+        https://bugs.webkit.org/show_bug.cgi?id=193347
+
+        Reviewed by Tadeu Zagallo.
+
+        * stress/global-lexical-environment-access-from-module.js: Added.
+        (shouldBe):
+        (import.string_appeared_here.then):
+        * stress/resources/global-lexical-environment-access-from-module-child.js: Added.
+        (export.read):
+        (export.write):
+
 2020-04-09  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2020-04-09  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] ModuleEnvironment do not have JSGlobalLexicalEnvironment as its upper scope
+        https://bugs.webkit.org/show_bug.cgi?id=193347
+
+        Reviewed by Tadeu Zagallo.
+
+        * web-platform-tests/html/semantics/scripting-1/the-script-element/module/inline-async-execorder-expected.txt:
+
 2020-04-08  Chris Dumez  <cdumez@apple.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/inline-async-execorder-expected.txt

@@ -1 @@
-CONSOLE MESSAGE: line 1: ReferenceError: Can't find variable: loaded
-CONSOLE MESSAGE: line 1: ReferenceError: Can't find variable: loaded
-CONSOLE MESSAGE: line 3: ReferenceError: Can't find variable: loaded
-CONSOLE MESSAGE: line 3: ReferenceError: Can't find variable: loaded
 
-Harness Error (FAIL), message = ReferenceError: Can't find variable: loaded
+PASS Inline async module script execution order 
 
-FAIL Inline async module script execution order assert_array_equals: lengths differ, expected 6 got 0
-

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-04-09  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] ModuleEnvironment do not have JSGlobalLexicalEnvironment as its upper scope
+        https://bugs.webkit.org/show_bug.cgi?id=193347
+
+        Reviewed by Tadeu Zagallo.
+
+        The upper scope of module scope should be global lexical environment instead of global object.
+        This patch fixes it to allow modules to access global lexical environment's variables.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::notifyLexicalBindingUpdate):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * runtime/JSModuleEnvironment.h:
+        * runtime/JSModuleRecord.cpp:
+        (JSC::JSModuleRecord::instantiateDeclarations):
+
 2020-04-09  Alexey Shvayka  <shvaikalesh@gmail.com>
 

trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -2885 +2885 @@
 void CodeBlock::notifyLexicalBindingUpdate()
 {
-    // FIXME: Currently, module code do not query to JSGlobalLexicalEnvironment. So this case should be removed once it is fixed.
-    // https://bugs.webkit.org/show_bug.cgi?id=193347
-    if (scriptMode() == JSParserScriptMode::Module)
-        return;
     JSGlobalObject* globalObject = m_globalObject.get();
     JSGlobalLexicalEnvironment* globalLexicalEnvironment = jsCast<JSGlobalLexicalEnvironment*>(globalObject->globalScope());

trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

@@ -6678 +6678 @@
                 m_graph.watchpoints().addLazily(m_inlineStackTop->m_codeBlock->globalObject()->varInjectionWatchpoint());
 
-            // FIXME: Currently, module code do not query to JSGlobalLexicalEnvironment. So this case should be removed once it is fixed.
-            // https://bugs.webkit.org/show_bug.cgi?id=193347
-            if (m_inlineStackTop->m_codeBlock->scriptMode() != JSParserScriptMode::Module) {
-                if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
-                    JSGlobalObject* globalObject = m_inlineStackTop->m_codeBlock->globalObject();
-                    unsigned identifierNumber = m_inlineStackTop->m_identifierRemap[bytecode.m_var];
-                    if (!m_graph.watchGlobalProperty(globalObject, identifierNumber))
-                        addToGraph(ForceOSRExit);
-                }
+            if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
+                JSGlobalObject* globalObject = m_inlineStackTop->m_codeBlock->globalObject();
+                unsigned identifierNumber = m_inlineStackTop->m_identifierRemap[bytecode.m_var];
+                if (!m_graph.watchGlobalProperty(globalObject, identifierNumber))
+                    addToGraph(ForceOSRExit);
             }
 
@@ -6794 +6790 @@
             case GlobalProperty:
             case GlobalPropertyWithVarInjectionChecks: {
-                // FIXME: Currently, module code do not query to JSGlobalLexicalEnvironment. So this case should be removed once it is fixed.
-                // https://bugs.webkit.org/show_bug.cgi?id=193347
-                if (m_inlineStackTop->m_codeBlock->scriptMode() != JSParserScriptMode::Module) {
-                    if (!m_graph.watchGlobalProperty(globalObject, identifierNumber))
-                        addToGraph(ForceOSRExit);
-                }
+                if (!m_graph.watchGlobalProperty(globalObject, identifierNumber))
+                    addToGraph(ForceOSRExit);
 
                 SpeculatedType prediction = getPrediction();
@@ -6971 +6963 @@
             case GlobalProperty:
             case GlobalPropertyWithVarInjectionChecks: {
-                // FIXME: Currently, module code do not query to JSGlobalLexicalEnvironment. So this case should be removed once it is fixed.
-                // https://bugs.webkit.org/show_bug.cgi?id=193347
-                if (m_inlineStackTop->m_codeBlock->scriptMode() != JSParserScriptMode::Module) {
-                    if (!m_graph.watchGlobalProperty(globalObject, identifierNumber))
-                        addToGraph(ForceOSRExit);
-                }
+                if (!m_graph.watchGlobalProperty(globalObject, identifierNumber))
+                    addToGraph(ForceOSRExit);
 
                 PutByIdStatus status;

trunk/Source/JavaScriptCore/runtime/JSModuleEnvironment.h

@@ -43 +43 @@
     static constexpr unsigned StructureFlags = Base::StructureFlags | OverridesGetOwnPropertySlot | OverridesGetPropertyNames;
 
-    static JSModuleEnvironment* create(VM&, Structure*, JSScope*, SymbolTable*, JSValue initialValue, AbstractModuleRecord*);
-
     static JSModuleEnvironment* create(VM& vm, JSGlobalObject* globalObject, JSScope* currentScope, SymbolTable* symbolTable, JSValue initialValue, AbstractModuleRecord* moduleRecord)
     {
@@ -83 +81 @@
     JSModuleEnvironment(VM&, Structure*, JSScope*, SymbolTable*);
 
+    static JSModuleEnvironment* create(VM&, Structure*, JSScope*, SymbolTable*, JSValue initialValue, AbstractModuleRecord*);
+
     void finishCreation(VM&, JSValue initialValue, AbstractModuleRecord*);
 

trunk/Source/JavaScriptCore/runtime/JSModuleRecord.cpp

@@ -104 +104 @@
 
     SymbolTable* symbolTable = moduleProgramExecutable->moduleEnvironmentSymbolTable();
-    JSModuleEnvironment* moduleEnvironment = JSModuleEnvironment::create(vm, globalObject, globalObject, symbolTable, jsTDZValue(), this);
+    JSModuleEnvironment* moduleEnvironment = JSModuleEnvironment::create(vm, globalObject, globalObject->globalLexicalEnvironment(), symbolTable, jsTDZValue(), this);
 
     // http://www.ecma-international.org/ecma-262/6.0/#sec-moduledeclarationinstantiation