Skip to content

Commit

Permalink
Bug 1875030 - Update element ranges on the mark stack before and afte…
Browse files Browse the repository at this point in the history 
…r marking to take account of shifted elements r=sfink,jandem

Differential Revision: https://phabricator.services.mozilla.com/D198782
  • Loading branch information
@jonco3
jonco3 committed Jan 26, 2024
1 parent 462c421 commit 1b6c209
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 20 deletions.
16 changes: 5 additions & 11 deletions js/src/gc/GC.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3143,20 +3143,14 @@ GCRuntime::MarkQueueProgress GCRuntime::processTestMarkQueue() {
return QueueSuspended;
}

// Mark the object and push it onto the stack.
size_t oldPosition = marker().stack.position();
marker().markAndTraverse<NormalMarkingOptions>(obj);

// If we overflow the stack here and delay marking, then we won't be
// testing what we think we're testing.
if (marker().stack.position() == oldPosition) {
// Mark the object.
AutoEnterOOMUnsafeRegion oomUnsafe;
if (!marker().markOneObjectForTest(obj)) {
// If we overflowed the stack here and delayed marking, then we won't be
// testing what we think we're testing.
MOZ_ASSERT(obj->asTenured().arena()->onDelayedMarkingList());
AutoEnterOOMUnsafeRegion oomUnsafe;
oomUnsafe.crash("Overflowed stack while marking test queue");
}

SliceBudget unlimited = SliceBudget::unlimited();
marker().processMarkStackTop<NormalMarkingOptions>(unlimited);
} else if (val.isString()) {
JSLinearString* str = &val.toString()->asLinear();
if (js::StringEqualsLiteral(str, "yield") && isIncrementalGc()) {
Expand Down
41 changes: 40 additions & 1 deletion js/src/gc/GCMarker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ namespace gc {
enum IncrementalProgress { NotFinished = 0, Finished };

class AutoSetMarkColor;
class AutoUpdateMarkStackRanges;
struct Cell;
class MarkStackIter;
class ParallelMarker;
class UnmarkGrayTracer;

Expand Down Expand Up @@ -141,10 +143,12 @@ class MarkStack {
size_t start() const;
TaggedPtr ptr() const;

void setStart(size_t newStart);

private:
static constexpr size_t StartShift = 2;
static constexpr size_t KindMask = (1 << StartShift) - 1;

private:
uintptr_t startAndKind_;
TaggedPtr ptr_;
};
Expand Down Expand Up @@ -229,6 +233,13 @@ class MarkStack {
// The maximum stack capacity to grow to.
MainThreadOrGCTaskData<size_t> maxCapacity_{SIZE_MAX};
#endif

#ifdef DEBUG
MainThreadOrGCTaskData<bool> elementsRangesAreValid;
friend class js::GCMarker;
#endif

friend class MarkStackIter;
};

static_assert(unsigned(SlotsOrElementsKind::Unused) ==
Expand All @@ -237,6 +248,25 @@ static_assert(unsigned(SlotsOrElementsKind::Unused) ==
"difference between SlotsOrElementsRange::startAndKind_ and a "
"tagged SlotsOrElementsRange");

class MOZ_STACK_CLASS MarkStackIter {
MarkStack& stack_;
size_t pos_;

public:
explicit MarkStackIter(MarkStack& stack);

bool done() const;
void next();

MarkStack::Tag peekTag() const;
bool isSlotsOrElementsRange() const;
MarkStack::SlotsOrElementsRange& slotsOrElementsRange();

private:
size_t position() const;
MarkStack::TaggedPtr peekPtr() const;
};

// Bitmask of options to parameterize MarkingTracerT.
namespace MarkingOptions {
enum : uint32_t {
Expand Down Expand Up @@ -371,6 +401,8 @@ class GCMarker {
void setCheckAtomMarking(bool check);

bool shouldCheckCompartments() { return strictCompartmentChecking; }

bool markOneObjectForTest(JSObject* obj);
#endif

bool markCurrentColorInParallel(SliceBudget& budget);
Expand Down Expand Up @@ -417,6 +449,13 @@ class GCMarker {
template <typename Tracer>
void setMarkingStateAndTracer(MarkingState prev, MarkingState next);

// The mutator can shift object elements which could invalidate any elements
// index on the mark stack. Change the index to be relative to the elements
// allocation (to ignore shifted elements) while the mutator is running.
void updateRangesAtStartOfSlice();
void updateRangesAtEndOfSlice();
friend class gc::AutoUpdateMarkStackRanges;

template <uint32_t markingOptions>
bool processMarkStackTop(SliceBudget& budget);
friend class gc::GCRuntime;
Expand Down
126 changes: 118 additions & 8 deletions js/src/gc/Marking.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1349,9 +1349,20 @@ bool GCMarker::doMarking(SliceBudget& budget, ShouldReportMarkTime reportTime) {
return true;
}

class MOZ_RAII gc::AutoUpdateMarkStackRanges {
GCMarker& marker_;

public:
explicit AutoUpdateMarkStackRanges(GCMarker& marker) : marker_(marker) {
marker_.updateRangesAtStartOfSlice();
}
~AutoUpdateMarkStackRanges() { marker_.updateRangesAtEndOfSlice(); }
};

template <uint32_t opts, MarkColor color>
bool GCMarker::markOneColor(SliceBudget& budget) {
AutoSetMarkColor setColor(*this, color);
AutoUpdateMarkStackRanges updateRanges(*this);

while (processMarkStackTop<opts>(budget)) {
if (stack.isEmpty()) {
Expand All @@ -1363,6 +1374,8 @@ bool GCMarker::markOneColor(SliceBudget& budget) {
}

bool GCMarker::markCurrentColorInParallel(SliceBudget& budget) {
AutoUpdateMarkStackRanges updateRanges(*this);

ParallelMarker::AtomicCount& waitingTaskCount =
parallelMarker_->waitingTaskCountRef();

Expand All @@ -1382,6 +1395,26 @@ bool GCMarker::markCurrentColorInParallel(SliceBudget& budget) {
return false;
}

#ifdef DEBUG
bool GCMarker::markOneObjectForTest(JSObject* obj) {
MOZ_ASSERT(obj->zone()->isGCMarking());
MOZ_ASSERT(!obj->isMarked(markColor()));

size_t oldPosition = stack.position();
markAndTraverse<NormalMarkingOptions>(obj);
if (stack.position() == oldPosition) {
return false;
}

AutoUpdateMarkStackRanges updateRanges(*this);

SliceBudget unlimited = SliceBudget::unlimited();
processMarkStackTop<NormalMarkingOptions>(unlimited);

return true;
}
#endif

static inline void CheckForCompartmentMismatch(JSObject* obj, JSObject* obj2) {
#ifdef DEBUG
if (MOZ_UNLIKELY(obj->compartment() != obj2->compartment())) {
Expand All @@ -1408,6 +1441,44 @@ static inline size_t NumUsedDynamicSlots(NativeObject* obj) {
return nslots - nfixed;
}

void GCMarker::updateRangesAtStartOfSlice() {
for (MarkStackIter iter(stack); !iter.done(); iter.next()) {
if (iter.isSlotsOrElementsRange()) {
MarkStack::SlotsOrElementsRange& range = iter.slotsOrElementsRange();
if (range.kind() == SlotsOrElementsKind::Elements) {
NativeObject* obj = &range.ptr().asRangeObject()->as<NativeObject>();
size_t index = range.start();
size_t numShifted = obj->getElementsHeader()->numShiftedElements();
index -= std::min(numShifted, index);
range.setStart(index);
}
}
}

#ifdef DEBUG
MOZ_ASSERT(!stack.elementsRangesAreValid);
stack.elementsRangesAreValid = true;
#endif
}

void GCMarker::updateRangesAtEndOfSlice() {
for (MarkStackIter iter(stack); !iter.done(); iter.next()) {
if (iter.isSlotsOrElementsRange()) {
MarkStack::SlotsOrElementsRange& range = iter.slotsOrElementsRange();
if (range.kind() == SlotsOrElementsKind::Elements) {
NativeObject* obj = &range.ptr().asRangeObject()->as<NativeObject>();
size_t numShifted = obj->getElementsHeader()->numShiftedElements();
range.setStart(range.start() + numShifted);
}
}
}

#ifdef DEBUG
MOZ_ASSERT(stack.elementsRangesAreValid);
stack.elementsRangesAreValid = false;
#endif
}

template <uint32_t opts>
inline bool GCMarker::processMarkStackTop(SliceBudget& budget) {
/*
Expand All @@ -1421,6 +1492,7 @@ inline bool GCMarker::processMarkStackTop(SliceBudget& budget) {
*/

MOZ_ASSERT(!stack.isEmpty());
MOZ_ASSERT(stack.elementsRangesAreValid);
MOZ_ASSERT_IF(markColor() == MarkColor::Gray, !hasBlackEntries());

JSObject* obj; // The object being scanned.
Expand Down Expand Up @@ -1451,12 +1523,7 @@ inline bool GCMarker::processMarkStackTop(SliceBudget& budget) {

case SlotsOrElementsKind::Elements: {
base = nobj->getDenseElements();

// Account for shifted elements.
size_t numShifted = nobj->getElementsHeader()->numShiftedElements();
size_t initlen = nobj->getDenseInitializedLength();
index = std::max(index, numShifted) - numShifted;
end = initlen;
end = nobj->getDenseInitializedLength();
break;
}

Expand Down Expand Up @@ -1632,11 +1699,9 @@ struct MapTypeToMarkStackTag<BaseScript*> {
static const auto value = MarkStack::ScriptTag;
};

#ifdef DEBUG
static inline bool TagIsRangeTag(MarkStack::Tag tag) {
return tag == MarkStack::SlotsOrElementsRangeTag;
}
#endif

inline MarkStack::TaggedPtr::TaggedPtr(Tag tag, Cell* ptr)
: bits(tag | uintptr_t(ptr)) {
Expand Down Expand Up @@ -1703,6 +1768,11 @@ inline size_t MarkStack::SlotsOrElementsRange::start() const {
return startAndKind_ >> StartShift;
}

inline void MarkStack::SlotsOrElementsRange::setStart(size_t newStart) {
startAndKind_ = (newStart << StartShift) | uintptr_t(kind());
MOZ_ASSERT(start() == newStart);
}

inline MarkStack::TaggedPtr MarkStack::SlotsOrElementsRange::ptr() const {
return ptr_;
}
Expand Down Expand Up @@ -1973,6 +2043,45 @@ size_t MarkStack::sizeOfExcludingThis(
return stack().sizeOfExcludingThis(mallocSizeOf);
}

MarkStackIter::MarkStackIter(MarkStack& stack)
: stack_(stack), pos_(stack.position()) {}

inline size_t MarkStackIter::position() const { return pos_; }

inline bool MarkStackIter::done() const { return position() == 0; }

inline void MarkStackIter::next() {
if (isSlotsOrElementsRange()) {
MOZ_ASSERT(position() >= ValueRangeWords);
pos_ -= ValueRangeWords;
return;
}

MOZ_ASSERT(!done());
pos_--;
}

inline bool MarkStackIter::isSlotsOrElementsRange() const {
return TagIsRangeTag(peekTag());
}

inline MarkStack::Tag MarkStackIter::peekTag() const { return peekPtr().tag(); }

inline MarkStack::TaggedPtr MarkStackIter::peekPtr() const {
MOZ_ASSERT(!done());
return stack_.stack()[pos_ - 1];
}

inline MarkStack::SlotsOrElementsRange& MarkStackIter::slotsOrElementsRange() {
MOZ_ASSERT(TagIsRangeTag(peekTag()));
MOZ_ASSERT(position() >= ValueRangeWords);

MarkStack::TaggedPtr* ptr = &stack_.stack()[pos_ - ValueRangeWords];
auto& range = *reinterpret_cast<MarkStack::SlotsOrElementsRange*>(ptr);
range.assertValid();
return range;
}

/*** GCMarker ***************************************************************/

/*
Expand Down Expand Up @@ -2289,6 +2398,7 @@ void GCRuntime::processDelayedMarkingList(MarkColor color) {
// were added.

AutoSetMarkColor setColor(marker(), color);
AutoUpdateMarkStackRanges updateRanges(marker());

do {
delayedMarkingWorkAdded = false;
Expand Down

0 comments on commit 1b6c209

Please sign in to comment.