diff --git a/src/heap/cppgc-js/unified-heap-marking-verifier.cc b/src/heap/cppgc-js/unified-heap-marking-verifier.cc index ea14b52048..b0f8595ec7 100644 --- a/src/heap/cppgc-js/unified-heap-marking-verifier.cc +++ b/src/heap/cppgc-js/unified-heap-marking-verifier.cc @@ -35,7 +35,7 @@ class UnifiedHeapVerificationVisitor final : public JSVisitor { void VisitWeakContainer(const void* object, cppgc::TraceDescriptor, cppgc::TraceDescriptor weak_desc, cppgc::WeakCallback, - const void*) { + const void*) final { if (!object) return; // Contents of weak containers are found themselves through page iteration @@ -58,13 +58,8 @@ class UnifiedHeapVerificationVisitor final : public JSVisitor { UnifiedHeapMarkingVerifier::UnifiedHeapMarkingVerifier( cppgc::internal::HeapBase& heap_base) : MarkingVerifierBase( - heap_base, std::make_unique(state_)) { -} - -void UnifiedHeapMarkingVerifier::SetCurrentParent( - const cppgc::internal::HeapObjectHeader* parent) { - state_.SetCurrentParent(parent); -} + heap_base, state_, + std::make_unique(state_)) {} } // namespace internal } // namespace v8 diff --git a/src/heap/cppgc-js/unified-heap-marking-verifier.h b/src/heap/cppgc-js/unified-heap-marking-verifier.h index 3a54b4dd32..bb2ac09e38 100644 --- a/src/heap/cppgc-js/unified-heap-marking-verifier.h +++ b/src/heap/cppgc-js/unified-heap-marking-verifier.h @@ -16,8 +16,6 @@ class V8_EXPORT_PRIVATE UnifiedHeapMarkingVerifier final explicit UnifiedHeapMarkingVerifier(cppgc::internal::HeapBase&); ~UnifiedHeapMarkingVerifier() final = default; - void SetCurrentParent(const cppgc::internal::HeapObjectHeader*) final; - private: // TODO(chromium:1056170): Use a verification state that can handle JS // references. diff --git a/src/heap/cppgc/marking-verifier.cc b/src/heap/cppgc/marking-verifier.cc index 42e3c4eb3e..95bc0e06d0 100644 --- a/src/heap/cppgc/marking-verifier.cc +++ b/src/heap/cppgc/marking-verifier.cc @@ -14,8 +14,10 @@ namespace cppgc { namespace internal { MarkingVerifierBase::MarkingVerifierBase( - HeapBase& heap, std::unique_ptr visitor) + HeapBase& heap, VerificationState& verification_state, + std::unique_ptr visitor) : ConservativeTracingVisitor(heap, *heap.page_backend(), *visitor.get()), + verification_state_(verification_state), visitor_(std::move(visitor)) {} void MarkingVerifierBase::Run(Heap::Config::StackState stack_state) { @@ -36,23 +38,38 @@ void VerificationState::VerifyMarked(const void* base_object_payload) const { "MarkingVerifier: Encountered unmarked object.\n" "#\n" "# Hint:\n" - "# %s\n" - "# \\-> %s", - parent_->GetName().value, child_header.GetName().value); + "# %s (%p)\n" + "# \\-> %s (%p)", + parent_ ? parent_->GetName().value : "Stack", + parent_ ? parent_->Payload() : nullptr, child_header.GetName().value, + child_header.Payload()); } } void MarkingVerifierBase::VisitInConstructionConservatively( HeapObjectHeader& header, TraceConservativelyCallback callback) { - CHECK(header.IsMarked()); if (in_construction_objects_->find(&header) != in_construction_objects_->end()) return; in_construction_objects_->insert(&header); + + // Stack case: Parent is stack and this is merely ensuring that the object + // itself is marked. If the object is marked, then it is being processed by + // the on-heap phase. + if (verification_state_.IsParentOnStack()) { + verification_state_.VerifyMarked(header.Payload()); + return; + } + + // Heap case: Dispatching parent object that must be marked (pre-condition). + CHECK(header.IsMarked()); callback(this, header); } void MarkingVerifierBase::VisitPointer(const void* address) { + // Entry point for stack walk. The conservative visitor dispatches as follows: + // - Fully constructed objects: Visit() + // - Objects in construction: VisitInConstructionConservatively() TraceConservativelyIfNeeded(address); } @@ -62,7 +79,7 @@ bool MarkingVerifierBase::VisitHeapObjectHeader(HeapObjectHeader* header) { DCHECK(!header->IsFree()); - SetCurrentParent(header); + verification_state_.SetCurrentParent(header); if (!header->IsInConstruction()) { header->Trace(visitor_.get()); @@ -71,6 +88,8 @@ bool MarkingVerifierBase::VisitHeapObjectHeader(HeapObjectHeader* header) { TraceConservativelyIfNeeded(*header); } + verification_state_.SetCurrentParent(nullptr); + return true; } @@ -112,12 +131,8 @@ class VerificationVisitor final : public cppgc::Visitor { } // namespace MarkingVerifier::MarkingVerifier(HeapBase& heap_base) - : MarkingVerifierBase(heap_base, + : MarkingVerifierBase(heap_base, state_, std::make_unique(state_)) {} -void MarkingVerifier::SetCurrentParent(const HeapObjectHeader* parent) { - state_.SetCurrentParent(parent); -} - } // namespace internal } // namespace cppgc diff --git a/src/heap/cppgc/marking-verifier.h b/src/heap/cppgc/marking-verifier.h index eeced68449..6f658b3641 100644 --- a/src/heap/cppgc/marking-verifier.h +++ b/src/heap/cppgc/marking-verifier.h @@ -21,6 +21,9 @@ class VerificationState { void VerifyMarked(const void*) const; void SetCurrentParent(const HeapObjectHeader* header) { parent_ = header; } + // No parent means parent was on stack. + bool IsParentOnStack() const { return !parent_; } + private: const HeapObjectHeader* parent_ = nullptr; }; @@ -40,9 +43,8 @@ class V8_EXPORT_PRIVATE MarkingVerifierBase void Run(Heap::Config::StackState); protected: - MarkingVerifierBase(HeapBase&, std::unique_ptr); - - virtual void SetCurrentParent(const HeapObjectHeader*) = 0; + MarkingVerifierBase(HeapBase&, VerificationState&, + std::unique_ptr); private: void VisitInConstructionConservatively(HeapObjectHeader&, @@ -51,6 +53,7 @@ class V8_EXPORT_PRIVATE MarkingVerifierBase bool VisitHeapObjectHeader(HeapObjectHeader*); + VerificationState& verification_state_; std::unique_ptr visitor_; std::unordered_set in_construction_objects_heap_; @@ -64,8 +67,6 @@ class V8_EXPORT_PRIVATE MarkingVerifier final : public MarkingVerifierBase { explicit MarkingVerifier(HeapBase&); ~MarkingVerifier() final = default; - void SetCurrentParent(const HeapObjectHeader*) final; - private: VerificationState state_; };