Commit 53be79a9 authored by Dominik Inführ's avatar Dominik Inführ Committed by V8 LUCI CQ

[heap] Fix race in sweeper when compacting maps

Access of forwarded map isn't allowed without synchronization. The fix
is to not invoke IsMap() on the forwarded map. If we would want that we
would need a release-store when setting the forwarding pointer on an
evacuated object.

Bug: chromium:1315622, v8:12578
Change-Id: I2f03c810c39875e565bc769c57452af75849044f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3585567Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80003}
parent b4cbe028
......@@ -31,7 +31,7 @@ bool InvalidatedSlotsFilter::IsValid(Address slot) {
HeapObject invalidated_object = HeapObject::FromAddress(invalidated_start_);
if (invalidated_size_ == 0) {
DCHECK(MarkCompactCollector::IsMapOrForwardedMap(invalidated_object.map()));
DCHECK(MarkCompactCollector::IsMapOrForwarded(invalidated_object.map()));
invalidated_size_ = invalidated_object.Size();
}
......
......@@ -197,7 +197,7 @@ void LiveObjectRange<mode>::iterator::AdvanceToNextValidObject() {
HeapObject black_object = HeapObject::FromAddress(addr);
map = black_object.map(cage_base, kAcquireLoad);
// Map might be forwarded during GC.
DCHECK(MarkCompactCollector::IsMapOrForwardedMap(map));
DCHECK(MarkCompactCollector::IsMapOrForwarded(map));
size = black_object.SizeFromMap(map);
CHECK_LE(addr + size, chunk_->area_end());
Address end = addr + size - kTaggedSize;
......
......@@ -517,11 +517,12 @@ void MarkCompactCollector::TearDown() {
}
// static
bool MarkCompactCollector::IsMapOrForwardedMap(Map map) {
bool MarkCompactCollector::IsMapOrForwarded(Map map) {
MapWord map_word = map.map_word(kRelaxedLoad);
if (map_word.IsForwardingAddress()) {
return map_word.ToForwardingAddress().IsMap();
// During GC we can't access forwarded maps without synchronization.
return true;
} else {
return map_word.ToMap().IsMap();
}
......@@ -3231,7 +3232,7 @@ static inline SlotCallbackResult UpdateSlot(PtrComprCageBase cage_base,
DCHECK(!Heap::InFromPage(target));
DCHECK(!MarkCompactCollector::IsOnEvacuationCandidate(target));
} else {
DCHECK(MarkCompactCollector::IsMapOrForwardedMap(map_word.ToMap()));
DCHECK(MarkCompactCollector::IsMapOrForwarded(map_word.ToMap()));
}
return REMOVE_SLOT;
}
......
......@@ -511,7 +511,7 @@ class MarkCompactCollector final : public MarkCompactCollectorBase {
uint32_t offset;
};
static V8_EXPORT_PRIVATE bool IsMapOrForwardedMap(Map map);
static V8_EXPORT_PRIVATE bool IsMapOrForwarded(Map map);
static bool ShouldRecordRelocSlot(Code host, RelocInfo* rinfo,
HeapObject target);
......
......@@ -404,8 +404,7 @@ int Sweeper::RawSweep(Page* p, FreeListRebuildingMode free_list_mode,
sweeping_mode, &old_to_new_cleanup);
}
Map map = object.map(cage_base, kAcquireLoad);
// Map might be forwarded during GC.
DCHECK(MarkCompactCollector::IsMapOrForwardedMap(map));
DCHECK(MarkCompactCollector::IsMapOrForwarded(map));
int size = object.SizeFromMap(map);
live_bytes += size;
free_start = free_end + size;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment