diff --git a/CMakeLists.txt b/CMakeLists.txt index 878e5cb..f4f8e67 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -50,13 +50,6 @@ int main() { } " SNMALLOC_PLATFORM_HAS_GETENTROPY) -if (NOT SNMALLOC_CI_BUILD) - option(USE_POSIX_COMMIT_CHECKS "Instrument Posix PAL to check for access to unused blocks of memory." Off) -else () - # This is enabled in every bit of CI to detect errors. - option(USE_POSIX_COMMIT_CHECKS "Instrument Posix PAL to check for access to unused blocks of memory." On) -endif () - # Provide as macro so other projects can reuse macro(warnings_high) if(MSVC) @@ -169,10 +162,6 @@ if(SNMALLOC_CI_BUILD) target_compile_definitions(snmalloc_lib INTERFACE -DSNMALLOC_CI_BUILD) endif() -if(USE_POSIX_COMMIT_CHECKS) - target_compile_definitions(snmalloc_lib INTERFACE -DUSE_POSIX_COMMIT_CHECKS) -endif() - if(SNMALLOC_PLATFORM_HAS_GETENTROPY) target_compile_definitions(snmalloc_lib INTERFACE -DSNMALLOC_PLATFORM_HAS_GETENTROPY) endif() @@ -277,13 +266,13 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) set(SHARED_FILES src/override/new.cc) add_shim(snmallocshim SHARED ${SHARED_FILES}) add_shim(snmallocshim-checks SHARED ${SHARED_FILES}) - target_compile_definitions(snmallocshim-checks PRIVATE CHECK_CLIENT) + target_compile_definitions(snmallocshim-checks PRIVATE SNMALLOC_CHECK_CLIENT) endif() if(SNMALLOC_RUST_SUPPORT) add_shim(snmallocshim-rust STATIC src/override/rust.cc) add_shim(snmallocshim-checks-rust STATIC src/override/rust.cc) - target_compile_definitions(snmallocshim-checks-rust PRIVATE CHECK_CLIENT) + target_compile_definitions(snmallocshim-checks-rust PRIVATE SNMALLOC_CHECK_CLIENT) endif() enable_testing() @@ -313,14 +302,11 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) add_executable(${TESTNAME} ${SRC}) - # For all tests enable commit checking. - target_compile_definitions(${TESTNAME} PRIVATE -DUSE_POSIX_COMMIT_CHECKS) - if (${FLAVOUR} STREQUAL "malloc") target_compile_definitions(${TESTNAME} PRIVATE SNMALLOC_PASS_THROUGH) endif() if (${FLAVOUR} STREQUAL "check") - target_compile_definitions(${TESTNAME} PRIVATE CHECK_CLIENT) + target_compile_definitions(${TESTNAME} PRIVATE SNMALLOC_CHECK_CLIENT) endif() target_link_libraries(${TESTNAME} snmalloc_lib) if (${TEST} MATCHES "release-.*") diff --git a/src/ds/defines.h b/src/ds/defines.h index 88ff5ea..f5d6646 100644 --- a/src/ds/defines.h +++ b/src/ds/defines.h @@ -114,12 +114,6 @@ namespace snmalloc # endif #endif -// // The CHECK_CLIENT macro is used to turn on minimal checking of the client -// // calling the API correctly. -// #if !defined(NDEBUG) && !defined(CHECK_CLIENT) -// # define CHECK_CLIENT -// #endif - inline SNMALLOC_FAST_PATH void check_client_error(const char* const str) { //[[clang::musttail]] @@ -132,7 +126,7 @@ check_client_impl(bool test, const char* const str) if (unlikely(!test)) check_client_error(str); } -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT # define check_client(test, str) check_client_impl(test, str) #else # define check_client(test, str) diff --git a/src/ds/helpers.h b/src/ds/helpers.h index a3d3e88..e34387d 100644 --- a/src/ds/helpers.h +++ b/src/ds/helpers.h @@ -73,7 +73,7 @@ namespace snmalloc } }; -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT template class ModArray { diff --git a/src/mem/allocconfig.h b/src/mem/allocconfig.h index 153e05b..660bc94 100644 --- a/src/mem/allocconfig.h +++ b/src/mem/allocconfig.h @@ -80,7 +80,7 @@ namespace snmalloc static constexpr size_t MIN_CHUNK_SIZE = bits::one_at_bit(MIN_CHUNK_BITS); // Minimum number of objects on a slab -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT static constexpr size_t MIN_OBJECT_COUNT = 13; #else static constexpr size_t MIN_OBJECT_COUNT = 4; diff --git a/src/mem/corealloc.h b/src/mem/corealloc.h index 4df9ca8..fb4ff91 100644 --- a/src/mem/corealloc.h +++ b/src/mem/corealloc.h @@ -146,7 +146,7 @@ namespace snmalloc FreeListBuilder b; SNMALLOC_ASSERT(b.empty()); -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT // Structure to represent the temporary list elements struct PreAllocObject { @@ -214,7 +214,7 @@ namespace snmalloc meta->free_queue.close(fl, key); void* p = finish_alloc_no_zero(fl.take(key), sizeclass); -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT // Check free list is well-formed on platforms with // integers as pointers. size_t count = 1; // Already taken one above. diff --git a/src/mem/freelist.h b/src/mem/freelist.h index 50dbc04..e6a173b 100644 --- a/src/mem/freelist.h +++ b/src/mem/freelist.h @@ -90,7 +90,7 @@ namespace snmalloc // TODO: Should really use C++20 atomic_ref rather than a union. AtomicCapPtr atomic_next_object; }; -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT // Encoded representation of a back pointer. // Hard to fake, and provides consistency on // the next pointers. @@ -104,7 +104,7 @@ namespace snmalloc } /** - * Assign next_object and update its prev_encoded if CHECK_CLIENT. + * Assign next_object and update its prev_encoded if SNMALLOC_CHECK_CLIENT. * Static so that it can be used on reference to a FreeObject. * * Returns a pointer to the next_object field of the next parameter as an @@ -116,7 +116,7 @@ namespace snmalloc CapPtr next, FreeListKey& key) { -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT next->prev_encoded = signed_prev(address_cast(curr), address_cast(next), key); #else @@ -127,13 +127,13 @@ namespace snmalloc } /** - * Assign next_object and update its prev_encoded if CHECK_CLIENT + * Assign next_object and update its prev_encoded if SNMALLOC_CHECK_CLIENT * * Uses the atomic view of next, so can be used in the message queues. */ void atomic_store_next(CapPtr next, FreeListKey& key) { -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT next->prev_encoded = signed_prev(address_cast(this), address_cast(next), key); #else @@ -152,7 +152,7 @@ namespace snmalloc CapPtr atomic_read_next(FreeListKey& key) { auto n = atomic_next_object.load(std::memory_order_acquire); -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT if (n != nullptr) { n->check_prev(signed_prev(address_cast(this), address_cast(n), key)); @@ -194,7 +194,7 @@ namespace snmalloc class FreeListIter { CapPtr curr{nullptr}; -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT address_t prev{0}; #endif @@ -203,7 +203,7 @@ namespace snmalloc CapPtr head, address_t prev_value) : curr(head) { -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT prev = prev_value; #endif UNUSED(prev_value); @@ -237,7 +237,7 @@ namespace snmalloc Aal::prefetch(next.unsafe_ptr()); curr = next; -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT c->check_prev(prev); prev = signed_prev(address_cast(c), address_cast(next), key); #else @@ -251,7 +251,7 @@ namespace snmalloc /** * Used to build a free list in object space. * - * Adds signing of pointers in the CHECK_CLIENT mode + * Adds signing of pointers in the SNMALLOC_CHECK_CLIENT mode * * We use the template parameter, so that an enclosing * class can make use of the remaining bytes, which may not diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h index ae2b7ea..c8194df 100644 --- a/src/mem/metaslab.h +++ b/src/mem/metaslab.h @@ -30,7 +30,7 @@ namespace snmalloc * * Spare 32bits are used for the fields in MetaslabEnd. */ -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT FreeListBuilder free_queue; #else FreeListBuilder free_queue; @@ -149,7 +149,7 @@ namespace snmalloc auto p = tmp_fl.take(key); fast_free_list = tmp_fl; -#ifdef CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT entropy.refresh_bits(); #else UNUSED(entropy); diff --git a/src/mem/sizeclasstable.h b/src/mem/sizeclasstable.h index a8607d5..13c3f69 100644 --- a/src/mem/sizeclasstable.h +++ b/src/mem/sizeclasstable.h @@ -166,7 +166,7 @@ namespace snmalloc */ inline uint16_t threshold_for_waking_slab(sizeclass_t sizeclass) { - // #ifdef CHECK_CLIENT + // #ifdef SNMALLOC_CHECK_CLIENT return sizeclass_metadata.waking[sizeclass]; // #else // UNUSED(sizeclass); diff --git a/src/pal/pal_apple.h b/src/pal/pal_apple.h index 685ae93..0c24fa9 100644 --- a/src/pal/pal_apple.h +++ b/src/pal/pal_apple.h @@ -113,7 +113,7 @@ namespace snmalloc { SNMALLOC_ASSERT(is_aligned_block(p, size)); -# ifdef USE_POSIX_COMMIT_CHECKS +# if defined(SNMALLOC_CHECK_CLIENT) && !defined(NDEBUG) memset(p, 0x5a, size); # endif @@ -126,7 +126,7 @@ namespace snmalloc while (madvise(p, size, MADV_FREE_REUSABLE) == -1 && errno == EAGAIN) ; -# ifdef USE_POSIX_COMMIT_CHECKS +# ifdef SNMALLOC_CHECK_CLIENT // This must occur after `MADV_FREE_REUSABLE`. // // `mach_vm_protect` is observably slower in benchmarks. @@ -179,7 +179,7 @@ namespace snmalloc } } -# ifdef USE_POSIX_COMMIT_CHECKS +# ifdef SNMALLOC_CHECK_CLIENT // Mark pages as writable for `madvise` below. // // `mach_vm_protect` is observably slower in benchmarks. @@ -218,7 +218,7 @@ namespace snmalloc // must be initialized to 0 or addr is interepreted as a lower-bound. mach_vm_address_t addr = 0; -# ifdef USE_POSIX_COMMIT_CHECKS +# ifdef SNMALLOC_CHECK_CLIENT vm_prot_t prot = committed ? VM_PROT_READ | VM_PROT_WRITE : VM_PROT_NONE; # else vm_prot_t prot = VM_PROT_READ | VM_PROT_WRITE; diff --git a/src/pal/pal_bsd.h b/src/pal/pal_bsd.h index 1c35776..38b3571 100644 --- a/src/pal/pal_bsd.h +++ b/src/pal/pal_bsd.h @@ -35,11 +35,11 @@ namespace snmalloc { SNMALLOC_ASSERT(is_aligned_block(p, size)); // Call this Pal to simulate the Windows decommit in CI. -#ifdef USE_POSIX_COMMIT_CHECKS +#if defined(SNMALLOC_CHECK_CLIENT) && !defined(NDEBUG) memset(p, 0x5a, size); #endif madvise(p, size, MADV_FREE); -#ifdef USE_POSIX_COMMIT_CHECKS +#ifdef SNMALLOC_CHECK_CLIENT mprotect(p, size, PROT_NONE); #endif } diff --git a/src/pal/pal_bsd_aligned.h b/src/pal/pal_bsd_aligned.h index 1cdd234..c636e50 100644 --- a/src/pal/pal_bsd_aligned.h +++ b/src/pal/pal_bsd_aligned.h @@ -37,10 +37,16 @@ namespace snmalloc int log2align = static_cast(bits::next_pow2_bits(size)); +#ifdef SNMALLOC_CHECK_CLIENT + auto prot = committed ? PROT_READ | PROT_WRITE : PROT_NONE; +#else + auto prot = PROT_READ | PROT_WRITE; +#endif + void* p = mmap( nullptr, size, - PROT_READ | PROT_WRITE, + prot, MAP_PRIVATE | MAP_ANONYMOUS | MAP_ALIGNED(log2align), -1, 0); diff --git a/src/pal/pal_posix.h b/src/pal/pal_posix.h index dde665a..3538487 100644 --- a/src/pal/pal_posix.h +++ b/src/pal/pal_posix.h @@ -165,10 +165,12 @@ namespace snmalloc static void notify_not_using(void* p, size_t size) noexcept { SNMALLOC_ASSERT(is_aligned_block(p, size)); -#ifdef USE_POSIX_COMMIT_CHECKS +#ifdef SNMALLOC_CHECK_CLIENT // Fill memory so that when we switch the pages back on we don't make // assumptions on the content. +# if !defined(NDEBUG) memset(p, 0x5a, size); +# endif mprotect(p, size, PROT_NONE); #else UNUSED(p); @@ -189,7 +191,7 @@ namespace snmalloc SNMALLOC_ASSERT( is_aligned_block(p, size) || (zero_mem == NoZero)); -#ifdef USE_POSIX_COMMIT_CHECKS +#ifdef SNMALLOC_CHECK_CLIENT mprotect(p, size, PROT_READ | PROT_WRITE); #else UNUSED(p); @@ -260,6 +262,12 @@ namespace snmalloc constexpr size_t min_size = bits::is64() ? bits::one_at_bit(31) : bits::one_at_bit(27); +#ifdef SNMALLOC_CHECK_CLIENT + auto prot = PROT_NONE; +#else + auto prot = PROT_READ | PROT_WRITE; +#endif + for (size_t size_request = bits::max(size, min_size); size_request >= size; size_request = size_request / 2) @@ -267,7 +275,7 @@ namespace snmalloc void* p = mmap( nullptr, size_request, - PROT_READ | PROT_WRITE, + prot, MAP_PRIVATE | MAP_ANONYMOUS | DefaultMMAPFlags::flags, AnonFD::fd, 0);