cppgc: Introduce cppgc_enable_verify_heap

Adds a heap verification GN arg to gate the marking verifier and live
bytes verification on. The flag may be used in future for other more
expensive checks as well.

Currently, the flag is automatically enabled in dcheck_is_on and debug
builds.

The change enables live bytes verification for the library in regular
debug builds which may flush out issues.

Bug: v8:11785
Change-Id: I0f41bc0d76ebea9f6a8c9315c947598015ee5d68
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3097868
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76325}

BUILD.bazel

@@ -164,10 +164,10 @@ config_setting(
 # v8_enable_snapshot_compression
 # v8_control_flow_integrity
 # v8_enable_virtual_memory_cage
-# cppgc_enable_object_names
 # cppgc_enable_caged_heap
-# cppgc_enable_verify_live_bytes
 # cppgc_enable_check_assignments_in_prefinalizers
+# cppgc_enable_object_names
+# cppgc_enable_verify_heap
 # cppgc_enable_young_generation
 # v8_enable_zone_compression
 # v8_enable_heap_sandbox

BUILD.gn

@@ -290,9 +290,8 @@ declare_args() {
       v8_current_cpu == "x64" || v8_current_cpu == "arm64" ||
       v8_current_cpu == "loong64"
 
-  # Enable verification of live bytes in the marking verifier.
-  # TODO(v8:11785): Enable by default when running with the verifier.
-  cppgc_enable_verify_live_bytes = false
+  # Enables additional heap verification phases and checks.
+  cppgc_enable_verify_heap = ""
 
   # Enable assignment checks for Members/Persistents during prefinalizer invocations.
   # TODO(v8:11749): Enable by default after fixing any existing issues in Blink.
@@ -347,6 +346,9 @@ declare_args() {
 }
 
 # Derived defaults.
+if (cppgc_enable_verify_heap == "") {
+  cppgc_enable_verify_heap = v8_enable_debugging_features || dcheck_always_on
+}
 if (v8_enable_verify_heap == "") {
   v8_enable_verify_heap = v8_enable_debugging_features
 }
@@ -771,8 +773,8 @@ config("features") {
     ":cppgc_header_features",
   ]
 
-  if (cppgc_enable_verify_live_bytes) {
-    defines += [ "CPPGC_VERIFY_LIVE_BYTES" ]
+  if (cppgc_enable_verify_heap) {
+    defines += [ "CPPGC_VERIFY_HEAP" ]
   }
 
   if (cppgc_enable_check_assignments_in_prefinalizers) {

src/base/optional.h

@@ -35,7 +35,7 @@ constexpr in_place_t in_place = {};
 // http://en.cppreference.com/w/cpp/utility/optional/nullopt
 constexpr nullopt_t nullopt(0);
 
-// Forward declaration, which is refered by following helpers.
+// Forward declaration, which is referred by following helpers.
 template <typename T>
 class Optional;
 

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

@@ -484,12 +484,11 @@ void CppHeap::TraceEpilogue(TraceSummary* trace_summary) {
   // any pending allocated bytes updates should be discarded.
   buffered_allocated_bytes_ = 0;
   ExecutePreFinalizers();
-  // TODO(chromium:1056170): replace build flag with dedicated flag.
-#if DEBUG
+#if CPPGC_VERIFY_HEAP
   UnifiedHeapMarkingVerifier verifier(*this);
   verifier.Run(stack_state_of_prev_gc(), stack_end_of_current_gc(),
                stats_collector()->marked_bytes());
-#endif
+#endif  // CPPGC_VERIFY_HEAP
 
   {
     cppgc::subtle::NoGarbageCollectionScope no_gc(*this);

src/heap/cppgc/heap.cc

@@ -188,12 +188,11 @@ void Heap::FinalizeGarbageCollection(Config::StackState stack_state) {
   }
   marker_.reset();
   ExecutePreFinalizers();
-  // TODO(chromium:1056170): replace build flag with dedicated flag.
-#if DEBUG
+#if CPPGC_VERIFY_HEAP
   MarkingVerifier verifier(*this);
   verifier.Run(config_.stack_state, stack_end_of_current_gc(),
                stats_collector()->marked_bytes());
-#endif
+#endif  // CPPGC_VERIFY_HEAP
 
   subtle::NoGarbageCollectionScope no_gc(*this);
   const Sweeper::SweepingConfig sweeping_config{

src/heap/cppgc/marking-verifier.cc

@@ -21,9 +21,9 @@ MarkingVerifierBase::MarkingVerifierBase(
       verification_state_(verification_state),
       visitor_(std::move(visitor)) {}
 
-void MarkingVerifierBase::Run(Heap::Config::StackState stack_state,
-                              uintptr_t stack_end,
-                              size_t expected_marked_bytes) {
+void MarkingVerifierBase::Run(
+    Heap::Config::StackState stack_state, uintptr_t stack_end,
+    v8::base::Optional<size_t> expected_marked_bytes) {
   Traverse(heap_.raw_heap());
   if (stack_state == Heap::Config::StackState::kMayContainHeapPointers) {
     in_construction_objects_ = &in_construction_objects_stack_;
@@ -38,9 +38,9 @@ void MarkingVerifierBase::Run(Heap::Config::StackState stack_state,
                in_construction_objects_heap_.find(header));
     }
   }
-#ifdef CPPGC_VERIFY_LIVE_BYTES
-  CHECK_EQ(expected_marked_bytes, found_marked_bytes_);
-#endif  // CPPGC_VERIFY_LIVE_BYTES
+  if (expected_marked_bytes) {
+    CHECK_EQ(expected_marked_bytes.value(), found_marked_bytes_);
+  }
 }
 
 void VerificationState::VerifyMarked(const void* base_object_payload) const {

src/heap/cppgc/marking-verifier.h

@@ -7,6 +7,7 @@
 
 #include <unordered_set>
 
+#include "src/base/optional.h"
 #include "src/heap/base/stack.h"
 #include "src/heap/cppgc/heap-object-header.h"
 #include "src/heap/cppgc/heap-visitor.h"
@@ -40,7 +41,7 @@ class V8_EXPORT_PRIVATE MarkingVerifierBase
   MarkingVerifierBase(const MarkingVerifierBase&) = delete;
   MarkingVerifierBase& operator=(const MarkingVerifierBase&) = delete;
 
-  void Run(Heap::Config::StackState, uintptr_t, size_t);
+  void Run(Heap::Config::StackState, uintptr_t, v8::base::Optional<size_t>);
 
  protected:
   MarkingVerifierBase(HeapBase&, VerificationState&,

src/heap/cppgc/stats-collector.cc

@@ -41,19 +41,19 @@ void StatsCollector::NotifyAllocation(size_t bytes) {
   // The current GC may not have been started. This is ok as recording considers
   // the whole time range between garbage collections.
   allocated_bytes_since_safepoint_ += bytes;
-#ifdef CPPGC_VERIFY_LIVE_BYTES
-  DCHECK_GE(live_bytes_ + bytes, live_bytes_);
-  live_bytes_ += bytes;
-#endif  // CPPGC_VERIFY_LIVE_BYTES
+#ifdef CPPGC_VERIFY_HEAP
+  DCHECK_GE(tracked_live_bytes_ + bytes, tracked_live_bytes_);
+  tracked_live_bytes_ += bytes;
+#endif  // CPPGC_VERIFY_HEAP
 }
 
 void StatsCollector::NotifyExplicitFree(size_t bytes) {
   // See IncreaseAllocatedObjectSize for lifetime of the counter.
   explicitly_freed_bytes_since_safepoint_ += bytes;
-#ifdef CPPGC_VERIFY_LIVE_BYTES
-  DCHECK_GE(live_bytes_, bytes);
-  live_bytes_ -= bytes;
-#endif  // CPPGC_VERIFY_LIVE_BYTES
+#ifdef CPPGC_VERIFY_HEAP
+  DCHECK_GE(tracked_live_bytes_, bytes);
+  tracked_live_bytes_ -= bytes;
+#endif  // CPPGC_VERIFY_HEAP
 }
 
 void StatsCollector::NotifySafePointForConservativeCollection() {
@@ -124,9 +124,9 @@ void StatsCollector::NotifyMarkingCompleted(size_t marked_bytes) {
       explicitly_freed_bytes_since_safepoint_;
   allocated_bytes_since_safepoint_ = 0;
   explicitly_freed_bytes_since_safepoint_ = 0;
-#ifdef CPPGC_VERIFY_LIVE_BYTES
-  live_bytes_ = marked_bytes;
-#endif  // CPPGC_VERIFY_LIVE_BYTES
+#ifdef CPPGC_VERIFY_HEAP
+  tracked_live_bytes_ = marked_bytes;
+#endif  // CPPGC_VERIFY_HEAP
 
   DCHECK_LE(memory_freed_bytes_since_end_of_marking_, memory_allocated_bytes_);
   memory_allocated_bytes_ -= memory_freed_bytes_since_end_of_marking_;

src/heap/cppgc/stats-collector.h

@@ -334,9 +334,10 @@ class V8_EXPORT_PRIVATE StatsCollector final {
   // arithmetic for simplicity.
   int64_t allocated_bytes_since_safepoint_ = 0;
   int64_t explicitly_freed_bytes_since_safepoint_ = 0;
-#ifdef CPPGC_VERIFY_LIVE_BYTES
-  size_t live_bytes_ = 0;
-#endif  // CPPGC_VERIFY_LIVE_BYTES
+#ifdef CPPGC_VERIFY_HEAP
+  // Tracks live bytes for overflows.
+  size_t tracked_live_bytes_ = 0;
+#endif  // CPPGC_VERIFY_HEAP
 
   int64_t memory_allocated_bytes_ = 0;
   int64_t memory_freed_bytes_since_end_of_marking_ = 0;

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

@@ -198,7 +198,7 @@ TEST_F(MarkingVerifierDeathTest, DieOnUnmarkedWeakMember) {
       "");
 }
 
-#ifdef CPPGC_VERIFY_LIVE_BYTES
+#ifdef CPPGC_VERIFY_HEAP
 
 TEST_F(MarkingVerifierDeathTest, DieOnUnexpectedLiveByteCount) {
   GCed* object = MakeGarbageCollected<GCed>(GetAllocationHandle());
@@ -210,7 +210,7 @@ TEST_F(MarkingVerifierDeathTest, DieOnUnexpectedLiveByteCount) {
                             "");
 }
 
-#endif  // CPPGC_VERIFY_LIVE_BYTES
+#endif  // CPPGC_VERIFY_HEAP
 
 namespace {
 
@@ -256,7 +256,7 @@ void MarkingVerifierDeathTest::TestResurrectingPreFinalizer() {
   EXPECT_DEATH_IF_SUPPORTED(PreciseGC(), "");
 }
 
-#if DEBUG
+#if CPPGC_VERIFY_HEAP
 
 TEST_F(MarkingVerifierDeathTest, DiesOnResurrectedMember) {
   TestResurrectingPreFinalizer<Member>();
@@ -266,7 +266,7 @@ TEST_F(MarkingVerifierDeathTest, DiesOnResurrectedWeakMember) {
   TestResurrectingPreFinalizer<WeakMember>();
 }
 
-#endif  // DEBUG
+#endif  // CPPGC_VERIFY_HEAP
 
 }  // namespace internal
 }  // namespace cppgc

tools/cppgc/gen_cmake.py

@@ -244,7 +244,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 option(CPPGC_ENABLE_OBJECT_NAMES "Enable object names in cppgc for debug purposes" OFF)
 option(CPPGC_ENABLE_CAGED_HEAP "Enable heap reservation of size 4GB, only possible for 64bit archs" OFF)
-option(CPPGC_ENABLE_VERIFY_LIVE_BYTES " Enable verification of live bytes in the marking verifier" OFF)
+option(CPPGC_ENABLE_VERIFY_HEAP "Enables additional heap verification phases and checks" OFF)
 option(CPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS " Enable assignment checks for Members/Persistents during prefinalizer invocations" OFF)
 option(CPPGC_ENABLE_YOUNG_GENERATION "Enable young generation in cppgc" OFF)
 set(CPPGC_TARGET_ARCH "x64" CACHE STRING "Target architecture, possible options: x64, x86, arm, arm64, ppc64, s390x, mipsel, mips64el")
@@ -435,8 +435,8 @@ endif()
 if(CPPGC_ENABLE_CAGED_HEAP)
   target_compile_definitions({target.name} PRIVATE "-DCPPGC_CAGED_HEAP")
 endif()
-if(CPPGC_ENABLE_VERIFY_LIVE_BYTES)
-  target_compile_definitions({target.name} PRIVATE "-DCPPGC_VERIFY_LIVE_BYTES")
+if(CPPGC_ENABLE_VERIFY_HEAP)
+  target_compile_definitions({target.name} PRIVATE "-DCPPGC_ENABLE_VERIFY_HEAP")
 endif()
 if(CPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS)
   target_compile_definitions({target.name} PRIVATE "-DCPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS")