cppgc: Check ephemerons for unset key

Ephemerons are based around WeakMember which may just be null at the
time the pair is considered for liveness. Bail out of marking for null
keys, as they write barrier would anyways make the value strong when
marking the key.

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

include/cppgc/ephemeron-pair.h

@@ -5,6 +5,7 @@
 #ifndef INCLUDE_CPPGC_EPHEMERON_PAIR_H_
 #define INCLUDE_CPPGC_EPHEMERON_PAIR_H_
 
+#include "cppgc/liveness-broker.h"
 #include "cppgc/member.h"
 
 namespace cppgc {
@@ -18,6 +19,10 @@ struct EphemeronPair {
   EphemeronPair(K* k, V* v) : key(k), value(v) {}
   WeakMember<K> key;
   Member<V> value;
+
+  void ClearValueIfKeyIsDead(const LivenessBroker& broker) {
+    if (!broker.IsHeapObjectAlive(key)) value = nullptr;
+  }
 };
 
 }  // namespace cppgc

include/cppgc/visitor.h

@@ -152,6 +152,9 @@ class V8_EXPORT Visitor {
   template <typename K, typename V>
   void Trace(const EphemeronPair<K, V>& ephemeron_pair) {
     TraceEphemeron(ephemeron_pair.key, &ephemeron_pair.value);
+    RegisterWeakCallbackMethod<EphemeronPair<K, V>,
+                               &EphemeronPair<K, V>::ClearValueIfKeyIsDead>(
+        &ephemeron_pair);
   }
 
   /**
@@ -163,6 +166,8 @@ class V8_EXPORT Visitor {
    */
   template <typename K, typename V>
   void TraceEphemeron(const WeakMember<K>& key, const V* value) {
+    const K* k = key.GetRawAtomic();
+    if (!k) return;
     TraceDescriptor value_desc = TraceTrait<V>::GetTraceDescriptor(value);
     if (!value_desc.base_object_payload) return;
     VisitEphemeron(key, value_desc);

test/unittests/heap/cppgc/ephemeron-pair-unittest.cc

@@ -26,6 +26,10 @@ class EphemeronHolder : public GarbageCollected<GCed> {
   EphemeronHolder(GCed* key, GCed* value) : ephemeron_pair_(key, value) {}
   void Trace(cppgc::Visitor* visitor) const { visitor->Trace(ephemeron_pair_); }
 
+  const EphemeronPair<GCed, GCed>& ephemeron_pair() const {
+    return ephemeron_pair_;
+  }
+
  private:
   EphemeronPair<GCed, GCed> ephemeron_pair_;
 };
@@ -143,5 +147,26 @@ TEST_F(EphemeronPairTest, EmptyValue) {
   FinishMarking();
 }
 
+TEST_F(EphemeronPairTest, EmptyKey) {
+  GCed* value = MakeGarbageCollected<GCed>(GetAllocationHandle());
+  Persistent<EphemeronHolderTraceEphemeron> holder =
+      MakeGarbageCollected<EphemeronHolderTraceEphemeron>(GetAllocationHandle(),
+                                                          nullptr, value);
+  InitializeMarker(*Heap::From(GetHeap()), GetPlatformHandle().get());
+  FinishMarking();
+  // Key is not alive and value should thus not be held alive.
+  EXPECT_FALSE(HeapObjectHeader::FromPayload(value).IsMarked());
+}
+
+using EphemeronPairGCTest = testing::TestWithHeap;
+
+TEST_F(EphemeronPairGCTest, EphemeronPairValueIsCleared) {
+  GCed* value = MakeGarbageCollected<GCed>(GetAllocationHandle());
+  Persistent<EphemeronHolder> holder = MakeGarbageCollected<EphemeronHolder>(
+      GetAllocationHandle(), nullptr, value);
+  PreciseGC();
+  EXPECT_EQ(nullptr, holder->ephemeron_pair().value.Get());
+}
+
 }  // namespace internal
 }  // namespace cppgc