Bug 1489477 - Stop modules from entraining the top-level JSScript r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 11 Oct 2018 18:33:57 +0100
changeset 499255 f8b19c4105d2e0e78a5a2ffd8843c93c12af5c79
parent 499254 f73e13de8e712a5188866e4331f0cc6000a568cd
child 499256 6a40850883ffce1683a1c29f81314acaad331b9b
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1489477
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1489477 - Stop modules from entraining the top-level JSScript r=sfink
js/src/builtin/ModuleObject.cpp
js/src/builtin/ModuleObject.h
js/src/jsapi.h
js/src/shell/ModuleLoader.js
js/src/shell/js.cpp
js/src/vm/Stack.cpp
--- a/js/src/builtin/ModuleObject.cpp
+++ b/js/src/builtin/ModuleObject.cpp
@@ -984,28 +984,32 @@ AssertModuleScopesMatch(ModuleObject* mo
 void
 ModuleObject::fixEnvironmentsAfterCompartmentMerge()
 {
     AssertModuleScopesMatch(this);
     initialEnvironment().fixEnclosingEnvironmentAfterCompartmentMerge(script()->global());
     AssertModuleScopesMatch(this);
 }
 
-bool
-ModuleObject::hasScript() const
+JSScript*
+ModuleObject::maybeScript() const
 {
-    // When modules are parsed via the Reflect.parse() API, the module object
-    // doesn't have a script.
-    return !getReservedSlot(ScriptSlot).isUndefined();
+    Value value = getReservedSlot(ScriptSlot);
+    if (value.isUndefined())
+        return nullptr;
+
+    return value.toGCThing()->as<JSScript>();
 }
 
 JSScript*
 ModuleObject::script() const
 {
-    return getReservedSlot(ScriptSlot).toGCThing()->as<JSScript>();
+    JSScript* ptr = maybeScript();
+    MOZ_RELEASE_ASSERT(ptr);
+    return ptr;
 }
 
 static inline void
 AssertValidModuleStatus(ModuleStatus status)
 {
     MOZ_ASSERT(status >= MODULE_STATUS_UNINSTANTIATED &&
                status <= MODULE_STATUS_EVALUATED_ERROR);
 }
@@ -1152,16 +1156,21 @@ ModuleObject::execute(JSContext* cx, Han
 #ifdef DEBUG
     MOZ_ASSERT(self->status() == MODULE_STATUS_EVALUATING);
     if (!AssertFrozen(cx, self)) {
         return false;
     }
 #endif
 
     RootedScript script(cx, self->script());
+
+    // The top-level script if a module is only ever executed once. Clear the
+    // reference to prevent us keeping this alive unnecessarily.
+    self->setReservedSlot(ScriptSlot, UndefinedValue());
+
     RootedModuleEnvironmentObject scope(cx, self->environment());
     if (!scope) {
         JS_ReportErrorASCII(cx, "Module declarations have not yet been instantiated");
         return false;
     }
 
     return Execute(cx, script, *scope, rval.address());
 }
--- a/js/src/builtin/ModuleObject.h
+++ b/js/src/builtin/ModuleObject.h
@@ -297,16 +297,17 @@ class ModuleObject : public NativeObject
                               HandleArrayObject indiretExportEntries,
                               HandleArrayObject starExportEntries);
     static bool Freeze(JSContext* cx, HandleModuleObject self);
 #ifdef DEBUG
     static bool AssertFrozen(JSContext* cx, HandleModuleObject self);
 #endif
     void fixEnvironmentsAfterCompartmentMerge();
 
+    JSScript* maybeScript() const;
     JSScript* script() const;
     Scope* enclosingScope() const;
     ModuleEnvironmentObject& initialEnvironment() const;
     ModuleEnvironmentObject* environment() const;
     ModuleNamespaceObject* namespace_();
     ModuleStatus status() const;
     bool hadEvaluationError() const;
     Value evaluationError() const;
@@ -343,17 +344,16 @@ class ModuleObject : public NativeObject
                                                   HandleObject exports);
 
   private:
     static const ClassOps classOps_;
 
     static void trace(JSTracer* trc, JSObject* obj);
     static void finalize(js::FreeOp* fop, JSObject* obj);
 
