diff --git a/src/backend/address_space.h b/src/backend/address_space.h index 77f45cb..d34b61e 100644 --- a/src/backend/address_space.h +++ b/src/backend/address_space.h @@ -42,8 +42,8 @@ namespace snmalloc * part of satisfying the request will be registered with the provided * arena_map for use in subsequent amplification. */ - template - CapPtr reserve(size_t size) + template + CapPtr reserve(size_t size, Pagemap& pagemap) { #ifdef SNMALLOC_TRACING std::cout << "ASM reserve request:" << size << std::endl; @@ -51,108 +51,88 @@ namespace snmalloc SNMALLOC_ASSERT(bits::is_pow2(size)); SNMALLOC_ASSERT(size >= sizeof(void*)); - if constexpr ((align == false) && !pal_supports) + /* + * For sufficiently large allocations with platforms that support + * aligned allocations and architectures that don't require + * StrictProvenance, try asking the platform first. + */ + if constexpr ( + pal_supports && !aal_supports) { - if constexpr (pal_supports) + if (size >= PAL::minimum_alloc_size) { - // TODO wasting size here. - size = bits::max(size, PAL::minimum_alloc_size); - return CapPtr( + auto base = CapPtr( PAL::template reserve_aligned(size)); - } - else - { - auto [block, size2] = PAL::reserve_at_least(size); - // TODO wasting size here. - UNUSED(size2); -#ifdef SNMALLOC_TRACING - std::cout << "Unaligned alloc here:" << block << " (" << size2 << ")" - << std::endl; -#endif - return CapPtr(block); + pagemap.register_range(address_cast(base), size); + return base; } } - else + + CapPtr res; { - /* - * For sufficiently large allocations with platforms that support - * aligned allocations and architectures that don't require - * StrictProvenance, try asking the platform first. - */ - if constexpr ( - pal_supports && - !aal_supports) + FlagLock lock(spin_lock); + res = core.template reserve(size); + if (res == nullptr) { - if (size >= PAL::minimum_alloc_size) - return CapPtr( - PAL::template reserve_aligned(size)); - } - - CapPtr res; - { - FlagLock lock(spin_lock); - res = core.template reserve(size); - if (res == nullptr) + // Allocation failed ask OS for more memory + CapPtr block = nullptr; + size_t block_size = 0; + if constexpr (pal_supports) { - // Allocation failed ask OS for more memory - CapPtr block = nullptr; - size_t block_size = 0; - if constexpr (pal_supports) - { - /* - * We will have handled the case where size >= - * minimum_alloc_size above, so we are left to handle only small - * things here. - */ - block_size = PAL::minimum_alloc_size; + /* + * We will have handled the case where size >= + * minimum_alloc_size above, so we are left to handle only small + * things here. + */ + block_size = PAL::minimum_alloc_size; - void* block_raw = - PAL::template reserve_aligned(block_size); + void* block_raw = PAL::template reserve_aligned(block_size); - // It's a bit of a lie to convert without applying bounds, but the - // platform will have bounded block for us and it's better that - // the rest of our internals expect CBChunk bounds. - block = CapPtr(block_raw); - } - else if constexpr (!pal_supports) - { - // Need at least 2 times the space to guarantee alignment. - // Hold lock here as a race could cause additional requests to - // the PAL, and this could lead to suprious OOM. This is - // particularly bad if the PAL gives all the memory on first call. - auto block_and_size = PAL::reserve_at_least(size * 2); - block = CapPtr(block_and_size.first); - block_size = block_and_size.second; - - // Ensure block is pointer aligned. - if ( - pointer_align_up(block, sizeof(void*)) != block || - bits::align_up(block_size, sizeof(void*)) > block_size) - { - auto diff = - pointer_diff(block, pointer_align_up(block, sizeof(void*))); - block_size = block_size - diff; - block_size = bits::align_down(block_size, sizeof(void*)); - } - } - if (block == nullptr) - { - return nullptr; - } - - core.template add_range(block, block_size); - - // still holding lock so guaranteed to succeed. - res = core.template reserve(size); + // It's a bit of a lie to convert without applying bounds, but the + // platform will have bounded block for us and it's better that + // the rest of our internals expect CBChunk bounds. + block = CapPtr(block_raw); } + else if constexpr (!pal_supports) + { + // Need at least 2 times the space to guarantee alignment. + // Hold lock here as a race could cause additional requests to + // the PAL, and this could lead to suprious OOM. This is + // particularly bad if the PAL gives all the memory on first call. + auto block_and_size = PAL::reserve_at_least(size * 2); + block = CapPtr(block_and_size.first); + block_size = block_and_size.second; + + // Ensure block is pointer aligned. + if ( + pointer_align_up(block, sizeof(void*)) != block || + bits::align_up(block_size, sizeof(void*)) > block_size) + { + auto diff = + pointer_diff(block, pointer_align_up(block, sizeof(void*))); + block_size = block_size - diff; + block_size = bits::align_down(block_size, sizeof(void*)); + } + } + if (block == nullptr) + { + return nullptr; + } + + pagemap.register_range(address_cast(block), block_size); + + core.template add_range(block, block_size); + + // still holding lock so guaranteed to succeed. + res = core.template reserve(size); } - - // Don't need lock while committing pages. - if constexpr (committed) - core.template commit_block(res, size); - - return res; } + + // Don't need lock while committing pages. + if constexpr (committed) + core.template commit_block(res, size); + + return res; } /** @@ -162,8 +142,8 @@ namespace snmalloc * This is useful for allowing the space required for alignment to be * used, by smaller objects. */ - template - CapPtr reserve_with_left_over(size_t size) + template + CapPtr reserve_with_left_over(size_t size, Pagemap& pagemap) { SNMALLOC_ASSERT(size >= sizeof(void*)); @@ -171,7 +151,7 @@ namespace snmalloc size_t rsize = bits::next_pow2(size); - auto res = reserve(rsize); + auto res = reserve(rsize, pagemap); if (res != nullptr) { diff --git a/src/backend/backend.h b/src/backend/backend.h index c5eb247..0d4618a 100644 --- a/src/backend/backend.h +++ b/src/backend/backend.h @@ -65,14 +65,13 @@ namespace snmalloc } template - std::enable_if_t - init(CapPtr base, size_t length) + std::enable_if_t init(void* base, size_t length) { static_assert( fixed_range_ == fixed_range, "Don't set SFINAE parameter!"); auto [heap_base, heap_length] = pagemap.init(base, length); - address_space.add_range(heap_base, heap_length); + address_space.add_range(CapPtr(heap_base), heap_length); if constexpr (!fixed_range) { @@ -106,7 +105,7 @@ namespace snmalloc auto& a = h.address_space; // TODO Improve heuristics and params auto refill_size = bits::max(size, bits::one_at_bit(21)); - auto refill = a.template reserve(refill_size); + auto refill = a.template reserve(refill_size, h.pagemap); if (refill == nullptr) return nullptr; local_state->local_address_space.template add_range( @@ -122,7 +121,7 @@ namespace snmalloc else { auto& a = h.address_space; - p = a.template reserve_with_left_over(size); + p = a.template reserve_with_left_over(size, h.pagemap); } return p; @@ -183,7 +182,7 @@ namespace snmalloc a < address_cast(pointer_offset(p, size)); a += MIN_CHUNK_SIZE) { - h.pagemap.add(a, t); + h.pagemap.set(a, t); } return {p, meta}; } diff --git a/src/backend/pagemap.h b/src/backend/pagemap.h index 06252d5..4b9d437 100644 --- a/src/backend/pagemap.h +++ b/src/backend/pagemap.h @@ -46,22 +46,23 @@ namespace snmalloc address_t base{0}; size_t size{0}; + public: /** - * Commit entry + * Ensure this range of pagemap is accessible */ - void commit_entry(void* p) + void register_range(address_t p, size_t length) { - auto entry_size = sizeof(T); - static_assert(sizeof(T) < OS_PAGE_SIZE); - // Rounding required for sub-page allocations. - auto page_start = pointer_align_down(p); - auto page_end = - pointer_align_up(pointer_offset(p, entry_size)); + // Calculate range in pagemap that is associated to this space. + auto first = &body[p >> SHIFT]; + auto last = &body[(p + length + bits::one_at_bit(SHIFT) - 1) >> SHIFT]; + + // Commit OS pages associated to the range. + 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); } - public: constexpr FlatPagemap() = default; /** @@ -70,14 +71,13 @@ namespace snmalloc * Returns usable range after pagemap has been allocated */ template - std::enable_if_t, size_t>> - init(CapPtr b, size_t s) + std::enable_if_t> + init(void* b, size_t s) { static_assert( has_bounds_ == has_bounds, "Don't set SFINAE template parameter!"); #ifdef SNMALLOC_TRACING - std::cout << "Pagemap.init " << b.unsafe_ptr() << " (" << s << ")" - << std::endl; + std::cout << "Pagemap.init " << b << " (" << s << ")" << std::endl; #endif SNMALLOC_ASSERT(s != 0); // TODO take account of pagemap size in the calculation of how big it @@ -92,7 +92,7 @@ namespace snmalloc // Put pagemap at start of range. // TODO CHERI capability bound here! - body = b.as_reinterpret().unsafe_ptr(); + body = reinterpret_cast(b); // Advance by size of pagemap. // TODO CHERI capability bound here! @@ -126,7 +126,8 @@ namespace snmalloc auto new_body = reinterpret_cast(new_body_untyped); // Ensure bottom page is committed - commit_entry(&new_body[0]); + // ASSUME: new memory is zeroed. + Pal::notify_using(new_body, OS_PAGE_SIZE); // Set up zero page new_body[0] = body[0]; @@ -166,7 +167,7 @@ namespace snmalloc // This means external pointer on Windows will be slow. if constexpr (potentially_out_of_range && !pal_supports) { - commit_entry(&body[p >> SHIFT]); + register_range(p, 1); } return body[p >> SHIFT]; @@ -188,27 +189,5 @@ namespace snmalloc body[p >> SHIFT] = t; } - - void add(address_t p, T t) - { -#ifdef SNMALLOC_TRACING - std::cout << "Pagemap.Add " << (void*)p << std::endl; -#endif - if constexpr (has_bounds) - { - if (p - base > size) - { - PAL::error("Internal error: Pagemap new write access out of range."); - } - p = p - base; - } - - // This could be the first time this page is used - // This will potentially be expensive on Windows, - // and we should revisit the performance here. - commit_entry(&body[p >> SHIFT]); - - body[p >> SHIFT] = t; - } }; } // namespace snmalloc diff --git a/src/mem/fixedglobalconfig.h b/src/mem/fixedglobalconfig.h index 4878453..01b268f 100644 --- a/src/mem/fixedglobalconfig.h +++ b/src/mem/fixedglobalconfig.h @@ -66,7 +66,7 @@ namespace snmalloc snmalloc::register_clean_up(); } - static void init(CapPtr base, size_t length) + static void init(void* base, size_t length) { get_backend_state().init(base, length); } diff --git a/src/test/func/fixed_region/fixed_region.cc b/src/test/func/fixed_region/fixed_region.cc index 702aaee..a7db57a 100644 --- a/src/test/func/fixed_region/fixed_region.cc +++ b/src/test/func/fixed_region/fixed_region.cc @@ -18,18 +18,14 @@ int main() { #ifndef SNMALLOC_PASS_THROUGH // Depends on snmalloc specific features - // Create a standard address space to get initial allocation - // this just bypasses having to understand the test platform. - AddressSpaceManager address_space; - // 28 is large enough to produce a nested allocator. // It is also large enough for the example to run in. // For 1MiB superslabs, SUPERSLAB_BITS + 4 is not big enough for the example. - size_t size = bits::one_at_bit(28); - auto oe_base = address_space.reserve(size); - auto oe_end = pointer_offset(oe_base, size).unsafe_ptr(); - std::cout << "Allocated region " << oe_base.unsafe_ptr() << " - " - << pointer_offset(oe_base, size).unsafe_ptr() << std::endl; + auto [oe_base, size] = Pal::reserve_at_least(bits::one_at_bit(28)); + Pal::notify_using(oe_base, size); + auto oe_end = pointer_offset(oe_base, size); + std::cout << "Allocated region " << oe_base << " - " + << pointer_offset(oe_base, size) << std::endl; CustomGlobals fixed_handle; CustomGlobals::init(oe_base, size); @@ -47,7 +43,7 @@ int main() if (r1 == nullptr) break; - if (oe_base.unsafe_ptr() > r1) + if (oe_base > r1) { std::cout << "Allocated: " << r1 << std::endl; abort(); diff --git a/src/test/func/pagemap/pagemap.cc b/src/test/func/pagemap/pagemap.cc index 7758b4f..dc5d5af 100644 --- a/src/test/func/pagemap/pagemap.cc +++ b/src/test/func/pagemap/pagemap.cc @@ -47,14 +47,6 @@ void check_get( } } -void add(bool bounded, address_t address, T new_value) -{ - if (bounded) - pagemap_test_bound.add(address, new_value); - else - pagemap_test_unbound.add(address, new_value); -} - void set(bool bounded, address_t address, T new_value) { if (bounded) @@ -71,38 +63,36 @@ void test_pagemap(bool bounded) address_t high = bits::one_at_bit(30); // Nullptr needs to work before initialisation - CHECK_GET(true, 0, T()); + CHECK_GET(bounded, 0, T()); // Initialise the pagemap if (bounded) { - auto size = bits::one_at_bit(30); - auto base = address_space.reserve(size); - std::cout << "Fixed base: " << base.unsafe_ptr() << " (" << size << ") " - << " end: " << pointer_offset(base, size).unsafe_ptr() - << std::endl; + auto [base, size] = Pal::reserve_at_least(bits::one_at_bit(30)); + Pal::notify_using(base, size); + std::cout << "Fixed base: " << base << " (" << size << ") " + << " end: " << pointer_offset(base, size) << std::endl; auto [heap_base, heap_size] = pagemap_test_bound.init(base, size); - std::cout << "Heap base: " << heap_base.unsafe_ptr() << " (" << heap_size - << ") " - << " end: " << pointer_offset(heap_base, heap_size).unsafe_ptr() - << std::endl; + std::cout << "Heap base: " << heap_base << " (" << heap_size << ") " + << " end: " << pointer_offset(heap_base, heap_size) << std::endl; low = address_cast(heap_base); high = low + heap_size; } else { pagemap_test_unbound.init(); + pagemap_test_unbound.register_range(low, high - low); } // Nullptr should still work after init. - CHECK_GET(true, 0, T()); + CHECK_GET(bounded, 0, T()); // Store a pattern into page map T value = 1; for (uintptr_t ptr = low; ptr < high; ptr += bits::one_at_bit(GRANULARITY_BITS + 3)) { - add(false, ptr, value); + set(bounded, ptr, value); value.v++; if (value.v == T().v) value = 0; @@ -116,7 +106,7 @@ void test_pagemap(bool bounded) for (uintptr_t ptr = low; ptr < high; ptr += bits::one_at_bit(GRANULARITY_BITS + 3)) { - CHECK_GET(false, ptr, value); + CHECK_GET(bounded, ptr, value); value.v++; if (value.v == T().v) value = 0; diff --git a/src/test/func/two_alloc_types/alloc1.cc b/src/test/func/two_alloc_types/alloc1.cc index 2c15db5..4e5f9d0 100644 --- a/src/test/func/two_alloc_types/alloc1.cc +++ b/src/test/func/two_alloc_types/alloc1.cc @@ -20,6 +20,5 @@ namespace snmalloc extern "C" void oe_allocator_init(void* base, void* end) { snmalloc::CustomGlobals fixed_handle; - fixed_handle.init( - CapPtr(base), address_cast(end) - address_cast(base)); + fixed_handle.init(base, address_cast(end) - address_cast(base)); }