cppgc: Reorganize conservative stack scan defines

Pure refactoring. The re-organization allows to easily
add more platforms.

Bug: chromium:1056170
Change-Id: Ia191c37a1dabff6952414c5314beeeae881713b7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2129636Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66924}

src/heap/cppgc/asm/x64/push_registers.S

@@ -37,9 +37,9 @@ PushAllRegistersAndIterateStack:
     push %r13
     push %r14
     push %r15
-    // Pass 1st parameter (rdi) unchanged (this).
+    // Pass 1st parameter (rdi) unchanged (Stack*).
     // Pass 2nd parameter (rsi) unchanged (StackVisitor*).
-    // Save 3rd parameter (rdx; callback)
+    // Save 3rd parameter (rdx; IterateStackCallback)
     mov %rdx, %r8
     // Pass 3rd parameter as rsp (stack pointer).
     mov %rsp, %rdx

src/heap/cppgc/asm/x64/push_registers_win.S

@@ -28,9 +28,9 @@ PushAllRegistersAndIterateStack:
     push r13
     push r14
     push r15
-    ;; Pass 1st parameter (rcx) unchanged (this).
+    ;; Pass 1st parameter (rcx) unchanged (Stack*).
     ;; Pass 2nd parameter (rdx) unchanged (StackVisitor*).
-    ;; Save 3rd parameter (r8; callback)
+    ;; Save 3rd parameter (r8; IterateStackCallback)
     mov r9, r8
     ;; Pass 3rd parameter as rsp (stack pointer).
     mov r8, rsp

src/heap/cppgc/stack.cc

