[vm] Reserve larger alternative signal stack if needed

Commit 18bdb28ef6 have configured `SIGPROF` handler on Android
to use alternative signal stack to workaround a [bug][1] in Bionic
implementation of `setjmp`. However when implementing the
workaround we have misinterpreted `sigaltstack` documentation
and assumed that `sigaltstack` configures alternative stack
*globally*, similar to how `sigaction` configures the signal handler.
This is not correct: `sigaltstack` configures alternative stack
for the current thread only.

Nevertheless our workaround kinda worked as intended because Bionic's
`pthread_create` actually assigns an individual alternative
stack to each new thread since [Android L][2].

However older version of Bionic (pre [Android 6.0.1][3]) configured
alternative signal stack which was too small for ARM64. This meant
that non-trivial signal handler could easily overflow the stack
and cause a SIGSEGV when hitting a guard page.

This is what we observe in the flutter/flutter#130003: launching
Flutter application with profiler enabled in a ARM64 emulator
running Android 6.0 causes an immediate segfault in the
signal handler.

This CL changes our code to make sure that alternative signal
stack associated with the current thread is present and is large
enough and allocate a new one if it is not.

[1]: b/152210274
[2]: https://android-review.git.corp.google.com/c/platform/bionic/+/62238
[3]: https://android-review.git.corp.google.com/c/platform/bionic/+/172213
[4]: https://github.com/flutter/flutter/issues/130003

TEST=manually on an Android 6.0.0 ARM64 emulator

Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-ffi-android-release-arm64c-try,vm-ffi-android-release-arm-try
Change-Id: Ib87df25e72ad3f486e90cf2fc6d7f980a8e0b1dc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/327866
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>

runtime/vm/os_thread.cc

