From 4fea7b8bb1344787df311661f56ef0ae40404336 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 28 Jan 2020 14:47:40 +0000 Subject: [PATCH] Make Pals only return amount of memory requested The PAL API previously allowed for returning more memory than asked for. This was when the PAL performed the alignment work, now this is done in large alloc, so removing from the PAL. --- README.md | 4 ++-- src/mem/largealloc.h | 21 +++++++-------------- src/pal/pal_apple.h | 4 ++-- src/pal/pal_bsd_aligned.h | 4 ++-- src/pal/pal_freebsd_kernel.h | 4 ++-- src/pal/pal_open_enclave.h | 11 ++++------- src/pal/pal_posix.h | 4 ++-- src/pal/pal_windows.h | 8 ++++---- src/test/func/fixed_region/fixed_region.cc | 2 +- src/test/func/two_alloc_types/main.cc | 2 +- 10 files changed, 27 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 5566ab7..8b39dc9 100644 --- a/README.md +++ b/README.md @@ -163,9 +163,9 @@ pages, rather than zeroing them synchronously in this call ```c++ template -void* reserve(size_t* size, size_t align); +void* reserve(size_t size, size_t align); template -void* reserve(const size_t* size) noexcept; +void* reserve(size_t size) noexcept; ``` Only one of these needs to be implemented, depending on whether the underlying system can provide strongly aligned memory regions. diff --git a/src/mem/largealloc.h b/src/mem/largealloc.h index a5274fb..7ccfd1b 100644 --- a/src/mem/largealloc.h +++ b/src/mem/largealloc.h @@ -63,16 +63,11 @@ namespace snmalloc void new_block() { - size_t size = SUPERSLAB_SIZE; - void* r = reserve(&size, SUPERSLAB_SIZE); - - if (size < SUPERSLAB_SIZE) - error("out of memory"); - + void* r = reserve(SUPERSLAB_SIZE, SUPERSLAB_SIZE); PAL::template notify_using(r, OS_PAGE_SIZE); bump = r; - remaining = size; + remaining = SUPERSLAB_SIZE; } /** @@ -202,7 +197,7 @@ namespace snmalloc } template - void* reserve(size_t* size, size_t align) noexcept + void* reserve(size_t size, size_t align) noexcept { if constexpr (pal_supports) { @@ -210,22 +205,20 @@ namespace snmalloc } else { - size_t request = *size; + size_t request = size; // Add align, so we can guarantee to provide at least size. request += align; // Alignment must be a power of 2. assert(align == bits::next_pow2(align)); - void* p = PAL::template reserve(&request); + void* p = PAL::template reserve(request); - *size = request; auto p0 = address_cast(p); auto start = bits::align_up(p0, align); if (start > p0) { uintptr_t end = bits::align_down(p0 + request, align); - *size = end - start; PAL::notify_not_using(p, start - p0); PAL::notify_not_using(pointer_cast(end), (p0 + request) - end); p = pointer_cast(start); @@ -302,8 +295,7 @@ namespace snmalloc if (p == nullptr) { - size_t add = rsize; - p = memory_provider.template reserve(&add, rsize); + p = memory_provider.template reserve(rsize, rsize); memory_provider.template notify_using(p, size); } else @@ -340,6 +332,7 @@ namespace snmalloc } } + assert(p == pointer_align_up(p, rsize)); return p; } diff --git a/src/pal/pal_apple.h b/src/pal/pal_apple.h index 87c3eb8..2e7554f 100644 --- a/src/pal/pal_apple.h +++ b/src/pal/pal_apple.h @@ -56,11 +56,11 @@ namespace snmalloc * See comment below. */ template - void* reserve(const size_t* size) + void* reserve(size_t size) { void* p = mmap( nullptr, - *size, + size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, pal_anon_id, diff --git a/src/pal/pal_bsd_aligned.h b/src/pal/pal_bsd_aligned.h index 61abcfb..17e97ab 100644 --- a/src/pal/pal_bsd_aligned.h +++ b/src/pal/pal_bsd_aligned.h @@ -27,7 +27,7 @@ namespace snmalloc * Reserve memory at a specific alignment. */ template - void* reserve(const size_t* size, size_t align) noexcept + void* reserve(size_t size, size_t align) noexcept { // Alignment must be a power of 2. assert(align == bits::next_pow2(align)); @@ -38,7 +38,7 @@ namespace snmalloc void* p = mmap( nullptr, - *size, + size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_ALIGNED(log2align), -1, diff --git a/src/pal/pal_freebsd_kernel.h b/src/pal/pal_freebsd_kernel.h index 3af8d14..c09fc30 100644 --- a/src/pal/pal_freebsd_kernel.h +++ b/src/pal/pal_freebsd_kernel.h @@ -61,9 +61,9 @@ namespace snmalloc } template - void* reserve(size_t* size, size_t align) + void* reserve(size_t size, size_t align) { - size_t request = *size; + size_t request = size; vm_offset_t addr; if (vmem_xalloc( kernel_arena, diff --git a/src/pal/pal_open_enclave.h b/src/pal/pal_open_enclave.h index 9af5235..0361d53 100644 --- a/src/pal/pal_open_enclave.h +++ b/src/pal/pal_open_enclave.h @@ -18,7 +18,7 @@ namespace snmalloc * Bitmap of PalFeatures flags indicating the optional features that this * PAL supports. */ - static constexpr uint64_t pal_features = AlignedAllocation; + static constexpr uint64_t pal_features = 0; static void error(const char* const str) { UNUSED(str); @@ -26,7 +26,7 @@ namespace snmalloc } template - void* reserve(size_t* size, size_t align) noexcept + void* reserve(size_t size) noexcept { if (oe_base == 0) { @@ -36,21 +36,18 @@ namespace snmalloc } void* old_base = oe_base; - void* old_base2 = old_base; void* next_base; auto end = __oe_get_heap_end(); do { - old_base2 = old_base; - auto new_base = pointer_align_up(old_base, align); - next_base = pointer_offset(new_base, *size); + auto new_base = old_base; + next_base = pointer_offset(new_base, size); if (next_base > end) error("Out of memory"); } while (oe_base.compare_exchange_strong(old_base, next_base)); - *size = pointer_diff(old_base2, next_base); return old_base; } diff --git a/src/pal/pal_posix.h b/src/pal/pal_posix.h index b484cf5..7c5b6b2 100644 --- a/src/pal/pal_posix.h +++ b/src/pal/pal_posix.h @@ -121,11 +121,11 @@ namespace snmalloc * greater than a page. */ template - void* reserve(const size_t* size) noexcept + void* reserve(size_t size) noexcept { void* p = mmap( nullptr, - *size, + size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, diff --git a/src/pal/pal_windows.h b/src/pal/pal_windows.h index 2a98e49..75f87d1 100644 --- a/src/pal/pal_windows.h +++ b/src/pal/pal_windows.h @@ -183,7 +183,7 @@ namespace snmalloc } # elif defined(PLATFORM_HAS_VIRTUALALLOC2) template - void* reserve(size_t* size, size_t align) noexcept + void* reserve(size_t size, size_t align) noexcept { DWORD flags = MEM_RESERVE; @@ -209,7 +209,7 @@ namespace snmalloc param.Pointer = &addressReqs; void* ret = VirtualAlloc2FromApp( - nullptr, nullptr, *size, flags, PAGE_READWRITE, ¶m, 1); + nullptr, nullptr, size, flags, PAGE_READWRITE, ¶m, 1); if (ret == nullptr) { error("Failed to allocate memory\n"); @@ -218,14 +218,14 @@ namespace snmalloc } # else template - void* reserve(size_t* size) noexcept + void* reserve(size_t size) noexcept { DWORD flags = MEM_RESERVE; if (committed) flags |= MEM_COMMIT; - void* ret = VirtualAlloc(nullptr, *size, flags, PAGE_READWRITE); + void* ret = VirtualAlloc(nullptr, size, flags, PAGE_READWRITE); if (ret == nullptr) { error("Failed to allocate memory\n"); diff --git a/src/test/func/fixed_region/fixed_region.cc b/src/test/func/fixed_region/fixed_region.cc index fe1ae39..101177e 100644 --- a/src/test/func/fixed_region/fixed_region.cc +++ b/src/test/func/fixed_region/fixed_region.cc @@ -33,7 +33,7 @@ int main() MemoryProviderStateMixin mp; size_t size = 1ULL << 28; - oe_base = mp.reserve(&size, 0); + oe_base = mp.reserve(size, 1); oe_end = (uint8_t*)oe_base + size; std::cout << "Allocated region " << oe_base << " - " << oe_end << std::endl; diff --git a/src/test/func/two_alloc_types/main.cc b/src/test/func/two_alloc_types/main.cc index b8ee2ff..ef9c1b9 100644 --- a/src/test/func/two_alloc_types/main.cc +++ b/src/test/func/two_alloc_types/main.cc @@ -46,7 +46,7 @@ int main() MemoryProviderStateMixin mp; size_t size = 1ULL << 26; - oe_base = mp.reserve(&size, 1); + oe_base = mp.reserve(size, 1); oe_end = (uint8_t*)oe_base + size; std::cout << "Allocated region " << oe_base << " - " << oe_end << std::endl;