Bug 1515648 - Part 3: use unbarrieredGet() for Debugger. r=jonco
authorYoshi Cheng-Hao Huang <allstars.chh@gmail.com>
Fri, 14 Dec 2018 15:11:10 +0100
changeset 509278 c4c07de1d4f49d8be2262c5485c062675dcb812e
parent 509277 8fe391c74c656692ab176faf453ede4f5e387fc2
child 509279 64e9482f70bc03bd2f1da643fc940d127c2d32db
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1515648
milestone66.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 1515648 - Part 3: use unbarrieredGet() for Debugger. r=jonco
js/src/gc/Zone.cpp
js/src/vm/Debugger.cpp
js/src/vm/Realm.cpp
js/src/vm/SavedStacks.cpp
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -331,33 +331,36 @@ bool Zone::canCollect() {
   }
 
   // Zones that will be or are currently used by other threads cannot be
   // collected.
   return !createdForHelperThread();
 }
 
 void Zone::notifyObservingDebuggers() {
+  MOZ_ASSERT(JS::RuntimeHeapIsCollecting(),
+             "This method should be called during GC.");
+
   JSRuntime* rt = runtimeFromMainThread();
   JSContext* cx = rt->mainContextFromOwnThread();
 
   for (RealmsInZoneIter realms(this); !realms.done(); realms.next()) {
     RootedGlobalObject global(cx, realms->unsafeUnbarrieredMaybeGlobal());
     if (!global) {
       continue;
     }
 
     GlobalObject::DebuggerVector* dbgs = global->getDebuggers();
     if (!dbgs) {
       continue;
     }
 
     for (GlobalObject::DebuggerVector::Range r = dbgs->all(); !r.empty();
          r.popFront()) {
-      if (!r.front()->debuggeeIsBeingCollected(rt->gc.majorGCCount())) {
+      if (!r.front().unbarrieredGet()->debuggeeIsBeingCollected(rt->gc.majorGCCount())) {
 #ifdef DEBUG
         fprintf(stderr,
                 "OOM while notifying observing Debuggers of a GC: The "
                 "onGarbageCollection\n"
                 "hook will not be fired for this GC for some Debuggers!\n");
 #endif
         return;
       }
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -3033,17 +3033,20 @@ void Debugger::updateObservesAsmJSOnDebu
   auto existingCallback = global.realm()->getAllocationMetadataBuilder();
   return existingCallback && existingCallback != &SavedStacks::metadataBuilder;
 }
 
 /* static */ bool Debugger::isObservedByDebuggerTrackingAllocations(
     const GlobalObject& debuggee) {
   if (auto* v = debuggee.getDebuggers()) {
     for (auto p = v->begin(); p != v->end(); p++) {
-      if ((*p)->trackingAllocationSites && (*p)->enabled) {
+      // Use unbarrieredGet() to prevent triggering read barrier while
+      // collecting, this is safe as long as dbg does not escape.
+      Debugger* dbg = p->unbarrieredGet();
+      if (dbg->trackingAllocationSites && dbg->enabled) {
         return true;
       }
     }
   }
 
   return false;
 }
 
@@ -3171,16 +3174,18 @@ void Debugger::traceCrossCompartmentEdge
  *      that may yet be called.
  *   2. Mark breakpoint handlers.
  *
  * This happens during the iterative part of the GC mark phase. This method
  * returns true if it has to mark anything; GC calls it repeatedly until it
  * returns false.
  */
 /* static */ bool Debugger::markIteratively(GCMarker* marker) {
+  MOZ_ASSERT(JS::RuntimeHeapIsCollecting(),
+             "This method should be called during GC.");
   bool markedAny = false;
 
   // Find all Debugger objects in danger of GC. This code is a little
   // convoluted since the easiest way to find them is via their debuggees.
   JSRuntime* rt = marker->runtime();
   for (RealmsIter r(rt); !r.done(); r.next()) {
     if (r->isDebuggee()) {
       GlobalObject* global = r->unsafeUnbarrieredMaybeGlobal();
@@ -3188,17 +3193,17 @@ void Debugger::traceCrossCompartmentEdge
         continue;
       }
 
       // Every debuggee has at least one debugger, so in this case
       // getDebuggers can't return nullptr.
       const GlobalObject::DebuggerVector* debuggers = global->getDebuggers();
       MOZ_ASSERT(debuggers);
       for (auto p = debuggers->begin(); p != debuggers->end(); p++) {
-        Debugger* dbg = *p;
+        Debugger* dbg = p->unbarrieredGet();
 
         // dbg is a Debugger with at least one debuggee. Check three things:
         //   - dbg is actually in a compartment that is being marked
         //   - it isn't already marked
         //   - it actually has hooks that might be called
         GCPtrNativeObject& dbgobj = dbg->toJSObjectRef();
         if (!dbgobj->zone()->isGCMarking()) {
           continue;
@@ -4199,16 +4204,31 @@ static T* findDebuggerInVector(Debugger*
     if (*p == dbg) {
       break;
     }
   }
   MOZ_ASSERT(p != vec->end());
   return p;
 }
 
+// a ReadBarriered version for findDebuggerInVector
+// TODO: Bug 1515934 - findDebuggerInVector<T> triggers read barriers.
+static ReadBarriered<Debugger*>*
+findDebuggerInVector(Debugger* dbg,
+                     Vector<ReadBarriered<Debugger*>, 0, js::SystemAllocPolicy>* vec) {
+  ReadBarriered<Debugger*>* p;
+  for (p = vec->begin(); p != vec->end(); p++) {
+    if (p->unbarrieredGet() == dbg) {
+      break;
+    }
+  }
+  MOZ_ASSERT(p != vec->end());
+  return p;
+}
+
 void Debugger::removeDebuggeeGlobal(FreeOp* fop, GlobalObject* global,
                                     WeakGlobalObjectSet::Enum* debugEnum) {
   // The caller might have found global by enumerating this->debuggees; if
   // so, use HashSet::Enum::removeFront rather than HashSet::remove below,
   // to avoid invalidating the live enumerator.
   MOZ_ASSERT(debuggees.has(global));
   MOZ_ASSERT(debuggeeZones.has(global->zone()));
   MOZ_ASSERT_IF(debugEnum, debugEnum->front().unbarrieredGet() == global);
--- a/js/src/vm/Realm.cpp
+++ b/js/src/vm/Realm.cpp
@@ -791,17 +791,19 @@ void Realm::updateDebuggerObservesFlag(u
              flag == DebuggerObservesCoverage || flag == DebuggerObservesAsmJS);
 
   GlobalObject* global =
       zone()->runtimeFromMainThread()->gc.isForegroundSweeping()
           ? unsafeUnbarrieredMaybeGlobal()
           : maybeGlobal();
   const GlobalObject::DebuggerVector* v = global->getDebuggers();
   for (auto p = v->begin(); p != v->end(); p++) {
-    Debugger* dbg = *p;
+    // Use unbarrieredGet() to prevent triggering read barrier while collecting,
+    // this is safe as long as dbg does not escape.
+    Debugger* dbg = p->unbarrieredGet();
     if (flag == DebuggerObservesAllExecution
             ? dbg->observesAllExecution()
             : flag == DebuggerObservesCoverage
                   ? dbg->observesCoverage()
                   : flag == DebuggerObservesAsmJS && dbg->observesAsmJS()) {
       debugModeBits_ |= flag;
       return;
     }
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -1685,25 +1685,28 @@ void SavedStacks::chooseSamplingProbabil
   if (!dbgs || dbgs->empty()) {
     return;
   }
 
   mozilla::DebugOnly<ReadBarriered<Debugger*>*> begin = dbgs->begin();
   mozilla::DebugOnly<bool> foundAnyDebuggers = false;
 
   double probability = 0;
-  for (auto dbgp = dbgs->begin(); dbgp < dbgs->end(); dbgp++) {
+  for (auto p = dbgs->begin(); p < dbgs->end(); p++) {
     // The set of debuggers had better not change while we're iterating,
     // such that the vector gets reallocated.
     MOZ_ASSERT(dbgs->begin() == begin);
+    // Use unbarrieredGet() to prevent triggering read barrier while collecting,
+    // this is safe as long as dbgp does not escape.
+    Debugger* dbgp = p->unbarrieredGet();
 
-    if ((*dbgp)->trackingAllocationSites && (*dbgp)->enabled) {
+    if (dbgp->trackingAllocationSites && dbgp->enabled) {
       foundAnyDebuggers = true;
       probability =
-          std::max((*dbgp)->allocationSamplingProbability, probability);
+          std::max(dbgp->allocationSamplingProbability, probability);
     }
   }
   MOZ_ASSERT(foundAnyDebuggers);
 
   if (!bernoulliSeeded) {
     mozilla::Array<uint64_t, 2> seed;
     GenerateXorShift128PlusSeed(seed);
     bernoulli.setRandomState(seed[0], seed[1]);