From 71e205fe17820bacc4ed98292cf740e6d19ba33f Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Wed, 9 Jul 2025 08:37:58 +0100 Subject: [PATCH] Fallible notify using (#790) On Windows, the most common way to out-of-memory is to fail to commit a page. The current design of the Pal assumes that notify_using always succeeds, thus we have to raise an error if it fails. This changes the specification of notify_using to return a bool, indicating whether the notify succeeded or not. This allows Windows to fail the notification, and then the surrounding code can handle the failure appropriately, such as by throwing an exception or returning the nullptr for the allocation. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- docs/PORTING.md | 6 ++-- src/snmalloc/backend_helpers/authmap.h | 9 +++-- src/snmalloc/backend_helpers/commitrange.h | 12 ++++++- src/snmalloc/backend_helpers/pagemap.h | 11 +++++-- .../backend_helpers/pagemapregisterrange.h | 8 ++++- src/snmalloc/ds/pagemap.h | 33 +++++++++++++++---- src/snmalloc/mem/backend_concept.h | 2 +- src/snmalloc/pal/pal_apple.h | 5 +-- src/snmalloc/pal/pal_concept.h | 4 +-- src/snmalloc/pal/pal_freebsd_kernel.h | 5 +-- src/snmalloc/pal/pal_noalloc.h | 3 +- src/snmalloc/pal/pal_plain.h | 3 +- src/snmalloc/pal/pal_posix.h | 8 +++-- src/snmalloc/pal/pal_windows.h | 10 +++--- 14 files changed, 87 insertions(+), 32 deletions(-) diff --git a/docs/PORTING.md b/docs/PORTING.md index f28a581..f40aeef 100644 --- a/docs/PORTING.md +++ b/docs/PORTING.md @@ -38,9 +38,9 @@ The state for a particular region of memory is set with static void notify_not_using(void* p, size_t size) noexcept; template -static void notify_using(void* p, size_t size) noexcept; +static bool notify_using(void* p, size_t size) noexcept; -static void notify_using_readonly(void* p, size_t size) noexcept; +static bool notify_using_readonly(void* p, size_t size) noexcept; ``` These functions notify the system that the range of memory from `p` to `p` + `size` is in the relevant state. @@ -48,6 +48,8 @@ These functions notify the system that the range of memory from `p` to `p` + If the template parameter is set to `YesZero` then `notify_using` must ensure the range is full of zeros. +The function should return `true` if the memory is now in the requested state, and `false` if it failed to make the memory useable. + ```c++ template static void zero(void* p, size_t size) noexcept; diff --git a/src/snmalloc/backend_helpers/authmap.h b/src/snmalloc/backend_helpers/authmap.h index f79ec8e..e2a0008 100644 --- a/src/snmalloc/backend_helpers/authmap.h +++ b/src/snmalloc/backend_helpers/authmap.h @@ -13,8 +13,10 @@ namespace snmalloc { static SNMALLOC_FAST_PATH void init() {} - static SNMALLOC_FAST_PATH void register_range(capptr::Arena, size_t) - {} + static SNMALLOC_FAST_PATH bool register_range(capptr::Arena, size_t) + { + return true; + } template static SNMALLOC_FAST_PATH capptr::Arena amplify(capptr::Alloc c) @@ -44,7 +46,7 @@ namespace snmalloc concreteAuthmap.template init(); } - static SNMALLOC_FAST_PATH void + static SNMALLOC_FAST_PATH bool register_range(capptr::Arena base, size_t size) { concreteAuthmap.register_range(address_cast(base), size); @@ -55,6 +57,7 @@ namespace snmalloc { concreteAuthmap.set(a, base); } + return true; } template diff --git a/src/snmalloc/backend_helpers/commitrange.h b/src/snmalloc/backend_helpers/commitrange.h index 2bbd7a5..4e83a33 100644 --- a/src/snmalloc/backend_helpers/commitrange.h +++ b/src/snmalloc/backend_helpers/commitrange.h @@ -34,7 +34,17 @@ namespace snmalloc PAL::page_size); auto range = parent.alloc_range(size); if (range != nullptr) - PAL::template notify_using(range.unsafe_ptr(), size); + { + auto result = + PAL::template notify_using(range.unsafe_ptr(), size); + if (!result) + { + // If notify_using fails, we deallocate the range and return + // nullptr. + parent.dealloc_range(range, size); + return CapPtr(nullptr); + } + } return range; } diff --git a/src/snmalloc/backend_helpers/pagemap.h b/src/snmalloc/backend_helpers/pagemap.h index a6c9090..2d6ed44 100644 --- a/src/snmalloc/backend_helpers/pagemap.h +++ b/src/snmalloc/backend_helpers/pagemap.h @@ -96,15 +96,22 @@ namespace snmalloc * Mark the MetaEntry at the bottom of the range as a boundary, preventing * consolidation with a lower range, unless CONSOLIDATE_PAL_ALLOCS. */ - static void register_range(capptr::Arena p, size_t sz) + static bool register_range(capptr::Arena p, size_t sz) { - concretePagemap.register_range(address_cast(p), sz); + auto result = concretePagemap.register_range(address_cast(p), sz); + + if (!result) + { + return false; + } + if constexpr (!CONSOLIDATE_PAL_ALLOCS) { // Mark start of allocation in pagemap. auto& entry = get_metaentry_mut(address_cast(p)); entry.set_boundary(); } + return true; } /** diff --git a/src/snmalloc/backend_helpers/pagemapregisterrange.h b/src/snmalloc/backend_helpers/pagemapregisterrange.h index 1a96cb0..b4b2a2d 100644 --- a/src/snmalloc/backend_helpers/pagemapregisterrange.h +++ b/src/snmalloc/backend_helpers/pagemapregisterrange.h @@ -30,7 +30,13 @@ namespace snmalloc if (base != nullptr) { - Pagemap::register_range(base, size); + auto result = Pagemap::register_range(base, size); + if (!result) + { + // If register_range fails, typically there is no recovery from this + // so just return nullptr. + return CapPtr(nullptr); + } } return base; diff --git a/src/snmalloc/ds/pagemap.h b/src/snmalloc/ds/pagemap.h index 257733c..2ee3cdd 100644 --- a/src/snmalloc/ds/pagemap.h +++ b/src/snmalloc/ds/pagemap.h @@ -55,7 +55,7 @@ namespace snmalloc /** * Ensure this range of pagemap is accessible */ - void register_range(address_t p, size_t length) + bool register_range(address_t p, size_t length) { SNMALLOC_ASSERT(is_initialised()); @@ -76,11 +76,18 @@ namespace snmalloc auto page_start = pointer_align_down(first); auto page_end = pointer_align_up(last); size_t using_size = pointer_diff(page_start, page_end); - PAL::template notify_using(page_start, using_size); + auto result = PAL::template notify_using(page_start, using_size); + if (!result) + { + // If notify_using fails, fails this call. + return false; + } if constexpr (pal_supports) { PAL::notify_do_dump(page_start, using_size); } + + return true; } constexpr FlatPagemap() = default; @@ -234,22 +241,35 @@ namespace snmalloc // Only commit readonly memory for this range, if the platform // supports lazy commit. Otherwise, this would be a lot of memory to // have mapped. - PAL::notify_using_readonly( + auto result = PAL::notify_using_readonly( start_page, pointer_diff(start_page, end_page)); + if (!result) + { + PAL::error("Failed to initialise snmalloc."); + } } } else { if constexpr (pal_supports) { - PAL::notify_using_readonly(new_body_untyped, REQUIRED_SIZE); + auto result = + PAL::notify_using_readonly(new_body_untyped, REQUIRED_SIZE); + if (!result) + { + PAL::error("Failed to initialise snmalloc."); + } } new_body = static_cast(new_body_untyped); } // Ensure bottom page is committed // ASSUME: new memory is zeroed. - PAL::template notify_using( + auto result = PAL::template notify_using( pointer_align_down(new_body), OS_PAGE_SIZE); + if (!result) + { + PAL::error("Failed to initialise snmalloc."); + } // Set up zero page new_body[0] = body[0]; @@ -364,7 +384,8 @@ namespace snmalloc /** * Return the starting address corresponding to a given entry within the - * Pagemap. Also checks that the reference actually points to a valid entry. + * Pagemap. Also checks that the reference actually points to a valid + * entry. */ [[nodiscard]] address_t get_address(const T& t) const { diff --git a/src/snmalloc/mem/backend_concept.h b/src/snmalloc/mem/backend_concept.h index 38ef9ae..9e0a21e 100644 --- a/src/snmalloc/mem/backend_concept.h +++ b/src/snmalloc/mem/backend_concept.h @@ -61,7 +61,7 @@ namespace snmalloc concept IsPagemapWithRegister = requires(capptr::Arena p, size_t sz) { { Pagemap::register_range(p, sz) - } -> ConceptSame; + } -> ConceptSame; }; /** diff --git a/src/snmalloc/pal/pal_apple.h b/src/snmalloc/pal/pal_apple.h index 60e4cff..7e800f7 100644 --- a/src/snmalloc/pal/pal_apple.h +++ b/src/snmalloc/pal/pal_apple.h @@ -189,7 +189,7 @@ namespace snmalloc * */ template - static void notify_using(void* p, size_t size) noexcept + static bool notify_using(void* p, size_t size) noexcept { KeepErrno e; SNMALLOC_ASSERT( @@ -207,7 +207,7 @@ namespace snmalloc if (SNMALLOC_LIKELY(r != MAP_FAILED)) { - return; + return true; } } @@ -232,6 +232,7 @@ namespace snmalloc { ::bzero(p, size); } + return true; } // Apple's `mmap` doesn't support user-specified alignment and only diff --git a/src/snmalloc/pal/pal_concept.h b/src/snmalloc/pal/pal_concept.h index db45813..be5678d 100644 --- a/src/snmalloc/pal/pal_concept.h +++ b/src/snmalloc/pal/pal_concept.h @@ -53,10 +53,10 @@ namespace snmalloc { PAL::template notify_using(vp, sz) - } noexcept -> ConceptSame; + } noexcept -> ConceptSame; { PAL::template notify_using(vp, sz) - } noexcept -> ConceptSame; + } noexcept -> ConceptSame; { PAL::template zero(vp, sz) diff --git a/src/snmalloc/pal/pal_freebsd_kernel.h b/src/snmalloc/pal/pal_freebsd_kernel.h index 11b951c..444ff89 100644 --- a/src/snmalloc/pal/pal_freebsd_kernel.h +++ b/src/snmalloc/pal/pal_freebsd_kernel.h @@ -51,14 +51,15 @@ namespace snmalloc /// Notify platform that we will be using these pages template - static void notify_using(void* p, size_t size) + static bool notify_using(void* p, size_t size) { vm_offset_t addr = get_vm_offset(p); int flags = M_WAITOK | ((zero_mem == YesZero) ? M_ZERO : 0); if (kmem_back(kernel_object, addr, size, flags) != KERN_SUCCESS) { - error("Out of memory"); + return false; } + return true; } /// OS specific function for zeroing memory diff --git a/src/snmalloc/pal/pal_noalloc.h b/src/snmalloc/pal/pal_noalloc.h index 590aefe..b6aae78 100644 --- a/src/snmalloc/pal/pal_noalloc.h +++ b/src/snmalloc/pal/pal_noalloc.h @@ -77,7 +77,7 @@ namespace snmalloc * This is a no-op in this stub, except for zeroing memory if required. */ template - static void notify_using(void* p, size_t size) noexcept + static bool notify_using(void* p, size_t size) noexcept { if constexpr (zero_mem == YesZero) { @@ -87,6 +87,7 @@ namespace snmalloc { UNUSED(p, size); } + return true; } /** diff --git a/src/snmalloc/pal/pal_plain.h b/src/snmalloc/pal/pal_plain.h index 005ef98..ff562af 100644 --- a/src/snmalloc/pal/pal_plain.h +++ b/src/snmalloc/pal/pal_plain.h @@ -16,7 +16,7 @@ namespace snmalloc // Notify platform that we will not be using these pages template - static void notify_using(void* p, size_t size) noexcept + static bool notify_using(void* p, size_t size) noexcept { if constexpr (zero_mem == YesZero) { @@ -26,6 +26,7 @@ namespace snmalloc { UNUSED(p, size); } + return true; } }; } // namespace snmalloc diff --git a/src/snmalloc/pal/pal_posix.h b/src/snmalloc/pal/pal_posix.h index 610bbe2..8feab7a 100644 --- a/src/snmalloc/pal/pal_posix.h +++ b/src/snmalloc/pal/pal_posix.h @@ -235,7 +235,7 @@ namespace snmalloc * function, or we have initially mapped the pages as PROT_NONE. */ template - static void notify_using(void* p, size_t size) noexcept + static bool notify_using(void* p, size_t size) noexcept { SNMALLOC_ASSERT( is_aligned_block(p, size) || (zero_mem == NoZero)); @@ -249,6 +249,8 @@ namespace snmalloc if constexpr (zero_mem == YesZero) zero(p, size); + + return true; } /** @@ -257,7 +259,7 @@ namespace snmalloc * On POSIX platforms, lazy commit means that this is a no-op, unless * we have initially mapped the pages as PROT_NONE. */ - static void notify_using_readonly(void* p, size_t size) noexcept + static bool notify_using_readonly(void* p, size_t size) noexcept { SNMALLOC_ASSERT(is_aligned_block(p, size)); @@ -267,6 +269,8 @@ namespace snmalloc { UNUSED(p, size); } + + return true; } /** diff --git a/src/snmalloc/pal/pal_windows.h b/src/snmalloc/pal/pal_windows.h index 47c233a..15dfaf2 100644 --- a/src/snmalloc/pal/pal_windows.h +++ b/src/snmalloc/pal/pal_windows.h @@ -242,19 +242,17 @@ namespace snmalloc /// Notify platform that we will be using these pages template - static void notify_using(void* p, size_t size) noexcept + static bool notify_using(void* p, size_t size) noexcept { SNMALLOC_ASSERT( is_aligned_block(p, size) || (zero_mem == NoZero)); void* r = VirtualAlloc(p, size, MEM_COMMIT, PAGE_READWRITE); - if (r == nullptr) - report_fatal_error( - "out of memory: {} ({}) could not be committed", p, size); + return r != nullptr; } - static void notify_using_readonly(void* p, size_t size) noexcept + static bool notify_using_readonly(void* p, size_t size) noexcept { initialise_readonly_av(); @@ -266,7 +264,7 @@ namespace snmalloc { r.first = (address_t)p; r.second = size; - return; + return true; } } }