diff --git a/js/public/HeapAPI.h b/js/public/HeapAPI.h
index b2fffbd75ecc23e5a5ea563cdccdaccf92fe9b9c..4459b91d7c53b2df747f9cd3058a3a3cca4b8ddc 100644
--- a/js/public/HeapAPI.h
+++ b/js/public/HeapAPI.h
@@ -379,8 +379,6 @@ class JS_FRIEND_API GCCellPtr {
return uintptr_t(p) | (uintptr_t(traceKind) & OutOfLineTraceKindMask);
}
- bool mayBeOwnedByOtherRuntimeSlow() const;
-
JS::TraceKind outOfLineKind() const;
uintptr_t ptr;
diff --git a/js/src/gc/Cell.h b/js/src/gc/Cell.h
index f1a50e25fda39fe20999540419b2cb26fd29ff14..34a333d0540809189dfb0864317a68b72103c1f8 100644
--- a/js/src/gc/Cell.h
+++ b/js/src/gc/Cell.h
@@ -52,6 +52,54 @@ class TenuredCell;
extern bool CurrentThreadIsGCMarking();
#endif
+// Like gc::MarkColor but allows the possibility of the cell being unmarked.
+//
+// This class mimics an enum class, but supports operator overloading.
+class CellColor {
+ public:
+ enum Color { White = 0, Gray = 1, Black = 2 };
+
+ CellColor() : color(White) {}
+
+ MOZ_IMPLICIT CellColor(MarkColor markColor)
+ : color(markColor == MarkColor::Black ? Black : Gray) {}
+
+ MOZ_IMPLICIT constexpr CellColor(Color c) : color(c) {}
+
+ MarkColor asMarkColor() const {
+ MOZ_ASSERT(color != White);
+ return color == Black ? MarkColor::Black : MarkColor::Gray;
+ }
+
+ // Implement a total ordering for CellColor, with white being 'least marked'
+ // and black being 'most marked'.
+ bool operator<(const CellColor other) const { return color < other.color; }
+ bool operator>(const CellColor other) const { return color > other.color; }
+ bool operator<=(const CellColor other) const { return color <= other.color; }
+ bool operator>=(const CellColor other) const { return color >= other.color; }
+ bool operator!=(const CellColor other) const { return color != other.color; }
+ bool operator==(const CellColor other) const { return color == other.color; }
+ explicit operator bool() const { return color != White; }
+
+#if defined(JS_GC_ZEAL) || defined(DEBUG)
+ const char* name() const {
+ switch (color) {
+ case CellColor::White:
+ return "white";
+ case CellColor::Black:
+ return "black";
+ case CellColor::Gray:
+ return "gray";
+ default:
+ MOZ_CRASH("Unexpected cell color");
+ }
+ }
+#endif
+
+ private:
+ Color color;
+};
+
// [SMDOC] GC Cell
//
// A GC cell is the base class for all GC things. All types allocated on the GC
@@ -90,6 +138,12 @@ struct alignas(gc::CellAlignBytes) Cell {
MOZ_ALWAYS_INLINE bool isMarked(gc::MarkColor color) const;
MOZ_ALWAYS_INLINE bool isMarkedAtLeast(gc::MarkColor color) const;
+ MOZ_ALWAYS_INLINE CellColor color() const {
+ return isMarkedBlack()
+ ? CellColor::Black
+ : isMarkedGray() ? CellColor::Gray : CellColor::White;
+ }
+
inline JSRuntime* runtimeFromMainThread() const;
// Note: Unrestricted access to the runtime of a GC thing from an arbitrary
@@ -169,6 +223,13 @@ class TenuredCell : public Cell {
MOZ_ALWAYS_INLINE bool isMarkedBlack() const;
MOZ_ALWAYS_INLINE bool isMarkedGray() const;
+ // Same as Cell::color, but skips nursery checks.
+ MOZ_ALWAYS_INLINE CellColor color() const {
+ return isMarkedBlack()
+ ? CellColor::Black
+ : isMarkedGray() ? CellColor::Gray : CellColor::White;
+ }
+
// The return value indicates if the cell went from unmarked to marked.
MOZ_ALWAYS_INLINE bool markIfUnmarked(
MarkColor color = MarkColor::Black) const;
diff --git a/js/src/gc/GC.cpp b/js/src/gc/GC.cpp
index cfcf076f400635a8a075cde8a0ba2d725a2117a9..f2dd2fc68842fef82c45fb269dae7c8c00cc0c68 100644
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -4300,7 +4300,7 @@ void js::gc::MarkingValidator::nonIncrementalMark(AutoGCSession& session) {
* collecting.
*/
- WeakMapSet markedWeakMaps;
+ WeakMapColors markedWeakMaps;
/*
* For saving, smush all of the keys into one big table and split them back
diff --git a/js/src/gc/GCMarker.h b/js/src/gc/GCMarker.h
index af59b1253486c3a78e5428cce9e33adb86e962c8..5993e44348291f11ab147fc15331f005e4086498 100644
--- a/js/src/gc/GCMarker.h
+++ b/js/src/gc/GCMarker.h
@@ -284,21 +284,6 @@ class GCMarker : public JSTracer {
// must be empty when this is called.
void setMainStackColor(gc::MarkColor newColor);
- // Return whether a cell is marked relative to the current marking color. If
- // the cell is black then this returns true, but if it's gray it will return
- // false if the mark color is black.
- template <typename T>
- bool isMarked(T* thingp) {
- return color == gc::MarkColor::Black ? gc::IsMarkedBlack(runtime(), thingp)
- : gc::IsMarked(runtime(), thingp);
- }
- template <typename T>
- bool isMarkedUnbarriered(T* thingp) {
- return color == gc::MarkColor::Black
- ? gc::IsMarkedBlackUnbarriered(runtime(), thingp)
- : gc::IsMarkedUnbarriered(runtime(), thingp);
- }
-
void enterWeakMarkingMode();
void leaveWeakMarkingMode();
void abortLinearWeakMarking() {
@@ -513,6 +498,9 @@ class MOZ_RAII AutoSetMarkColor {
marker_.setMarkColor(newColor);
}
+ AutoSetMarkColor(GCMarker& marker, CellColor newColor)
+ : AutoSetMarkColor(marker, newColor.asMarkColor()) {}
+
~AutoSetMarkColor() { marker_.setMarkColor(initialColor_); }
};
diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp
index dcaa6e8ae09161717e4c82ab7358a527ddc53cc1..370051f0ddf29bf9cc2c45a2c011340b7faa4ce7 100644
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -12,6 +12,7 @@
#include "mozilla/ReentrancyGuard.h"
#include "mozilla/ScopeExit.h"
#include "mozilla/TypeTraits.h"
+#include "mozilla/Unused.h"
#include <algorithm>
@@ -41,6 +42,7 @@
#include "gc/GC-inl.h"
#include "gc/Nursery-inl.h"
#include "gc/PrivateIterators-inl.h"
+#include "gc/WeakMap-inl.h"
#include "gc/Zone-inl.h"
#include "vm/GeckoProfiler-inl.h"
#include "vm/NativeObject-inl.h"
@@ -654,12 +656,7 @@ void GCMarker::markEphemeronValues(gc::Cell* markedCell,
DebugOnly<size_t> initialLen = values.length();
for (const auto& markable : values) {
- if (color == gc::MarkColor::Black &&
- markable.weakmap->markColor == gc::MarkColor::Gray) {
- continue;
- }
-
- markable.weakmap->markEntry(this, markedCell, markable.key);
+ markable.weakmap->markKey(this, markedCell, markable.key);
}
// The vector should not be appended to during iteration because the key is
@@ -2607,8 +2604,8 @@ void GCMarker::enterWeakMarkingMode() {
for (SweepGroupZonesIter zone(runtime()); !zone.done(); zone.next()) {
for (WeakMapBase* m : zone->gcWeakMapList()) {
- if (m->marked) {
- (void)m->markEntries(this);
+ if (m->mapColor) {
+ mozilla::Unused << m->markEntries(this);
}
}
}
diff --git a/js/src/gc/Verifier.cpp b/js/src/gc/Verifier.cpp
index 7a18709c0d2eefa6418fc84f8088f2af6f0f4bc1..07cf27f47f03ad2362cad0964b7dc1243da62ff5 100644
--- a/js/src/gc/Verifier.cpp
+++ b/js/src/gc/Verifier.cpp
@@ -10,6 +10,8 @@
#include "mozilla/IntegerPrintfMacros.h"
#include "mozilla/Sprintf.h"
+#include <algorithm>
+
#ifdef MOZ_VALGRIND
# include <valgrind/memcheck.h>
#endif
@@ -453,23 +455,6 @@ void js::gc::GCRuntime::finishVerifier() {
#if defined(JS_GC_ZEAL) || defined(DEBUG)
-static const char* CellColorName(CellColor color) {
- switch (color) {
- case CellColor::White:
- return "white";
- case CellColor::Black:
- return "black";
- case CellColor::Gray:
- return "gray";
- default:
- MOZ_CRASH("Unexpected cell color");
- }
-}
-
-static const char* GetCellColorName(Cell* cell) {
- return CellColorName(GetCellColor(cell));
-}
-
class HeapCheckTracerBase : public JS::CallbackTracer {
public:
explicit HeapCheckTracerBase(JSRuntime* rt, WeakMapTraceKind weakTraceKind);
@@ -587,7 +572,7 @@ void HeapCheckTracerBase::dumpCellInfo(Cell* cell) {
JSObject* obj =
kind == JS::TraceKind::Object ? static_cast<JSObject*>(cell) : nullptr;
- fprintf(stderr, "%s %s", GetCellColorName(cell), GCTraceKindToAscii(kind));
+ fprintf(stderr, "%s %s", cell->color().name(), GCTraceKindToAscii(kind));
if (obj) {
fprintf(stderr, " %s", obj->getClass()->name);
}
@@ -769,31 +754,26 @@ bool js::gc::CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key,
Zone* valueZone = GetCellZoneFromAnyThread(value);
MOZ_ASSERT(valueZone == zone || valueZone->isAtomsZone());
- CellColor mapColor =
- map->markColor == MarkColor::Black ? CellColor::Black : CellColor::Gray;
- if (object && GetCellColor(object) != mapColor) {
+ if (object && object->color() != map->mapColor) {
fprintf(stderr, "WeakMap object is marked differently to the map\n");
fprintf(stderr, "(map %p is %s, object %p is %s)\n", map,
- CellColorName(mapColor), object,
- CellColorName(GetCellColor(object)));
+ map->mapColor.name(), object, object->color().name());
ok = false;
}
- CellColor keyColor = GetCellColor(key);
-
// Values belonging to other runtimes or in uncollected zones are treated as
// black.
CellColor valueColor = CellColor::Black;
if (value->runtimeFromAnyThread() == zone->runtimeFromAnyThread() &&
valueZone->isGCMarking()) {
- valueColor = GetCellColor(value);
+ valueColor = value->color();
}
- if (valueColor < ExpectedWeakMapValueColor(mapColor, keyColor)) {
+ if (valueColor < std::min(map->mapColor, key->color())) {
fprintf(stderr, "WeakMap value is less marked than map and key\n");
fprintf(stderr, "(map %p is %s, key %p is %s, value %p is %s)\n", map,
- CellColorName(mapColor), key, CellColorName(keyColor), value,
- CellColorName(valueColor));
+ map->mapColor.name(), key, key->color().name(), value,
+ valueColor.name());
ok = false;
}
@@ -812,17 +792,17 @@ bool js::gc::CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key,
CellColor delegateColor;
if (delegate->zone()->isGCMarking() || delegate->zone()->isGCSweeping()) {
- delegateColor = GetCellColor(delegate);
+ delegateColor = delegate->color();
} else {
// IsMarked() assumes cells in uncollected zones are marked.
delegateColor = CellColor::Black;
}
- if (keyColor < ExpectedWeakMapKeyColor(mapColor, delegateColor)) {
- fprintf(stderr, "WeakMap key is less marked than map and delegate\n");
+ if (key->color() < std::min(map->mapColor, delegateColor)) {
+ fprintf(stderr, "WeakMap key is less marked than map or delegate\n");
fprintf(stderr, "(map %p is %s, delegate %p is %s, key %p is %s)\n", map,
- CellColorName(mapColor), delegate, CellColorName(delegateColor),
- key, CellColorName(keyColor));
+ map->mapColor.name(), delegate, delegateColor.name(), key,
+ key->color().name());
ok = false;
}
diff --git a/js/src/gc/Verifier.h b/js/src/gc/Verifier.h
index 537244f06eb6eb8f444fc89d08e51494f73fde34..0e7df3dc15b86c7a4c43345f8caf44338b9bfb2b 100644
--- a/js/src/gc/Verifier.h
+++ b/js/src/gc/Verifier.h
@@ -18,47 +18,12 @@
namespace js {
namespace gc {
-// Like gc::MarkColor but allows the possibility of the cell being
-// unmarked.
-enum class CellColor : uint8_t {
- White = 0,
- Gray = uint8_t(MarkColor::Gray),
- Black = uint8_t(MarkColor::Black)
-};
-
static constexpr CellColor AllCellColors[] = {CellColor::White, CellColor::Gray,
CellColor::Black};
static constexpr CellColor MarkedCellColors[] = {CellColor::Gray,
CellColor::Black};
-inline CellColor GetCellColor(Cell* cell) {
- if (cell->isMarkedBlack()) {
- return CellColor::Black;
- }
-
- if (cell->isMarkedGray()) {
- return CellColor::Gray;
- }
-
- return CellColor::White;
-}
-
-inline CellColor ExpectedWeakMapValueColor(CellColor keyColor,
- CellColor mapColor) {
- return std::min(keyColor, mapColor);
-}
-
-inline CellColor ExpectedWeakMapKeyColor(CellColor mapColor,
- CellColor delegateColor) {
- return std::min(mapColor, delegateColor);
-}
-
-inline CellColor ExpectedKeyAndDelegateColor(CellColor keyColor,
- CellColor delegateColor) {
- return std::max(keyColor, delegateColor);
-}
-
} // namespace gc
} // namespace js
diff --git a/js/src/gc/WeakMap-inl.h b/js/src/gc/WeakMap-inl.h
index e4396600abbdce628cb0086743554348dfa3bc21..15d2208f510531e9946ed30e0dd20d7a3e52dfd8 100644
--- a/js/src/gc/WeakMap-inl.h
+++ b/js/src/gc/WeakMap-inl.h
@@ -9,38 +9,79 @@
#include "gc/WeakMap.h"
+#include "mozilla/DebugOnly.h"
+#include "mozilla/Unused.h"
+
+#include <algorithm>
+
#include "gc/Zone.h"
#include "js/TraceKind.h"
#include "vm/JSContext.h"
namespace js {
+namespace gc {
+
+// Specializations for barriered types.
+template <typename T>
+inline Cell* ToMarkable(WriteBarriered<T>* thingp) {
+ return ToMarkable(thingp->get());
+}
+
+namespace detail {
template <typename T>
-static T extractUnbarriered(const WriteBarriered<T>& v) {
+static T ExtractUnbarriered(const WriteBarriered<T>& v) {
return v.get();
}
template <typename T>
-static T* extractUnbarriered(T* v) {
+static T* ExtractUnbarriered(T* v) {
return v;
}
-inline /* static */ JSObject* WeakMapBase::getDelegate(JSObject* key) {
- if (!IsWrapper(key)) {
- return nullptr;
+// Return the effective cell color given the current marking state.
+// This must be kept in sync with ShouldMark in Marking.cpp.
+template <typename T>
+static CellColor GetEffectiveColor(JSRuntime* rt, const T& item) {
+ Cell* cell = ToMarkable(item);
+ if (!cell->isTenured()) {
+ return CellColor::Black;
}
-
- return UncheckedUnwrapWithoutExpose(key);
+ const TenuredCell& t = cell->asTenured();
+ if (rt != t.runtimeFromAnyThread()) {
+ return CellColor::Black;
+ }
+ if (!t.zoneFromAnyThread()->shouldMarkInZone()) {
+ return CellColor::Black;
+ }
+ return cell->color();
}
-inline /* static */ JSObject* WeakMapBase::getDelegate(JSScript* script) {
+// Only objects have delegates, so default to returning nullptr. Note that some
+// compilation units will only ever use the object version.
+static MOZ_MAYBE_UNUSED JSObject* GetDelegateInternal(gc::Cell* key) {
return nullptr;
}
-inline /* static */ JSObject* WeakMapBase::getDelegate(LazyScript* script) {
- return nullptr;
+static JSObject* GetDelegateInternal(JSObject* key) {
+ JSObject* delegate = UncheckedUnwrapWithoutExpose(key);
+ return (key == delegate) ? nullptr : delegate;
}
+// Use a helper function to do overload resolution to handle cases like
+// Heap<ObjectSubclass*>: find everything that is convertible to JSObject* (and
+// avoid calling barriers).
+template <typename T>
+static inline JSObject* GetDelegate(const T& key) {
+ return GetDelegateInternal(ExtractUnbarriered(key));
+}
+
+template <>
+inline JSObject* GetDelegate(gc::Cell* const&) = delete;
+
+} /* namespace detail */
+} /* namespace gc */
+
template <class K, class V>
WeakMap<K, V>::WeakMap(JSContext* cx, JSObject* memOf)
: Base(cx->zone()), WeakMapBase(memOf, cx->zone()) {
@@ -56,37 +97,64 @@ WeakMap<K, V>::WeakMap(JSContext* cx, JSObject* memOf)
zone()->gcWeakMapList().insertFront(this);
if (zone()->wasGCStarted()) {
- marked = true;
- markColor = gc::MarkColor::Black;
+ mapColor = CellColor::Black;
}
}
// Trace a WeakMap entry based on 'markedCell' getting marked, where 'origKey'
-// is the key in the weakmap. These will probably be the same, but can be
-// different eg when markedCell is a delegate for origKey.
-//
-// This implementation does not use 'markedCell'; it looks up origKey and checks
-// the mark bits on everything it cares about, one of which will be
-// markedCell. But a subclass might use it to optimize the liveness check.
+// is the key in the weakmap. In the absence of delegates, these will be the
+// same, but when a delegate is marked then origKey will be its wrapper.
+// `markedCell` is only used for an assertion.
template <class K, class V>
-void WeakMap<K, V>::markEntry(GCMarker* marker, gc::Cell* markedCell,
- gc::Cell* origKey) {
- MOZ_ASSERT(marked);
+void WeakMap<K, V>::markKey(GCMarker* marker, gc::Cell* markedCell,
+ gc::Cell* origKey) {
+ MOZ_ASSERT(mapColor);
Ptr p = Base::lookup(static_cast<Lookup>(origKey));
MOZ_ASSERT(p.found());
- K key(p->key());
- MOZ_ASSERT((markedCell == extractUnbarriered(key)) ||
- (markedCell == getDelegate(key)));
- if (marker->isMarked(&key)) {
- TraceEdge(marker, &p->value(), "ephemeron value");
- } else if (keyNeedsMark(marker, key)) {
- TraceEdge(marker, &p->value(), "WeakMap ephemeron value");
- TraceEdge(marker, &key, "proxy-preserved WeakMap ephemeron key");
- MOZ_ASSERT(key == p->key()); // No moving
+ mozilla::DebugOnly<gc::Cell*> oldKey = gc::ToMarkable(p->key());
+ MOZ_ASSERT((markedCell == oldKey) ||
+ (markedCell == gc::detail::GetDelegate(p->key())));
+
+ markEntry(marker, p->mutableKey(), p->value());
+ MOZ_ASSERT(oldKey == gc::ToMarkable(p->key()), "no moving GC");
+}
+
+// If the entry is live, ensure its key and value are marked. Also make sure
+// the key is at least as marked as the delegate, so it cannot get discarded
+// and then recreated by rewrapping the delegate.
+template <class K, class V>
+bool WeakMap<K, V>::markEntry(GCMarker* marker, K& key, V& value) {
+ bool marked = false;
+ JSRuntime* rt = zone()->runtimeFromAnyThread();
+ CellColor keyColor = gc::detail::GetEffectiveColor(rt, key);
+ JSObject* delegate = gc::detail::GetDelegate(key);
+ if (delegate) {
+ CellColor delegateColor = gc::detail::GetEffectiveColor(rt, delegate);
+ if (keyColor < delegateColor) {
+ gc::AutoSetMarkColor autoColor(*marker, delegateColor);
+ TraceEdge(marker, &key, "proxy-preserved WeakMap entry key");
+ MOZ_ASSERT(key->color() >= delegateColor);
+ marked = true;
+ keyColor = delegateColor;
+ }
+ }
+
+ if (keyColor) {
+ gc::Cell* cellValue = gc::ToMarkable(&value);
+ if (cellValue) {
+ gc::AutoSetMarkColor autoColor(*marker, std::min(mapColor, keyColor));
+ CellColor valueColor = gc::detail::GetEffectiveColor(rt, cellValue);
+ if (valueColor < marker->markColor()) {
+ TraceEdge(marker, &value, "WeakMap entry value");
+ MOZ_ASSERT(cellValue->color() >= std::min(mapColor, keyColor));
+ marked = true;
+ }
+ }
}
- key.unsafeSet(nullptr); // Prevent destructor from running barriers.
+
+ return marked;
}
template <class K, class V>
@@ -99,17 +167,13 @@ void WeakMap<K, V>::trace(JSTracer* trc) {
MOZ_ASSERT(trc->weakMapAction() == ExpandWeakMaps);
auto marker = GCMarker::fromTracer(trc);
- // Don't change the map color from black to gray. This can happen when a
- // barrier pushes the map object onto the black mark stack when it's already
- // present on the gray mark stack, which is marked later.
- if (marked && markColor == gc::MarkColor::Black &&
- marker->markColor() == gc::MarkColor::Gray) {
- return;
+ // Don't downgrade the map color from black to gray. This can happen when a
+ // barrier pushes the map object onto the black mark stack when it's
+ // already present on the gray mark stack, which is marked later.
+ if (mapColor < marker->markColor()) {
+ mapColor = marker->markColor();
+ mozilla::Unused << markEntries(marker);
}
-
- marked = true;
- markColor = marker->markColor();
- (void)markEntries(marker);
return;
}
@@ -154,38 +218,32 @@ template <class K, class V>
template <class K, class V>
bool WeakMap<K, V>::markEntries(GCMarker* marker) {
- MOZ_ASSERT(marked);
- if (marker->markColor() == gc::MarkColor::Black &&
- markColor == gc::MarkColor::Gray) {
- return false;
- }
-
+ MOZ_ASSERT(mapColor);
bool markedAny = false;
for (Enum e(*this); !e.empty(); e.popFront()) {
- // If the entry is live, ensure its key and value are marked.
- bool keyIsMarked = marker->isMarked(&e.front().mutableKey());
- if (!keyIsMarked && keyNeedsMark(marker, e.front().key())) {
- TraceEdge(marker, &e.front().mutableKey(),
- "proxy-preserved WeakMap entry key");
- keyIsMarked = true;
+ if (markEntry(marker, e.front().mutableKey(), e.front().value())) {
markedAny = true;
}
- if (keyIsMarked) {
- if (!marker->isMarked(&e.front().value())) {
- TraceEdge(marker, &e.front().value(), "WeakMap entry value");
- markedAny = true;
- }
- } else if (marker->isWeakMarkingTracer()) {
- // Entry is not yet known to be live. Record this weakmap and
- // the lookup key in the list of weak keys. Also record the
- // delegate, if any, because marking the delegate also marks
- // the entry.
- gc::Cell* weakKey = extractUnbarriered(e.front().key());
+ JSRuntime* rt = zone()->runtimeFromAnyThread();
+ CellColor keyColor =
+ gc::detail::GetEffectiveColor(rt, e.front().key().get());
+
+ // Changes in the map's mark color will be handled in this code, but
+ // changes in the key's mark color are handled through the weak keys table.
+ // So we only need to populate the table if the key is less marked than the
+ // map, to catch later updates in the key's mark color.
+ if (keyColor < mapColor) {
+ MOZ_ASSERT(marker->weakMapAction() == ExpandWeakMaps);
+ // The final color of the key is not yet known. Record this weakmap and
+ // the lookup key in the list of weak keys. If the key has a delegate,
+ // then the lookup key is the delegate (because marking the key will end
+ // up marking the delegate and thereby mark the entry.)
+ gc::Cell* weakKey = gc::detail::ExtractUnbarriered(e.front().key());
gc::WeakMarkable markable(this, weakKey);
addWeakEntry(marker, weakKey, markable);
- if (JSObject* delegate = getDelegate(e.front().key())) {
+ if (JSObject* delegate = gc::detail::GetDelegate(e.front().key())) {
addWeakEntry(marker, delegate, markable);
}
}
@@ -194,28 +252,6 @@ bool WeakMap<K, V>::markEntries(GCMarker* marker) {
return markedAny;
}
-template <class K, class V>
-inline bool WeakMap<K, V>::keyNeedsMark(GCMarker* marker, JSObject* key) const {
- JSObject* delegate = getDelegate(key);
- /*
- * Check if the delegate is marked with any color to properly handle
- * gray marking when the key's delegate is black and the map is gray.
- */
- return delegate && marker->isMarkedUnbarriered(&delegate);
-}
-
-template <class K, class V>
-inline bool WeakMap<K, V>::keyNeedsMark(GCMarker* marker,
- JSScript* script) const {
- return false;
-}
-
-template <class K, class V>
-inline bool WeakMap<K, V>::keyNeedsMark(GCMarker* marker,
- LazyScript* script) const {
- return false;
-}
-
template <class K, class V>
void WeakMap<K, V>::sweep() {
/* Remove all entries whose keys remain unmarked. */
@@ -251,7 +287,7 @@ void WeakMap<K, V>::assertEntriesNotAboutToBeFinalized() {
for (Range r = Base::all(); !r.empty(); r.popFront()) {
K k(r.front().key());
MOZ_ASSERT(!gc::IsAboutToBeFinalized(&k));
- JSObject* delegate = getDelegate(k);
+ JSObject* delegate = gc::detail::GetDelegate(k);
if (delegate) {
MOZ_ASSERT(!gc::IsAboutToBeFinalizedUnbarriered(&delegate),
"weakmap marking depends on a key tracing its delegate");
diff --git a/js/src/gc/WeakMap.cpp b/js/src/gc/WeakMap.cpp
index 4659aa68363e1957251fbc14f28ceb242a3ef612..c9637ae9e56aff5e06c67a45052948a7b573ab24 100644
--- a/js/src/gc/WeakMap.cpp
+++ b/js/src/gc/WeakMap.cpp
@@ -23,7 +23,7 @@ using namespace js;
using namespace js::gc;
WeakMapBase::WeakMapBase(JSObject* memOf, Zone* zone)
- : memberOf(memOf), zone_(zone), marked(false), markColor(MarkColor::Black) {
+ : memberOf(memOf), zone_(zone), mapColor(CellColor::White) {
MOZ_ASSERT_IF(memberOf, memberOf->compartment()->zone() == zone);
}
@@ -33,7 +33,7 @@ WeakMapBase::~WeakMapBase() {
void WeakMapBase::unmarkZone(JS::Zone* zone) {
for (WeakMapBase* m : zone->gcWeakMapList()) {
- m->marked = false;
+ m->mapColor = CellColor::White;
}
}
@@ -52,7 +52,7 @@ bool WeakMapBase::checkMarkingForZone(JS::Zone* zone) {
bool ok = true;
for (WeakMapBase* m : zone->gcWeakMapList()) {
- if (m->marked && !m->checkMarking()) {
+ if (m->mapColor && !m->checkMarking()) {
ok = false;
}
}
@@ -64,7 +64,7 @@ bool WeakMapBase::checkMarkingForZone(JS::Zone* zone) {
bool WeakMapBase::markZoneIteratively(JS::Zone* zone, GCMarker* marker) {
bool markedAny = false;
for (WeakMapBase* m : zone->gcWeakMapList()) {
- if (m->marked && m->markEntries(marker)) {
+ if (m->mapColor && m->markEntries(marker)) {
markedAny = true;
}
}
@@ -83,7 +83,7 @@ bool WeakMapBase::findSweepGroupEdgesForZone(JS::Zone* zone) {
void WeakMapBase::sweepZone(JS::Zone* zone) {
for (WeakMapBase* m = zone->gcWeakMapList().getFirst(); m;) {
WeakMapBase* next = m->getNext();
- if (m->marked) {
+ if (m->mapColor) {
m->sweep();
} else {
m->clearAndCompact();
@@ -94,7 +94,7 @@ void WeakMapBase::sweepZone(JS::Zone* zone) {
#ifdef DEBUG
for (WeakMapBase* m : zone->gcWeakMapList()) {
- MOZ_ASSERT(m->isInList() && m->marked);
+ MOZ_ASSERT(m->isInList() && m->mapColor);
}
#endif
}
@@ -111,21 +111,22 @@ void WeakMapBase::traceAllMappings(WeakMapTracer* tracer) {
}
bool WeakMapBase::saveZoneMarkedWeakMaps(JS::Zone* zone,
- WeakMapSet& markedWeakMaps) {
+ WeakMapColors& markedWeakMaps) {
for (WeakMapBase* m : zone->gcWeakMapList()) {
- if (m->marked && !markedWeakMaps.put(m)) {
+ if (m->mapColor && !markedWeakMaps.put(m, m->mapColor)) {
return false;
}
}
return true;
}
-void WeakMapBase::restoreMarkedWeakMaps(WeakMapSet& markedWeakMaps) {
- for (WeakMapSet::Range r = markedWeakMaps.all(); !r.empty(); r.popFront()) {
- WeakMapBase* map = r.front();
+void WeakMapBase::restoreMarkedWeakMaps(WeakMapColors& markedWeakMaps) {
+ for (WeakMapColors::Range r = markedWeakMaps.all(); !r.empty();
+ r.popFront()) {
+ WeakMapBase* map = r.front().key();
MOZ_ASSERT(map->zone()->isGCMarking());
- MOZ_ASSERT(!map->marked);
- map->marked = true;
+ MOZ_ASSERT(map->mapColor == CellColor::White);
+ map->mapColor = r.front().value();
}
}
@@ -146,7 +147,7 @@ bool ObjectValueWeakMap::findSweepGroupEdges() {
if (key->asTenured().isMarkedBlack()) {
continue;
}
- JSObject* delegate = getDelegate(key);
+ JSObject* delegate = gc::detail::GetDelegate(key);
if (!delegate) {
continue;
}
diff --git a/js/src/gc/WeakMap.h b/js/src/gc/WeakMap.h
index 3e4b911d09abbb1ab923349c2ddca55d006ae131..02517e0d00ab5ae6c53b423d3f77b7405fdf654a 100644
--- a/js/src/gc/WeakMap.h
+++ b/js/src/gc/WeakMap.h
@@ -47,8 +47,8 @@ bool CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key, Cell* value);
// the implicit edges stored in the map) and of removing (sweeping) table
// entries when collection is complete.
-typedef HashSet<WeakMapBase*, DefaultHasher<WeakMapBase*>, SystemAllocPolicy>
- WeakMapSet;
+using WeakMapColors = HashMap<WeakMapBase*, js::gc::CellColor,
+ DefaultHasher<WeakMapBase*>, SystemAllocPolicy>;
// Common base class for all WeakMap specializations, used for calling
// subclasses' GC-related methods.
@@ -56,6 +56,8 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
friend class js::GCMarker;
public:
+ using CellColor = js::gc::CellColor;
+
WeakMapBase(JSObject* memOf, JS::Zone* zone);
virtual ~WeakMapBase();
@@ -88,19 +90,15 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
// Save information about which weak maps are marked for a zone.
static bool saveZoneMarkedWeakMaps(JS::Zone* zone,
- WeakMapSet& markedWeakMaps);
+ WeakMapColors& markedWeakMaps);
// Restore information about which weak maps are marked for many zones.
- static void restoreMarkedWeakMaps(WeakMapSet& markedWeakMaps);
+ static void restoreMarkedWeakMaps(WeakMapColors& markedWeakMaps);
#if defined(JS_GC_ZEAL) || defined(DEBUG)
static bool checkMarkingForZone(JS::Zone* zone);
#endif
- static JSObject* getDelegate(JSObject* key);
- static JSObject* getDelegate(JSScript* script);
- static JSObject* getDelegate(LazyScript* script);
-
protected:
// Instance member functions called by the above. Instantiations of WeakMap
// override these with definitions appropriate for their Key and Value types.
@@ -112,8 +110,7 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
// Any weakmap key types that want to participate in the non-iterative
// ephemeron marking must override this method.
- virtual void markEntry(GCMarker* marker, gc::Cell* markedCell,
- gc::Cell* l) = 0;
+ virtual void markKey(GCMarker* marker, gc::Cell* markedCell, gc::Cell* l) = 0;
virtual bool markEntries(GCMarker* marker) = 0;
@@ -132,10 +129,21 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
// Whether this object has been marked during garbage collection and which
// color it was marked.
- bool marked;
- gc::MarkColor markColor;
+ gc::CellColor mapColor;
+};
+
+namespace detail {
+
+template <typename T>
+struct RemoveBarrier {};
+
+template <typename T>
+struct RemoveBarrier<js::HeapPtr<T>> {
+ using Type = T;
};
+} // namespace detail
+
template <class Key, class Value>
class WeakMap
: private HashMap<Key, Value, MovableCellHasher<Key>, ZoneAllocPolicy>,
@@ -161,6 +169,8 @@ class WeakMap
// Resolve ambiguity with LinkedListElement<>::remove.
using Base::remove;
+ using UnbarrieredKey = typename detail::RemoveBarrier<Key>::Type;
+
explicit WeakMap(JSContext* cx, JSObject* memOf = nullptr);
// Add a read barrier to prevent an incorrectly gray value from escaping the
@@ -211,8 +221,10 @@ class WeakMap
}
#endif
- void markEntry(GCMarker* marker, gc::Cell* markedCell,
- gc::Cell* origKey) override;
+ void markKey(GCMarker* marker, gc::Cell* markedCell,
+ gc::Cell* origKey) override;
+
+ bool markEntry(GCMarker* marker, Key& key, Value& value);
void trace(JSTracer* trc) override;
@@ -242,10 +254,6 @@ class WeakMap
JS::ExposeObjectToActiveJS(obj);
}
- bool keyNeedsMark(GCMarker* marker, JSObject* key) const;
- bool keyNeedsMark(GCMarker* marker, JSScript* script) const;
- bool keyNeedsMark(GCMarker* marker, LazyScript* script) const;
-
bool findSweepGroupEdges() override {
// This is overridden by ObjectValueWeakMap and DebuggerWeakMap.
return true;
diff --git a/js/src/gc/Zone.cpp b/js/src/gc/Zone.cpp
index d40ef25df1c248073a0cc3774d4c783b4c241ec4..7fc299680e101aa822319bf01724b858de72b3f3 100644
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -228,7 +228,7 @@ void Zone::sweepWeakKeysAfterMinorGC() {
// If the key has a delegate, then it will map to a WeakKeyEntryVector
// containing the key that needs to be updated.
- JSObject* delegate = WeakMapBase::getDelegate(key->as<JSObject>());
+ JSObject* delegate = gc::detail::GetDelegate(key->as<JSObject>());
if (!delegate) {
continue;
}
diff --git a/js/src/jit-test/tests/gc/weak-marking-03.js b/js/src/jit-test/tests/gc/weak-marking-03.js
index 7ae58181fe843cfb13b090042a6f8ebb973ed8f4..3ef6b32a6739a3b476980d324a65e672751f01cc 100644
--- a/js/src/jit-test/tests/gc/weak-marking-03.js
+++ b/js/src/jit-test/tests/gc/weak-marking-03.js
@@ -412,6 +412,7 @@ function grayMapKey() {
"key is now marked black");
finishgc(); // Finish the GC
+
assertEq(getMarks().join("/"), "gray/black/gray",
"at end: black/gray => gray");
@@ -483,3 +484,83 @@ function grayKeyMap() {
if (this.enqueueMark)
runtest(grayKeyMap);
+
+// Cause a key to be marked black *during gray marking*, by first marking a
+// delegate black, then marking the map and key gray. When the key is scanned,
+// it should be seen to be a CCW of a black delegate and so should itself be
+// marked black.
+//
+// Note that this is currently buggy -- the key will be marked because the
+// delegate is marked, but the color won't be taken into account. So the key
+// will be marked gray (or rather, it will see that it's already gray.) The bad
+// behavior this would cause is if:
+//
+// 1. You wrap an object in a CCW and use it as a weakmap key to some
+// information.
+// 2. You keep a strong reference to the object (in its compartment).
+// 3. The only references to the CCW are gray, and are in fact part of a cycle.
+// 4. The CC runs and discards the CCW.
+// 5. You look up the object in the weakmap again. This creates a new wrapper
+// to use as a key. It is not in the weakmap, so the information you stored
+// before is not found. (It may have even been collected, if you had no
+// other references to it.)
+//
+function blackDuringGray() {
+ const g = newGlobal({newCompartment: true});
+ const vals = {};
+ vals.map = new WeakMap();
+ vals.key = g.eval("Object.create(null)");
+ vals.val = Object.create(null);
+ vals.map.set(vals.key, vals.val);
+
+ g.delegate = vals.key;
+ addMarkObservers([vals.map, vals.key]);
+ g.addMarkObservers([vals.key]);
+ addMarkObservers([vals.val]);
+ // Mark observers: map, key, delegate, value
+
+ gc();
+
+ g.enqueueMark(vals.key); // Mark the delegate black
+ enqueueMark("yield"); // checkpoint 1
+
+ // Mark the map gray. This will scan through all entries, find our key, and
+ // mark it black because its delegate is black.
+ enqueueMark("set-color-gray");
+ enqueueMark(vals.map); // Mark the map gray
+
+ vals.map = null;
+ vals.val = null;
+ vals.key = null;
+ g.delegate = null;
+
+ const showmarks = () => {
+ print("[map,key,delegate,value] marked " + JSON.stringify(getMarks()));
+ };
+
+ print("Starting incremental GC");
+ startgc(100000);
+ // Checkpoint 1, after marking delegate black
+ showmarks();
+ var marks = getMarks();
+ assertEq(marks[0], "unmarked", "map is not marked yet");
+ assertEq(marks[1], "unmarked", "key is not marked yet");
+ assertEq(marks[2], "black", "delegate is black");
+ assertEq(marks[3], "unmarked", "values is not marked yet");
+
+ finishgc();
+ showmarks();
+ marks = getMarks();
+ assertEq(marks[0], "gray", "map is gray");
+ assertEq(marks[1], "black", "key inherits black even though in gray marking");
+ assertEq(marks[2], "black", "delegate is still black");
+ assertEq(marks[3], "gray", "gray map + black key => gray value");
+
+ clearMarkQueue();
+ clearMarkObservers();
+ grayRoot().length = 0;
+ g.eval("grayRoot().length = 0");
+}
+
+if (this.enqueueMark)
+ runtest(blackDuringGray);
diff --git a/js/src/jsapi-tests/testGCGrayMarking.cpp b/js/src/jsapi-tests/testGCGrayMarking.cpp
index 59c768c5a3d597a7936fa173f9f5e02bb387a9ba..090d151754a9633b2c96e3b48147d3b7b23e85e1 100644
--- a/js/src/jsapi-tests/testGCGrayMarking.cpp
+++ b/js/src/jsapi-tests/testGCGrayMarking.cpp
@@ -5,6 +5,8 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+#include <algorithm>
+
#include "gc/Heap.h"
#include "gc/Verifier.h"
#include "gc/WeakMap.h"
@@ -155,7 +157,7 @@ bool TestMarking() {
return true;
}
-static const CellColor DontMark = CellColor::White;
+static constexpr CellColor DontMark = CellColor::White;
enum MarkKeyOrDelegate : bool { MarkKey = true, MarkDelegate = false };
@@ -163,8 +165,7 @@ bool TestJSWeakMaps() {
for (auto keyOrDelegateColor : MarkedCellColors) {
for (auto mapColor : MarkedCellColors) {
for (auto markKeyOrDelegate : {MarkKey, MarkDelegate}) {
- CellColor expected =
- ExpectedWeakMapValueColor(keyOrDelegateColor, mapColor);
+ CellColor expected = std::min(keyOrDelegateColor, mapColor);
CHECK(TestJSWeakMap(markKeyOrDelegate, keyOrDelegateColor, mapColor,
expected));
#ifdef JS_GC_ZEAL
@@ -186,10 +187,10 @@ bool TestInternalWeakMaps() {
continue;
}
- CellColor keyOrDelegateColor =
- ExpectedKeyAndDelegateColor(keyMarkColor, delegateMarkColor);
- CellColor expected =
- ExpectedWeakMapValueColor(keyOrDelegateColor, CellColor::Black);
+ // The map is black. The delegate marks its key via wrapper preservation.
+ // The key maps its delegate and the value. Thus, all three end up the
+ // maximum of the key and delegate colors.
+ CellColor expected = std::max(keyMarkColor, delegateMarkColor);
CHECK(TestInternalWeakMap(keyMarkColor, delegateMarkColor, expected));
#ifdef JS_GC_ZEAL
@@ -240,9 +241,9 @@ bool TestJSWeakMap(MarkKeyOrDelegate markKey, CellColor weakMapMarkColor,
ClearGrayRoots();
- CHECK(GetCellColor(weakMap) == weakMapMarkColor);
- CHECK(GetCellColor(keyOrDelegate) == keyOrDelegateMarkColor);
- CHECK(GetCellColor(value) == expectedValueColor);
+ CHECK(weakMap->color() == weakMapMarkColor);
+ CHECK(keyOrDelegate->color() == keyOrDelegateMarkColor);
+ CHECK(value->color() == expectedValueColor);
}
return true;
@@ -296,9 +297,9 @@ bool TestJSWeakMapWithGrayUnmarking(MarkKeyOrDelegate markKey,
ClearGrayRoots();
- CHECK(GetCellColor(weakMap) == weakMapMarkColor);
- CHECK(GetCellColor(keyOrDelegate) == keyOrDelegateMarkColor);
- CHECK(GetCellColor(value) == expectedValueColor);
+ CHECK(weakMap->color() == weakMapMarkColor);
+ CHECK(keyOrDelegate->color() == keyOrDelegateMarkColor);
+ CHECK(value->color() == expectedValueColor);
}
JS_UnsetGCZeal(cx, uint8_t(ZealMode::YieldWhileGrayMarking));
@@ -369,9 +370,9 @@ bool TestInternalWeakMap(CellColor keyMarkColor, CellColor delegateMarkColor,
ClearGrayRoots();
- CHECK(GetCellColor(key) == expectedColor);
- CHECK(GetCellColor(delegate) == expectedColor);
- CHECK(GetCellColor(value) == expectedColor);
+ CHECK(key->color() == expectedColor);
+ CHECK(delegate->color() == expectedColor);
+ CHECK(value->color() == expectedColor);
}
return true;
@@ -421,9 +422,9 @@ bool TestInternalWeakMapWithGrayUnmarking(CellColor keyMarkColor,
ClearGrayRoots();
- CHECK(GetCellColor(key) == expectedColor);
- CHECK(GetCellColor(delegate) == expectedColor);
- CHECK(GetCellColor(value) == expectedColor);
+ CHECK(key->color() == expectedColor);
+ CHECK(delegate->color() == expectedColor);
+ CHECK(value->color() == expectedColor);
}
JS_UnsetGCZeal(cx, uint8_t(ZealMode::YieldWhileGrayMarking));