Merge pull request #917 from Altinity/backports/25.3.6/79147

25.3.6 Backport of ClickHouse#79147: Remove guard pages from fibers and alternative stack for signals in threads

Author: zvonand

programs/keeper/Keeper.cpp

@@ -1,6 +1,7 @@
 #include "Keeper.h"
 
 #include <Common/ClickHouseRevision.h>
+#include <Common/formatReadable.h>
 #include <Common/getMultipleKeysFromConfig.h>
 #include <Common/DNSResolver.h>
 #include <Interpreters/DNSCacheUpdater.h>

src/Client/BuzzHouse/Generator/FuzzConfig.cpp

@@ -3,6 +3,7 @@
 #include <ranges>
 #include <IO/copyData.h>
 #include <Common/Exception.h>
+#include <Common/formatReadable.h>
 
 namespace DB
 {

src/Client/ClientBase.cpp

@@ -10,6 +10,7 @@
 #include <Core/Protocol.h>
 #include <Common/DateLUT.h>
 #include <Common/MemoryTracker.h>
+#include <Common/formatReadable.h>
 #include <Common/scope_guard_safe.h>
 #include <Common/Exception.h>
 #include <Common/ErrorCodes.h>

src/Client/Connection.cpp

@@ -23,6 +23,7 @@
 #include <Common/DNSResolver.h>
 #include <Common/StringUtils.h>
 #include <Common/OpenSSLHelpers.h>
+#include <Common/formatReadable.h>
 #include <Common/randomSeed.h>
 #include <Core/Block.h>
 #include <Core/ProtocolDefines.h>

src/Common/AsyncTaskExecutor.cpp

@@ -1,5 +1,6 @@
 #include <Common/AsyncTaskExecutor.h>
 #include <base/scope_guard.h>
+#include <fmt/format.h>
 
 
 namespace DB

src/Common/AsyncTaskExecutor.h

@@ -1,5 +1,8 @@
 #pragma once
 
+#include <atomic>
+#include <mutex>
+#include <base/types.h>
 #include <Common/Epoll.h>
 #include <Common/Fiber.h>
 #include <Common/FiberStack.h>

src/Common/FiberStack.cpp

@@ -0,0 +1,102 @@
+#include <base/defines.h>
+#include <Common/formatReadable.h>
+#include <Common/CurrentMemoryTracker.h>
+#include <Common/Exception.h>
+#include <Common/memory.h>
+#include <base/getPageSize.h>
+#include <sys/time.h>
+#include <sys/resource.h>
+#include <sys/mman.h>
+#include <Common/FiberStack.h>
+
+#if defined(BOOST_USE_VALGRIND)
+#include <valgrind/valgrind.h>
+#endif
+
+/// Required for older Darwin builds, that lack definition of MAP_ANONYMOUS
+#ifndef MAP_ANONYMOUS
+#define MAP_ANONYMOUS MAP_ANON
+#endif
+
+namespace
+{
+constexpr bool guardPagesEnabled()
+{
+#ifdef DEBUG_OR_SANITIZER_BUILD
+    return true;
+#else
+    return false;
+#endif
+}
+}
+
+namespace DB::ErrorCodes
+{
+    extern const int CANNOT_ALLOCATE_MEMORY;
+}
+
+
+FiberStack::FiberStack(size_t stack_size_)
+    : stack_size(stack_size_)
+    , page_size(getPageSize())
+{
+}
+
+boost::context::stack_context FiberStack::allocate() const
+{
+    size_t num_pages = 1 + (stack_size - 1) / page_size;
+
+    if constexpr (guardPagesEnabled())
+        /// Add one page at bottom that will be used as guard-page
+        num_pages += 1;
+
+    size_t num_bytes = num_pages * page_size;
+
+    void * data = aligned_alloc(page_size, num_bytes);
+
+    if (!data)
+        throw DB::ErrnoException(DB::ErrorCodes::CANNOT_ALLOCATE_MEMORY, "Cannot allocate FiberStack");
+
+    if constexpr (guardPagesEnabled())
+    {
+        /// TODO: make reports on illegal guard page access more clear.
+        /// Currently we will see segfault and almost random stacktrace.
+        try
+        {
+            memoryGuardInstall(data, page_size);
+        }
+        catch (...)
+        {
+            free(data);
+            throw;
+        }
+    }
+
+    auto trace = CurrentMemoryTracker::alloc(num_bytes);
+    trace.onAlloc(data, num_bytes);
+
+    boost::context::stack_context sctx;
+    sctx.size = num_bytes;
+    sctx.sp = static_cast< char * >(data) + sctx.size;
+#if defined(BOOST_USE_VALGRIND)
+    sctx.valgrind_stack_id = VALGRIND_STACK_REGISTER(sctx.sp, data);
+#endif
+    return sctx;
+}
+
+void FiberStack::deallocate(boost::context::stack_context & sctx) const
+{
+#if defined(BOOST_USE_VALGRIND)
+    VALGRIND_STACK_DEREGISTER(sctx.valgrind_stack_id);
+#endif
+    void * data = static_cast< char * >(sctx.sp) - sctx.size;
+
+    if constexpr (guardPagesEnabled())
+        memoryGuardRemove(data, page_size);
+
+    free(data);
+
+    /// Do not count guard page in memory usage.
+    auto trace = CurrentMemoryTracker::free(sctx.size);
+    trace.onFree(data, sctx.size - page_size);
+}

src/Common/FiberStack.h

@@ -1,36 +1,11 @@
 #pragma once
-#include <base/defines.h>
 #include <boost/context/stack_context.hpp>
-#include <Common/formatReadable.h>
-#include <Common/CurrentMemoryTracker.h>
-#include <Common/Exception.h>
-#include <base/getPageSize.h>
-#include <sys/time.h>
-#include <sys/resource.h>
-#include <sys/mman.h>
-
-#if defined(BOOST_USE_VALGRIND)
-#include <valgrind/valgrind.h>
-#endif
-
-/// Required for older Darwin builds, that lack definition of MAP_ANONYMOUS
-#ifndef MAP_ANONYMOUS
-#define MAP_ANONYMOUS MAP_ANON
-#endif
-
-namespace DB::ErrorCodes
-{
-    extern const int CANNOT_ALLOCATE_MEMORY;
-}
 
 /// This is an implementation of allocator for fiber stack.
 /// The reference implementation is protected_fixedsize_stack from boost::context.
 /// This implementation additionally track memory usage. It is the main reason why it is needed.
 class FiberStack
 {
-private:
-    size_t stack_size;
-    size_t page_size = 0;
 public:
     /// NOTE: If you see random segfaults in CI and stack starts from boost::context::...fiber...
     /// probably it worth to try to increase stack size for coroutines.
@@ -39,51 +14,12 @@ class FiberStack
     /// way. We will have 80 pages with 4KB page size.
     static constexpr size_t default_stack_size = 320 * 1024; /// 64KB was not enough for tests
 
-    explicit FiberStack(size_t stack_size_ = default_stack_size) : stack_size(stack_size_)
-    {
-        page_size = getPageSize();
-    }
-
-    boost::context::stack_context allocate() const
-    {
-        size_t num_pages = 1 + (stack_size - 1) / page_size;
-        size_t num_bytes = (num_pages + 1) * page_size; /// Add one page at bottom that will be used as guard-page
+    explicit FiberStack(size_t stack_size_ = default_stack_size);
 
-        void * vp = ::mmap(nullptr, num_bytes, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-        if (MAP_FAILED == vp)
-            throw DB::ErrnoException(DB::ErrorCodes::CANNOT_ALLOCATE_MEMORY, "FiberStack: Cannot mmap {}.", ReadableSize(num_bytes));
+    boost::context::stack_context allocate() const;
+    void deallocate(boost::context::stack_context & sctx) const;
 
-        /// TODO: make reports on illegal guard page access more clear.
-        /// Currently we will see segfault and almost random stacktrace.
-        if (-1 == ::mprotect(vp, page_size, PROT_NONE))
-        {
-            ::munmap(vp, num_bytes);
-            throw DB::ErrnoException(DB::ErrorCodes::CANNOT_ALLOCATE_MEMORY, "FiberStack: cannot protect guard page");
-        }
-
-        /// Do not count guard page in memory usage.
-        auto trace = CurrentMemoryTracker::alloc(num_pages * page_size);
-        trace.onAlloc(vp, num_pages * page_size);
-
-        boost::context::stack_context sctx;
-        sctx.size = num_bytes;
-        sctx.sp = static_cast< char * >(vp) + sctx.size;
-#if defined(BOOST_USE_VALGRIND)
-        sctx.valgrind_stack_id = VALGRIND_STACK_REGISTER(sctx.sp, vp);
-#endif
-        return sctx;
-    }
-
-    void deallocate(boost::context::stack_context & sctx) const
-    {
-#if defined(BOOST_USE_VALGRIND)
-        VALGRIND_STACK_DEREGISTER(sctx.valgrind_stack_id);
-#endif
-        void * vp = static_cast< char * >(sctx.sp) - sctx.size;
-        ::munmap(vp, sctx.size);
-
-        /// Do not count guard page in memory usage.
-        auto trace = CurrentMemoryTracker::free(sctx.size - page_size);
-        trace.onFree(vp, sctx.size - page_size);
-    }
+private:
+    const size_t stack_size;
+    const size_t page_size;
 };

src/Common/ThreadStatus.cpp

@@ -5,6 +5,7 @@
 #include <Common/CurrentThread.h>
 #include <Common/logger_useful.h>
 #include <Common/MemoryTrackerBlockerInThread.h>
+#include <Common/memory.h>
 #include <base/getPageSize.h>
 #include <base/errnoToString.h>
 #include <Interpreters/Context.h>
@@ -19,10 +20,24 @@ namespace DB
 {
 thread_local ThreadStatus constinit * current_thread = nullptr;
 
+namespace ErrorCodes
+{
+    extern const int CANNOT_ALLOCATE_MEMORY;
+}
+
 #if !defined(SANITIZER)
 namespace
 {
 
+constexpr bool guardPagesEnabled()
+{
+#ifdef DEBUG_OR_SANITIZER_BUILD
+    return true;
+#else
+    return false;
+#endif
+}
+
 /// For aarch64 16K is not enough (likely due to tons of registers)
 constexpr size_t UNWIND_MINSIGSTKSZ = 32 << 10;
 
@@ -41,24 +56,48 @@ constexpr size_t UNWIND_MINSIGSTKSZ = 32 << 10;
 struct ThreadStack
 {
     ThreadStack()
-        : data(aligned_alloc(getPageSize(), getSize()))
     {
-        /// Add a guard page
-        /// (and since the stack grows downward, we need to protect the first page).
-        mprotect(data, getPageSize(), PROT_NONE);
+        auto page_size = getPageSize();
+        data = aligned_alloc(page_size, getSize());
+        if (!data)
+            throw ErrnoException(ErrorCodes::CANNOT_ALLOCATE_MEMORY, "Cannot allocate ThreadStack");
+
+        if constexpr (guardPagesEnabled())
+        {
+            try
+            {
+                /// Since the stack grows downward, we need to protect the first page
+                memoryGuardInstall(data, page_size);
+            }
+            catch (...)
+            {
+                free(data);
+                throw;
+            }
+        }
     }
     ~ThreadStack()
     {
-        mprotect(data, getPageSize(), PROT_WRITE|PROT_READ);
+        if constexpr (guardPagesEnabled())
+            memoryGuardRemove(data, getPageSize());
+
         free(data);
     }
 
-    static size_t getSize() { return std::max<size_t>(UNWIND_MINSIGSTKSZ, MINSIGSTKSZ); }
+    static size_t getSize()
+    {
+        auto size = std::max<size_t>(UNWIND_MINSIGSTKSZ, MINSIGSTKSZ);
+
+        if constexpr (guardPagesEnabled())
+            size += getPageSize();
+
+        return size;
+    }
     void * getData() const { return data; }
 
 private:
     /// 16 KiB - not too big but enough to handle error.
-    void * data;
+    void * data = nullptr;
 };
 
 }

src/Common/memory.cpp

@@ -0,0 +1,62 @@
+#include <sys/mman.h>
+#include <Poco/Environment.h>
+#include <Common/Exception.h>
+#include <Common/VersionNumber.h>
+#include <Common/memory.h>
+
+#ifdef OS_LINUX
+#if !defined(MADV_GUARD_INSTALL)
+#define MADV_GUARD_INSTALL 102
+#endif
+
+#if !defined(MADV_GUARD_REMOVE)
+#define MADV_GUARD_REMOVE 103
+#endif
+
+static bool supportsGuardPages()
+{
+    DB::VersionNumber madv_guard_minimal_version(6, 13, 0);
+    DB::VersionNumber linux_version(Poco::Environment::osVersion());
+    return (linux_version >= madv_guard_minimal_version);
+}
+static bool supports_guard_pages = supportsGuardPages();
+#endif // OS_LINUX
+
+namespace DB::ErrorCodes
+{
+    extern const int SYSTEM_ERROR;
+}
+
+/// Uses MADV_GUARD_INSTALL if available, or mprotect() if not
+void memoryGuardInstall(void *addr, size_t len)
+{
+#ifdef OS_LINUX
+    if (supports_guard_pages)
+    {
+        if (madvise(addr, len, MADV_GUARD_INSTALL))
+            throw DB::ErrnoException(DB::ErrorCodes::SYSTEM_ERROR, "Cannot madvise(MADV_GUARD_INSTALL)");
+    }
+    else
+#endif
+    {
+        if (mprotect(addr, len, PROT_NONE))
+            throw DB::ErrnoException(DB::ErrorCodes::SYSTEM_ERROR, "Cannot mprotect(PROT_NONE)");
+    }
+}
+
+/// Uses MADV_GUARD_REMOVE if available, or mprotect() if not
+void memoryGuardRemove(void *addr, size_t len)
+{
+#ifdef OS_LINUX
+    if (supports_guard_pages)
+    {
+        if (madvise(addr, len, MADV_GUARD_REMOVE))
+            throw DB::ErrnoException(DB::ErrorCodes::SYSTEM_ERROR, "Cannot madvise(MADV_GUARD_INSTALL)");
+    }
+    else
+#endif
+    {
+        if (mprotect(addr, len, PROT_READ|PROT_WRITE))
+            throw DB::ErrnoException(DB::ErrorCodes::SYSTEM_ERROR, "Cannot mprotect(PROT_NONE)");
+    }
+}