Commit f20949fa authored by Nikolaos Papaspyrou's avatar Nikolaos Papaspyrou Committed by V8 LUCI CQ

heap: Fix bug in minor MC heap verification

Minor MC heap verification requires heap iterability. This however was
not directly ensured. Coincidentally, there was an unrelated call to
`Heap::Verify` that ensured `Heap::MakeHeapIterable` had been called,
so the precondition was met. This call was moved to an earlier point
by https://crrev.com/c/3497318 and, because of that, some combination
of flags now results in a crash.

This CL fixes the issue by directly ensuring heap iterability. It also
moves back the call to `Heap::Verify`, so that it takes place inside
the GC safepoint.

Bug: v8:12768
Change-Id: I2c66de0d0a735b84dd9435ff503e78bb3611ce55
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3569224Reviewed-by: 's avatarDominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79844}
parent 9f128f4e
......@@ -2241,17 +2241,17 @@ size_t Heap::PerformGarbageCollection(
const char* collector_reason, const v8::GCCallbackFlags gc_callback_flags) {
DisallowJavascriptExecution no_js(isolate());
if (IsYoungGenerationCollector(collector)) {
CompleteSweepingYoung(collector);
#ifdef VERIFY_HEAP
if (FLAG_verify_heap) {
// We don't really perform a GC here but need this scope for the nested
// SafepointScope inside Verify().
AllowGarbageCollection allow_gc;
Verify();
// If heap verification is enabled, we want to ensure that sweeping is
// completed here, as it will be triggered from Heap::Verify anyway.
// In this way, sweeping finalization is accounted to the corresponding
// full GC cycle.
CompleteSweepingFull();
}
#endif // VERIFY_HEAP
if (IsYoungGenerationCollector(collector)) {
CompleteSweepingYoung(collector);
tracer()->StartCycle(collector, gc_reason, collector_reason,
GCTracer::MarkingType::kAtomic);
} else {
......@@ -2284,6 +2284,15 @@ size_t Heap::PerformGarbageCollection(
collection_barrier_->StopTimeToCollectionTimer();
#ifdef VERIFY_HEAP
if (FLAG_verify_heap) {
// We don't really perform a GC here but need this scope for the nested
// SafepointScope inside Verify().
AllowGarbageCollection allow_gc;
Verify();
}
#endif // VERIFY_HEAP
tracer()->StartInSafepoint();
GarbageCollectionPrologueInSafepoint();
......
......@@ -395,6 +395,7 @@ class FullEvacuationVerifier : public EvacuationVerifier {
explicit FullEvacuationVerifier(Heap* heap) : EvacuationVerifier(heap) {}
void Run() override {
DCHECK(!heap_->mark_compact_collector()->sweeping_in_progress());
VerifyRoots();
VerifyEvacuation(heap_->new_space());
VerifyEvacuation(heap_->old_space());
......@@ -4929,6 +4930,7 @@ class YoungGenerationEvacuationVerifier : public EvacuationVerifier {
: EvacuationVerifier(heap) {}
void Run() override {
DCHECK(!heap_->mark_compact_collector()->sweeping_in_progress());
VerifyRoots();
VerifyEvacuation(heap_->new_space());
VerifyEvacuation(heap_->old_space());
......
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