cppgc: Conservative stack for ia32 Windows using clang

Add support for ia32 Windows using clang. Change the callback
function from a member function to a free function. This the
compiler generating thiscall calling convention for the member
function which allows for keeping the asm trampoline uniform.

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

BUILD.gn

@@ -348,12 +348,8 @@ config("libbase_config") {
 
 # This config should be applied to code using the cppgc_base.
 config("cppgc_base_config") {
-  if (target_cpu == "x64") {
-    if (is_clang) {
-      defines = [ "CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN" ]
-    }
-  } else if (target_cpu == "x86") {
-    if (is_clang && !is_win) {
+  if (is_clang) {
+    if (target_cpu == "x64" || target_cpu == "x86") {
       defines = [ "CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN" ]
     }
   }
@@ -3969,12 +3965,10 @@ v8_source_set("cppgc_base") {
     "src/heap/cppgc/stack.h",
   ]
 
-  if (target_cpu == "x64") {
-    if (is_clang) {
+  if (is_clang) {
+    if (target_cpu == "x64") {
       sources += [ "src/heap/cppgc/asm/x64/push_registers_clang.cc" ]
-    }
-  } else if (target_cpu == "x86") {
-    if (is_clang && !is_win) {
+    } else if (target_cpu == "x86") {
       sources += [ "src/heap/cppgc/asm/ia32/push_registers_clang.cc" ]
     }
   }

src/heap/cppgc/asm/ia32/push_registers_clang.cc

@@ -7,14 +7,9 @@
 //
 // See asm/x64/push_registers_clang.cc for why the function is not generated
 // using clang.
-
+//
 // Do not depend on V8_TARGET_OS_* defines as some embedders may override the
 // GN toolchain (e.g. ChromeOS) and not provide them.
-#ifdef _WIN32
-
-#error "Not yet supported"
-
-#else  // !_WIN32
 
 // We maintain 16-byte alignment at calls. There is an 4-byte return address
 // on the stack and we push 28 bytes which maintains 16-byte stack alignment
@@ -22,9 +17,15 @@
 //
 // The following assumes cdecl calling convention.
 // Source: https://en.wikipedia.org/wiki/X86_calling_conventions#cdecl
-asm(".globl PushAllRegistersAndIterateStack             \n"
+asm(
+#ifdef _WIN32
+    ".globl _PushAllRegistersAndIterateStack            \n"
+    "_PushAllRegistersAndIterateStack:                  \n"
+#else   // !_WIN32
+    ".globl PushAllRegistersAndIterateStack             \n"
     ".hidden PushAllRegistersAndIterateStack            \n"
     "PushAllRegistersAndIterateStack:                   \n"
+#endif  // !_WIN32
     // [ IterateStackCallback ]
     // [ StackVisitor*        ]
     // [ Stack*               ]
@@ -49,5 +50,3 @@ asm(".globl PushAllRegistersAndIterateStack             \n"
     // Restore rbp as it was used as frame pointer.
     "  pop %ebp                                         \n"
     "  ret                                              \n");
-
-#endif  // !_WIN32

src/heap/cppgc/stack.cc

@@ -13,7 +13,7 @@
 namespace cppgc {
 namespace internal {
 
-using IterateStackCallback = void (Stack::*)(StackVisitor*, intptr_t*) const;
+using IterateStackCallback = void (*)(const Stack*, StackVisitor*, intptr_t*);
 extern "C" void PushAllRegistersAndIterateStack(const Stack*, StackVisitor*,
                                                 IterateStackCallback);
 
@@ -88,23 +88,14 @@ void IterateSafeStackIfNecessary(StackVisitor* visitor) {
 #endif  // defined(__has_feature)
 }
 
-#endif  // CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
-
-}  // namespace
-
-#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  // CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
-
+// Called by the trampoline that pushes registers on the stack. This method
+// should never be inlined to ensure that a possible redzone cannot contain
+// any data that needs to be scanned.
+V8_NOINLINE
 // No ASAN support as method accesses redzones while walking the stack.
 NO_SANITIZE_ADDRESS
-void Stack::IteratePointersImpl(StackVisitor* visitor,
-                                intptr_t* stack_end) const {
+void IteratePointersImpl(const Stack* stack, StackVisitor* visitor,
+                         intptr_t* stack_end) {
 #ifdef V8_USE_ADDRESS_SANITIZER
   void* asan_fake_stack = __asan_get_current_fake_stack();
 #endif  // V8_USE_ADDRESS_SANITIZER
@@ -113,7 +104,7 @@ void Stack::IteratePointersImpl(StackVisitor* visitor,
   constexpr size_t kMinStackAlignment = sizeof(void*);
   void** current = reinterpret_cast<void**>(stack_end);
   CHECK_EQ(0u, reinterpret_cast<uintptr_t>(current) & (kMinStackAlignment - 1));
-  for (; current < stack_start_; ++current) {
+  for (; current < stack->stack_start(); ++current) {
     // MSAN: Instead of unpoisoning the whole stack, the slot's value is copied
     // into a local which is unpoisoned.
     void* address = *current;
@@ -121,11 +112,24 @@ void Stack::IteratePointersImpl(StackVisitor* visitor,
     if (address == nullptr) continue;
     visitor->VisitPointer(address);
 #ifdef V8_USE_ADDRESS_SANITIZER
-    IterateAsanFakeFrameIfNecessary(visitor, asan_fake_stack, stack_start_,
-                                    stack_end, address);
+    IterateAsanFakeFrameIfNecessary(visitor, asan_fake_stack,
+                                    stack->stack_start(), stack_end, address);
 #endif  // V8_USE_ADDRESS_SANITIZER
   }
 }
 
+#endif  // CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
+
+}  // namespace
+
+#ifdef CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
+void Stack::IteratePointers(StackVisitor* visitor) const {
+  PushAllRegistersAndIterateStack(this, visitor, &IteratePointersImpl);
+  // No need to deal with callee-saved registers as they will be kept alive by
+  // the regular conservative stack iteration.
+  IterateSafeStackIfNecessary(visitor);
+}
+#endif  // CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
+
 }  // namespace internal
 }  // namespace cppgc

src/heap/cppgc/stack.h

@@ -32,13 +32,10 @@ class V8_EXPORT_PRIVATE Stack final {
   void IteratePointers(StackVisitor* visitor) const;
 #endif  // CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN
 
- private:
-  // Called by the trampoline that pushes registers on the stack. This method
-  // should never be inlined to ensure that a possible redzone cannot contain
-  // any data that needs to be scanned.
-  V8_NOINLINE void IteratePointersImpl(StackVisitor* visitor,
-                                       intptr_t* stack_end) const;
+  // Returns the start of the stack.
+  const void* stack_start() const { return stack_start_; }
 
+ private:
   const void* stack_start_;
 };