From 972ab6d437272edb268f2bf1a515027192b14b58 Mon Sep 17 00:00:00 2001
From: Moonchild <moonchild@palemoon.org>
Date: Mon, 3 Aug 2020 09:41:42 +0000
Subject: [js] Try to catch bad pointers for GC and bail if not valid.

---
 js/src/gc/Marking.cpp | 5 +++++
 1 file changed, 5 insertions(+)

(limited to 'js/src')

diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp
index 43e325394..b4db0297a 100644
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -2267,6 +2267,8 @@ void
 js::gc::StoreBuffer::SlotsEdge::trace(TenuringTracer& mover) const
 {
     NativeObject* obj = object();
+    if(!IsCellPointerValid(obj))
+        return;
 
     // Beware JSObject::swap exchanging a native object for a non-native one.
     if (!obj->isNative())
@@ -2336,6 +2338,8 @@ js::gc::StoreBuffer::traceWholeCells(TenuringTracer& mover)
 {
     for (ArenaCellSet* cells = bufferWholeCell; cells; cells = cells->next) {
         Arena* arena = cells->arena;
+        if(!IsCellPointerValid(arena))
+            continue;
 
         MOZ_ASSERT(arena->bufferedCells == cells);
         arena->bufferedCells = &ArenaCellSet::Empty;
@@ -2364,6 +2368,7 @@ js::gc::StoreBuffer::CellPtrEdge::trace(TenuringTracer& mover) const
 {
     if (!*edge)
         return;
+    // XXX: We should check if the cell pointer is valid here too
 
     MOZ_ASSERT((*edge)->getTraceKind() == JS::TraceKind::Object);
     mover.traverse(reinterpret_cast<JSObject**>(edge));
-- 
cgit v1.2.3


From d107016d376bfce4e6b70fcc06efb18e1a9899f3 Mon Sep 17 00:00:00 2001
From: Moonchild <moonchild@palemoon.org>
Date: Wed, 19 Aug 2020 07:16:29 +0000
Subject: [js] Reinstate precise floating point model for all js sources.

---
 js/src/moz.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'js/src')

diff --git a/js/src/moz.build b/js/src/moz.build
index c0cef9929..3e6402f71 100644
--- a/js/src/moz.build
+++ b/js/src/moz.build
@@ -687,8 +687,8 @@ if CONFIG['_MSC_VER']:
     elif CONFIG['CPU_ARCH'] == 'x86_64' and CONFIG['JS_HAS_CTYPES']:
         SOURCES['ctypes/CTypes.cpp'].no_pgo = True # Bug 810661
     # Prevent floating point errors caused by VC++ optimizations
-    # XXX We should add this to CXXFLAGS, too?
     CFLAGS += ['-fp:precise']
+    CXXFLAGS += ['-fp:precise']    
     # C4805 warns mixing bool with other integral types in computation.
     # But given the conversion from bool is specified, and this is a
     # pattern widely used in code in js/src, suppress this warning here.
-- 
cgit v1.2.3


From 498b1ab0c8db07784badbd2148f372027ef8cc27 Mon Sep 17 00:00:00 2001
From: Moonchild <moonchild@palemoon.org>
Date: Wed, 8 Jul 2020 12:58:30 +0000
Subject: Issue #618 - Align error handling for module scripts with the spec
 (again)

This updates module implementation to match spec regarding handling of
instantiation errors, after it was changed yet again, this time to not remember
instantiation errors, but instead immediately rethrow applicable ones.

Ref: BZ 1420420
---
 js/src/builtin/Module.js                       | 292 ++++++++++++-------------
 js/src/builtin/ModuleObject.cpp                |  38 ++--
 js/src/builtin/ModuleObject.h                  |  13 +-
 js/src/builtin/SelfHostingDefines.h            |  14 +-
 js/src/builtin/TestingFunctions.cpp            |   6 +
 js/src/jit-test/tests/modules/bug-1284486.js   |  12 +-
 js/src/jit-test/tests/modules/bug-1420420-2.js |  19 ++
 js/src/jit-test/tests/modules/bug-1420420-3.js |   9 +
 js/src/jit-test/tests/modules/bug-1420420-4.js |  16 ++
 js/src/jit-test/tests/modules/bug-1420420.js   |  19 ++
 js/src/js.msg                                  |   1 -
 js/src/jsapi.cpp                               |  12 -
 js/src/jsapi.h                                 |   6 -
 js/src/vm/EnvironmentObject.cpp                |   2 +-
 js/src/vm/SelfHosting.cpp                      |   1 -
 15 files changed, 252 insertions(+), 208 deletions(-)
 create mode 100644 js/src/jit-test/tests/modules/bug-1420420-2.js
 create mode 100644 js/src/jit-test/tests/modules/bug-1420420-3.js
 create mode 100644 js/src/jit-test/tests/modules/bug-1420420-4.js
 create mode 100644 js/src/jit-test/tests/modules/bug-1420420.js

(limited to 'js/src')

diff --git a/js/src/builtin/Module.js b/js/src/builtin/Module.js
index 64d62d216..86b44880d 100644
--- a/js/src/builtin/Module.js
+++ b/js/src/builtin/Module.js
@@ -69,8 +69,13 @@ function ModuleSetStatus(module, newStatus)
 {
     assert(newStatus >= MODULE_STATUS_ERRORED && newStatus <= MODULE_STATUS_EVALUATED,
            "Bad new module status in ModuleSetStatus");
-    if (newStatus !== MODULE_STATUS_ERRORED)
-        assert(newStatus > module.status, "New module status inconsistent with current status");
+
+    // Note that under OOM conditions we can fail the module instantiation
+    // process even after modules have been marked as instantiated.
+    assert((module.status <= MODULE_STATUS_INSTANTIATED &&
+            newStatus === MODULE_STATUS_UNINSTANTIATED) ||
+           newStatus > module.status,
+           "New module status inconsistent with current status");
 
     UnsafeSetReservedSlot(module, MODULE_OBJECT_STATUS_SLOT, newStatus);
 }
@@ -80,20 +85,15 @@ function ModuleSetStatus(module, newStatus)
 // Returns an object describing the location of the resolved export or
 // indicating a failure.
 //
-// On success this returns: { resolved: true, module, bindingName }
-//
-// There are three failure cases:
+// On success this returns a resolved binding record: { module, bindingName }
 //
-//  - The resolution failure can be blamed on a particular module.
-//    Returns: { resolved: false, module, ambiguous: false }
+// There are two failure cases:
 //
-//  - No culprit can be determined and the resolution failure was due to star
-//    export ambiguity.
-//    Returns: { resolved: false, module: null, ambiguous: true }
+//  - If no definition was found or the request is found to be circular, *null*
+//    is returned.
 //
-//  - No culprit can be determined and the resolution failure was not due to
-//    star export ambiguity.
-//    Returns: { resolved: false, module: null, ambiguous: false }
+//  - If the request is found to be ambiguous, the string `"ambiguous"` is
+//    returned.
 //
 function ModuleResolveExport(exportName, resolveSet = [])
 {
@@ -106,53 +106,47 @@ function ModuleResolveExport(exportName, resolveSet = [])
     let module = this;
 
     // Step 2
-    assert(module.status !== MODULE_STATUS_ERRORED, "Bad module status in ResolveExport");
-    
-    // Step 3
     for (let i = 0; i < resolveSet.length; i++) {
         let r = resolveSet[i];
         if (r.module === module && r.exportName === exportName) {
             // This is a circular import request.
-            return {resolved: false, module: null, ambiguous: false};
+            return null;
         }
     }
 
-    // Step 4
+    // Step 3
     _DefineDataProperty(resolveSet, resolveSet.length, {module: module, exportName: exportName});
 
-    // Step 5
+    // Step 4
     let localExportEntries = module.localExportEntries;
     for (let i = 0; i < localExportEntries.length; i++) {
         let e = localExportEntries[i];
         if (exportName === e.exportName)
-            return {resolved: true, module, bindingName: e.localName};
+            return {module, bindingName: e.localName};
     }
 
-    // Step 6
+    // Step 5
     let indirectExportEntries = module.indirectExportEntries;
     for (let i = 0; i < indirectExportEntries.length; i++) {
         let e = indirectExportEntries[i];
         if (exportName === e.exportName) {
             let importedModule = CallModuleResolveHook(module, e.moduleRequest,
                                                        MODULE_STATUS_UNINSTANTIATED);
-            let resolution = callFunction(importedModule.resolveExport, importedModule, e.importName,
-                                          resolveSet);
-            if (!resolution.resolved && !resolution.module)
-                resolution.module = module;
-            return resolution;
+            return callFunction(importedModule.resolveExport, importedModule, e.importName,
+                                resolveSet);
         }
     }
 
-    // Step 7
+    // Step 6
     if (exportName === "default") {
         // A default export cannot be provided by an export *.
-        return {resolved: false, module: null, ambiguous: false};
+        return null;
     }
 
-    // Step 8
+    // Step 7
     let starResolution = null;
 
-    // Step 9
+    // Step 8
     let starExportEntries = module.starExportEntries;
     for (let i = 0; i < starExportEntries.length; i++) {
         let e = starExportEntries[i];
@@ -160,27 +154,31 @@ function ModuleResolveExport(exportName, resolveSet = [])
                                                    MODULE_STATUS_UNINSTANTIATED);
         let resolution = callFunction(importedModule.resolveExport, importedModule, exportName,
                                       resolveSet);
-        if (!resolution.resolved && (resolution.module || resolution.ambiguous))
+        if (resolution === "ambiguous")
             return resolution;
 
-        if (resolution.resolved) {
+        if (resolution !== null) {
             if (starResolution === null) {
                 starResolution = resolution;
             } else {
                 if (resolution.module !== starResolution.module ||
                     resolution.bindingName !== starResolution.bindingName)
                 {
-                    return {resolved: false, module: null, ambiguous: true};
+                    return "ambiguous";
                 }
             }
         }
     }
 
-    // Step 10
-    if (starResolution !== null)
-        return starResolution;
+    // Step 9
+    return starResolution;
+}
 
-    return {resolved: false, module: null, ambiguous: false};
+function IsResolvedBinding(resolution)
+{
+    assert(resolution === "ambiguous" || typeof resolution === "object",
+           "Bad module resolution result");
+    return typeof resolution === "object" && resolution !== null;
 }
 
 // 15.2.1.18 GetModuleNamespace(module)
@@ -189,12 +187,12 @@ function GetModuleNamespace(module)
     // Step 1
     assert(IsModule(module), "GetModuleNamespace called with non-module");
 
-    // Step 2
+    // Steps 2-3
     assert(module.status !== MODULE_STATUS_UNINSTANTIATED &&
-           module.status !== MODULE_STATUS_ERRORED,
+           module.status !== MODULE_STATUS_EVALUATED_ERROR,
            "Bad module status in GetModuleNamespace");
 
-    // Step 3
+    // Step 4
     let namespace = module.namespace;
 
     if (typeof namespace === "undefined") {
@@ -203,7 +201,7 @@ function GetModuleNamespace(module)
         for (let i = 0; i < exportedNames.length; i++) {
             let name = exportedNames[i];
             let resolution = callFunction(module.resolveExport, module, name);
-            if (resolution.resolved)
+            if (IsResolvedBinding(resolution))
                 _DefineDataProperty(unambiguousNames, unambiguousNames.length, name);
         }
         namespace = ModuleNamespaceCreate(module, unambiguousNames);
@@ -225,7 +223,7 @@ function ModuleNamespaceCreate(module, exports)
     for (let i = 0; i < exports.length; i++) {
         let name = exports[i];
         let binding = callFunction(module.resolveExport, module, name);
-        assert(binding.resolved, "Failed to resolve binding");
+        assert(IsResolvedBinding(binding), "Failed to resolve binding");
         AddModuleNamespaceBinding(ns, name, binding.module, binding.bindingName);
     }
 
@@ -236,11 +234,6 @@ function GetModuleEnvironment(module)
 {
     assert(IsModule(module), "Non-module passed to GetModuleEnvironment");
 
-    // Check for a previous failed attempt to instantiate this module. This can
-    // only happen due to a bug in the module loader.
-    if (module.status === MODULE_STATUS_ERRORED)
-        ThrowInternalError(JSMSG_MODULE_INSTANTIATE_FAILED, module.status);
-
     let env = UnsafeGetReservedSlot(module, MODULE_OBJECT_ENVIRONMENT_SLOT);
     assert(env === undefined || IsModuleEnvironment(env),
            "Module environment slot contains unexpected value");
@@ -248,19 +241,6 @@ function GetModuleEnvironment(module)
     return env;
 }
 
-function RecordModuleError(module, error)
-{
-    // Set the module's status to 'errored' to indicate a failed module
-    // instantiation and record the exception. The environment slot is also
-    // reset to 'undefined'.
-
-    assert(IsObject(module) && IsModule(module), "Non-module passed to RecordModuleError");
-
-    ModuleSetStatus(module, MODULE_STATUS_ERRORED);
-    UnsafeSetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT, error);
-    UnsafeSetReservedSlot(module, MODULE_OBJECT_ENVIRONMENT_SLOT, undefined);
-}
-
 function CountArrayValues(array, value)
 {
     let count = 0;
@@ -280,6 +260,16 @@ function ArrayContains(array, value)
     return false;
 }
 
+function HandleModuleInstantiationFailure(module)
+{
+    // Reset the module to the "uninstantiated" state. Don't reset the
+    // environment slot as the environment object will be required by any
+    // possible future instantiation attempt.
+    ModuleSetStatus(module, MODULE_STATUS_UNINSTANTIATED);
+    UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_INDEX_SLOT, undefined);
+    UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT, undefined);
+}
+
 // 15.2.1.16.4 ModuleInstantiate()
 function ModuleInstantiate()
 {
@@ -301,38 +291,30 @@ function ModuleInstantiate()
 
     // Steps 4-5
     try {
-        InnerModuleDeclarationInstantiation(module, stack, 0);
+        InnerModuleInstantiation(module, stack, 0);
     } catch (error) {
         for (let i = 0; i < stack.length; i++) {
             let m = stack[i];
 
-            assert(m.status === MODULE_STATUS_INSTANTIATING ||
-                   m.status === MODULE_STATUS_ERRORED,
-                   "Bad module status after failed instantiation");
-
-            RecordModuleError(m, error);
+            assert(m.status === MODULE_STATUS_INSTANTIATING,
+                   "Expected instantiating status during failed instantiation");
+            HandleModuleInstantiationFailure(m);
         }
 
-        if (stack.length === 0 &&
-            typeof(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT)) === "undefined")
-        {
-            // This can happen due to OOM when appending to the stack.
-            assert(error === "out of memory",
-                   "Stack must contain module unless we hit OOM");
-            RecordModuleError(module, error);
-        }
+        // Handle OOM when appending to the stack or over-recursion errors.
+        if (stack.length === 0)
+            HandleModuleInstantiationFailure(module);
 
-        assert(module.status === MODULE_STATUS_ERRORED,
-               "Bad module status after failed instantiation");
-        assert(SameValue(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT), error),
-               "Module has different error set after failed instantiation");
+        assert(module.status === MODULE_STATUS_UNINSTANTIATED,
+               "Expected uninstantiated status after failed instantiation");
 
         throw error;
     }
 
     // Step 6
