From be7e57665ebbd4d7a79d54cb4f6c4851280a2e78 Mon Sep 17 00:00:00 2001 From: Michael Lippautz Date: Thu, 2 Apr 2020 00:11:38 +0200 Subject: [PATCH] cppgc: Use inline asm to generate x64 stack scanning trampoline Use inline asm to generate the x64 PushAllRegistersAndIterateStack which is the trampoline for conservative stack scanning. Keep the function definition as C code to allow clang to generate the correct mangling for each platform. This approach has the benefit that it immediately works for all platforms that support clang. Bug: chromium:1056170 Change-Id: Ic7a1c1b57e67ae1442bd8bda4e55d89112facfc7 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2132787 Reviewed-by: Omer Katz Reviewed-by: Anton Bikineev Commit-Queue: Michael Lippautz Cr-Commit-Position: refs/heads/master@{#66958} --- BUILD.gn | 18 +++- src/heap/cppgc/asm/x64/push_registers.S | 52 ------------ .../cppgc/asm/x64/push_registers_clang.cc | 83 +++++++++++++++++++ src/heap/cppgc/stack.h | 11 --- test/unittests/BUILD.gn | 1 + 5 files changed, 99 insertions(+), 66 deletions(-) delete mode 100644 src/heap/cppgc/asm/x64/push_registers.S create mode 100644 src/heap/cppgc/asm/x64/push_registers_clang.cc diff --git a/BUILD.gn b/BUILD.gn index 17b0159848..e6725b5362 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -346,6 +346,15 @@ 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" ] + } + } +} + # This config should be applied to code using the libsampler. config("libsampler_config") { include_dirs = [ "include" ] @@ -3955,12 +3964,15 @@ v8_source_set("cppgc_base") { ] if (target_cpu == "x64") { - if (!is_win) { - sources += [ "src/heap/cppgc/asm/x64/push_registers.S" ] + if (is_clang) { + sources += [ "src/heap/cppgc/asm/x64/push_registers_clang.cc" ] } } - configs = [ ":internal_config" ] + configs = [ + ":internal_config", + ":cppgc_base_config", + ] public_deps = [ ":v8_libbase" ] } diff --git a/src/heap/cppgc/asm/x64/push_registers.S b/src/heap/cppgc/asm/x64/push_registers.S deleted file mode 100644 index c73247383e..0000000000 --- a/src/heap/cppgc/asm/x64/push_registers.S +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright 2020 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -.att_syntax - -.text - -#if defined(V8_TARGET_OS_MACOSX) || defined(V8_TARGET_OS_IOS) - -.globl _PushAllRegistersAndIterateStack -_PushAllRegistersAndIterateStack: - -#else // !(defined(V8_TARGET_OS_MACOSX) || defined(V8_TARGET_OS_IOS)) - -.type PushAllRegistersAndIterateStack, %function -.global PushAllRegistersAndIterateStack -.hidden PushAllRegistersAndIterateStack -PushAllRegistersAndIterateStack: - -#endif // !(defined(V8_TARGET_OS_MACOSX) || defined(V8_TARGET_OS_IOS)) - - // Push all callee-saved registers to get them on the stack for conservative - // stack scanning. - // - // We maintain 16-byte alignment at calls. There is an 8-byte return address - // on the stack and we push 56 bytes which maintains 16-byte stack alignment - // at the call. - // Source: https://github.com/hjl-tools/x86-psABI/wiki/x86-64-psABI-1.0.pdf - // - // rbp is callee-saved. Maintain proper frame pointer for debugging. - push %rbp - mov %rsp, %rbp - push $0xCDCDCD // Dummy for alignment. - push %rbx - push %r12 - push %r13 - push %r14 - push %r15 - // Pass 1st parameter (rdi) unchanged (Stack*). - // Pass 2nd parameter (rsi) unchanged (StackVisitor*). - // Save 3rd parameter (rdx; IterateStackCallback) - mov %rdx, %r8 - // Pass 3rd parameter as rsp (stack pointer). - mov %rsp, %rdx - // Call the callback. - call *%r8 - // Pop the callee-saved registers. - add $48, %rsp - // Restore rbp as it was used as frame pointer. - pop %rbp - ret diff --git a/src/heap/cppgc/asm/x64/push_registers_clang.cc b/src/heap/cppgc/asm/x64/push_registers_clang.cc new file mode 100644 index 0000000000..9bdc57523a --- /dev/null +++ b/src/heap/cppgc/asm/x64/push_registers_clang.cc @@ -0,0 +1,83 @@ +// Copyright 2020 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef V8_HAVE_TARGET_OS +#error "File assumes V8_TARGET_OS_* defines are present" +#endif // V8_HAVE_TARGET_OS + +// Push all callee-saved registers to get them on the stack for conservative +// stack scanning. +// +// Do not add any C code to the function. The function is naked to avoid +// emitting a prologue/epilogue that could violate alginment computations. +extern "C" __attribute__((naked, noinline)) void +PushAllRegistersAndIterateStack(void* /* {Stack*} */, + void* /* {StackVisitor*} */, + void* /* {IterateStackCallback} */) { +#ifdef V8_TARGET_OS_WIN + + // We maintain 16-byte alignment at calls. There is an 8-byte return address + // on the stack and we push 72 bytes which maintains 16-byte stack alignment + // at the call. + // Source: https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention + asm volatile( + // rbp is callee-saved. Maintain proper frame pointer for debugging. + " push %rbp \n" + " mov %rsp, %rbp \n" + // Dummy for alignment. + " push $0xCDCDCD \n" + " push %rsi \n" + " push %rdi \n" + " push %rbx \n" + " push %r12 \n" + " push %r13 \n" + " push %r14 \n" + " push %r15 \n" + // Pass 1st parameter (rcx) unchanged (Stack*). + // Pass 2nd parameter (rdx) unchanged (StackVisitor*). + // Save 3rd parameter (r8; IterateStackCallback) + " mov %r8, %r9 \n" + // Pass 3rd parameter as rsp (stack pointer). + " mov %rsp, %r8 \n" + // Call the callback. + " call *%r9 \n" + // Pop the callee-saved registers. + " add $64, %rsp \n" + // Restore rbp as it was used as frame pointer. + " pop %rbp \n" + " ret \n"); + +#else // !V8_TARGET_OS_WIN + + // We maintain 16-byte alignment at calls. There is an 8-byte return address + // on the stack and we push 56 bytes which maintains 16-byte stack alignment + // at the call. + // Source: https://github.com/hjl-tools/x86-psABI/wiki/x86-64-psABI-1.0.pdf + asm volatile( + // rbp is callee-saved. Maintain proper frame pointer for debugging. + " push %rbp \n" + " mov %rsp, %rbp \n" + // Dummy for alignment. + " push $0xCDCDCD \n" + " push %rbx \n" + " push %r12 \n" + " push %r13 \n" + " push %r14 \n" + " push %r15 \n" + // Pass 1st parameter (rdi) unchanged (Stack*). + // Pass 2nd parameter (rsi) unchanged (StackVisitor*). + // Save 3rd parameter (rdx; IterateStackCallback) + " mov %rdx, %r8 \n" + // Pass 3rd parameter as rsp (stack pointer). + " mov %rsp, %rdx \n" + // Call the callback. + " call *%r8 \n" + // Pop the callee-saved registers. + " add $48, %rsp \n" + // Restore rbp as it was used as frame pointer. + " pop %rbp \n" + " ret \n"); + +#endif // !V8_TARGET_OS_WIN +} diff --git a/src/heap/cppgc/stack.h b/src/heap/cppgc/stack.h index d97029277d..3494c5af25 100644 --- a/src/heap/cppgc/stack.h +++ b/src/heap/cppgc/stack.h @@ -7,17 +7,6 @@ #include "src/base/macros.h" -// TODO(chromium:1056170): Implement all platforms. -// TODO(chromium:1056170): Should use HOST arch instead of target arch. Fix -// requires fixing e.g. simulator platforms. -#ifdef __clang__ -#if defined(V8_TARGET_ARCH_X64) -#if !defined(V8_TARGET_OS_WIN) -#define CPPGC_SUPPORTS_CONSERVATIVE_STACK_SCAN 1 -#endif -#endif -#endif - namespace cppgc { namespace internal { diff --git a/test/unittests/BUILD.gn b/test/unittests/BUILD.gn index af93dcb266..25372e6953 100644 --- a/test/unittests/BUILD.gn +++ b/test/unittests/BUILD.gn @@ -58,6 +58,7 @@ v8_source_set("cppgc_unittests_sources") { configs = [ "../..:external_config", "../..:internal_config_base", + "../..:cppgc_base_config", ] deps = [