fix native stack checking on Android/Bionic

Summary:
Related to: facebook#1535

It turns out that on Android/Bionic `pthread_attr_getstack()` includes
the stack guard pages in the returned size. Whether that is a bug or not
is debatable, but it differs in behavior from Linux/glibc. Plus,
returning a stack range that is not actually not all accessible is less
than useful.

In practice it worked with our approach because the default stack guard
size is 4KB on Android, while our "native gap" is 64KB. Our gap is
larger than the stack guard size, so we would still correctly detect and
catch stack overflow.

However, if the stack guard size is 64KB (or larger), we would not
detect the overflow and crash. It is possible to manually set a larger
guard size, plus security-oriented OS-es like GrapheneOS do it by
default.

The fix is to subtract the guard size from the available stack size when
running on Bionic.

Added tests checking the behavior in the main thread, in a default
thread and in a thread with a 64KB guard. Two separate tests are
performed under these circumstances:
- Unbounded recursion checking for native stack overflow
- Manually reading the available stack range to ensure it is all
accessible.

Differential Revision: D64308925

lib/Support/OSCompatPosix.cpp

@@ -642,7 +642,24 @@ std::pair<const void *, size_t> thread_stack_bounds(unsigned gap) {
 
   void *origin;
   size_t size;
-  pthread_attr_getstack(&attr, &origin, &size);
+  if (pthread_attr_getstack(&attr, &origin, &size))
+    hermes_fatal("Unable to obtain native stack bounds");
+
+#ifdef __BIONIC__
+  // It appears that on Android/Bionic, the range returned by
+  // pthread_attr_getstack() includes the stack guard pages. We must remove them
+  // from the bounds.
+  size_t guardSize;
+  if (pthread_attr_getguardsize(&attr, &guardSize)) {
+    // Don't give up in case of error.
+    guardSize = 0;
+  }
+  if (guardSize > size)
+    guardSize = size;
+  // Exclude the guard pages from the available stack.
+  origin = (char *)origin + guardSize;
+  size -= guardSize;
+#endif
 
   pthread_attr_destroy(&attr);
 

unittests/Support/CMakeLists.txt

@@ -21,6 +21,7 @@ set(SupportSources
   RegexTest.cpp
   SNPrintfBufTest.cpp
   SourceErrorManagerTest.cpp
+  StackBoundsTest.cpp
   StatsAccumulatorTest.cpp
   StringSetVectorTest.cpp
   UnicodeTest.cpp

unittests/Support/StackBoundsTest.cpp

@@ -0,0 +1,159 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include "hermes/Support/OSCompat.h"
+
+#include "gtest/gtest.h"
+
+#include <thread>
+
+using namespace hermes;
+
+namespace {
+
+/// Upper bound on the stack, nullptr if currently unknown.
+const void *nativeStackHigh{nullptr};
+/// This has already taken \c nativeStackGap into account,
+/// so any stack outside [nativeStackHigh-nativeStackSize, nativeStackHigh]
+/// is overflowing.
+size_t nativeStackSize{0};
+/// Native stack remaining before assuming overflow.
+unsigned nativeStackGap = 16 * 1024;
+
+/// Slow path for \c isOverflowing.
+/// Sets \c stackLow_ \c stackHigh_.
+/// \return true if the native stack is overflowing the bounds of the
+///   current thread.
+bool isStackOverflowingSlowPath() {
+  auto [highPtr, size] = oscompat::thread_stack_bounds(nativeStackGap);
+  nativeStackHigh = (const char *)highPtr;
+  nativeStackSize = size;
+  void *sp = __builtin_frame_address(0);
+  return (uintptr_t)nativeStackHigh - (uintptr_t)sp > nativeStackSize;
+}
+
+/// \return true if the native stack is overflowing the bounds of the
+///   current thread. Updates the stack bounds if the thread which Runtime
+///   is executing on changes.
+inline bool isOverflowing() {
+  void *sp = __builtin_frame_address(0);
+  // Check for overflow by subtracting the sp from the high pointer.
+  // If the sp is outside the valid stack range, the difference will
+  // be greater than the known stack size.
+  // This is clearly true when 0 < sp < nativeStackHigh_ - size.
+  // If nativeStackHigh_ < sp, then the subtraction will wrap around.
+  // We know that nativeStackSize_ <= nativeStackHigh_
+  // (because otherwise the stack wouldn't fit in the memory),
+  // so the overflowed difference will be greater than nativeStackSize_.
+  if (!((uintptr_t)nativeStackHigh - (uintptr_t)sp > nativeStackSize)) {
+    // Fast path: quickly check the stored stack bounds.
+    // NOTE: It is possible to have a false negative here (highly unlikely).
+    // If the program creates many threads and destroys them, a new
+    // thread's stack could overlap the saved stack so we'd be checking
+    // against the wrong bounds.
+    return false;
+  }
+  // Slow path: might be overflowing, but update the stack bounds first
+  // in case execution has changed threads.
+  return isStackOverflowingSlowPath();
+}
+
+/// Helper variable for recursiveCall().
+volatile unsigned recCount = 0;
+volatile char *volatile dataPtr;
+
+/// A recursive call that reliably stops when it is about to overflow the stack.
+unsigned recursiveCall() {
+  volatile char data[2000];
+  dataPtr = data;
+
+  ++recCount;
+  if (isOverflowing()) {
+    printf("Stack overflow, count=%u\n", recCount);
+    return recCount;
+  }
+
+  // Prevent a tall call.
+  return recursiveCall() + (--recCount);
+}
+
+void unboundedRecursion() {
+  if (!oscompat::thread_stack_bounds(0).first) // Unsupported on this platform
+    return;
+  recursiveCall();
+}
+
+volatile unsigned sum = 0;
+
+/// Ensure that we can read from the entire extent of the reported stack
+void manualStackScan() {
+  auto [high, size] = oscompat::thread_stack_bounds(0);
+  if (!high) // Unsupported on this platform
+    return;
+  const char *low = (const char *)high - size;
+  for (const char *p = (const char *)high - 16; p > low; p -= 2048) {
+    volatile char x = *((volatile const char *)p);
+    sum += x;
+  }
+}
+
+TEST(StackBoundsTest, manualStackScan_mainThread) {
+  manualStackScan();
+}
+TEST(StackBoundsTest, unboundRecursion_mainThread) {
+  unboundedRecursion();
+}
+TEST(StackBoundsTest, manualStackScan_thread) {
+  std::thread t(manualStackScan);
+  t.join();
+}
+TEST(StackBoundsTest, unboundRecursion_thread) {
+  std::thread t(unboundedRecursion);
+  t.join();
+}
+
+#if !defined(_WINDOWS) && !defined(__EMSCRIPTEN__)
+void runInThreadWith64KGuard(void *(threadFunc)(void *)) {
+  pthread_t thread;
+  pthread_attr_t attr;
+  size_t guard_size = 64 * 1024; // 64KB
+
+  // Initialize thread attributes object
+  pthread_attr_init(&attr);
+
+  // Set the guard size attribute
+  if (pthread_attr_setguardsize(&attr, guard_size) != 0) {
+    perror("Failed to set guard size");
+    exit(EXIT_FAILURE);
+  }
+
+  if (pthread_create(&thread, &attr, threadFunc, nullptr) != 0) {
+    perror("Failed to create thread");
+    exit(EXIT_FAILURE);
+  }
+
+  // Clean up
+  pthread_attr_destroy(&attr);
+  pthread_join(thread, nullptr);
+}
+
+TEST(StackBoundsTest, manualStackScan_thread64KGuard) {
+  runInThreadWith64KGuard([](void *) -> void * {
+    manualStackScan();
+    return nullptr;
+  });
+}
+TEST(StackBoundsTest, unboundRecursion_thread64KGuard) {
+  runInThreadWith64KGuard([](void *) -> void * {
+    unboundedRecursion();
+    return nullptr;
+  });
+}
+
+#endif
+
+} // namespace