Bug 1399866 - Add gray marking assertions when setting proxy target r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 20 Sep 2017 18:23:47 +0100
changeset 433881 fbdfaa4bae2d1efadd68f7cac2fdefc2e24dd79c
parent 433880 5e070a86daad50fa6ea1902f86bf9d29fad390bf
child 433882 a079a0a2971dc11a7b88670d48dff46b011dac04
push id1567
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 12:36:05 +0000
treeherdermozilla-release@e512c14a0406 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1399866
milestone57.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 1399866 - Add gray marking assertions when setting proxy target r=sfink
js/src/jscompartment.cpp
js/src/proxy/Proxy.cpp
js/src/vm/ProxyObject.cpp
js/src/vm/ProxyObject.h
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -708,17 +708,17 @@ JSCompartment::traceOutgoingCrossCompart
         if (e.front().key().is<JSObject*>()) {
             Value v = e.front().value().unbarrieredGet();
             ProxyObject* wrapper = &v.toObject().as<ProxyObject>();
 
             /*
              * We have a cross-compartment wrapper. Its private pointer may
              * point into the compartment being collected, so we should mark it.
              */
-            TraceEdge(trc, wrapper->slotOfPrivate(), "cross-compartment wrapper");
+            ProxyObject::traceEdgeToTarget(trc, wrapper);
         }
     }
 }
 
 /* static */ void
 JSCompartment::traceIncomingCrossCompartmentEdgesForZoneGC(JSTracer* trc)
 {
     gcstats::AutoPhase ap(trc->runtime()->gc.stats(), gcstats::PhaseKind::MARK_CCWS);
--- a/js/src/proxy/Proxy.cpp
+++ b/js/src/proxy/Proxy.cpp
@@ -673,16 +673,22 @@ static bool
 proxy_DeleteProperty(JSContext* cx, HandleObject obj, HandleId id, ObjectOpResult& result)
 {
     if (!Proxy::delete_(cx, obj, id, result))
         return false;
     return SuppressDeletedProperty(cx, obj, id); // XXX is this necessary?
 }
 
 /* static */ void
+ProxyObject::traceEdgeToTarget(JSTracer* trc, ProxyObject* obj)
+{
+    TraceCrossCompartmentEdge(trc, obj, obj->slotOfPrivate(), "proxy target");
+}
+
+/* static */ void
 ProxyObject::trace(JSTracer* trc, JSObject* obj)
 {
     ProxyObject* proxy = &obj->as<ProxyObject>();
 
     TraceEdge(trc, &proxy->shape_, "ProxyObject_shape");
 
 #ifdef DEBUG
     if (TlsContext.get()->isStrictProxyCheckingEnabled() && proxy->is<WrapperObject>()) {
@@ -697,17 +703,18 @@ ProxyObject::trace(JSTracer* trc, JSObje
             MOZ_ASSERT(p);
             MOZ_ASSERT(*p->value().unsafeGet() == ObjectValue(*proxy));
         }
     }
 #endif
 
     // Note: If you add new slots here, make sure to change
     // nuke() to cope.
-    TraceCrossCompartmentEdge(trc, obj, proxy->slotOfPrivate(), "private");
+
+    traceEdgeToTarget(trc, proxy);
 
     size_t nreserved = proxy->numReservedSlots();
     for (size_t i = 0; i < nreserved; i++) {
         /*
          * The GC can use the second reserved slot to link the cross compartment
          * wrappers into a linked list, in which case we don't want to trace it.
          */
         if (proxy->is<CrossCompartmentWrapperObject>() &&
--- a/js/src/vm/ProxyObject.cpp
+++ b/js/src/vm/ProxyObject.cpp
@@ -109,23 +109,31 @@ ProxyObject::allocKindForTenure() const
     MOZ_ASSERT(usingInlineValueArray());
     Value priv = const_cast<ProxyObject*>(this)->private_();
     return GetProxyGCObjectKind(getClass(), data.handler, priv);
 }
 
 void
 ProxyObject::setCrossCompartmentPrivate(const Value& priv)
 {
-    *slotOfPrivate() = priv;
+    setPrivate(priv);
 }
 
 void
 ProxyObject::setSameCompartmentPrivate(const Value& priv)
 {
     MOZ_ASSERT(IsObjectValueInCompartment(priv, compartment()));
+    setPrivate(priv);
+}
+
+inline void
+ProxyObject::setPrivate(const Value& priv)
+{
+    MOZ_ASSERT_IF(IsMarkedBlack(this) && priv.isGCThing(),
+                  !JS::GCThingIsMarkedGray(JS::GCCellPtr(priv)));
     *slotOfPrivate() = priv;
 }
 
 void
 ProxyObject::nuke()
 {
     // Select a dead proxy handler based on the properties of this wrapper.
     // Do this before clearing the target.
--- a/js/src/vm/ProxyObject.h
+++ b/js/src/vm/ProxyObject.h
@@ -57,20 +57,16 @@ class ProxyObject : public ShapedObject
 
     const Value& private_() {
         return GetProxyPrivate(this);
     }
 
     void setCrossCompartmentPrivate(const Value& priv);
     void setSameCompartmentPrivate(const Value& priv);
 
-    GCPtrValue* slotOfPrivate() {
-        return reinterpret_cast<GCPtrValue*>(&detail::GetProxyDataLayout(this)->values()->privateSlot);
-    }
-
     JSObject* target() const {
         return const_cast<ProxyObject*>(this)->private_().toObjectOrNull();
     }
 
     const BaseProxyHandler* handler() const {
         return GetProxyHandler(const_cast<ProxyObject*>(this));
     }
 
@@ -98,16 +94,22 @@ class ProxyObject : public ShapedObject
 
     gc::AllocKind allocKindForTenure() const;
 
   private:
     GCPtrValue* reservedSlotPtr(size_t n) {
         return reinterpret_cast<GCPtrValue*>(&detail::GetProxyDataLayout(this)->reservedSlots->slots[n]);
     }
 
+    GCPtrValue* slotOfPrivate() {
+        return reinterpret_cast<GCPtrValue*>(&detail::GetProxyDataLayout(this)->values()->privateSlot);
+    }
+
+    void setPrivate(const Value& priv);
+
     static bool isValidProxyClass(const Class* clasp) {
         // Since we can take classes from the outside, make sure that they
         // are "sane". They have to quack enough like proxies for us to belive
         // they should be treated as such.
 
         // Proxy classes are not allowed to have call or construct hooks directly. Their
         // callability is instead decided by handler()->isCallable().
         return clasp->isProxy() &&
@@ -117,16 +119,18 @@ class ProxyObject : public ShapedObject
 
   public:
     static unsigned grayLinkReservedSlot(JSObject* obj);
 
     void renew(const BaseProxyHandler* handler, const Value& priv);
 
     static void trace(JSTracer* trc, JSObject* obj);
 
+    static void traceEdgeToTarget(JSTracer* trc, ProxyObject* obj);
+
     void nuke();
 
     // There is no class_ member to force specialization of JSObject::is<T>().
     // The implementation in JSObject is incorrect for proxies since it doesn't
     // take account of the handler type.
     static const Class proxyClass;
 };