From 070aa9467d2bf8821115707887857184818d918b Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 14 Apr 2020 09:53:48 +0100 Subject: [PATCH 1/5] First calloc test. --- .../func/first_operation/first_operation.cc | 63 +++++++++++++++++-- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/src/test/func/first_operation/first_operation.cc b/src/test/func/first_operation/first_operation.cc index 239ac35..6387eef 100644 --- a/src/test/func/first_operation/first_operation.cc +++ b/src/test/func/first_operation/first_operation.cc @@ -70,6 +70,52 @@ void alloc4(size_t size) a->dealloc(r); } +void check_calloc(void* p, size_t size) +{ + if (p != nullptr) + { + for (size_t i = 0; i < size; i++) + { + if (((uint8_t*)p)[i] != 0) + abort(); + // ((uint8_t*)p)[i] = 0x5a; + } + } +} + +void calloc1(size_t size) +{ + void* r = + snmalloc::ThreadAlloc::get_noncachable()->alloc( + size); + check_calloc(r, size); + snmalloc::ThreadAlloc::get_noncachable()->dealloc(r); +} + +void calloc2(size_t size) +{ + auto a = snmalloc::ThreadAlloc::get_noncachable(); + void* r = a->alloc(size); + check_calloc(r, size); + a->dealloc(r); +} + +void calloc3(size_t size) +{ + auto a = snmalloc::ThreadAlloc::get_noncachable(); + void* r = a->alloc(size); + check_calloc(r, size); + a->dealloc(r, size); +} + +void calloc4(size_t size) +{ + auto a = snmalloc::ThreadAlloc::get(); + void* r = a->alloc(size); + check_calloc(r, size); + a->dealloc(r); +} + void dealloc1(void* p, size_t) { snmalloc::ThreadAlloc::get_noncachable()->dealloc(p); @@ -97,16 +143,21 @@ void f(size_t size) auto t3 = std::thread(alloc3, size); auto t4 = std::thread(alloc4, size); + auto t5 = std::thread(calloc1, size); + auto t6 = std::thread(calloc2, size); + auto t7 = std::thread(calloc3, size); + auto t8 = std::thread(calloc4, size); + auto a = snmalloc::current_alloc_pool()->acquire(); auto p1 = a->alloc(size); auto p2 = a->alloc(size); auto p3 = a->alloc(size); auto p4 = a->alloc(size); - auto t5 = std::thread(dealloc1, p1, size); - auto t6 = std::thread(dealloc2, p2, size); - auto t7 = std::thread(dealloc3, p3, size); - auto t8 = std::thread(dealloc4, p4, size); + auto t9 = std::thread(dealloc1, p1, size); + auto t10 = std::thread(dealloc2, p2, size); + auto t11 = std::thread(dealloc3, p3, size); + auto t12 = std::thread(dealloc4, p4, size); t1.join(); t2.join(); @@ -116,6 +167,10 @@ void f(size_t size) t6.join(); t7.join(); t8.join(); + t9.join(); + t10.join(); + t11.join(); + t12.join(); snmalloc::current_alloc_pool()->release(a); snmalloc::current_alloc_pool()->debug_in_use(0); printf("."); From d4fccfa4ab9816a48f3dfb9b7272ba6f97af1150 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 14 Apr 2020 11:34:19 +0100 Subject: [PATCH 2/5] Fix callbacks This change does two things * correctly passes the template parameters into the callbacks fixing correct zeroing of memory. * By making the callbacks more specific it removes the warnings that GCC was generating. --- src/mem/alloc.h | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/mem/alloc.h b/src/mem/alloc.h index bf389b4..8cd2dbb 100644 --- a/src/mem/alloc.h +++ b/src/mem/alloc.h @@ -1131,7 +1131,7 @@ namespace snmalloc stats().sizeclass_alloc(sizeclass); return small_alloc_new_free_list(sizeclass); } - return small_alloc_first_alloc(size); + return small_alloc_first_alloc(sizeclass, size); } /** @@ -1139,10 +1139,13 @@ namespace snmalloc * then directs the allocation request to the newly created allocator. */ template - SNMALLOC_SLOW_PATH void* small_alloc_first_alloc(size_t size) + SNMALLOC_SLOW_PATH void* + small_alloc_first_alloc(sizeclass_t sizeclass, size_t size) { - return InitThreadAllocator([size](void* alloc) { - return reinterpret_cast(alloc)->alloc(size); + return InitThreadAllocator([sizeclass, size](void* alloc) { + return reinterpret_cast(alloc) + ->template small_alloc_inner( + sizeclass, size); }); } @@ -1320,8 +1323,9 @@ namespace snmalloc { if (NeedsInitialisation(this)) { - return InitThreadAllocator([size](void* alloc) { - return reinterpret_cast(alloc)->alloc(size); + return InitThreadAllocator([size, rsize, sizeclass](void* alloc) { + return reinterpret_cast(alloc) + ->medium_alloc(sizeclass, rsize, size); }); } slab = reinterpret_cast( @@ -1394,7 +1398,8 @@ namespace snmalloc if (NeedsInitialisation(this)) { return InitThreadAllocator([size](void* alloc) { - return reinterpret_cast(alloc)->alloc(size); + return reinterpret_cast(alloc) + ->large_alloc(size); }); } @@ -1420,8 +1425,8 @@ namespace snmalloc if (NeedsInitialisation(this)) { - InitThreadAllocator([p](void* alloc) { - reinterpret_cast(alloc)->dealloc(p); + InitThreadAllocator([p, size](void* alloc) { + reinterpret_cast(alloc)->large_dealloc(p, size); return nullptr; }); return; From 47547c4f66be046544faedf023322c0b729af288 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 14 Apr 2020 11:34:37 +0100 Subject: [PATCH 3/5] Re-enable GCC warning. --- CMakeLists.txt | 8 -------- 1 file changed, 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fb3d38a..13887a6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -33,14 +33,6 @@ macro(warnings_high) add_compile_options(-Wsign-conversion) endif () add_compile_options(-Wall -Wextra -Werror -Wundef) - # There are a few places with subtle reasons for array access being correct - # GCC's warnings are too aggressive and incorrectly assume the code is wrong. - # Disabling only in Release is so the ASSUME can be mapped to assert and check - # at runtime in debug. This ensures we are covering the cases of concern with - # debug checks, but not incurring runtime penalties in release. - if ((CMAKE_CXX_COMPILER_ID STREQUAL "GNU") AND (CMAKE_BUILD_TYPE STREQUAL "Release")) - add_compile_options(-Wno-array-bounds) - endif () endif() endmacro() From d135786ad5e7771fb844b24aab086650eb7f40ca Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 14 Apr 2020 12:11:04 +0100 Subject: [PATCH 4/5] Test case for large non-pagealigned calloc size. --- src/test/func/memory/memory.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/test/func/memory/memory.cc b/src/test/func/memory/memory.cc index 329c447..8a0d122 100644 --- a/src/test/func/memory/memory.cc +++ b/src/test/func/memory/memory.cc @@ -296,6 +296,20 @@ void test_calloc_16M() alloc->dealloc(p1); } +void test_calloc_large_bug() +{ + auto alloc = ThreadAlloc::get(); + // Perform large calloc, to check for correct zeroing from PAL. + // Some PALS have special paths for PAGE aligned zeroing of large + // allocations. This is a large allocation that is intentionally + // not a multiple of page size. + const size_t size = (SUPERSLAB_SIZE << 3) - 7; + + void* p1 = alloc->alloc(size); + SNMALLOC_ASSERT(Alloc::alloc_size(Alloc::external_pointer(p1)) >= size); + alloc->dealloc(p1); +} + int main(int argc, char** argv) { setup(); @@ -309,6 +323,7 @@ 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(); From 60005c809a97cd162666dc367bb4c86cbc73aeb3 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 14 Apr 2020 12:12:51 +0100 Subject: [PATCH 5/5] Fix to page alignment for size of commit. --- src/mem/largealloc.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mem/largealloc.h b/src/mem/largealloc.h index 50120ee..14cbd53 100644 --- a/src/mem/largealloc.h +++ b/src/mem/largealloc.h @@ -366,7 +366,8 @@ namespace snmalloc p = memory_provider.template reserve(large_class); if (p == nullptr) return nullptr; - memory_provider.template notify_using(p, size); + memory_provider.template notify_using( + p, bits::align_up(size, OS_PAGE_SIZE)); } else {