api,heap: Add support for on-stack TracedGlobal

Similar to TracedReference, support TracedGlobal on stack as well.

Bug: chromium:1040038
Change-Id: If3400a2df8b4a11410032bd5ad1b7bed64063b93
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2005071
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65841}

src/handles/global-handles.cc

@@ -19,6 +19,7 @@
 #include "src/sanitizer/asan.h"
 #include "src/tasks/cancelable-task.h"
 #include "src/tasks/task-utils.h"
+#include "src/utils/utils.h"
 
 namespace v8 {
 namespace internal {
@@ -690,6 +691,12 @@ class GlobalHandles::TracedNode final
     DCHECK(!IsInUse());
   }
 
+  void Verify() {
+    DCHECK(IsInUse());
+    DCHECK_IMPLIES(!has_destructor(), nullptr == parameter());
+    DCHECK_IMPLIES(has_destructor() && !HasFinalizationCallback(), parameter());
+  }
+
  protected:
   using NodeState = base::BitField8<State, 0, 2>;
   using IsInYoungList = NodeState::Next<bool, 1>;
@@ -762,14 +769,6 @@ class GlobalHandles::OnStackTracedNodeSpace final {
 
   uintptr_t GetStackAddressForSlot(uintptr_t slot) const;
 
-  V8_NOINLINE uintptr_t GetCurrentStackPosition() const {
-#if V8_CC_MSVC
-    return reinterpret_cast<uintptr_t>(_AddressOfReturnAddress());
-#else
-    return reinterpret_cast<uintptr_t>(__builtin_frame_address(0));
-#endif  // V8_CC_MSVC
-  }
-
   // Keeps track of registered handles and their stack address. The data
   // structure is cleaned on iteration and when adding new references using the
   // current stack address.
@@ -894,8 +893,6 @@ Handle<Object> GlobalHandles::CreateTraced(Object value, Address* slot,
                                            bool is_on_stack) {
   GlobalHandles::TracedNode* result;
   if (is_on_stack) {
-    CHECK_WITH_MSG(!has_destructor,
-                   "TracedGlobal is prohibited from on-stack usage.");
     result = on_stack_nodes_->Acquire(value, reinterpret_cast<uintptr_t>(slot));
   } else {
     result = traced_nodes_->Acquire(value);
@@ -903,9 +900,9 @@ Handle<Object> GlobalHandles::CreateTraced(Object value, Address* slot,
       traced_young_nodes_.push_back(result);
       result->set_in_young_list(true);
     }
-    result->set_parameter(slot);
-    result->set_has_destructor(has_destructor);
   }
+  result->set_has_destructor(has_destructor);
+  result->set_parameter(has_destructor ? slot : nullptr);
   return result->handle();
 }
 
@@ -934,7 +931,9 @@ void GlobalHandles::CopyTracedGlobal(const Address* const* from, Address** to) {
   // Copying a traced handle with finalization callback is prohibited because
   // the callback may require knowing about multiple copies of the traced
   // handle.
-  CHECK(!node->HasFinalizationCallback());
+  CHECK_WITH_MSG(!node->HasFinalizationCallback(),
+                 "Copying of references is not supported when "
+                 "SetFinalizationCallback is set.");
 
   GlobalHandles* global_handles =
       GlobalHandles::From(const_cast<TracedNode*>(node));
@@ -983,35 +982,53 @@ void GlobalHandles::MoveTracedGlobal(Address** from, Address** to) {
         reinterpret_cast<uintptr_t>(to));
   }
 
+  // Moving a traced handle with finalization callback is prohibited because
+  // the callback may require knowing about multiple copies of the traced
+  // handle.
+  CHECK_WITH_MSG(!from_node->HasFinalizationCallback(),
+                 "Moving of references is not supported when "
+                 "SetFinalizationCallback is set.");
+  // Types in v8.h ensure that we only copy/move handles that have the same
+  // destructor behavior.
+  DCHECK_IMPLIES(to_node,
+                 to_node->has_destructor() == from_node->has_destructor());
+
   // Moving.
   if (from_on_stack || to_on_stack) {
     // Move involving a stack slot.
-    DCHECK(!from_node->has_destructor());
-    DCHECK(!from_node->HasFinalizationCallback());
     if (!to_node) {
       DCHECK(global_handles);
       Handle<Object> o = global_handles->CreateTraced(
-          from_node->object(), reinterpret_cast<Address*>(to), false,
-          to_on_stack);
+          from_node->object(), reinterpret_cast<Address*>(to),
+          from_node->has_destructor(), to_on_stack);
       *to = o.location();
-      DCHECK(TracedNode::FromLocation(*to)->markbit());
+      to_node = TracedNode::FromLocation(*to);
+      DCHECK(to_node->markbit());
     } else {
       // To node already exists, just copy fields.
       *TracedNode::FromLocation(*to) = *from_node;
+      // Fixup back reference for destructor.
+      if (to_node->has_destructor()) {
+        to_node->set_parameter(to);
+      }
     }
     DestroyTraced(*from);
     *from = nullptr;
+    to_node->Verify();
   } else {
     // Pure heap move.
     DestroyTraced(*to);
     *to = *from;
+    to_node = from_node;
     DCHECK_NOT_NULL(*from);
     DCHECK_NOT_NULL(*to);
     DCHECK_EQ(*from, *to);
-    if (!from_node->HasFinalizationCallback()) {
-      from_node->set_parameter(to);
+    // Fixup back reference for destructor.
+    if (to_node->has_destructor()) {
+      to_node->set_parameter(to);
     }
     *from = nullptr;
+    to_node->Verify();
   }
 }
 

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

@@ -56,7 +56,7 @@ class TestEmbedderHeapTracer final : public v8::EmbedderHeapTracer {
                                embedder_fields.begin(), embedder_fields.end());
   }
 
-  void AddReferenceForTracing(v8::TracedGlobal<v8::Object>* global) {
+  void AddReferenceForTracing(v8::TracedGlobal<v8::Value>* global) {
     to_register_with_v8_.push_back(global);
   }
 
@@ -106,7 +106,7 @@ class TestEmbedderHeapTracer final : public v8::EmbedderHeapTracer {
 
  private:
   std::vector<std::pair<void*, void*>> registered_from_v8_;
-  std::vector<v8::TracedGlobal<v8::Object>*> to_register_with_v8_;
+  std::vector<v8::TracedGlobal<v8::Value>*> to_register_with_v8_;
   std::vector<v8::TracedReference<v8::Value>*> to_register_with_v8_references_;
   bool consider_traced_global_as_root_ = true;
   TracePrologueBehavior prologue_behavior_ = TracePrologueBehavior::kNoop;
@@ -147,10 +147,10 @@ TEST(EmbedderRegisteringV8Reference) {
   v8::Local<v8::Context> context = v8::Context::New(isolate);
   v8::Context::Scope context_scope(context);
 
-  v8::TracedGlobal<v8::Object> g;
+  v8::TracedGlobal<v8::Value> g;
   {
     v8::HandleScope inner_scope(isolate);
-    v8::Local<v8::Object> o =
+    v8::Local<v8::Value> o =
         v8::Local<v8::Object>::New(isolate, v8::Object::New(isolate));
     g.Reset(isolate, o);
   }
@@ -909,10 +909,11 @@ TEST(TracedGlobalNoDestructorReclaimedOnScavenge) {
 
 namespace {
 
-V8_NOINLINE void TracedReferenceOnStack(TestEmbedderHeapTracer* tracer) {
+template <typename T>
+V8_NOINLINE void OnStackTest(TestEmbedderHeapTracer* tracer) {
   v8::Isolate* isolate = CcTest::isolate();
   v8::Global<v8::Object> observer;
-  v8::TracedReference<v8::Value> stack_ref;
+  T stack_ref;
   {
     v8::HandleScope scope(isolate);
     v8::Local<v8::Object> object(ConstructTraceableJSApiObject(
@@ -937,7 +938,7 @@ V8_NOINLINE void CreateTracedReferenceInDeepStack(
   observer->SetWeak();
 }
 
-V8_NOINLINE void TracedReferenceNotifyEmptyStack(
+V8_NOINLINE void TracedReferenceNotifyEmptyStackTest(
     TestEmbedderHeapTracer* tracer) {
   v8::Isolate* isolate = CcTest::isolate();
   v8::Global<v8::Object> observer;
@@ -953,13 +954,25 @@ enum class Operation {
   kMove,
 };
 
-V8_NOINLINE void TracedReferenceStackToHeapTest(TestEmbedderHeapTracer* tracer,
-                                                Operation op) {
+template <typename T>
+void PerformOperation(Operation op, T* lhs, T* rhs) {
+  switch (op) {
+    case Operation::kMove:
+      *lhs = std::move(*rhs);
+      break;
+    case Operation::kCopy:
+      *lhs = *rhs;
+      rhs->Reset();
+      break;
+  }
+}
+
+template <typename T>
+V8_NOINLINE void StackToHeapTest(TestEmbedderHeapTracer* tracer, Operation op) {
   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>();
+  T stack_handle;
+  T* heap_handle = new T();
   {
     v8::HandleScope scope(isolate);
     v8::Local<v8::Object> object(ConstructTraceableJSApiObject(
@@ -972,15 +985,7 @@ V8_NOINLINE void TracedReferenceStackToHeapTest(TestEmbedderHeapTracer* tracer,
   heap::InvokeMarkSweep();
   CHECK(!observer.IsEmpty());
   tracer->AddReferenceForTracing(heap_handle);
-  switch (op) {
-    case Operation::kMove:
-      *heap_handle = std::move(stack_handle);
-      break;
-    case Operation::kCopy:
-      *heap_handle = stack_handle;
-      stack_handle.Reset();
-      break;
-  }
+  PerformOperation(op, heap_handle, &stack_handle);
   heap::InvokeMarkSweep();
   CHECK(!observer.IsEmpty());
   heap::InvokeMarkSweep();
@@ -988,13 +993,12 @@ V8_NOINLINE void TracedReferenceStackToHeapTest(TestEmbedderHeapTracer* tracer,
   delete heap_handle;
 }
 
-V8_NOINLINE void TracedReferenceHeapToStackTest(TestEmbedderHeapTracer* tracer,
-                                                Operation op) {
+template <typename T>
+V8_NOINLINE void HeapToStackTest(TestEmbedderHeapTracer* tracer, Operation op) {
   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>();
+  T stack_handle;
+  T* heap_handle = new T();
   {
     v8::HandleScope scope(isolate);
     v8::Local<v8::Object> object(ConstructTraceableJSApiObject(
@@ -1007,28 +1011,22 @@ V8_NOINLINE void TracedReferenceHeapToStackTest(TestEmbedderHeapTracer* tracer,
   tracer->AddReferenceForTracing(heap_handle);
   heap::InvokeMarkSweep();
   CHECK(!observer.IsEmpty());
-  switch (op) {
-    case Operation::kMove:
-      stack_handle = std::move(*heap_handle);
-      break;
-    case Operation::kCopy:
-      stack_handle = *heap_handle;
-      break;
-  }
-  delete heap_handle;
+  PerformOperation(op, &stack_handle, heap_handle);
   heap::InvokeMarkSweep();
   CHECK(!observer.IsEmpty());
   stack_handle.Reset();
   heap::InvokeMarkSweep();
   CHECK(observer.IsEmpty());
+  delete heap_handle;
 }
 
-V8_NOINLINE void TracedReferenceStackToStackTest(TestEmbedderHeapTracer* tracer,
+template <typename T>
+V8_NOINLINE void StackToStackTest(TestEmbedderHeapTracer* tracer,
                                   Operation op) {
   v8::Isolate* isolate = CcTest::isolate();
   v8::Global<v8::Object> observer;
-  v8::TracedReference<v8::Value> stack_handle1;
-  v8::TracedReference<v8::Value> stack_handle2;
+  T stack_handle1;
+  T stack_handle2;
   {
     v8::HandleScope scope(isolate);
     v8::Local<v8::Object> object(ConstructTraceableJSApiObject(
@@ -1040,15 +1038,7 @@ V8_NOINLINE void TracedReferenceStackToStackTest(TestEmbedderHeapTracer* tracer,
   CHECK(!observer.IsEmpty());
   heap::InvokeMarkSweep();
   CHECK(!observer.IsEmpty());
-  switch (op) {
-    case Operation::kMove:
-      stack_handle2 = std::move(stack_handle1);
-      break;
-    case Operation::kCopy:
-      stack_handle2 = stack_handle1;
-      stack_handle1.Reset();
-      break;
-  }
+  PerformOperation(op, &stack_handle2, &stack_handle1);
   heap::InvokeMarkSweep();
   CHECK(!observer.IsEmpty());
   stack_handle2.Reset();
@@ -1056,6 +1046,7 @@ V8_NOINLINE void TracedReferenceStackToStackTest(TestEmbedderHeapTracer* tracer,
   CHECK(observer.IsEmpty());
 }
 
+template <typename T>
 V8_NOINLINE void TracedReferenceCleanedTest(TestEmbedderHeapTracer* tracer) {
   v8::Isolate* isolate = CcTest::isolate();
   v8::HandleScope scope(isolate);
@@ -1064,7 +1055,7 @@ V8_NOINLINE void TracedReferenceCleanedTest(TestEmbedderHeapTracer* tracer) {
   const size_t before =
       CcTest::i_isolate()->global_handles()->NumberOfOnStackHandlesForTesting();
   for (int i = 0; i < 100; i++) {
-    v8::TracedReference<v8::Value> stack_handle;
+    T stack_handle;
     stack_handle.Reset(isolate, object);
   }
   CHECK_EQ(before + 1, CcTest::i_isolate()
@@ -1072,6 +1063,27 @@ V8_NOINLINE void TracedReferenceCleanedTest(TestEmbedderHeapTracer* tracer) {
                            ->NumberOfOnStackHandlesForTesting());
 }
 
+V8_NOINLINE void TracedGlobalDestructorTest(TestEmbedderHeapTracer* tracer) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::Global<v8::Object> observer;
+  {
+    v8::TracedGlobal<v8::Value> stack_handle;
+    {
+      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());
+  }
+  heap::InvokeMarkSweep();
+  CHECK(observer.IsEmpty());
+}
+
 }  // namespace
 
 TEST(TracedReferenceOnStack) {
@@ -1081,87 +1093,109 @@ TEST(TracedReferenceOnStack) {
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
                                                       &tracer);
   tracer.SetStackStart(&manual_gc);
-  TracedReferenceOnStack(&tracer);
+  OnStackTest<v8::TracedReference<v8::Value>>(&tracer);
 }
 
-TEST(TracedReferenceNotifyEmptyStack) {
+TEST(TracedGlobalOnStack) {
   ManualGCScope manual_gc;
   CcTest::InitializeVM();
   TestEmbedderHeapTracer tracer;
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
                                                       &tracer);
   tracer.SetStackStart(&manual_gc);
-  TracedReferenceNotifyEmptyStack(&tracer);
+  OnStackTest<v8::TracedGlobal<v8::Value>>(&tracer);
 }
 
-TEST(TracedReferenceCopyStackToHeap) {
+TEST(TracedReferenceCleaned) {
   ManualGCScope manual_gc;
   CcTest::InitializeVM();
   TestEmbedderHeapTracer tracer;
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
                                                       &tracer);
   tracer.SetStackStart(&manual_gc);
-  TracedReferenceStackToHeapTest(&tracer, Operation::kCopy);
+  TracedReferenceCleanedTest<v8::TracedReference<v8::Value>>(&tracer);
 }
 
-TEST(TracedReferenceCopyHeapToStack) {
+TEST(TracedGlobalCleaned) {
   ManualGCScope manual_gc;
   CcTest::InitializeVM();
   TestEmbedderHeapTracer tracer;
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
                                                       &tracer);
   tracer.SetStackStart(&manual_gc);
-  TracedReferenceHeapToStackTest(&tracer, Operation::kCopy);
+  TracedReferenceCleanedTest<v8::TracedGlobal<v8::Value>>(&tracer);
 }
 
-TEST(TracedReferenceCopyStackToStack) {
+TEST(TracedReferenceMove) {
+  using ReferenceType = v8::TracedReference<v8::Value>;
   ManualGCScope manual_gc;
   CcTest::InitializeVM();
   TestEmbedderHeapTracer tracer;
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
                                                       &tracer);
   tracer.SetStackStart(&manual_gc);
-  TracedReferenceStackToStackTest(&tracer, Operation::kCopy);
+  StackToHeapTest<ReferenceType>(&tracer, Operation::kMove);
+  HeapToStackTest<ReferenceType>(&tracer, Operation::kMove);
+  StackToStackTest<ReferenceType>(&tracer, Operation::kMove);
 }
 
-TEST(TracedReferenceMoveStackToHeap) {
+TEST(TracedReferenceCopy) {
+  using ReferenceType = v8::TracedReference<v8::Value>;
   ManualGCScope manual_gc;
   CcTest::InitializeVM();
   TestEmbedderHeapTracer tracer;
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
                                                       &tracer);
   tracer.SetStackStart(&manual_gc);
-  TracedReferenceStackToHeapTest(&tracer, Operation::kMove);
+  StackToHeapTest<ReferenceType>(&tracer, Operation::kCopy);
+  HeapToStackTest<ReferenceType>(&tracer, Operation::kCopy);
+  StackToStackTest<ReferenceType>(&tracer, Operation::kCopy);
 }
 
-TEST(TracedReferenceMoveHeapToStack) {
+TEST(TraceGlobalMove) {
+  using ReferenceType = v8::TracedGlobal<v8::Value>;
   ManualGCScope manual_gc;
   CcTest::InitializeVM();
   TestEmbedderHeapTracer tracer;
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
                                                       &tracer);
   tracer.SetStackStart(&manual_gc);
-  TracedReferenceHeapToStackTest(&tracer, Operation::kMove);
+  StackToHeapTest<ReferenceType>(&tracer, Operation::kMove);
+  HeapToStackTest<ReferenceType>(&tracer, Operation::kMove);
+  StackToStackTest<ReferenceType>(&tracer, Operation::kMove);
 }
 
-TEST(TracedReferenceMoveStackToStack) {
+TEST(TracedGlobalCopy) {
+  using ReferenceType = v8::TracedGlobal<v8::Value>;
   ManualGCScope manual_gc;
   CcTest::InitializeVM();
   TestEmbedderHeapTracer tracer;
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
                                                       &tracer);
   tracer.SetStackStart(&manual_gc);
-  TracedReferenceStackToStackTest(&tracer, Operation::kMove);
+  StackToHeapTest<ReferenceType>(&tracer, Operation::kCopy);
+  HeapToStackTest<ReferenceType>(&tracer, Operation::kCopy);
+  StackToStackTest<ReferenceType>(&tracer, Operation::kCopy);
 }
 
-TEST(TracedReferenceCleaned) {
+TEST(TracedGlobalDestructor) {
+  ManualGCScope manual_gc;
+  CcTest::InitializeVM();
+  TestEmbedderHeapTracer tracer;
+  heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
+                                                      &tracer);
+  tracer.SetStackStart(&manual_gc);
+  TracedGlobalDestructorTest(&tracer);
+}
+
+TEST(NotifyEmptyStack) {
   ManualGCScope manual_gc;
   CcTest::InitializeVM();
   TestEmbedderHeapTracer tracer;
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(CcTest::isolate(),
                                                       &tracer);
   tracer.SetStackStart(&manual_gc);
-  TracedReferenceCleanedTest(&tracer);
+  TracedReferenceNotifyEmptyStackTest(&tracer);
 }
 
 }  // namespace heap