Bug 1564178 - Don't create cross compartment wrappers for debugger wrapper objects r=jimb

This removes the code to create CCWs for all debugger wrapper objects and updates compartment checks to query the debugger weakmaps. Differential Revision: https://phabricator.services.mozilla.com/D40041 --HG-- extra : moz-landing-system : lando
2024-10-19 16:25:38 +00:00 · 2019-08-01 16:09:40 +00:00 · 2019-08-01 16:09:40 +00:00 · 28bd1e594a
commit 28bd1e594a
parent 78110ef406
7 changed files with 139 additions and 103 deletions
--- a/js/src/debugger/DebugAPI.h
+++ b/js/src/debugger/DebugAPI.h
@ -114,6 +114,11 @@ class DebugAPI {
  static void checkDebugScriptAfterMovingGC(DebugScript* ds);
 #endif

+#ifdef DEBUG
+  static bool edgeIsInDebuggerWeakmap(JSRuntime* rt, JSObject* src,
+                                      JS::GCCellPtr dst);
+#endif
+
  /*** Methods for querying script breakpoint state. **************************/

  // Query information about whether any debuggers are observing a script.
--- a/js/src/debugger/Debugger.cpp
+++ b/js/src/debugger/Debugger.cpp
@ -438,13 +438,20 @@ JS_STATIC_ASSERT(unsigned(DebuggerFrame::OWNER_SLOT) ==
 JS_STATIC_ASSERT(unsigned(DebuggerFrame::OWNER_SLOT) ==
                 unsigned(DebuggerEnvironment::OWNER_SLOT));

+#ifdef DEBUG
+/* static */
+bool Debugger::isChildJSObject(JSObject* obj) {
+  return obj->getClass() == &DebuggerFrame::class_ ||
+         obj->getClass() == &DebuggerScript::class_ ||
+         obj->getClass() == &DebuggerSource_class ||
+         obj->getClass() == &DebuggerObject::class_ ||
+         obj->getClass() == &DebuggerEnvironment::class_;
+}
+#endif
+
 /* static */
 Debugger* Debugger::fromChildJSObject(JSObject* obj) {
-  MOZ_ASSERT(obj->getClass() == &DebuggerFrame::class_ ||
-             obj->getClass() == &DebuggerScript::class_ ||
-             obj->getClass() == &DebuggerSource_class ||
-             obj->getClass() == &DebuggerObject::class_ ||
-             obj->getClass() == &DebuggerEnvironment::class_);
+  MOZ_ASSERT(isChildJSObject(obj));
  JSObject* dbgobj = &obj->as<NativeObject>()
                          .getReservedSlot(JSSLOT_DEBUGOBJECT_OWNER)
                          .toObject();
@ -1137,14 +1144,6 @@ bool Debugger::wrapEnvironment(JSContext* cx, Handle<Env*> env,
      return false;
    }

-    CrossCompartmentKey key(
-        CrossCompartmentKey::DebuggeeEnvironment(object, env));
-    if (!object->compartment()->putWrapper(cx, key, ObjectValue(*envobj))) {
-      NukeDebuggerWrapper(envobj);
-      environments.remove(env);
-      return false;
-    }
-
    result.set(envobj);
  }

@ -1234,16 +1233,6 @@ bool Debugger::wrapDebuggeeObject(JSContext* cx, HandleObject obj,
      return false;
    }

-    if (obj->compartment() != object->compartment()) {
-      CrossCompartmentKey key(CrossCompartmentKey::DebuggeeObject(object, obj));
-      if (!object->compartment()->putWrapper(cx, key, ObjectValue(*dobj))) {
-        NukeDebuggerWrapper(dobj);
-        objects.remove(obj);
-        ReportOutOfMemory(cx);
-        return false;
-      }
-    }
-
    result.set(dobj);
  }

@ -3489,6 +3478,80 @@ void DebugAPI::traceCrossCompartmentEdges(JSTracer* trc) {
  }
 }

