[heap] Remove #ifdef usages with HeapVerifier

This CL defines empty methods when building without VERIFY_HEAP. This
removes the need for some preprocessor statements around heap
verification.

Bug: v8:11708
Change-Id: I354fd2793ab5d8ca4ab7de0822c832e2a868d832
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3872267Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83005}

src/flags/flag-definitions.h

@@ -1365,6 +1365,9 @@ DEFINE_IMPLICATION(trace_detached_contexts, track_detached_contexts)
 DEFINE_BOOL(verify_heap, false, "verify heap pointers before and after GC")
 DEFINE_BOOL(verify_heap_skip_remembered_set, false,
             "disable remembered set verification")
+#else
+DEFINE_BOOL_READONLY(verify_heap, false,
+                     "verify heap pointers before and after GC")
 #endif
 DEFINE_BOOL(move_object_start, true, "enable moving of object starts")
 DEFINE_BOOL(memory_reducer, true, "use memory reducer")

src/heap/gc-tracer.cc

@@ -511,16 +511,16 @@ void GCTracer::StopYoungCycleIfNeeded() {
 }
 
 void GCTracer::NotifySweepingCompleted() {
-#ifdef VERIFY_HEAP
-  // If heap verification is enabled, sweeping finalization can also be
-  // triggered from inside a full GC cycle's atomic pause.
-  DCHECK((current_.type == Event::MARK_COMPACTOR ||
-          current_.type == Event::INCREMENTAL_MARK_COMPACTOR) &&
-         (current_.state == Event::State::SWEEPING ||
-          (v8_flags.verify_heap && current_.state == Event::State::ATOMIC)));
-#else
-  DCHECK(IsSweepingInProgress());
-#endif
+  if (v8_flags.verify_heap) {
+    // If heap verification is enabled, sweeping finalization can also be
+    // triggered from inside a full GC cycle's atomic pause.
+    DCHECK((current_.type == Event::MARK_COMPACTOR ||
+            current_.type == Event::INCREMENTAL_MARK_COMPACTOR) &&
+           (current_.state == Event::State::SWEEPING ||
+            (v8_flags.verify_heap && current_.state == Event::State::ATOMIC)));
+  } else {
+    DCHECK(IsSweepingInProgress());
+  }
 
   // Stop a full GC cycle only when both v8 and cppgc (if available) GCs have
   // finished sweeping. This method is invoked by v8.

src/heap/heap-verifier.cc

@@ -68,6 +68,7 @@ class HeapVerification final {
 
 void HeapVerification::Verify() {
   CHECK(heap()->HasBeenSetUp());
+  AllowGarbageCollection allow_gc;
   IgnoreLocalGCRequests ignore_gc_requests(heap());
   SafepointScope safepoint_scope(heap());
   HandleScope scope(isolate());

src/heap/heap-verifier.h

@@ -6,9 +6,10 @@
 #define V8_HEAP_HEAP_VERIFIER_H_
 
 #include "src/common/globals.h"
+#include "src/flags/flags.h"
 #include "src/heap/read-only-heap.h"
+#include "src/objects/map.h"
 
-#ifdef VERIFY_HEAP
 namespace v8 {
 namespace internal {
 
@@ -17,6 +18,7 @@ class ReadOnlyHeap;
 
 class HeapVerifier final {
  public:
+#ifdef VERIFY_HEAP
   // Verify the heap is in its normal state before or after a GC.
   V8_EXPORT_PRIVATE static void VerifyHeap(Heap* heap);
 
@@ -43,6 +45,23 @@ class HeapVerifier final {
                                                          HeapObject object,
                                                          Map new_map);
 
+#else
+  static void VerifyHeap(Heap* heap) {}
+  static void VerifyReadOnlyHeap(Heap* heap) {}
+  static void VerifySharedHeap(Heap* heap, Isolate* initiator) {}
+  static void VerifyRememberedSetFor(Heap* heap, HeapObject object) {}
+  static void VerifySafeMapTransition(Heap* heap, HeapObject object,
+                                      Map new_map) {}
+  static void VerifyObjectLayoutChange(Heap* heap, HeapObject object,
+                                       Map new_map) {}
+#endif
+
+  V8_INLINE static void VerifyHeapIfEnabled(Heap* heap) {
+    if (v8_flags.verify_heap) {
+      VerifyHeap(heap);
+    }
+  }
+
  private:
   HeapVerifier();
 };
@@ -50,5 +69,4 @@ class HeapVerifier final {
 }  // namespace internal
 }  // namespace v8
 
-#endif  // VERIFY_HEAP
 #endif  // V8_HEAP_HEAP_VERIFIER_H_

src/heap/heap.cc

@@ -2263,7 +2263,6 @@ size_t Heap::PerformGarbageCollection(
 
   if (IsYoungGenerationCollector(collector)) {
     CompleteSweepingYoung(collector);
-#ifdef VERIFY_HEAP
     if (v8_flags.verify_heap) {
       // If heap verification is enabled, we want to ensure that sweeping is
       // completed here, as it will be triggered from Heap::Verify anyway.
@@ -2271,7 +2270,6 @@ size_t Heap::PerformGarbageCollection(
       // full GC cycle.
       CompleteSweepingFull();
     }
-#endif  // VERIFY_HEAP
     if (!v8_flags.minor_mc || incremental_marking_->IsStopped()) {
       // If v8_flags.minor_mc is false, then the young GC is Scavenger, which
       // may interrupt an incremental full GC. If MinorMC incremental marking
@@ -2312,14 +2310,7 @@ size_t Heap::PerformGarbageCollection(
 
   collection_barrier_->StopTimeToCollectionTimer();
 
-#ifdef VERIFY_HEAP
-  if (v8_flags.verify_heap) {
-    // We don't really perform a GC here but need this scope for the nested
-    // SafepointScope inside Verify().
-    AllowGarbageCollection allow_gc;
-    HeapVerifier::VerifyHeap(this);
-  }
-#endif  // VERIFY_HEAP
+  HeapVerifier::VerifyHeapIfEnabled(this);
 
   tracer()->StartInSafepoint();
 
@@ -2396,14 +2387,7 @@ size_t Heap::PerformGarbageCollection(
 
   tracer()->StopInSafepoint();
 
-#ifdef VERIFY_HEAP
-  if (v8_flags.verify_heap) {
-    // We don't really perform a GC here but need this scope for the nested
-    // SafepointScope inside Verify().
-    AllowGarbageCollection allow_gc;
-    HeapVerifier::VerifyHeap(this);
-  }
-#endif  // VERIFY_HEAP
+  HeapVerifier::VerifyHeapIfEnabled(this);
 
   return freed_global_handles;
 }
@@ -2443,28 +2427,14 @@ void Heap::PerformSharedGarbageCollection(Isolate* initiator,
       client->heap()->concurrent_marking()->Pause();
     }
 
-#ifdef VERIFY_HEAP
-    if (v8_flags.verify_heap) {
-      // We don't really perform a GC here but need this scope for the nested
-      // SafepointScope inside Verify().
-      AllowGarbageCollection allow_gc;
-      HeapVerifier::VerifyHeap(client->heap());
-    }
-#endif  // VERIFY_HEAP
+    HeapVerifier::VerifyHeapIfEnabled(client->heap());
   });
 
   const GarbageCollector collector = GarbageCollector::MARK_COMPACTOR;
   PerformGarbageCollection(collector, gc_reason, nullptr);
 
   isolate()->global_safepoint()->IterateClientIsolates([](Isolate* client) {
-#ifdef VERIFY_HEAP
-    if (v8_flags.verify_heap) {
-      // We don't really perform a GC here but need this scope for the nested
-      // SafepointScope inside Verify().
-      AllowGarbageCollection allow_gc;
-      HeapVerifier::VerifyHeap(client->heap());
-    }
-#endif  // VERIFY_HEAP
+    HeapVerifier::VerifyHeapIfEnabled(client->heap());
 
     if (v8_flags.concurrent_marking &&
         client->heap()->incremental_marking()->IsMarking()) {
@@ -2936,11 +2906,9 @@ void Heap::ExternalStringTable::UpdateYoungReferences(
 
   DCHECK(last <= end);
   young_strings_.resize(last - start);
-#ifdef VERIFY_HEAP
   if (v8_flags.verify_heap) {
     VerifyYoung();
   }
-#endif
 }
 
 void Heap::ExternalStringTable::PromoteYoung() {
@@ -5744,12 +5712,10 @@ void Heap::StartTearDown() {
 
   FreeMainThreadSharedLinearAllocationAreas();
 
-#ifdef VERIFY_HEAP
   // {StartTearDown} is called fairly early during Isolate teardown, so it's
   // a good time to run heap verification (if requested), before starting to
   // tear down parts of the Isolate.
   if (v8_flags.verify_heap) {
-    AllowGarbageCollection allow_gc;
     HeapVerifier::VerifyHeap(this);
 
     // If this is a client Isolate of a shared Isolate, verify that there are no
@@ -5759,7 +5725,6 @@ void Heap::StartTearDown() {
       HeapVerifier::VerifySharedHeap(shared_isolate->heap(), isolate());
     }
   }
-#endif
 }
 
 void Heap::TearDown() {
@@ -6411,11 +6376,9 @@ void Heap::ExternalStringTable::CleanUpAll() {
     old_strings_[last++] = o;
   }
   old_strings_.resize(last);
-#ifdef VERIFY_HEAP
   if (v8_flags.verify_heap && !v8_flags.enable_third_party_heap) {
     Verify();
   }
-#endif
 }
 
 void Heap::ExternalStringTable::TearDown() {

src/objects/objects-inl.h

@@ -830,7 +830,6 @@ void HeapObject::set_map(Map value, MemoryOrder order, VerificationMode mode) {
   // background threads.
   DCHECK_IMPLIES(mode != VerificationMode::kSafeMapTransition,
                  !LocalHeap::Current());
-#ifdef VERIFY_HEAP
   if (v8_flags.verify_heap && !value.is_null()) {
     Heap* heap = GetHeapFromWritableObject(*this);
     if (mode == VerificationMode::kSafeMapTransition) {
@@ -840,7 +839,6 @@ void HeapObject::set_map(Map value, MemoryOrder order, VerificationMode mode) {
       HeapVerifier::VerifyObjectLayoutChange(heap, *this, value);
     }
   }
-#endif
   set_map_word(MapWord::FromMap(value), order);
 #ifndef V8_DISABLE_WRITE_BARRIERS
   if (!value.is_null()) {