api,heap: Avoid dropping global handles when stack state is overridden

This CL only affects non-production code. In non-production code, test
runners may invoke tasks (base::RunLoop()) with an interesting stack.
V8 assumes that it can clear certain data structures when running from
a non-nested task due to not having any interesting stack on top.
During testing this can lead to UAF on stack as data structures are
prematurely cleared.

With cppgc this failure can be fixed as the information on whether
test runners invoke tasks with a non-trivial stack is actually
present.

Example failure: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8847453411432681120/+/steps/webkit_unit_tests__with_patch__on_Ubuntu-18.04/0/logs/Flaky_failure:_WebSocketStreamTest.ConnectWithFailedHandshake__status_CRASH_SUCCESS_/0

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

include/v8.h

@@ -8051,6 +8051,9 @@ class V8_EXPORT EmbedderHeapTracer {
   /**
    * Called by the embedder to notify V8 of an empty execution stack.
    */
+  V8_DEPRECATE_SOON(
+      "This call only optimized internal caches which V8 is able to figure out "
+      "on its own now.")
   void NotifyEmptyEmbedderStack();
 
   /**

src/api/api.cc

@@ -9881,7 +9881,8 @@ void EmbedderHeapTracer::SetStackStart(void* stack_start) {
 void EmbedderHeapTracer::NotifyEmptyEmbedderStack() {
   CHECK(isolate_);
   reinterpret_cast<i::Isolate*>(isolate_)
-      ->global_handles()
+      ->heap()
+      ->local_embedder_heap_tracer()
       ->NotifyEmptyEmbedderStack();
 }
 

src/heap/cppgc/heap-base.h

@@ -158,6 +158,9 @@ class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
   size_t ObjectPayloadSize() const;
 
   StackSupport stack_support() const { return stack_support_; }
+  const EmbedderStackState* override_stack_state() const {
+    return override_stack_state_.get();
+  }
 
   void AdvanceIncrementalGarbageCollectionOnAllocationIfNeeded();
 

src/heap/embedder-tracing.cc

@@ -76,9 +76,8 @@ void LocalEmbedderHeapTracer::SetEmbedderStackStateForNextFinalization(
   if (!InUse()) return;
 
   embedder_stack_state_ = stack_state;
-  if (EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers == stack_state) {
-    remote_tracer()->NotifyEmptyEmbedderStack();
-  }
+  if (EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers == stack_state)
+    NotifyEmptyEmbedderStack();
 }
 
 namespace {
@@ -165,6 +164,16 @@ void LocalEmbedderHeapTracer::StartIncrementalMarkingIfNeeded() {
   }
 }
 
+void LocalEmbedderHeapTracer::NotifyEmptyEmbedderStack() {
+  auto* overriden_stack_state = isolate_->heap()->overriden_stack_state();
+  if (overriden_stack_state &&
+      (*overriden_stack_state ==
+       cppgc::EmbedderStackState::kMayContainHeapPointers))
+    return;
+
+  isolate_->global_handles()->NotifyEmptyEmbedderStack();
+}
+
 bool DefaultEmbedderRootsHandler::IsRoot(
     const v8::TracedReference<v8::Value>& handle) {
   return !tracer_ || tracer_->IsRootForNonTracingGC(handle);

src/heap/embedder-tracing.h

@@ -132,6 +132,8 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
     return default_embedder_roots_handler_;
   }
 
+  void NotifyEmptyEmbedderStack();
+
  private:
   static constexpr size_t kEmbedderAllocatedThreshold = 128 * KB;
 
@@ -186,11 +188,8 @@ class V8_EXPORT_PRIVATE V8_NODISCARD EmbedderStackStateScope final {
       : local_tracer_(local_tracer),
         old_stack_state_(local_tracer_->embedder_stack_state_) {
     local_tracer_->embedder_stack_state_ = stack_state;
-    if (EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers ==
-        stack_state) {
-      if (local_tracer->remote_tracer())
-        local_tracer->remote_tracer()->NotifyEmptyEmbedderStack();
-    }
+    if (EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers == stack_state)
+      local_tracer_->NotifyEmptyEmbedderStack();
   }
 
   ~EmbedderStackStateScope() {

src/heap/heap.cc

@@ -5647,6 +5647,11 @@ EmbedderHeapTracer::TraceFlags Heap::flags_for_embedder_tracer() const {
   return EmbedderHeapTracer::TraceFlags::kNoFlags;
 }
 
+const cppgc::EmbedderStackState* Heap::overriden_stack_state() const {
+  const auto* cpp_heap = CppHeap::From(cpp_heap_);
+  return cpp_heap ? cpp_heap->override_stack_state() : nullptr;
+}
+
 void Heap::RegisterExternallyReferencedObject(Address* location) {
   GlobalHandles::MarkTraced(location);
   Object object(*location);

src/heap/heap.h

@@ -1174,6 +1174,8 @@ class Heap {
 
   v8::CppHeap* cpp_heap() const { return cpp_heap_; }
 
+  const cppgc::EmbedderStackState* overriden_stack_state() const;
+
   // ===========================================================================
   // Embedder roots optimizations. =============================================
   // ===========================================================================

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

@@ -7,7 +7,9 @@
 
 #include "include/v8.h"
 #include "src/api/api-inl.h"
+#include "src/heap/embedder-tracing.h"
 #include "src/heap/heap-inl.h"
+#include "src/heap/heap.h"
 #include "src/heap/safepoint.h"
 #include "src/objects/module.h"
 #include "src/objects/objects-inl.h"
@@ -961,7 +963,10 @@ V8_NOINLINE void TracedReferenceNotifyEmptyStackTest(
   v8::Global<v8::Object> observer;
   CreateTracedReferenceInDeepStack(isolate, &observer);
   CHECK(!observer.IsEmpty());
-  tracer->NotifyEmptyEmbedderStack();
+  reinterpret_cast<i::Isolate*>(isolate)
+      ->heap()
+      ->local_embedder_heap_tracer()
+      ->NotifyEmptyEmbedderStack();
   heap::InvokeMarkSweep();
   CHECK(observer.IsEmpty());
 }