api,handles: Support for on-stack TracedReference

- Introduces a API to set top of the stack through
  EmbedderHeapTracer::SetStackTop.
- Introduces a new API to inform V8 about an empty embedder stack.
- Switch internal representation of TracedReference
  for on-stack handles to a proper stack that considers all
  contained handles as roots.
- Handle garbage is avoided by cleaning up on handle creation or
  GC.

Design doc: https://bit.ly/on-stack-traced-reference

Bug: chromium:1040038
Change-Id: I927ef0abb268fdb5853c9e17b1bc96e2491cf101
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1993973
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65757}

include/v8.h

@@ -1048,6 +1048,11 @@ class TracedGlobal : public TracedReferenceBase<T> {
  * to ensure that the handle is not accessed once the V8 object has been
  * reclaimed. This can happen when the handle is not passed through the
  * EmbedderHeapTracer. For more details see TracedReferenceBase.
+ *
+ * The reference assumes the embedder has precise knowledge about references at
+ * all times. In case V8 needs to separately handle on-stack references, the
+ * embedder is required to set the stack start through
+ * |EmbedderHeapTracer::SetStackStart|.
  */
 template <typename T>
 class TracedReference : public TracedReferenceBase<T> {
@@ -7870,6 +7875,17 @@ class V8_EXPORT EmbedderHeapTracer {
    */
   void IterateTracedGlobalHandles(TracedGlobalHandleVisitor* visitor);
 
+  /**
+   * Called by the embedder to set the start of the stack which is e.g. used by
+   * V8 to determine whether handles are used from stack or heap.
+   */
+  void SetStackStart(void* stack_start);
+
+  /**
+   * Called by the embedder to notify V8 of an empty execution stack.
+   */
+  void NotifyEmptyEmbedderStack();
+
   /**
    * Called by v8 to register internal fields of found wrappers.
    *

include/v8config.h

@@ -378,12 +378,11 @@
 
 
 // A macro used to tell the compiler to never inline a particular function.
-// Don't bother for debug builds.
 // Use like:
 //   V8_NOINLINE int GetMinusOne() { return -1; }
-#if !defined(DEBUG) && V8_HAS_ATTRIBUTE_NOINLINE
+#if V8_HAS_ATTRIBUTE_NOINLINE
 # define V8_NOINLINE __attribute__((noinline))
-#elif !defined(DEBUG) && V8_HAS_DECLSPEC_NOINLINE
+#elif V8_HAS_DECLSPEC_NOINLINE
 # define V8_NOINLINE __declspec(noinline)
 #else
 # define V8_NOINLINE /* NOT SUPPORTED */

src/api/api.cc

@@ -10750,6 +10750,19 @@ void HeapProfiler::RemoveBuildEmbedderGraphCallback(
       callback, data);
 }
 
+void EmbedderHeapTracer::SetStackStart(void* stack_start) {
+  CHECK(isolate_);
+  reinterpret_cast<i::Isolate*>(isolate_)->global_handles()->SetStackStart(
+      stack_start);
+}
+
+void EmbedderHeapTracer::NotifyEmptyEmbedderStack() {
+  CHECK(isolate_);
+  reinterpret_cast<i::Isolate*>(isolate_)
+      ->global_handles()
+      ->NotifyEmptyEmbedderStack();
+}
+
 void EmbedderHeapTracer::FinalizeTracing() {
   if (isolate_) {
     i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);

src/handles/global-handles.h

@@ -182,6 +182,11 @@ class V8_EXPORT_PRIVATE GlobalHandles final {
     return old;
   }
 
+  void SetStackStart(void* stack_start);
+  void NotifyEmptyEmbedderStack();
+  void CleanupOnStackReferencesBelowCurrentStackPosition();
+  size_t NumberOfOnStackHandlesForTesting();
+
 #ifdef DEBUG
   void PrintStats();
   void Print();
@@ -196,6 +201,7 @@ class V8_EXPORT_PRIVATE GlobalHandles final {
   class NodeSpace;
   class PendingPhantomCallback;
   class TracedNode;
+  class OnStackTracedNodeSpace;
 
   bool InRecursiveGC(unsigned gc_processing_counter);
 
@@ -224,6 +230,7 @@ class V8_EXPORT_PRIVATE GlobalHandles final {
 
   std::unique_ptr<NodeSpace<TracedNode>> traced_nodes_;
   std::vector<TracedNode*> traced_young_nodes_;
+  std::unique_ptr<OnStackTracedNodeSpace> on_stack_nodes_;
 
   // Field always containing the number of handles to global objects.
   size_t handles_count_ = 0;

src/heap/embedder-tracing.cc

@@ -69,6 +69,9 @@ void LocalEmbedderHeapTracer::SetEmbedderStackStateForNextFinalization(
   if (!InUse()) return;
 
   embedder_stack_state_ = stack_state;
+  if (EmbedderHeapTracer::EmbedderStackState::kEmpty == stack_state) {
+    remote_tracer()->NotifyEmptyEmbedderStack();
+  }
 }
 
 LocalEmbedderHeapTracer::ProcessingScope::ProcessingScope(

src/heap/embedder-tracing.h

@@ -145,6 +145,10 @@ class V8_EXPORT_PRIVATE EmbedderStackStateScope final {
       : local_tracer_(local_tracer),
         old_stack_state_(local_tracer_->embedder_stack_state_) {
     local_tracer_->embedder_stack_state_ = stack_state;
+    if (EmbedderHeapTracer::EmbedderStackState::kEmpty == stack_state) {
+      if (local_tracer->remote_tracer())
+        local_tracer->remote_tracer()->NotifyEmptyEmbedderStack();
+    }
   }
 
   ~EmbedderStackStateScope() {

src/heap/heap.cc

@@ -1520,6 +1520,11 @@ bool Heap::CollectGarbage(AllocationSpace space,
     InvokeNearHeapLimitCallback();
   }
 
+  // Filter on-stack reference below this method.
+  isolate()
+      ->global_handles()
+      ->CleanupOnStackReferencesBelowCurrentStackPosition();
+
   // Ensure that all pending phantom callbacks are invoked.
   isolate()->global_handles()->InvokeSecondPassPhantomCallbacks();
 

test/cctest/heap/test-embedder-tracing.cc

@@ -907,6 +907,171 @@ TEST(TracedGlobalNoDestructorReclaimedOnScavenge) {
   CHECK_EQ(initial_count, global_handles->handles_count());
 }
 
+namespace {
+
+V8_NOINLINE void TracedReferenceOnStack(TestEmbedderHeapTracer* tracer) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::Global<v8::Object> observer;
+  v8::TracedReference<v8::Value> stack_ref;
+  {
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Object> object(ConstructTraceableJSApiObject(
+        isolate->GetCurrentContext(), nullptr, nullptr));
+    stack_ref.Reset(isolate, object);
+    observer.Reset(isolate, object);
+    observer.SetWeak();
+  }
+  CHECK(!observer.IsEmpty());
+  heap::InvokeMarkSweep();
+  CHECK(!observer.IsEmpty());
+}
+
+V8_NOINLINE void CreateTracedReferenceInDeepStack(
+    v8::Isolate* isolate, v8::Global<v8::Object>* observer) {
+  v8::TracedReference<v8::Value> stack_ref;
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::Object> object(ConstructTraceableJSApiObject(
+      isolate->GetCurrentContext(), nullptr, nullptr));
+  stack_ref.Reset(isolate, object);
+  observer->Reset(isolate, object);
+  observer->SetWeak();
+}
+
+V8_NOINLINE void TracedReferenceNotifyEmptyStack(
+    TestEmbedderHeapTracer* tracer) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::Global<v8::Object> observer;
+  CreateTracedReferenceInDeepStack(isolate, &observer);
+  CHECK(!observer.IsEmpty());
+  tracer->NotifyEmptyEmbedderStack();
+  heap::InvokeMarkSweep();
+  CHECK(observer.IsEmpty());
+}
+
+V8_NOINLINE void TracedReferenceCopyStackToHeapTest(
+    TestEmbedderHeapTracer* tracer) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::Global<v8::Object> observer;
+  v8::TracedReference<v8::Value> stack_handle;
+  v8::TracedReference<v8::Value>* heap_handle =
+      new v8::TracedReference<v8::Value>();
+  {
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Object> object(ConstructTraceableJSApiObject(
+        isolate->GetCurrentContext(), nullptr, nullptr));
+    stack_handle.Reset(isolate, object);
+    observer.Reset(isolate, object);
+    observer.SetWeak();
+  }
+  CHECK(!observer.IsEmpty());
+  heap::InvokeMarkSweep();
+  CHECK(!observer.IsEmpty());
+  tracer->AddReferenceForTracing(heap_handle);
+  *heap_handle = stack_handle;
+  stack_handle.Reset();
+  heap::InvokeMarkSweep();
+  CHECK(!observer.IsEmpty());
+  heap::InvokeMarkSweep();
+  CHECK(observer.IsEmpty());
+  delete heap_handle;
+}
+
+V8_NOINLINE void TracedReferenceCopyHeapToStackTest(
+    TestEmbedderHeapTracer* tracer) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::Global<v8::Object> observer;
+  v8::TracedReference<v8::Value> stack_handle;
+  v8::TracedReference<v8::Value>* heap_handle =
+      new v8::TracedReference<v8::Value>();
+  {
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Object> object(ConstructTraceableJSApiObject(
+        isolate->GetCurrentContext(), nullptr, nullptr));
+    heap_handle->Reset(isolate, object);
+    observer.Reset(isolate, object);
+    observer.SetWeak();
+  }
+  CHECK(!observer.IsEmpty());
+  tracer->AddReferenceForTracing(heap_handle);
+  heap::InvokeMarkSweep();
+  CHECK(!observer.IsEmpty());
+  stack_handle = *heap_handle;
+  delete heap_handle;
+  heap::InvokeMarkSweep();
+  CHECK(!observer.IsEmpty());
+  stack_handle.Reset();
+  heap::InvokeMarkSweep();
+  CHECK(observer.IsEmpty());
+}
+
+V8_NOINLINE void TracedReferenceCleanedTest(TestEmbedderHeapTracer* tracer) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::Object> object(ConstructTraceableJSApiObject(
+      isolate->GetCurrentContext(), nullptr, nullptr));
+  const size_t before =
+      CcTest::i_isolate()->global_handles()->NumberOfOnStackHandlesForTesting();
+  for (int i = 0; i < 100; i++) {
+    v8::TracedReference<v8::Value> stack_handle;
+    stack_handle.Reset(isolate, object);
+  }
+  CHECK_EQ(before + 1, CcTest::i_isolate()
+                           ->global_handles()
+                           ->NumberOfOnStackHandlesForTesting());
+}
+
+}  // namespace
+
+TEST(TracedReferenceOnStack) {
+  ManualGCScope manual_gc;
+  CcTest::InitializeVM();
+  TestEmbedderHeapTracer tracer;
+  heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
+                                                      &tracer);
+  tracer.SetStackStart(&manual_gc);
+  TracedReferenceOnStack(&tracer);
+}
+
+TEST(TracedReferenceNotifyEmptyStack) {
+  ManualGCScope manual_gc;
+  CcTest::InitializeVM();
+  TestEmbedderHeapTracer tracer;
+  heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
+                                                      &tracer);
+  tracer.SetStackStart(&manual_gc);
+  TracedReferenceNotifyEmptyStack(&tracer);
+}
+
+TEST(TracedReferenceCopyStackToHeap) {
+  ManualGCScope manual_gc;
+  CcTest::InitializeVM();
+  TestEmbedderHeapTracer tracer;
+  heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
+                                                      &tracer);
+  tracer.SetStackStart(&manual_gc);
+  TracedReferenceCopyStackToHeapTest(&tracer);
+}
+
+TEST(TracedReferenceCopyHeapToStack) {
+  ManualGCScope manual_gc;
+  CcTest::InitializeVM();
+  TestEmbedderHeapTracer tracer;
+  heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
+                                                      &tracer);
+  tracer.SetStackStart(&manual_gc);
+  TracedReferenceCopyHeapToStackTest(&tracer);
+}
+
+TEST(TracedReferenceCleaned) {
+  ManualGCScope manual_gc;
+  CcTest::InitializeVM();
+  TestEmbedderHeapTracer tracer;
+  heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
+                                                      &tracer);
+  tracer.SetStackStart(&manual_gc);
+  TracedReferenceCleanedTest(&tracer);
+}
+
 }  // namespace heap
 }  // namespace internal
 }  // namespace v8

