Bug 1529991 Part 1 - Ensure Debugger.Script identity for scripts that can't be relazified, r=tcampbell.
authorBrian Hackett <bhackett1024@gmail.com>
Thu, 29 Aug 2019 23:21:58 +0000
changeset 554532 6365885e2c436b5c8bc25871a1864773971527c1
parent 554531 6fe9875279652b5818d36fc0025af1e84f5e6ae1
child 554533 09f20d0f43cc671bbdcf40d8f256d90306d3f11b
push id2165
push userffxbld-merge
push dateMon, 14 Oct 2019 16:30:58 +0000
treeherdermozilla-release@0eae18af659f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1529991
milestone70.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 1529991 Part 1 - Ensure Debugger.Script identity for scripts that can't be relazified, r=tcampbell. Differential Revision: https://phabricator.services.mozilla.com/D38800
js/src/debugger/Debugger.cpp
js/src/jit-test/tests/debug/Debugger-findScripts-25.js
js/src/vm/JSFunction.cpp
js/src/vm/JSScript.h
--- a/js/src/debugger/Debugger.cpp
+++ b/js/src/debugger/Debugger.cpp
@@ -6128,42 +6128,63 @@ typename Map::WrapperType* Debugger::wra
   }
 
   return &p->value()->template as<typename Map::WrapperType>();
 }
 
 DebuggerScript* Debugger::wrapVariantReferent(
     JSContext* cx, Handle<DebuggerScriptReferent> referent) {
   DebuggerScript* obj;
+
+  // A single script can be, at different times, represented by a JSScript,
+  // LazyScript, or both a LazyScript and JSScript. In the latter case, the
+  // LazyScript will have a pointer to the JSScript, and the JSScript might have
+  // a pointer to the LazyScript.
+  //
+  // When the same Debugger wraps either the LazyScript or JSScript at any
+  // different time, we have to ensure that the wrapper is always the same,
+  // i.e. Debugger.Script identity is preserved. We have to pick either the
+  // JSScript or the LazyScript consistently as the canonical referent.
+  //
+  // We have to work together with the lazification code in order to ensure we
+  // can always pick a canonical referent for the script. If a LazyScript with
+  // no JSScript is wrapped by any Debugger, and a JSScript is created later,
+  // that JSScript must point to the LazyScript.
+  //
+  // The JSScript is canonical when there is a JSScript with no LazyScript
+  // pointer. Either the JSScript is not lazifiable, or there is (or was)
+  // a LazyScript but the JSScript is not relazifiable. In the latter case we
+  // know from above that no Debugger ever wrapped the LazyScript.
+  //
+  // The LazyScript is canonical in all other cases.
+
   if (referent.is<JSScript*>()) {
     Handle<JSScript*> untaggedReferent = referent.template as<JSScript*>();
     if (untaggedReferent->maybeLazyScript()) {
-      // If the JSScript has corresponding LazyScript, wrap the LazyScript
-      // instead.
-      //
-      // This is necessary for Debugger.Script identity.  If we use both
-      // JSScript and LazyScript for same single script, those 2 wrapped
-      // scripts become not identical, while the referent script is
-      // actually identical.
-      //
-      // If a script has corresponding LazyScript and JSScript, the
-      // lifetime of the LazyScript is always longer than the JSScript.
-      // So we can use the LazyScript as a proxy for the JSScript.
+      // This JSScript has a LazyScript, so the LazyScript is canonical.
       Rooted<LazyScript*> lazyScript(cx, untaggedReferent->maybeLazyScript());
-      Rooted<DebuggerScriptReferent> lazyScriptReferent(cx, lazyScript.get());
-
-      obj = wrapVariantReferent(cx, lazyScripts, lazyScriptReferent);
-      MOZ_ASSERT_IF(obj, obj->getReferent() == lazyScriptReferent);
-      return obj;
-    } else {
-      // If the JSScript doesn't have corresponding LazyScript, the script
-      // is not lazifiable, and we can safely use JSScript as referent.
-      obj = wrapVariantReferent(cx, scripts, referent);
-    }
+      return wrapLazyScript(cx, lazyScript);
+    }
+    // This JSScript doesn't have a LazyScript, so the JSScript is canonical.
+    obj = wrapVariantReferent(cx, scripts, referent);
   } else if (referent.is<LazyScript*>()) {
+    Handle<LazyScript*> untaggedReferent = referent.template as<LazyScript*>();
+    if (untaggedReferent->maybeScript()) {
+      RootedScript script(cx, untaggedReferent->maybeScript());
+      if (!script->maybeLazyScript()) {
+        // Even though we have a LazyScript, we found a JSScript which doesn't
+        // have a LazyScript (which also means that no Debugger has wrapped this
+        // LazyScript), and the JSScript is canonical.
+        MOZ_ASSERT(!untaggedReferent->isWrappedByDebugger());
+        return wrapScript(cx, script);
+      }
+    }
+    // If there is an associated JSScript, this LazyScript is reachable from it,
+    // so this LazyScript is canonical.
+    untaggedReferent->setWrappedByDebugger();
     obj = wrapVariantReferent(cx, lazyScripts, referent);
   } else {
     referent.template as<WasmInstanceObject*>();
     obj = wrapVariantReferent(cx, wasmInstanceScripts, referent);
   }
   MOZ_ASSERT_IF(obj, obj->getReferent() == referent);
   return obj;
 }
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Debugger-findScripts-25.js
@@ -0,0 +1,14 @@
+// In a debugger with multiple debuggees, findScripts finds scripts across all debuggees.
+var g1 = newGlobal({newCompartment: true});
+var g2 = newGlobal({newCompartment: true});
+var dbg = new Debugger();
+var g1w = dbg.addDebuggee(g1);
+var g2w = dbg.addDebuggee(g2);
+
+// Add eval() to make functions non-relazifiable.
+g1.eval('function f() { eval(""); }');
+g2.eval('function g() { eval(""); }');
+
+var scripts = dbg.findScripts();
+assertEq(scripts.indexOf(g1w.makeDebuggeeValue(g1.f).script) != -1, true);
+assertEq(scripts.indexOf(g2w.makeDebuggeeValue(g2.g).script) != -1, true);
--- a/js/src/vm/JSFunction.cpp
+++ b/js/src/vm/JSFunction.cpp
@@ -1559,16 +1559,17 @@ bool JSFunction::createScriptForLazilyIn
 
     if (script) {
       // This function is non-canonical function, and the canonical
       // function is already delazified.
       fun->setUnlazifiedScript(script);
       // Remember the lazy script on the compiled script, so it can be
       // stored on the function again in case of re-lazification.
       if (canRelazify) {
+        MOZ_RELEASE_ASSERT(script->maybeLazyScript() == lazy);
         script->setLazyScript(lazy);
       }
       return true;
     }
 
     if (fun != lazy->functionNonDelazifying()) {
       // This function is non-canonical function, and the canonical
       // function is lazy.
@@ -1659,16 +1660,25 @@ bool JSFunction::createScriptForLazilyIn
       // If an identical lazy script is encountered later a match can be
       // determined based on line and column number.
       MOZ_ASSERT(lazy->column() == script->column());
 
       // Remember the lazy script on the compiled script, so it can be
       // stored on the function again in case of re-lazification.
       // Only functions without inner functions are re-lazified.
       script->setLazyScript(lazy);
+    } else if (lazy->isWrappedByDebugger()) {
+      // Associate the LazyScript with the JSScript even though the latter
+      // can't be relazified. This is needed to ensure the Debugger can find
+      // the LazyScript when wrapping the JSScript. Mark the script as
+      // unrelazifiable so we don't get confused later.
+      //
+      // See Debugger::wrapVariantReferent.
+      script->setLazyScript(lazy);
+      script->setDoNotRelazify(true);
     }
 
     // XDR the newly delazified function.
     if (script->scriptSource()->hasEncoder()) {
       RootedScriptSourceObject sourceObject(cx, lazy->sourceObject());
       if (!script->scriptSource()->xdrEncodeFunction(cx, fun, sourceObject)) {
         return false;
       }
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -1563,16 +1563,19 @@ class BaseScript : public gc::TenuredCel
     NeedsArgsAnalysis = 1 << 24,
     NeedsArgsObj = 1 << 25,
 
     // Set if the debugger's onNewScript hook has not yet been called.
     HideScriptFromDebugger = 1 << 26,
 
     // Set if the script has opted into spew
     SpewEnabled = 1 << 27,
+
+    // Set for LazyScripts which have been wrapped by some Debugger.
+    WrappedByDebugger = 1 << 28,
   };
 
   uint8_t* jitCodeRaw() const { return jitCodeRaw_; }
 
   ScriptSourceObject* sourceObject() const { return sourceObject_; }
   ScriptSource* scriptSource() const { return sourceObject()->source(); }
   ScriptSource* maybeForwardedScriptSource() const;
 
@@ -3304,16 +3307,21 @@ class LazyScript : public BaseScript {
     if (hasFlag(ImmutableFlags::IsModule)) {
       return frontend::ParseGoal::Module;
     }
     return frontend::ParseGoal::Script;
   }
 
   bool isBinAST() const { return scriptSource()->hasBinASTSource(); }
 
+  bool isWrappedByDebugger() const {
+    return hasFlag(MutableFlags::WrappedByDebugger);
+  }
+  void setWrappedByDebugger() { setFlag(MutableFlags::WrappedByDebugger); }
+
   void setFieldInitializers(FieldInitializers fieldInitializers) {
     MOZ_ASSERT(lazyData_);
     lazyData_->fieldInitializers_ = fieldInitializers;
   }
 
   const FieldInitializers& getFieldInitializers() const {
     MOZ_ASSERT(lazyData_);
     return lazyData_->fieldInitializers_;