-    assert(module.status == MODULE_STATUS_INSTANTIATED ||
-           module.status == MODULE_STATUS_EVALUATED,
+    assert(module.status === MODULE_STATUS_INSTANTIATED ||
+           module.status === MODULE_STATUS_EVALUATED ||
+           module.status === MODULE_STATUS_EVALUATED_ERROR,
            "Bad module status after successful instantiation");
 
     // Step 7
@@ -344,8 +326,8 @@ function ModuleInstantiate()
 }
 _SetCanonicalName(ModuleInstantiate, "ModuleInstantiate");
 
-// 15.2.1.16.4.1 InnerModuleDeclarationInstantiation(module, stack, index)
-function InnerModuleDeclarationInstantiation(module, stack, index)
+// 15.2.1.16.4.1 InnerModuleInstantiation(module, stack, index)
+function InnerModuleInstantiation(module, stack, index)
 {
     // Step 1
     // TODO: Support module records other than source text module records.
@@ -353,42 +335,40 @@ function InnerModuleDeclarationInstantiation(module, stack, index)
     // Step 2
     if (module.status === MODULE_STATUS_INSTANTIATING ||
         module.status === MODULE_STATUS_INSTANTIATED ||
-        module.status === MODULE_STATUS_EVALUATED)
+        module.status === MODULE_STATUS_EVALUATED ||
+        module.status === MODULE_STATUS_EVALUATED_ERROR)
     {
         return index;
     }
 
     // Step 3
-    if (module.status === MODULE_STATUS_ERRORED)
-        throw module.error;
-
-    // Step 4
     assert(module.status === MODULE_STATUS_UNINSTANTIATED,
           "Bad module status in ModuleDeclarationInstantiation");
 
-    // Steps 5
+    // Step 4
     ModuleSetStatus(module, MODULE_STATUS_INSTANTIATING);
 
-    // Step 6-8
+    // Steps 5-7
     UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_INDEX_SLOT, index);
     UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT, index);
     index++;
 
-    // Step 9
+    // Step 8
     _DefineDataProperty(stack, stack.length, module);
 
-    // Step 10
+    // Step 9
     let requestedModules = module.requestedModules;
     for (let i = 0; i < requestedModules.length; i++) {
         let required = requestedModules[i];
-        let requiredModule = CallModuleResolveHook(module, required, MODULE_STATUS_ERRORED);
+        let requiredModule = CallModuleResolveHook(module, required, MODULE_STATUS_UNINSTANTIATED);
 
-        index = InnerModuleDeclarationInstantiation(requiredModule, stack, index);
+        index = InnerModuleInstantiation(requiredModule, stack, index);
 
         assert(requiredModule.status === MODULE_STATUS_INSTANTIATING ||
                requiredModule.status === MODULE_STATUS_INSTANTIATED ||
-               requiredModule.status === MODULE_STATUS_EVALUATED,
-               "Bad required module status after InnerModuleDeclarationInstantiation");
+               requiredModule.status === MODULE_STATUS_EVALUATED ||
+               requiredModule.status === MODULE_STATUS_EVALUATED_ERROR,
+               "Bad required module status after InnerModuleInstantiation");
 
         assert((requiredModule.status === MODULE_STATUS_INSTANTIATING) ===
                ArrayContains(stack, requiredModule),
@@ -404,16 +384,16 @@ function InnerModuleDeclarationInstantiation(module, stack, index)
         }
     }
 
-    // Step 11
+    // Step 10
     ModuleDeclarationEnvironmentSetup(module);
 
-    // Steps 12-13
+    // Steps 11-12
     assert(CountArrayValues(stack, module) === 1,
            "Current module should appear exactly once in the stack");
     assert(module.dfsAncestorIndex <= module.dfsIndex,
            "Bad DFS ancestor index");
 
