Bug 1244956 - Simplify Wrapper rooting mechanism; r=sfink
authorTerrence Cole <terrence@mozilla.com>
Tue, 02 Feb 2016 09:32:16 -0800
changeset 329868 7acb1edc3f914e4184b3626fe615377750067a89
parent 329867 b7ecabf42762c4a152dd32cd53280d27e84faef2
child 329869 146d12e846150bd072ec432775ce0f419b506267
child 329878 fbc28495bd61801869be7e15b780da64edcc3d91
child 329927 d7089999c42c7e23026080ffef0f2f1cc39e1ecb
push id10618
push userdmitchell@mozilla.com
push dateTue, 09 Feb 2016 17:40:09 +0000
reviewerssfink
bugs1244956
milestone47.0a1
Bug 1244956 - Simplify Wrapper rooting mechanism; r=sfink
js/src/gc/GCRuntime.h
js/src/gc/RootMarking.cpp
js/src/jscompartment.h
js/src/jsgc.cpp
js/src/proxy/CrossCompartmentWrapper.cpp
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -16,16 +16,17 @@
 #include "gc/Nursery.h"
 #include "gc/Statistics.h"
 #include "gc/StoreBuffer.h"
 #include "gc/Tracer.h"
 
 namespace js {
 
 class AutoLockGC;
+class NonEscapingWrapperVector;
 class VerifyPreTracer;
 
 namespace gc {
 
 typedef Vector<JS::Zone*, 4, SystemAllocPolicy> ZoneVector;
 
 class MarkingValidator;
 class AutoTraceSession;
@@ -584,16 +585,18 @@ class GCRuntime
     inline void clearZealMode(ZealMode mode);
     inline bool upcomingZealousGC();
     inline bool needZealousGC();
 
     bool addRoot(Value* vp, const char* name);
     void removeRoot(Value* vp);
     void setMarkStackLimit(size_t limit, AutoLockGC& lock);
 
+    void registerStackWrapperVector(NonEscapingWrapperVector* vec);
+
     bool setParameter(JSGCParamKey key, uint32_t value, AutoLockGC& lock);
     uint32_t getParameter(JSGCParamKey key, const AutoLockGC& lock);
 
     bool triggerGC(JS::gcreason::Reason reason);
     void maybeAllocTriggerZoneGC(Zone* zone, const AutoLockGC& lock);
     bool triggerZoneGC(Zone* zone, JS::gcreason::Reason reason);
     bool maybeGC(Zone* zone);
     void maybePeriodicFullGC();
@@ -916,16 +919,17 @@ class GCRuntime
 
     void pushZealSelectedObjects();
     void purgeRuntime();
     bool beginMarkPhase(JS::gcreason::Reason reason);
     bool shouldPreserveJITCode(JSCompartment* comp, int64_t currentTime,
                                JS::gcreason::Reason reason);
     void bufferGrayRoots();
     void markCompartments();
+    void traceStackWrappers(JSTracer* trc);
     IncrementalProgress drainMarkStack(SliceBudget& sliceBudget, gcstats::Phase phase);
     template <class CompartmentIterT> void markWeakReferences(gcstats::Phase phase);
     void markWeakReferencesInCurrentGroup(gcstats::Phase phase);
     template <class ZoneIterT, class CompartmentIterT> void markGrayReferences(gcstats::Phase phase);
     void markBufferedGrayRoots(JS::Zone* zone);
     void markGrayReferencesInCurrentGroup(gcstats::Phase phase);
     void markAllWeakReferences(gcstats::Phase phase);
     void markAllGrayReferences(gcstats::Phase phase);
@@ -1017,16 +1021,19 @@ class GCRuntime
     ChunkPool             availableChunks_;
 
     // When all arenas in a chunk are used, it is moved to the fullChunks pool
     // so as to reduce the cost of operations on the available lists.
     ChunkPool             fullChunks_;
 
     RootedValueMap rootsHash;
 
+    // The list of currently live stack-stored wrappers.
+    mozilla::LinkedList<NonEscapingWrapperVector> stackWrappers;
+
     size_t maxMallocBytes;
 
     // An incrementing id used to assign unique ids to cells that require one.
     uint64_t nextCellUniqueId_;
 
     /*
      * Number of the committed arenas in all GC chunks including empty chunks.
      */
--- a/js/src/gc/RootMarking.cpp
+++ b/js/src/gc/RootMarking.cpp
@@ -175,39 +175,16 @@ AutoGCRooter::trace(JSTracer* trc)
         return;
       }
 
       case IONMASM: {
         static_cast<js::jit::MacroAssembler::AutoRooter*>(this)->masm()->trace(trc);
         return;
       }
 
-      case WRAPPER: {
-        /*
-         * We need to use TraceManuallyBarrieredEdge here because we mark
-         * wrapper roots in every slice. This is because of some rule-breaking
-         * in RemapAllWrappersForObject; see comment there.
-         */
-        TraceManuallyBarrieredEdge(trc, &static_cast<AutoWrapperRooter*>(this)->value.get(),
-                                   "JS::AutoWrapperRooter.value");
-        return;
-      }
-
-      case WRAPVECTOR: {
-        AutoWrapperVector::VectorImpl& vector = static_cast<AutoWrapperVector*>(this)->vector;
-        /*
-         * We need to use TraceManuallyBarrieredEdge here because we mark
-         * wrapper roots in every slice. This is because of some rule-breaking
-         * in RemapAllWrappersForObject; see comment there.
-         */
-        for (WrapperValue* p = vector.begin(); p < vector.end(); p++)
-            TraceManuallyBarrieredEdge(trc, &p->get(), "js::AutoWrapperVector.vector");
-        return;
-      }
-
       case CUSTOM:
         static_cast<JS::CustomAutoRooter*>(this)->trace(trc);
         return;
     }
 
     MOZ_ASSERT(tag_ >= 0);
     if (Value* vp = static_cast<AutoArrayRooter*>(this)->array)
         TraceRootRange(trc, tag_, vp, "JS::AutoArrayRooter.array");
