[heap] Merge mechanisms for disabling CSS

EmbedderStackStateScope is used to disable conservative stack scanning
for cppgc when the stack is known to not contain heap pointers. Also,
DisableConservativeStackScanningScopeForTesting is used to disable CSS
for the V8 heap in tests that assume a precise GC. Until now, these two
have used two different mechanisms for disabling CSS. This CL merges
the two mechanisms and implements the latter scope via the former.

Bug: v8:13257
Change-Id: Ieca082657854fe2eff9eb5d95a30d48bb8eab44f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4111954
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84964}

src/heap/heap.cc

@@ -4872,16 +4872,16 @@ void Heap::IterateStackRoots(RootVisitor* v, StackState stack_state,
   isolate()->Iterate(v);
 
 #ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-  if (stack_state == StackState::kNoHeapPointers ||
-      disable_conservative_stack_scanning_for_testing_)
-    return;
+  if (stack_state == StackState::kNoHeapPointers || !IsGCWithStack()) return;
 
   // In case of a shared GC, we're interested in the main isolate for CSS.
-  Isolate* main_isolate = mode == IterateRootsMode::kShared
-                              ? isolate()->shared_heap_isolate()
-                              : isolate();
-  if (main_isolate->heap()->disable_conservative_stack_scanning_for_testing_)
-    return;
+  Isolate* main_isolate;
+  if (mode == IterateRootsMode::kShared) {
+    main_isolate = isolate()->shared_heap_isolate();
+    if (!main_isolate->heap()->IsGCWithStack()) return;
+  } else {
+    main_isolate = isolate();
+  }
 
   ConservativeStackVisitor stack_visitor(main_isolate, v);
   stack().IteratePointers(&stack_visitor);

src/heap/heap.h

