Commit 4898cd54 authored by Dominik Inführ's avatar Dominik Inführ Committed by V8 LUCI CQ

Reland "[heap] Ensure all old-to-shared slots are recorded"

This is a reland of commit c3a5c5b1

The previous CL was writing into the wrong sets when invoking
CollectSlots<OLD_TO_SHARED>(). Also move the NULL checks out of
that condition to also check this for chunks in the young generation.

Original change's description:
> [heap] Ensure all old-to-shared slots are recorded
>
> This CL adds verification of the old-to-shared remembered set to
> --verify-heap. During shared GCs client heaps will be scanned for
> references into the shared heap, this CL will CHECK that every found
> slot is contained in the old-to-shared remembered set. After this
> gets a bit more stable, the full heap iteration can be dropped and we
> can fully rely on the remembered set instead.
>
> Bug: v8:11708
> Change-Id: I0b5c4edfe3271306e4e7af7394472534113e1953
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3792605
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82578}

Bug: v8:11708
Change-Id: I24b7787977f06708efb7a017dd1ec72f78d0ea13
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3841570Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82653}
parent 2115ba50
......@@ -283,6 +283,13 @@ bool Heap::InYoungGeneration(HeapObject heap_object) {
return result;
}
// static
bool Heap::InSharedWritableHeap(MaybeObject object) {
HeapObject heap_object;
return object->GetHeapObject(&heap_object) &&
heap_object.InSharedWritableHeap();
}
// static
bool Heap::InFromPage(Object object) {
DCHECK(!HasWeakHeapObjectTag(object));
......
......@@ -4685,6 +4685,18 @@ class OldToNewSlotVerifyingVisitor : public SlotVerifyingVisitor {
EphemeronRememberedSet* ephemeron_remembered_set_;
};
class OldToSharedSlotVerifyingVisitor : public SlotVerifyingVisitor {
public:
OldToSharedSlotVerifyingVisitor(Isolate* isolate, std::set<Address>* untyped,
std::set<std::pair<SlotType, Address>>* typed)
: SlotVerifyingVisitor(isolate, untyped, typed) {}
bool ShouldHaveBeenRecorded(HeapObject host, MaybeObject target) override {
return target->IsStrongOrWeak() && Heap::InSharedWritableHeap(target) &&
!Heap::InYoungGeneration(host) && !host.InSharedWritableHeap();
}
};
template <RememberedSetType direction>
void CollectSlots(MemoryChunk* chunk, Address start, Address end,
std::set<Address>* untyped,
......@@ -4716,14 +4728,31 @@ void Heap::VerifyRememberedSetFor(HeapObject object) {
PtrComprCageBase cage_base(isolate());
Address start = object.address();
Address end = start + object.Size(cage_base);
std::set<Address> old_to_new;
std::set<std::pair<SlotType, Address>> typed_old_to_new;
if (chunk->InSharedHeap() || InYoungGeneration(object)) {
CHECK_NULL(chunk->slot_set<OLD_TO_NEW>());
CHECK_NULL(chunk->typed_slot_set<OLD_TO_NEW>());
CHECK_NULL(chunk->slot_set<OLD_TO_OLD>());
CHECK_NULL(chunk->typed_slot_set<OLD_TO_OLD>());
}
if (!InYoungGeneration(object)) {
std::set<Address> old_to_new;
std::set<std::pair<SlotType, Address>> typed_old_to_new;
CollectSlots<OLD_TO_NEW>(chunk, start, end, &old_to_new, &typed_old_to_new);
OldToNewSlotVerifyingVisitor visitor(isolate(), &old_to_new,
&typed_old_to_new,
&this->ephemeron_remembered_set_);
object.IterateBody(cage_base, &visitor);
OldToNewSlotVerifyingVisitor old_to_new_visitor(
isolate(), &old_to_new, &typed_old_to_new,
&this->ephemeron_remembered_set_);
object.IterateBody(cage_base, &old_to_new_visitor);
std::set<Address> old_to_shared;
std::set<std::pair<SlotType, Address>> typed_old_to_shared;
CollectSlots<OLD_TO_SHARED>(chunk, start, end, &old_to_shared,
&typed_old_to_shared);
OldToSharedSlotVerifyingVisitor old_to_shared_visitor(
isolate(), &old_to_shared, &typed_old_to_shared);
object.IterateBody(cage_base, &old_to_shared_visitor);
}
// TODO(v8:11797): Add old to old slot set verification once all weak objects
// have their own instance types and slots are recorded for all weak fields.
......
......@@ -1261,6 +1261,9 @@ class Heap {
// spaces.
V8_EXPORT_PRIVATE bool ContainsCode(HeapObject value) const;
// Checks whether object resides in the non-read-only shared heap.
static inline bool InSharedWritableHeap(MaybeObject object);
// Checks whether an address/object is in the non-read-only heap (including
// auxiliary area and unused area). Use IsValidHeapObject if checking both
// heaps is required.
......
......@@ -1369,9 +1369,16 @@ class MarkCompactCollector::SharedHeapObjectVisitor final
DCHECK(!host.InSharedHeap());
if (!object.IsHeapObject()) return;
HeapObject heap_object = HeapObject::cast(object);
if (!heap_object.InSharedHeap()) return;
RememberedSet<OLD_TO_SHARED>::Insert<AccessMode::NON_ATOMIC>(
MemoryChunk::FromHeapObject(host), slot.address());
if (!heap_object.InSharedWritableHeap()) return;
DCHECK(heap_object.InSharedWritableHeap());
MemoryChunk* host_chunk = MemoryChunk::FromHeapObject(host);
if (host_chunk->InYoungGeneration()) {
RememberedSet<OLD_TO_SHARED>::Insert<AccessMode::NON_ATOMIC>(
host_chunk, slot.address());
} else {
CHECK(RememberedSet<OLD_TO_SHARED>::Contains(host_chunk, slot.address()));
}
collector_->MarkRootObject(Root::kClientHeap, heap_object);
}
......
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