@@ -38,22 +38,13 @@ inline void UpdateTimelineTrackMetadata(const OSThread& thread) {
 OSThread::OSThread()
     : BaseThread(true),
       id_(OSThread::GetCurrentThreadId()),
-#if defined(DEBUG)
-      join_id_(kInvalidThreadJoinId),
-#endif
 #ifdef SUPPORT_TIMELINE
       trace_id_(OSThread::GetCurrentThreadTraceId()),
 #endif
       name_(OSThread::GetCurrentThreadName()),
       timeline_block_lock_(),
-      timeline_block_(nullptr),
-      thread_list_next_(nullptr),
       thread_interrupt_disabled_(1),  // Thread interrupts disabled by default.
-      log_(new class Log()),
-      stack_base_(0),
-      stack_limit_(0),
-      stack_headroom_(0),
-      thread_(nullptr) {
+      log_(new class Log()) {
   // Try to get accurate stack bounds from pthreads, etc.
   if (!GetCurrentStackBounds(&stack_limit_, &stack_base_)) {
     FATAL("Failed to retrieve stack bounds");
@@ -106,6 +97,11 @@ OSThread::~OSThread() {
 #endif
   timeline_block_ = nullptr;
   free(name_);
+  if (FLAG_profiler && prepared_for_interrupts_) {
+    ThreadInterrupter::CleanupCurrentThreadState(thread_interrupter_state_);
+    thread_interrupter_state_ = nullptr;
+    prepared_for_interrupts_ = false;
+  }
 }
 
 void OSThread::SetName(const char* name) {
@@ -147,6 +143,10 @@ void OSThread::EnableThreadInterrupts() {
   if (FLAG_profiler && (old == 1)) {
     // We just decremented from 1 to 0.
     // Make sure the thread interrupter is awake.
+    if (!prepared_for_interrupts_) {
+      thread_interrupter_state_ = ThreadInterrupter::PrepareCurrentThread();
+      prepared_for_interrupts_ = true;
+    }
     ThreadInterrupter::WakeUp();
   }
   if (old == 0) {

runtime/vm/os_thread.h

@@ -281,7 +281,7 @@ class OSThread : public BaseThread {
 #if defined(DEBUG)
   // In DEBUG mode we use this field to ensure that GetCurrentThreadJoinId is
   // only called once per OSThread.
-  ThreadJoinId join_id_;
+  ThreadJoinId join_id_ = kInvalidThreadJoinId;
 #endif
 #ifdef SUPPORT_TIMELINE
   const ThreadId trace_id_;  // Used to interface with tracing tools.
@@ -291,17 +291,19 @@ class OSThread : public BaseThread {
   mutable Mutex timeline_block_lock_;
   // The block that the timeline recorder has permitted this thread to write
   // events to.
-  TimelineEventBlock* timeline_block_;
+  TimelineEventBlock* timeline_block_ = nullptr;
 
   // All |Thread|s are registered in the thread list.
-  OSThread* thread_list_next_;
+  OSThread* thread_list_next_ = nullptr;
 
   RelaxedAtomic<uintptr_t> thread_interrupt_disabled_;
+  bool prepared_for_interrupts_ = false;
+  void* thread_interrupter_state_ = nullptr;
   Log* log_;
-  uword stack_base_;
-  uword stack_limit_;
-  uword stack_headroom_;
-  ThreadState* thread_;
+  uword stack_base_ = 0;
+  uword stack_limit_ = 0;
+  uword stack_headroom_ = 0;
+  ThreadState* thread_ = nullptr;
   // The ThreadPool::Worker which owns this OSThread. If this OSThread was not
   // started by a ThreadPool it will be nullptr. This TLS value is not
   // protected and should only be read/written by the OSThread itself.

runtime/vm/signal_handler.h

@@ -51,6 +51,15 @@ class SignalHandler : public AllStatic {
   static uintptr_t GetCStackPointer(const mcontext_t& mcontext);
   static uintptr_t GetDartStackPointer(const mcontext_t& mcontext);
   static uintptr_t GetLinkRegister(const mcontext_t& mcontext);
+
+#if defined(DART_HOST_OS_ANDROID)
+  // Prepare current thread for handling interrupts. Returns
+  // opaque pointer to the allocated state (if any).
+  static void* PrepareCurrentThread();
+
+  // Cleanup any state which was created by |PrepareCurrentThread|.
+  static void CleanupCurrentThreadState(void* state);
+#endif
 };
 
 #undef USE_SIGNAL_HANDLER_TRAMPOLINE

runtime/vm/signal_handler_android.cc

@@ -110,21 +110,19 @@ void SignalHandler::Install(SignalAction action) {
   // or outside of writable space at all. In the first case we
   // get memory corruption and in the second case kernel would send
   // SIGSEGV to the process. See b/152210274 for details.
-  // To work around this issue we are using alternative signal stack
-  // to handle SIGPROF signals.
-  stack_t ss;
-  ss.ss_size = SIGSTKSZ;
-  ss.ss_sp = malloc(ss.ss_size);
-  ss.ss_flags = 0;
-  int r = sigaltstack(&ss, nullptr);
-  ASSERT(r == 0);
-
+  // To work around this issue we request SIGPROF signals to be delivered
+  // on the alternative signal stack by setting SA_ONSTACK. The stack itself
+  // is configured when interrupts are enabled for a particular thread.
+  // In reality Bionic's |pthread_create| eagerly creates and assigns an
+  // alternative signal stack for each thread. However older versions of Bionic
+  // (L and below) make the size of alternative stack too small which causes
+  // stack overflows and crashes.
   struct sigaction act = {};
   act.sa_sigaction = action;
   sigemptyset(&act.sa_mask);
   sigaddset(&act.sa_mask, SIGPROF);  // Prevent nested signals.
   act.sa_flags = SA_RESTART | SA_SIGINFO | SA_ONSTACK;
-  r = sigaction(SIGPROF, &act, nullptr);
+  int r = sigaction(SIGPROF, &act, nullptr);
   ASSERT(r == 0);
 }
 
@@ -135,14 +133,56 @@ void SignalHandler::Remove() {
   act.sa_handler = SIG_IGN;
   sigemptyset(&act.sa_mask);
   int r = sigaction(SIGPROF, &act, nullptr);
+  RELEASE_ASSERT(r == 0);
+}
+
+void* SignalHandler::PrepareCurrentThread() {
+  // These constants are selected to prevent allocating alternative signal
+  // stack if Bionic has already allocated large enough one for us. They
+  // match current values used in Bionic[1].
+  //
+  // [1]: https://cs.android.com/android/platform/superproject/main/+/main:bionic/libc/bionic/pthread_internal.h;drc=3649db34a154cedb8ef53a5adbaa349970159b58;l=243
+  const intptr_t kGuardPageSize = 4 * KB;
+#if defined(TARGET_ARCH_IS_64_BIT)
+  const intptr_t kSigAltStackSize = 32 * KB;
+#else
+  const intptr_t kSigAltStackSize = 16 * KB;
+#endif
+
+  // First check if the alternative signal stack is already installed and
+  // large enough.
+  int r;
+  stack_t ss;
+  memset(&ss, 0, sizeof(ss));
+  r = sigaltstack(nullptr, &ss);
+  ASSERT(r == 0);
+  if (ss.ss_flags == 0 && ss.ss_size >= (kSigAltStackSize - kGuardPageSize)) {
+    // Bionic has created a large enough stack already.
+    return nullptr;
+  }
+
+  // We are running on an older version of Android, where Bionic creates
+  // stacks which are too small.
+  ss.ss_sp = malloc(kSigAltStackSize);
+  ss.ss_size = kSigAltStackSize;
+  ss.ss_flags = 0;
+  r = sigaltstack(&ss, nullptr);
   ASSERT(r == 0);
 
-  // Disable and delete alternative signal stack.
-  stack_t ss, old_ss;
-  ss.ss_flags = SS_DISABLE;
-  r = sigaltstack(&ss, &old_ss);
-  ASSERT(r == 0);
-  free(old_ss.ss_sp);
+  return ss.ss_sp;
+}
+
+void SignalHandler::CleanupCurrentThreadState(void* stack) {
+  if (stack != nullptr) {
+    // Disable alternative stack then free allocated memory.
+    stack_t ss, old_ss;
+    memset(&ss, 0, sizeof(ss));
+    ss.ss_flags = SS_DISABLE;
+    int r = sigaltstack(&ss, &old_ss);
+    ASSERT(r == 0);
+    ASSERT(old_ss.ss_sp == stack);
+    free(stack);
+  }
 }
 
 }  // namespace dart

runtime/vm/thread_interrupter.cc

@@ -230,6 +230,14 @@ void ThreadInterrupter::ThreadMain(uword parameters) {
   }
 }
 
+#if !defined(DART_HOST_OS_ANDROID)
+void* ThreadInterrupter::PrepareCurrentThread() {
+  return nullptr;
+}
+
+void ThreadInterrupter::CleanupCurrentThreadState(void* state) {}
+#endif
+
 #endif  // !PRODUCT
 
 }  // namespace dart

runtime/vm/thread_interrupter.h

@@ -36,6 +36,13 @@ class ThreadInterrupter : public AllStatic {
   // Interrupt a thread.
   static void InterruptThread(OSThread* thread);
 
+  // Prepare current thread for handling interrupts. Returns
+  // opaque pointer to the allocated state (if any).
+  static void* PrepareCurrentThread();
+
+  // Cleanup any state which was created by |PrepareCurrentThread|.
+  static void CleanupCurrentThreadState(void* state);
+
  private:
   static constexpr intptr_t kMaxThreads = 4096;
   static bool initialized_;

runtime/vm/thread_interrupter_android.cc

@@ -86,6 +86,14 @@ void ThreadInterrupter::RemoveSignalHandler() {
   SignalHandler::Remove();
 }
 
+void* ThreadInterrupter::PrepareCurrentThread() {
+  return SignalHandler::PrepareCurrentThread();
+}
+
+void ThreadInterrupter::CleanupCurrentThreadState(void* state) {
+  SignalHandler::CleanupCurrentThreadState(state);
+}
+
 #endif  // !PRODUCT
 
 }  // namespace dart