-    // Step 14
+    // Step 13
     if (module.dfsAncestorIndex === module.dfsIndex) {
         let requiredModule;
         do {
@@ -434,11 +414,9 @@ function ModuleDeclarationEnvironmentSetup(module)
     for (let i = 0; i < indirectExportEntries.length; i++) {
         let e = indirectExportEntries[i];
         let resolution = callFunction(module.resolveExport, module, e.exportName);
-        assert(resolution.resolved || resolution.module,
-               "Unexpected failure to resolve export in ModuleDeclarationEnvironmentSetup");
-        if (!resolution.resolved) {
-            return ResolutionError(resolution, "indirectExport", e.exportName,
-                                   e.lineNumber, e.columnNumber)
+        if (!IsResolvedBinding(resolution)) {
+            ThrowResolutionError(module, resolution, "indirectExport", e.exportName,
+                                 e.lineNumber, e.columnNumber);
         }
     }
 
@@ -458,12 +436,9 @@ function ModuleDeclarationEnvironmentSetup(module)
         } else {
             let resolution = callFunction(importedModule.resolveExport, importedModule,
                                           imp.importName);
-            if (!resolution.resolved && !resolution.module)
-                resolution.module = module;
-
-            if (!resolution.resolved) {
-                return ResolutionError(resolution, "import", imp.importName,
-                                       imp.lineNumber, imp.columnNumber);
+            if (!IsResolvedBinding(resolution)) {
+                ThrowResolutionError(module, resolution, "import", imp.importName,
+                                     imp.lineNumber, imp.columnNumber);
             }
 
             CreateImportBinding(env, imp.localName, resolution.module, resolution.bindingName);
@@ -473,38 +448,58 @@ function ModuleDeclarationEnvironmentSetup(module)
     InstantiateModuleFunctionDeclarations(module);
 }
 
-// 15.2.1.16.4.3 ResolutionError(module)
-function ResolutionError(resolution, kind, name, line, column)
+function ThrowResolutionError(module, resolution, kind, name, line, column)
 {
-    let module = resolution.module;
-    assert(module !== null,
-           "Null module passed to ResolutionError");
-
-    assert(module.status === MODULE_STATUS_UNINSTANTIATED ||
-           module.status === MODULE_STATUS_INSTANTIATING,
-           "Unexpected module status in ResolutionError");
+    assert(module.status === MODULE_STATUS_INSTANTIATING,
+           "Unexpected module status in ThrowResolutionError");
 
     assert(kind === "import" || kind === "indirectExport",
-           "Unexpected kind in ResolutionError");
+           "Unexpected kind in ThrowResolutionError");
 
     assert(line > 0,
            "Line number should be present for all imports and indirect exports");
 
+    let ambiguous = resolution === "ambiguous";
+
     let errorNumber;
-    if (kind === "import") {
-        errorNumber = resolution.ambiguous ? JSMSG_AMBIGUOUS_IMPORT
-                                           : JSMSG_MISSING_IMPORT;
-    } else {
-        errorNumber = resolution.ambiguous ? JSMSG_AMBIGUOUS_INDIRECT_EXPORT
-                                           : JSMSG_MISSING_INDIRECT_EXPORT;
-    }
+    if (kind === "import")
+        errorNumber = ambiguous ? JSMSG_AMBIGUOUS_IMPORT : JSMSG_MISSING_IMPORT;
+    else
+        errorNumber = ambiguous ? JSMSG_AMBIGUOUS_INDIRECT_EXPORT : JSMSG_MISSING_INDIRECT_EXPORT;
 
     let message = GetErrorMessage(errorNumber) + ": " + name;
     let error = CreateModuleSyntaxError(module, line, column, message);
-    RecordModuleError(module, error);
     throw error;
 }
 
+function GetModuleEvaluationError(module)
+{
+    assert(IsObject(module) && IsModule(module),
+           "Non-module passed to GetModuleEvaluationError");
+    assert(module.status === MODULE_STATUS_EVALUATED_ERROR,
+           "Bad module status in GetModuleEvaluationError");
+    return UnsafeGetReservedSlot(module, MODULE_OBJECT_EVALUATION_ERROR_SLOT);
+}
+
+function RecordModuleEvaluationError(module, error)
+{
+    // Set the module's EvaluationError slot to indicate a failed module
+    // evaluation.
+
+    assert(IsObject(module) && IsModule(module),
+           "Non-module passed to RecordModuleEvaluationError");
+
+    if (module.status === MODULE_STATUS_EVALUATED_ERROR) {
+        // It would be nice to assert that |error| is the same as the one we
+        // previously recorded, but that's not always true in the case of out of
+        // memory and over-recursion errors.
+        return;
+    }
+
+    ModuleSetStatus(module, MODULE_STATUS_EVALUATED_ERROR);
+    UnsafeSetReservedSlot(module, MODULE_OBJECT_EVALUATION_ERROR_SLOT, error);
+}
+
 // 15.2.1.16.5 ModuleEvaluate()
 function ModuleEvaluate()
 {
@@ -515,9 +510,9 @@ function ModuleEvaluate()
     let module = this;
 
     // Step 2
-    if (module.status !== MODULE_STATUS_ERRORED &&
-        module.status !== MODULE_STATUS_INSTANTIATED &&
-        module.status !== MODULE_STATUS_EVALUATED)
+    if (module.status !== MODULE_STATUS_INSTANTIATED &&
+        module.status !== MODULE_STATUS_EVALUATED &&
+        module.status !== MODULE_STATUS_EVALUATED_ERROR)
     {
         ThrowInternalError(JSMSG_BAD_MODULE_STATUS);
     }
@@ -535,27 +530,20 @@ function ModuleEvaluate()
             assert(m.status === MODULE_STATUS_EVALUATING,
                    "Bad module status after failed evaluation");
 
-            RecordModuleError(m, error);
+            RecordModuleEvaluationError(m, error);
         }
 
-        if (stack.length === 0 &&
-            typeof(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT)) === "undefined")
-        {
-            // This can happen due to OOM when appending to the stack.
-            assert(error === "out of memory",
-                  "Stack must contain module unless we hit OOM");
-            RecordModuleError(module, error);
-        }
+        // Handle OOM when appending to the stack or over-recursion errors.
+        if (stack.length === 0)
+            RecordModuleEvaluationError(module, error);
 
-        assert(module.status === MODULE_STATUS_ERRORED,
+        assert(module.status === MODULE_STATUS_EVALUATED_ERROR,
                "Bad module status after failed evaluation");
-        assert(SameValue(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT), error),
-               "Module has different error set after failed evaluation");
 
         throw error;
     }
 
-    assert(module.status == MODULE_STATUS_EVALUATED,
+    assert(module.status === MODULE_STATUS_EVALUATED,
            "Bad module status after successful evaluation");
     assert(stack.length === 0,
            "Stack should be empty after successful evaluation");
@@ -571,19 +559,19 @@ function InnerModuleEvaluation(module, stack, index)
     // TODO: Support module records other than source text module records.
 
     // Step 2
-    if (module.status === MODULE_STATUS_EVALUATING ||
-        module.status === MODULE_STATUS_EVALUATED)
-    {
+    if (module.status === MODULE_STATUS_EVALUATED_ERROR)
+        throw GetModuleEvaluationError(module);
+
+    if (module.status === MODULE_STATUS_EVALUATED)
         return index;
-    }
 
     // Step 3
-    if (module.status === MODULE_STATUS_ERRORED)
-        throw module.error;
+    if (module.status === MODULE_STATUS_EVALUATING)
+        return index;
 
     // Step 4
     assert(module.status === MODULE_STATUS_INSTANTIATED,
-          "Bad module status in ModuleEvaluation");
+          "Bad module status in InnerModuleEvaluation");
 
     // Step 5
     ModuleSetStatus(module, MODULE_STATUS_EVALUATING);
@@ -605,8 +593,8 @@ function InnerModuleEvaluation(module, stack, index)
 
         index = InnerModuleEvaluation(requiredModule, stack, index);
 
