cppgc,api: Add support for JS->C++ write barriers

Provide a way to trigger a write barrier when updating the embedder
fields. In future, such a mechanism should be encapsulated into V8.

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

include/cppgc/internal/write-barrier.h

@@ -57,6 +57,9 @@ class V8_EXPORT WriteBarrier final {
   // Returns the required write barrier for a given `slot`.
   static V8_INLINE Type GetWriteBarrierType(const void* slot, Params& params);
 
+  static V8_INLINE Type GetWriteBarrierTypeForExternallyReferencedObject(
+      const void* value, Params& params);
+
   static V8_INLINE void DijkstraMarkingBarrier(const Params& params,
                                                const void* object);
   static V8_INLINE void DijkstraMarkingBarrierRange(
@@ -140,6 +143,20 @@ class WriteBarrierTypeForCagedHeapPolicy final {
 #endif  // !CPPGC_YOUNG_GENERATION
   }
 
+  static V8_INLINE WriteBarrier::Type GetForExternallyReferenced(
+      const void* value, WriteBarrier::Params& params) {
+    if (!TryGetCagedHeap(value, value, params)) {
+      return WriteBarrier::Type::kNone;
+    }
+    if (V8_UNLIKELY(params.caged_heap().is_marking_in_progress)) {
+#if V8_ENABLE_CHECKS
+      params.type = WriteBarrier::Type::kMarking;
+#endif  // !V8_ENABLE_CHECKS
+      return WriteBarrier::Type::kMarking;
+    }
+    return WriteBarrier::Type::kNone;
+  }
+
  private:
   WriteBarrierTypeForCagedHeapPolicy() = delete;
 
@@ -164,6 +181,17 @@ class WriteBarrierTypeForNonCagedHeapPolicy final {
   template <WriteBarrier::ValueMode value_mode>
   static V8_INLINE WriteBarrier::Type Get(const void* slot, const void* value,
                                           WriteBarrier::Params& params) {
+    return GetInternal(params);
+  }
+
+  static V8_INLINE WriteBarrier::Type GetForExternallyReferenced(
+      const void* value, WriteBarrier::Params& params) {
+    return GetInternal(params);
+  }
+
+ private:
+  static V8_INLINE WriteBarrier::Type GetInternal(
+      WriteBarrier::Params& params) {
     WriteBarrier::Type type =
         V8_LIKELY(!ProcessHeap::IsAnyIncrementalOrConcurrentMarking())
             ? WriteBarrier::Type::kNone
@@ -174,7 +202,6 @@ class WriteBarrierTypeForNonCagedHeapPolicy final {
     return type;
   }
 
- private:
   WriteBarrierTypeForNonCagedHeapPolicy() = delete;
 };
 
@@ -192,6 +219,13 @@ WriteBarrier::Type WriteBarrier::GetWriteBarrierType(
                                                                  params);
 }
 
+// static
+WriteBarrier::Type
+WriteBarrier::GetWriteBarrierTypeForExternallyReferencedObject(
+    const void* value, Params& params) {
+  return WriteBarrierTypePolicy::GetForExternallyReferenced(value, params);
+}
+
 // static
 void WriteBarrier::DijkstraMarkingBarrier(const Params& params,
                                           const void* object) {

include/v8-cppgc.h

@@ -83,7 +83,7 @@ class JSVisitor : public cppgc::Visitor {
  * Consistency helpers that aid in maintaining a consistent internal state of
  * the garbage collector.
  */
-class JSHeapConsistency final {
+class V8_EXPORT JSHeapConsistency final {
  public:
   using WriteBarrierParams = cppgc::internal::WriteBarrier::Params;
   using WriteBarrierType = cppgc::internal::WriteBarrier::Type;
@@ -91,6 +91,8 @@ class JSHeapConsistency final {
   /**
    * Gets the required write barrier type for a specific write.
    *
+   * Note: Handling for C++ to JS references.
+   *
    * \param ref The reference being written to.
    * \param params Parameters that may be used for actual write barrier calls.
    *   Only filled if return value indicates that a write barrier is needed. The
@@ -103,6 +105,30 @@ class JSHeapConsistency final {
     return cppgc::internal::WriteBarrier::GetWriteBarrierType(&ref, params);
   }
 
+  /**
+   * Gets the required write barrier type for a specific write.
+   *
+   * Note: Handling for JS to C++ references.
+   *
+   * \param wrapper The wrapper that has been written into.
+   * \param wrapper_index The wrapper index in `wrapper` that has been written
+   *   into.
+   * \param wrappable The value that was written.
+   * \param params Parameters that may be used for actual write barrier calls.
+   *   Only filled if return value indicates that a write barrier is needed. The
+   *   contents of the `params` are an implementation detail.
+   * \returns whether a write barrier is needed and which barrier to invoke.
+   */
+  static V8_INLINE WriteBarrierType
+  GetWriteBarrierType(v8::Local<v8::Object>& wrapper, int wrapper_index,
+                      const void* wrappable, WriteBarrierParams& params) {
+#if V8_ENABLE_CHECKS
+    CheckWrapper(wrapper, wrapper_index, wrappable);
+#endif  // V8_ENABLE_CHECKS
+    return cppgc::internal::WriteBarrier::
+        GetWriteBarrierTypeForExternallyReferencedObject(wrappable, params);
+  }
+
   /**
    * Conservative Dijkstra-style write barrier that processes an object if it
    * has not yet been processed.
@@ -118,6 +144,20 @@ class JSHeapConsistency final {
     DijkstraMarkingBarrierSlow(heap_handle, ref);
   }
 
+  /**
+   * Conservative Dijkstra-style write barrier that processes an object if it
+   * has not yet been processed.
+   *
+   * \param params The parameters retrieved from `GetWriteBarrierType()`.
+   * \param object The pointer to the object. May be an interior pointer to a
+   *   an interface of the actual object.
+   */
+  static V8_INLINE void DijkstraMarkingBarrier(const WriteBarrierParams& params,
+                                               cppgc::HeapHandle& heap_handle,
+                                               const void* object) {
+    cppgc::internal::WriteBarrier::DijkstraMarkingBarrier(params, object);
+  }
+
   /**
    * Generational barrier for maintaining consistency when running with multiple
    * generations.
@@ -131,6 +171,8 @@ class JSHeapConsistency final {
  private:
   JSHeapConsistency() = delete;
 
+  static void CheckWrapper(v8::Local<v8::Object>&, int, const void*);
+
   static void DijkstraMarkingBarrierSlow(cppgc::HeapHandle&,
                                          const TracedReferenceBase& ref);
 };

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

@@ -50,6 +50,12 @@ void JSHeapConsistency::DijkstraMarkingBarrierSlow(
   static_cast<JSVisitor*>(&heap_base.marker()->Visitor())->Trace(ref);
 }
 
+void JSHeapConsistency::CheckWrapper(v8::Local<v8::Object>& wrapper,
+                                     int wrapper_index, const void* wrappable) {
+  CHECK_EQ(wrappable,
+           wrapper->GetAlignedPointerFromInternalField(wrapper_index));
+}
+
 namespace internal {
 
 namespace {
@@ -169,7 +175,6 @@ CppHeap::CppHeap(
                                 cppgc::internal::HeapBase::StackSupport::
                                     kSupportsConservativeStackScan),
       isolate_(*reinterpret_cast<Isolate*>(isolate)) {
-  CHECK(!FLAG_incremental_marking_wrappers);
   if (isolate_.heap_profiler()) {
     isolate_.heap_profiler()->AddBuildEmbedderGraphCallback(
         &CppGraphBuilder::Run, this);

test/unittests/BUILD.gn

@@ -300,6 +300,7 @@ v8_source_set("unittests_sources") {
     "heap/gc-tracer-unittest.cc",
     "heap/heap-controller-unittest.cc",
     "heap/heap-unittest.cc",
+    "heap/heap-utils.cc",
     "heap/heap-utils.h",
     "heap/index-generator-unittest.cc",
     "heap/item-parallel-job-unittest.cc",

test/unittests/heap/heap-utils.cc

+// Copyright 2020 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 "test/unittests/heap/heap-utils.h"
+
+#include "src/heap/incremental-marking.h"
+#include "src/heap/mark-compact.h"
+#include "src/heap/safepoint.h"
+
+namespace v8 {
+namespace internal {
+
+void HeapInternalsBase::SimulateIncrementalMarking(Heap* heap,
+                                                   bool force_completion) {
+  constexpr double kStepSizeInMs = 100;
+  CHECK(FLAG_incremental_marking);
+  i::IncrementalMarking* marking = heap->incremental_marking();
+  i::MarkCompactCollector* collector = heap->mark_compact_collector();
+  if (collector->sweeping_in_progress()) {
+    SafepointScope scope(heap);
+    collector->EnsureSweepingCompleted();
+  }
+  CHECK(marking->IsMarking() || marking->IsStopped() || marking->IsComplete());
+  if (marking->IsStopped()) {
+    heap->StartIncrementalMarking(i::Heap::kNoGCFlags,
+                                  i::GarbageCollectionReason::kTesting);
+  }
+  CHECK(marking->IsMarking() || marking->IsComplete());
+  if (!force_completion) return;
+
+  while (!marking->IsComplete()) {
+    marking->Step(kStepSizeInMs, i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
+                  i::StepOrigin::kV8);
+    if (marking->IsReadyToOverApproximateWeakClosure()) {
+      SafepointScope scope(heap);
+      marking->FinalizeIncrementally();
+    }
+  }
+  CHECK(marking->IsComplete());
+}
+
+}  // namespace internal
+}  // namespace v8

test/unittests/heap/heap-utils.h

@@ -13,8 +13,13 @@
 namespace v8 {
 namespace internal {
 
+class HeapInternalsBase {
+ protected:
+  void SimulateIncrementalMarking(Heap* heap, bool force_completion);
+};
+
 template <typename TMixin>
-class WithHeapInternals : public TMixin {
+class WithHeapInternals : public TMixin, HeapInternalsBase {
  public:
   WithHeapInternals() = default;
   WithHeapInternals(const WithHeapInternals&) = delete;
@@ -25,6 +30,11 @@ class WithHeapInternals : public TMixin {
   }
 
   Heap* heap() const { return this->i_isolate()->heap(); }
+
+  void SimulateIncrementalMarking(bool force_completion = true) {
+    return HeapInternalsBase::SimulateIncrementalMarking(heap(),
+                                                         force_completion);
+  }
 };
 
 using TestWithHeapInternals =       //

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

@@ -5,6 +5,7 @@
 #include "include/cppgc/allocation.h"
 #include "include/cppgc/garbage-collected.h"
 #include "include/cppgc/platform.h"
+#include "include/v8-cppgc.h"
 #include "src/api/api-inl.h"
 #include "src/heap/cppgc-js/cpp-heap.h"
 #include "src/objects/objects-inl.h"
@@ -37,6 +38,7 @@ TEST_F(UnifiedHeapTest, FindingV8ToBlinkReference) {
   v8::Context::Scope context_scope(context);
   v8::Local<v8::Object> api_object = WrapperHelper::CreateWrapper(
       context, cppgc::MakeGarbageCollected<Wrappable>(allocation_handle()));
+  Wrappable::destructor_callcount = 0;
   EXPECT_FALSE(api_object.IsEmpty());
   EXPECT_EQ(0u, Wrappable::destructor_callcount);
   CollectGarbageWithoutEmbedderStack();
@@ -48,5 +50,32 @@ TEST_F(UnifiedHeapTest, FindingV8ToBlinkReference) {
   EXPECT_EQ(1u, Wrappable::destructor_callcount);
 }
 
+TEST_F(UnifiedHeapTest, WriteBarrierV8ToBlinkReference) {
+  v8::HandleScope scope(v8_isolate());
+  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
+  v8::Context::Scope context_scope(context);
+  void* wrappable = cppgc::MakeGarbageCollected<Wrappable>(allocation_handle());
+  v8::Local<v8::Object> api_object =
+      WrapperHelper::CreateWrapper(context, wrappable);
+  Wrappable::destructor_callcount = 0;
+  WrapperHelper::ResetWrappableConnection(api_object);
+  SimulateIncrementalMarking();
+  {
+    // The following snippet shows the embedder code for implementing a GC-safe
+    // setter for JS to C++ references.
+    WrapperHelper::SetWrappableConnection(api_object, wrappable, wrappable);
+    JSHeapConsistency::WriteBarrierParams params;
+    auto barrier_type = JSHeapConsistency::GetWriteBarrierType(
+        api_object, 1, wrappable, params);
+    EXPECT_EQ(JSHeapConsistency::WriteBarrierType::kMarking, barrier_type);
+    JSHeapConsistency::DijkstraMarkingBarrier(
+        params, cpp_heap().GetHeapHandle(), wrappable);
+  }
+  CollectGarbageWithoutEmbedderStack();
+  // Calling CollectGarbage twice to force the first GC to finish sweeping.
+  CollectGarbageWithoutEmbedderStack();
+  EXPECT_EQ(0u, Wrappable::destructor_callcount);
+}
+
 }  // namespace internal
 }  // namespace v8

test/unittests/heap/unified-heap-utils.cc

@@ -14,16 +14,10 @@
 namespace v8 {
 namespace internal {
 
-UnifiedHeapTest::UnifiedHeapTest()
-    : saved_incremental_marking_wrappers_(FLAG_incremental_marking_wrappers) {
-  FLAG_incremental_marking_wrappers = false;
+UnifiedHeapTest::UnifiedHeapTest() {
   isolate()->heap()->ConfigureCppHeap(std::make_unique<CppHeapCreateParams>());
 }
 
-UnifiedHeapTest::~UnifiedHeapTest() {
-  FLAG_incremental_marking_wrappers = saved_incremental_marking_wrappers_;
-}
-
 void UnifiedHeapTest::CollectGarbageWithEmbedderStack() {
   heap()->SetEmbedderStackStateForNextFinalization(
       EmbedderHeapTracer::EmbedderStackState::kMayContainHeapPointers);
@@ -76,5 +70,12 @@ void WrapperHelper::ResetWrappableConnection(v8::Local<v8::Object> api_object) {
   api_object->SetAlignedPointerInInternalField(1, nullptr);
 }
 
+// static
+void WrapperHelper::SetWrappableConnection(v8::Local<v8::Object> api_object,
+                                           void* field1, void* field2) {
+  api_object->SetAlignedPointerInInternalField(0, field1);
+  api_object->SetAlignedPointerInInternalField(1, field2);
+}
+
 }  // namespace internal
 }  // namespace v8

test/unittests/heap/unified-heap-utils.h

@@ -17,16 +17,13 @@ class CppHeap;
 class UnifiedHeapTest : public TestWithHeapInternals {
  public:
   UnifiedHeapTest();
-  ~UnifiedHeapTest() override;
+  ~UnifiedHeapTest() override = default;
 
   void CollectGarbageWithEmbedderStack();
   void CollectGarbageWithoutEmbedderStack();
 
   CppHeap& cpp_heap() const;
   cppgc::AllocationHandle& allocation_handle();
-
- private:
-  bool saved_incremental_marking_wrappers_;
 };
 
 class WrapperHelper {
@@ -41,6 +38,11 @@ class WrapperHelper {
   // Resets the connection of a wrapper (JS) to its wrappable (C++), meaning
   // that the wrappable object is not longer kept alive by the wrapper object.
   static void ResetWrappableConnection(v8::Local<v8::Object> api_object);
+
+  // Sets up the connection of a wrapper (JS) to its wrappable (C++). Does not
+  // emit any possibly needed write barrier.
+  static void SetWrappableConnection(v8::Local<v8::Object> api_object, void*,
+                                     void*);
 };
 
 }  // namespace internal