-    bool hasScript() const;
     bool hasImportBindings() const;
     FunctionDeclarationVector* functionDeclarations();
 };
 
 // Process a module's parse tree to collate the import and export data used when
 // creating a ModuleObject.
 class MOZ_STACK_CLASS ModuleBuilder
 {
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3115,16 +3115,19 @@ GetRequestedModules(JSContext* cx, JS::H
 
 extern JS_PUBLIC_API(JSString*)
 GetRequestedModuleSpecifier(JSContext* cx, JS::HandleValue requestedModuleObject);
 
 extern JS_PUBLIC_API(void)
 GetRequestedModuleSourcePos(JSContext* cx, JS::HandleValue requestedModuleObject,
                             uint32_t* lineNumber, uint32_t* columnNumber);
 
+/*
+ * Get the top-level script for a module which has not yet been executed.
+ */
 extern JS_PUBLIC_API(JSScript*)
 GetModuleScript(JS::HandleObject moduleRecord);
 
 } /* namespace JS */
 
 #if defined(JS_BUILD_BINAST)
 
 namespace JS {
--- a/js/src/shell/ModuleLoader.js
+++ b/js/src/shell/ModuleLoader.js
@@ -1,14 +1,15 @@
 /* -*- Mode: javascript; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-/* global getModuleLoadPath setModuleLoadHook setModuleResolveHook parseModule os */
+/* global getModuleLoadPath setModuleLoadHook setModuleResolveHook setModuleMetadataHook */
+/* global parseModule os */ 
 
 // A basic synchronous module loader for testing the shell.
 {
 // Save standard built-ins before scripts can modify them.
 const ArrayPrototypeJoin = Array.prototype.join;
 const MapPrototypeGet = Map.prototype.get;
 const MapPrototypeHas = Map.prototype.has;
 const MapPrototypeSet = Map.prototype.set;
@@ -168,18 +169,35 @@ const ReflectLoader = new class {
     importRoot(path) {
         return this.loadAndExecute(path);
     }
 
     ["import"](name, referrer) {
         let path = this.resolve(name, null);
         return this.loadAndExecute(path);
     }
+
+    populateImportMeta(module, metaObject) {
+        // For the shell, use the script's filename as the base URL.
+
+        let path;
+        if (ReflectApply(MapPrototypeHas, this.modulePaths, [module])) {
+            path = ReflectApply(MapPrototypeGet, this.modulePaths, [module]);
+        } else {
+            path = "(unknown)";
+        }
+        metaObject.url = path;
+    }
 };
 
 setModuleLoadHook((path) => ReflectLoader.importRoot(path));
 
 setModuleResolveHook((module, requestName) => {
     let path = ReflectLoader.resolve(requestName, module);
     return ReflectLoader.loadAndParse(path);
 });
 
+setModuleMetadataHook((module, metaObject) => {
+    ReflectLoader.populateImportMeta(module, metaObject);
+});
+
 }
+
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -187,16 +187,17 @@ enum JSShellExitCode {
     EXITCODE_TIMEOUT            = 6
 };
 
 // Define use of application-specific slots on the shell's global object.
 enum GlobalAppSlot
 {
     GlobalAppSlotModuleLoadHook,           // Shell-specific; load a module graph
     GlobalAppSlotModuleResolveHook,        // HostResolveImportedModule
+    GlobalAppSlotModuleMetadataHook,       // HostPopulateImportMeta
     GlobalAppSlotCount
 };
 static_assert(GlobalAppSlotCount <= JSCLASS_GLOBAL_APPLICATION_SLOTS,
               "Too many applications slots defined for shell global");
 
 /*
  * Note: This limit should match the stack limit set by the browser in
  *       js/xpconnect/src/XPCJSContext.cpp
@@ -3270,17 +3271,17 @@ DisassembleToSprinter(JSContext* cx, uns
             }
         }
     } else {
         for (unsigned i = 0; i < p.argc; i++) {
             RootedFunction fun(cx);
             RootedScript script(cx);
             RootedValue value(cx, p.argv[i]);
             if (value.isObject() && value.toObject().is<ModuleObject>()) {
-                script = value.toObject().as<ModuleObject>().script();
+                script = value.toObject().as<ModuleObject>().maybeScript();
             } else {
                 script = TestingFunctionArgumentToScript(cx, value, fun.address());
             }
             if (!script) {
                 return false;
             }
             if (!DisassembleScript(cx, script, fun, p.lines, p.recursive, p.sourceNotes, sprinter)) {
                 return false;
@@ -4745,33 +4746,55 @@ CallModuleResolveHook(JSContext* cx, Han
          JS_ReportErrorASCII(cx, "Module resolve hook did not return Module object");
          return nullptr;
     }
 
     return &result.toObject();
 }
 
 static bool
-ShellModuleMetadataHook(JSContext* cx, HandleObject module, HandleObject metaObject)
-{
-    // For the shell, just use the script's filename as the base URL.
-    RootedScript script(cx, module->as<ModuleObject>().script());
-    const char* filename = script->scriptSource()->filename();
-    MOZ_ASSERT(filename);
-
-    RootedString url(cx, NewStringCopyZ<CanGC>(cx, filename));
-    if (!url) {
-        return false;
-    }
-
-    if (!JS_DefineProperty(cx, metaObject, "url", url, JSPROP_ENUMERATE)) {
-        return false;
-    }
-
-    return true;
+SetModuleMetadataHook(JSContext* cx, unsigned argc, Value* vp)
+{
+    CallArgs args = CallArgsFromVp(argc, vp);
+    if (args.length() != 1) {
+        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_MORE_ARGS_NEEDED,
+                                  "setModuleMetadataHook", "0", "s");
+        return false;
+    }
+
+    if (!args[0].isObject() || !args[0].toObject().is<JSFunction>()) {
+        const char* typeName = InformalValueTypeName(args[0]);
+        JS_ReportErrorASCII(cx, "expected hook function, got %s", typeName);
+        return false;
+    }
+
+    Handle<GlobalObject*> global = cx->global();
+    global->setReservedSlot(GlobalAppSlotModuleMetadataHook, args[0]);
+
+    args.rval().setUndefined();
+    return true;
+}
+
+static bool
+CallModuleMetadataHook(JSContext* cx, HandleObject module, HandleObject metaObject)
+{
+    Handle<GlobalObject*> global = cx->global();
+    RootedValue hookValue(cx, global->getReservedSlot(GlobalAppSlotModuleMetadataHook));
+    if (hookValue.isUndefined()) {
+        JS_ReportErrorASCII(cx, "Module metadata hook not set");
+        return false;
+    }
+    MOZ_ASSERT(hookValue.toObject().is<JSFunction>());
+
+    JS::AutoValueArray<2> args(cx);
+    args[0].setObject(*module);
+    args[1].setObject(*metaObject);
+
+    RootedValue dummy(cx);
+    return JS_CallFunctionValue(cx, nullptr, hookValue, args, &dummy);
 }
 
 static bool
 GetModuleLoadPath(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     ShellContext* sc = GetShellContext(cx);
@@ -7253,17 +7276,21 @@ DumpScopeChain(JSContext* cx, unsigned a
     if (obj->is<JSFunction>()) {
         RootedFunction fun(cx, &obj->as<JSFunction>());
         if (!fun->isInterpreted()) {
             ReportUsageErrorASCII(cx, callee, "Argument must be an interpreted function");
             return false;
         }
         script = JSFunction::getOrCreateScript(cx, fun);
     } else {
-        script = obj->as<ModuleObject>().script();
+        script = obj->as<ModuleObject>().maybeScript();
+        if (!script) {
+            JS_ReportErrorASCII(cx, "module does not have an associated script");
+            return false;
+        }
     }
 
     script->bodyScope()->dump();
 
     args.rval().setUndefined();
     return true;
 }
 
@@ -7973,16 +8000,22 @@ static const JSFunctionSpecWithHelp shel
 "  module loader."),
 
     JS_FN_HELP("setModuleResolveHook", SetModuleResolveHook, 1, 0,
 "setModuleResolveHook(function(module, specifier) {})",
 "  Set the HostResolveImportedModule hook to |function|.\n"
 "  This hook is used to look up a previously loaded module object.  It should\n"
 "  be implemented by the module loader."),
 
+    JS_FN_HELP("setModuleMetadataHook", SetModuleMetadataHook, 1, 0,
+"setModuleMetadataHook(function(module) {})",
+"  Set the HostPopulateImportMeta hook to |function|.\n"
+"  This hook is used to create the metadata object returned by import.meta for\n"
+"  a module.  It should be implemented by the module loader."),
+
     JS_FN_HELP("getModuleLoadPath", GetModuleLoadPath, 0, 0,
 "getModuleLoadPath()",
 "  Return any --module-load-path argument passed to the shell.  Used by the\n"
 "  module loader.\n"),
 
 #if defined(JS_BUILD_BINAST)
 
 JS_FN_HELP("parseBin", BinParse, 1, 0,
@@ -9682,17 +9715,17 @@ ProcessArgs(JSContext* cx, OptionParser*
     } else {
         sc->moduleLoadPath = js::shell::GetCWD();
     }
 
     if (!sc->moduleLoadPath) {
         return false;
     }
 
-    if (!modulePaths.empty() && !InitModuleLoader(cx)) {
+    if (!InitModuleLoader(cx)) {
         return false;
     }
 
     while (!filePaths.empty() || !codeChunks.empty() || !modulePaths.empty() || !binASTPaths.empty()) {
         size_t fpArgno = filePaths.empty() ? SIZE_MAX : filePaths.argno();
         size_t ccArgno = codeChunks.empty() ? SIZE_MAX : codeChunks.argno();
         size_t mpArgno = modulePaths.empty() ? SIZE_MAX : modulePaths.argno();
         size_t baArgno = binASTPaths.empty() ? SIZE_MAX : binASTPaths.argno();
@@ -10684,17 +10717,17 @@ main(int argc, char** argv, char** envp)
         JS_SetGCParameter(cx, JSGC_DYNAMIC_MARK_SLICE, 1);
         JS_SetGCParameter(cx, JSGC_SLICE_TIME_BUDGET, 10);
     }
 #endif
 
     js::SetPreserveWrapperCallback(cx, DummyPreserveWrapperCallback);
 
     JS::SetModuleResolveHook(cx->runtime(), CallModuleResolveHook);
-    JS::SetModuleMetadataHook(cx->runtime(), ShellModuleMetadataHook);
+    JS::SetModuleMetadataHook(cx->runtime(), CallModuleMetadataHook);
 
     result = Shell(cx, &op, envp);
 
 #ifdef DEBUG
     if (OOM_printAllocationCount) {
         printf("OOM max count: %" PRIu64 "\n", js::oom::counter);
     }
 #endif
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -149,21 +149,23 @@ AssertScopeMatchesEnvironment(Scope* sco
                 env = &env->as<LexicalEnvironmentObject>().enclosingEnvironment();
                 MOZ_ASSERT(env->is<GlobalObject>());
                 break;
 
               case ScopeKind::NonSyntactic:
                 MOZ_CRASH("NonSyntactic should not have a syntactic environment");
                 break;
 
-              case ScopeKind::Module:
-                MOZ_ASSERT(env->as<ModuleEnvironmentObject>().module().script() ==
-                           si.scope()->as<ModuleScope>().script());
+              case ScopeKind::Module: {
+                  ModuleObject* module = &env->as<ModuleEnvironmentObject>().module();
+                  MOZ_ASSERT_IF(module->maybeScript(),
+                                module->script() == si.scope()->as<ModuleScope>().script());
                 env = &env->as<ModuleEnvironmentObject>().enclosingEnvironment();
                 break;
+              }
 
               case ScopeKind::WasmInstance:
                 env = &env->as<WasmInstanceEnvironmentObject>().enclosingEnvironment();
                 break;
 
               case ScopeKind::WasmFunction:
                 env = &env->as<WasmFunctionCallObject>().enclosingEnvironment();
                 break;