@@ -215,24 +192,22 @@ AutoGCRooter::trace(JSTracer* trc)
 
 /* static */ void
 AutoGCRooter::traceAll(JSTracer* trc)
 {
     for (ContextIter cx(trc->runtime()); !cx.done(); cx.next())
         traceAllInContext(&*cx, trc);
 }
 
-/* static */ void
-AutoGCRooter::traceAllWrappers(JSTracer* trc)
+void
+GCRuntime::traceStackWrappers(JSTracer* trc)
 {
-    for (ContextIter cx(trc->runtime()); !cx.done(); cx.next()) {
-        for (AutoGCRooter* gcr = cx->roots.autoGCRooters_; gcr; gcr = gcr->down) {
-            if (gcr->tag_ == WRAPVECTOR || gcr->tag_ == WRAPPER)
-                gcr->trace(trc);
-        }
+    for (auto* vec : stackWrappers) {
+        for (auto& v : *vec)
+            TraceManuallyBarrieredEdge(trc, &v, "stack wrapper");
     }
 }
 
 void
 StackShape::trace(JSTracer* trc)
 {
     if (base)
         TraceRoot(trc, &base, "StackShape base");
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -857,88 +857,67 @@ class ErrorCopier
 
   public:
     explicit ErrorCopier(mozilla::Maybe<AutoCompartment>& ac)
       : ac(ac) {}
     ~ErrorCopier();
 };
 
 /*
- * AutoWrapperVector and AutoWrapperRooter can be used to store wrappers that
- * are obtained from the cross-compartment map. However, these classes should
- * not be used if the wrapper will escape. For example, it should not be stored
- * in the heap.
+ * NonEscapingWrapperVector can be used to safely store unbarriered wrappers
+ * obtained from the cross-compartment map that do not escape onto the heap.
+ *
+ * The reason that we need this class is complex, but straightforward enough
+ * with the right background. Specifically, we need to be able to observe
+ * wrappers in dead compartments without revivifying the compartment.
+ * For example, if we TransplantObject from a compartment that is becoming
+ * dead into a new or existing compartment (e.g. the user clicked the back
+ * button), we do not want to hold the old tab live (particularly if that
+ * navigation was caused by a misbehaving site).
  *
- * The AutoWrapper rooters are different from other autorooters because their
- * wrappers are marked on every GC slice rather than just the first one. If
- * there's some wrapper that we want to use temporarily without causing it to be
- * marked, we can use these AutoWrapper classes. If we get unlucky and a GC
- * slice runs during the code using the wrapper, the GC will mark the wrapper so
- * that it doesn't get swept out from under us. Otherwise, the wrapper needn't
- * be marked. This is useful in functions like JS_TransplantObject that
- * manipulate wrappers in compartments that may no longer be alive.
+ * What this means is that we must skip the barrier when observing the weakly
+ * held Value. The result of skipping the read barrier means that if we store
+ * the wrapper on the stack after marking our roots in the first slice and
+ * continue using it after sweeping, it will get swept out from under us.
+ * Additionally, it means that if we leak the wrapper onto the heap, it will
+ * get held live, but not be present in the wrapper map. In this case a future
+ * compartmental GC can sweep the thing while it is live on the heap, leading
+ * to a UAF, sec-crits, and crashes.
+ *
+ * The mechanism the NonEscapingWrapperVector uses to prevent the above badness
+ * from happening is to mark all such wrappers at every GC slice. This can keep
+ * a compartment live for longer than we'd like, but only in the (hopefully
+ * rare) case that a GC happens while we are manipulating the wrapper.
  */
 
-/*
- * This class stores the data for AutoWrapperVector and AutoWrapperRooter. It
- * should not be used in any other situations.
- */
-struct WrapperValue
+class MOZ_RAII NonEscapingWrapperVector
+  : public mozilla::LinkedListElement<NonEscapingWrapperVector>
 {
-    /*
-     * We use unsafeGet() in the constructors to avoid invoking a read barrier
-     * on the wrapper, which may be dead (see the comment about bug 803376 in
-     * jsgc.cpp regarding this). If there is an incremental GC while the wrapper
-     * is in use, the AutoWrapper rooter will ensure the wrapper gets marked.
-     */
-    explicit WrapperValue(const WrapperMap::Ptr& ptr)
-      : value(*ptr->value().unsafeGet())
-    {}
+    using Vec = js::GCVector<Value, 1, RuntimeAllocPolicy>;
+    Vec storage;
 
-    explicit WrapperValue(const WrapperMap::Enum& e)
-      : value(*e.front().value().unsafeGet())
-    {}
-
-    Value& get() { return value; }
-    Value get() const { return value; }
-    operator const Value&() const { return value; }
-    JSObject& toObject() const { return value.toObject(); }
-
-  private:
-    Value value;
-};
-
-class MOZ_RAII AutoWrapperVector : public JS::AutoVectorRooterBase<WrapperValue>
-{
   public:
-    explicit AutoWrapperVector(JSContext* cx
-                               MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
-        : AutoVectorRooterBase<WrapperValue>(cx, WRAPVECTOR)
-    {
-        MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    explicit NonEscapingWrapperVector(JSRuntime* rt) : storage(rt) {
+        rt->gc.registerStackWrapperVector(this);
     }
 
-    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
-};
+    bool reserve(size_t aRequest) { return storage.reserve(aRequest); }
 
-class MOZ_RAII AutoWrapperRooter : private JS::AutoGCRooter {
-  public:
-    AutoWrapperRooter(JSContext* cx, WrapperValue v
-                      MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
-      : JS::AutoGCRooter(cx, WRAPPER), value(v)
-    {
-        MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    Value& operator[](size_t aIndex) { return storage[aIndex]; }
+    Value* begin() { return storage.begin(); }
+    Value* end() { return storage.end(); }
+
+    // Avoid the barrier, for the reason described above. See bug 803376 and
+    // the comment about said bug in jsgc.cpp.
+    void infallibleAppend(const WrapperMap::Enum& e) {
+        storage.infallibleAppend(*e.front().value().unsafeGet());
     }
-
-    operator JSObject*() const {
-        return value.get().toObjectOrNull();
+    void infallibleAppend(const WrapperMap::Ptr& p) {
+        storage.infallibleAppend(*p->value().unsafeGet());
     }
-
-    friend void JS::AutoGCRooter::trace(JSTracer* trc);
-
-  private:
-    WrapperValue value;
-    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+    bool append(const WrapperMap::Enum& e) {
+        return storage.append(*e.front().value().unsafeGet());
+    }
 };
 
 } /* namespace js */
 
 #endif /* jscompartment_h */
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -1782,16 +1782,22 @@ GCRuntime::addRoot(Value* vp, const char
 
 void
 GCRuntime::removeRoot(Value* vp)
 {
     rootsHash.remove(vp);
     poke();
 }
 
+void
+GCRuntime::registerStackWrapperVector(NonEscapingWrapperVector* vec)
+{
+    stackWrappers.insertBack(vec);
+}
+
 extern JS_FRIEND_API(bool)
 js::AddRawValueRoot(JSContext* cx, Value* vp, const char* name)
 {
     MOZ_ASSERT(vp);
     MOZ_ASSERT(name);
     bool ok = cx->runtime()->gc.addRoot(vp, name);
     if (!ok)
         JS_ReportOutOfMemory(cx);
@@ -6101,17 +6107,17 @@ GCRuntime::incrementalCollectSlice(Slice
         incrementalState = MARK;
 
         if (isIncremental && useZeal && hasZealMode(ZealMode::IncrementalRootsThenFinish))
             break;
 
         MOZ_FALLTHROUGH;
 
       case MARK:
-        AutoGCRooter::traceAllWrappers(&marker);
+        traceStackWrappers(&marker);
 
         /* If we needed delayed marking for gray roots, then collect until done. */
         if (!hasBufferedGrayRoots()) {
             budget.makeUnlimited();
             isIncremental = false;
         }
 
         if (drainMarkStack(budget, gcstats::PHASE_MARK) == NotFinished)
--- a/js/src/proxy/CrossCompartmentWrapper.cpp
+++ b/js/src/proxy/CrossCompartmentWrapper.cpp
@@ -465,29 +465,30 @@ js::NukeCrossCompartmentWrappers(JSConte
         // Iterate the wrappers looking for anything interesting.
         for (JSCompartment::WrapperEnum e(c); !e.empty(); e.popFront()) {
             // Some cross-compartment wrappers are for strings.  We're not
             // interested in those.
             const CrossCompartmentKey& k = e.front().key();
             if (k.kind != CrossCompartmentKey::ObjectWrapper)
                 continue;
 
-            AutoWrapperRooter wobj(cx, WrapperValue(e));
-            JSObject* wrapped = UncheckedUnwrap(wobj);
+            NonEscapingWrapperVector wobj(cx->runtime());
+            MOZ_ALWAYS_TRUE(wobj.append(e));
+            JSObject* wrapped = UncheckedUnwrap(&wobj[0].toObject());
 
             if (nukeReferencesToWindow == DontNukeWindowReferences &&
                 IsWindowProxy(wrapped))
             {
                 continue;
             }
 
             if (targetFilter.match(wrapped->compartment())) {
                 // We found a wrapper to nuke.
                 e.removeFront();
-                NukeCrossCompartmentWrapper(cx, wobj);
+                NukeCrossCompartmentWrapper(cx, &wobj[0].toObject());
             }
         }
     }
 
     return true;
 }
 
 // Given a cross-compartment wrapper |wobj|, update it to point to
@@ -558,40 +559,40 @@ js::RemapWrapper(JSContext* cx, JSObject
 // |newTarget|. All wrappers are recomputed.
 JS_FRIEND_API(bool)
 js::RemapAllWrappersForObject(JSContext* cx, JSObject* oldTargetArg,
                               JSObject* newTargetArg)
 {
     RootedValue origv(cx, ObjectValue(*oldTargetArg));
     RootedObject newTarget(cx, newTargetArg);
 
-    AutoWrapperVector toTransplant(cx);
+    NonEscapingWrapperVector toTransplant(cx->runtime());
     if (!toTransplant.reserve(cx->runtime()->numCompartments))
         return false;
 
     for (CompartmentsIter c(cx->runtime(), SkipAtoms); !c.done(); c.next()) {
         if (WrapperMap::Ptr wp = c->lookupWrapper(origv)) {
             // We found a wrapper. Remember and root it.
-            toTransplant.infallibleAppend(WrapperValue(wp));
+            toTransplant.infallibleAppend(wp);
         }
     }
 
-    for (const WrapperValue& v : toTransplant) {
+    for (const Value& v : toTransplant) {
         if (!RemapWrapper(cx, &v.toObject(), newTarget))
             MOZ_CRASH();
     }
 
     return true;
 }
 
 JS_FRIEND_API(bool)
 js::RecomputeWrappers(JSContext* cx, const CompartmentFilter& sourceFilter,
                       const CompartmentFilter& targetFilter)
 {
-    AutoWrapperVector toRecompute(cx);
+    NonEscapingWrapperVector toRecompute(cx->runtime());
 
     for (CompartmentsIter c(cx->runtime(), SkipAtoms); !c.done(); c.next()) {
         // Filter by source compartment.
         if (!sourceFilter.match(c))
             continue;
 
         // Iterate over the wrappers, filtering appropriately.
         for (JSCompartment::WrapperEnum e(c); !e.empty(); e.popFront()) {
@@ -600,23 +601,23 @@ js::RecomputeWrappers(JSContext* cx, con
             if (k.kind != CrossCompartmentKey::ObjectWrapper)
                 continue;
 
             // Filter by target compartment.
             if (!targetFilter.match(static_cast<JSObject*>(k.wrapped)->compartment()))
                 continue;
 
             // Add it to the list.
-            if (!toRecompute.append(WrapperValue(e)))
+            if (!toRecompute.append(e))
                 return false;
         }
     }
 
     // Recompute all the wrappers in the list.
-    for (const WrapperValue& v : toRecompute) {
+    for (const Value& v : toRecompute) {
         JSObject* wrapper = &v.toObject();
         JSObject* wrapped = Wrapper::wrappedObject(wrapper);
         if (!RemapWrapper(cx, wrapper, wrapped))
             MOZ_CRASH();
     }
 
     return true;
 }