Bug 1546817: Refactor and document Debugger support in js::CrossCompartmentKey. r=sfink
authorJim Blandy <jimb@mozilla.com>
Thu, 09 May 2019 17:38:13 +0000
changeset 532078 b97c44f9234fbfb400c4e578bf0052de06ea97a4
parent 532077 258bcf1c182c88ee2e59f0720b50d729259df1ba
child 532079 0d0548740e1e4649c9cc5d97b5d837071c35f07a
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1546817
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 1546817: Refactor and document Debugger support in js::CrossCompartmentKey. r=sfink This replaces the various Debugger-related member classes in js::CrossCompartmentKey with a series of small structs that can be used directly as alternatives in CrossCompartmentKey::WrappedType. Thus, instead of having a two-level tag --- the Variant tag, and then for DebuggerAndObject case, a DebuggerObjectKind value --- the Variant tag can do all the work by itself. This also tightens up the types a bit: choosing the wrong DebuggerObjectKind would be a silent error, but using the wrong constructor might get you a type error (although unfortunately some of the types are not as specific as you might hope: NativeObject instead of WasmInstanceObject, for example). A new comment explains the rationale for giving Debugger API objects entries in the wrapper map. Differential Revision: https://phabricator.services.mozilla.com/D28781
js/src/vm/Compartment.h
js/src/vm/Debugger.cpp
--- a/js/src/vm/Compartment.h
+++ b/js/src/vm/Compartment.h
@@ -18,58 +18,127 @@
 #include "gc/Barrier.h"
 #include "gc/NurseryAwareHashMap.h"
 #include "js/UniquePtr.h"
 #include "vm/JSObject.h"
 #include "vm/JSScript.h"
 
 namespace js {
 
+// A key in a WrapperMap, a compartment's map from entities in other
+// compartments to the local values the compartment's own code must use to refer
+// to them.
+//
+// WrapperMaps have a complex key type because, in addition to mapping JSObjects
+// to their cross-compartment wrappers, they must also map non-atomized
+// JSStrings to their copies in the local compartment, and debuggee entities
+// (objects, scripts, etc.) to their representative objects in the Debugger API.
 class CrossCompartmentKey {
  public:
-  enum DebuggerObjectKind : uint8_t {
-    DebuggerSource,
-    DebuggerEnvironment,
-    DebuggerObject,
-    DebuggerWasmScript,
-    DebuggerWasmSource
+  // [SMDOC]: Cross-compartment wrapper map entries for Debugger API objects
+  //
+  // The Debugger API creates objects like Debugger.Object, Debugger.Script,
+  // Debugger.Environment, etc. to refer to things in the debuggee. Each
+  // Debugger gets at most one Debugger.Mumble for each referent:
+  // Debugger.Mumbles are unique per referent per Debugger.
+  //
+  // Since a Debugger and its debuggee must be in different compartments, a
+  // Debugger.Mumble's pointer to its referent is a cross-compartment edge, from
+  // the debugger's compartment into the debuggee compartment. Like any other
+  // sort of cross-compartment edge, the GC needs to be able to find all of
+  // these edges readily.
+  //
+  // Our solution is to treat Debugger.Mumble objects as wrappers stored in
+  // JSCompartment::crossCompartmentWrappers, where the GC already looks when it
+  // needs to find any other sort of cross-compartment edges. This also meshes
+  // nicely with existing sanity checks that trace the heap looking for
+  // cross-compartment edges and check that each one has an entry in the right
+  // wrapper map.
+  //
+  // That approach means that a given referent may have multiple entries in the
+  // wrapper map: its ordinary cross-compartment wrapper, and then any
+  // Debugger.Mumbles referring to it. If there are multiple Debuggers in a
+  // compartment, each needs its own Debugger.Mumble for the referent, and each
+  // of those needs its own entry in the WrapperMap. And some referents may have
+  // more than one type of Debugger.Mumble that can refer to them: for example,
+  // a WasmInstanceObject can be the referent of both a Debugger.Script and a
+  // Debugger.Source.
+  //
+  // Hence, to look up a Debugger.Mumble in the WrapperMap, we need a key that
+  // includes 1) the referent, 2) the Debugger to which the Mumble belongs, and
+  // 3) the specific type of Mumble we're looking for. Since mozilla::Variant
+  // distinguishes alternatives by type only, we include a distinct type in
+  // WrappedType for each sort of Debugger.Mumble.
+
+  // Common structure for all Debugger.Mumble keys.
+  template <typename Referent>
+  struct Debuggee {
+    Debuggee(NativeObject* debugger, Referent* referent)
+        : debugger(debugger), referent(referent) {}
+
+    bool operator==(const Debuggee& other) const {
+      return debugger == other.debugger && referent == other.referent;
+    }
+
+    bool operator!=(const Debuggee& other) const { return !(*this == other); }
+
+    NativeObject* debugger;
+    Referent* referent;
   };
-  using DebuggerAndObject =
-      mozilla::Tuple<NativeObject*, JSObject*, DebuggerObjectKind>;
-  using DebuggerAndScript = mozilla::Tuple<NativeObject*, JSScript*>;
-  using DebuggerAndLazyScript = mozilla::Tuple<NativeObject*, LazyScript*>;
+
+  // Key under which we find debugger's Debugger.Object referring to referent.
+  struct DebuggeeObject : Debuggee<JSObject> {
+    DebuggeeObject(NativeObject* debugger, JSObject* referent)
+        : Debuggee(debugger, referent) {}
+  };
+
+  // Keys under which we find Debugger.Scripts.
+  using DebuggeeJSScript = Debuggee<JSScript>;
+  using DebuggeeWasmScript = Debuggee<NativeObject>;  // WasmInstanceObject
+  using DebuggeeLazyScript = Debuggee<LazyScript>;
+
+  // Key under which we find debugger's Debugger.Environment referring to
+  // referent.
+  struct DebuggeeEnvironment : Debuggee<JSObject> {
+    DebuggeeEnvironment(NativeObject* debugger, JSObject* referent)
+        : Debuggee(debugger, referent) {}
+  };
+
+  // Key under which we find debugger's Debugger.Source referring to referent.
+  struct DebuggeeSource : Debuggee<NativeObject> {
+    DebuggeeSource(NativeObject* debugger, NativeObject* referent)
+        : Debuggee(debugger, referent) {}
+  };
+
   using WrappedType =
-      mozilla::Variant<JSObject*, JSString*, DebuggerAndScript,
-                       DebuggerAndLazyScript, DebuggerAndObject>;
+      mozilla::Variant<JSObject*, JSString*, DebuggeeObject, DebuggeeJSScript,
+                       DebuggeeWasmScript, DebuggeeLazyScript,
+                       DebuggeeEnvironment, DebuggeeSource>;
 
   explicit CrossCompartmentKey(JSObject* obj) : wrapped(obj) {
     MOZ_RELEASE_ASSERT(obj);
   }
   explicit CrossCompartmentKey(JSString* str) : wrapped(str) {
     MOZ_RELEASE_ASSERT(str);
   }
   explicit CrossCompartmentKey(const JS::Value& v)
       : wrapped(v.isString() ? WrappedType(v.toString())
                              : WrappedType(&v.toObject())) {}
-  explicit CrossCompartmentKey(NativeObject* debugger, JSObject* obj,
-                               DebuggerObjectKind kind)
-      : wrapped(DebuggerAndObject(debugger, obj, kind)) {
-    MOZ_RELEASE_ASSERT(debugger);
-    MOZ_RELEASE_ASSERT(obj);
-  }
-  explicit CrossCompartmentKey(NativeObject* debugger, JSScript* script)
-      : wrapped(DebuggerAndScript(debugger, script)) {
-    MOZ_RELEASE_ASSERT(debugger);
-    MOZ_RELEASE_ASSERT(script);
-  }
-  explicit CrossCompartmentKey(NativeObject* debugger, LazyScript* lazyScript)
-      : wrapped(DebuggerAndLazyScript(debugger, lazyScript)) {
-    MOZ_RELEASE_ASSERT(debugger);
-    MOZ_RELEASE_ASSERT(lazyScript);
-  }
+
+  // For most debuggee keys, we must let the caller choose the key type
+  // themselves. But for JSScript and LazyScript, there is only one key type
+  // that makes sense, so we provide an overloaded constructor.
+  explicit CrossCompartmentKey(DebuggeeObject&& key) : wrapped(key) {}
+  explicit CrossCompartmentKey(DebuggeeSource&& key) : wrapped(key) {}
+  explicit CrossCompartmentKey(DebuggeeEnvironment&& key) : wrapped(key) {}
+  explicit CrossCompartmentKey(DebuggeeWasmScript&& key) : wrapped(key) {}
+  explicit CrossCompartmentKey(NativeObject* debugger, JSScript* referent)
+      : wrapped(DebuggeeJSScript(debugger, referent)) {}
+  explicit CrossCompartmentKey(NativeObject* debugger, LazyScript* referent)
+      : wrapped(DebuggeeLazyScript(debugger, referent)) {}
 
   bool operator==(const CrossCompartmentKey& other) const {
     return wrapped == other.wrapped;
   }
   bool operator!=(const CrossCompartmentKey& other) const {
     return wrapped != other.wrapped;
   }
 
@@ -77,73 +146,64 @@ class CrossCompartmentKey {
   bool is() const {
     return wrapped.is<T>();
   }
   template <typename T>
   const T& as() const {
     return wrapped.as<T>();
   }
 
+ private:
+  template <typename F>
+  struct ApplyToWrappedMatcher {
+    F f_;
+    explicit ApplyToWrappedMatcher(F f) : f_(f) {}
+    auto operator()(JSObject*& obj) { return f_(&obj); }
+    auto operator()(JSString*& str) { return f_(&str); }
+    template <typename Referent>
+    auto operator()(Debuggee<Referent>& dbg) {
+      return f_(&dbg.referent);
+    }
+  };
+
+  template <typename F>
+  struct ApplyToDebuggerMatcher {
+    F f_;
+    explicit ApplyToDebuggerMatcher(F f) : f_(f) {}
+
+    using ReturnType = decltype(f_(static_cast<NativeObject**>(nullptr)));
+    ReturnType operator()(JSObject*& obj) { return ReturnType(); }
+    ReturnType operator()(JSString*& str) { return ReturnType(); }
+    template <typename Referent>
+    ReturnType operator()(Debuggee<Referent>& dbg) {
+      return f_(&dbg.debugger);
+    }
+  };
+
+ public:
   template <typename F>
   auto applyToWrapped(F f) {
-    struct WrappedMatcher {
-      F f_;
-      explicit WrappedMatcher(F f) : f_(f) {}
-      auto operator()(JSObject*& obj) { return f_(&obj); }
-      auto operator()(JSString*& str) { return f_(&str); }
-      auto operator()(DebuggerAndScript& tpl) {
-        return f_(&mozilla::Get<1>(tpl));
-      }
-      auto operator()(DebuggerAndLazyScript& tpl) {
-        return f_(&mozilla::Get<1>(tpl));
-      }
-      auto operator()(DebuggerAndObject& tpl) {
-        return f_(&mozilla::Get<1>(tpl));
-      }
-    } matcher(f);
-    return wrapped.match(matcher);
+    return wrapped.match(ApplyToWrappedMatcher<F>(f));
   }
 
   template <typename F>
   auto applyToDebugger(F f) {
-    using ReturnType = decltype(f(static_cast<NativeObject**>(nullptr)));
-    struct DebuggerMatcher {
-      F f_;
-      explicit DebuggerMatcher(F f) : f_(f) {}
-      ReturnType operator()(JSObject*& obj) { return ReturnType(); }
-      ReturnType operator()(JSString*& str) { return ReturnType(); }
-      ReturnType operator()(DebuggerAndScript& tpl) {
-        return f_(&mozilla::Get<0>(tpl));
-      }
-      ReturnType operator()(DebuggerAndLazyScript& tpl) {
-        return f_(&mozilla::Get<0>(tpl));
-      }
-      ReturnType operator()(DebuggerAndObject& tpl) {
-        return f_(&mozilla::Get<0>(tpl));
-      }
-    } matcher(f);
-    return wrapped.match(matcher);
+    return wrapped.match(ApplyToDebuggerMatcher<F>(f));
   }
 
-  bool isDebuggerKey() const {
-    struct DebuggerMatcher {
-      bool operator()(JSObject* const& obj) { return false; }
-      bool operator()(JSString* const& str) { return false; }
-      bool operator()(const DebuggerAndScript& tpl) {
-        return true;
-      }
-      bool operator()(const DebuggerAndLazyScript& tpl) {
-        return true;
-      }
-      bool operator()(const DebuggerAndObject& tpl) {
-        return true;
-      }
-    } matcher;
-    return wrapped.match(matcher);
-  }
+  struct IsDebuggerKeyMatcher {
+    bool operator()(JSObject* const& obj) { return false; }
+    bool operator()(JSString* const& str) { return false; }
+    template <typename Referent>
+    bool operator()(Debuggee<Referent> const& dbg) {
+      return true;
+    }
+  };
+
+  bool isDebuggerKey() const { return wrapped.match(IsDebuggerKeyMatcher()); }
 
   JS::Compartment* compartment() {
     return applyToWrapped([](auto tp) { return (*tp)->maybeCompartment(); });
   }
 
   JS::Zone* zone() {
     return applyToWrapped([](auto tp) { return (*tp)->zone(); });
   }
@@ -151,32 +211,23 @@ class CrossCompartmentKey {
   struct Hasher : public DefaultHasher<CrossCompartmentKey> {
     struct HashFunctor {
       HashNumber operator()(JSObject* obj) {
         return DefaultHasher<JSObject*>::hash(obj);
       }
       HashNumber operator()(JSString* str) {
         return DefaultHasher<JSString*>::hash(str);
       }
-      HashNumber operator()(const DebuggerAndScript& tpl) {
-        return DefaultHasher<NativeObject*>::hash(mozilla::Get<0>(tpl)) ^
-               DefaultHasher<JSScript*>::hash(mozilla::Get<1>(tpl));
-      }
-      HashNumber operator()(const DebuggerAndLazyScript& tpl) {
-        return DefaultHasher<NativeObject*>::hash(mozilla::Get<0>(tpl)) ^
-               DefaultHasher<LazyScript*>::hash(mozilla::Get<1>(tpl));
-      }
-      HashNumber operator()(const DebuggerAndObject& tpl) {
-        return DefaultHasher<NativeObject*>::hash(mozilla::Get<0>(tpl)) ^
-               DefaultHasher<JSObject*>::hash(mozilla::Get<1>(tpl)) ^
-               (mozilla::Get<2>(tpl) << 5);
+      template <typename Referent>
+      HashNumber operator()(const Debuggee<Referent>& dbg) {
+        return mozilla::HashGeneric(dbg.debugger, dbg.referent);
       }
     };
     static HashNumber hash(const CrossCompartmentKey& key) {
-      return key.wrapped.match(HashFunctor());
+      return key.wrapped.addTagToHash(key.wrapped.match(HashFunctor()));
     }
 
     static bool match(const CrossCompartmentKey& l,
                       const CrossCompartmentKey& k) {
       return l.wrapped == k.wrapped;
     }
   };
 
@@ -185,16 +236,18 @@ class CrossCompartmentKey {
     return self->applyToWrapped([](auto tp) { return (*tp)->isTenured(); });
   }
 
   void trace(JSTracer* trc);
   bool needsSweep();
 
  private:
   CrossCompartmentKey() = delete;
+  explicit CrossCompartmentKey(WrappedType&& wrapped)
+      : wrapped(std::move(wrapped)) {}
   WrappedType wrapped;
 };
 
 // The data structure for storing CCWs, which has a map per target compartment
 // so we can access them easily. Note string CCWs are stored separately from the
 // others because they have target compartment nullptr.
 class WrapperMap {
   static const size_t InitialInnerMapSize = 4;
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -1269,18 +1269,18 @@ bool Debugger::wrapEnvironment(JSContext
       return false;
     }
 
     if (!p.add(cx, environments, env, envobj)) {
       NukeDebuggerWrapper(envobj);
       return false;
     }
 
-    CrossCompartmentKey key(object, env,
-                            CrossCompartmentKey::DebuggerEnvironment);
+    CrossCompartmentKey key(
+        CrossCompartmentKey::DebuggeeEnvironment(object, env));
     if (!object->compartment()->putWrapper(cx, key, ObjectValue(*envobj))) {
       NukeDebuggerWrapper(envobj);
       environments.remove(env);
       return false;
     }
 
     result.set(envobj);
   }
@@ -1367,17 +1367,17 @@ bool Debugger::wrapDebuggeeObject(JSCont
     }
 
     if (!p.add(cx, objects, obj, dobj)) {
       NukeDebuggerWrapper(dobj);
       return false;
     }
 
     if (obj->compartment() != object->compartment()) {
-      CrossCompartmentKey key(object, obj, CrossCompartmentKey::DebuggerObject);
+      CrossCompartmentKey key(CrossCompartmentKey::DebuggeeObject(object, obj));
       if (!object->compartment()->putWrapper(cx, key, ObjectValue(*dobj))) {
         NukeDebuggerWrapper(dobj);
         objects.remove(obj);
         ReportOutOfMemory(cx);
         return false;
       }
     }
 
@@ -6077,19 +6077,17 @@ JSObject* Debugger::wrapVariantReferent(
         cx, CrossCompartmentKey(object, untaggedReferent));
     obj =
         wrapVariantReferent<DebuggerScriptReferent, LazyScript*,
                             LazyScriptWeakMap>(cx, lazyScripts, key, referent);
   } else {
     Handle<WasmInstanceObject*> untaggedReferent =
         referent.template as<WasmInstanceObject*>();
     Rooted<CrossCompartmentKey> key(
-        cx, CrossCompartmentKey(
-                object, untaggedReferent,
-                CrossCompartmentKey::DebuggerObjectKind::DebuggerWasmScript));
+        cx, CrossCompartmentKey::DebuggeeWasmScript(object, untaggedReferent));
     obj = wrapVariantReferent<DebuggerScriptReferent, WasmInstanceObject*,
                               WasmInstanceWeakMap>(cx, wasmInstanceScripts, key,
                                                    referent);
   }
   MOZ_ASSERT_IF(obj, GetScriptReferent(obj) == referent);
   return obj;
 }
 
@@ -8363,28 +8361,24 @@ NativeObject* Debugger::newDebuggerSourc
 
 JSObject* Debugger::wrapVariantReferent(
     JSContext* cx, Handle<DebuggerSourceReferent> referent) {
   JSObject* obj;
   if (referent.is<ScriptSourceObject*>()) {
     Handle<ScriptSourceObject*> untaggedReferent =
         referent.template as<ScriptSourceObject*>();
     Rooted<CrossCompartmentKey> key(
-        cx, CrossCompartmentKey(
-                object, untaggedReferent,
-                CrossCompartmentKey::DebuggerObjectKind::DebuggerSource));
+        cx, CrossCompartmentKey::DebuggeeSource(object, untaggedReferent));
     obj = wrapVariantReferent<DebuggerSourceReferent, ScriptSourceObject*,
                               SourceWeakMap>(cx, sources, key, referent);
   } else {
     Handle<WasmInstanceObject*> untaggedReferent =
         referent.template as<WasmInstanceObject*>();
     Rooted<CrossCompartmentKey> key(
-        cx, CrossCompartmentKey(
-                object, untaggedReferent,
-                CrossCompartmentKey::DebuggerObjectKind::DebuggerWasmSource));
+        cx, CrossCompartmentKey::DebuggeeSource(object, untaggedReferent));
     obj = wrapVariantReferent<DebuggerSourceReferent, WasmInstanceObject*,
                               WasmInstanceWeakMap>(cx, wasmInstanceSources, key,
                                                    referent);
   }
   MOZ_ASSERT_IF(obj, GetSourceReferent(obj) == referent);
   return obj;
 }