Bug 1284486 - Disallow returying ModuleDeclarationInstantiation after error r=shu

js/src/builtin/Module.js

@@ -183,6 +183,30 @@ function ModuleNamespaceCreate(module, exports)
     return ns;
 }
 
+function GetModuleEnvironment(module)
+{
+    assert(IsModule(module), "Non-module passed to GetModuleEnvironment");
+
+    let env = UnsafeGetReservedSlot(module, MODULE_OBJECT_ENVIRONMENT_SLOT);
+    assert(env === undefined || env === null || IsModuleEnvironment(env),
+          "Module environment slot contains unexpected value");
+
+    // Check for a previous failed attempt to instantiate this module. This can
+    // only happen due to a bug in the module loader.
+    if (env === null)
+        ThrowInternalError(JSMSG_MODULE_INSTANTIATE_FAILED);
+
+    return env;
+}
+
+function RecordInstantationFailure(module)
+{
+    // Set the module's environment slot to 'null' to indicate a failed module
+    // instantiation.
+    assert(IsModule(module), "Non-module passed to RecordInstantationFailure");
+    UnsafeSetReservedSlot(module, MODULE_OBJECT_ENVIRONMENT_SLOT, null);
+}
+
 // 15.2.1.16.4 ModuleDeclarationInstantiation()
 function ModuleDeclarationInstantiation()
 {
@@ -200,46 +224,51 @@ function ModuleDeclarationInstantiation()
     CreateModuleEnvironment(module);
     let env = GetModuleEnvironment(module);
 
-    // Step 8
+    try {
-    let requestedModules = module.requestedModules;
+        // Step 8
-    for (let i = 0; i < requestedModules.length; i++) {
+        let requestedModules = module.requestedModules;
-        let required = requestedModules[i];
+        for (let i = 0; i < requestedModules.length; i++) {
-        let requiredModule = HostResolveImportedModule(module, required);
+            let required = requestedModules[i];
-        callFunction(requiredModule.declarationInstantiation, requiredModule);
+            let requiredModule = HostResolveImportedModule(module, required);
-    }
+            callFunction(requiredModule.declarationInstantiation, requiredModule);
-
-    // Step 9
-    let indirectExportEntries = module.indirectExportEntries;
-    for (let i = 0; i < indirectExportEntries.length; i++) {
-        let e = indirectExportEntries[i];
-        let resolution = callFunction(module.resolveExport, module, e.exportName);
-        if (resolution === null)
-            ThrowSyntaxError(JSMSG_MISSING_INDIRECT_EXPORT, e.exportName);
-        if (resolution === "ambiguous")
-            ThrowSyntaxError(JSMSG_AMBIGUOUS_INDIRECT_EXPORT, e.exportName);
-    }
-
-    // Step 12
-    let importEntries = module.importEntries;
-    for (let i = 0; i < importEntries.length; i++) {
-        let imp = importEntries[i];
-        let importedModule = HostResolveImportedModule(module, imp.moduleRequest);
-        if (imp.importName === "*") {
-            let namespace = GetModuleNamespace(importedModule);
-            CreateNamespaceBinding(env, imp.localName, namespace);
-        } else {
-            let resolution = callFunction(importedModule.resolveExport, importedModule,
-                                          imp.importName);
-            if (resolution === null)
-                ThrowSyntaxError(JSMSG_MISSING_IMPORT, imp.importName);
-            if (resolution === "ambiguous")
-                ThrowSyntaxError(JSMSG_AMBIGUOUS_IMPORT, imp.importName);
-            CreateImportBinding(env, imp.localName, resolution.module, resolution.bindingName);
         }
-    }
 
-    // Step 16.iv
+        // Step 9
-    InstantiateModuleFunctionDeclarations(module);
+        let indirectExportEntries = module.indirectExportEntries;
+        for (let i = 0; i < indirectExportEntries.length; i++) {
+            let e = indirectExportEntries[i];
+            let resolution = callFunction(module.resolveExport, module, e.exportName);
+            if (resolution === null)
+                ThrowSyntaxError(JSMSG_MISSING_INDIRECT_EXPORT, e.exportName);
+            if (resolution === "ambiguous")
+                ThrowSyntaxError(JSMSG_AMBIGUOUS_INDIRECT_EXPORT, e.exportName);
+        }
+
+        // Step 12
+        let importEntries = module.importEntries;
+        for (let i = 0; i < importEntries.length; i++) {
+            let imp = importEntries[i];
+            let importedModule = HostResolveImportedModule(module, imp.moduleRequest);
+            if (imp.importName === "*") {
+                let namespace = GetModuleNamespace(importedModule);
+                CreateNamespaceBinding(env, imp.localName, namespace);
+            } else {
+                let resolution = callFunction(importedModule.resolveExport, importedModule,
+                                              imp.importName);
+                if (resolution === null)
+                    ThrowSyntaxError(JSMSG_MISSING_IMPORT, imp.importName);
+                if (resolution === "ambiguous")
+                    ThrowSyntaxError(JSMSG_AMBIGUOUS_IMPORT, imp.importName);
+                CreateImportBinding(env, imp.localName, resolution.module, resolution.bindingName);
+            }
+        }
+
+        // Step 16.iv
+        InstantiateModuleFunctionDeclarations(module);
+    } catch (e) {
+        RecordInstantationFailure(module);
+        throw e;
+    }
 }
 _SetCanonicalName(ModuleDeclarationInstantiation, "ModuleDeclarationInstantiation");
 

js/src/builtin/ModuleObject.cpp

@@ -629,6 +629,8 @@ ModuleEnvironmentObject*
 ModuleObject::environment() const
 {
     Value value = getReservedSlot(EnvironmentSlot);
+    MOZ_ASSERT(!value.isNull());
+
     if (value.isUndefined())
         return nullptr;
 

js/src/builtin/ModuleObject.h

@@ -10,11 +10,10 @@
 #include "jsapi.h"
 #include "jsatom.h"
 
+#include "builtin/SelfHostingDefines.h"
 #include "gc/Zone.h"
-
 #include "js/GCVector.h"
 #include "js/Id.h"
-
 #include "vm/NativeObject.h"
 #include "vm/ProxyObject.h"
 
@@ -224,6 +223,9 @@ class ModuleObject : public NativeObject
         SlotCount
     };
 
+    static_assert(EnvironmentSlot == MODULE_OBJECT_ENVIRONMENT_SLOT,
+                  "EnvironmentSlot must match self-hosting define");
+
     static const Class class_;
 
     static bool isInstance(HandleValue value);

js/src/builtin/SelfHostingDefines.h

@@ -99,4 +99,6 @@
 #define REGEXP_STICKY_FLAG      0x08
 #define REGEXP_UNICODE_FLAG     0x10
 
+#define MODULE_OBJECT_ENVIRONMENT_SLOT 3
+
 #endif

js/src/jit-test/tests/modules/bug-1284486.js

@@ -0,0 +1,23 @@
+// |jit-test| error: InternalError
+
+// This tests that attempting to perform ModuleDeclarationInstantation a second
+// time after a failure throws an error. Doing this would be a bug in the module
+// loader, which is expected to throw away modules if there is an error
+// instantiating them.
+//
+// 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).
+
+let moduleRepo = {};
+setModuleResolveHook(function(module, specifier) {
+    return moduleRepo[specifier];
+});
+try {
+    let b = moduleRepo['b'] = parseModule("export var b = 3; export var c = 4;");
+    let c = moduleRepo['c'] = parseModule("export * from 'a'; export * from 'b';");
+    c.declarationInstantiation();
+} catch (exc) {}
+let a = moduleRepo['a'] = parseModule("export var a = 1; export var b = 2;");
+let d = moduleRepo['d'] = parseModule("import { a } from 'c'; a;");
+d.declarationInstantiation();

js/src/js.msg

@@ -536,6 +536,7 @@ MSG_DEF(JSMSG_MISSING_IMPORT,            1, JSEXN_SYNTAXERR, "import '{0}' not f
 MSG_DEF(JSMSG_AMBIGUOUS_IMPORT,          1, JSEXN_SYNTAXERR, "ambiguous import '{0}'")
 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")
 
 // Promise
 MSG_DEF(JSMSG_CANNOT_RESOLVE_PROMISE_WITH_ITSELF,       0, JSEXN_TYPEERR, "A promise cannot be resolved with itself.")

js/src/vm/SelfHosting.cpp

@@ -369,6 +369,16 @@ intrinsic_ThrowSyntaxError(JSContext* cx, unsigned argc, Value* vp)
     return false;
 }
 
+static bool
+intrinsic_ThrowInternalError(JSContext* cx, unsigned argc, Value* vp)
+{
+    CallArgs args = CallArgsFromVp(argc, vp);
+    MOZ_ASSERT(args.length() >= 1);
+
+    ThrowErrorWithType(cx, JSEXN_INTERNALERR, args);
+    return false;
+}
+
 /**
  * Handles an assertion failure in self-hosted code just like an assertion
  * failure in C++ code. Information about the failure can be provided in args[0].
@@ -2115,23 +2125,6 @@ intrinsic_HostResolveImportedModule(JSContext* cx, unsigned argc, Value* vp)
     return true;
 }
 
-static bool
-intrinsic_GetModuleEnvironment(JSContext* cx, unsigned argc, Value* vp)
-{
-    CallArgs args = CallArgsFromVp(argc, vp);
-    MOZ_ASSERT(args.length() == 1);
-    RootedModuleObject module(cx, &args[0].toObject().as<ModuleObject>());
-    RootedModuleEnvironmentObject env(cx, module->environment());
-    args.rval().setUndefined();
-    if (!env) {
-        args.rval().setUndefined();
-        return true;
-    }
-
-    args.rval().setObject(*env);
-    return true;
-}
-
 static bool
 intrinsic_CreateModuleEnvironment(JSContext* cx, unsigned argc, Value* vp)
 {
@@ -2381,6 +2374,7 @@ static const JSFunctionSpec intrinsic_functions[] = {
     JS_FN("ThrowRangeError",         intrinsic_ThrowRangeError,         4,0),
     JS_FN("ThrowTypeError",          intrinsic_ThrowTypeError,          4,0),
     JS_FN("ThrowSyntaxError",        intrinsic_ThrowSyntaxError,        4,0),
+    JS_FN("ThrowInternalError",      intrinsic_ThrowInternalError,      4,0),
     JS_FN("AssertionFailed",         intrinsic_AssertionFailed,         1,0),
     JS_FN("DumpMessage",             intrinsic_DumpMessage,             1,0),
     JS_FN("OwnPropertyKeys",         intrinsic_OwnPropertyKeys,         1,0),
@@ -2628,7 +2622,7 @@ static const JSFunctionSpec intrinsic_functions[] = {
     JS_FN("CallModuleMethodIfWrapped",
           CallNonGenericSelfhostedMethod<Is<ModuleObject>>, 2, 0),
     JS_FN("HostResolveImportedModule", intrinsic_HostResolveImportedModule, 2, 0),
-    JS_FN("GetModuleEnvironment", intrinsic_GetModuleEnvironment, 1, 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),