[vm] Avoid reserved register error on newer compilers

Don't mark SP as clobbered because newer clang versions
do not like that.

Fixes https://github.com/dart-lang/sdk/issues/46873

TEST=ci

Cq-Include-Trybots: luci.dart.try:vm-ffi-android-release-arm-try,vm-ffi-android-product-arm-try
Change-Id: I5b794e7bb02e62576c4c40b8132f9c798cb7639c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209917
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>

runtime/vm/BUILD.gn

@@ -109,6 +109,12 @@ library_for_all_configs("libdart_vm") {
   sources = vm_sources + rebase_path(compiler_api_sources, ".", "./compiler/") +
             rebase_path(disassembler_sources, ".", "./compiler/") +
             rebase_path(heap_sources, ".", "./heap/")
+  if (is_android) {
+    # Android specific workaround for a kernel bug. This source file can't
+    # go into vm_sources list because it will break Windows build which
+    # uses different assembler syntax.
+    sources += [ "thread_interrupter_android_arm.S" ]
+  }
   include_dirs = [ ".." ]
 }
 

runtime/vm/signal_handler.h

@@ -38,84 +38,19 @@ struct sigset_t {};
 #include <ucontext.h>  // NOLINT
 #endif
 
-// Old linux kernels on ARM might require a trampoline to
-// work around incorrect Thumb -> ARM transitions. See SignalHandlerTrampoline
-// below for more details.
-#if defined(HOST_ARCH_ARM) &&                                                  \
-    (defined(DART_HOST_OS_LINUX) || defined(DART_HOST_OS_ANDROID)) &&          \
-    !defined(__thumb__)
-#define USE_SIGNAL_HANDLER_TRAMPOLINE
-#endif
-
 namespace dart {
 
 typedef void (*SignalAction)(int signal, siginfo_t* info, void* context);
 
 class SignalHandler : public AllStatic {
  public:
-  template <SignalAction action>
-  static void Install() {
-#if defined(USE_SIGNAL_HANDLER_TRAMPOLINE)
-    InstallImpl(SignalHandlerTrampoline<action>);
-#else
-    InstallImpl(action);
-#endif  // defined(USE_SIGNAL_HANDLER_TRAMPOLINE)
-  }
+  static void Install(SignalAction action);
   static void Remove();
   static uintptr_t GetProgramCounter(const mcontext_t& mcontext);
   static uintptr_t GetFramePointer(const mcontext_t& mcontext);
   static uintptr_t GetCStackPointer(const mcontext_t& mcontext);
   static uintptr_t GetDartStackPointer(const mcontext_t& mcontext);
   static uintptr_t GetLinkRegister(const mcontext_t& mcontext);
-
- private:
-  static void InstallImpl(SignalAction action);
-
-#if defined(USE_SIGNAL_HANDLER_TRAMPOLINE)
-  // Work around for a bug in old kernels (only fixed in 3.18 Android kernel):
-  //
-  // Kernel does not clear If-Then execution state bits when entering ARM signal
-  // handler which violates requirements imposed by ARM architecture reference.
-  // Some CPUs look at these bits even while in ARM mode which causes them
-  // to skip some instructions in the prologue of the signal handler.
-  //
-  // To work around the issue we insert enough NOPs in the prologue to ensure
-  // that no actual instructions are skipped and then branch to the actual
-  // signal handler.
-  //
-  // For the kernel patch that fixes the issue see:
-  // http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6ecf830e5029598732e04067e325d946097519cb
-  //
-  // Note: this function is marked "naked" because we must guarantee that
-  // our NOPs occur before any compiler generated prologue.
-  template <SignalAction action>
-  static __attribute__((naked)) void SignalHandlerTrampoline(int signal,
-                                                             siginfo_t* info,
-                                                             void* context_) {
-    // IT (If-Then) instruction makes up to four instructions that follow it
-    // conditional.
-    // Note: clobber all register so that compiler does not attempt to hoist
-    // anything from the next assembly block past this one.
-    asm volatile("nop; nop; nop; nop;"
-                 :
-                 :
-                 : "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", "r8", "r9",
-                   "r10", "r11", "r12", "r13", "r14", "memory");
-
-    // Tail-call into the actual signal handler.
-    //
-    // Note: this code is split into a separate inline assembly block because
-    // any code that compiler generates to satisfy register constraints must
-    // be generated after four NOPs.
-    //
-    // Note: there is no portable way to specify that we want to have
-    // signal, info and context_ in r0 - r2 respectively. So we just mark them
-    // as clobbered and hope that compiler does not emit any code that uses
-    // these registers to satisfy action constraint (we tested on clang and
-    // the generated code looks like one would expect).
-    asm volatile("bx %0;" : : "r"(action) : "r0", "r1", "r2", "memory");
-  }
-#endif  // defined(USE_SIGNAL_HANDLER_TRAMPOLINE)
 };
 
 #undef USE_SIGNAL_HANDLER_TRAMPOLINE

runtime/vm/signal_handler_android.cc

@@ -94,7 +94,7 @@ uintptr_t SignalHandler::GetLinkRegister(const mcontext_t& mcontext) {
   return lr;
 }
 
-void SignalHandler::InstallImpl(SignalAction action) {
+void SignalHandler::Install(SignalAction action) {
   // Bionic implementation of setjmp temporary mangles SP register
   // in place which breaks signal delivery on the thread stack - when
   // kernel tries to deliver SIGPROF and we are in the middle of

runtime/vm/signal_handler_fuchsia.cc

@@ -36,7 +36,7 @@ uintptr_t SignalHandler::GetLinkRegister(const mcontext_t& mcontext) {
   return 0;
 }
 
-void SignalHandler::InstallImpl(SignalAction action) {
+void SignalHandler::Install(SignalAction action) {
   UNIMPLEMENTED();
 }
 

runtime/vm/signal_handler_linux.cc

@@ -94,7 +94,7 @@ uintptr_t SignalHandler::GetLinkRegister(const mcontext_t& mcontext) {
   return lr;
 }
 
-void SignalHandler::InstallImpl(SignalAction action) {
+void SignalHandler::Install(SignalAction action) {
   struct sigaction act = {};
   act.sa_handler = NULL;
   act.sa_sigaction = action;

runtime/vm/signal_handler_macos.cc

@@ -90,7 +90,7 @@ uintptr_t SignalHandler::GetLinkRegister(const mcontext_t& mcontext) {
   return lr;
 }
 
-void SignalHandler::InstallImpl(SignalAction action) {
+void SignalHandler::Install(SignalAction action) {
   struct sigaction act = {};
   act.sa_handler = NULL;
   act.sa_sigaction = action;

runtime/vm/signal_handler_win.cc

@@ -33,7 +33,7 @@ uintptr_t SignalHandler::GetLinkRegister(const mcontext_t& mcontext) {
   return 0;
 }
 
-void SignalHandler::InstallImpl(SignalAction action) {
+void SignalHandler::Install(SignalAction action) {
   UNIMPLEMENTED();
 }
 

runtime/vm/thread_interrupter_android.cc

@@ -18,36 +18,48 @@ namespace dart {
 
 #ifndef PRODUCT
 
+// Old linux kernels on ARM might require a trampoline to
+// work around incorrect Thumb -> ARM transitions.
+// See thread_interrupted_android_arm.S for more details.
+#if defined(HOST_ARCH_ARM) &&                                                  \
+    (defined(DART_HOST_OS_LINUX) || defined(DART_HOST_OS_ANDROID)) &&          \
+    !defined(__thumb__)
+#define USE_SIGNAL_HANDLER_TRAMPOLINE
+#endif
+
 DECLARE_FLAG(bool, trace_thread_interrupter);
 
-class ThreadInterrupterAndroid : public AllStatic {
- public:
-  static void ThreadInterruptSignalHandler(int signal,
-                                           siginfo_t* info,
-                                           void* context_) {
-    if (signal != SIGPROF) {
-      return;
-    }
-    Thread* thread = Thread::Current();
-    if (thread == NULL) {
-      return;
-    }
-    ThreadInterrupter::SampleBufferWriterScope scope;
-    if (!scope.CanSample()) {
-      return;
-    }
-    // Extract thread state.
-    ucontext_t* context = reinterpret_cast<ucontext_t*>(context_);
-    mcontext_t mcontext = context->uc_mcontext;
-    InterruptedThreadState its;
-    its.pc = SignalHandler::GetProgramCounter(mcontext);
-    its.fp = SignalHandler::GetFramePointer(mcontext);
-    its.csp = SignalHandler::GetCStackPointer(mcontext);
-    its.dsp = SignalHandler::GetDartStackPointer(mcontext);
-    its.lr = SignalHandler::GetLinkRegister(mcontext);
-    Profiler::SampleThread(thread, its);
+namespace {
+#if defined(USE_SIGNAL_HANDLER_TRAMPOLINE)
+extern "C" {
+#endif
+void ThreadInterruptSignalHandler(int signal, siginfo_t* info, void* context_) {
+  if (signal != SIGPROF) {
+    return;
   }
-};
+  Thread* thread = Thread::Current();
+  if (thread == NULL) {
+    return;
+  }
+  ThreadInterrupter::SampleBufferWriterScope scope;
+  if (!scope.CanSample()) {
+    return;
+  }
+  // Extract thread state.
+  ucontext_t* context = reinterpret_cast<ucontext_t*>(context_);
+  mcontext_t mcontext = context->uc_mcontext;
+  InterruptedThreadState its;
+  its.pc = SignalHandler::GetProgramCounter(mcontext);
+  its.fp = SignalHandler::GetFramePointer(mcontext);
+  its.csp = SignalHandler::GetCStackPointer(mcontext);
+  its.dsp = SignalHandler::GetDartStackPointer(mcontext);
+  its.lr = SignalHandler::GetLinkRegister(mcontext);
+  Profiler::SampleThread(thread, its);
+}
+#if defined(USE_SIGNAL_HANDLER_TRAMPOLINE)
+}  // extern "C"
+#endif
+}  // namespace
 
 bool ThreadInterrupter::IsDebuggerAttached() {
   return false;
@@ -62,9 +74,19 @@ void ThreadInterrupter::InterruptThread(OSThread* thread) {
   ASSERT((result == 0) || (result == ESRCH));
 }
 
+#if defined(USE_SIGNAL_HANDLER_TRAMPOLINE)
+// Defined in thread_interrupted_android_arm.S
+extern "C" void ThreadInterruptSignalHandlerTrampoline(int signal,
+                                                       siginfo_t* info,
+                                                       void* context_);
+#endif
+
 void ThreadInterrupter::InstallSignalHandler() {
-  SignalHandler::Install<
-      ThreadInterrupterAndroid::ThreadInterruptSignalHandler>();
+#if defined(USE_SIGNAL_HANDLER_TRAMPOLINE)
+  SignalHandler::Install(&ThreadInterruptSignalHandlerTrampoline);
+#else
+  SignalHandler::Install(&ThreadInterruptSignalHandler);
+#endif
 }
 
 void ThreadInterrupter::RemoveSignalHandler() {

runtime/vm/thread_interrupter_android_arm.S

@@ -0,0 +1,43 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Work around for a bug in old kernels (only fixed in 3.18 Android kernel):
+//
+// Kernel does not clear If-Then execution state bits when entering ARM signal
+// handler which violates requirements imposed by ARM architecture reference.
+// Some CPUs look at these bits even while in ARM mode which causes them
+// to skip some instructions in the prologue of the signal handler.
+//
+// To work around the issue we insert enough NOPs in the prologue to ensure
+// that no actual instructions are skipped and then branch to the actual
+// signal handler.
+//
+// For the kernel patch that fixes the issue see:
+// http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6ecf830e5029598732e04067e325d946097519cb
+//
+
+// Note: can't use DART_* defines here because this file does not include
+// globals.h.
+#if !defined(PRODUCT) && defined(__ARMEL__) && defined(__ANDROID__) &&         \
+    !defined(__thumb__)
+
+.text
+
+.globl ThreadInterruptSignalHandlerTrampoline
+.hidden ThreadInterruptSignalHandlerTrampoline
+.type ThreadInterruptSignalHandlerTrampoline, %function
+ThreadInterruptSignalHandlerTrampoline:
+  // IT (If-Then) instruction makes up to four instructions that follow it
+  // conditional.
+  nop
+  nop
+  nop
+  nop
+  // Tail-call the actual handler.
+  // Note: no need to use interworking because we know that we are not
+  // compiling for Thumb.
+  b ThreadInterruptSignalHandler
+.size ThreadInterruptSignalHandlerTrampoline,.-ThreadInterruptSignalHandlerTrampoline
+
+#endif

runtime/vm/thread_interrupter_linux.cc

@@ -62,8 +62,7 @@ void ThreadInterrupter::InterruptThread(OSThread* thread) {
 }
 
 void ThreadInterrupter::InstallSignalHandler() {
-  SignalHandler::Install<
-      ThreadInterrupterLinux::ThreadInterruptSignalHandler>();
+  SignalHandler::Install(&ThreadInterrupterLinux::ThreadInterruptSignalHandler);
 }
 
 void ThreadInterrupter::RemoveSignalHandler() {

runtime/vm/thread_interrupter_macos.cc

@@ -83,8 +83,7 @@ void ThreadInterrupter::InterruptThread(OSThread* thread) {
 }
 
 void ThreadInterrupter::InstallSignalHandler() {
-  SignalHandler::Install<
-      ThreadInterrupterMacOS::ThreadInterruptSignalHandler>();
+  SignalHandler::Install(&ThreadInterrupterMacOS::ThreadInterruptSignalHandler);
 }
 
 void ThreadInterrupter::RemoveSignalHandler() {