@@ -63,7 +63,7 @@ void IterateAsanFakeFrameIfNecessary(StackVisitor* visitor,
 
 #endif  // V8_USE_ADDRESS_SANITIZER
 
-#ifdef V8_TARGET_ARCH_X64
+#ifdef CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
 
 void IterateSafeStackIfNecessary(StackVisitor* visitor) {
 #if defined(__has_feature)
@@ -88,18 +88,18 @@ void IterateSafeStackIfNecessary(StackVisitor* visitor) {
 #endif  // defined(__has_feature)
 }
 
-#endif  // V8_TARGET_ARCH_X64
+#endif  // CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
 
 }  // namespace
 
-#ifdef V8_TARGET_ARCH_X64
+#ifdef CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
 void Stack::IteratePointers(StackVisitor* visitor) const {
   PushAllRegistersAndIterateStack(this, visitor, &Stack::IteratePointersImpl);
   // No need to deal with callee-saved registers as they will be kept alive by
   // the regular conservative stack iteration.
   IterateSafeStackIfNecessary(visitor);
 }
-#endif  // V8_TARGET_ARCH_X64
+#endif  // CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
 
 // No ASAN support as method accesses redzones while walking the stack.
 NO_SANITIZE_ADDRESS

src/heap/cppgc/stack.h

@@ -7,6 +7,11 @@
 
 #include "src/base/macros.h"
 
+// TODO(chromium:1056170): Implement all platforms.
+#if defined(V8_TARGET_ARCH_X64)
+#define CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN 1
+#endif
+
 namespace cppgc {
 namespace internal {
 
@@ -28,11 +33,9 @@ class V8_EXPORT_PRIVATE Stack final {
 
   // Word-aligned iteration of the stack. Slot values are passed on to
   // |visitor|.
-  //
-  // TODO(chromium:1056170): Implement all platforms.
-#ifdef V8_TARGET_ARCH_X64
+#ifdef CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
   void IteratePointers(StackVisitor* visitor) const;
-#endif  // V8_TARGET_ARCH_X64
+#endif  // CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
 
  private:
   void IteratePointersImpl(StackVisitor* visitor, intptr_t* stack_end) const;

test/unittests/heap/cppgc/stack_unittest.cc

@@ -46,45 +46,7 @@ TEST_F(GCStackTest, IsOnStackForHeapValue) {
   EXPECT_FALSE(GetStack()->IsOnStack(dummy.get()));
 }
 
-// The following test uses inline assembly and has been checked to work on clang
-// to verify that the stack-scanning trampoline pushes callee-saved registers.
-//
-// The test uses a macro loop as asm() can only be passed string literals.
-//
-// TODO(chromium:1056170): Add more platforms as backends are implemented.
-#ifdef __clang__
-#ifdef V8_TARGET_ARCH_X64
-
-// All of x64 support conservative stack scanning.
-#define CONSERVATIVE_STACK_SCAN_SUPPORTED 1
-
-#ifdef V8_OS_WIN
-
-// Excluded from test: rbp
-#define FOR_ALL_CALLEE_SAVED_REGS(V) \
-  V("rdi")                           \
-  V("rsi")                           \
-  V("rbx")                           \
-  V("r12")                           \
-  V("r13")                           \
-  V("r14")                           \
-  V("r15")
-
-#else  // !V8_OS_WIN
-
-// Excluded from test: rbp
-#define FOR_ALL_CALLEE_SAVED_REGS(V) \
-  V("rbx")                           \
-  V("r12")                           \
-  V("r13")                           \
-  V("r14")                           \
-  V("r15")
-
-#endif  // !V8_OS_WIN
-#endif  // V8_TARGET_ARCH_X64
-#endif  // __clang__
-
-#ifdef CONSERVATIVE_STACK_SCAN_SUPPORTED
+#ifdef CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
 
 namespace {
 
@@ -200,6 +162,40 @@ TEST_F(GCStackTest, IteratePointersFindsParameterInNestedFunction) {
   EXPECT_TRUE(scanner->found());
 }
 
+// The following test uses inline assembly and has been checked to work on clang
+// to verify that the stack-scanning trampoline pushes callee-saved registers.
+//
+// The test uses a macro loop as asm() can only be passed string literals.
+#ifdef __clang__
+#ifdef V8_TARGET_ARCH_X64
+#ifdef V8_OS_WIN
+
+// Excluded from test: rbp
+#define FOR_ALL_CALLEE_SAVED_REGS(V) \
+  V("rdi")                           \
+  V("rsi")                           \
+  V("rbx")                           \
+  V("r12")                           \
+  V("r13")                           \
+  V("r14")                           \
+  V("r15")
+
+#else  // !V8_OS_WIN
+
+// Excluded from test: rbp
+#define FOR_ALL_CALLEE_SAVED_REGS(V) \
+  V("rbx")                           \
+  V("r12")                           \
+  V("r13")                           \
+  V("r14")                           \
+  V("r15")
+
+#endif  // !V8_OS_WIN
+#endif  // V8_TARGET_ARCH_X64
+#endif  // __clang__
+
+#ifdef FOR_ALL_CALLEE_SAVED_REGS
+
 TEST_F(GCStackTest, IteratePointersFindsCalleeSavedRegisters) {
   auto scanner = std::make_unique<StackScanner>();
 
@@ -236,6 +232,7 @@ TEST_F(GCStackTest, IteratePointersFindsCalleeSavedRegisters) {
 #undef KEEP_ALIVE_FROM_CALLEE_SAVED
 #undef FOR_ALL_CALLEE_SAVED_REGS
 }
+#endif  // FOR_ALL_CALLEE_SAVED_REGS
 
 #if V8_OS_LINUX && (V8_HOST_ARCH_IA32 || V8_HOST_ARCH_X64)
 class CheckStackAlignmentVisitor final : public StackVisitor {
@@ -253,7 +250,7 @@ TEST_F(GCStackTest, StackAlignment) {
 }
 #endif  // V8_OS_LINUX && (V8_HOST_ARCH_IA32 || V8_HOST_ARCH_X64)
 
-#endif  // CONSERVATIVE_STACK_SCAN_SUPPORTED
+#endif  // CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
 
 }  // namespace internal
 }  // namespace cppgc