Bug 1343261 - dead object proxies must be swept with their former targets, r=jonco

MozReview-Commit-ID: KM6gNtGWvws

--HG--
extra : rebase_source : 51fc138bce7759773d8858e0285023590b5b903a

js/src/gc/GCRuntime.h

@@ -979,7 +979,7 @@ class GCRuntime
 
     void beginSweepPhase(bool lastGC, AutoLockForExclusiveAccess& lock);
     void findZoneGroups(AutoLockForExclusiveAccess& lock);
-    MOZ_MUST_USE bool findZoneEdgesForWeakMaps();
+    MOZ_MUST_USE bool findInterZoneEdges();
     void getNextZoneGroup();
     void endMarkingZoneGroup();
     void beginSweepingZoneGroup(AutoLockForExclusiveAccess& lock);

js/src/gc/Zone.cpp

@@ -38,6 +38,7 @@ JS::Zone::Zone(JSRuntime* rt, ZoneGroup* group)
     weakCaches_(group),
     gcWeakKeys_(group, SystemAllocPolicy(), rt->randomHashCodeScrambler()),
     gcZoneGroupEdges_(group),
+    hasDeadProxies_(group),
     typeDescrObjects_(group, this, SystemAllocPolicy()),
     markedAtoms_(group),
     usage(&rt->gc.usage),

js/src/gc/Zone.h

@@ -371,9 +371,17 @@ struct Zone : public JS::shadow::Zone,
     // This is used during GC while calculating zone groups to record edges that
     // can't be determined by examining this zone by itself.
     js::ZoneGroupData<ZoneSet> gcZoneGroupEdges_;
+
+    // Zones with dead proxies require an extra scan through the wrapper map,
+    // so track whether any dead proxies are known to exist.
+    js::ZoneGroupData<bool> hasDeadProxies_;
+
   public:
     ZoneSet& gcZoneGroupEdges() { return gcZoneGroupEdges_.ref(); }
 
+    bool hasDeadProxies() { return hasDeadProxies_; }
+    void setHasDeadProxies(bool b) { hasDeadProxies_ = b; }
+
     // Keep track of all TypeDescr and related objects in this compartment.
     // This is used by the GC to trace them all first when compacting, since the
     // TypedObject trace hook may access these objects.

js/src/jit-test/tests/cacheir/nukedCCW.js

@@ -1,6 +1,6 @@
-var wrapper = evaluate("({a: 15, b: {c: 42}})", {global: newGlobal({sameZoneAs: this})});
+function testNuke() {
+    var wrapper = evaluate("({a: 15, b: {c: 42}})", {global: newGlobal({sameZoneAs: this})});
 
-function test() {
     var i, error;
     try {
         for (i = 0; i < 150; i++) {
@@ -20,4 +20,19 @@ function test() {
     assertEq(i, 143);
 }
 
-test();
+function testSweep() {
+    var wrapper = evaluate("({a: 15, b: {c: 42}})", {global: newGlobal({})});
+    var error;
+    nukeCCW(wrapper);
+    gczeal(8, 1); // Sweep zones separately
+    try {
+      // Next access to wrapper.b should throw.
+      wrapper.x = 4;
+    } catch (e) {
+        error = e;
+    }
+    assertEq(error.message.includes("dead object"), true);
+}
+
+testNuke();
+testSweep();

js/src/jscompartment.h

@@ -745,6 +745,8 @@ struct JSCompartment
 
     void findOutgoingEdges(js::gc::ZoneComponentFinder& finder);
 
+    MOZ_MUST_USE bool findDeadProxyZoneEdges(bool* foundAny);
+
     js::DtoaCache dtoaCache;
     js::NewProxyCache newProxyCache;
 

js/src/jsgc.cpp

@@ -3182,7 +3182,7 @@ GCRuntime::sweepBackgroundThings(ZoneList& zones, LifoAlloc& freeBlocks)
 
     AutoLockGC lock(rt);
 
-    // Release swept areans, dropping and reaquiring the lock every so often to
+    // Release swept arenas, dropping and reaquiring the lock every so often to
     // avoid blocking the active thread from allocating chunks.
     static const size_t LockReleasePeriod = 32;
     size_t releaseCount = 0;
@@ -4400,8 +4400,8 @@ DropStringWrappers(JSRuntime* rt)
  *
  * If compartment A has an edge to an unmarked object in compartment B, then we
  * must not sweep A in a later slice than we sweep B. That's because a write
- * barrier in A that could lead to the unmarked object in B becoming
- * marked. However, if we had already swept that object, we would be in trouble.
+ * barrier in A could lead to the unmarked object in B becoming marked.
+ * However, if we had already swept that object, we would be in trouble.
  *
  * If we consider these dependencies as a graph, then all the compartments in
  * any strongly-connected component of this graph must be swept in the same
@@ -4451,6 +4451,28 @@ JSCompartment::findOutgoingEdges(ZoneComponentFinder& finder)
     }
 }
 
+bool
+JSCompartment::findDeadProxyZoneEdges(bool* foundAny)
+{
+    // As an optimization, return whether any dead proxy objects are found in
+    // this compartment so that if a zone has none, its cross compartment
+    // wrappers do not need to be scanned.
+    *foundAny = false;
+    for (js::WrapperMap::Enum e(crossCompartmentWrappers); !e.empty(); e.popFront()) {
+        Value value = e.front().value().get();
+        if (value.isObject()) {
+            if (IsDeadProxyObject(&value.toObject())) {
+                *foundAny = true;
+                CrossCompartmentKey& key = e.front().mutableKey();
+                if (!key.as<JSObject*>()->zone()->gcZoneGroupEdges().put(zone()))
+                    return false;
+            }
+        }
+    }
+
+    return true;
+}
+
 void
 Zone::findOutgoingEdges(ZoneComponentFinder& finder)
 {
@@ -4475,7 +4497,7 @@ Zone::findOutgoingEdges(ZoneComponentFinder& finder)
 }
 
 bool
-GCRuntime::findZoneEdgesForWeakMaps()
+GCRuntime::findInterZoneEdges()
 {
     /*
      * Weakmaps which have keys with delegates in a different zone introduce the
@@ -4492,6 +4514,20 @@ GCRuntime::findZoneEdgesForWeakMaps()
             return false;
     }
 
+    for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
+        if (zone->hasDeadProxies()) {
+            bool foundInZone = false;
+            for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next()) {
+                bool foundInCompartment = false;
+                if (!comp->findDeadProxyZoneEdges(&foundInCompartment))
+                    return false;
+                foundInZone = foundInZone || foundInCompartment;
+            }
+            if (!foundInZone)
+                zone->setHasDeadProxies(false);
+        }
+    }
+
     return true;
 }
 
@@ -4505,7 +4541,7 @@ GCRuntime::findZoneGroups(AutoLockForExclusiveAccess& lock)
 
     JSContext* cx = TlsContext.get();
     ZoneComponentFinder finder(cx->nativeStackLimit[JS::StackForSystemCode], lock);
-    if (!isIncremental || !findZoneEdgesForWeakMaps())
+    if (!isIncremental || !findInterZoneEdges())
         finder.useOneComponent();
 
     for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
@@ -4780,6 +4816,8 @@ js::NotifyGCNukeWrapper(JSObject* obj)
      * remember to mark it.
      */
     RemoveFromGrayList(obj);
+
+    obj->zone()->setHasDeadProxies(true);
 }
 
 enum {