commit 78e84d8f3bd294fc337296f8090f9e57b25f9546
parent 641fddc5eb8cab99f74f3b22419245c6d876aa52
Author: Narcis Beleuzu <nbeleuzu@mozilla.com>
Date: Mon, 10 Nov 2025 20:20:21 +0200
Revert "Bug 1827612 - Remove stable hashing for WeakMap and rekey entries instead r=sfink" for crashtest failures on Atomics.h
This reverts commit 7cfef661b87ad8cb294317c5dd2f248b1fead088.
Diffstat:
13 files changed, 120 insertions(+), 394 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() { vector.clear(); }
- void clearAndFree() { vector.clearAndFree(); }
+ void clear() { return vector.clear(); }
+ void clearAndFree() { return 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)) {
- PreBarriered<DebuggerFrame*>& frameObj = entry->value();
+ HeapPtr<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,13 +394,8 @@ class DebuggerWeakMap : private WeakMap<Referent*, Wrapper*, ZoneAllocPolicy> {
public:
void traceCrossCompartmentEdges(JSTracer* tracer) {
for (Enum e(*this); !e.empty(); e.popFront()) {
- // The values are debugger objects which contain a cross-compartment
- // debuggee pointer, so trace their contents.
+ TraceEdge(tracer, &e.front().mutableKey(), "Debugger WeakMap key");
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 GetSymbolHash.
+#include "gc/WeakMap.h" // For WeakTargetHasher
#include "gc/ZoneAllocator.h"
#include "js/GCHashTable.h"
#include "js/GCVector.h"
@@ -147,41 +147,6 @@ 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.
@@ -230,10 +195,4 @@ 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,10 +102,6 @@ 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);
@@ -113,21 +109,13 @@ 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(const T& t) {
- if (!IsForwarded(t)) {
- return t;
+inline T MaybeForwarded(T t) {
+ if (IsForwarded(t)) {
+ t = Forwarded(t);
}
- T result = Forwarded(t);
- MOZ_ASSERT(!IsForwarded(result));
- return result;
+ MOZ_ASSERT(!IsForwarded(t));
+ return t;
}
inline const JSClass* MaybeForwardedObjectClass(const JSObject* obj) {
diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp
@@ -547,9 +547,17 @@ template void js::TraceSameZoneCrossCompartmentEdge(
template <typename T>
void js::TraceWeakMapKeyEdgeInternal(JSTracer* trc, Zone* weakMapZone,
T** thingp, const char* name) {
- // 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.
+ // 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
// 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(const T& t);
+inline T MaybeForwarded(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,8 +1713,6 @@ void js::Nursery::traceRoots(AutoGCSession& session, TenuringTracer& mover) {
DebugAPI::traceAllForMovingGC(&mover);
}
endProfile(ProfileKey::MarkDebugger);
-
- traceWeakMaps(mover);
}
bool js::Nursery::shouldTenureEverything(JS::GCReason reason) {
@@ -2596,15 +2594,7 @@ 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,7 +73,6 @@ class NurseryDecommitTask;
class NurserySweepTask;
class SetObject;
class JS_PUBLIC_API Sprinter;
-class WeakMapBase;
namespace gc {
@@ -315,11 +314,6 @@ 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();
@@ -498,8 +492,6 @@ class Nursery {
void clearMapAndSetNurseryIterators();
void sweepMapAndSetObjects();
- void traceWeakMaps(gc::TenuringTracer& trc);
-
void sweepStringsWithBuffer();
void sweepBuffers();
@@ -726,12 +718,6 @@ 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,6 +117,9 @@ 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,7 +163,8 @@ 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,
- Enum& iter, bool populateWeakKeysTable) {
+ BarrieredKey& key, BarrieredValue& value,
+ bool populateWeakKeysTable) {
#ifdef DEBUG
MOZ_ASSERT(IsMarked(mapColor));
if (marker->isParallelMarking()) {
@@ -171,9 +172,6 @@ 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);
@@ -203,7 +201,8 @@ bool WeakMap<K, V, AP>::markEntry(GCMarker* marker, gc::CellColor mapColor,
if (keyColor < proxyPreserveColor) {
MOZ_ASSERT(markColor >= proxyPreserveColor);
if (markColor == proxyPreserveColor) {
- traceKey(trc, iter);
+ TraceWeakMapKeyEdge(trc, zone(), &key,
+ "proxy-preserved WeakMap entry key");
MOZ_ASSERT(keyCell->color() >= proxyPreserveColor);
marked = true;
keyColor = proxyPreserveColor;
@@ -273,31 +272,18 @@ void WeakMap<K, V, AP>::trace(JSTracer* trc) {
return;
}
- 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);
+ // 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");
}
}
-}
-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);
+ // Always trace all values (unless weakMapAction() is Skip).
+ for (Range r = all(); !r.empty(); r.popFront()) {
+ TraceEdge(trc, &r.front().value(), "WeakMap entry value");
}
-
- // 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>
@@ -326,7 +312,8 @@ 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, populateWeakKeysTable)) {
+ if (markEntry(marker, mapColor, e.front().mutableKey(), e.front().value(),
+ populateWeakKeysTable)) {
markedAny = true;
}
}
@@ -336,27 +323,20 @@ bool WeakMap<K, V, AP>::markEntries(GCMarker* marker) {
template <class K, class V, class AP>
void WeakMap<K, V, AP>::traceWeakEdges(JSTracer* trc) {
- // This is used for sweeping but not for anything that can move GC things.
- MOZ_ASSERT(!trc->isTenuringTracer() && trc->kind() != JS::TracerKind::Moving);
+ MOZ_ASSERT(zone()->isGCSweeping());
// 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")) {
- MOZ_ASSERT(e.front().key() == prior);
- keyKindBarrier(e.front().key());
+ keyWriteBarrier(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.
@@ -364,84 +344,6 @@ 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) {
@@ -545,10 +447,6 @@ 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());
@@ -586,11 +484,6 @@ 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,19 +187,6 @@ 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,12 +10,10 @@
#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"
@@ -42,26 +40,6 @@ 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
@@ -178,12 +156,6 @@ 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.
@@ -194,12 +166,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; }
@@ -225,106 +197,24 @@ 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;
};
-// 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 <typename Key>
+struct WeakMapKeyHasher : public StableCellHasher<HeapPtr<Key>> {};
template <class Key, class Value, class AllocPolicy>
class WeakMap : public WeakMapBase {
- using BarrieredKey = PreBarriered<Key>;
- using BarrieredValue = PreBarriered<Value>;
+ using BarrieredKey = HeapPtr<Key>;
+ using BarrieredValue = HeapPtr<Value>;
- using Map = HashMap<BarrieredKey, BarrieredValue,
- WeakMapKeyHasher<BarrieredKey>, AllocPolicy>;
+ using Map =
+ HashMap<HeapPtr<Key>, HeapPtr<Value>, WeakMapKeyHasher<Key>, AllocPolicy>;
using UnbarrieredMap =
- HashMap<Key, Value, WeakMapKeyHasher<Key>, AllocPolicy>;
+ HashMap<Key, Value, StableCellHasher<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;
@@ -386,32 +276,36 @@ class WeakMap : public WeakMapBase {
[[nodiscard]] bool add(AddPtr& p, const Key& k, const Value& v) {
MOZ_ASSERT(gc::ToMarkable(k));
- writeBarrier(k, v);
+ keyWriteBarrier(k);
return map().add(p, k, v);
}
[[nodiscard]] bool relookupOrAdd(AddPtr& p, const Key& k, const Value& v) {
MOZ_ASSERT(gc::ToMarkable(k));
- writeBarrier(k, v);
+ keyWriteBarrier(k);
return map().relookupOrAdd(p, k, v);
}
[[nodiscard]] bool put(const Key& k, const Value& v) {
MOZ_ASSERT(gc::ToMarkable(k));
- writeBarrier(k, v);
+ keyWriteBarrier(k);
return map().put(k, v);
}
[[nodiscard]] bool putNew(const Key& k, const Value& v) {
MOZ_ASSERT(gc::ToMarkable(k));
- writeBarrier(k, v);
+ keyWriteBarrier(k);
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;
}
@@ -423,15 +317,11 @@ class WeakMap : public WeakMapBase {
}
#endif
- bool markEntry(GCMarker* marker, gc::CellColor mapColor, Enum& iter,
- bool populateWeakKeysTable);
+ bool markEntry(GCMarker* marker, gc::CellColor mapColor, BarrieredKey& key,
+ BarrieredValue& value, 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:
@@ -467,57 +357,84 @@ class WeakMap : public WeakMapBase {
JS::ExposeObjectToActiveJS(obj);
}
- void writeBarrier(const Key& key, const Value& value) {
- keyKindBarrier(key);
- nurseryEntryBarrier(key, value);
- }
-
- void keyKindBarrier(const JS::Value& key) {
- if (key.isSymbol()) {
+ void keyWriteBarrier(const JS::Value& v) {
+ if (v.isSymbol()) {
mayHaveSymbolKeys = true;
}
- if (key.isObject()) {
- keyKindBarrier(&key.toObject());
+ if (v.isObject()) {
+ keyWriteBarrier(&v.toObject());
}
}
- void keyKindBarrier(JSObject* key) {
+ void keyWriteBarrier(JSObject* key) {
JSObject* delegate = UncheckedUnwrapWithoutExpose(key);
if (delegate != key || ObjectMayBeSwapped(key)) {
mayHaveKeyDelegates = true;
}
}
- 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 keyWriteBarrier(BaseScript* key) {}
void traceWeakEdges(JSTracer* trc) override;
void clearAndCompact() override {
- clear();
+ map().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;
- bool traceNurseryEntriesOnMinorGC(JSTracer* trc) override;
+ 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());
+ }
};
+} // 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 */
