commit 059f9442394566ff0be7439a15ee55703c5e0057
parent 9d35d29935755c8d272692f81ef4fdbcd5778c90
Author: Jon Coppeard <jcoppeard@mozilla.com>
Date: Wed, 12 Nov 2025 15:18:01 +0000
Bug 1827612 - Remove stable hashing for WeakMap and rekey entries instead r=sfink
This adds a vector of keys of entries where the key or value is in the nursery
so that we can trace these at the start or minor GC. If the vector grows too
large we sweep the whole map instead.
This replaces the post barrier for WeakMap keys so the table now uses
PreBarriered<Key> instead of HeapPtr<Key>.
This doesn't provide much improvement to WeakMap.get performance on its own but
it is a prerequisite for bug 1995021.
Stable hashing is still used for WeakRef and FinalizationRegistry targets.
Differential Revision: https://phabricator.services.mozilla.com/D270562
Diffstat:
14 files changed, 396 insertions(+), 124 deletions(-)
diff --git a/js/public/GCVector.h b/js/public/GCVector.h
@@ -85,8 +85,8 @@ class GCVector {
[[nodiscard]] bool growBy(size_t amount) { return vector.growBy(amount); }
[[nodiscard]] bool resize(size_t newLen) { return vector.resize(newLen); }
- void clear() { return vector.clear(); }
- void clearAndFree() { return vector.clearAndFree(); }
+ void clear() { vector.clear(); }
+ void clearAndFree() { vector.clearAndFree(); }
template <typename U>
bool append(U&& item) {
@@ -185,7 +185,7 @@ class GCVector {
// Like eraseIf, but may mutate the contents of the vector. Iterates from
// |startIndex| to the last element of the vector.
template <typename Pred>
- void mutableEraseIf(Pred pred, size_t startIndex = 0) {
+ void mutableEraseIf(Pred&& pred, size_t startIndex = 0) {
MOZ_ASSERT(startIndex <= length());
T* src = begin() + startIndex;
diff --git a/js/src/debugger/Debugger.cpp b/js/src/debugger/Debugger.cpp
@@ -4006,7 +4006,7 @@ void DebugAPI::slowPathTraceGeneratorFrame(JSTracer* tracer,
if (Debugger::GeneratorWeakMap::Ptr entry =
dbg->generatorFrames.lookupUnbarriered(generator)) {
- HeapPtr<DebuggerFrame*>& frameObj = entry->value();
+ PreBarriered<DebuggerFrame*>& frameObj = entry->value();
if (frameObj->hasAnyHooks()) {
// See comment above.
TraceCrossCompartmentEdge(tracer, generator, &frameObj,
diff --git a/js/src/debugger/Debugger.h b/js/src/debugger/Debugger.h
@@ -394,8 +394,13 @@ class DebuggerWeakMap : private WeakMap<Referent*, Wrapper*, ZoneAllocPolicy> {
public:
void traceCrossCompartmentEdges(JSTracer* tracer) {
for (Enum e(*this); !e.empty(); e.popFront()) {
- TraceEdge(tracer, &e.front().mutableKey(), "Debugger WeakMap key");
+ // The values are debugger objects which contain a cross-compartment
+ // debuggee pointer, so trace their contents.
e.front().value()->trace(tracer);
+
+ // Trace the keys, which are cross compartment debuggee pointers.
+ // This can rekey the entry and invalidate |e.front()|.
+ Base::traceKey(tracer, e);
}
}
diff --git a/js/src/gc/FinalizationObservers.h b/js/src/gc/FinalizationObservers.h
@@ -8,7 +8,7 @@
#define gc_FinalizationObservers_h
#include "gc/Barrier.h"
-#include "gc/WeakMap.h" // For WeakTargetHasher
+#include "gc/WeakMap.h" // For GetSymbolHash.
#include "gc/ZoneAllocator.h"
#include "js/GCHashTable.h"
#include "js/GCVector.h"
@@ -147,6 +147,41 @@ class ObserverList {
void insertFront(ObserverListObject* obj);
};
+// A hasher for GC things used as WeakRef and FinalizationRegistry targets. Uses
+// stable cell hashing, except for symbols where it uses the symbol's stored
+// hash.
+struct WeakTargetHasher {
+ using Key = HeapPtr<Value>;
+ using Lookup = Value;
+
+ static bool maybeGetHash(const Lookup& l, HashNumber* hashOut) {
+ if (l.isSymbol()) {
+ *hashOut = GetSymbolHash(l.toSymbol());
+ return true;
+ }
+ return StableCellHasher<Cell*>::maybeGetHash(l.toGCThing(), hashOut);
+ }
+ static bool ensureHash(const Lookup& l, HashNumber* hashOut) {
+ if (l.isSymbol()) {
+ *hashOut = GetSymbolHash(l.toSymbol());
+ return true;
+ }
+ return StableCellHasher<Cell*>::ensureHash(l.toGCThing(), hashOut);
+ }
+ static HashNumber hash(const Lookup& l) {
+ if (l.isSymbol()) {
+ return GetSymbolHash(l.toSymbol());
+ }
+ return StableCellHasher<Cell*>::hash(l.toGCThing());
+ }
+ static bool match(const Key& k, const Lookup& l) {
+ if (l.isSymbol()) {
+ return k.toSymbol() == l.toSymbol();
+ }
+ return StableCellHasher<Cell*>::match(k.toGCThing(), l.toGCThing());
+ }
+};
+
// Per-zone data structures to support FinalizationRegistry and WeakRef.
class FinalizationObservers {
// The set of all finalization registries in the associated zone.
@@ -195,4 +230,10 @@ class FinalizationObservers {
} // namespace gc
} // namespace js
+namespace mozilla {
+template <>
+struct FallibleHashMethods<js::gc::WeakTargetHasher>
+ : public js::gc::WeakTargetHasher {};
+} // namespace mozilla
+
#endif // gc_FinalizationObservers_h
diff --git a/js/src/gc/Marking-inl.h b/js/src/gc/Marking-inl.h
@@ -102,6 +102,10 @@ inline bool IsForwarded<Cell>(const Cell* t) {
return t->isForwarded();
}
+inline bool IsForwarded(const JS::Value& value) {
+ return value.isGCThing() && IsForwarded(value.toGCThing());
+}
+
template <typename T>
inline T* Forwarded(const T* t) {
const RelocationOverlay* overlay = RelocationOverlay::fromCell(t);
@@ -109,13 +113,21 @@ inline T* Forwarded(const T* t) {
return reinterpret_cast<T*>(overlay->forwardingAddress());
}
+inline JS::Value Forwarded(const JS::Value& value) {
+ MOZ_ASSERT(IsForwarded(value));
+ JS::Value result = value;
+ result.changeGCThingPayload(Forwarded(value.toGCThing()));
+ return result;
+}
+
template <typename T>
-inline T MaybeForwarded(T t) {
- if (IsForwarded(t)) {
- t = Forwarded(t);
+inline T MaybeForwarded(const T& t) {
+ if (!IsForwarded(t)) {
+ return t;
}
- MOZ_ASSERT(!IsForwarded(t));
- return t;
+ T result = Forwarded(t);
+ MOZ_ASSERT(!IsForwarded(result));
+ return result;
}
inline const JSClass* MaybeForwardedObjectClass(const JSObject* obj) {
diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp
@@ -547,17 +547,9 @@ template void js::TraceSameZoneCrossCompartmentEdge(
template <typename T>
void js::TraceWeakMapKeyEdgeInternal(JSTracer* trc, Zone* weakMapZone,
T** thingp, const char* name) {
- // We can't use ShouldTraceCrossCompartment here because that assumes the
- // source of the edge is a CCW object which could be used to delay gray
- // marking. Instead, assert that the weak map zone is in the same marking
- // state as the target thing's zone and therefore we can go ahead and mark it.
-#ifdef DEBUG
- auto thing = *thingp;
- if (trc->isMarkingTracer()) {
- MOZ_ASSERT(weakMapZone->isGCMarking());
- MOZ_ASSERT(weakMapZone->gcState() == thing->zone()->gcState());
- }
-#endif
+ // We'd like to assert that the the thing's zone is currently being marked but
+ // that's not always true when tracing debugger weak maps which have keys in
+ // other compartments.
// Clear expected compartment for cross-compartment edge.
AutoClearTracingSource acts(trc);
diff --git a/js/src/gc/Marking.h b/js/src/gc/Marking.h
@@ -112,14 +112,14 @@ namespace gc {
template <typename T>
inline bool IsForwarded(const T* t);
+inline bool IsForwarded(const JS::Value& value);
template <typename T>
inline T* Forwarded(const T* t);
-
inline Value Forwarded(const JS::Value& value);
template <typename T>
-inline T MaybeForwarded(T t);
+inline T MaybeForwarded(const T& t);
// Helper functions for use in situations where the object's group might be
// forwarded, for example while marking.
diff --git a/js/src/gc/Nursery.cpp b/js/src/gc/Nursery.cpp
@@ -1713,6 +1713,8 @@ void js::Nursery::traceRoots(AutoGCSession& session, TenuringTracer& mover) {
DebugAPI::traceAllForMovingGC(&mover);
}
endProfile(ProfileKey::MarkDebugger);
+
+ traceWeakMaps(mover);
}
bool js::Nursery::shouldTenureEverything(JS::GCReason reason) {
@@ -2594,7 +2596,15 @@ void js::Nursery::sweepMapAndSetObjects() {
}
}
+void Nursery::traceWeakMaps(TenuringTracer& trc) {
+ // Conservatively assume all weak map keys and values are live.
+ MOZ_ASSERT(trc.weakMapAction() == JS::WeakMapTraceAction::TraceKeysAndValues);
+ weakMapsWithNurseryEntries_.eraseIf(
+ [&](WeakMapBase* wm) { return wm->traceNurseryEntriesOnMinorGC(&trc); });
+}
+
void js::Nursery::joinSweepTask() { sweepTask->join(); }
+
void js::Nursery::joinDecommitTask() { decommitTask->join(); }
#ifdef DEBUG
diff --git a/js/src/gc/Nursery.h b/js/src/gc/Nursery.h
@@ -73,6 +73,7 @@ class NurseryDecommitTask;
class NurserySweepTask;
class SetObject;
class JS_PUBLIC_API Sprinter;
+class WeakMapBase;
namespace gc {
@@ -314,6 +315,11 @@ class Nursery {
setsWithNurseryIterators_.back() != obj);
return setsWithNurseryIterators_.append(obj);
}
+ bool addWeakMapWithNurseryEntries(WeakMapBase* wm) {
+ MOZ_ASSERT_IF(!weakMapsWithNurseryEntries_.empty(),
+ weakMapsWithNurseryEntries_.back() != wm);
+ return weakMapsWithNurseryEntries_.append(wm);
+ }
void joinSweepTask();
void joinDecommitTask();
@@ -492,6 +498,8 @@ class Nursery {
void clearMapAndSetNurseryIterators();
void sweepMapAndSetObjects();
+ void traceWeakMaps(gc::TenuringTracer& trc);
+
void sweepStringsWithBuffer();
void sweepBuffers();
@@ -718,6 +726,12 @@ class Nursery {
StringBufferVector stringBuffersToReleaseAfterMinorGC_;
UniquePtr<NurserySweepTask> sweepTask;
+
+ // Lists of weakmaps with nursery allocated keys or values. Such objects need
+ // to be swept after minor GC.
+ using WeakMapVector = Vector<WeakMapBase*, 0, SystemAllocPolicy>;
+ WeakMapVector weakMapsWithNurseryEntries_;
+
UniquePtr<NurseryDecommitTask> decommitTask;
// Whether the previous collection tenured everything. This may be false if
diff --git a/js/src/gc/Tracer.h b/js/src/gc/Tracer.h
@@ -117,9 +117,6 @@ bool TraceEdgeInternal(JSTracer* trc, wasm::AnyRef* thingp, const char* name);
template <typename T>
void TraceRangeInternal(JSTracer* trc, size_t len, T* vec, const char* name);
-template <typename T>
-bool TraceWeakMapKeyInternal(JSTracer* trc, Zone* zone, T* thingp,
- const char* name);
#ifdef DEBUG
void AssertRootMarkingPhase(JSTracer* trc);
diff --git a/js/src/gc/WeakMap-inl.h b/js/src/gc/WeakMap-inl.h
@@ -163,8 +163,7 @@ WeakMap<K, V, AP>::~WeakMap() {
// value (or the key).
template <class K, class V, class AP>
bool WeakMap<K, V, AP>::markEntry(GCMarker* marker, gc::CellColor mapColor,
- BarrieredKey& key, BarrieredValue& value,
- bool populateWeakKeysTable) {
+ Enum& iter, bool populateWeakKeysTable) {
#ifdef DEBUG
MOZ_ASSERT(IsMarked(mapColor));
if (marker->isParallelMarking()) {
@@ -172,6 +171,9 @@ bool WeakMap<K, V, AP>::markEntry(GCMarker* marker, gc::CellColor mapColor,
}
#endif
+ BarrieredKey& key = iter.front().mutableKey();
+ BarrieredValue& value = iter.front().value();
+
JSTracer* trc = marker->tracer();
gc::Cell* keyCell = gc::ToMarkable(key);
MOZ_ASSERT(keyCell);
@@ -201,8 +203,7 @@ bool WeakMap<K, V, AP>::markEntry(GCMarker* marker, gc::CellColor mapColor,
if (keyColor < proxyPreserveColor) {
MOZ_ASSERT(markColor >= proxyPreserveColor);
if (markColor == proxyPreserveColor) {
- TraceWeakMapKeyEdge(trc, zone(), &key,
- "proxy-preserved WeakMap entry key");
+ traceKey(trc, iter);
MOZ_ASSERT(keyCell->color() >= proxyPreserveColor);
marked = true;
keyColor = proxyPreserveColor;
@@ -272,18 +273,31 @@ void WeakMap<K, V, AP>::trace(JSTracer* trc) {
return;
}
- // Trace keys only if weakMapAction() says to.
- if (trc->weakMapAction() == JS::WeakMapTraceAction::TraceKeysAndValues) {
- for (Enum e(*this); !e.empty(); e.popFront()) {
- TraceWeakMapKeyEdge(trc, zone(), &e.front().mutableKey(),
- "WeakMap entry key");
+ for (Enum e(*this); !e.empty(); e.popFront()) {
+ // Always trace all values (unless weakMapAction() is Skip).
+ TraceEdge(trc, &e.front().value(), "WeakMap entry value");
+
+ // Trace keys only if weakMapAction() says to.
+ if (trc->weakMapAction() == JS::WeakMapTraceAction::TraceKeysAndValues) {
+ traceKey(trc, e);
}
}
+}
- // Always trace all values (unless weakMapAction() is Skip).
- for (Range r = all(); !r.empty(); r.popFront()) {
- TraceEdge(trc, &r.front().value(), "WeakMap entry value");
+template <class K, class V, class AP>
+void WeakMap<K, V, AP>::traceKey(JSTracer* trc, Enum& iter) {
+ PreBarriered<K> key = iter.front().key();
+ TraceWeakMapKeyEdge(trc, zone(), &key, "WeakMap entry key");
+ if (key != iter.front().key()) {
+ iter.rekeyFront(key);
}
+
+ // TODO: This is a work around to prevent the pre-barrier firing. The
+ // rekeyFront() method requires passing in an instance of the key which in
+ // this case has a barrier. It should be possible to create the key in place
+ // by passing in a pointer as happens for other hash table methods that create
+ // entries.
+ key.unbarrieredSet(JS::SafelyInitialized<K>::create());
}
template <class K, class V, class AP>
@@ -312,8 +326,7 @@ bool WeakMap<K, V, AP>::markEntries(GCMarker* marker) {
gc::CellColor mapColor = this->mapColor();
for (Enum e(*this); !e.empty(); e.popFront()) {
- if (markEntry(marker, mapColor, e.front().mutableKey(), e.front().value(),
- populateWeakKeysTable)) {
+ if (markEntry(marker, mapColor, e, populateWeakKeysTable)) {
markedAny = true;
}
}
@@ -323,20 +336,27 @@ bool WeakMap<K, V, AP>::markEntries(GCMarker* marker) {
template <class K, class V, class AP>
void WeakMap<K, V, AP>::traceWeakEdges(JSTracer* trc) {
- MOZ_ASSERT(zone()->isGCSweeping());
+ // This is used for sweeping but not for anything that can move GC things.
+ MOZ_ASSERT(!trc->isTenuringTracer() && trc->kind() != JS::TracerKind::Moving);
// Scan the map, removing all entries whose keys remain unmarked. Rebuild
// cached key state at the same time.
mayHaveSymbolKeys = false;
mayHaveKeyDelegates = false;
for (Enum e(*this); !e.empty(); e.popFront()) {
+#ifdef DEBUG
+ K prior = e.front().key();
+#endif
if (TraceWeakEdge(trc, &e.front().mutableKey(), "WeakMap key")) {
- keyWriteBarrier(e.front().key());
+ MOZ_ASSERT(e.front().key() == prior);
+ keyKindBarrier(e.front().key());
} else {
e.removeFront();
}
}
+ // TODO: Shrink nurseryKeys storage?
+
#if DEBUG
// Once we've swept, all remaining edges should stay within the known-live
// part of the graph.
@@ -344,6 +364,84 @@ void WeakMap<K, V, AP>::traceWeakEdges(JSTracer* trc) {
#endif
}
+template <class K, class V, class AP>
+void WeakMap<K, V, AP>::addNurseryKey(const K& key) {
+ MOZ_ASSERT(hasNurseryEntries); // Must be set before calling this.
+
+ if (!nurseryKeysValid) {
+ return;
+ }
+
+ if (nurseryKeys.length() >= map().count() / 2) {
+ // Don't bother recording every key if there a lot of them. We will scan the
+ // map instead.
+ nurseryKeysValid = false;
+ return;
+ }
+
+ if (!nurseryKeys.append(key)) {
+ nurseryKeysValid = false;
+ }
+}
+
+template <class K, class V, class AP>
+bool WeakMap<K, V, AP>::traceNurseryEntriesOnMinorGC(JSTracer* trc) {
+ MOZ_ASSERT(hasNurseryEntries);
+
+ using Entry = typename Map::Entry;
+ auto traceEntry = [trc](K& key,
+ const Entry& entry) -> std::tuple<bool, bool> {
+ TraceEdge(trc, &entry.value(), "WeakMap nursery value");
+ bool hasNurseryValue = !JS::GCPolicy<V>::isTenured(entry.value());
+
+ MOZ_ASSERT(key == entry.key());
+ TraceManuallyBarrieredEdge(trc, &key, "WeakMap nursery key");
+ bool hasNurseryKey = !JS::GCPolicy<K>::isTenured(key);
+ bool keyUpdated = key != entry.key();
+
+ return {keyUpdated, hasNurseryKey || hasNurseryValue};
+ };
+
+ if (nurseryKeysValid) {
+ nurseryKeys.mutableEraseIf([&](K& key) {
+ auto ptr = lookupUnbarriered(key);
+ if (!ptr) {
+ return true;
+ }
+
+ auto [keyUpdated, hasNurseryKeyOrValue] = traceEntry(key, *ptr);
+
+ if (keyUpdated) {
+ map().rekeyAs(ptr->key(), key, key);
+ }
+
+ return !hasNurseryKeyOrValue;
+ });
+ } else {
+ nurseryKeys.clear();
+ nurseryKeysValid = true;
+
+ for (Enum e(*this); !e.empty(); e.popFront()) {
+ Entry& entry = e.front();
+
+ K key = entry.key();
+ auto [keyUpdated, hasNurseryKeyOrValue] = traceEntry(key, entry);
+
+ if (keyUpdated) {
+ entry.mutableKey() = key;
+ e.rekeyFront(key);
+ }
+
+ if (hasNurseryKeyOrValue) {
+ addNurseryKey(key);
+ }
+ }
+ }
+
+ hasNurseryEntries = !nurseryKeysValid || !nurseryKeys.empty();
+ return !hasNurseryEntries;
+}
+
// memberOf can be nullptr, which means that the map is not part of a JSObject.
template <class K, class V, class AP>
void WeakMap<K, V, AP>::traceMappings(WeakMapTracer* tracer) {
@@ -447,6 +545,10 @@ bool WeakMap<K, V, AP>::checkMarking() const {
#ifdef JSGC_HASH_TABLE_CHECKS
template <class K, class V, class AP>
void WeakMap<K, V, AP>::checkAfterMovingGC() const {
+ MOZ_RELEASE_ASSERT(!hasNurseryEntries);
+ MOZ_RELEASE_ASSERT(nurseryKeysValid);
+ MOZ_RELEASE_ASSERT(nurseryKeys.empty());
+
for (Range r = all(); !r.empty(); r.popFront()) {
gc::Cell* key = gc::ToMarkable(r.front().key());
gc::Cell* value = gc::ToMarkable(r.front().value());
@@ -484,6 +586,11 @@ static MOZ_ALWAYS_INLINE bool CanBeHeldWeakly(Value value) {
inline HashNumber GetSymbolHash(JS::Symbol* sym) { return sym->hash(); }
+/* static */
+inline void WeakMapKeyHasher<JS::Value>::checkValueType(const Value& value) {
+ MOZ_ASSERT(CanBeHeldWeakly(value));
+}
+
} // namespace js
#endif /* gc_WeakMap_inl_h */
diff --git a/js/src/gc/WeakMap.cpp b/js/src/gc/WeakMap.cpp
@@ -187,6 +187,19 @@ void WeakMapBase::restoreMarkedWeakMaps(WeakMapColors& markedWeakMaps) {
}
}
+void WeakMapBase::setHasNurseryEntries() {
+ MOZ_ASSERT(!hasNurseryEntries);
+
+ AutoEnterOOMUnsafeRegion oomUnsafe;
+
+ GCRuntime* gc = &zone()->runtimeFromMainThread()->gc;
+ if (!gc->nursery().addWeakMapWithNurseryEntries(this)) {
+ oomUnsafe.crash("WeakMapBase::setHasNurseryEntries");
+ }
+
+ hasNurseryEntries = true;
+}
+
namespace js {
template class WeakMap<JSObject*, JSObject*, ZoneAllocPolicy>;
} // namespace js
diff --git a/js/src/gc/WeakMap.h b/js/src/gc/WeakMap.h
@@ -10,10 +10,12 @@
#include "mozilla/Atomics.h"
#include "mozilla/LinkedList.h"
+#include "gc/AllocKind.h"
#include "gc/Barrier.h"
#include "gc/Marking.h"
#include "gc/Tracer.h"
#include "gc/ZoneAllocator.h"
+#include "js/GCVector.h"
#include "js/HashTable.h"
#include "js/HeapAPI.h"
#include "vm/JSObject.h"
@@ -40,6 +42,26 @@ namespace gc {
bool CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key, Cell* value);
#endif
+template <typename PtrT>
+struct MightBeInNursery {
+ using T = std::remove_pointer_t<PtrT>;
+ static_assert(std::is_base_of_v<Cell, T>);
+ static_assert(!std::is_same_v<Cell, T> && !std::is_same_v<TenuredCell, T>);
+
+#define CAN_NURSERY_ALLOC_KIND_OR(_1, _2, Type, _3, _4, canNurseryAlloc, _5) \
+ std::is_base_of_v<Type, T> ? canNurseryAlloc:
+
+ // FOR_EACH_ALLOCKIND doesn't cover every possible type: make sure
+ // to default to `true` for unknown types.
+ static constexpr bool value =
+ FOR_EACH_ALLOCKIND(CAN_NURSERY_ALLOC_KIND_OR) true;
+#undef CAN_NURSERY_ALLOC_KIND_OR
+};
+template <>
+struct MightBeInNursery<JS::Value> {
+ static constexpr bool value = true;
+};
+
} // namespace gc
// A subclass template of js::HashMap whose keys and values may be
@@ -156,6 +178,12 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
virtual void traceMappings(WeakMapTracer* tracer) = 0;
virtual void clearAndCompact() = 0;
+ virtual bool markEntries(GCMarker* marker) = 0;
+
+ // Trace any keys and values that are in the nursery. Return false if any
+ // remain in the nursery.
+ virtual bool traceNurseryEntriesOnMinorGC(JSTracer* trc) = 0;
+
// We have a key that, if it or its delegate is marked, may lead to a WeakMap
// value getting marked. Insert the necessary edges into the appropriate
// zone's gcEphemeronEdges or gcNurseryEphemeronEdges tables.
@@ -166,12 +194,12 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
[[nodiscard]] bool addEphemeronEdge(gc::MarkColor color, gc::Cell* src,
gc::Cell* dst);
- virtual bool markEntries(GCMarker* marker) = 0;
-
gc::CellColor mapColor() const { return gc::CellColor(uint32_t(mapColor_)); }
void setMapColor(gc::CellColor newColor) { mapColor_ = uint32_t(newColor); }
bool markMap(gc::MarkColor markColor);
+ void setHasNurseryEntries();
+
#ifdef JS_GC_ZEAL
virtual bool checkMarking() const = 0;
virtual bool allowKeysInOtherZones() const { return false; }
@@ -197,24 +225,106 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
bool mayHaveKeyDelegates = false;
bool mayHaveSymbolKeys = false;
+ // Whether this map contains entries with nursery keys or values.
+ bool hasNurseryEntries = false;
+
+ // Whether the |nurseryKeys| vector contains the keys of all entries with
+ // nursery keys or values. This can be false if it gets too large or on OOM.
+ bool nurseryKeysValid = true;
+
friend class JS::Zone;
+ friend class js::Nursery;
};
-template <typename Key>
-struct WeakMapKeyHasher : public StableCellHasher<HeapPtr<Key>> {};
+// Get the hash from a Symbol.
+HashNumber GetSymbolHash(JS::Symbol* sym);
+
+// By default weak maps use default hasher for the key type, which hashes
+// the pointer itself for pointer types.
+template <typename T>
+struct WeakMapKeyHasher : public DefaultHasher<T> {};
+
+// We only support JS::Value keys that contain objects or symbols. For objects
+// we hash the pointer and for symbols we use its stored hash, which is randomly
+// generated on creation.
+//
+// Equality is based on a bitwise test not on JS Value semantics.
+//
+// Take care when modifying this code! Previously there have been security
+// issues around using pointer hashing for maps (e.g. bug 1312001).
+//
+// Although this does use pointer hashing for objects, we don't think those
+// concerns apply here because:
+//
+// 1) This uses an open addressed hash table rather than a chained one which
+// makes the attack much more difficult.
+//
+// 2) The allowed key types are restricted to objects and non-registered
+// symbols, so it's not possible to use int32 keys as were used in the
+// attack.
+//
+// 3) Symbols use their own random hash codes which can't be predicted.
+//
+// 4) Registered symbols are not allowed, which means it's not possible to leak
+// information about such symbols used by another zone.
+//
+// 5) Although sequentially allocated objects will have similar pointers,
+// ScrambleHashCode should work well enough to distribute these keys and
+// make predicting the hash code from the pointer difficult.
+template <>
+struct WeakMapKeyHasher<JS::Value> {
+ using Key = JS::Value;
+ using Lookup = JS::Value;
+
+ static HashNumber hash(const Lookup& l) {
+ checkValueType(l);
+ if (l.isSymbol()) {
+ return GetSymbolHash(l.toSymbol());
+ }
+ return mozilla::HashGeneric(l.asRawBits());
+ }
+
+ static bool match(const Key& k, const Lookup& l) {
+ checkValueType(k);
+ return k == l;
+ }
+
+ static void rekey(Key& k, const Key& newKey) { k = newKey; }
+
+ private:
+ static void checkValueType(const Value& value);
+};
+
+template <>
+struct WeakMapKeyHasher<PreBarriered<JS::Value>> {
+ using Key = PreBarriered<JS::Value>;
+ using Lookup = JS::Value;
+
+ static HashNumber hash(const Lookup& l) {
+ return WeakMapKeyHasher<JS::Value>::hash(l);
+ }
+ static bool match(const Key& k, const Lookup& l) {
+ return WeakMapKeyHasher<JS::Value>::match(k, l);
+ }
+ static void rekey(Key& k, const Key& newKey) { k.unbarrieredSet(newKey); }
+};
template <class Key, class Value, class AllocPolicy>
class WeakMap : public WeakMapBase {
- using BarrieredKey = HeapPtr<Key>;
- using BarrieredValue = HeapPtr<Value>;
+ using BarrieredKey = PreBarriered<Key>;
+ using BarrieredValue = PreBarriered<Value>;
- using Map =
- HashMap<HeapPtr<Key>, HeapPtr<Value>, WeakMapKeyHasher<Key>, AllocPolicy>;
+ using Map = HashMap<BarrieredKey, BarrieredValue,
+ WeakMapKeyHasher<BarrieredKey>, AllocPolicy>;
using UnbarrieredMap =
- HashMap<Key, Value, StableCellHasher<Key>, AllocPolicy>;
+ HashMap<Key, Value, WeakMapKeyHasher<Key>, AllocPolicy>;
UnbarrieredMap map_; // Barriers are added by |map()| accessor.
+ // The keys of entries where either the key or value is allocated in the
+ // nursery.
+ GCVector<Key, 0, SystemAllocPolicy> nurseryKeys;
+
public:
using Lookup = typename Map::Lookup;
using Entry = typename Map::Entry;
@@ -276,36 +386,32 @@ class WeakMap : public WeakMapBase {
[[nodiscard]] bool add(AddPtr& p, const Key& k, const Value& v) {
MOZ_ASSERT(gc::ToMarkable(k));
- keyWriteBarrier(k);
+ writeBarrier(k, v);
return map().add(p, k, v);
}
[[nodiscard]] bool relookupOrAdd(AddPtr& p, const Key& k, const Value& v) {
MOZ_ASSERT(gc::ToMarkable(k));
- keyWriteBarrier(k);
+ writeBarrier(k, v);
return map().relookupOrAdd(p, k, v);
}
[[nodiscard]] bool put(const Key& k, const Value& v) {
MOZ_ASSERT(gc::ToMarkable(k));
- keyWriteBarrier(k);
+ writeBarrier(k, v);
return map().put(k, v);
}
[[nodiscard]] bool putNew(const Key& k, const Value& v) {
MOZ_ASSERT(gc::ToMarkable(k));
- keyWriteBarrier(k);
+ writeBarrier(k, v);
return map().putNew(k, v);
}
- void putNewInfallible(const Key& k, const Value& v) {
- MOZ_ASSERT(gc::ToMarkable(k));
- keyWriteBarrier(k);
- map().putNewInfallible(k, k);
- }
-
void clear() {
map().clear();
+ nurseryKeys.clear();
+ nurseryKeysValid = true;
mayHaveSymbolKeys = false;
mayHaveKeyDelegates = false;
}
@@ -317,11 +423,15 @@ class WeakMap : public WeakMapBase {
}
#endif
- bool markEntry(GCMarker* marker, gc::CellColor mapColor, BarrieredKey& key,
- BarrieredValue& value, bool populateWeakKeysTable);
+ bool markEntry(GCMarker* marker, gc::CellColor mapColor, Enum& iter,
+ bool populateWeakKeysTable);
void trace(JSTracer* trc) override;
+ // Used by the debugger to trace cross-compartment edges.
+ void traceKeys(JSTracer* trc);
+ void traceKey(JSTracer* trc, Enum& iter);
+
size_t sizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
protected:
@@ -357,84 +467,57 @@ class WeakMap : public WeakMapBase {
JS::ExposeObjectToActiveJS(obj);
}
- void keyWriteBarrier(const JS::Value& v) {
- if (v.isSymbol()) {
+ void writeBarrier(const Key& key, const Value& value) {
+ keyKindBarrier(key);
+ nurseryEntryBarrier(key, value);
+ }
+
+ void keyKindBarrier(const JS::Value& key) {
+ if (key.isSymbol()) {
mayHaveSymbolKeys = true;
}
- if (v.isObject()) {
- keyWriteBarrier(&v.toObject());
+ if (key.isObject()) {
+ keyKindBarrier(&key.toObject());
}
}
- void keyWriteBarrier(JSObject* key) {
+ void keyKindBarrier(JSObject* key) {
JSObject* delegate = UncheckedUnwrapWithoutExpose(key);
if (delegate != key || ObjectMayBeSwapped(key)) {
mayHaveKeyDelegates = true;
}
}
- void keyWriteBarrier(BaseScript* key) {}
+ void keyKindBarrier(BaseScript* key) {}
+
+ void nurseryEntryBarrier(const Key& key, const Value& value) {
+ if ((gc::MightBeInNursery<Key>::value &&
+ !JS::GCPolicy<Key>::isTenured(key)) ||
+ (gc::MightBeInNursery<Value>::value &&
+ !JS::GCPolicy<Value>::isTenured(value))) {
+ if (!hasNurseryEntries) {
+ setHasNurseryEntries();
+ }
+
+ addNurseryKey(key);
+ }
+ }
+
+ void addNurseryKey(const Key& key);
void traceWeakEdges(JSTracer* trc) override;
void clearAndCompact() override {
- map().clear();
+ clear();
map().compact();
+ nurseryKeys.clearAndFree();
}
// memberOf can be nullptr, which means that the map is not part of a
// JSObject.
void traceMappings(WeakMapTracer* tracer) override;
-};
-
-// Get the hash from the Symbol.
-HashNumber GetSymbolHash(JS::Symbol* sym);
-
-namespace gc {
-
-// A hasher for GC things used as WeakMap keys and WeakRef targets. Uses stable
-// cell hashing, except for symbols where it uses the symbol's stored hash.
-struct WeakTargetHasher {
- using Key = HeapPtr<Value>;
- using Lookup = Value;
- static bool maybeGetHash(const Lookup& l, HashNumber* hashOut) {
- if (l.isSymbol()) {
- *hashOut = GetSymbolHash(l.toSymbol());
- return true;
- }
- return StableCellHasher<Cell*>::maybeGetHash(l.toGCThing(), hashOut);
- }
- static bool ensureHash(const Lookup& l, HashNumber* hashOut) {
- if (l.isSymbol()) {
- *hashOut = GetSymbolHash(l.toSymbol());
- return true;
- }
- return StableCellHasher<Cell*>::ensureHash(l.toGCThing(), hashOut);
- }
- static HashNumber hash(const Lookup& l) {
- if (l.isSymbol()) {
- return GetSymbolHash(l.toSymbol());
- }
- return StableCellHasher<Cell*>::hash(l.toGCThing());
- }
- static bool match(const Key& k, const Lookup& l) {
- if (l.isSymbol()) {
- return k.toSymbol() == l.toSymbol();
- }
- return StableCellHasher<Cell*>::match(k.toGCThing(), l.toGCThing());
- }
+ bool traceNurseryEntriesOnMinorGC(JSTracer* trc) override;
};
-} // namespace gc
-
-template <>
-struct WeakMapKeyHasher<JS::Value> : public gc::WeakTargetHasher {};
-
} /* namespace js */
-namespace mozilla {
-template <typename T>
-struct FallibleHashMethods<js::WeakMapKeyHasher<T>>
- : public js::WeakMapKeyHasher<T> {};
-} // namespace mozilla
-
#endif /* gc_WeakMap_h */
diff --git a/js/src/gc/Zone.cpp b/js/src/gc/Zone.cpp
@@ -539,11 +539,9 @@ void JS::Zone::beforeClearDelegateInternal(JSObject* wrapper,
MOZ_ASSERT(needsIncrementalBarrier());
MOZ_ASSERT(!RuntimeFromMainThreadIsHeapMajorCollecting(this));
- // If |wrapper| might be a key in a weak map, trigger a barrier to account for
+ // |wrapper| might be a key in a weak map, so trigger a barrier to account for
// the removal of the automatically added edge from delegate to wrapper.
- if (HasUniqueId(wrapper)) {
- PreWriteBarrier(wrapper);
- }
+ PreWriteBarrier(wrapper);
}
#ifdef JSGC_HASH_TABLE_CHECKS
