From 8c57507111d56bb5adb974bfb08f4fae2f44dcd7 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Mon, 6 Mar 2017 12:27:43 -0800 Subject: [PATCH] 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 | 2 +- js/src/gc/Zone.cpp | 1 + js/src/gc/Zone.h | 8 ++++ js/src/jit-test/tests/cacheir/nukedCCW.js | 21 ++++++++-- js/src/jscompartment.h | 2 + js/src/jsgc.cpp | 48 ++++++++++++++++++++--- 6 files changed, 73 insertions(+), 9 deletions(-) diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h index 8b1821a81a4e..f8a8777ad380 100644 --- a/js/src/gc/GCRuntime.h +++ b/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); diff --git a/js/src/gc/Zone.cpp b/js/src/gc/Zone.cpp index f815442fd867..67030ea4d7ca 100644 --- a/js/src/gc/Zone.cpp +++ b/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), diff --git a/js/src/gc/Zone.h b/js/src/gc/Zone.h index 15f234a00d5c..c9874caa86d5 100644 --- a/js/src/gc/Zone.h +++ b/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 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 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. diff --git a/js/src/jit-test/tests/cacheir/nukedCCW.js b/js/src/jit-test/tests/cacheir/nukedCCW.js index 303750ce482a..2aad9d330c65 100644 --- a/js/src/jit-test/tests/cacheir/nukedCCW.js +++ b/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(); diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h index 65516fc26531..9f25a6bfde06 100644 --- a/js/src/jscompartment.h +++ b/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; diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index cbdc51e37232..74026d313f34 100644 --- a/js/src/jsgc.cpp +++ b/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()->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 {