From 923705e514e850ee0070cef470fca9d9c47ca3da Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Mon, 28 Sep 2020 10:08:19 +0100 Subject: [PATCH] Natural alignment for USE_MALLOC (#248) * Add concept of natural alignment to tests. snmalloc naturally aligns blocks very heavily, so that the largest power-of-two in the rounded size is the alignment. This checks that in the test, and provides a method for finding the natural alignment of a block. * Improve USE_MALLOC to provide alignment snmalloc provides a lot of alginment guarantees. This ensures that when we pass through to the system allocator we still get those alignment guarantees. The commit also fixes the tests to work with USE_MALLOC, and builds a set of unit tests for ctest to check behaviour. --- CMakeLists.txt | 17 ++++- src/ds/defines.h | 9 +++ src/mem/alloc.h | 47 +++++++++----- src/mem/external_alloc.h | 64 +++++++++++++++++++ src/mem/globalalloc.h | 4 +- src/mem/sizeclass.h | 8 +++ src/mem/threadalloc.h | 2 +- src/override/malloc.cc | 37 +++++++---- src/test/func/fixed_region/fixed_region.cc | 2 + src/test/func/malloc/malloc.cc | 22 ++++++- src/test/func/memory/memory.cc | 24 +++---- src/test/func/memory_usage/memory_usage.cc | 3 +- src/test/func/sizeclass/sizeclass.cc | 2 +- src/test/func/statistics/stats.cc | 2 + .../perf/external_pointer/externalpointer.cc | 8 ++- src/test/perf/singlethread/singlethread.cc | 6 +- 16 files changed, 203 insertions(+), 54 deletions(-) create mode 100644 src/mem/external_alloc.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 07fe14f..f353b50 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -280,7 +280,17 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) foreach(TEST_CATEGORY ${TEST_CATEGORIES}) subdirlist(TESTS ${TESTDIR}/${TEST_CATEGORY}) foreach(TEST ${TESTS}) - foreach(SUPER_SLAB_SIZE 1;16;oe) + if (WIN32 + OR (CMAKE_SYSTEM_NAME STREQUAL NetBSD) + OR (CMAKE_SYSTEM_NAME STREQUAL OpenBSD)) + # Windows does not support aligned allocation well enough + # for pass through. + # NetBSD and OpenBSD do not support malloc*size calls. + set(FLAVOURS 1;16;oe) + else() + set(FLAVOURS 1;16;oe;malloc) + endif() + foreach(SUPER_SLAB_SIZE ${FLAVOURS}) unset(SRC) aux_source_directory(${TESTDIR}/${TEST_CATEGORY}/${TEST} SRC) set(TESTNAME "${TEST_CATEGORY}-${TEST}-${SUPER_SLAB_SIZE}") @@ -289,9 +299,12 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) if (${SUPER_SLAB_SIZE} EQUAL 16) target_compile_definitions(${TESTNAME} PRIVATE SNMALLOC_USE_LARGE_CHUNKS) endif() - if (${SUPER_SLAB_SIZE} EQUAL oe) + if (${SUPER_SLAB_SIZE} STREQUAL "oe") oe_simulate(${TESTNAME}) endif() + if (${SUPER_SLAB_SIZE} STREQUAL "malloc") + target_compile_definitions(${TESTNAME} PRIVATE SNMALLOC_PASS_THROUGH) + endif() target_link_libraries(${TESTNAME} snmalloc_lib) if (${TEST} MATCHES "release-.*") message(STATUS "Adding test: ${TESTNAME} only for release configs") diff --git a/src/ds/defines.h b/src/ds/defines.h index 7b97359..7ccbdb2 100644 --- a/src/ds/defines.h +++ b/src/ds/defines.h @@ -57,6 +57,15 @@ namespace snmalloc } #endif +#define SNMALLOC_CHECK(expr) \ + { \ + if (!(expr)) \ + { \ + snmalloc::error("Check fail: " #expr " in " __FILE__ \ + " on " TOSTRING(__LINE__)); \ + } \ + } + #ifndef NDEBUG # define SNMALLOC_ASSUME(x) SNMALLOC_ASSERT(x) #else diff --git a/src/mem/alloc.h b/src/mem/alloc.h index fac0f1b..26b3ed7 100644 --- a/src/mem/alloc.h +++ b/src/mem/alloc.h @@ -10,6 +10,7 @@ #include "../test/histogram.h" #include "allocstats.h" #include "chunkmap.h" +#include "external_alloc.h" #include "largealloc.h" #include "mediumslab.h" #include "pooled.h" @@ -125,14 +126,18 @@ namespace snmalloc SNMALLOC_FAST_PATH ALLOCATOR void* alloc() { static_assert(size != 0, "Size must not be zero."); -#ifdef USE_MALLOC +#ifdef SNMALLOC_PASS_THROUGH static_assert( allow_reserve == YesReserve, "When passing to malloc, cannot require NoResereve"); - if constexpr (zero_mem == NoZero) - return malloc(size); - else - return calloc(1, size); + // snmalloc guarantees a lot of alignment, so we can depend on this + // make pass through call aligned_alloc with the alignment snmalloc + // would guarantee. + void* result = external_alloc::aligned_alloc( + natural_alignment(size), round_size(size)); + if constexpr (zero_mem == YesZero) + memset(result, 0, size); + return result; #else constexpr sizeclass_t sizeclass = size_to_sizeclass_const(size); @@ -162,14 +167,18 @@ namespace snmalloc template SNMALLOC_FAST_PATH ALLOCATOR void* alloc(size_t size) { -#ifdef USE_MALLOC +#ifdef SNMALLOC_PASS_THROUGH static_assert( allow_reserve == YesReserve, "When passing to malloc, cannot require NoResereve"); - if constexpr (zero_mem == NoZero) - return malloc(size); - else - return calloc(1, size); + // snmalloc guarantees a lot of alignment, so we can depend on this + // make pass through call aligned_alloc with the alignment snmalloc + // would guarantee. + void* result = external_alloc::aligned_alloc( + natural_alignment(size), round_size(size)); + if constexpr (zero_mem == YesZero) + memset(result, 0, size); + return result; #else // Perform the - 1 on size, so that zero wraps around and ends up on // slow path. @@ -241,9 +250,9 @@ namespace snmalloc template void dealloc(void* p) { -#ifdef USE_MALLOC +#ifdef SNMALLOC_PASS_THROUGH UNUSED(size); - return free(p); + return external_alloc::free(p); #else check_size(p, size); constexpr sizeclass_t sizeclass = size_to_sizeclass_const(size); @@ -281,9 +290,9 @@ namespace snmalloc */ SNMALLOC_FAST_PATH void dealloc(void* p, size_t size) { -#ifdef USE_MALLOC +#ifdef SNMALLOC_PASS_THROUGH UNUSED(size); - return free(p); + return external_alloc::free(p); #else SNMALLOC_ASSERT(p != nullptr); check_size(p, size); @@ -327,8 +336,8 @@ namespace snmalloc */ SNMALLOC_FAST_PATH void dealloc(void* p) { -#ifdef USE_MALLOC - return free(p); +#ifdef SNMALLOC_PASS_THROUGH + return external_alloc::free(p); #else uint8_t size = chunkmap().get(address_cast(p)); @@ -397,7 +406,7 @@ namespace snmalloc template static void* external_pointer(void* p) { -#ifdef USE_MALLOC +#ifdef SNMALLOC_PASS_THROUGH error("Unsupported"); UNUSED(p); #else @@ -463,6 +472,9 @@ namespace snmalloc public: SNMALLOC_FAST_PATH static size_t alloc_size(const void* p) { +#ifdef SNMALLOC_PASS_THROUGH + return external_alloc::malloc_usable_size(const_cast(p)); +#else // This must be called on an external pointer. size_t size = ChunkMap::get(address_cast(p)); @@ -492,6 +504,7 @@ namespace snmalloc } return alloc_size_error(); +#endif } size_t get_id() diff --git a/src/mem/external_alloc.h b/src/mem/external_alloc.h new file mode 100644 index 0000000..5bda42f --- /dev/null +++ b/src/mem/external_alloc.h @@ -0,0 +1,64 @@ +#pragma once + +#ifdef SNMALLOC_PASS_THROUGH +# include +# if defined(_WIN32) //|| defined(__APPLE__) +# error "Pass through not supported on this platform" +// The Windows aligned allocation API is not capable of supporting the +// snmalloc API Apple was not providing aligned memory in some tests. +# else +// Defines malloc_size for the platform. +# if defined(_WIN32) +namespace snmalloc::external_alloc +{ + inline size_t malloc_usable_size(void* ptr) + { + return _msize(ptr); + } +} +# elif defined(__APPLE__) +# include +namespace snmalloc::external_alloc +{ + inline size_t malloc_usable_size(void* ptr) + { + return malloc_size(ptr); + } +} +# elif defined(__linux__) +# include +namespace snmalloc::external_alloc +{ + using ::malloc_usable_size; +} +# elif defined(__sun) || defined(__HAIKU__) || defined(__NetBSD__) || \ + defined(__OpenBSD__) +namespace snmalloc::external_alloc +{ + using ::malloc_usable_size; +} +# elif defined(__FreeBSD__) +# include +namespace snmalloc::external_alloc +{ + using ::malloc_usable_size; +} +# else +# error Define malloc size macro for this platform. +# endif +namespace snmalloc::external_alloc +{ + inline void* aligned_alloc(size_t alignment, size_t size) + { + void* result; + if (posix_memalign(&result, alignment, size) != 0) + { + result = nullptr; + } + return result; + } + + using ::free; +} +# endif +#endif diff --git a/src/mem/globalalloc.h b/src/mem/globalalloc.h index dac401f..f94ad45 100644 --- a/src/mem/globalalloc.h +++ b/src/mem/globalalloc.h @@ -72,7 +72,7 @@ namespace snmalloc void cleanup_unused() { -#ifndef USE_MALLOC +#ifndef SNMALLOC_PASS_THROUGH // Call this periodically to free and coalesce memory allocated by // allocators that are not currently in use by any thread. // One atomic operation to extract the stack, another to restore it. @@ -102,7 +102,7 @@ namespace snmalloc */ void debug_check_empty(bool* result = nullptr) { -#ifndef USE_MALLOC +#ifndef SNMALLOC_PASS_THROUGH // This is a debugging function. It checks that all memory from all // allocators has been freed. auto* alloc = Parent::iterate(); diff --git a/src/mem/sizeclass.h b/src/mem/sizeclass.h index 801192d..5321aaf 100644 --- a/src/mem/sizeclass.h +++ b/src/mem/sizeclass.h @@ -198,4 +198,12 @@ namespace snmalloc } return sizeclass_to_size(size_to_sizeclass(size)); } + + /// Returns the alignment that this size naturally has, that is + /// all allocations of size `size` will be aligned to the returned value. + SNMALLOC_FAST_PATH static size_t natural_alignment(size_t size) + { + auto rsize = round_size(size); + return bits::one_at_bit(bits::ctz(rsize)); + } } // namespace snmalloc diff --git a/src/mem/threadalloc.h b/src/mem/threadalloc.h index ba8d2fa..306c782 100644 --- a/src/mem/threadalloc.h +++ b/src/mem/threadalloc.h @@ -189,7 +189,7 @@ namespace snmalloc */ static SNMALLOC_FAST_PATH Alloc* get() { -# ifdef USE_MALLOC +# ifdef SNMALLOC_PASS_THROUGH return get_reference(); # else auto*& alloc = get_reference(); diff --git a/src/override/malloc.cc b/src/override/malloc.cc index fd2ff1c..dc9f7a4 100644 --- a/src/override/malloc.cc +++ b/src/override/malloc.cc @@ -24,6 +24,18 @@ using namespace snmalloc; extern "C" { + void SNMALLOC_NAME_MANGLE(check_start)(void* ptr) + { +#if !defined(NDEBUG) && !defined(SNMALLOC_PASS_THROUGH) + if (Alloc::external_pointer(ptr) != ptr) + { + error("Using pointer that is not to the start of an allocation"); + } +#else + UNUSED(ptr); +#endif + } + SNMALLOC_EXPORT void* SNMALLOC_NAME_MANGLE(__malloc_end_pointer)(void* ptr) { return Alloc::external_pointer(ptr); @@ -36,6 +48,7 @@ extern "C" SNMALLOC_EXPORT void SNMALLOC_NAME_MANGLE(free)(void* ptr) { + SNMALLOC_NAME_MANGLE(check_start)(ptr); ThreadAlloc::get_noncachable()->dealloc(ptr); } @@ -79,24 +92,26 @@ extern "C" SNMALLOC_NAME_MANGLE(free)(ptr); return nullptr; } -#ifndef NDEBUG - // This check is redundant, because the check in memcpy will fail if this - // is skipped, but it's useful for debugging. - if (Alloc::external_pointer(ptr) != ptr) - { - error( - "Calling realloc on pointer that is not to the start of an allocation"); - } -#endif + + SNMALLOC_NAME_MANGLE(check_start)(ptr); + size_t sz = Alloc::alloc_size(ptr); // Keep the current allocation if the given size is in the same sizeclass. if (sz == round_size(size)) + { +#ifdef SNMALLOC_PASS_THROUGH + // snmallocs alignment guarantees can be broken by realloc in pass-through + // this is not exercised, by existing clients, but is tested. + if (pointer_align_up(ptr, natural_alignment(size)) == ptr) + return ptr; +#else return ptr; - +#endif + } void* p = SNMALLOC_NAME_MANGLE(malloc)(size); if (p != nullptr) { - SNMALLOC_ASSERT(p == Alloc::external_pointer(p)); + SNMALLOC_NAME_MANGLE(check_start)(p); sz = bits::min(size, sz); memcpy(p, ptr, sz); SNMALLOC_NAME_MANGLE(free)(ptr); diff --git a/src/test/func/fixed_region/fixed_region.cc b/src/test/func/fixed_region/fixed_region.cc index 8f25c57..5fba76a 100644 --- a/src/test/func/fixed_region/fixed_region.cc +++ b/src/test/func/fixed_region/fixed_region.cc @@ -23,6 +23,7 @@ extern "C" void oe_abort() using namespace snmalloc; int main() { +#ifndef SNMALLOC_PASS_THROUGH // Depends on snmalloc specific features auto& mp = *MemoryProviderStateMixin::make(); // 28 is large enough to produce a nested allocator. @@ -51,4 +52,5 @@ int main() if (oe_end < r1) abort(); } +#endif } diff --git a/src/test/func/malloc/malloc.cc b/src/test/func/malloc/malloc.cc index 029428d..b206cd5 100644 --- a/src/test/func/malloc/malloc.cc +++ b/src/test/func/malloc/malloc.cc @@ -22,7 +22,15 @@ void check_result(size_t size, size_t align, void* p, int err, bool null) const auto alloc_size = our_malloc_usable_size(p); const auto expected_size = round_size(size); - if ((align == 1) && (alloc_size != expected_size)) +#ifdef SNMALLOC_PASS_THROUGH + // Calling system allocator may allocate a larger block than + // snmalloc. Note, we have called the system allocator with + // the size snmalloc would allocate, so it won't be smaller. + const auto exact_size = false; +#else + const auto exact_size = align == 1; +#endif + if (exact_size && (alloc_size != expected_size)) { printf( "Usable size is %zu, but required to be %zu.\n", @@ -30,7 +38,7 @@ void check_result(size_t size, size_t align, void* p, int err, bool null) expected_size); abort(); } - if ((align != 1) && (alloc_size < expected_size)) + if ((!exact_size) && (alloc_size < expected_size)) { printf( "Usable size is %zu, but required to be at least %zu.\n", @@ -46,6 +54,16 @@ void check_result(size_t size, size_t align, void* p, int err, bool null) align); abort(); } + if ( + static_cast( + reinterpret_cast(p) % natural_alignment(size)) != 0) + { + printf( + "Address is 0x%zx, but should have natural alignment to 0x%zx.\n", + reinterpret_cast(p), + natural_alignment(size)); + abort(); + } our_free(p); } diff --git a/src/test/func/memory/memory.cc b/src/test/func/memory/memory.cc index e733e08..d123395 100644 --- a/src/test/func/memory/memory.cc +++ b/src/test/func/memory/memory.cc @@ -140,7 +140,7 @@ void test_random_allocation() cell = alloc->alloc(16); auto pair = allocated.insert(cell); // Check not already allocated - SNMALLOC_ASSERT(pair.second); + SNMALLOC_CHECK(pair.second); UNUSED(pair); alloc_count++; } @@ -197,14 +197,14 @@ void test_double_alloc() for (size_t i = 0; i < (n * 2); i++) { void* p = a1->alloc(20); - SNMALLOC_ASSERT(set1.find(p) == set1.end()); + SNMALLOC_CHECK(set1.find(p) == set1.end()); set1.insert(p); } for (size_t i = 0; i < (n * 2); i++) { void* p = a2->alloc(20); - SNMALLOC_ASSERT(set2.find(p) == set2.end()); + SNMALLOC_CHECK(set2.find(p) == set2.end()); set2.insert(p); } @@ -245,8 +245,8 @@ void test_external_pointer() void* p4 = Alloc::external_pointer(p2); UNUSED(p3); UNUSED(p4); - SNMALLOC_ASSERT(p1 == p3); - SNMALLOC_ASSERT((size_t)p4 == (size_t)p1 + size - 1); + SNMALLOC_CHECK(p1 == p3); + SNMALLOC_CHECK((size_t)p4 == (size_t)p1 + size - 1); } alloc->dealloc(p1, size); @@ -348,7 +348,7 @@ void test_alloc_16M() const size_t size = 16'000'000; void* p1 = alloc->alloc(size); - SNMALLOC_ASSERT(Alloc::alloc_size(Alloc::external_pointer(p1)) >= size); + SNMALLOC_CHECK(Alloc::alloc_size(Alloc::external_pointer(p1)) >= size); alloc->dealloc(p1); } @@ -359,7 +359,7 @@ void test_calloc_16M() const size_t size = 16'000'000; void* p1 = alloc->alloc(size); - SNMALLOC_ASSERT(Alloc::alloc_size(Alloc::external_pointer(p1)) >= size); + SNMALLOC_CHECK(Alloc::alloc_size(Alloc::external_pointer(p1)) >= size); alloc->dealloc(p1); } @@ -373,7 +373,7 @@ void test_calloc_large_bug() const size_t size = (SUPERSLAB_SIZE << 3) - 7; void* p1 = alloc->alloc(size); - SNMALLOC_ASSERT(Alloc::alloc_size(Alloc::external_pointer(p1)) >= size); + SNMALLOC_CHECK(Alloc::alloc_size(Alloc::external_pointer(p1)) >= size); alloc->dealloc(p1); } @@ -403,15 +403,17 @@ int main(int argc, char** argv) UNUSED(argv); #endif - test_calloc_large_bug(); - test_external_pointer_dealloc_bug(); - test_external_pointer_large(); test_alloc_dealloc_64k(); test_random_allocation(); test_calloc(); test_double_alloc(); +#ifndef SNMALLOC_PASS_THROUGH // Depends on snmalloc specific features + test_calloc_large_bug(); + test_external_pointer_dealloc_bug(); + test_external_pointer_large(); test_external_pointer(); test_alloc_16M(); test_calloc_16M(); +#endif return 0; } diff --git a/src/test/func/memory_usage/memory_usage.cc b/src/test/func/memory_usage/memory_usage.cc index bcea6d7..47e4ce9 100644 --- a/src/test/func/memory_usage/memory_usage.cc +++ b/src/test/func/memory_usage/memory_usage.cc @@ -76,7 +76,7 @@ int main(int argc, char** argv) { UNUSED(argc); UNUSED(argv); - +#ifndef SNMALLOC_PASS_THROUGH // Depends on snmalloc specific features setup(); add_n_allocs(5); @@ -100,4 +100,5 @@ int main(int argc, char** argv) remove_n_allocs(3); std::cout << "Teardown complete!" << std::endl; +#endif } diff --git a/src/test/func/sizeclass/sizeclass.cc b/src/test/func/sizeclass/sizeclass.cc index 7d6eaf8..696d014 100644 --- a/src/test/func/sizeclass/sizeclass.cc +++ b/src/test/func/sizeclass/sizeclass.cc @@ -12,7 +12,7 @@ void test_align_size() { bool failed = false; - SNMALLOC_ASSERT(snmalloc::aligned_size(128, 160) == 256); + SNMALLOC_CHECK(snmalloc::aligned_size(128, 160) == 256); for (size_t size = 1; size < snmalloc::sizeclass_to_size(snmalloc::NUM_SIZECLASSES - 1); diff --git a/src/test/func/statistics/stats.cc b/src/test/func/statistics/stats.cc index 6ecbe09..bd9dfec 100644 --- a/src/test/func/statistics/stats.cc +++ b/src/test/func/statistics/stats.cc @@ -2,6 +2,7 @@ int main() { +#ifndef SNMALLOC_PASS_THROUGH // This test depends on snmalloc internals snmalloc::Alloc* a = snmalloc::ThreadAlloc::get(); bool result; @@ -36,4 +37,5 @@ int main() { abort(); } +#endif } \ No newline at end of file diff --git a/src/test/perf/external_pointer/externalpointer.cc b/src/test/perf/external_pointer/externalpointer.cc index 20e7bba..513d3de 100644 --- a/src/test/perf/external_pointer/externalpointer.cc +++ b/src/test/perf/external_pointer/externalpointer.cc @@ -85,16 +85,18 @@ namespace test int main(int, char**) { +#ifndef SNMALLOC_PASS_THROUGH // Depends on snmalloc specific features setup(); xoroshiro::p128r64 r; -#ifdef NDEBUG +# ifdef NDEBUG size_t nn = 30; -#else +# else size_t nn = 3; -#endif +# endif for (size_t n = 0; n < nn; n++) test::test_external_pointer(r); return 0; +#endif } diff --git a/src/test/perf/singlethread/singlethread.cc b/src/test/perf/singlethread/singlethread.cc index 97053ce..0088028 100644 --- a/src/test/perf/singlethread/singlethread.cc +++ b/src/test/perf/singlethread/singlethread.cc @@ -20,7 +20,7 @@ void test_alloc_dealloc(size_t count, size_t size, bool write) for (size_t i = 0; i < ((count * 3) / 2); i++) { void* p = alloc->alloc(size); - SNMALLOC_ASSERT(set.find(p) == set.end()); + SNMALLOC_CHECK(set.find(p) == set.end()); if (write) *(int*)p = 4; @@ -35,14 +35,14 @@ void test_alloc_dealloc(size_t count, size_t size, bool write) void* p = *it; alloc->dealloc(p, size); set.erase(it); - SNMALLOC_ASSERT(set.find(p) == set.end()); + SNMALLOC_CHECK(set.find(p) == set.end()); } // alloc 1x objects for (size_t i = 0; i < count; i++) { void* p = alloc->alloc(size); - SNMALLOC_ASSERT(set.find(p) == set.end()); + SNMALLOC_CHECK(set.find(p) == set.end()); if (write) *(int*)p = 4;