Merge pull request #25509 from azat/fix-altstack

Fix alternative stack for SIGSEGV handling

base/glibc-compatibility/glibc-compatibility.c

@@ -8,13 +8,6 @@
 extern "C" {
 #endif
 
-#include <pthread.h>
-
-size_t __pthread_get_minstack(const pthread_attr_t * attr)
-{
-    return 1048576;        /// This is a guess. Don't sure it is correct.
-}
-
 #include <signal.h>
 #include <unistd.h>
 #include <string.h>
@@ -141,6 +134,8 @@ int __open_2(const char *path, int oflag)
 }
 
 
+#include <pthread.h>
+
 /// No-ops.
 int pthread_setname_np(pthread_t thread, const char *name) { return 0; }
 int pthread_getname_np(pthread_t thread, char *name, size_t len) { name[0] = '\0'; return 0; };

src/Common/ThreadStatus.cpp

@@ -7,8 +7,9 @@
 
 #include <Poco/Logger.h>
 #include <common/getThreadId.h>
+#include <common/getPageSize.h>
 
-#include <signal.h>
+#include <csignal>
 
 
 namespace DB
@@ -25,8 +26,48 @@ thread_local ThreadStatus * current_thread = nullptr;
 thread_local ThreadStatus * main_thread = nullptr;
 
 #if !defined(SANITIZER) && !defined(ARCADIA_BUILD)
-    alignas(4096) static thread_local char alt_stack[std::max<size_t>(MINSIGSTKSZ, 4096)];
-    static thread_local bool has_alt_stack = false;
+namespace
+{
+
+/// Alternative stack for signal handling.
+///
+/// This stack should not be located in the TLS (thread local storage), since:
+/// - TLS locates data on the per-thread stack
+/// - And in case of stack in the signal handler will grow too much,
+///   it will start overwriting TLS storage
+///   (and note, that it is not too small, due to StackTrace obtaining)
+/// - Plus there is no way to determine TLS block size, yes there is
+///   __pthread_get_minstack() in glibc, but it is private and hence not portable.
+///
+/// Also we should not use getStackSize() (pthread_attr_getstack()) since it
+/// will return 8MB, and this is too huge for signal stack.
+struct ThreadStack
+{
+    ThreadStack()
+        : data(aligned_alloc(getPageSize(), size))
+    {
+        /// Add a guard page
+        /// (and since the stack grows downward, we need to protect the first page).
+        mprotect(data, getPageSize(), PROT_NONE);
+    }
+    ~ThreadStack()
+    {
+        mprotect(data, getPageSize(), PROT_WRITE|PROT_READ);
+        free(data);
+    }
+
+    static size_t getSize() { return size; }
+    void * getData() const { return data; }
+
+private:
+    static constexpr size_t size = 16 << 10; /// 16 KiB - not too big but enough to handle error.
+    void * data;
+};
+
+}
+
+static thread_local ThreadStack alt_stack;
+static thread_local bool has_alt_stack = false;
 #endif
 
 
@@ -54,9 +95,9 @@ ThreadStatus::ThreadStatus()
 
         /// We have to call 'sigaltstack' before first 'sigaction'. (It does not work other way, for unknown reason).
         stack_t altstack_description{};
-        altstack_description.ss_sp = alt_stack;
+        altstack_description.ss_sp = alt_stack.getData();
         altstack_description.ss_flags = 0;