test/unittests/heap/embedder-tracing-unittest.cc

@@ -109,9 +109,10 @@ TEST(LocalEmbedderHeapTracer, EnterFinalPauseDefaultStackStateUnkown) {
   local_tracer.EnterFinalPause();
 }
 
-TEST(LocalEmbedderHeapTracer, EnterFinalPauseStackStateIsForwarded) {
+TEST_F(LocalEmbedderHeapTracerWithIsolate,
+       EnterFinalPauseStackStateIsForwarded) {
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
-  LocalEmbedderHeapTracer local_tracer(nullptr);
+  LocalEmbedderHeapTracer local_tracer(isolate());
   local_tracer.SetRemoteTracer(&remote_tracer);
   local_tracer.SetEmbedderStackStateForNextFinalization(
       EmbedderHeapTracer::kEmpty);
@@ -119,9 +120,9 @@ TEST(LocalEmbedderHeapTracer, EnterFinalPauseStackStateIsForwarded) {
   local_tracer.EnterFinalPause();
 }
 
-TEST(LocalEmbedderHeapTracer, TemporaryEmbedderStackState) {
+TEST_F(LocalEmbedderHeapTracerWithIsolate, TemporaryEmbedderStackState) {
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
-  LocalEmbedderHeapTracer local_tracer(nullptr);
+  LocalEmbedderHeapTracer local_tracer(isolate());
   local_tracer.SetRemoteTracer(&remote_tracer);
   // Default is unknown, see above.
   {
@@ -131,9 +132,10 @@ TEST(LocalEmbedderHeapTracer, TemporaryEmbedderStackState) {
   }
 }
 
-TEST(LocalEmbedderHeapTracer, TemporaryEmbedderStackStateRestores) {
+TEST_F(LocalEmbedderHeapTracerWithIsolate,
+       TemporaryEmbedderStackStateRestores) {
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
-  LocalEmbedderHeapTracer local_tracer(nullptr);
+  LocalEmbedderHeapTracer local_tracer(isolate());
   local_tracer.SetRemoteTracer(&remote_tracer);
   // Default is unknown, see above.
   {
@@ -149,9 +151,9 @@ TEST(LocalEmbedderHeapTracer, TemporaryEmbedderStackStateRestores) {
   }
 }
 
-TEST(LocalEmbedderHeapTracer, EnterFinalPauseStackStateResets) {
+TEST_F(LocalEmbedderHeapTracerWithIsolate, EnterFinalPauseStackStateResets) {
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
-  LocalEmbedderHeapTracer local_tracer(nullptr);
+  LocalEmbedderHeapTracer local_tracer(isolate());
   local_tracer.SetRemoteTracer(&remote_tracer);
   local_tracer.SetEmbedderStackStateForNextFinalization(
       EmbedderHeapTracer::kEmpty);