Bug 1254376 - Add a read barrier to the global's debugger vector r=jimb
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 18 Mar 2016 10:14:30 +0000
changeset 289335 a803bb1e41a2bbecc47ff6f2998d2920c01dc18f
parent 289334 162fce03a86b256d9c5da0f14c07782657b7df89
child 289336 3b4efd3e713cba744009493a8283e524275e8d03
push id30102
push userryanvm@gmail.com
push dateSat, 19 Mar 2016 15:23:17 +0000
treeherdermozilla-central@720fb3d55e28 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs1254376
milestone48.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 1254376 - Add a read barrier to the global's debugger vector r=jimb
js/src/gc/Barrier.cpp
js/src/gc/Barrier.h
js/src/jscompartment.cpp
js/src/vm/Debugger.cpp
js/src/vm/Debugger.h
js/src/vm/GlobalObject.h
js/src/vm/SavedStacks.cpp
--- a/js/src/gc/Barrier.cpp
+++ b/js/src/gc/Barrier.cpp
@@ -18,31 +18,16 @@
 #include "vm/ScopeObject.h"
 #include "vm/SharedArrayObject.h"
 #include "vm/Symbol.h"
 
 namespace js {
 
 #ifdef DEBUG
 
-template <typename T>
-void
-BarrieredBase<T>::assertTypeConstraints() const
-{
-    static_assert(mozilla::IsBaseOf<gc::Cell, typename mozilla::RemovePointer<T>::Type>::value ||
-                  mozilla::IsSame<JS::Value, T>::value ||
-                  mozilla::IsSame<jsid, T>::value ||
-                  mozilla::IsSame<TaggedProto, T>::value,
-                  "ensure only supported types are instantiated with barriers");
-}
-#define INSTANTIATE_ALL_VALID_TYPES(type) \
-    template void BarrieredBase<type>::assertTypeConstraints() const;
-FOR_EACH_GC_POINTER_TYPE(INSTANTIATE_ALL_VALID_TYPES)
-#undef INSTANTIATE_ALL_VALID_TYPES
-
 bool
 HeapSlot::preconditionForSet(NativeObject* owner, Kind kind, uint32_t slot)
 {
     return kind == Slot
          ? &owner->getSlotRef(slot) == this
          : &owner->getDenseElement(slot) == (const Value*)this;
 }
 
--- a/js/src/gc/Barrier.h
+++ b/js/src/gc/Barrier.h
@@ -168,16 +168,21 @@
  *      -> T::writeBarrierPost
  *  -> InternalBarrierMethods<Value>::postBarrier
  *      -> StoreBuffer::put
  *
  * These classes are designed to be used by the internals of the JS engine.
  * Barriers designed to be used externally are provided in js/RootingAPI.h.
  * These external barriers call into the same post-barrier implementations at
  * InternalBarrierMethods<T>::post via an indirect call to Heap(.+)Barrier.
+ *
+ * These clases are designed to be used to wrap GC thing pointers or values that
+ * act like them (i.e. JS::Value and jsid).  It is possible to use them for
+ * other types by supplying the necessary barrier implementations but this
+ * is not usually necessary and should be done with caution.
  */
 
 class JSAtom;
 struct JSCompartment;
 class JSFlatString;
 class JSLinearString;
 
 namespace JS {
@@ -317,40 +322,29 @@ template <typename T>
 class BarrieredBaseMixins {};
 
 // Base class of all barrier types.
 template <typename T>
 class BarrieredBase : public BarrieredBaseMixins<T>
 {
   protected:
     // BarrieredBase is not directly instantiable.
-    explicit BarrieredBase(T v) : value(v) {
-#ifdef DEBUG
-        assertTypeConstraints();
-#endif
-    }
+    explicit BarrieredBase(T v) : value(v) {}
 
     // Storage for all barrier classes. |value| must be a GC thing reference
     // type: either a direct pointer to a GC thing or a supported tagged
     // pointer that can reference GC things, such as JS::Value or jsid. Nested
     // barrier types are NOT supported. See assertTypeConstraints.
     T value;
 
   public:
     // Note: this is public because C++ cannot friend to a specific template instantiation.
     // Friending to the generic template leads to a number of unintended consequences, including
     // template resolution ambiguity and a circular dependency with Tracing.h.
     T* unsafeUnbarrieredForTracing() { return &value; }
-
-  private:
-#ifdef DEBUG
-    // Static type assertions about T must be moved out of line to avoid
-    // circular dependencies between Barrier classes and GC memory definitions.
-    void assertTypeConstraints() const;
-#endif
 };
 
 // Base class for barriered pointer types that intercept only writes.
 template <class T>
 class WriteBarrieredBase : public BarrieredBase<T>
 {
   protected:
     // WriteBarrieredBase is not directly instantiable.
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -1010,17 +1010,17 @@ JSCompartment::updateDebuggerObservesFla
     MOZ_ASSERT(flag == DebuggerObservesAllExecution ||
                flag == DebuggerObservesCoverage ||
                flag == DebuggerObservesAsmJS);
 
     GlobalObject* global = zone()->runtimeFromMainThread()->gc.isForegroundSweeping()
                            ? unsafeUnbarrieredMaybeGlobal()
                            : maybeGlobal();
     const GlobalObject::DebuggerVector* v = global->getDebuggers();
-    for (Debugger * const* p = v->begin(); p != v->end(); p++) {
+    for (auto p = v->begin(); p != v->end(); p++) {
         Debugger* dbg = *p;
         if (flag == DebuggerObservesAllExecution ? dbg->observesAllExecution() :
             flag == DebuggerObservesCoverage ? dbg->observesCoverage() :
             dbg->observesAsmJS())
         {
             debugModeBits |= flag;
             return;
         }
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -641,17 +641,17 @@ Debugger::getScriptFrameWithIter(JSConte
     vp.setObject(*p->value());
     return true;
 }
 
 /* static */ bool
 Debugger::hasLiveHook(GlobalObject* global, Hook which)
 {
     if (GlobalObject::DebuggerVector* debuggers = global->getDebuggers()) {
-        for (Debugger** p = debuggers->begin(); p != debuggers->end(); p++) {
+        for (auto p = debuggers->begin(); p != debuggers->end(); p++) {
             Debugger* dbg = *p;
             if (dbg->enabled && dbg->getHook(which))
                 return true;
         }
     }
     return false;
 }
 
@@ -1567,17 +1567,17 @@ Debugger::dispatchHook(JSContext* cx, Ho
      * calling into arbitrary JS.
      *
      * Note: In the general case, 'triggered' contains references to objects in
      * different compartments--every compartment *except* this one.
      */
     AutoValueVector triggered(cx);
     Handle<GlobalObject*> global = cx->global();
     if (GlobalObject::DebuggerVector* debuggers = global->getDebuggers()) {
-        for (Debugger** p = debuggers->begin(); p != debuggers->end(); p++) {
+        for (auto p = debuggers->begin(); p != debuggers->end(); p++) {
             Debugger* dbg = *p;
             if (dbg->enabled && hookIsEnabled(dbg)) {
                 if (!triggered.append(ObjectValue(*dbg->toJSObject())))
                     return JSTRAP_ERROR;
             }
         }
     }
 
@@ -1748,17 +1748,17 @@ Debugger::onSingleStep(JSContext* cx, Mu
      * The converse --- ensuring that we do receive traps when we should --- can
      * be done with unit tests.
      */
     {
         uint32_t stepperCount = 0;
         JSScript* trappingScript = iter.script();
         GlobalObject* global = cx->global();
         if (GlobalObject::DebuggerVector* debuggers = global->getDebuggers()) {
-            for (Debugger** p = debuggers->begin(); p != debuggers->end(); p++) {
+            for (auto p = debuggers->begin(); p != debuggers->end(); p++) {
                 Debugger* dbg = *p;
                 for (FrameMap::Range r = dbg->frames.all(); !r.empty(); r.popFront()) {
                     AbstractFramePtr frame = r.front().key();
                     NativeObject* frameobj = r.front().value();
                     if (frame.script() == trappingScript &&
                         !frameobj->getReservedSlot(JSSLOT_DEBUGFRAME_ONSTEP_HANDLER).isUndefined())
                     {
                         stepperCount++;
@@ -1885,37 +1885,32 @@ Debugger::slowPathOnNewGlobalObject(JSCo
     MOZ_ASSERT(!cx->isExceptionPending());
 }
 
 /* static */ bool
 Debugger::slowPathOnLogAllocationSite(JSContext* cx, HandleObject obj, HandleSavedFrame frame,
                                       double when, GlobalObject::DebuggerVector& dbgs)
 {
     MOZ_ASSERT(!dbgs.empty());
-    mozilla::DebugOnly<Debugger**> begin = dbgs.begin();
+    mozilla::DebugOnly<ReadBarriered<Debugger*>*> begin = dbgs.begin();
 
     // Root all the Debuggers while we're iterating over them;
     // appendAllocationSite calls JSCompartment::wrap, and thus can GC.
     //
     // SpiderMonkey protocol is generally for the caller to prove that it has
     // rooted the stuff it's asking you to operate on (i.e. by passing a
     // Handle), but in this case, we're iterating over a global's list of
     // Debuggers, and globals only hold their Debuggers weakly.
     Rooted<GCVector<JSObject*>> activeDebuggers(cx, GCVector<JSObject*>(cx));
-    for (Debugger** dbgp = dbgs.begin(); dbgp < dbgs.end(); dbgp++) {
-        // Since we're pulling these Debugger objects out of the GlobalObject's
-        // debugger array, which holds them only weakly, we need to let the
-        // incremental GC know that a possibly previously unreachable Debugger
-        // object just became reachable.
-        InternalBarrierMethods<JSObject*>::readBarrier((*dbgp)->object);
+    for (auto dbgp = dbgs.begin(); dbgp < dbgs.end(); dbgp++) {
         if (!activeDebuggers.append((*dbgp)->object))
             return false;
     }
 
-    for (Debugger** dbgp = dbgs.begin(); dbgp < dbgs.end(); dbgp++) {
+    for (auto dbgp = dbgs.begin(); dbgp < dbgs.end(); dbgp++) {
         // The set of debuggers had better not change while we're iterating,
         // such that the vector gets reallocated.
         MOZ_ASSERT(dbgs.begin() == begin);
 
         if ((*dbgp)->trackingAllocationSites &&
             (*dbgp)->enabled &&
             !(*dbgp)->appendAllocationSite(cx, obj, frame, when))
         {
@@ -2512,18 +2507,17 @@ Debugger::cannotTrackAllocations(const G
     auto existingCallback = global.compartment()->getObjectMetadataCallback();
     return existingCallback && existingCallback != SavedStacksMetadataCallback;
 }
 
 /* static */ bool
 Debugger::isObservedByDebuggerTrackingAllocations(const GlobalObject& debuggee)
 {
     if (auto* v = debuggee.getDebuggers()) {
-        Debugger** p;
-        for (p = v->begin(); p != v->end(); p++) {
+        for (auto p = v->begin(); p != v->end(); p++) {
             if ((*p)->trackingAllocationSites && (*p)->enabled) {
                 return true;
             }
         }
     }
 
     return false;
 }
@@ -2681,17 +2675,17 @@ Debugger::markAllIteratively(GCMarker* t
                 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 (Debugger * const* p = debuggers->begin(); p != debuggers->end(); p++) {
+            for (auto p = debuggers->begin(); p != debuggers->end(); p++) {
                 Debugger* dbg = *p;
 
                 /*
                  * 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
                  */
@@ -3546,17 +3540,17 @@ Debugger::addDebuggeeGlobal(JSContext* c
         }
 
         /*
          * Find all compartments containing debuggers debugging c's global
          * object. Add those compartments to visited.
          */
         if (c->isDebuggee()) {
             GlobalObject::DebuggerVector* v = c->maybeGlobal()->getDebuggers();
-            for (Debugger** p = v->begin(); p != v->end(); p++) {
+            for (auto p = v->begin(); p != v->end(); p++) {
                 JSCompartment* next = (*p)->object->compartment();
                 if (Find(visited, next) == visited.end() && !visited.append(next))
                     return false;
             }
         }
     }
 
     /*
@@ -3654,21 +3648,21 @@ Debugger::recomputeDebuggeeZoneSet()
     AutoEnterOOMUnsafeRegion oomUnsafe;
     debuggeeZones.clear();
     for (auto range = debuggees.all(); !range.empty(); range.popFront()) {
         if (!debuggeeZones.put(range.front().unbarrieredGet()->zone()))
             oomUnsafe.crash("Debugger::removeDebuggeeGlobal");
     }
 }
 
-template<typename V>
-static Debugger**
-findDebuggerInVector(Debugger* dbg, V* vec)
-{
-    Debugger** p;
+template <typename T>
+static T*
+findDebuggerInVector(Debugger* dbg, Vector<T, 0, js::SystemAllocPolicy>* vec)
+{
+    T* p;
     for (p = vec->begin(); p != vec->end(); p++) {
         if (*p == dbg)
             break;
     }
     MOZ_ASSERT(p != vec->end());
     return p;
 }
 
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -340,25 +340,31 @@ class Debugger : private mozilla::Linked
         void trace(JSTracer* trc) {
             if (frame)
                 TraceEdge(trc, &frame, "Debugger::AllocationsLogEntry::frame");
             if (ctorName)
                 TraceEdge(trc, &ctorName, "Debugger::AllocationsLogEntry::ctorName");
         }
     };
 
+    // Barrier methods so we can have ReadBarriered<Debugger*>.
+    static void readBarrier(Debugger* dbg) {
+        InternalBarrierMethods<JSObject*>::readBarrier(dbg->object);
+    }
+    static void writeBarrierPost(Debugger** vp, Debugger* prev, Debugger* next) {}
+
   private:
     HeapPtrNativeObject object;         /* The Debugger object. Strong reference. */
     WeakGlobalObjectSet debuggees;      /* Debuggee globals. Cross-compartment weak references. */
     JS::ZoneSet debuggeeZones; /* Set of zones that we have debuggees in. */
     js::HeapPtrObject uncaughtExceptionHook; /* Strong reference. */
     bool enabled;
     bool allowUnobservedAsmJS;
 
-    // Wether to enable code coverage on the Debuggee.
+    // Whether to enable code coverage on the Debuggee.
     bool collectCoverageInfo;
 
     JSCList breakpoints;                /* Circular list of all js::Breakpoints in this debugger */
 
     // The set of GC numbers for which one or more of this Debugger's observed
     // debuggees participated in.
     using GCNumberSet = HashSet<uint64_t, DefaultHasher<uint64_t>, RuntimeAllocPolicy>;
     GCNumberSet observedGCs;
--- a/js/src/vm/GlobalObject.h
+++ b/js/src/vm/GlobalObject.h
@@ -740,17 +740,17 @@ class GlobalObject : public NativeObject
     // Implemented in builtin/SIMD.cpp
     static bool initSimdObject(JSContext* cx, Handle<GlobalObject*> global);
     static bool initSimdType(JSContext* cx, Handle<GlobalObject*> global, SimdType simdType);
 
     static bool initStandardClasses(JSContext* cx, Handle<GlobalObject*> global);
     static bool initSelfHostingBuiltins(JSContext* cx, Handle<GlobalObject*> global,
                                         const JSFunctionSpec* builtins);
 
-    typedef js::Vector<js::Debugger*, 0, js::SystemAllocPolicy> DebuggerVector;
+    typedef js::Vector<js::ReadBarriered<js::Debugger*>, 0, js::SystemAllocPolicy> DebuggerVector;
 
     /*
      * The collection of Debugger objects debugging this global. If this global
      * is not a debuggee, this returns either nullptr or an empty vector.
      */
     DebuggerVector* getDebuggers() const;
 
     /*
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -1380,21 +1380,21 @@ SavedStacks::chooseSamplingProbability(J
     GlobalObject* global = compartment->maybeGlobal();
     if (!global)
         return;
 
     GlobalObject::DebuggerVector* dbgs = global->getDebuggers();
     if (!dbgs || dbgs->empty())
         return;
 
-    mozilla::DebugOnly<Debugger**> begin = dbgs->begin();
+    mozilla::DebugOnly<ReadBarriered<Debugger*>*> begin = dbgs->begin();
     mozilla::DebugOnly<bool> foundAnyDebuggers = false;
 
     double probability = 0;
-    for (Debugger** dbgp = dbgs->begin(); dbgp < dbgs->end(); dbgp++) {
+    for (auto dbgp = dbgs->begin(); dbgp < dbgs->end(); dbgp++) {
         // The set of debuggers had better not change while we're iterating,
         // such that the vector gets reallocated.
         MOZ_ASSERT(dbgs->begin() == begin);
 
         if ((*dbgp)->trackingAllocationSites && (*dbgp)->enabled) {
             foundAnyDebuggers = true;
             probability = std::max((*dbgp)->allocationSamplingProbability,
                                    probability);