+static inline DebuggerSourceReferent GetSourceReferent(JSObject* obj);
+
+#ifdef DEBUG
+
+static bool RuntimeHasDebugger(JSRuntime* rt, Debugger* dbg) {
+  for (Debugger* d : rt->debuggerList()) {
+    if (d == dbg) {
+      return true;
+    }
+  }
+  return false;
+}
+
+/* static */
+bool DebugAPI::edgeIsInDebuggerWeakmap(JSRuntime* rt, JSObject* src,
+                                       JS::GCCellPtr dst) {
+  if (!Debugger::isChildJSObject(src)) {
+    return false;
+  }
+
+  Debugger* dbg = Debugger::fromChildJSObject(src);
+  MOZ_ASSERT(RuntimeHasDebugger(rt, dbg));
+
+  if (src->is<DebuggerFrame>()) {
+    if (dst.is<JSScript>()) {
+      // The generatorFrames map is not keyed on the associated JSScript. Get
+      // the key from the source object and check everything matches.
+      DebuggerFrame* frame = &src->as<DebuggerFrame>();
+      AbstractGeneratorObject* genObj = &frame->unwrappedGenerator();
+      return frame->generatorScript() == &dst.as<JSScript>() &&
+             dbg->generatorFrames.hasEntry(genObj, src);
+    }
+    return dst.is<JSObject>() &&
+           dbg->generatorFrames.hasEntry(&dst.as<JSObject>(), src);
+  }
+  if (src->is<DebuggerObject>()) {
+    return dst.is<JSObject>() &&
+           dbg->objects.hasEntry(&dst.as<JSObject>(), src);
+  }
+  if (src->is<DebuggerEnvironment>()) {
+    return dst.is<JSObject>() &&
+           dbg->environments.hasEntry(&dst.as<JSObject>(), src);
+  }
+  if (src->is<DebuggerScript>()) {
+    return src->as<DebuggerScript>().getReferent().match(
+        [=](JSScript* script) {
+          return dst.is<JSScript>() && script == &dst.as<JSScript>() &&
+                 dbg->scripts.hasEntry(script, src);
+        },
+        [=](LazyScript* lazy) {
+          return dst.is<LazyScript>() && lazy == &dst.as<LazyScript>() &&
+                 dbg->lazyScripts.hasEntry(lazy, src);
+        },
+        [=](WasmInstanceObject* instance) {
+          return dst.is<JSObject>() && instance == &dst.as<JSObject>() &&
+                 dbg->wasmInstanceScripts.hasEntry(instance, src);
+        });
+  }
+  if (src->getClass() == &DebuggerSource_class) {
+    return GetSourceReferent(src).match(
+        [=](ScriptSourceObject* sso) {
+          return dst.is<JSObject>() && sso == &dst.as<JSObject>() &&
+                 dbg->sources.hasEntry(sso, src);
+        },
+        [=](WasmInstanceObject* instance) {
+          return dst.is<JSObject>() && instance == &dst.as<JSObject>() &&
+                 dbg->wasmInstanceSources.hasEntry(instance, src);
+        });
+  }
+  MOZ_ASSERT_UNREACHABLE("Unhandled cross-compartment edge");
+}
+
+#endif
+
 /*
 * This method has two tasks:
 *   1. Mark Debugger objects that are unreachable except for debugger hooks
@ -4697,8 +4760,6 @@ void Debugger::removeDebuggeeGlobal(FreeOp* fop, GlobalObject* global,
  }
 }

-static inline DebuggerSourceReferent GetSourceReferent(JSObject* obj);
-
 class MOZ_STACK_CLASS Debugger::QueryBase {
 protected:
  QueryBase(JSContext* cx, Debugger* dbg)
@ -6014,7 +6075,6 @@ DebuggerScript* Debugger::newDebuggerScript(
 template <typename Wrapper, typename ReferentVariant, typename Referent,
          typename Map>
 Wrapper* Debugger::wrapVariantReferent(JSContext* cx, Map& map,
-                                       Handle<CrossCompartmentKey> key,
                                       Handle<ReferentVariant> referent) {
  cx->check(object);

@ -6032,13 +6092,6 @@ Wrapper* Debugger::wrapVariantReferent(JSContext* cx, Map& map,
      NukeDebuggerWrapper(wrapper);
      return nullptr;
    }
-
-    if (!object->compartment()->putWrapper(cx, key, ObjectValue(*wrapper))) {
-      NukeDebuggerWrapper(wrapper);
-      map.remove(untaggedReferent);
-      ReportOutOfMemory(cx);
-      return nullptr;
-    }
  }

  return &p->value()->template as<Wrapper>();
@ -6064,37 +6117,27 @@ DebuggerScript* Debugger::wrapVariantReferent(
      Rooted<LazyScript*> lazyScript(cx, untaggedReferent->maybeLazyScript());
      Rooted<DebuggerScriptReferent> lazyScriptReferent(cx, lazyScript.get());

-      Rooted<CrossCompartmentKey> key(cx,
-                                      CrossCompartmentKey(object, lazyScript));
      obj = wrapVariantReferent<DebuggerScript, DebuggerScriptReferent,
                                LazyScript*, LazyScriptWeakMap>(
-          cx, lazyScripts, key, lazyScriptReferent);
+          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.
-      Rooted<CrossCompartmentKey> key(
-          cx, CrossCompartmentKey(object, untaggedReferent));
      obj =
          wrapVariantReferent<DebuggerScript, DebuggerScriptReferent, JSScript*,
-                              ScriptWeakMap>(cx, scripts, key, referent);
+                              ScriptWeakMap>(cx, scripts, referent);
    }
  } else if (referent.is<LazyScript*>()) {
-    Handle<LazyScript*> untaggedReferent = referent.template as<LazyScript*>();
-    Rooted<CrossCompartmentKey> key(
-        cx, CrossCompartmentKey(object, untaggedReferent));
    obj =
        wrapVariantReferent<DebuggerScript, DebuggerScriptReferent, LazyScript*,
-                            LazyScriptWeakMap>(cx, lazyScripts, key, referent);
+                            LazyScriptWeakMap>(cx, lazyScripts, referent);
  } else {
-    Handle<WasmInstanceObject*> untaggedReferent =
        referent.template as<WasmInstanceObject*>();
-    Rooted<CrossCompartmentKey> key(
-        cx, CrossCompartmentKey::DebuggeeWasmScript(object, untaggedReferent));
-    obj = wrapVariantReferent<DebuggerScript, DebuggerScriptReferent,
-                              WasmInstanceObject*, WasmInstanceWeakMap>(
-        cx, wasmInstanceScripts, key, referent);
+        obj = wrapVariantReferent<DebuggerScript, DebuggerScriptReferent,
+                                  WasmInstanceObject*, WasmInstanceWeakMap>(
+            cx, wasmInstanceScripts, referent);
  }
  MOZ_ASSERT_IF(obj, obj->getReferent() == referent);
  return obj;
@ -6408,21 +6451,13 @@ 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::DebuggeeSource(object, untaggedReferent));
    obj = wrapVariantReferent<NativeObject, DebuggerSourceReferent,
-                              ScriptSourceObject*, SourceWeakMap>(
-        cx, sources, key, referent);
+                              ScriptSourceObject*, SourceWeakMap>(cx, sources,
+                                                                  referent);
  } else {
-    Handle<WasmInstanceObject*> untaggedReferent =
-        referent.template as<WasmInstanceObject*>();
-    Rooted<CrossCompartmentKey> key(
-        cx, CrossCompartmentKey::DebuggeeSource(object, untaggedReferent));
    obj = wrapVariantReferent<NativeObject, DebuggerSourceReferent,
                              WasmInstanceObject*, WasmInstanceWeakMap>(
-        cx, wasmInstanceSources, key, referent);
+        cx, wasmInstanceSources, referent);
  }
  MOZ_ASSERT_IF(obj, GetSourceReferent(obj) == referent);
  return obj;
--- a/js/src/debugger/Debugger.h
+++ b/js/src/debugger/Debugger.h
@ -283,6 +283,9 @@ class DebuggerWeakMap
  using Base::lookupForAdd;
  using Base::remove;
  using Base::trace;
+#ifdef DEBUG
+  using Base::hasEntry;
+#endif

  class Enum : public Base::Enum {
   public:
@ -929,7 +932,6 @@ class Debugger : private mozilla::LinkedListElement<Debugger> {
  template <typename Wrapper, typename ReferentVariant, typename Referent,
            typename Map>
  Wrapper* wrapVariantReferent(JSContext* cx, Map& map,
-                               Handle<CrossCompartmentKey> key,
                               Handle<ReferentVariant> referent);
  DebuggerScript* wrapVariantReferent(JSContext* cx,
                                      Handle<DebuggerScriptReferent> referent);
@ -978,6 +980,10 @@ class Debugger : private mozilla::LinkedListElement<Debugger> {
  inline const js::GCPtrNativeObject& toJSObject() const;
  inline js::GCPtrNativeObject& toJSObjectRef();
  static inline Debugger* fromJSObject(const JSObject* obj);
+
+#ifdef DEBUG
+  static bool isChildJSObject(JSObject* obj);
+#endif
  static Debugger* fromChildJSObject(JSObject* obj);

  Zone* zone() const { return toJSObject()->zone(); }
--- a/js/src/debugger/Frame.cpp
+++ b/js/src/debugger/Frame.cpp
@ -252,6 +252,12 @@ js::AbstractGeneratorObject& js::DebuggerFrame::unwrappedGenerator() const {
  return generatorInfo()->unwrappedGenerator();
 }

+#ifdef DEBUG
+JSScript* js::DebuggerFrame::generatorScript() const {
+  return generatorInfo()->generatorScript();
+}
+#endif
+
 bool DebuggerFrame::setGenerator(JSContext* cx,
                                 Handle<AbstractGeneratorObject*> genObj) {
  cx->check(this);
@ -264,17 +270,14 @@ bool DebuggerFrame::setGenerator(JSContext* cx,
    return true;
  }

-  // There are four relations we must establish:
+  // There are three relations we must establish:
  //
  // 1) The DebuggerFrame must point to the AbstractGeneratorObject.
  //
  // 2) generatorFrames must map the AbstractGeneratorObject to the
  //    DebuggerFrame.
  //
-  // 3) The compartment's crossCompartmentWrappers map must map this Debugger
-  //    and the AbstractGeneratorObject to the DebuggerFrame.
-  //
-  // 4) The generator's script's observer count must be bumped.
+  // 3) The generator's script's observer count must be bumped.
  RootedScript script(cx, genObj->callee().nonLazyScript());
  auto* info = cx->new_<GeneratorInfo>(genObj, script);
  if (!info) {
@ -290,18 +293,6 @@ bool DebuggerFrame::setGenerator(JSContext* cx,
  auto generatorFramesGuard =
      MakeScopeExit([&] { owner()->generatorFrames.remove(genObj); });

-  Rooted<CrossCompartmentKey> generatorKey(
-      cx, CrossCompartmentKey::DebuggeeFrameGenerator(owner()->toJSObject(),
-                                                      genObj));
-  if (!compartment()->putWrapper(cx, generatorKey, ObjectValue(*this))) {
-    return false;
-  }
-  auto crossCompartmentKeysGuard = MakeScopeExit([&] {
-    WrapperMap::Ptr generatorPtr = compartment()->lookupWrapper(generatorKey);
-    MOZ_ASSERT(generatorPtr);
-    compartment()->removeWrapper(generatorPtr);
-  });
-
  {
    AutoRealm ar(cx, script);
    if (!DebugScript::incrementGeneratorObserverCount(cx, script)) {
@ -312,7 +303,6 @@ bool DebuggerFrame::setGenerator(JSContext* cx,
  InitReservedSlot(this, GENERATOR_INFO_SLOT, info,
                   MemoryUse::DebuggerFrameGeneratorInfo);

-  crossCompartmentKeysGuard.release();
  generatorFramesGuard.release();
  infoGuard.release();

@ -357,18 +347,10 @@ void DebuggerFrame::clearGenerator(
    return;
  }

-  // 3) The compartment's crossCompartmentWrappers map must map this Debugger
-  //    and the AbstractGeneratorObject to the DebuggerFrame.
-  //
-  GeneratorInfo* info = generatorInfo();
-  CrossCompartmentKey generatorKey(CrossCompartmentKey::DebuggeeFrameGenerator(
-      owner->object, &info->unwrappedGenerator()));
-  auto generatorPtr = compartment()->lookupWrapper(generatorKey);
-  MOZ_ASSERT(generatorPtr);
-  compartment()->removeWrapper(generatorPtr);

  // 2) generatorFrames must no longer map the AbstractGeneratorObject to the
  // DebuggerFrame.
+  GeneratorInfo* info = generatorInfo();
  if (maybeGeneratorFramesEnum) {
    maybeGeneratorFramesEnum->removeFront();
  } else {
--- a/js/src/debugger/Frame.h
+++ b/js/src/debugger/Frame.h
@ -192,6 +192,10 @@ class DebuggerFrame : public NativeObject {
  // Debugger.Frame's generator object.
  AbstractGeneratorObject& unwrappedGenerator() const;

+#ifdef DEBUG
+  JSScript* generatorScript() const;
+#endif
+
  /*
   * Associate the generator object genObj with this Debugger.Frame. This
   * association allows the Debugger.Frame to track the generator's execution
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@ -4219,7 +4219,11 @@ class CompartmentCheckTracer final : public JS::CallbackTracer {
  Compartment* compartment;
 };

-static bool InCrossCompartmentMap(JSObject* src, JS::GCCellPtr dst) {
+static bool InCrossCompartmentMap(JSRuntime* rt, JSObject* src,
+                                  JS::GCCellPtr dst) {
+  // Cross compartment edges are either in the cross compartment map or in a
+  // debugger weakmap.
+
  Compartment* srccomp = src->compartment();

  if (dst.is<JSObject>()) {
@ -4231,17 +4235,8 @@ static bool InCrossCompartmentMap(JSObject* src, JS::GCCellPtr dst) {
    }
  }

-  /*
-   * If the cross-compartment edge is caused by the debugger, then we don't
-   * know the right hashtable key, so we have to iterate.
-   */
-  for (Compartment::WrapperEnum e(srccomp); !e.empty(); e.popFront()) {
-    auto& key = e.front().mutableKey();
-    const auto& value = e.front().value();
-    if (key.applyToWrapped([dst](auto tp) { return *tp == dst.asCell(); }) &&
-        ToMarkable(value.unbarrieredGet()) == src) {
-      return true;
-    }
+  if (DebugAPI::edgeIsInDebuggerWeakmap(rt, src, dst)) {
+    return true;
  }

  return false;
@ -4251,9 +4246,10 @@ bool CompartmentCheckTracer::onChild(const JS::GCCellPtr& thing) {
  Compartment* comp =
      MapGCThingTyped(thing, [](auto t) { return t->maybeCompartment(); });
  if (comp && compartment) {
-    MOZ_ASSERT(comp == compartment ||
-               (srcKind == JS::TraceKind::Object &&
-                InCrossCompartmentMap(static_cast<JSObject*>(src), thing)));
+    MOZ_ASSERT(
+        comp == compartment ||
+        (srcKind == JS::TraceKind::Object &&
+         InCrossCompartmentMap(runtime(), static_cast<JSObject*>(src), thing)));
  } else {
    TenuredCell* tenured = TenuredCell::fromPointer(thing.asCell());
    Zone* thingZone = tenured->zoneFromAnyThread();
--- a/js/src/gc/WeakMap.h
+++ b/js/src/gc/WeakMap.h
@ -203,6 +203,14 @@ class WeakMap
                               std::forward<ValueInput>(value));
  }

+#ifdef DEBUG
+  template <typename KeyInput, typename ValueInput>
+  bool hasEntry(KeyInput&& key, ValueInput&& value) {
+    Ptr p = Base::lookup(std::forward<KeyInput>(key));
+    return p && p->value() == value;
+  }
+#endif
+
  void markEntry(GCMarker* marker, gc::Cell* markedCell,
                 gc::Cell* origKey) override;