-        altstack_description.ss_size = sizeof(alt_stack);
+        altstack_description.ss_size = alt_stack.getSize();
 
         if (0 != sigaltstack(&altstack_description, nullptr))
         {

src/Common/checkStackSize.cpp

@@ -22,6 +22,52 @@ namespace DB
 static thread_local void * stack_address = nullptr;
 static thread_local size_t max_stack_size = 0;
 
+/**
+ * @param out_address - if not nullptr, here the address of the stack will be written.
+ * @return stack size
+ */
+size_t getStackSize(void ** out_address)
+{
+    using namespace DB;
+
+    size_t size;
+    void * address;
+
+#if defined(OS_DARWIN)
+    // pthread_get_stacksize_np() returns a value too low for the main thread on
+    // OSX 10.9, http://mail.openjdk.java.net/pipermail/hotspot-dev/2013-October/011369.html
+    //
+    // Multiple workarounds possible, adopt the one made by https://github.com/robovm/robovm/issues/274
+    // https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/Multithreading/CreatingThreads/CreatingThreads.html
+    // Stack size for the main thread is 8MB on OSX excluding the guard page size.
+    pthread_t thread = pthread_self();
+    size = pthread_main_np() ? (8 * 1024 * 1024) : pthread_get_stacksize_np(thread);
+
+    // stack address points to the start of the stack, not the end how it's returned by pthread_get_stackaddr_np
+    address = reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(pthread_get_stackaddr_np(thread)) - max_stack_size);
+#else
+    pthread_attr_t attr;
+#   if defined(__FreeBSD__) || defined(OS_SUNOS)
+    pthread_attr_init(&attr);
+    if (0 != pthread_attr_get_np(pthread_self(), &attr))
+        throwFromErrno("Cannot pthread_attr_get_np", ErrorCodes::CANNOT_PTHREAD_ATTR);
+#   else
+    if (0 != pthread_getattr_np(pthread_self(), &attr))
+        throwFromErrno("Cannot pthread_getattr_np", ErrorCodes::CANNOT_PTHREAD_ATTR);
+#   endif
+
+    SCOPE_EXIT({ pthread_attr_destroy(&attr); });
+
+    if (0 != pthread_attr_getstack(&attr, &address, &size))
+        throwFromErrno("Cannot pthread_getattr_np", ErrorCodes::CANNOT_PTHREAD_ATTR);
+#endif // OS_DARWIN
+
+    if (out_address)
+        *out_address = address;
+
+    return size;
+}
+
 /** It works fine when interpreters are instantiated by ClickHouse code in properly prepared threads,
   *  but there are cases when ClickHouse runs as a library inside another application.
   * If application is using user-space lightweight threads with manually allocated stacks,
@@ -34,36 +80,7 @@ __attribute__((__weak__)) void checkStackSize()
     using namespace DB;
 
     if (!stack_address)
-    {
-#if defined(OS_DARWIN)
-        // pthread_get_stacksize_np() returns a value too low for the main thread on
-        // OSX 10.9, http://mail.openjdk.java.net/pipermail/hotspot-dev/2013-October/011369.html
-        //
-        // Multiple workarounds possible, adopt the one made by https://github.com/robovm/robovm/issues/274
-        // https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/Multithreading/CreatingThreads/CreatingThreads.html
-        // Stack size for the main thread is 8MB on OSX excluding the guard page size.
-        pthread_t thread = pthread_self();
-        max_stack_size = pthread_main_np() ? (8 * 1024 * 1024) : pthread_get_stacksize_np(thread);
-
-        // stack_address points to the start of the stack, not the end how it's returned by pthread_get_stackaddr_np
-        stack_address = reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(pthread_get_stackaddr_np(thread)) - max_stack_size);
-#else
-        pthread_attr_t attr;
-#   if defined(__FreeBSD__) || defined(OS_SUNOS)
-        pthread_attr_init(&attr);
-        if (0 != pthread_attr_get_np(pthread_self(), &attr))
-            throwFromErrno("Cannot pthread_attr_get_np", ErrorCodes::CANNOT_PTHREAD_ATTR);
-#   else
-        if (0 != pthread_getattr_np(pthread_self(), &attr))
-            throwFromErrno("Cannot pthread_getattr_np", ErrorCodes::CANNOT_PTHREAD_ATTR);
-#   endif
-
-        SCOPE_EXIT({ pthread_attr_destroy(&attr); });
-
-        if (0 != pthread_attr_getstack(&attr, &stack_address, &max_stack_size))
-            throwFromErrno("Cannot pthread_getattr_np", ErrorCodes::CANNOT_PTHREAD_ATTR);
-#endif // OS_DARWIN
-    }
+        max_stack_size = getStackSize(&stack_address);
 
     const void * frame_address = __builtin_frame_address(0);
     uintptr_t int_frame_address = reinterpret_cast<uintptr_t>(frame_address);