cppgc: LivenessBroker: Treat nullptr as live

nullptr objects are considered alive to allow weakness to be used from
stack while running into a conservative GC. Treating nullptr as dead
would mean that e.g. custom collectins could not be strongified on
stack.

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

include/cppgc/liveness-broker.h

@@ -44,7 +44,10 @@ class V8_EXPORT LivenessBroker final {
  public:
   template <typename T>
   bool IsHeapObjectAlive(const T* object) const {
-    return object &&
+    // nullptr objects are considered alive to allow weakness to be used from
+    // stack while running into a conservative GC. Treating nullptr as dead
+    // would mean that e.g. custom collectins could not be strongified on stack.
+    return !object ||
            IsHeapObjectAliveImpl(
                TraceTrait<T>::GetTraceDescriptor(object).base_object_payload);
   }

test/unittests/BUILD.gn

@@ -106,6 +106,7 @@ v8_source_set("cppgc_unittests_sources") {
     "heap/cppgc/heap-statistics-collector-unittest.cc",
     "heap/cppgc/heap-unittest.cc",
     "heap/cppgc/incremental-marking-schedule-unittest.cc",
+    "heap/cppgc/liveness-broker-unittest.cc",
     "heap/cppgc/logging-unittest.cc",
     "heap/cppgc/marker-unittest.cc",
     "heap/cppgc/marking-verifier-unittest.cc",

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

@@ -162,9 +162,12 @@ TEST_F(EphemeronPairTest, EmptyKey) {
 using EphemeronPairGCTest = testing::TestWithHeap;
 
 TEST_F(EphemeronPairGCTest, EphemeronPairValueIsCleared) {
+  GCed* key = MakeGarbageCollected<GCed>(GetAllocationHandle());
   GCed* value = MakeGarbageCollected<GCed>(GetAllocationHandle());
-  Persistent<EphemeronHolder> holder = MakeGarbageCollected<EphemeronHolder>(
-      GetAllocationHandle(), nullptr, value);
+  Persistent<EphemeronHolder> holder =
+      MakeGarbageCollected<EphemeronHolder>(GetAllocationHandle(), key, value);
+  // The precise GC will not find the `key` anywhere and thus clear the
+  // ephemeron.
   PreciseGC();
   EXPECT_EQ(nullptr, holder->ephemeron_pair().value.Get());
 }

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

@@ -367,16 +367,5 @@ TEST_F(GCHeapDeathTest, LargeChainOfNewStates) {
   EXPECT_DEATH_IF_SUPPORTED(Heap::From(GetHeap())->Terminate(), "");
 }
 
-TEST_F(GCHeapTest, IsHeapObjectAliveForConstPointer) {
-  // Regression test: http://crbug.com/661363.
-  GCed<64>* object = MakeGarbageCollected<GCed<64>>(GetAllocationHandle());
-  HeapObjectHeader& header = HeapObjectHeader::FromObject(object);
-  LivenessBroker broker = internal::LivenessBrokerFactory::Create();
-  EXPECT_TRUE(header.TryMarkAtomic());
-  EXPECT_TRUE(broker.IsHeapObjectAlive(object));
-  const GCed<64>* const_object = const_cast<const GCed<64>*>(object);
-  EXPECT_TRUE(broker.IsHeapObjectAlive(const_object));
-}
-
 }  // namespace internal
 }  // namespace cppgc

test/unittests/heap/cppgc/liveness-broker-unittest.cc

+// Copyright 2021 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 "include/cppgc/liveness-broker.h"
+
+#include "include/cppgc/allocation.h"
+#include "include/cppgc/garbage-collected.h"
+#include "src/heap/cppgc/heap-object-header.h"
+#include "src/heap/cppgc/liveness-broker.h"
+#include "test/unittests/heap/cppgc/tests.h"
+
+namespace cppgc {
+namespace internal {
+
+namespace {
+
+using LivenessBrokerTest = testing::TestSupportingAllocationOnly;
+
+class GCed : public GarbageCollected<GCed> {
+ public:
+  void Trace(cppgc::Visitor*) const {}
+};
+
+}  // namespace
+
+TEST_F(LivenessBrokerTest, IsHeapObjectAliveForConstPointer) {
+  // Regression test: http://crbug.com/661363.
+  GCed* object = MakeGarbageCollected<GCed>(GetAllocationHandle());
+  HeapObjectHeader& header = HeapObjectHeader::FromObject(object);
+  LivenessBroker broker = internal::LivenessBrokerFactory::Create();
+  EXPECT_TRUE(header.TryMarkAtomic());
+  EXPECT_TRUE(broker.IsHeapObjectAlive(object));
+  const GCed* const_object = const_cast<const GCed*>(object);
+  EXPECT_TRUE(broker.IsHeapObjectAlive(const_object));
+}
+
+TEST_F(LivenessBrokerTest, IsHeapObjectAliveNullptr) {
+  GCed* object = nullptr;
+  LivenessBroker broker = internal::LivenessBrokerFactory::Create();
+  EXPECT_TRUE(broker.IsHeapObjectAlive(object));
+}
+
+}  // namespace internal
+}  // namespace cppgc