From 4d6759aca47359f21c8168d76aebbde2fcff727d Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Wed, 4 Dec 2019 16:53:35 +0000 Subject: [PATCH 1/4] Make "pal_supports" not a function But rather a template vardecl, as per C++14 --- src/mem/alloc.h | 2 +- src/mem/chunkmap.h | 2 +- src/mem/largealloc.h | 4 ++-- src/pal/pal.h | 5 +---- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/mem/alloc.h b/src/mem/alloc.h index 1260e9c..d483f7a 100644 --- a/src/mem/alloc.h +++ b/src/mem/alloc.h @@ -1120,7 +1120,7 @@ namespace snmalloc else if constexpr (decommit_strategy == DecommitSuperLazy) { static_assert( - pal_supports(), + pal_supports, "A lazy decommit strategy cannot be implemented on platforms " "without low memory notifications"); } diff --git a/src/mem/chunkmap.h b/src/mem/chunkmap.h index b261293..1cb2edf 100644 --- a/src/mem/chunkmap.h +++ b/src/mem/chunkmap.h @@ -47,7 +47,7 @@ namespace snmalloc // Use flat map is under a single node. # define SNMALLOC_MAX_FLATPAGEMAP_SIZE PAGEMAP_NODE_SIZE #endif - static constexpr bool USE_FLATPAGEMAP = pal_supports() || + static constexpr bool USE_FLATPAGEMAP = pal_supports || (SNMALLOC_MAX_FLATPAGEMAP_SIZE >= sizeof(FlatPagemap)); diff --git a/src/mem/largealloc.h b/src/mem/largealloc.h index f7fcc92..c70a9b9 100644 --- a/src/mem/largealloc.h +++ b/src/mem/largealloc.h @@ -190,7 +190,7 @@ namespace snmalloc SNMALLOC_FAST_PATH uint64_t low_memory_epoch() { - if constexpr (pal_supports()) + if constexpr (pal_supports) { return PAL::low_memory_epoch(); } @@ -203,7 +203,7 @@ namespace snmalloc template void* reserve(size_t* size, size_t align) noexcept { - if constexpr (pal_supports()) + if constexpr (pal_supports) { return PAL::template reserve(size, align); } diff --git a/src/pal/pal.h b/src/pal/pal.h index 8b967b1..11ae017 100644 --- a/src/pal/pal.h +++ b/src/pal/pal.h @@ -61,8 +61,5 @@ namespace snmalloc * Query whether the PAL supports a specific feature. */ template - constexpr static bool pal_supports() - { - return (PAL::pal_features & F) == F; - } + constexpr static bool pal_supports = (PAL::pal_features & F) == F; } // namespace snmalloc From ef40f1cf1dae7fc1022ccfb4487255d1a93c45ef Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Wed, 4 Dec 2019 16:36:55 +0000 Subject: [PATCH 2/4] Replace "AAL" type with "Aal" to parallel "Pal" --- src/aal/aal.h | 2 +- src/ds/flaglock.h | 2 +- src/ds/mpscq.h | 2 +- src/mem/allocstats.h | 4 ++-- src/mem/pagemap.h | 2 +- src/test/perf/contention/contention.cc | 6 +++--- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/aal/aal.h b/src/aal/aal.h index f1f598d..6870378 100644 --- a/src/aal/aal.h +++ b/src/aal/aal.h @@ -63,7 +63,7 @@ namespace snmalloc namespace snmalloc { - using AAL = AAL_Generic; + using Aal = AAL_Generic; } // namespace snmalloc #if defined(_MSC_VER) && defined(SNMALLOC_VA_BITS_32) diff --git a/src/ds/flaglock.h b/src/ds/flaglock.h index 06d5337..9b2458d 100644 --- a/src/ds/flaglock.h +++ b/src/ds/flaglock.h @@ -13,7 +13,7 @@ namespace snmalloc FlagLock(std::atomic_flag& lock) : lock(lock) { while (lock.test_and_set(std::memory_order_acquire)) - AAL::pause(); + Aal::pause(); } ~FlagLock() diff --git a/src/ds/mpscq.h b/src/ds/mpscq.h index 9785ff9..8fd7b79 100644 --- a/src/ds/mpscq.h +++ b/src/ds/mpscq.h @@ -70,7 +70,7 @@ namespace snmalloc if (next != nullptr) { front = next; - AAL::prefetch(&(next->next)); + Aal::prefetch(&(next->next)); assert(front); std::atomic_thread_fence(std::memory_order_acquire); invariant(); diff --git a/src/mem/allocstats.h b/src/mem/allocstats.h index 19388f1..d7d038f 100644 --- a/src/mem/allocstats.h +++ b/src/mem/allocstats.h @@ -66,7 +66,7 @@ namespace snmalloc { CurrentMaxPair count; CurrentMaxPair slab_count; - uint64_t time = AAL::tick(); + uint64_t time = Aal::tick(); uint64_t ticks = 0; double online_average = 0; @@ -83,7 +83,7 @@ namespace snmalloc void addToRunningAverage() { - uint64_t now = AAL::tick(); + uint64_t now = Aal::tick(); if (slab_count.current != 0) { diff --git a/src/mem/pagemap.h b/src/mem/pagemap.h index ce2aa5e..f931ea0 100644 --- a/src/mem/pagemap.h +++ b/src/mem/pagemap.h @@ -162,7 +162,7 @@ namespace snmalloc while (address_cast(e->load(std::memory_order_relaxed)) == LOCKED_ENTRY) { - AAL::pause(); + Aal::pause(); } value = e->load(std::memory_order_acquire); } diff --git a/src/test/perf/contention/contention.cc b/src/test/perf/contention/contention.cc index b53ec12..845c4d2 100644 --- a/src/test/perf/contention/contention.cc +++ b/src/test/perf/contention/contention.cc @@ -31,18 +31,18 @@ private: auto prev = ready.fetch_add(1); if (prev + 1 == cores) { - start = AAL::tick(); + start = Aal::tick(); flag = true; } while (!flag) - AAL::pause(); + Aal::pause(); f(id); prev = complete.fetch_add(1); if (prev + 1 == cores) { - end = AAL::tick(); + end = Aal::tick(); } } From 0d6f708166b0c06f2affea47fcc92a24cf213a3a Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Tue, 3 Dec 2019 17:16:22 +0000 Subject: [PATCH 3/4] AAL: feature flags --- src/aal/aal.h | 16 ++++++++++++++++ src/aal/aal_x86.h | 5 +++++ 2 files changed, 21 insertions(+) diff --git a/src/aal/aal.h b/src/aal/aal.h index 6870378..e963250 100644 --- a/src/aal/aal.h +++ b/src/aal/aal.h @@ -11,6 +11,19 @@ namespace snmalloc { + /** + * Flags in a bitfield of attributes of this architecture, much like + * PalFeatures. + */ + enum AalFeatures : uint64_t + { + /** + * This architecture does not discriminate between integers and pointers, + * and so may use bit operations on pointer values. + */ + IntegerPointers = (1 << 0), + }; + /** * Architecture Abstraction Layer. Includes default implementations of some * functions using compiler builtins. Falls back to the definitions in the @@ -64,6 +77,9 @@ namespace snmalloc namespace snmalloc { using Aal = AAL_Generic; + + template + constexpr static bool aal_supports = (AAL::aal_features & F) == F; } // namespace snmalloc #if defined(_MSC_VER) && defined(SNMALLOC_VA_BITS_32) diff --git a/src/aal/aal_x86.h b/src/aal/aal_x86.h index 9259ee9..d30df33 100644 --- a/src/aal/aal_x86.h +++ b/src/aal/aal_x86.h @@ -55,6 +55,11 @@ namespace snmalloc } public: + /** + * Bitmap of AalFeature flags + */ + static constexpr uint64_t aal_features = IntegerPointers; + /** * On pipelined processors, notify the core that we are in a spin loop and * that speculative execution past this point may not be a performance gain. From 2fa60c719f9678d4507fc3d4a12a148af399de4e Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Mon, 15 Jul 2019 16:14:36 +0100 Subject: [PATCH 4/4] POISON only integer pointers For architectures that can't manipulate pointers like integers, don't try XORing them like this. It's not ideal -- perhaps we should have "else" branches to these tests. --- src/mem/metaslab.h | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h index 0f959cb..6a87c89 100644 --- a/src/mem/metaslab.h +++ b/src/mem/metaslab.h @@ -109,11 +109,12 @@ namespace snmalloc /// simple corruptions. static SNMALLOC_FAST_PATH void store_next(void* p, void* head) { -#ifndef CHECK_CLIENT *static_cast(p) = head; -#else - *static_cast(p) = head; - *(static_cast(p) + 1) = address_cast(head) ^ POISON; +#if defined(CHECK_CLIENT) + if constexpr (aal_supports) + { + *(static_cast(p) + 1) = address_cast(head) ^ POISON; + } #endif } @@ -121,11 +122,14 @@ namespace snmalloc /// In Debug checks for simple corruptions. static SNMALLOC_FAST_PATH void* follow_next(void* node) { -#ifdef CHECK_CLIENT - uintptr_t next = *static_cast(node); - uintptr_t chk = *(static_cast(node) + 1); - if ((next ^ chk) != POISON) - error("Detected memory corruption. Use-after-free."); +#if defined(CHECK_CLIENT) + if constexpr (aal_supports) + { + uintptr_t next = *static_cast(node); + uintptr_t chk = *(static_cast(node) + 1); + if ((next ^ chk) != POISON) + error("Detected memory corruption. Use-after-free."); + } #endif return *static_cast(node); }