cppgc: Port MarkingVerifier

This CL ports MarkingVerifier from blink.

The existing verifier checks only references on heap.
This new verifier checks references both on heap and on stack.

Bug: chromium:1056170
Change-Id: I083dcb0087125312cca34a2201015a9aecfe6ea4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2300484
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68891}

BUILD.gn

@@ -4233,6 +4233,8 @@ v8_source_set("cppgc_base") {
     "src/heap/cppgc/marker.cc",
     "src/heap/cppgc/marker.h",
     "src/heap/cppgc/marking-state.h",
+    "src/heap/cppgc/marking-verifier.cc",
+    "src/heap/cppgc/marking-verifier.h",
     "src/heap/cppgc/marking-visitor.cc",
     "src/heap/cppgc/marking-visitor.h",
     "src/heap/cppgc/marking-worklists.cc",

src/heap/cppgc-js/cpp-heap.cc

@@ -156,6 +156,11 @@ void CppHeap::TraceEpilogue(TraceSummary* trace_summary) {
     marker()->ProcessWeakness();
     prefinalizer_handler()->InvokePreFinalizers();
   }
+  marker_.reset();
+  // TODO(chromium:1056170): replace build flag with dedicated flag.
+#if DEBUG
+  VerifyMarking(cppgc::Heap::StackState::kNoHeapPointers);
+#endif
   {
     NoGCScope no_gc(*this);
     sweeper().Start(cppgc::internal::Sweeper::Config::kAtomic);

src/heap/cppgc/heap-base.cc

@@ -12,6 +12,7 @@
 #include "src/heap/cppgc/heap-page.h"
 #include "src/heap/cppgc/heap-visitor.h"
 #include "src/heap/cppgc/marker.h"
+#include "src/heap/cppgc/marking-verifier.h"
 #include "src/heap/cppgc/page-memory.h"
 #include "src/heap/cppgc/prefinalizer-handler.h"
 #include "src/heap/cppgc/stats-collector.h"
@@ -84,5 +85,9 @@ HeapBase::NoGCScope::NoGCScope(HeapBase& heap) : heap_(heap) {
 
 HeapBase::NoGCScope::~NoGCScope() { heap_.no_gc_scope_--; }
 
+void HeapBase::VerifyMarking(cppgc::Heap::StackState stack_state) {
+  MarkingVerifier verifier(*this, stack_state);
+}
+
 }  // namespace internal
 }  // namespace cppgc

src/heap/cppgc/heap-base.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <set>
 
+#include "include/cppgc/heap.h"
 #include "include/cppgc/internal/persistent-node.h"
 #include "include/cppgc/macros.h"
 #include "src/base/macros.h"
@@ -116,6 +117,8 @@ class V8_EXPORT_PRIVATE HeapBase {
   size_t ObjectPayloadSize() const;
 
  protected:
+  void VerifyMarking(cppgc::Heap::StackState);
+
   bool in_no_gc_scope() const { return no_gc_scope_ > 0; }
 
   RawHeap raw_heap_;

src/heap/cppgc/heap.cc

@@ -107,12 +107,16 @@ void Heap::CollectGarbage(Config config) {
   marker_->FinishMarking(marking_config);
   // "Sweeping and finalization".
   {
-    // Pre finalizers are forbidden from allocating objects
+    // Pre finalizers are forbidden from allocating objects.
     ObjectAllocator::NoAllocationScope no_allocation_scope_(object_allocator_);
     marker_->ProcessWeakness();
     prefinalizer_handler_->InvokePreFinalizers();
   }
   marker_.reset();
+  // TODO(chromium:1056170): replace build flag with dedicated flag.
+#if DEBUG
+  VerifyMarking(config.stack_state);
+#endif
   {
     NoGCScope no_gc(*this);
     sweeper_.Start(config.sweeping_type);

src/heap/cppgc/marking-verifier.cc

@@ -0,0 +1,63 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/heap/cppgc/marking-verifier.h"
+
+#include "src/base/logging.h"
+#include "src/heap/cppgc/gc-info-table.h"
+#include "src/heap/cppgc/heap.h"
+
+namespace cppgc {
+namespace internal {
+
+MarkingVerifier::MarkingVerifier(HeapBase& heap,
+                                 Heap::Config::StackState stack_state)
+    : cppgc::Visitor(VisitorFactory::CreateKey()),
+      ConservativeTracingVisitor(heap, *heap.page_backend(), *this) {
+  Traverse(&heap.raw_heap());
+  if (stack_state == Heap::Config::StackState::kMayContainHeapPointers)
+    heap.stack()->IteratePointers(this);
+}
+
+void MarkingVerifier::Visit(const void* object, TraceDescriptor desc) {
+  VerifyChild(desc.base_object_payload);
+}
+
+void MarkingVerifier::VisitWeak(const void* object, TraceDescriptor desc,
+                                WeakCallback, const void*) {
+  // Weak objects should have been cleared at this point. As a consequence, all
+  // objects found through weak references have to point to live objects at this
+  // point.
+  VerifyChild(desc.base_object_payload);
+}
+
+void MarkingVerifier::VerifyChild(const void* base_object_payload) {
+  const HeapObjectHeader& child_header =
+      HeapObjectHeader::FromPayload(base_object_payload);
+
+  CHECK(child_header.IsMarked());
+}
+
+void MarkingVerifier::VisitConservatively(
+    HeapObjectHeader& header, TraceConservativelyCallback callback) {
+  CHECK(header.IsMarked());
+}
+
+void MarkingVerifier::VisitPointer(const void* address) {
+  TraceConservativelyIfNeeded(address);
+}
+
+bool MarkingVerifier::VisitHeapObjectHeader(HeapObjectHeader* header) {
+  // Verify only non-free marked objects.
+  if (!header->IsMarked()) return true;
+
+  DCHECK(!header->IsFree());
+
+  GlobalGCInfoTable::GCInfoFromIndex(header->GetGCInfoIndex())
+      .trace(this, header->Payload());
+  return true;
+}
+
+}  // namespace internal
+}  // namespace cppgc

src/heap/cppgc/marking-verifier.h

@@ -0,0 +1,42 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_HEAP_CPPGC_MARKING_VERIFIER_H_
+#define V8_HEAP_CPPGC_MARKING_VERIFIER_H_
+
+#include "src/heap/base/stack.h"
+#include "src/heap/cppgc/heap-visitor.h"
+#include "src/heap/cppgc/heap.h"
+#include "src/heap/cppgc/visitor.h"
+
+namespace cppgc {
+namespace internal {
+
+class V8_EXPORT_PRIVATE MarkingVerifier final
+    : private HeapVisitor<MarkingVerifier>,
+      public cppgc::Visitor,
+      public ConservativeTracingVisitor,
+      public heap::base::StackVisitor {
+  friend class HeapVisitor<MarkingVerifier>;
+
+ public:
+  explicit MarkingVerifier(HeapBase&, Heap::Config::StackState);
+
+  void Visit(const void*, TraceDescriptor) final;
+  void VisitWeak(const void*, TraceDescriptor, WeakCallback, const void*) final;
+
+ private:
+  void VerifyChild(const void*);
+
+  void VisitConservatively(HeapObjectHeader&,
+                           TraceConservativelyCallback) final;
+  void VisitPointer(const void*) final;
+
+  bool VisitHeapObjectHeader(HeapObjectHeader*);
+};
+
+}  // namespace internal
+}  // namespace cppgc
+
+#endif  // V8_HEAP_CPPGC_MARKING_VERIFIER_H_

test/unittests/BUILD.gn

@@ -57,6 +57,7 @@ v8_source_set("cppgc_unittests_sources") {
     "heap/cppgc/heap-unittest.cc",
     "heap/cppgc/logging-unittest.cc",
     "heap/cppgc/marker-unittest.cc",
+    "heap/cppgc/marking-verifier-unittest.cc",
     "heap/cppgc/marking-visitor-unittest.cc",
     "heap/cppgc/member-unittest.cc",
     "heap/cppgc/minor-gc-unittest.cc",

test/unittests/heap/cppgc/marking-verifier-unittest.cc

@@ -0,0 +1,114 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/heap/cppgc/marking-verifier.h"
+
+#include "include/cppgc/allocation.h"
+#include "include/cppgc/member.h"
+#include "include/cppgc/persistent.h"
+#include "src/heap/cppgc/heap-object-header.h"
+#include "src/heap/cppgc/heap.h"
+#include "test/unittests/heap/cppgc/tests.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace cppgc {
+namespace internal {
+
+namespace {
+
+class MarkingVerifierTest : public testing::TestWithHeap {
+ public:
+  using StackState = Heap::Config::StackState;
+
+  void VerifyMarking(HeapBase& heap, StackState stack_state) {
+    Heap::From(GetHeap())->object_allocator().ResetLinearAllocationBuffers();
+    MarkingVerifier verifier(heap, stack_state);
+  }
+};
+
+class GCed : public GarbageCollected<GCed> {
+ public:
+  void SetChild(GCed* child) { child_ = child; }
+  void SetWeakChild(GCed* child) { weak_child_ = child; }
+  GCed* child() const { return child_.Get(); }
+  GCed* weak_child() const { return weak_child_.Get(); }
+  void Trace(cppgc::Visitor* visitor) const {
+    visitor->Trace(child_);
+    visitor->Trace(weak_child_);
+  }
+
+ private:
+  Member<GCed> child_;
+  WeakMember<GCed> weak_child_;
+};
+
+template <typename T>
+V8_NOINLINE T access(volatile const T& t) {
+  return t;
+}
+
+}  // namespace
+
+// Following tests should not crash.
+
+TEST_F(MarkingVerifierTest, DoesntDieOnMarkedOnStackReference) {
+  GCed* object = MakeGarbageCollected<GCed>(GetAllocationHandle());
+  HeapObjectHeader::FromPayload(object).TryMarkAtomic();
+  VerifyMarking(Heap::From(GetHeap())->AsBase(),
+                StackState::kMayContainHeapPointers);
+  access(object);
+}
+
+TEST_F(MarkingVerifierTest, DoesntDieOnMarkedMember) {
+  Persistent<GCed> parent = MakeGarbageCollected<GCed>(GetAllocationHandle());
+  HeapObjectHeader::FromPayload(parent.Get()).TryMarkAtomic();
+  parent->SetChild(MakeGarbageCollected<GCed>(GetAllocationHandle()));
+  HeapObjectHeader::FromPayload(parent->child()).TryMarkAtomic();
+  VerifyMarking(Heap::From(GetHeap())->AsBase(), StackState::kNoHeapPointers);
+}
+
+TEST_F(MarkingVerifierTest, DoesntDieOnMarkedWeakMember) {
+  Persistent<GCed> parent = MakeGarbageCollected<GCed>(GetAllocationHandle());
+  HeapObjectHeader::FromPayload(parent.Get()).TryMarkAtomic();
+  parent->SetWeakChild(MakeGarbageCollected<GCed>(GetAllocationHandle()));
+  HeapObjectHeader::FromPayload(parent->weak_child()).TryMarkAtomic();
+  VerifyMarking(Heap::From(GetHeap())->AsBase(), StackState::kNoHeapPointers);
+}
+
+// Death tests.
+
+namespace {
+
+class MarkingVerifierDeathTest : public MarkingVerifierTest {};
+
+}  // namespace
+
+TEST_F(MarkingVerifierDeathTest, DieOnUnmarkedOnStackReference) {
+  GCed* object = MakeGarbageCollected<GCed>(GetAllocationHandle());
+  EXPECT_DEATH_IF_SUPPORTED(VerifyMarking(Heap::From(GetHeap())->AsBase(),
+                                          StackState::kMayContainHeapPointers),
+                            "");
+  access(object);
+}
+
+TEST_F(MarkingVerifierDeathTest, DieOnUnmarkedMember) {
+  Persistent<GCed> parent = MakeGarbageCollected<GCed>(GetAllocationHandle());
+  HeapObjectHeader::FromPayload(parent.Get()).TryMarkAtomic();
+  parent->SetChild(MakeGarbageCollected<GCed>(GetAllocationHandle()));
+  EXPECT_DEATH_IF_SUPPORTED(VerifyMarking(Heap::From(GetHeap())->AsBase(),
+                                          StackState::kNoHeapPointers),
+                            "");
+}
+
+TEST_F(MarkingVerifierDeathTest, DieOnUnmarkedWeakMember) {
+  Persistent<GCed> parent = MakeGarbageCollected<GCed>(GetAllocationHandle());
+  HeapObjectHeader::FromPayload(parent.Get()).TryMarkAtomic();
+  parent->SetWeakChild(MakeGarbageCollected<GCed>(GetAllocationHandle()));
+  EXPECT_DEATH_IF_SUPPORTED(VerifyMarking(Heap::From(GetHeap())->AsBase(),
+                                          StackState::kNoHeapPointers),
+                            "");
+}
+
+}  // namespace internal
+}  // namespace cppgc