cppgc: Fix check for assignments in prefinalizers

Off heap members are "safe" to reference dead objects since they are not
connected to the object graph and do not ressurect the object.

This is needed becuase Members are used as temporary on stack variables
in Blink, e.g. when querying if a HeapHashMap contains a key.

Bug: v8:11749
Change-Id: I7ab2559d00c366480a3efbc0512bb1d1f63b64e7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3182223Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77063}

include/cppgc/internal/pointer-policies.h

@@ -51,7 +51,16 @@ struct NoWriteBarrierPolicy {
   static void AssigningBarrier(const void*, const void*) {}
 };
 
-class V8_EXPORT EnabledCheckingPolicy {
+class V8_EXPORT EnabledCheckingPolicyBase {
+ protected:
+  void CheckPointerImpl(const void* ptr, bool points_to_payload,
+                        bool check_off_heap_assignments);
+
+  const HeapBase* heap_ = nullptr;
+};
+
+template <bool kCheckOffHeapAssignments>
+class V8_EXPORT EnabledCheckingPolicy : private EnabledCheckingPolicyBase {
  protected:
   template <typename T>
   void CheckPointer(const T* ptr) {
@@ -61,23 +70,20 @@ class V8_EXPORT EnabledCheckingPolicy {
   }
 
  private:
-  void CheckPointerImpl(const void* ptr, bool points_to_payload);
-
   template <typename T, bool = IsCompleteV<T>>
   struct CheckPointersImplTrampoline {
     static void Call(EnabledCheckingPolicy* policy, const T* ptr) {
-      policy->CheckPointerImpl(ptr, false);
+      policy->CheckPointerImpl(ptr, false, kCheckOffHeapAssignments);
     }
   };
 
   template <typename T>
   struct CheckPointersImplTrampoline<T, true> {
     static void Call(EnabledCheckingPolicy* policy, const T* ptr) {
-      policy->CheckPointerImpl(ptr, IsGarbageCollectedTypeV<T>);
+      policy->CheckPointerImpl(ptr, IsGarbageCollectedTypeV<T>,
+                               kCheckOffHeapAssignments);
     }
   };
-
-  const HeapBase* heap_ = nullptr;
 };
 
 class DisabledCheckingPolicy {
@@ -86,8 +92,12 @@ class DisabledCheckingPolicy {
 };
 
 #if V8_ENABLE_CHECKS
-using DefaultMemberCheckingPolicy = EnabledCheckingPolicy;
-using DefaultPersistentCheckingPolicy = EnabledCheckingPolicy;
+// Off heap members are not connected to object graph and thus cannot ressurect
+// dead objects.
+using DefaultMemberCheckingPolicy =
+    EnabledCheckingPolicy<false /* kCheckOffHeapAssignments*/>;
+using DefaultPersistentCheckingPolicy =
+    EnabledCheckingPolicy<true /* kCheckOffHeapAssignments*/>;
 #else
 using DefaultMemberCheckingPolicy = DisabledCheckingPolicy;
 using DefaultPersistentCheckingPolicy = DisabledCheckingPolicy;

src/heap/cppgc/pointer-policies.cc

@@ -30,8 +30,8 @@ bool IsOnStack(const void* address) {
 
 }  // namespace
 
-void EnabledCheckingPolicy::CheckPointerImpl(const void* ptr,
-                                             bool points_to_payload) {
+void EnabledCheckingPolicyBase::CheckPointerImpl(
+    const void* ptr, bool points_to_payload, bool check_off_heap_assignments) {
   // `ptr` must not reside on stack.
   DCHECK(!IsOnStack(ptr));
   auto* base_page = BasePage::FromPayload(ptr);
@@ -41,12 +41,14 @@ void EnabledCheckingPolicy::CheckPointerImpl(const void* ptr,
 
   // References cannot change their heap association which means that state is
   // immutable once it is set.
+  bool is_on_heap = true;
   if (!heap_) {
     heap_ = &base_page->heap();
     if (!heap_->page_backend()->Lookup(reinterpret_cast<Address>(this))) {
       // If `this` is not contained within the heap of `ptr`, we must deal with
       // an on-stack or off-heap reference. For both cases there should be no
       // heap registered.
+      is_on_heap = false;
       CHECK(!HeapRegistry::TryFromManagedPointer(this));
     }
   }
@@ -69,18 +71,21 @@ void EnabledCheckingPolicy::CheckPointerImpl(const void* ptr,
   }
 
 #ifdef CPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS
+  if (check_off_heap_assignments || is_on_heap) {
     if (heap_->prefinalizer_handler()->IsInvokingPreFinalizers()) {
-    // During prefinalizers invocation, check that |ptr| refers to a live object
-    // and that it is assigned to a live slot.
+      // During prefinalizers invocation, check that |ptr| refers to a live
+      // object and that it is assigned to a live slot.
       DCHECK(header->IsMarked());
       // Slot can be in a large object.
       const auto* slot_page = BasePage::FromInnerAddress(heap_, this);
       // Off-heap slots (from other heaps or on-stack) are considered live.
       bool slot_is_live =
-        !slot_page || slot_page->ObjectHeaderFromInnerAddress(this).IsMarked();
+          !slot_page ||
+          slot_page->ObjectHeaderFromInnerAddress(this).IsMarked();
       DCHECK(slot_is_live);
       USE(slot_is_live);
     }
+  }
 #endif  // CPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS
 }