[heap] Remove CSS build flag

omerktz · V8 LUCI CQ · commit 1eb7d3ce47e7 · 2025-04-15T00:25:07.000-07:00
Where needed, the build flag is replaced by the runtime flag. For using CSS with direct handles, `v8_enable_direct_handle = true` should be added to the GN args. Bug: 41480448 Change-Id: If15cf6030e6e66a8dbfb057c95aa42355b4c95f2 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6448975 Commit-Queue: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Nikolaos Papaspyrou <nikolaos@chromium.org> Auto-Submit: Omer Katz <omerkatz@chromium.org> Cr-Commit-Position: refs/heads/main@{#99783}
diff --git a/BUILD.bazel b/BUILD.bazel
@@ -41,7 +41,6 @@ load(":bazel/v8-non-pointer-compression.bzl", "v8_binary_non_pointer_compression
 # v8_enable_trace_feedback_updates
 # v8_enable_atomic_object_field_writes
 # v8_enable_concurrent_marking
-# v8_enable_conservative_stack_scanning
 # v8_enable_direct_handle
 # v8_enable_local_off_stack_check
 # v8_enable_ignition_dispatch_counting
diff --git a/BUILD.gn b/BUILD.gn
@@ -985,7 +985,6 @@ external_v8_defines = [
   "V8_USE_PERFETTO",
   "V8_MAP_PACKING",
   "V8_IS_TSAN",
-  "V8_ENABLE_CONSERVATIVE_STACK_SCANNING",
   "V8_ENABLE_DIRECT_HANDLE",
   "V8_MINORMS_STRING_SHORTCUTTING",
   "V8_HAVE_TARGET_OS",
@@ -1040,9 +1039,6 @@ if (v8_enable_map_packing) {
 if (is_tsan) {
   enabled_external_v8_defines += [ "V8_IS_TSAN" ]
 }
-if (v8_enable_conservative_stack_scanning) {
-  enabled_external_v8_defines += [ "V8_ENABLE_CONSERVATIVE_STACK_SCANNING" ]
-}
 if (v8_enable_direct_handle) {
   enabled_external_v8_defines += [ "V8_ENABLE_DIRECT_HANDLE" ]
 }
@@ -2792,7 +2788,6 @@ action("v8_dump_build_config") {
     "code_comments=$v8_code_comments",
     "component_build=$is_component_build",
     "concurrent_marking=$v8_enable_concurrent_marking",
-    "conservative_stack_scanning=$v8_enable_conservative_stack_scanning",
     "current_cpu=\"$current_cpu\"",
     "dcheck_always_on=$v8_dcheck_always_on",
     "debug_code=$v8_enable_debug_code",
@@ -2858,8 +2853,6 @@ generated_file("v8_generate_features_json") {
   contents = {
     v8_deprecation_warnings = v8_deprecation_warnings
     v8_enable_31bit_smis_on_64bit_arch = v8_enable_31bit_smis_on_64bit_arch
-    v8_enable_conservative_stack_scanning =
-        v8_enable_conservative_stack_scanning
     v8_enable_direct_handle = v8_enable_direct_handle
     v8_enable_extensible_ro_snapshot = v8_enable_extensible_ro_snapshot
     v8_enable_gdbjit = v8_enable_gdbjit
diff --git a/gni/v8.gni b/gni/v8.gni
@@ -123,11 +123,8 @@ declare_args() {
                        !(defined(build_with_node) && build_with_node) &&
                        !(is_win && is_component_build) && is_clang
 
-  # Scan the call stack conservatively during garbage collection.
-  v8_enable_conservative_stack_scanning = false
-
   # Use direct pointers in handles (v8::internal::Handle and v8::Local).
-  v8_enable_direct_handle = ""
+  v8_enable_direct_handle = false
 
   # Check for off-stack allocated local handles.
   v8_enable_local_off_stack_check = false
@@ -254,12 +251,6 @@ if (v8_enable_turbofan == "") {
 assert(v8_enable_turbofan || !v8_enable_webassembly,
        "Webassembly is not available when Turbofan is disabled.")
 
-# Direct internal handles and direct locals are enabled by default if
-# conservative stack scanning is enabled.
-if (v8_enable_direct_handle == "") {
-  v8_enable_direct_handle = v8_enable_conservative_stack_scanning
-}
-
 if (v8_target_is_simulator == "") {
   # We compare target cpu with v8 target cpu to not affect simulator builds for
   # making cross-compile snapshots.
diff --git a/infra/mb/mb_config.pyl b/infra/mb/mb_config.pyl
@@ -853,7 +853,7 @@
     },
 
     'conservative_stack_scanning': {
-      'gn_args': 'v8_enable_conservative_stack_scanning=true',
+      'gn_args': 'v8_enable_direct_handle=true',
     },
 
     'dcheck_always_on': {
diff --git a/src/common/ptr-compr-inl.h b/src/common/ptr-compr-inl.h
@@ -213,26 +213,10 @@ constexpr Tagged_t ExternalCodeCompressionScheme::CompressAny(Address tagged) {
   return static_cast<Tagged_t>(tagged);
 }
 
-// static
-Address ExternalCodeCompressionScheme::DecompressTaggedSigned(
-    Tagged_t raw_value) {
-  // For runtime code the upper 32-bits of the Smi value do not matter.
-  return static_cast<Address>(raw_value);
-}
-
 // static
 template <typename TOnHeapAddress>
 Address ExternalCodeCompressionScheme::DecompressTagged(
     TOnHeapAddress on_heap_addr, Tagged_t raw_value) {
-#ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-  // During conservative stack scanning, if we are trying to decompress a value
-  // that looks like a SMI (i.e., it's not tagged), we always need to add the
-  // cage base.
-#else
-  // Avoid complex decompression code for Smis.
-  if (HAS_SMI_TAG(raw_value)) return DecompressTaggedSigned(raw_value);
-#endif
-
 #ifdef V8_COMPRESS_POINTERS
   Address cage_base = base();
 #ifdef V8_COMPRESS_POINTERS_IN_MULTIPLE_CAGES
diff --git a/src/common/ptr-compr.h b/src/common/ptr-compr.h
@@ -165,9 +165,6 @@ class ExternalCodeCompressionScheme {
   // object, or a marker bit pattern).
   V8_INLINE static constexpr Tagged_t CompressAny(Address tagged);
 
-  // Decompresses smi value.
-  V8_INLINE static Address DecompressTaggedSigned(Tagged_t raw_value);
-
   // Decompresses any tagged value, preserving both weak- and smi- tags.
   template <typename TOnHeapAddress>
   V8_INLINE static Address DecompressTagged(TOnHeapAddress on_heap_addr,
diff --git a/src/flags/flag-definitions.h b/src/flags/flag-definitions.h
@@ -460,14 +460,14 @@ DEFINE_BOOL_READONLY(
     single_generation, V8_SINGLE_GENERATION_BOOL,
     "allocate all objects from young generation to old generation")
 
-#ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-#define V8_ENABLE_CONSERVATIVE_STACK_SCANNING_BOOL true
-#else
-#define V8_ENABLE_CONSERVATIVE_STACK_SCANNING_BOOL false
-#endif
-DEFINE_BOOL_READONLY(conservative_stack_scanning,
-                     V8_ENABLE_CONSERVATIVE_STACK_SCANNING_BOOL,
+#ifdef V8_ENABLE_DIRECT_HANDLE
+// Direct handles require conservative stack scanning.
+DEFINE_BOOL_READONLY(conservative_stack_scanning, true,
                      "use conservative stack scanning")
+#else
+DEFINE_EXPERIMENTAL_FEATURE(conservative_stack_scanning,
+                            "use conservative stack scanning")
+#endif  // V8_ENABLE_DIRECT_HANDLE
 DEFINE_IMPLICATION(conservative_stack_scanning,
                    scavenger_conservative_object_pinning)
 DEFINE_NEG_IMPLICATION(conservative_stack_scanning, compact_with_stack)
diff --git a/src/handles/handles.h b/src/handles/handles.h
@@ -394,7 +394,7 @@ static constexpr bool is_direct_handle_v = is_direct_handle<T>::value;
 
 // Direct handles should not be used without conservative stack scanning,
 // as this would break the correctness of the GC.
-static_assert(V8_ENABLE_CONSERVATIVE_STACK_SCANNING_BOOL);
+static_assert(v8_flags.conservative_stack_scanning.value());
 
 // ----------------------------------------------------------------------------
 // Base class for DirectHandle instantiations. Don't use directly.
diff --git a/src/heap/heap.cc b/src/heap/heap.cc
@@ -49,6 +49,7 @@
 #include "src/heap/collection-barrier.h"
 #include "src/heap/combined-heap.h"
 #include "src/heap/concurrent-marking.h"
+#include "src/heap/conservative-stack-visitor-inl.h"
 #include "src/heap/cppgc-js/cpp-heap.h"
 #include "src/heap/ephemeron-remembered-set.h"
 #include "src/heap/evacuation-verifier-inl.h"
@@ -128,10 +129,6 @@
 #include "src/utils/utils-inl.h"
 #include "src/utils/utils.h"
 
-#ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-#include "src/heap/conservative-stack-visitor-inl.h"
-#endif  // V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-
 #if V8_ENABLE_WEBASSEMBLY
 #include "src/wasm/wasm-engine.h"
 #endif  // V8_ENABLE_WEBASSEMBLY
@@ -4846,8 +4843,7 @@ void Heap::IterateStackRoots(RootVisitor* v) { isolate_->Iterate(v); }
 
 void Heap::IterateConservativeStackRoots(RootVisitor* root_visitor,
                                          IterateRootsMode roots_mode) {
-#ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-  if (!IsGCWithStack()) return;
+  if (!v8_flags.conservative_stack_scanning || !IsGCWithStack()) return;
 
   // In case of a shared GC, we're interested in the main isolate for CSS.
   Isolate* main_isolate = roots_mode == IterateRootsMode::kClientIsolate
@@ -4856,7 +4852,6 @@ void Heap::IterateConservativeStackRoots(RootVisitor* root_visitor,
 
   ConservativeStackVisitor stack_visitor(main_isolate, root_visitor);
   IterateConservativeStackRoots(&stack_visitor);
-#endif  // V8_ENABLE_CONSERVATIVE_STACK_SCANNING
 }
 
 void Heap::IterateConservativeStackRoots(
diff --git a/src/runtime/runtime-strings.cc b/src/runtime/runtime-strings.cc
@@ -387,10 +387,6 @@ RUNTIME_FUNCTION(Runtime_StringEqual) {
   SaveAndClearThreadInWasmFlag non_wasm_scope(isolate);
   HandleScope handle_scope(isolate);
   DCHECK_EQ(2, args.length());
-#ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-  DirectHandle<String> x = args.at<String>(0);
-  DirectHandle<String> y = args.at<String>(1);
-#else
   // This function can be called from Wasm: optimized Wasm code calls
   // straight to the "StringEqual" builtin, which tail-calls here. So on
   // the stack, the CEntryStub's EXIT frame will sit right on top of the
@@ -403,9 +399,11 @@ RUNTIME_FUNCTION(Runtime_StringEqual) {
   // In the future, Conservative Stack Scanning will trivially solve the
   // problem. In the meantime, we can work around it by explicitly creating
   // handles here (rather than treating the on-stack arguments as handles).
+  //
+  // TODO(42203211): Don't create new handles here once direct handles and CSS
+  // are enabled by default.
   DirectHandle<String> x(*args.at<String>(0), isolate);
   DirectHandle<String> y(*args.at<String>(1), isolate);
-#endif  // V8_ENABLE_CONSERVATIVE_STACK_SCANNING
   return isolate->heap()->ToBoolean(String::Equals(isolate, x, y));
 }
 
diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status
@@ -855,10 +855,10 @@
   'test-run-unwinding-info/RunUnwindingInfo': [SKIP],
 }],
 
-['conservative_stack_scanning', {
+['direct_handle', {
   # Needs --predictable.
   'test-serialize/StaticRootsPredictableSnapshot': [SKIP],
-}],  # conservative_stack_scanning
+}],  # direct_handle
 
 ['clang_coverage and mode == debug', {
   # Too slow tests.
diff --git a/test/cctest/heap/test-concurrent-allocation.cc b/test/cctest/heap/test-concurrent-allocation.cc
@@ -143,14 +143,10 @@ UNINITIALIZED_TEST(ConcurrentAllocationInOldSpaceFromMainThread) {
 }
 
 UNINITIALIZED_TEST(ConcurrentAllocationWhileMainThreadIsParked) {
-#ifndef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-  v8_flags.max_old_space_size = 4;
-#else
   // With CSS, it is expected that the GCs triggered by concurrent allocation
   // will reclaim less memory. If this test fails, this limit should probably
   // be further increased.
-  v8_flags.max_old_space_size = 10;
-#endif
+  v8_flags.max_old_space_size = v8_flags.conservative_stack_scanning ? 10 : 4;
   v8_flags.stress_concurrent_allocation = false;
 
   v8::Isolate::CreateParams create_params;
@@ -179,14 +175,10 @@ UNINITIALIZED_TEST(ConcurrentAllocationWhileMainThreadIsParked) {
 }
 
 UNINITIALIZED_TEST(ConcurrentAllocationWhileMainThreadParksAndUnparks) {
-#ifndef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-  v8_flags.max_old_space_size = 4;
-#else
   // With CSS, it is expected that the GCs triggered by concurrent allocation
   // will reclaim less memory. If this test fails, this limit should probably
   // be further increased.
-  v8_flags.max_old_space_size = 10;
-#endif
+  v8_flags.max_old_space_size = v8_flags.conservative_stack_scanning ? 10 : 4;
   v8_flags.stress_concurrent_allocation = false;
   v8_flags.incremental_marking = false;
   i::FlagList::EnforceFlagImplications();
@@ -224,14 +216,10 @@ UNINITIALIZED_TEST(ConcurrentAllocationWhileMainThreadParksAndUnparks) {
 }
 
 UNINITIALIZED_TEST(ConcurrentAllocationWhileMainThreadRunsWithSafepoints) {
-#ifndef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-  v8_flags.max_old_space_size = 4;
-#else
   // With CSS, it is expected that the GCs triggered by concurrent allocation
   // will reclaim less memory. If this test fails, this limit should probably
   // be further increased.
-  v8_flags.max_old_space_size = 10;
-#endif
+  v8_flags.max_old_space_size = v8_flags.conservative_stack_scanning ? 10 : 4;
   v8_flags.stress_concurrent_allocation = false;
   v8_flags.incremental_marking = false;
   i::FlagList::EnforceFlagImplications();
diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc
@@ -6661,12 +6661,10 @@ UNINITIALIZED_TEST(BreakPointAccessorContextSnapshot) {
 // static roots, to be able to generate the static-roots.h file.
 #if defined(V8_COMPRESS_POINTERS_IN_SHARED_CAGE) && defined(V8_SHARED_RO_HEAP)
 UNINITIALIZED_TEST(StaticRootsPredictableSnapshot) {
-#ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
   // TODO(jgruber): Snapshot determinism requires predictable heap layout
   // (v8_flags.predictable), but this flag is currently known not to work with
   // CSS due to false positives.
-  UNREACHABLE();
-#else
+  CHECK(!v8_flags.conservative_stack_scanning);
   if (v8_flags.random_seed == 0) return;
   const int random_seed = v8_flags.random_seed;
 
@@ -6705,7 +6703,6 @@ UNINITIALIZED_TEST(StaticRootsPredictableSnapshot) {
   blobs1.Dispose();
   blobs2.Dispose();
   FreeCurrentEmbeddedBlob();
-#endif  // V8_ENABLE_CONSERVATIVE_STACK_SCANNING
 }
 #endif  // defined(V8_COMPRESS_POINTERS_IN_SHARED_CAGE) &&
         // defined(V8_SHARED_RO_HEAP)
diff --git a/test/inspector/inspector.status b/test/inspector/inspector.status
@@ -257,7 +257,7 @@
 }],  # 'clang_coverage and mode == debug'
 
 ##############################################################################
-['((arch != x64 and arch != arm64 and arch != ia32 and arch != arm and arch != ppc64 and arch != s390x) or (simulator_run and conservative_stack_scanning))', {
+['((arch != x64 and arch != arm64 and arch != ia32 and arch != arm and arch != ppc64 and arch != s390x) or (simulator_run and direct_handle))', {
   # Stack switching is only supported on x64/arm64/ia32/arm.
   'debugger/wasm-jspi-async-stack': [SKIP],
 }],  # (arch != x64 and arch != arm64 and arch != ia32 and arch != arm and arch != ppc64 and arch != s390x)
diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
@@ -2404,11 +2404,11 @@
 }], # single_generation
 
 ################################################################################
-['conservative_stack_scanning', {
+['direct_handle', {
   # These tests are known to timeout with CSS, especially with variants that
   # stress the GC.
   'regress/regress-crbug-820312': [SKIP],
-}], # conservative_stack_scanning
+}], # direct_handle
 
 ##############################################################################
 ['arch == mips64el', {
diff --git a/test/unittests/BUILD.gn b/test/unittests/BUILD.gn
@@ -331,6 +331,7 @@ v8_source_set("v8_unittests_sources") {
     "gay-shortest.cc",
     "gay-shortest.h",
     "heap/allocation-observer-unittest.cc",
+    "heap/conservative-stack-visitor-unittest.cc",
     "heap/cppgc-js/embedder-roots-handler-unittest.cc",
     "heap/cppgc-js/traced-reference-unittest.cc",
     "heap/cppgc-js/unified-heap-snapshot-unittest.cc",
@@ -348,6 +349,7 @@ v8_source_set("v8_unittests_sources") {
     "heap/heap-utils.cc",
     "heap/heap-utils.h",
     "heap/index-generator-unittest.cc",
+    "heap/inner-pointer-resolution-unittest.cc",
     "heap/iterators-unittest.cc",
     "heap/list-unittest.cc",
     "heap/local-factory-unittest.cc",
@@ -644,11 +646,6 @@ v8_source_set("v8_unittests_sources") {
     sources += [ "wasm/wasm-gdbserver-unittest.cc" ]
   }
 
-  if (v8_enable_conservative_stack_scanning) {
-    sources += [ "heap/conservative-stack-visitor-unittest.cc" ]
-    sources += [ "heap/inner-pointer-resolution-unittest.cc" ]
-  }
-
   if (v8_enable_i18n_support) {
     defines = [ "V8_INTL_SUPPORT" ]
     public_deps = [ v8_icu_path ]
diff --git a/test/unittests/heap/inner-pointer-resolution-unittest.cc b/test/unittests/heap/inner-pointer-resolution-unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "src/flags/flags.h"
 #include "src/heap/conservative-stack-visitor-inl.h"
 #include "src/heap/gc-tracer.h"
 #include "test/unittests/heap/heap-utils.h"
@@ -588,6 +589,8 @@ using InnerPointerResolutionHeapTest =
     WithInnerPointerResolutionMixin<TestWithHeapInternalsAndContext>;
 
 TEST_F(InnerPointerResolutionHeapTest, UnusedRegularYoungPages) {
+  if (v8_flags.single_generation) return;
+
   ManualGCScope manual_gc_scope(isolate());
   DisableConservativeStackScanningScopeForTesting no_stack_scanning(heap());
 
@@ -728,6 +731,8 @@ TEST_F(InnerPointerResolutionHeapTest, UnusedRegularYoungPages) {
 }
 
 TEST_F(InnerPointerResolutionHeapTest, UnusedLargeYoungPage) {
+  if (v8_flags.single_generation) return;
+
   ManualGCScope manual_gc_scope(isolate());
   DisableConservativeStackScanningScopeForTesting no_stack_scanning(heap());
 
diff --git a/test/unittests/heap/shared-heap-unittest.cc b/test/unittests/heap/shared-heap-unittest.cc
@@ -759,8 +759,6 @@ TEST_ALL_SCENARIA(SharedHeapTestStateWithHandleParked, ToEachTheirOwn,
 TEST_ALL_SCENARIA(SharedHeapTestStateWithHandleUnparked, ToEachTheirOwn,
                   ToEachTheirOwnWithHandle)
 
-#ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-
 namespace {
 
 // Testing the shared heap using raw pointers.
@@ -790,6 +788,8 @@ using SharedHeapTestStateWithRawPointerUnparked =
 
 template <typename TestType, AllocationType allocation, AllocationSpace space>
 void ToEachTheirOwnWithRawPointer(TestType* test) {
+  if (!v8_flags.conservative_stack_scanning) return;
+
   using ThreadType = typename TestType::ThreadType;
   ThreadType* thread = test->thread();
 
@@ -837,8 +837,6 @@ TEST_ALL_SCENARIA(SharedHeapTestStateWithRawPointerParked, ToEachTheirOwn,
 TEST_ALL_SCENARIA(SharedHeapTestStateWithRawPointerUnparked, ToEachTheirOwn,
                   ToEachTheirOwnWithRawPointer)
 
-#endif  // V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-
 #undef TEST_SCENARIO
 #undef TEST_ALL_SCENARIA
 
