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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@213452 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index ef4729a..7fe8a41 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,15 @@
+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>
 
         WebAssemblyFunction should have Function.prototype as its prototype

diff --git a/JSTests/stress/re-execute-error-module.js b/JSTests/stress/re-execute-error-module.js
new file mode 100644 (file)
index 0000000..dfc7754
--- /dev/null
+++ b/JSTests/stress/re-execute-error-module.js
@@ -0,0 +1,26 @@
+function shouldBe(actual, expected)
+{
+    if (actual !== expected)
+        throw new Error(`bad value: ${String(actual)}`);
+}
+
+(async function () {
+    {
+        let errorMessage = null;
+        try {
+            await import("./resources/error-module.js");
+        } catch (error) {
+            errorMessage = String(error);
+        }
+        shouldBe(errorMessage, `SyntaxError: Importing binding name 'x' is not found.`);
+    }
+    {
+        let errorMessage = null;
+        try {
+            await import("./resources/error-module.js");
+        } catch (error) {
+            errorMessage = String(error);
+        }
+        shouldBe(errorMessage, `SyntaxError: Importing binding name 'x' is not found.`);
+    }
+}()).catch(abort);

diff --git a/JSTests/stress/resources/error-module.js b/JSTests/stress/resources/error-module.js
new file mode 100644 (file)
index 0000000..b68f953
--- /dev/null
+++ b/JSTests/stress/resources/error-module.js
@@ -0,0 +1 @@
+import {x} from "./error-module.js"

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 47d29c7..2608b57 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,5 +1,20 @@
 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>
+
         [GTK][JSCOnly] Enable WebAssembly on Linux environment
         https://bugs.webkit.org/show_bug.cgi?id=164032
 

diff --git a/Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js b/Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js
index e341ab8..29556ea 100644 (file)
--- a/Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js
+++ b/Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js
@@ -92,14 +92,14 @@ function newRegistryEntry(key)
     return {
         key: key,
         state: @ModuleFetch,
-        metadata: @undefined,
         fetch: @undefined,
         instantiate: @undefined,
         satisfy: @undefined,
         dependencies: [], // To keep the module order, we store the module keys in the array.
         dependenciesMap: @undefined,
         module: @undefined, // JSModuleRecord
-        error: @undefined,
+        linkError: @undefined,
+        linkSucceeded: true,
     };
 }
 
@@ -350,24 +350,28 @@ function link(entry, fetcher)
 
     "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);
-    }
+    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);
+        this.moduleDeclarationInstantiation(entry.module, fetcher);
+    } catch (error) {
+        entry.linkSucceeded = false;
+        entry.linkError = error;
+        throw error;
+    }
 }
 
 // Module semantics.

diff --git a/Source/JavaScriptCore/runtime/JSModuleRecord.cpp b/Source/JavaScriptCore/runtime/JSModuleRecord.cpp
index 0098ecc..ecb0584 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSModuleRecord.cpp
+++ b/Source/JavaScriptCore/runtime/JSModuleRecord.cpp
@@ -86,8 +86,9 @@ void JSModuleRecord::link(ExecState* exec)
         throwSyntaxError(exec, scope);
         return;
     }
-    m_moduleProgramExecutable.set(vm, this, executable);
     instantiateDeclarations(exec, executable);
+    RETURN_IF_EXCEPTION(scope, void());
+    m_moduleProgramExecutable.set(vm, this, executable);
 }
 
 void JSModuleRecord::instantiateDeclarations(ExecState* exec, ModuleProgramExecutable* moduleProgramExecutable)