Changeset 213452 in webkit

Timestamp:

Mar 6, 2017 8:57:11 AM (7 years ago)

Author:

Yusuke Suzuki

Message:

Null pointer crash when loading module with unresolved import also as a script file
​https://bugs.webkit.org/show_bug.cgi?id=168971

Reviewed by Saam Barati.

JSTests:

• stress/re-execute-error-module.js: Added.

(shouldBe):
(async):

• stress/resources/error-module.js: Added.

Source/JavaScriptCore:

If linking throws an error, this error should be re-thrown
when requesting the same module.

• builtins/ModuleLoaderPrototype.js:

(globalPrivate.newRegistryEntry):

• runtime/JSModuleRecord.cpp:

(JSC::JSModuleRecord::link):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/re-execute-error-module.js (added)
• JSTests/stress/resources/error-module.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js (modified) (3 diffs)
• Source/JavaScriptCore/runtime/JSModuleRecord.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2017-03-06  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Null pointer crash when loading module with unresolved import also as a script file
+        https://bugs.webkit.org/show_bug.cgi?id=168971
+
+        Reviewed by Saam Barati.
+
+        * stress/re-execute-error-module.js: Added.
+        (shouldBe):
+        (async):
+        * stress/resources/error-module.js: Added.
+
 2017-03-02  Keith Miller  <keith_miller@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2017-03-06  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Null pointer crash when loading module with unresolved import also as a script file
+        https://bugs.webkit.org/show_bug.cgi?id=168971
+
+        Reviewed by Saam Barati.
+
+        If linking throws an error, this error should be re-thrown
+        when requesting the same module.
+
+        * builtins/ModuleLoaderPrototype.js:
+        (globalPrivate.newRegistryEntry):
+        * runtime/JSModuleRecord.cpp:
+        (JSC::JSModuleRecord::link):
+
 2017-03-06  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js

@@ -93 +93 @@
         key: key,
         state: @ModuleFetch,
-        metadata: @undefined,
         fetch: @undefined,
         instantiate: @undefined,
@@ -100 +99 @@
         dependenciesMap: @undefined,
         module: @undefined, // JSModuleRecord
-        error: @undefined,
+        linkError: @undefined,
+        linkSucceeded: true,
     };
 }
@@ -351 +351 @@
     "use strict";
 
-    // FIXME: Current implementation does not support optionalInstance.
-    // So Link's step 3 is skipped.
-    // https://bugs.webkit.org/show_bug.cgi?id=148171
-
+    if (!entry.linkSucceeded)
+        throw entry.linkError;
     if (entry.state === @ModuleReady)
         return;
     @setStateToMax(entry, @ModuleReady);
 
-    // Since we already have the "dependencies" field,
-    // we can call moduleDeclarationInstantiation with the correct order
-    // without constructing the dependency graph by calling dependencyGraph.
-    var dependencies = entry.dependencies;
-    for (var i = 0, length = dependencies.length; i < length; ++i) {
-        var pair = dependencies[i];
-        this.link(pair.value.registryEntry, fetcher);
-    }
-
-    this.moduleDeclarationInstantiation(entry.module, fetcher);
+    try {
+        // Since we already have the "dependencies" field,
+        // we can call moduleDeclarationInstantiation with the correct order
+        // without constructing the dependency graph by calling dependencyGraph.
+        var dependencies = entry.dependencies;
+        for (var i = 0, length = dependencies.length; i < length; ++i) {
+            var pair = dependencies[i];
+            this.link(pair.value.registryEntry, fetcher);
+        }
+
+        this.moduleDeclarationInstantiation(entry.module, fetcher);
+    } catch (error) {
+        entry.linkSucceeded = false;
+        entry.linkError = error;
+        throw error;
+    }
 }
 

trunk/Source/JavaScriptCore/runtime/JSModuleRecord.cpp

@@ -87 +87 @@
         return;
     }
+    instantiateDeclarations(exec, executable);
+    RETURN_IF_EXCEPTION(scope, void());
     m_moduleProgramExecutable.set(vm, this, executable);
-    instantiateDeclarations(exec, executable);
 }