Bug 1518308 - Include unaliased module scope variables in debug environments, r=jonco.
authorBrian Hackett <bhackett1024@gmail.com>
Wed, 15 May 2019 07:14:28 -1000
changeset 533285 45fb04e542c521dcdbf361091b06f4de6979789f
parent 533284 4478ea184906f92ad82672ca5e91ae96a3a34107
child 533286 541a6a19a385608655f5a8ae6a1b3a11494021ba
push id11276
push userrgurzau@mozilla.com
push dateMon, 20 May 2019 13:11:24 +0000
treeherdermozilla-beta@847755a7c325 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1518308
milestone68.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 1518308 - Include unaliased module scope variables in debug environments, r=jonco.
js/src/builtin/ModuleObject.cpp
js/src/jit-test/tests/debug/Environment-module-01.js
js/src/vm/EnvironmentObject.cpp
js/src/vm/EnvironmentObject.h
--- a/js/src/builtin/ModuleObject.cpp
+++ b/js/src/builtin/ModuleObject.cpp
@@ -1038,18 +1038,21 @@ bool ModuleObject::execute(JSContext* cx
   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());
+  // reference at exit to prevent us keeping this alive unnecessarily. This is
+  // kept while executing so it is available to the debugger.
+  auto guardA = mozilla::MakeScopeExit([&] {
+      self->setReservedSlot(ScriptSlot, UndefinedValue());
+    });
 
   RootedModuleEnvironmentObject scope(cx, self->environment());
   if (!scope) {
     JS_ReportErrorASCII(cx,
                         "Module declarations have not yet been instantiated");
     return false;
   }
 
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Environment-module-01.js
@@ -0,0 +1,26 @@
+// Debug environments for module environments should include variables that are
+// are not closed over or exported.
+
+var g = newGlobal({newCompartment: true});
+var dbg = Debugger(g);
+dbg.onEnterFrame = function (frame) {
+  if (!frame.older) {
+    return;
+  }
+  const env = frame.older.environment;
+  assertEq(env.names().join(','), "foo,y,x,z");
+  assertEq(env.getVariable('x'), 0);
+  assertEq(env.getVariable('y'), 1);
+  assertEq(env.getVariable('z'), 2);
+  env.setVariable('x', 3);
+  assertEq(env.getVariable('x'), 3);
+};
+const m = g.parseModule(`
+  var x = 0;
+  export var y = 1;
+  const z = 2;
+  foo();
+  function foo() {}
+`);
+m.declarationInstantiation();
+m.evaluation();
--- a/js/src/vm/EnvironmentObject.cpp
+++ b/js/src/vm/EnvironmentObject.cpp
@@ -410,21 +410,21 @@ ModuleEnvironmentObject* ModuleEnvironme
   }
   MOZ_ASSERT(env->lastProperty()->getObjectFlags() & BaseShape::NOT_EXTENSIBLE);
   MOZ_ASSERT(!env->inDictionaryMode());
 #endif
 
   return env;
 }
 
