From 39c04335fd618fd5bd2fa5a8178c38f46840d722 Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Mon, 17 Jul 2023 17:19:34 +0200 Subject: [PATCH 1/2] Free a correctly-sized stack when guard pages are disabled --- .../pika/coroutines/detail/posix_utility.hpp | 79 ++++++++++++------- 1 file changed, 52 insertions(+), 27 deletions(-) diff --git a/libs/pika/coroutines/include/pika/coroutines/detail/posix_utility.hpp b/libs/pika/coroutines/include/pika/coroutines/detail/posix_utility.hpp index be52d9c9e5..46cd462d06 100644 --- a/libs/pika/coroutines/include/pika/coroutines/detail/posix_utility.hpp +++ b/libs/pika/coroutines/include/pika/coroutines/detail/posix_utility.hpp @@ -32,6 +32,7 @@ #include #include +#include // include unist.d conditionally to check for POSIX version. Not all OSs have the // unistd header... @@ -75,9 +76,56 @@ namespace pika::threads::coroutines::detail::posix { # if defined(PIKA_HAVE_THREAD_STACK_MMAP) && defined(_POSIX_MAPPED_FILES) && _POSIX_MAPPED_FILES > 0 + inline void* to_stack_with_guard_page(void* stack) + { +# if defined(PIKA_HAVE_THREAD_GUARD_PAGE) + if (use_guard_pages) + { + void** stack = static_cast(stack) - (EXEC_PAGESIZE / sizeof(void*)); + return static_cast(stack); + } +# endif + return stack; + } + + inline void* to_stack_without_guard_page(void* stack) + { +# if defined(PIKA_HAVE_THREAD_GUARD_PAGE) + if (use_guard_pages) + { + void** stack = static_cast(stack) + (EXEC_PAGESIZE / sizeof(void*)); + return static_cast(stack); + } +# endif + return stack; + } + + inline void add_guard_page(void* stack) + { +# if defined(PIKA_HAVE_THREAD_GUARD_PAGE) + if (use_guard_pages) + { + ::mprotect(stack, EXEC_PAGESIZE, PROT_NONE); + } +# else + PIKA_UNUSED(stack); +# endif + } + + inline std::size_t stack_size_with_guard_page(std::size_t size) + { +# if defined(PIKA_HAVE_THREAD_GUARD_PAGE) + if (use_guard_pages) + { + return size + EXEC_PAGESIZE; + } +# endif + return size; + } + inline void* alloc_stack(std::size_t size) { - void* real_stack = ::mmap(nullptr, size + EXEC_PAGESIZE, PROT_READ | PROT_WRITE, + void* real_stack = ::mmap(nullptr, stack_size_with_guard_page(size), PROT_READ | PROT_WRITE, # if defined(__APPLE__) MAP_PRIVATE | MAP_ANON | MAP_NORESERVE, # elif defined(__FreeBSD__) @@ -99,19 +147,8 @@ namespace pika::threads::coroutines::detail::posix { throw std::runtime_error(error_message); } -# if defined(PIKA_HAVE_THREAD_GUARD_PAGE) - if (use_guard_pages) - { - // Add a guard page. - ::mprotect(real_stack, EXEC_PAGESIZE, PROT_NONE); - - void** stack = static_cast(real_stack) + (EXEC_PAGESIZE / sizeof(void*)); - return static_cast(stack); - } - return real_stack; -# else - return real_stack; -# endif + add_guard_page(real_stack); + return to_stack_without_guard_page(real_stack); } inline void watermark_stack(void* stack, std::size_t size) @@ -142,19 +179,7 @@ namespace pika::threads::coroutines::detail::posix { inline void free_stack(void* stack, std::size_t size) { -# if defined(PIKA_HAVE_THREAD_GUARD_PAGE) - if (use_guard_pages) - { - void** real_stack = static_cast(stack) - (EXEC_PAGESIZE / sizeof(void*)); - ::munmap(static_cast(real_stack), size + EXEC_PAGESIZE); - } - else - { - ::munmap(stack, size); - } -# else - ::munmap(stack, size); -# endif + ::munmap(to_stack_with_guard_page(stack), stack_size_with_guard_page(size)); } # else // non-mmap() From 7392b381772b76773d5cd0ccb7cf5a830c178201 Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Mon, 17 Jul 2023 17:30:46 +0200 Subject: [PATCH 2/2] Add error checking to stack munmap/mprotect/madvise calls --- .../pika/coroutines/detail/posix_utility.hpp | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/libs/pika/coroutines/include/pika/coroutines/detail/posix_utility.hpp b/libs/pika/coroutines/include/pika/coroutines/detail/posix_utility.hpp index 46cd462d06..98f75bc5bf 100644 --- a/libs/pika/coroutines/include/pika/coroutines/detail/posix_utility.hpp +++ b/libs/pika/coroutines/include/pika/coroutines/detail/posix_utility.hpp @@ -48,7 +48,9 @@ # include # include # include +# include # include +# include # if defined(_POSIX_MAPPED_FILES) && _POSIX_MAPPED_FILES > 0 # include @@ -105,7 +107,13 @@ namespace pika::threads::coroutines::detail::posix { # if defined(PIKA_HAVE_THREAD_GUARD_PAGE) if (use_guard_pages) { - ::mprotect(stack, EXEC_PAGESIZE, PROT_NONE); + int r = ::mprotect(stack, EXEC_PAGESIZE, PROT_NONE); + if (r != 0) + { + std::string error_message = "mprotect on a stack allocation failed with errno " + + std::to_string(errno) + " (" + std::strerror(errno) + ")"; + throw std::runtime_error(error_message); + } } # else PIKA_UNUSED(stack); @@ -137,13 +145,20 @@ namespace pika::threads::coroutines::detail::posix { if (real_stack == MAP_FAILED) { - char const* error_message = "mmap() failed to allocate thread stack"; +# if defined(PIKA_HAVE_THREAD_GUARD_PAGE) if (ENOMEM == errno && use_guard_pages) { - error_message = "mmap() failed to allocate thread stack due to insufficient " - "resources, increase /proc/sys/vm/max_map_count or add " - "-Ipika.stacks.use_guard_pages=0 to the command line"; + char const* error_message = + "mmap failed to allocate thread stack due to insufficient resources. " + "Increasing /proc/sys/vm/max_map_count or disabling guard pages with the " + "configuration option pika.stacks.use_guard_pages=0 may reduce memory " + "consumption."; + throw std::runtime_error(error_message); } +# endif + + std::string error_message = "mmap failed to allocate thread stack with errno " + + std::to_string(errno) + " (" + std::strerror(errno) + ")"; throw std::runtime_error(error_message); } @@ -170,7 +185,13 @@ namespace pika::threads::coroutines::detail::posix { { // We never free up the first page, as it's initialized only when the // stack is created. - ::madvise(stack, size - EXEC_PAGESIZE, MADV_DONTNEED); + int r = ::madvise(stack, size - EXEC_PAGESIZE, MADV_DONTNEED); + if (r != 0) + { + std::string error_message = "madvise on a stack allocation failed with errno " + + std::to_string(errno) + " (" + std::strerror(errno) + ")"; + throw std::runtime_error(error_message); + } return true; } @@ -179,7 +200,13 @@ namespace pika::threads::coroutines::detail::posix { inline void free_stack(void* stack, std::size_t size) { - ::munmap(to_stack_with_guard_page(stack), stack_size_with_guard_page(size)); + int r = ::munmap(to_stack_with_guard_page(stack), stack_size_with_guard_page(size)); + if (r != 0) + { + std::string error_message = "munmap failed to deallocate thread stack with errno " + + std::to_string(errno) + " (" + std::strerror(errno) + ")"; + throw std::runtime_error(error_message); + } } # else // non-mmap()