From 28981210df7d284ad10576f3bfe5efd0024067dd Mon Sep 17 00:00:00 2001
From: Yoshi Cheng-Hao Huang <allstars.chh@gmail.com>
Date: Fri, 14 Dec 2018 15:11:10 +0100
Subject: [PATCH] Bug 1515648 - Part 3: use unbarrieredGet() for Debugger.
 r=jonco

---
 js/src/gc/Zone.cpp        |  5 ++++-
 js/src/vm/Debugger.cpp    | 24 ++++++++++++++++++++++--
 js/src/vm/Realm.cpp       |  4 +++-
 js/src/vm/SavedStacks.cpp |  9 ++++++---
 4 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/js/src/gc/Zone.cpp b/js/src/gc/Zone.cpp
index d2878a1f8c6b..49b0687e99e6 100644
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -336,6 +336,9 @@ bool Zone::canCollect() {
 }
 
 void Zone::notifyObservingDebuggers() {
+  MOZ_ASSERT(JS::RuntimeHeapIsCollecting(),
+             "This method should be called during GC.");
+
   JSRuntime* rt = runtimeFromMainThread();
   JSContext* cx = rt->mainContextFromOwnThread();
 
@@ -352,7 +355,7 @@ void Zone::notifyObservingDebuggers() {
 
     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 "
diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp
index 669463947f37..9308e2475bf3 100644
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -3038,7 +3038,10 @@ void Debugger::updateObservesAsmJSOnDebuggees(IsObserving observing) {
     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;
       }
     }
@@ -3176,6 +3179,8 @@ void Debugger::traceCrossCompartmentEdges(JSTracer* trc) {
  * 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
@@ -3193,7 +3198,7 @@ void Debugger::traceCrossCompartmentEdges(JSTracer* trc) {
       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
@@ -4204,6 +4209,21 @@ static T* findDebuggerInVector(Debugger* dbg,
   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
diff --git a/js/src/vm/Realm.cpp b/js/src/vm/Realm.cpp
index afafd49f5d41..8aeff71ac441 100644
--- a/js/src/vm/Realm.cpp
+++ b/js/src/vm/Realm.cpp
@@ -796,7 +796,9 @@ void Realm::updateDebuggerObservesFlag(unsigned flag) {
           : 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
diff --git a/js/src/vm/SavedStacks.cpp b/js/src/vm/SavedStacks.cpp
index 629ed60cc2df..026a1b03361a 100644
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -1690,15 +1690,18 @@ void SavedStacks::chooseSamplingProbability(Realm* realm) {
   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);