-ModuleObject& ModuleEnvironmentObject::module() {
+ModuleObject& ModuleEnvironmentObject::module() const {
   return getReservedSlot(MODULE_SLOT).toObject().as<ModuleObject>();
 }
 
-IndirectBindingMap& ModuleEnvironmentObject::importBindings() {
+IndirectBindingMap& ModuleEnvironmentObject::importBindings() const {
   return module().importBindings();
 }
 
 bool ModuleEnvironmentObject::createImportBinding(JSContext* cx,
                                                   HandleAtom importName,
                                                   HandleModuleObject module,
                                                   HandleAtom localName) {
   RootedId importNameId(cx, AtomToId(importName));
@@ -1476,38 +1476,47 @@ class DebugEnvironmentProxyHandler : pub
                              Action action, MutableHandleValue vp,
                              AccessResult* accessResult) const {
     MOZ_ASSERT(&debugEnv->environment() == env);
     MOZ_ASSERT_IF(action == SET, !debugEnv->isOptimizedOut());
     *accessResult = ACCESS_GENERIC;
     LiveEnvironmentVal* maybeLiveEnv =
         DebugEnvironments::hasLiveEnvironment(*env);
 
-    if (env->is<ModuleEnvironmentObject>()) {
-      /* Everything is aliased and stored in the environment object. */
-      return true;
-    }
-
-    /* Handle unaliased formals, vars, lets, and consts at function scope. */
-    if (env->is<CallObject>()) {
-      CallObject& callobj = env->as<CallObject>();
-      RootedFunction fun(cx, &callobj.callee());
-      RootedScript script(cx, JSFunction::getOrCreateScript(cx, fun));
-      if (!script->ensureHasAnalyzedArgsUsage(cx)) {
-        return false;
+    // Handle unaliased formals, vars, lets, and consts at function or module
+    // scope.
+    if (env->is<CallObject>() || env->is<ModuleEnvironmentObject>()) {
+      RootedScript script(cx);
+      if (env->is<CallObject>()) {
+        CallObject& callobj = env->as<CallObject>();
+        RootedFunction fun(cx, &callobj.callee());
+        script = JSFunction::getOrCreateScript(cx, fun);
+        if (!script->ensureHasAnalyzedArgsUsage(cx)) {
+          return false;
+        }
+      } else {
+        script = env->as<ModuleEnvironmentObject>().module().maybeScript();
+        if (!script) {
+          *accessResult = ACCESS_LOST;
+          return true;
+        }
       }
 
       BindingIter bi(script);
       while (bi && NameToId(bi.name()->asPropertyName()) != id) {
         bi++;
       }
       if (!bi) {
         return true;
       }
 
+      if (bi.location().kind() == BindingLocation::Kind::Import) {
+        return true;
+      }
+
       if (!bi.hasArgumentSlot()) {
         if (bi.closedOver()) {
           return true;
         }
 
         uint32_t i = bi.location().slot();
         if (maybeLiveEnv) {
           AbstractFramePtr frame = maybeLiveEnv->frame();
@@ -1772,16 +1781,20 @@ class DebugEnvironmentProxyHandler : pub
     return env.is<LexicalEnvironmentObject>() &&
            !env.as<LexicalEnvironmentObject>().isExtensible();
   }
 
   static Scope* getEnvironmentScope(const JSObject& env) {
     if (isFunctionEnvironment(env)) {
       return env.as<CallObject>().callee().nonLazyScript()->bodyScope();
     }
+    if (env.is<ModuleEnvironmentObject>()) {
+      JSScript* script = env.as<ModuleEnvironmentObject>().module().maybeScript();
+      return script ? script->bodyScope() : nullptr;
+    }
     if (isNonExtensibleLexicalEnvironment(env)) {
       return &env.as<LexicalEnvironmentObject>().scope();
     }
     if (env.is<VarEnvironmentObject>()) {
       return &env.as<VarEnvironmentObject>().scope();
     }
     if (env.is<WasmInstanceEnvironmentObject>()) {
       return &env.as<WasmInstanceEnvironmentObject>().scope();
--- a/js/src/vm/EnvironmentObject.h
+++ b/js/src/vm/EnvironmentObject.h
@@ -403,18 +403,18 @@ class ModuleEnvironmentObject : public E
 
  public:
   static const Class class_;
 
   static const uint32_t RESERVED_SLOTS = 2;
 
   static ModuleEnvironmentObject* create(JSContext* cx,
                                          HandleModuleObject module);
-  ModuleObject& module();
-  IndirectBindingMap& importBindings();
+  ModuleObject& module() const;
+  IndirectBindingMap& importBindings() const;
 
   bool createImportBinding(JSContext* cx, HandleAtom importName,
                            HandleModuleObject module, HandleAtom exportName);
 
   bool hasImportBinding(HandlePropertyName name);
 
   bool lookupImport(jsid name, ModuleEnvironmentObject** envOut,
                     Shape** shapeOut);