@@ -2409,7 +2409,6 @@ class Heap {
   bool force_oom_ = false;
   bool force_gc_on_next_allocation_ = false;
   bool delay_sweeper_tasks_for_testing_ = false;
-  bool disable_conservative_stack_scanning_for_testing_ = false;
 
   UnorderedHeapObjectMap<HeapObject> retainer_;
   UnorderedHeapObjectMap<Root> retaining_root_;
@@ -2686,23 +2685,6 @@ class V8_EXPORT_PRIVATE V8_NODISCARD SaveStackContextScope {
   ::heap::base::Stack* stack_;
 };
 
-class V8_NODISCARD DisableConservativeStackScanningScopeForTesting {
- public:
-  explicit inline DisableConservativeStackScanningScopeForTesting(Heap* heap)
-      : heap_(heap),
-        old_value_(heap_->disable_conservative_stack_scanning_for_testing_) {
-    heap_->disable_conservative_stack_scanning_for_testing_ = true;
-  }
-
-  inline ~DisableConservativeStackScanningScopeForTesting() {
-    heap_->disable_conservative_stack_scanning_for_testing_ = old_value_;
-  }
-
- private:
-  Heap* heap_;
-  bool old_value_;
-};
-
 // Space iterator for iterating over all the paged spaces of the heap: Map
 // space, old space and code space. Returns each space in turn, and null when it
 // is done.
@@ -2858,6 +2840,17 @@ class V8_EXPORT_PRIVATE V8_NODISCARD EmbedderStackStateScope final {
   const StackState old_stack_state_;
 };
 
+class V8_NODISCARD DisableConservativeStackScanningScopeForTesting {
+ public:
+  explicit inline DisableConservativeStackScanningScopeForTesting(Heap* heap)
+      : embedder_scope_(EmbedderStackStateScope::ExplicitScopeForTesting(
+            heap->local_embedder_heap_tracer(),
+            cppgc::EmbedderStackState::kNoHeapPointers)) {}
+
+ private:
+  EmbedderStackStateScope embedder_scope_;
+};
+
 class V8_NODISCARD CppClassNamesAsHeapObjectNameScope final {
  public:
   explicit CppClassNamesAsHeapObjectNameScope(v8::CppHeap* heap);

src/heap/mark-compact.cc

@@ -2115,8 +2115,7 @@ void MarkCompactCollector::MarkRoots(RootVisitor* root_visitor) {
     // v8::TracedReference alive from the stack. This is only needed when using
     // `EmbedderHeapTracer` and not using `CppHeap`.
     auto& stack = heap()->stack();
-    if (heap_->local_embedder_heap_tracer()->embedder_stack_state() ==
-        cppgc::EmbedderStackState::kMayContainHeapPointers) {
+    if (heap_->IsGCWithStack()) {
       ConservativeTracedHandlesMarkingVisitor conservative_marker(
           *heap_, *local_marking_worklists_,
           cppgc::internal::CollectionType::kMajor);

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

@@ -517,12 +517,6 @@ V8_NOINLINE void StackToHeapTest(v8::Isolate* v8_isolate, Operation op,
     // Disable scanning, assuming the slots are overwritten.
     DisableConservativeStackScanningScopeForTesting no_stack_scanning(
         reinterpret_cast<i::Isolate*>(v8_isolate)->heap());
-    EmbedderStackStateScope scope =
-        EmbedderStackStateScope::ExplicitScopeForTesting(
-            reinterpret_cast<i::Isolate*>(v8_isolate)
-                ->heap()
-                ->local_embedder_heap_tracer(),
-            cppgc::EmbedderStackState::kNoHeapPointers);
     FullGC(v8_isolate);
   }
   ASSERT_TRUE(observer.IsEmpty());
@@ -565,7 +559,13 @@ V8_NOINLINE void HeapToStackTest(v8::Isolate* v8_isolate, Operation op,
   FullGC(v8_isolate);
   EXPECT_FALSE(observer.IsEmpty());
   stack_handle.Reset();
-  FullGC(v8_isolate);
+  {
+    // Conservative scanning may find stale pointers to on-stack handles.
+    // Disable scanning, assuming the slots are overwritten.
+    DisableConservativeStackScanningScopeForTesting no_stack_scanning(
+        reinterpret_cast<i::Isolate*>(v8_isolate)->heap());
+    FullGC(v8_isolate);
+  }
   EXPECT_TRUE(observer.IsEmpty());
 }
 
@@ -603,7 +603,13 @@ V8_NOINLINE void StackToStackTest(v8::Isolate* v8_isolate, Operation op,
   FullGC(v8_isolate);
   EXPECT_FALSE(observer.IsEmpty());
   stack_handle2.Reset();
-  FullGC(v8_isolate);
+  {
+    // Conservative scanning may find stale pointers to on-stack handles.
+    // Disable scanning, assuming the slots are overwritten.
+    DisableConservativeStackScanningScopeForTesting no_stack_scanning(
+        reinterpret_cast<i::Isolate*>(v8_isolate)->heap());
+    FullGC(v8_isolate);
+  }
   EXPECT_TRUE(observer.IsEmpty());
 }
 

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

@@ -484,12 +484,6 @@ TEST_F(EmbedderTracingTest, TracedReferenceHandlesMarking) {
       // Disable scanning, assuming the slots are overwritten.
       DisableConservativeStackScanningScopeForTesting no_stack_scanning(
           i_isolate()->heap());
-      EmbedderStackStateScope scope =
-          EmbedderStackStateScope::ExplicitScopeForTesting(
-              reinterpret_cast<i::Isolate*>(v8_isolate())
-                  ->heap()
-                  ->local_embedder_heap_tracer(),
-              EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
       FullGC();
     }
     const size_t final_count = traced_handles->used_node_count();
@@ -590,12 +584,6 @@ TEST_F(EmbedderTracingTest, BasicTracedReference) {
     // Disable scanning, assuming the slots are overwritten.
     DisableConservativeStackScanningScopeForTesting no_stack_scanning(
         i_isolate()->heap());
-    EmbedderStackStateScope scope =
-        EmbedderStackStateScope::ExplicitScopeForTesting(
-            reinterpret_cast<i::Isolate*>(v8_isolate())
-                ->heap()
-                ->local_embedder_heap_tracer(),
-            EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
     FullGC();
   }
   EXPECT_EQ(initial_count, traced_handles->used_node_count());