-        assert(requiredModule.status == MODULE_STATUS_EVALUATING ||
-               requiredModule.status == MODULE_STATUS_EVALUATED,
+        assert(requiredModule.status === MODULE_STATUS_EVALUATING ||
+               requiredModule.status === MODULE_STATUS_EVALUATED,
               "Bad module status after InnerModuleEvaluation");
 
         assert((requiredModule.status === MODULE_STATUS_EVALUATING) ===
diff --git a/js/src/builtin/ModuleObject.cpp b/js/src/builtin/ModuleObject.cpp
index 30e7120c0..a9a257178 100644
--- a/js/src/builtin/ModuleObject.cpp
+++ b/js/src/builtin/ModuleObject.cpp
@@ -18,10 +18,10 @@
 using namespace js;
 using namespace js::frontend;
 
-static_assert(MODULE_STATUS_ERRORED < MODULE_STATUS_UNINSTANTIATED &&
-              MODULE_STATUS_UNINSTANTIATED < MODULE_STATUS_INSTANTIATING &&
+static_assert(MODULE_STATUS_UNINSTANTIATED < MODULE_STATUS_INSTANTIATING &&
               MODULE_STATUS_INSTANTIATING < MODULE_STATUS_INSTANTIATED &&
-              MODULE_STATUS_INSTANTIATED < MODULE_STATUS_EVALUATED,
+              MODULE_STATUS_INSTANTIATED < MODULE_STATUS_EVALUATED &&
+              MODULE_STATUS_EVALUATED < MODULE_STATUS_EVALUATED_ERROR,
               "Module statuses are ordered incorrectly");
 
 template<typename T, Value ValueGetter(const T* obj)>
@@ -273,12 +273,12 @@ IndirectBindingMap::trace(JSTracer* trc)
 }
 
 bool
-IndirectBindingMap::putNew(JSContext* cx, HandleId name,
-                           HandleModuleEnvironmentObject environment, HandleId localName)
+IndirectBindingMap::put(JSContext* cx, HandleId name,
+                        HandleModuleEnvironmentObject environment, HandleId localName)
 {
     RootedShape shape(cx, environment->lookup(cx, localName));
     MOZ_ASSERT(shape);
-    if (!map_.putNew(name, Binding(environment, shape))) {
+    if (!map_.put(name, Binding(environment, shape))) {
         ReportOutOfMemory(cx);
         return false;
     }
@@ -359,7 +359,7 @@ ModuleNamespaceObject::addBinding(JSContext* cx, HandleAtom exportedName,
     RootedModuleEnvironmentObject environment(cx, &targetModule->initialEnvironment());
     RootedId exportedNameId(cx, AtomToId(exportedName));
     RootedId localNameId(cx, AtomToId(localName));
-    return bindings->putNew(cx, exportedNameId, environment, localNameId);
+    return bindings->put(cx, exportedNameId, environment, localNameId);
 }
 
 const char ModuleNamespaceObject::ProxyHandler::family = 0;
@@ -660,6 +660,9 @@ ModuleObject::finalize(js::FreeOp* fop, JSObject* obj)
 ModuleEnvironmentObject*
 ModuleObject::environment() const
 {
+//-    MOZ_ASSERT(status() != MODULE_STATUS_ERRORED);
+//+    MOZ_ASSERT(!hadEvaluationError());
+
     Value value = getReservedSlot(EnvironmentSlot);
     if (value.isUndefined())
         return nullptr;
@@ -723,7 +726,7 @@ void
 ModuleObject::init(HandleScript script)
 {
     initReservedSlot(ScriptSlot, PrivateValue(script));
-    initReservedSlot(StatusSlot, Int32Value(MODULE_STATUS_ERRORED));
+    initReservedSlot(StatusSlot, Int32Value(MODULE_STATUS_UNINSTANTIATED));
 }
 
 void
@@ -827,7 +830,8 @@ ModuleObject::script() const
 static inline void
 AssertValidModuleStatus(ModuleStatus status)
 {
-    MOZ_ASSERT(status >= MODULE_STATUS_ERRORED && status <= MODULE_STATUS_EVALUATED);
+    MOZ_ASSERT(status >= MODULE_STATUS_UNINSTANTIATED &&
+               status <= MODULE_STATUS_EVALUATED_ERROR);
 }
 
 ModuleStatus
@@ -838,11 +842,17 @@ ModuleObject::status() const
     return status;
 }
 
+bool
+ModuleObject::hadEvaluationError() const
+{
+    return status() == MODULE_STATUS_EVALUATED_ERROR;
+}
+
 Value
-ModuleObject::error() const
+ModuleObject::evaluationError() const
 {
-    MOZ_ASSERT(status() == MODULE_STATUS_ERRORED);
-    return getReservedSlot(ErrorSlot);
+    MOZ_ASSERT(hadEvaluationError());
+    return getReservedSlot(EvaluationErrorSlot);
 }
 
 Value
@@ -1005,7 +1015,7 @@ ModuleObject::Evaluate(JSContext* cx, HandleModuleObject self)
 
 DEFINE_GETTER_FUNCTIONS(ModuleObject, namespace_, NamespaceSlot)
 DEFINE_GETTER_FUNCTIONS(ModuleObject, status, StatusSlot)
-DEFINE_GETTER_FUNCTIONS(ModuleObject, error, ErrorSlot)
+DEFINE_GETTER_FUNCTIONS(ModuleObject, evaluationError, EvaluationErrorSlot)
 DEFINE_GETTER_FUNCTIONS(ModuleObject, requestedModules, RequestedModulesSlot)
 DEFINE_GETTER_FUNCTIONS(ModuleObject, importEntries, ImportEntriesSlot)
 DEFINE_GETTER_FUNCTIONS(ModuleObject, localExportEntries, LocalExportEntriesSlot)
@@ -1020,7 +1030,7 @@ GlobalObject::initModuleProto(JSContext* cx, Handle<GlobalObject*> global)
     static const JSPropertySpec protoAccessors[] = {
         JS_PSG("namespace", ModuleObject_namespace_Getter, 0),
         JS_PSG("status", ModuleObject_statusGetter, 0),
-        JS_PSG("error", ModuleObject_errorGetter, 0),
+        JS_PSG("evaluationError", ModuleObject_evaluationErrorGetter, 0),
         JS_PSG("requestedModules", ModuleObject_requestedModulesGetter, 0),
         JS_PSG("importEntries", ModuleObject_importEntriesGetter, 0),
         JS_PSG("localExportEntries", ModuleObject_localExportEntriesGetter, 0),
diff --git a/js/src/builtin/ModuleObject.h b/js/src/builtin/ModuleObject.h
index be0315215..5a809c865 100644
--- a/js/src/builtin/ModuleObject.h
+++ b/js/src/builtin/ModuleObject.h
@@ -107,8 +107,8 @@ class IndirectBindingMap
 
     void trace(JSTracer* trc);
 
-    bool putNew(JSContext* cx, HandleId name,
-                HandleModuleEnvironmentObject environment, HandleId localName);
+    bool put(JSContext* cx, HandleId name,
+             HandleModuleEnvironmentObject environment, HandleId localName);
 
     size_t count() const {
         return map_.count();
@@ -218,7 +218,7 @@ class ModuleObject : public NativeObject
         EnvironmentSlot,
         NamespaceSlot,
         StatusSlot,
-        ErrorSlot,
+        EvaluationErrorSlot,
         HostDefinedSlot,
         RequestedModulesSlot,
         ImportEntriesSlot,
@@ -238,8 +238,8 @@ class ModuleObject : public NativeObject
                   "EnvironmentSlot must match self-hosting define");
     static_assert(StatusSlot == MODULE_OBJECT_STATUS_SLOT,
                   "StatusSlot must match self-hosting define");
-    static_assert(ErrorSlot == MODULE_OBJECT_ERROR_SLOT,
-                  "ErrorSlot must match self-hosting define");
+    static_assert(EvaluationErrorSlot == MODULE_OBJECT_EVALUATION_ERROR_SLOT,
+                  "EvaluationErrorSlot must match self-hosting define");
     static_assert(DFSIndexSlot == MODULE_OBJECT_DFS_INDEX_SLOT,
                   "DFSIndexSlot must match self-hosting define");
     static_assert(DFSAncestorIndexSlot == MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT,
@@ -269,7 +269,8 @@ class ModuleObject : public NativeObject
     ModuleEnvironmentObject* environment() const;
     ModuleNamespaceObject* namespace_();
     ModuleStatus status() const;
-    Value error() const;
+    bool hadEvaluationError() const;
+    Value evaluationError() const;
     Value hostDefinedField() const;
     ArrayObject& requestedModules() const;
     ArrayObject& importEntries() const;
diff --git a/js/src/builtin/SelfHostingDefines.h b/js/src/builtin/SelfHostingDefines.h
index a06c2aa62..6afa641b1 100644
--- a/js/src/builtin/SelfHostingDefines.h
+++ b/js/src/builtin/SelfHostingDefines.h
@@ -99,15 +99,15 @@
 
 #define MODULE_OBJECT_ENVIRONMENT_SLOT        2
 #define MODULE_OBJECT_STATUS_SLOT             4
-#define MODULE_OBJECT_ERROR_SLOT              5
+#define MODULE_OBJECT_EVALUATION_ERROR_SLOT   5 // 4
 #define MODULE_OBJECT_DFS_INDEX_SLOT          16
 #define MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT 17
 
-#define MODULE_STATUS_ERRORED        0
-#define MODULE_STATUS_UNINSTANTIATED 1
-#define MODULE_STATUS_INSTANTIATING  2
-#define MODULE_STATUS_INSTANTIATED   3
-#define MODULE_STATUS_EVALUATING     4
-#define MODULE_STATUS_EVALUATED      5
+#define MODULE_STATUS_UNINSTANTIATED  0
+#define MODULE_STATUS_INSTANTIATING   1
+#define MODULE_STATUS_INSTANTIATED    2
+#define MODULE_STATUS_EVALUATING      3
+#define MODULE_STATUS_EVALUATED       4
+#define MODULE_STATUS_EVALUATED_ERROR 5
 
 #endif
diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp
index 025620766..e93ebeec3 100644
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -3585,6 +3585,9 @@ GetModuleEnvironmentNames(JSContext* cx, unsigned argc, Value* vp)
         return false;
     }
 
+//-    if (module->status() == MODULE_STATUS_ERRORED) {
+//+    if (module->hadEvaluationError()) {
+
     RootedModuleEnvironmentObject env(cx, GetModuleEnvironment(cx, args[0]));
     Rooted<IdVector> ids(cx, IdVector(cx));
     if (!JS_Enumerate(cx, env, &ids))
@@ -3622,6 +3625,9 @@ GetModuleEnvironmentValue(JSContext* cx, unsigned argc, Value* vp)
         return false;
     }
 
+//-    if (module->status() == MODULE_STATUS_ERRORED) {
+//+    if (module->hadEvaluationError()) {
+
     RootedModuleEnvironmentObject env(cx, GetModuleEnvironment(cx, args[0]));
     RootedString name(cx, args[1].toString());
     RootedId id(cx);
diff --git a/js/src/jit-test/tests/modules/bug-1284486.js b/js/src/jit-test/tests/modules/bug-1284486.js
index 08286393a..6fbd67d9d 100644
--- a/js/src/jit-test/tests/modules/bug-1284486.js
+++ b/js/src/jit-test/tests/modules/bug-1284486.js
@@ -1,9 +1,8 @@
-// This tests that attempting to perform ModuleDeclarationInstantation a second
-// time after a failure re-throws the same error.
+// This tests that module instantiation can succeed when executed a second
+// time after a failure.
 //
 // The first attempt fails becuase module 'a' is not available. The second
-// attempt fails because of the previous failure (it would otherwise succeed as
-// 'a' is now available).
+// attempt succeeds as 'a' is now available.
 
 load(libdir + "dummyModuleResolveHook.js");
 
@@ -25,12 +24,9 @@ let a = moduleRepo['a'] = parseModule("export var a = 1; export var b = 2;");
 let d = moduleRepo['d'] = parseModule("import { a } from 'c'; a;");
 
 threw = false;
-let e2;
 try {
     d.declarationInstantiation();
 } catch (exc) {
     threw = true;
-    e2 = exc;
 }
-assertEq(threw, true);
-assertEq(e1, e2);
+assertEq(threw, false);
diff --git a/js/src/jit-test/tests/modules/bug-1420420-2.js b/js/src/jit-test/tests/modules/bug-1420420-2.js
new file mode 100644
index 000000000..0511e8126
--- /dev/null
+++ b/js/src/jit-test/tests/modules/bug-1420420-2.js
@@ -0,0 +1,19 @@
+// Test re-instantiation module after failure.
+
+load(libdir + "asserts.js");
+load(libdir + "dummyModuleResolveHook.js");
+
+moduleRepo["good"] = parseModule(`export let x`);
+
+moduleRepo["y1"] = parseModule(`export let y`);
+moduleRepo["y2"] = parseModule(`export let y`);
+moduleRepo["bad"] = parseModule(`export* from "y1"; export* from "y2";`);
+
+moduleRepo["a"] = parseModule(`import* as ns from "good"; import {y} from "bad";`);
+
+let b = moduleRepo["b"] = parseModule(`import "a";`);
+let c = moduleRepo["c"] = parseModule(`import "a";`);
+
+assertThrowsInstanceOf(() => b.declarationInstantiation(), SyntaxError);
+assertThrowsInstanceOf(() => c.declarationInstantiation(), SyntaxError);
+
diff --git a/js/src/jit-test/tests/modules/bug-1420420-3.js b/js/src/jit-test/tests/modules/bug-1420420-3.js
new file mode 100644
index 000000000..236c023b9
--- /dev/null
+++ b/js/src/jit-test/tests/modules/bug-1420420-3.js
@@ -0,0 +1,9 @@
+if (!('stackTest' in this))
+   quit();
+
+let a = parseModule(`throw new Error`);
+a.declarationInstantiation();
+stackTest(function() {
+    a.evaluation();
+});
+
diff --git a/js/src/jit-test/tests/modules/bug-1420420-4.js b/js/src/jit-test/tests/modules/bug-1420420-4.js
new file mode 100644
index 000000000..721c770bc
--- /dev/null
+++ b/js/src/jit-test/tests/modules/bug-1420420-4.js
@@ -0,0 +1,16 @@
+load(libdir + "asserts.js");
+load(libdir + "dummyModuleResolveHook.js");
+
+moduleRepo["a"] = parseModule(`throw undefined`);
+
+let b = moduleRepo["b"] = parseModule(`import "a";`);
+let c = moduleRepo["c"] = parseModule(`import "a";`);
+
+b.declarationInstantiation();
+c.declarationInstantiation();
+
+let count = 0;
+try { b.evaluation() } catch (e) { count++; }
+try { c.evaluation() } catch (e) { count++; }
+assertEq(count, 2);
+
diff --git a/js/src/jit-test/tests/modules/bug-1420420.js b/js/src/jit-test/tests/modules/bug-1420420.js
new file mode 100644
index 000000000..cd1101c76
--- /dev/null
+++ b/js/src/jit-test/tests/modules/bug-1420420.js
@@ -0,0 +1,19 @@
+// Test re-instantiation module after failure.
+
+load(libdir + "asserts.js");
+load(libdir + "dummyModuleResolveHook.js");
+
+moduleRepo["good"] = parseModule(`export let x`);
+
+moduleRepo["y1"] = parseModule(`export let y`);
+moduleRepo["y2"] = parseModule(`export let y`);
+moduleRepo["bad"] = parseModule(`export* from "y1"; export* from "y2";`);
+
+moduleRepo["a"] = parseModule(`import {x} from "good"; import {y} from "bad";`);
+
+let b = moduleRepo["b"] = parseModule(`import "a";`);
+let c = moduleRepo["c"] = parseModule(`import "a";`);
+
+assertThrowsInstanceOf(() => b.declarationInstantiation(), SyntaxError);
+assertThrowsInstanceOf(() => c.declarationInstantiation(), SyntaxError);
+
diff --git a/js/src/js.msg b/js/src/js.msg
index 9c508ebbd..587481864 100644
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -579,7 +579,6 @@ MSG_DEF(JSMSG_MISSING_IMPORT,            0, JSEXN_SYNTAXERR, "import not found")
 MSG_DEF(JSMSG_AMBIGUOUS_IMPORT,          0, JSEXN_SYNTAXERR, "ambiguous import")
 MSG_DEF(JSMSG_MISSING_NAMESPACE_EXPORT,  0, JSEXN_SYNTAXERR, "export not found for namespace")
 MSG_DEF(JSMSG_MISSING_EXPORT,            1, JSEXN_SYNTAXERR, "local binding for export '{0}' not found")
-MSG_DEF(JSMSG_MODULE_INSTANTIATE_FAILED, 0, JSEXN_INTERNALERR, "attempt to re-instantiate module after failure")
 MSG_DEF(JSMSG_BAD_MODULE_STATUS,         0, JSEXN_INTERNALERR, "module record has unexpected status")
 
 // Promise
diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
index cf5880e03..3e0c63811 100644
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4739,18 +4739,6 @@ JS::GetModuleScript(JSContext* cx, JS::HandleObject moduleArg)
     return moduleArg->as<ModuleObject>().script();
 }
 
-JS_PUBLIC_API(bool)
-JS::IsModuleErrored(JSObject* moduleArg)
-{
-    return moduleArg->as<ModuleObject>().status() == MODULE_STATUS_ERRORED;
-}
-
-JS_PUBLIC_API(JS::Value)
-JS::GetModuleError(JSObject* moduleArg)
-{
-    return moduleArg->as<ModuleObject>().error();
-}
-
 JS_PUBLIC_API(JSObject*)
 JS_New(JSContext* cx, HandleObject ctor, const JS::HandleValueArray& inputArgs)
 {
diff --git a/js/src/jsapi.h b/js/src/jsapi.h
index 9c3bf8151..76781cf06 100644
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -4395,12 +4395,6 @@ GetRequestedModules(JSContext* cx, JS::HandleObject moduleRecord);
 extern JS_PUBLIC_API(JSScript*)
 GetModuleScript(JSContext* cx, JS::HandleObject moduleRecord);
 
-extern JS_PUBLIC_API(bool)
-IsModuleErrored(JSObject* moduleRecord);
-
-extern JS_PUBLIC_API(JS::Value)
-GetModuleError(JSObject* moduleRecord);
-
 } /* namespace JS */
 
 extern JS_PUBLIC_API(bool)
diff --git a/js/src/vm/EnvironmentObject.cpp b/js/src/vm/EnvironmentObject.cpp
index 7834940f1..871ae3f19 100644
--- a/js/src/vm/EnvironmentObject.cpp
+++ b/js/src/vm/EnvironmentObject.cpp
@@ -491,7 +491,7 @@ ModuleEnvironmentObject::createImportBinding(JSContext* cx, HandleAtom importNam
     RootedId importNameId(cx, AtomToId(importName));
     RootedId localNameId(cx, AtomToId(localName));
     RootedModuleEnvironmentObject env(cx, &module->initialEnvironment());
-    if (!importBindings().putNew(cx, importNameId, env, localNameId)) {
+    if (!importBindings().put(cx, importNameId, env, localNameId)) {
         ReportOutOfMemory(cx);
         return false;
     }
diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp
index 0dfeffc36..f98e968b8 100644
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -2087,7 +2087,6 @@ intrinsic_CreateNamespaceBinding(JSContext* cx, unsigned argc, Value* vp)
     // the slot directly.
     RootedShape shape(cx, environment->lookup(cx, name));
     MOZ_ASSERT(shape);
-    MOZ_ASSERT(environment->getSlot(shape->slot()).isMagic(JS_UNINITIALIZED_LEXICAL));
     environment->setSlot(shape->slot(), args[2]);
     args.rval().setUndefined();
     return true;
-- 
cgit v1.2.3


From 70bafe4df514219dfc02185804286d1619290a16 Mon Sep 17 00:00:00 2001
From: Moonchild <moonchild@palemoon.org>
Date: Wed, 8 Jul 2020 14:12:57 +0000
Subject: Issue #618 - Use a single slot for the module's environment object.

According to the spec this isn't created until the module is instantiated, but
we create it when we compile the module.
We stored this previously in InitialEnvironmentSlot and copied it to
EnvironmentSlot when it was supposed to be created, but we can just store it in
the latter slot straight away and check the module's status and return null if
it shouldn't exist yet.

This reduces the number of slots needed on a moduleObject to 17.

Re: BZ 1420412 Part 1
We can't implement the second part to further reduce our number of slots,
because it relies on SetProxyReservedSlot which in turn relies on
rearchitecturing JS proxies to make reserved slots dynamic. That's a rabbit hole
we really don't want to fall into.
So, we'll end up being a bit slower because it can't be in-line allocated with
having more than 16 slots, but so be it. I sincerely doubt it will make any
practical difference.
---
 js/src/builtin/Module.js            |  7 +++++--
 js/src/builtin/ModuleObject.cpp     | 41 +++++++++++++++++--------------------
 js/src/builtin/ModuleObject.h       |  4 ----
 js/src/builtin/SelfHostingDefines.h | 10 ++++-----
 js/src/builtin/TestingFunctions.cpp |  2 --
 js/src/vm/SelfHosting.cpp           | 12 -----------
 6 files changed, 29 insertions(+), 47 deletions(-)

(limited to 'js/src')

diff --git a/js/src/builtin/Module.js b/js/src/builtin/Module.js
index 86b44880d..c9f20c18c 100644
--- a/js/src/builtin/Module.js
+++ b/js/src/builtin/Module.js
@@ -234,8 +234,11 @@ function GetModuleEnvironment(module)
 {
     assert(IsModule(module), "Non-module passed to GetModuleEnvironment");
 
+    assert(module.status >= MODULE_STATUS_INSTANTIATING,
+           "Attempt to access module environement before instantation");
+
     let env = UnsafeGetReservedSlot(module, MODULE_OBJECT_ENVIRONMENT_SLOT);
-    assert(env === undefined || IsModuleEnvironment(env),
+    assert(IsModuleEnvironment(env),
            "Module environment slot contains unexpected value");
 
     return env;
@@ -421,7 +424,7 @@ function ModuleDeclarationEnvironmentSetup(module)
     }
 
     // Steps 5-6
-    CreateModuleEnvironment(module);
+    // Note that we have already created the environment by this point.
     let env = GetModuleEnvironment(module);
 
     // Step 8
diff --git a/js/src/builtin/ModuleObject.cpp b/js/src/builtin/ModuleObject.cpp
index a9a257178..14d27845c 100644
--- a/js/src/builtin/ModuleObject.cpp
+++ b/js/src/builtin/ModuleObject.cpp
@@ -657,17 +657,24 @@ ModuleObject::finalize(js::FreeOp* fop, JSObject* obj)
         fop->delete_(funDecls);
 }
 
+ModuleEnvironmentObject&
+ModuleObject::initialEnvironment() const
+{
+    Value value = getReservedSlot(EnvironmentSlot);
+    return value.toObject().as<ModuleEnvironmentObject>();
+}
+
 ModuleEnvironmentObject*
 ModuleObject::environment() const
 {
-//-    MOZ_ASSERT(status() != MODULE_STATUS_ERRORED);
-//+    MOZ_ASSERT(!hadEvaluationError());
+    MOZ_ASSERT(!hadEvaluationError());
 
-    Value value = getReservedSlot(EnvironmentSlot);
-    if (value.isUndefined())
+    // According to the spec the environment record is created during
+    // instantiation, but we create it earlier than that.
+    if (status() < MODULE_STATUS_INSTANTIATED)
         return nullptr;
 
-    return &value.toObject().as<ModuleEnvironmentObject>();
+    return &initialEnvironment();
 }
 
 bool
@@ -732,7 +739,7 @@ ModuleObject::init(HandleScript script)
 void
 ModuleObject::setInitialEnvironment(HandleModuleEnvironmentObject initialEnvironment)
 {
-    initReservedSlot(InitialEnvironmentSlot, ObjectValue(*initialEnvironment));
+    initReservedSlot(EnvironmentSlot, ObjectValue(*initialEnvironment));
 }
 
 void
@@ -867,12 +874,6 @@ ModuleObject::setHostDefinedField(const JS::Value& value)
     setReservedSlot(HostDefinedSlot, value);
 }
 
-ModuleEnvironmentObject&
-ModuleObject::initialEnvironment() const
-{
-    return getReservedSlot(InitialEnvironmentSlot).toObject().as<ModuleEnvironmentObject>();
-}
-
 Scope*
 ModuleObject::enclosingScope() const
 {
@@ -898,16 +899,6 @@ ModuleObject::trace(JSTracer* trc, JSObject* obj)
         funDecls->trace(trc);
 }
 
-void
-ModuleObject::createEnvironment()
-{
-    // The environment has already been created, we just neet to set it in the
-    // right slot.
-    MOZ_ASSERT(!getReservedSlot(InitialEnvironmentSlot).isUndefined());
-    MOZ_ASSERT(getReservedSlot(EnvironmentSlot).isUndefined());
-    setReservedSlot(EnvironmentSlot, getReservedSlot(InitialEnvironmentSlot));
-}
-
 bool
 ModuleObject::noteFunctionDeclaration(ExclusiveContext* cx, HandleAtom name, HandleFunction fun)
 {
@@ -923,7 +914,10 @@ ModuleObject::noteFunctionDeclaration(ExclusiveContext* cx, HandleAtom name, Han
 /* static */ bool
 ModuleObject::instantiateFunctionDeclarations(JSContext* cx, HandleModuleObject self)
 {
+#ifdef DEBUG
+    MOZ_ASSERT(self->status() == MODULE_STATUS_INSTANTIATING);
     MOZ_ASSERT(IsFrozen(cx, self));
+#endif
 
     FunctionDeclarationVector* funDecls = self->functionDeclarations();
     if (!funDecls) {
@@ -954,7 +948,10 @@ ModuleObject::instantiateFunctionDeclarations(JSContext* cx, HandleModuleObject
 /* static */ bool
 ModuleObject::execute(JSContext* cx, HandleModuleObject self, MutableHandleValue rval)
 {
+#ifdef DEBUG
+    MOZ_ASSERT(self->status() == MODULE_STATUS_EVALUATING);
     MOZ_ASSERT(IsFrozen(cx, self));
+#endif
 
     RootedScript script(cx, self->script());
     RootedModuleEnvironmentObject scope(cx, self->environment());
diff --git a/js/src/builtin/ModuleObject.h b/js/src/builtin/ModuleObject.h
index 5a809c865..bd3e7044e 100644
--- a/js/src/builtin/ModuleObject.h
+++ b/js/src/builtin/ModuleObject.h
@@ -214,7 +214,6 @@ class ModuleObject : public NativeObject
     enum
     {
         ScriptSlot = 0,
-        InitialEnvironmentSlot,
         EnvironmentSlot,
         NamespaceSlot,
         StatusSlot,
@@ -286,9 +285,6 @@ class ModuleObject : public NativeObject
 
     void setHostDefinedField(const JS::Value& value);
 
-    // For intrinsic_CreateModuleEnvironment.
-    void createEnvironment();
-
     // For BytecodeEmitter.
     bool noteFunctionDeclaration(ExclusiveContext* cx, HandleAtom name, HandleFunction fun);
 
diff --git a/js/src/builtin/SelfHostingDefines.h b/js/src/builtin/SelfHostingDefines.h
index 6afa641b1..a3ba36804 100644
--- a/js/src/builtin/SelfHostingDefines.h
+++ b/js/src/builtin/SelfHostingDefines.h
@@ -97,11 +97,11 @@
 #define REGEXP_STRING_ITERATOR_FLAGS_SLOT 2
 #define REGEXP_STRING_ITERATOR_DONE_SLOT 3
 
-#define MODULE_OBJECT_ENVIRONMENT_SLOT        2
-#define MODULE_OBJECT_STATUS_SLOT             4
-#define MODULE_OBJECT_EVALUATION_ERROR_SLOT   5 // 4
-#define MODULE_OBJECT_DFS_INDEX_SLOT          16
-#define MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT 17
+#define MODULE_OBJECT_ENVIRONMENT_SLOT        1
+#define MODULE_OBJECT_STATUS_SLOT             3
+#define MODULE_OBJECT_EVALUATION_ERROR_SLOT   4
+#define MODULE_OBJECT_DFS_INDEX_SLOT          15
+#define MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT 16
 
 #define MODULE_STATUS_UNINSTANTIATED  0
 #define MODULE_STATUS_INSTANTIATING   1
diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp
index e93ebeec3..f589b8076 100644
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -3566,8 +3566,6 @@ GetModuleEnvironment(JSContext* cx, HandleValue moduleValue)
     // before they have been instantiated.
     RootedModuleEnvironmentObject env(cx, &module->initialEnvironment());
     MOZ_ASSERT(env);
-    MOZ_ASSERT_IF(module->environment(), module->environment() == env);
-
     return env;
 }
 
diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp
index f98e968b8..50b0c01de 100644
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -2048,17 +2048,6 @@ intrinsic_HostResolveImportedModule(JSContext* cx, unsigned argc, Value* vp)
     return true;
 }
 
-static bool
-intrinsic_CreateModuleEnvironment(JSContext* cx, unsigned argc, Value* vp)
-{
-    CallArgs args = CallArgsFromVp(argc, vp);
-    MOZ_ASSERT(args.length() == 1);
-    RootedModuleObject module(cx, &args[0].toObject().as<ModuleObject>());
-    module->createEnvironment();
-    args.rval().setUndefined();
-    return true;
-}
-
 static bool
 intrinsic_CreateImportBinding(JSContext* cx, unsigned argc, Value* vp)
 {
@@ -2658,7 +2647,6 @@ static const JSFunctionSpec intrinsic_functions[] = {
           CallNonGenericSelfhostedMethod<Is<ModuleObject>>, 2, 0),
     JS_FN("HostResolveImportedModule", intrinsic_HostResolveImportedModule, 2, 0),
     JS_FN("IsModuleEnvironment", intrinsic_IsInstanceOfBuiltin<ModuleEnvironmentObject>, 1, 0),
-    JS_FN("CreateModuleEnvironment", intrinsic_CreateModuleEnvironment, 1, 0),
     JS_FN("CreateImportBinding", intrinsic_CreateImportBinding, 4, 0),
     JS_FN("CreateNamespaceBinding", intrinsic_CreateNamespaceBinding, 3, 0),
     JS_FN("InstantiateModuleFunctionDeclarations",
-- 
cgit v1.2.3


From c8a38346c88995b4ba7e07a225c3a8ba860567c6 Mon Sep 17 00:00:00 2001
From: Moonchild <moonchild@palemoon.org>
Date: Wed, 8 Jul 2020 14:53:31 +0000
Subject: Issue #618 - Lazily initialise module binding maps

Make it so they are not allocated on a background thread in a different zone to
the final module.

Ref: BZ 1372258
---
 js/src/builtin/ModuleObject.cpp | 46 +++++++++++++++++++++++------------------
 js/src/builtin/ModuleObject.h   | 16 +++++++-------
 2 files changed, 35 insertions(+), 27 deletions(-)

(limited to 'js/src')

diff --git a/js/src/builtin/ModuleObject.cpp b/js/src/builtin/ModuleObject.cpp
index 14d27845c..44e5a2c88 100644
--- a/js/src/builtin/ModuleObject.cpp
+++ b/js/src/builtin/ModuleObject.cpp
@@ -248,21 +248,13 @@ IndirectBindingMap::Binding::Binding(ModuleEnvironmentObject* environment, Shape
   : environment(environment), shape(shape)
 {}
 
-IndirectBindingMap::IndirectBindingMap(Zone* zone)
-  : map_(ZoneAllocPolicy(zone))
-{
-}
-
-bool
-IndirectBindingMap::init()
-{
-    return map_.init();
-}
-
 void
 IndirectBindingMap::trace(JSTracer* trc)
 {
-    for (Map::Enum e(map_); !e.empty(); e.popFront()) {
+    if (!map_)
+        return;
+
+    for (Map::Enum e(*map_); !e.empty(); e.popFront()) {
         Binding& b = e.front().value();
         TraceEdge(trc, &b.environment, "module bindings environment");
         TraceEdge(trc, &b.shape, "module bindings shape");
@@ -276,9 +268,22 @@ bool
 IndirectBindingMap::put(JSContext* cx, HandleId name,
                         HandleModuleEnvironmentObject environment, HandleId localName)
 {
+    // This object might have been allocated on the background parsing thread in
+    // different zone to the final module. Lazily allocate the map so we don't
+    // have to switch its zone when merging compartments.
+    if (!map_) {
+        MOZ_ASSERT(!cx->zone()->group()->createdForHelperThread());
+        map_.emplace(cx->zone());
+        if (!map_->init()) {
+            map_.reset();
+            ReportOutOfMemory(cx);
+            return false;
+        }
+    }
+
     RootedShape shape(cx, environment->lookup(cx, localName));
     MOZ_ASSERT(shape);
-    if (!map_.put(name, Binding(environment, shape))) {
+    if (!map_->put(name, Binding(environment, shape))) {
         ReportOutOfMemory(cx);
         return false;
     }
@@ -289,7 +294,10 @@ IndirectBindingMap::put(JSContext* cx, HandleId name,
 bool
 IndirectBindingMap::lookup(jsid name, ModuleEnvironmentObject** envOut, Shape** shapeOut) const
 {
-    auto ptr = map_.lookup(name);
+    if (!map_)
+        return false;
+
+    auto ptr = map_->lookup(name);
     if (!ptr)
         return false;
 
@@ -625,10 +633,9 @@ ModuleObject::create(ExclusiveContext* cx)
     RootedModuleObject self(cx, &obj->as<ModuleObject>());
 
     Zone* zone = cx->zone();
-    IndirectBindingMap* bindings = zone->new_<IndirectBindingMap>(zone);
-    if (!bindings || !bindings->init()) {
+    IndirectBindingMap* bindings = zone->new_<IndirectBindingMap>();
+    if (!bindings) {
         ReportOutOfMemory(cx);
-        js_delete<IndirectBindingMap>(bindings);
         return nullptr;
     }
 
@@ -974,10 +981,9 @@ ModuleObject::createNamespace(JSContext* cx, HandleModuleObject self, HandleObje
         return nullptr;
 
     Zone* zone = cx->zone();
-    IndirectBindingMap* bindings = zone->new_<IndirectBindingMap>(zone);
-    if (!bindings || !bindings->init()) {
+    IndirectBindingMap* bindings = zone->new_<IndirectBindingMap>();
+    if (!bindings) {
         ReportOutOfMemory(cx);
-        js_delete<IndirectBindingMap>(bindings);
         return nullptr;
     }
 
diff --git a/js/src/builtin/ModuleObject.h b/js/src/builtin/ModuleObject.h
index bd3e7044e..5e4e7f342 100644
--- a/js/src/builtin/ModuleObject.h
+++ b/js/src/builtin/ModuleObject.h
@@ -7,6 +7,8 @@
 #ifndef builtin_ModuleObject_h
 #define builtin_ModuleObject_h
 
+#include "mozilla/Maybe.h"
+
 #include "jsapi.h"
 #include "jsatom.h"
 
@@ -102,27 +104,27 @@ typedef Handle<ExportEntryObject*> HandleExportEntryObject;
 class IndirectBindingMap
 {
   public:
-    explicit IndirectBindingMap(Zone* zone);
-    bool init();
-
     void trace(JSTracer* trc);
 
     bool put(JSContext* cx, HandleId name,
              HandleModuleEnvironmentObject environment, HandleId localName);
 
     size_t count() const {
-        return map_.count();
+        return map_ ? map_->count() : 0;
     }
 
     bool has(jsid name) const {
-        return map_.has(name);
+        return map_ ? map_->has(name) : false;
     }
 
     bool lookup(jsid name, ModuleEnvironmentObject** envOut, Shape** shapeOut) const;
 
     template <typename Func>
     void forEachExportedName(Func func) const {
-        for (auto r = map_.all(); !r.empty(); r.popFront())
+        if (!map_)
+            return;
+
+        for (auto r = map_->all(); !r.empty(); r.popFront())
             func(r.front().key());
     }
 
@@ -136,7 +138,7 @@ class IndirectBindingMap
 
     typedef HashMap<jsid, Binding, DefaultHasher<jsid>, ZoneAllocPolicy> Map;
 
-    Map map_;
+    mozilla::Maybe<Map> map_;
 };
 
 class ModuleNamespaceObject : public ProxyObject
-- 
cgit v1.2.3


From daf56131a73bb2b214fee4d7ba78abe0576ba696 Mon Sep 17 00:00:00 2001
From: Gaming4JC <g4jc@hyperbola.info>
Date: Mon, 3 Aug 2020 14:04:26 -0400
Subject: Issue #618 - Lazily initialise module binding maps - Debug follow up

The added debug assertion does not work due to missing API.
They were added in BZ 1337491, 1395366, and others, but were primarily used for multi-threading.
This uses our existing non-multithreaded syntax instead, resolving a `no member named` build error.
---
 js/src/builtin/ModuleObject.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'js/src')

diff --git a/js/src/builtin/ModuleObject.cpp b/js/src/builtin/ModuleObject.cpp
index 44e5a2c88..728929e8c 100644
--- a/js/src/builtin/ModuleObject.cpp
+++ b/js/src/builtin/ModuleObject.cpp
@@ -272,7 +272,7 @@ IndirectBindingMap::put(JSContext* cx, HandleId name,
     // different zone to the final module. Lazily allocate the map so we don't
     // have to switch its zone when merging compartments.
     if (!map_) {
-        MOZ_ASSERT(!cx->zone()->group()->createdForHelperThread());
+        MOZ_ASSERT(!cx->zone()->usedByExclusiveThread);
         map_.emplace(cx->zone());
         if (!map_->init()) {
             map_.reset();
-- 
cgit v1.2.3


From 770b1c9071d379c74b8e68c4c7488365b60ff08f Mon Sep 17 00:00:00 2001
From: Gaming4JC <g4jc@hyperbola.info>
Date: Mon, 3 Aug 2020 14:31:43 -0400
Subject: Issue #618 - Align error handling for module scripts with the spec -
 Debug follow up

MODULE_STATUS_ERRORED is no more. Replacing with newer API.

Ref: BZ 1420420
---
 js/src/builtin/Module.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'js/src')

diff --git a/js/src/builtin/Module.js b/js/src/builtin/Module.js
index c9f20c18c..355303d44 100644
--- a/js/src/builtin/Module.js
+++ b/js/src/builtin/Module.js
@@ -67,7 +67,8 @@ function ModuleGetExportedNames(exportStarSet = [])
 
 function ModuleSetStatus(module, newStatus)
 {
-    assert(newStatus >= MODULE_STATUS_ERRORED && newStatus <= MODULE_STATUS_EVALUATED,
+    assert(newStatus >= MODULE_STATUS_UNINSTANTIATED &&
+           newStatus <= MODULE_STATUS_EVALUATED_ERROR,
            "Bad new module status in ModuleSetStatus");
 
     // Note that under OOM conditions we can fail the module instantiation
-- 
cgit v1.2.3


From 1012dbe9e5b2d00f967b0523f94ac8cc7ed3118d Mon Sep 17 00:00:00 2001
From: Moonchild <moonchild@palemoon.org>
Date: Thu, 6 Aug 2020 18:27:32 +0000
Subject: [js] Add some utility functions to get the current JS runtime.

---
 js/src/jsapi.cpp | 5 +++++
 js/src/jsapi.h   | 3 +++
 2 files changed, 8 insertions(+)

(limited to 'js/src')

diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
index 3e0c63811..77124355c 100644
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -483,6 +483,11 @@ JS_DestroyContext(JSContext* cx)
     DestroyContext(cx);
 }
 
+JS_PUBLIC_API(JSRuntime*)
+JS_GetRuntime(JSContext* cx) {
+    return cx->runtime();
+}
+
 static JS_CurrentEmbedderTimeFunction currentEmbedderTimeFunction;
 
 JS_PUBLIC_API(void)
diff --git a/js/src/jsapi.h b/js/src/jsapi.h
index 76781cf06..c6299e3f5 100644
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -995,6 +995,9 @@ JS_NewContext(uint32_t maxbytes,
 extern JS_PUBLIC_API(void)
 JS_DestroyContext(JSContext* cx);
 
+extern JS_PUBLIC_API(JSRuntime*)
+JS_GetRuntime(JSContext* cx);
+
 typedef double (*JS_CurrentEmbedderTimeFunction)();
 
 /**
-- 
cgit v1.2.3


From 10a10fd3757374123eb5e3aab1e4720f86575f47 Mon Sep 17 00:00:00 2001
From: Moonchild <moonchild@palemoon.org>
Date: Thu, 6 Aug 2020 18:31:36 +0000
Subject: Issue #618 - Simplify module resolve hook to be a function pointer

This is an ahead-of time port to try and address #1624.
This is based on BZ 1461751 and Jon Coppeard's work in it.
---
 js/src/jsapi.cpp          | 15 +++++----------
 js/src/jsapi.h            | 12 +++++++-----
 js/src/shell/js.cpp       | 33 +++++++++++++++++++++++++++++----
 js/src/vm/GlobalObject.h  | 14 --------------
 js/src/vm/Runtime.cpp     |  6 ++++--
 js/src/vm/Runtime.h       |  3 +++
 js/src/vm/SelfHosting.cpp | 15 ++++++++-------
 7 files changed, 56 insertions(+), 42 deletions(-)

(limited to 'js/src')

diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
index 77124355c..69a3ba2ac 100644
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4667,21 +4667,16 @@ JS::Evaluate(JSContext* cx, const ReadOnlyCompileOptions& optionsArg,
     return ::Evaluate(cx, optionsArg, filename, rval);
 }
 
-JS_PUBLIC_API(JSFunction*)
-JS::GetModuleResolveHook(JSContext* cx)
+JS_PUBLIC_API(JS::ModuleResolveHook)
+JS::GetModuleResolveHook(JSRuntime* rt)
 {
-    AssertHeapIsIdle(cx);
-    CHECK_REQUEST(cx);
-    return cx->global()->moduleResolveHook();
+    return rt->moduleResolveHook;
 }
 
 JS_PUBLIC_API(void)
-JS::SetModuleResolveHook(JSContext* cx, HandleFunction func)
+JS::SetModuleResolveHook(JSRuntime* rt, JS::ModuleResolveHook func)
 {
-    AssertHeapIsIdle(cx);
-    CHECK_REQUEST(cx);
-    assertSameCompartment(cx, func);
-    cx->global()->setModuleResolveHook(func);
+    rt->moduleResolveHook = func;
 }
 
 JS_PUBLIC_API(bool)
diff --git a/js/src/jsapi.h b/js/src/jsapi.h
index c6299e3f5..5cdfd958e 100644
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -4325,17 +4325,19 @@ extern JS_PUBLIC_API(bool)
 Evaluate(JSContext* cx, const ReadOnlyCompileOptions& options,
          const char* filename, JS::MutableHandleValue rval);
 
+using ModuleResolveHook = JSObject* (*)(JSContext*, HandleObject, HandleString);
+
 /**
- * Get the HostResolveImportedModule hook for a global.
+ * Get the HostResolveImportedModule hook for the runtime.
  */
-extern JS_PUBLIC_API(JSFunction*)
-GetModuleResolveHook(JSContext* cx);
+extern JS_PUBLIC_API(ModuleResolveHook)
+GetModuleResolveHook(JSRuntime* rt);
 
 /**
- * Set the HostResolveImportedModule hook for a global to the given function.
+ * Set the HostResolveImportedModule hook for the runtime to the given function.
  */
 extern JS_PUBLIC_API(void)
-SetModuleResolveHook(JSContext* cx, JS::HandleFunction func);
+SetModuleResolveHook(JSRuntime* rt, ModuleResolveHook func);
 
 /**
  * Parse the given source buffer as a module in the scope of the current global
diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp
index 8cd821b31..bbf35459f 100644
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -274,6 +274,7 @@ struct ShellContext
     JS::PersistentRooted<JobQueue> jobQueue;
     ExclusiveData<ShellAsyncTasks> asyncTasks;
     bool drainingJobQueue;
+    JS::PersistentRootedFunction moduleResolveHook;
 
     /*
      * Watchdog thread state.
@@ -439,7 +440,8 @@ ShellContext::ShellContext(JSContext* cx)
     exitCode(0),
     quitting(false),
     readLineBufPos(0),
-    spsProfilingStackSize(0)
+    spsProfilingStackSize(0),
+    moduleResolveHook(cx)
 {}
 
 static ShellContext*
@@ -4030,13 +4032,34 @@ SetModuleResolveHook(JSContext* cx, unsigned argc, Value* vp)
         return false;
     }
 
-    RootedFunction hook(cx, &args[0].toObject().as<JSFunction>());
-    Rooted<GlobalObject*> global(cx, cx->global());
-    global->setModuleResolveHook(hook);
+    ShellContext* sc = GetShellContext(cx);
+    sc->moduleResolveHook = &args[0].toObject().as<JSFunction>();
+
     args.rval().setUndefined();
     return true;
 }
 
+static JSObject*
+CallModuleResolveHook(JSContext* cx, HandleObject module, HandleString specifier)
+{
+    ShellContext* sc = GetShellContext(cx);
+
+    JS::AutoValueArray<2> args(cx);
+    args[0].setObject(*module);
+    args[1].setString(specifier);
+
+    RootedValue result(cx);
+    if (!JS_CallFunction(cx, nullptr, sc->moduleResolveHook, args, &result))
+        return nullptr;
+
+    if (!result.isObject() || !result.toObject().is<ModuleObject>()) {
+         JS_ReportErrorASCII(cx, "Module resolve hook did not return Module object");
+         return nullptr;
+    }
+
+    return &result.toObject();
+}
+
 static bool
 GetModuleLoadPath(JSContext* cx, unsigned argc, Value* vp)
 {
@@ -7962,6 +7985,8 @@ main(int argc, char** argv, char** envp)
 
     js::SetPreserveWrapperCallback(cx, DummyPreserveWrapperCallback);
 
+    JS::SetModuleResolveHook(cx->runtime(), CallModuleResolveHook);
+
     result = Shell(cx, &op, envp);
 
 #ifdef DEBUG
diff --git a/js/src/vm/GlobalObject.h b/js/src/vm/GlobalObject.h
index 720f63e69..a9f07d495 100644
--- a/js/src/vm/GlobalObject.h
+++ b/js/src/vm/GlobalObject.h
@@ -119,7 +119,6 @@ class GlobalObject : public NativeObject
         DEBUGGERS,
         INTRINSICS,
         FOR_OF_PIC_CHAIN,
-        MODULE_RESOLVE_HOOK,
         WINDOW_PROXY,
         GLOBAL_THIS_RESOLVED,
 
@@ -879,19 +878,6 @@ class GlobalObject : public NativeObject
         setReservedSlot(WINDOW_PROXY, ObjectValue(*windowProxy));
     }
 
-    void setModuleResolveHook(HandleFunction hook) {
-        MOZ_ASSERT(hook);
-        setSlot(MODULE_RESOLVE_HOOK, ObjectValue(*hook));
-    }
-
-    JSFunction* moduleResolveHook() {
-        Value value = getSlotRef(MODULE_RESOLVE_HOOK);
-        if (value.isUndefined())
-            return nullptr;
-
-        return &value.toObject().as<JSFunction>();
-    }
-
     // Returns either this global's star-generator function prototype, or null
     // if that object was never created.  Dodgy; for use only in also-dodgy
     // GlobalHelperThreadState::mergeParseTaskCompartment().
diff --git a/js/src/vm/Runtime.cpp b/js/src/vm/Runtime.cpp
index 284a4f3d7..5e0246b85 100644
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -241,8 +241,10 @@ JSRuntime::JSRuntime(JSRuntime* parentRuntime)
     lastAnimationTime(0),
     performanceMonitoring(thisFromCtor()),
     ionLazyLinkListSize_(0),
-    stackFormat_(parentRuntime ? js::StackFormat::Default
-                               : js::StackFormat::SpiderMonkey)
+    stackFormat_(parentRuntime ?
+                 js::StackFormat::Default :
+                 js::StackFormat::SpiderMonkey),
+    moduleResolveHook()
 {
     setGCStoreBufferPtr(&gc.storeBuffer);
 
diff --git a/js/src/vm/Runtime.h b/js/src/vm/Runtime.h
index e60371e38..247a2dc9d 100644
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -1294,6 +1294,9 @@ struct JSRuntime : public JS::shadow::Runtime,
     // For inherited heap state accessors.
     friend class js::gc::AutoTraceSession;
     friend class JS::AutoEnterCycleCollection;
+
+    // The implementation-defined abstract operation HostResolveImportedModule.
+    JS::ModuleResolveHook moduleResolveHook;
 };
 
 namespace js {
diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp
index 50b0c01de..f324a0a67 100644
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -2026,25 +2026,26 @@ intrinsic_HostResolveImportedModule(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     MOZ_ASSERT(args.length() == 2);
-    MOZ_ASSERT(args[0].toObject().is<ModuleObject>());
-    MOZ_ASSERT(args[1].isString());
+    RootedModuleObject module(cx, &args[0].toObject().as<ModuleObject>());
+    RootedString specifier(cx, args[1].toString());
 
-    RootedFunction moduleResolveHook(cx, cx->global()->moduleResolveHook());
+    JS::ModuleResolveHook moduleResolveHook = cx->runtime()->moduleResolveHook;
     if (!moduleResolveHook) {
         JS_ReportErrorASCII(cx, "Module resolve hook not set");
         return false;
     }
 
-    RootedValue result(cx);
-    if (!JS_CallFunction(cx, nullptr, moduleResolveHook, args, &result))
+    RootedObject result(cx);
+    result = moduleResolveHook(cx, module, specifier);
+    if (!result)
         return false;
 
-    if (!result.isObject() || !result.toObject().is<ModuleObject>()) {
+    if (!result->is<ModuleObject>()) {
         JS_ReportErrorASCII(cx, "Module resolve hook did not return Module object");
         return false;
     }
 
-    args.rval().set(result);
+    args.rval().setObject(*result);
     return true;
 }
 
-- 
cgit v1.2.3