diff --git a/src/backend/backend.h b/src/backend/backend.h index d1022a3..6d33827 100644 --- a/src/backend/backend.h +++ b/src/backend/backend.h @@ -1,12 +1,12 @@ #pragma once #include "../mem/allocconfig.h" +#include "../mem/metadata.h" #include "../pal/pal.h" #include "commitrange.h" #include "commonconfig.h" #include "empty_range.h" #include "globalrange.h" #include "largebuddyrange.h" -#include "metatypes.h" #include "pagemap.h" #include "pagemapregisterrange.h" #include "palrange.h" @@ -34,23 +34,88 @@ namespace snmalloc * This class implements the standard backend for handling allocations. * It abstracts page table management and address space management. */ - template< - SNMALLOC_CONCEPT(ConceptPAL) PAL, - bool fixed_range, - typename PageMapEntry = MetaEntry> + template class BackendAllocator : public CommonConfig { public: using Pal = PAL; + using SlabMetadata = FrontendSlabMetadata; + class Pagemap { friend class BackendAllocator; + public: + /** + * Export the type stored in the pagemap. + * The following class could be replaced by: + * + * ``` + * using Entry = FrontendMetaEntry; + * ``` + * + * The full form here provides an example of how to extend the pagemap + * entries. It also guarantees that the front end never directly + * constructs meta entries, it only ever reads them or modifies them in + * place. + */ + class Entry : public FrontendMetaEntry + { + /** + * The private initialising constructor is usable only by this back end. + */ + friend class BackendAllocator; + + /** + * The private default constructor is usable only by the pagemap. + */ + friend class FlatPagemap; + + /** + * The only constructor that creates newly initialised meta entries. + * This is callable only by the back end. The front end may copy, + * query, and update these entries, but it may not create them + * directly. This contract allows the back end to store any arbitrary + * metadata in meta entries when they are first constructed. + */ + SNMALLOC_FAST_PATH + Entry(SlabMetadata* meta, uintptr_t ras) + : FrontendMetaEntry(meta, ras) + {} + + /** + * Default constructor. This must be callable from the pagemap. + */ + SNMALLOC_FAST_PATH Entry() = default; + + /** + * Copy assignment is used only by the pagemap. + */ + Entry& operator=(const Entry& other) + { + FrontendMetaEntry::operator=(other); + return *this; + } + }; + + private: SNMALLOC_REQUIRE_CONSTINIT - static inline FlatPagemap + static inline FlatPagemap concretePagemap; + /** + * Set the metadata associated with a chunk. + */ + SNMALLOC_FAST_PATH + static void set_metaentry(address_t p, size_t size, const Entry& t) + { + for (address_t a = p; a < p + size; a += MIN_CHUNK_SIZE) + { + concretePagemap.set(a, t); + } + } + public: /** * Get the metadata associated with a chunk. @@ -59,7 +124,7 @@ namespace snmalloc * to access a location that is not backed by a chunk. */ template - SNMALLOC_FAST_PATH static const MetaEntry& get_metaentry(address_t p) + SNMALLOC_FAST_PATH static const auto& get_metaentry(address_t p) { return concretePagemap.template get(p); } @@ -71,23 +136,11 @@ namespace snmalloc * to access a location that is not backed by a chunk. */ template - SNMALLOC_FAST_PATH static MetaEntry& get_metaentry_mut(address_t p) + SNMALLOC_FAST_PATH static auto& get_metaentry_mut(address_t p) { return concretePagemap.template get_mut(p); } - /** - * Set the metadata associated with a chunk. - */ - SNMALLOC_FAST_PATH - static void set_metaentry(address_t p, size_t size, const MetaEntry& t) - { - for (address_t a = p; a < p + size; a += MIN_CHUNK_SIZE) - { - concretePagemap.set(a, t); - } - } - static void register_range(address_t p, size_t sz) { concretePagemap.register_range(p, sz); @@ -246,26 +299,23 @@ namespace snmalloc /** * Returns a chunk of memory with alignment and size of `size`, and a - * metaslab block. + * block containing metadata about the slab. * * It additionally set the meta-data for this chunk of memory to * be - * (remote, sizeclass, metaslab) - * where metaslab, is the second element of the pair return. + * (remote, sizeclass, slab_metadata) + * where slab_metadata, is the second element of the pair return. */ - static std::pair, Metaslab*> + static std::pair, SlabMetadata*> alloc_chunk(LocalState& local_state, size_t size, uintptr_t ras) { SNMALLOC_ASSERT(bits::is_pow2(size)); SNMALLOC_ASSERT(size >= MIN_CHUNK_SIZE); - SNMALLOC_ASSERT((ras & MetaEntry::REMOTE_BACKEND_MARKER) == 0); - ras &= ~MetaEntry::REMOTE_BACKEND_MARKER; - auto meta_cap = - local_state.get_meta_range()->alloc_range(PAGEMAP_METADATA_STRUCT_SIZE); + local_state.get_meta_range()->alloc_range(sizeof(SlabMetadata)); - auto meta = meta_cap.template as_reinterpret().unsafe_ptr(); + auto meta = meta_cap.template as_reinterpret().unsafe_ptr(); if (meta == nullptr) { @@ -281,7 +331,7 @@ namespace snmalloc if (p == nullptr) { local_state.get_meta_range()->dealloc_range( - meta_cap, PAGEMAP_METADATA_STRUCT_SIZE); + meta_cap, sizeof(SlabMetadata)); errno = ENOMEM; #ifdef SNMALLOC_TRACING message<1024>("Out of memory"); @@ -289,32 +339,44 @@ namespace snmalloc return {p, nullptr}; } - meta->meta_common.chunk = p; - - MetaEntry t(&meta->meta_common, ras); + typename Pagemap::Entry t(meta, ras); Pagemap::set_metaentry(address_cast(p), size, t); p = Aal::capptr_bound(p, size); return {p, meta}; } - static void - dealloc_chunk(LocalState& local_state, MetaCommon& meta_common, size_t size) + static void dealloc_chunk( + LocalState& local_state, + SlabMetadata& slab_metadata, + capptr::Alloc alloc, + size_t size) { - auto chunk = meta_common.chunk; - /* * The backend takes possession of these chunks now, by disassociating * any existing remote allocator and metadata structure. If - * interrogated, the sizeclass reported by the MetaEntry is 0, which has - * size 0. + * interrogated, the sizeclass reported by the FrontendMetaEntry is 0, + * which has size 0. */ - MetaEntry t(nullptr, MetaEntry::REMOTE_BACKEND_MARKER); - Pagemap::set_metaentry(address_cast(chunk), size, t); + typename Pagemap::Entry t(nullptr, 0); + t.claim_for_backend(); + SNMALLOC_ASSERT_MSG( + Pagemap::get_metaentry(address_cast(alloc)).get_slab_metadata() == + &slab_metadata, + "Slab metadata {} passed for address {} does not match the meta entry " + "{} that is used for that address", + &slab_metadata, + address_cast(alloc), + Pagemap::get_metaentry(address_cast(alloc)).get_slab_metadata()); + Pagemap::set_metaentry(address_cast(alloc), size, t); local_state.get_meta_range()->dealloc_range( - capptr::Chunk(&meta_common), PAGEMAP_METADATA_STRUCT_SIZE); + capptr::Chunk(&slab_metadata), sizeof(SlabMetadata)); + // On non-CHERI platforms, we don't need to re-derive to get a pointer to + // the chunk. On CHERI platforms this will need to be stored in the + // SlabMetadata or similar. + capptr::Chunk chunk{alloc.unsafe_ptr()}; local_state.object_range->dealloc_range(chunk, size); } diff --git a/src/backend/backend_concept.h b/src/backend/backend_concept.h index 895e19a..27aa8df 100644 --- a/src/backend/backend_concept.h +++ b/src/backend/backend_concept.h @@ -1,34 +1,32 @@ #pragma once #ifdef __cpp_concepts -# include +# include "../ds/address.h" # include "../ds/concept.h" # include "../pal/pal_concept.h" -# include "../ds/address.h" + +# include namespace snmalloc { - class MetaEntry; - /** * The core of the static pagemap accessor interface: {get,set}_metadata. * - * get_metadata takes a bool-ean template parameter indicating whether it may + * get_metadata takes a boolean template parameter indicating whether it may * be accessing memory that is not known to be committed. */ template concept ConceptBackendMeta = - requires( - address_t addr, - size_t sz, - const MetaEntry& t) + requires(address_t addr, size_t sz, const typename Meta::Entry& t) { - { Meta::set_metaentry(addr, sz, t) } -> ConceptSame; + { + Meta::template get_metaentry(addr) + } + ->ConceptSame; - { Meta::template get_metaentry(addr) } - -> ConceptSame; - - { Meta::template get_metaentry(addr) } - -> ConceptSame; + { + Meta::template get_metaentry(addr) + } + ->ConceptSame; }; /** @@ -39,10 +37,27 @@ namespace snmalloc * underscore) below, which combines this and the core concept, above. */ template - concept ConceptBackendMeta_Range = - requires(address_t addr, size_t sz) + concept ConceptBackendMeta_Range = requires(address_t addr, size_t sz) { - { Meta::register_range(addr, sz) } -> ConceptSame; + { + Meta::register_range(addr, sz) + } + ->ConceptSame; + }; + + template + concept ConceptBuddyRangeMeta = + requires(address_t addr, size_t sz, const typename Meta::Entry& t) + { + { + Meta::template get_metaentry_mut(addr) + } + ->ConceptSame; + + { + Meta::template get_metaentry_mut(addr) + } + ->ConceptSame; }; /** @@ -55,7 +70,7 @@ namespace snmalloc */ template concept ConceptBackendMetaRange = - ConceptBackendMeta && ConceptBackendMeta_Range; + ConceptBackendMeta&& ConceptBackendMeta_Range; /** * The backend also defines domestication (that is, the difference between @@ -65,16 +80,18 @@ namespace snmalloc */ template concept ConceptBackendDomestication = - requires( - typename Globals::LocalState* ls, - capptr::AllocWild ptr) + requires(typename Globals::LocalState* ls, capptr::AllocWild ptr) + { { - { Globals::capptr_domesticate(ls, ptr) } - -> ConceptSame>; + Globals::capptr_domesticate(ls, ptr) + } + ->ConceptSame>; - { Globals::capptr_domesticate(ls, ptr.template as_static()) } - -> ConceptSame>; - }; + { + Globals::capptr_domesticate(ls, ptr.template as_static()) + } + ->ConceptSame>; + }; class CommonConfig; struct Flags; @@ -88,24 +105,33 @@ namespace snmalloc * * have static pagemap accessors via T::Pagemap * * define a T::LocalState type (and alias it as T::Pagemap::LocalState) * * define T::Options of type snmalloc::Flags - * * expose the global allocator pool via T::pool() + * * expose the global allocator pool via T::pool() if pool allocation is + * used. * */ template concept ConceptBackendGlobals = - std::is_base_of::value && - ConceptPAL && - ConceptBackendMetaRange && - requires() + std::is_base_of::value&& + ConceptPAL&& + ConceptBackendMetaRange&& requires() + { + typename Globals::LocalState; + { - typename Globals::LocalState; - - { Globals::Options } -> ConceptSameModRef; - + Globals::Options + } + ->ConceptSameModRef; + } + &&( + requires() { + Globals::Options.CoreAllocIsPoolAllocated == true; typename Globals::GlobalPoolState; - { Globals::pool() } -> ConceptSame; - - }; + { + Globals::pool() + } + ->ConceptSame; + } || + requires() { Globals::Options.CoreAllocIsPoolAllocated == false; }); /** * The lazy version of the above; please see ds/concept.h and use sparingly. diff --git a/src/backend/largebuddyrange.h b/src/backend/largebuddyrange.h index a4a807a..a57ed69 100644 --- a/src/backend/largebuddyrange.h +++ b/src/backend/largebuddyrange.h @@ -4,6 +4,7 @@ #include "../ds/bits.h" #include "../mem/allocconfig.h" #include "../pal/pal.h" +#include "backend_concept.h" #include "buddy.h" #include "range_helpers.h" @@ -14,7 +15,7 @@ namespace snmalloc /** * Class for using the pagemap entries for the buddy allocator. */ - template + template class BuddyChunkRep { public: @@ -23,68 +24,86 @@ namespace snmalloc * of) chunks of the address space; as such, bits in (MIN_CHUNK_SIZE - 1) * are unused and so the RED_BIT is packed therein. However, in practice, * these are not "just any" uintptr_t-s, but specifically the uintptr_t-s - * inside the Pagemap's MetaEntry structures. As such, there are some - * additional bit-swizzling concerns; see set() and get() below. + * inside the Pagemap's BackendAllocator::Entry structures. + * + * The BackendAllocator::Entry provides us with helpers that guarantee that + * we use only the bits that we are allowed to. + * @{ */ - using Holder = uintptr_t; + using Handle = MetaEntryBase::BackendStateWordRef; using Contents = uintptr_t; + ///@} - static constexpr address_t RED_BIT = 1 << 1; + /** + * The bit that we will use to mark an entry as red. + * This has constraints in two directions, it must not be one of the + * reserved bits from the perspective of the meta entry and it must not be + * a bit that is a valid part of the address of a chunk. + * @{ + */ + static constexpr address_t RED_BIT = 1 << 8; static_assert(RED_BIT < MIN_CHUNK_SIZE); - static_assert(RED_BIT != MetaEntry::META_BOUNDARY_BIT); - static_assert(RED_BIT != MetaEntry::REMOTE_BACKEND_MARKER); + static_assert(MetaEntryBase::is_backend_allowed_value( + MetaEntryBase::Word::One, RED_BIT)); + static_assert(MetaEntryBase::is_backend_allowed_value( + MetaEntryBase::Word::Two, RED_BIT)); + ///@} + /// The value of a null node, as returned by `get` static constexpr Contents null = 0; + /// The value of a null node, as stored in a `uintptr_t`. + static constexpr Contents root = 0; - static void set(Holder* ptr, Contents r) + /** + * Set the value. Preserve the red/black colour. + */ + static void set(Handle ptr, Contents r) { - SNMALLOC_ASSERT((r & (MIN_CHUNK_SIZE - 1)) == 0); - /* - * Preserve lower bits, claim as backend, and update contents of holder. - * - * This is excessive at present but no harder than being more precise - * while also being future-proof. All that is strictly required would be - * to preserve META_BOUNDARY_BIT and RED_BIT in ->meta and to assert - * REMOTE_BACKEND_MARKER in ->remote_and_sizeclass (if it isn't already - * asserted). However, we don't know which Holder* we have been given, - * nor do we know whether this Holder* is completely new (and so we are - * the first reasonable opportunity to assert REMOTE_BACKEND_MARKER) or - * recycled from the frontend, and so we preserve and assert more than - * strictly necessary. - * - * The use of `address_cast` below is a CHERI-ism; otherwise both `r` and - * `*ptr & ...` are plausibly provenance-carrying values and the compiler - * balks at the ambiguity. - */ - *ptr = r | address_cast(*ptr & (MIN_CHUNK_SIZE - 1)) | - MetaEntry::REMOTE_BACKEND_MARKER; + ptr = r | (static_cast(ptr.get()) & RED_BIT); } - static Contents get(const Holder* ptr) + /** + * Returns the value, stripping out the red/black colour. + */ + static Contents get(const Handle ptr) { - return *ptr & ~(MIN_CHUNK_SIZE - 1); + return ptr.get() & ~RED_BIT; } - static Holder& ref(bool direction, Contents k) + /** + * Returns a pointer to the tree node for the specified address. + */ + static Handle ref(bool direction, Contents k) { - MetaEntry& entry = - Pagemap::template get_metaentry_mut(address_cast(k)); + // Special case for accessing the null entry. We want to make sure + // that this is never modified by the back end, so we make it point to + // a constant entry and use the MMU to trap even in release modes. + static const Contents null_entry = 0; + if (SNMALLOC_UNLIKELY(address_cast(k) == 0)) + { + return {const_cast(&null_entry)}; + } + auto& entry = Pagemap::template get_metaentry_mut(address_cast(k)); if (direction) - return entry.meta; + return entry.get_backend_word(Pagemap::Entry::Word::One); - return entry.remote_and_sizeclass; + return entry.get_backend_word(Pagemap::Entry::Word::Two); } static bool is_red(Contents k) { - return (ref(true, k) & RED_BIT) == RED_BIT; + return (ref(true, k).get() & RED_BIT) == RED_BIT; } static void set_red(Contents k, bool new_is_red) { if (new_is_red != is_red(k)) - ref(true, k) ^= RED_BIT; + { + auto v = ref(true, k); + v = v.get() ^ RED_BIT; + } + SNMALLOC_ASSERT(is_red(k) == new_is_red); } static Contents offset(Contents k, size_t size) @@ -117,6 +136,24 @@ namespace snmalloc return k; } + /** + * Convert the pointer wrapper into something that the snmalloc debug + * printing code can print. + */ + static address_t printable(Handle k) + { + return k.printable_address(); + } + + /** + * Returns the name for use in debugging traces. Not used in normal builds + * (release or debug), only when tracing is enabled. + */ + static const char* name() + { + return "BuddyChunkRep"; + } + static bool can_consolidate(Contents k, size_t size) { // Need to know both entries exist in the pagemap. @@ -125,7 +162,7 @@ namespace snmalloc // The buddy could be in a part of the pagemap that has // not been registered and thus could segfault on access. auto larger = bits::max(k, buddy(k, size)); - MetaEntry& entry = + auto& entry = Pagemap::template get_metaentry_mut(address_cast(larger)); return !entry.is_boundary(); } @@ -135,7 +172,7 @@ namespace snmalloc typename ParentRange, size_t REFILL_SIZE_BITS, size_t MAX_SIZE_BITS, - SNMALLOC_CONCEPT(ConceptBackendMeta) Pagemap, + SNMALLOC_CONCEPT(ConceptBuddyRangeMeta) Pagemap, bool Consolidate = true> class LargeBuddyRange { @@ -192,7 +229,9 @@ namespace snmalloc // Tag first entry so we don't consolidate it. if (first) { - Pagemap::get_metaentry_mut(address_cast(base)).set_boundary(); + auto& entry = Pagemap::get_metaentry_mut(address_cast(base)); + entry.claim_for_backend(); + entry.set_boundary(); } } else @@ -316,4 +355,4 @@ namespace snmalloc dealloc_overflow(overflow); } }; -} // namespace snmalloc \ No newline at end of file +} // namespace snmalloc diff --git a/src/backend/metatypes.h b/src/backend/metatypes.h deleted file mode 100644 index 354d8f0..0000000 --- a/src/backend/metatypes.h +++ /dev/null @@ -1,186 +0,0 @@ -#pragma once -#include "../ds/concept.h" -#include "../mem/allocconfig.h" /* TODO: CACHELINE_SIZE */ -#include "../pal/pal_concept.h" - -namespace snmalloc -{ - /** - * A guaranteed type-stable sub-structure of all metadata referenced by the - * Pagemap. Use-specific structures (Metaslab, ChunkRecord) are expected to - * have this at offset zero so that, even in the face of concurrent mutation - * and reuse of the memory backing that metadata, the types of these fields - * remain fixed. - * - * This class's data is fully private but is friends with the relevant backend - * types and, thus, is "opaque" to the frontend. - */ - struct MetaCommon - { - capptr::Chunk chunk; - - /** - * Expose the address of, though not the authority to, our corresponding - * chunk. - */ - [[nodiscard]] SNMALLOC_FAST_PATH address_t chunk_address() - { - return address_cast(this->chunk); - } - - /** - * Zero (possibly by unmapping) the memory backing this chunk. We must rely - * on the caller to tell us its size, which is a little unfortunate. - */ - template - SNMALLOC_FAST_PATH void zero_chunk(size_t chunk_size) - { - PAL::zero(this->chunk.unsafe_ptr(), chunk_size); - } - }; - - static const size_t PAGEMAP_METADATA_STRUCT_SIZE = -#ifdef __CHERI_PURE_CAPABILITY__ - 2 * CACHELINE_SIZE -#else - CACHELINE_SIZE -#endif - ; - - // clang-format off - /* This triggers an ICE (C1001) in MSVC, so disable it there */ -#if defined(__cpp_concepts) && !defined(_MSC_VER) - - template - concept ConceptMetadataStruct = - // Metadata structures must be standard layout (for offsetof()), - std::is_standard_layout_v && - // must be of a sufficiently small size, - sizeof(Meta) <= PAGEMAP_METADATA_STRUCT_SIZE && - // and must be pointer-interconvertable with MetaCommon. - ( - ConceptSame || - requires(Meta m) { - // Otherwise, meta must have MetaCommon field named meta_common ... - { &m.meta_common } -> ConceptSame; - // at offset zero. - (offsetof(Meta, meta_common) == 0); - } - ); - - static_assert(ConceptMetadataStruct); -# define USE_METADATA_CONCEPT -#endif - // clang-format on - - /** - * Entry stored in the pagemap. See docs/AddressSpace.md for the full - * MetaEntry lifecycle. - */ - class MetaEntry - { - template - friend class BuddyChunkRep; - - /** - * In common cases, the pointer to the metaslab. See docs/AddressSpace.md - * for additional details. - * - * The bottom bit is used to indicate if this is the first chunk in a PAL - * allocation, that cannot be combined with the preceeding chunk. - */ - uintptr_t meta{0}; - - /** - * Bit used to indicate this should not be considered part of the previous - * PAL allocation. - * - * Some platforms cannot treat different PalAllocs as a single allocation. - * This is true on CHERI as the combined permission might not be - * representable. It is also true on Windows as you cannot Commit across - * multiple continuous VirtualAllocs. - */ - static constexpr address_t META_BOUNDARY_BIT = 1 << 0; - - /** - * In common cases, a bit-packed pointer to the owning allocator (if any), - * and the sizeclass of this chunk. See mem/metaslab.h:MetaEntryRemote for - * details of this case and docs/AddressSpace.md for further details. - */ - uintptr_t remote_and_sizeclass{0}; - - public: - /** - * This bit is set in remote_and_sizeclass to discriminate between the case - * that it is in use by the frontend (0) or by the backend (1). For the - * former case, see mem/metaslab.h; for the latter, see backend/backend.h - * and backend/largebuddyrange.h. - * - * This value is statically checked by the frontend to ensure that its - * bit packing does not conflict; see mem/remoteallocator.h - */ - static constexpr address_t REMOTE_BACKEND_MARKER = 1 << 7; - - constexpr MetaEntry() = default; - - /** - * Constructor, provides the remote and sizeclass embedded in a single - * pointer-sized word. This format is not guaranteed to be stable and so - * the second argument of this must always be the return value from - * `get_remote_and_sizeclass`. - */ - SNMALLOC_FAST_PATH - MetaEntry(MetaCommon* meta, uintptr_t remote_and_sizeclass) - : meta(unsafe_to_uintptr(meta)), - remote_and_sizeclass(remote_and_sizeclass) - {} - - /** - * Return the remote and sizeclass in an implementation-defined encoding. - * This is not guaranteed to be stable across snmalloc releases and so the - * only safe use for this is to pass it to the two-argument constructor of - * this class. - */ - [[nodiscard]] SNMALLOC_FAST_PATH const uintptr_t& - get_remote_and_sizeclass() const - { - return remote_and_sizeclass; - } - - MetaEntry(const MetaEntry&) = delete; - - MetaEntry& operator=(const MetaEntry& other) - { - // Don't overwrite the boundary bit with the other's - meta = (other.meta & ~META_BOUNDARY_BIT) | - address_cast(meta & META_BOUNDARY_BIT); - remote_and_sizeclass = other.remote_and_sizeclass; - return *this; - } - - /** - * Return the Metaslab metadata associated with this chunk, guarded by an - * assert that this chunk is being used as a slab (i.e., has an associated - * owning allocator). - */ - [[nodiscard]] SNMALLOC_FAST_PATH MetaCommon* get_meta() const - { - return reinterpret_cast(meta & ~META_BOUNDARY_BIT); - } - - void set_boundary() - { - meta |= META_BOUNDARY_BIT; - } - - [[nodiscard]] bool is_boundary() const - { - return meta & META_BOUNDARY_BIT; - } - - bool clear_boundary_bit() - { - return meta &= ~META_BOUNDARY_BIT; - } - }; -} // namespace snmalloc diff --git a/src/backend/smallbuddyrange.h b/src/backend/smallbuddyrange.h index 4ee0758..2500e1f 100644 --- a/src/backend/smallbuddyrange.h +++ b/src/backend/smallbuddyrange.h @@ -1,6 +1,7 @@ #pragma once #include "../ds/address.h" +#include "../mem/allocconfig.h" #include "../pal/pal.h" #include "range_helpers.h" @@ -22,13 +23,14 @@ namespace snmalloc class BuddyInplaceRep { public: - using Holder = capptr::Chunk; + using Handle = capptr::Chunk*; using Contents = capptr::Chunk; static constexpr Contents null = nullptr; + static constexpr Contents root = nullptr; static constexpr address_t MASK = 1; - static void set(Holder* ptr, Contents r) + static void set(Handle ptr, Contents r) { SNMALLOC_ASSERT((address_cast(r) & MASK) == 0); if (r == nullptr) @@ -40,44 +42,45 @@ namespace snmalloc .template as_static(); } - static Contents get(Holder* ptr) + static Contents get(Handle ptr) { return pointer_align_down<2, FreeChunk>((*ptr).as_void()); } - static Holder& ref(bool direction, Contents r) + static Handle ref(bool direction, Contents r) { if (direction) - return r->left; + return &r->left; - return r->right; + return &r->right; } static bool is_red(Contents k) { if (k == nullptr) return false; - return (address_cast(ref(false, k)) & MASK) == MASK; + return (address_cast(*ref(false, k)) & MASK) == MASK; } static void set_red(Contents k, bool new_is_red) { if (new_is_red != is_red(k)) { - auto& r = ref(false, k); - auto old_addr = pointer_align_down<2, FreeChunk>(r.as_void()); + auto r = ref(false, k); + auto old_addr = pointer_align_down<2, FreeChunk>(r->as_void()); if (new_is_red) { if (old_addr == nullptr) - r = capptr::Chunk(reinterpret_cast(MASK)); + *r = capptr::Chunk(reinterpret_cast(MASK)); else - r = pointer_offset(old_addr, MASK).template as_static(); + *r = pointer_offset(old_addr, MASK).template as_static(); } else { - r = old_addr; + *r = old_addr; } + SNMALLOC_ASSERT(is_red(k) == new_is_red); } } @@ -115,6 +118,25 @@ namespace snmalloc return address_cast(k); } + /** + * Return the holder in some format suitable for printing by snmalloc's + * debug log mechanism. Used only when used in tracing mode, not normal + * debug or release builds. Raw pointers are printable already, so this is + * the identity function. + */ + static Handle printable(Handle k) + { + return k; + } + + /** + * Return a name for use in tracing mode. Unused in any other context. + */ + static const char* name() + { + return "BuddyInplaceRep"; + } + static bool can_consolidate(Contents k, size_t size) { UNUSED(k, size); @@ -225,4 +247,4 @@ namespace snmalloc add_range(base, size); } }; -} // namespace snmalloc \ No newline at end of file +} // namespace snmalloc diff --git a/src/ds/helpers.h b/src/ds/helpers.h index dd4d557..f599aac 100644 --- a/src/ds/helpers.h +++ b/src/ds/helpers.h @@ -471,4 +471,10 @@ namespace snmalloc return buffer.data(); } }; + + /** + * Convenience type that has no fields / methods. + */ + struct Empty + {}; } // namespace snmalloc diff --git a/src/ds/redblacktree.h b/src/ds/redblacktree.h index 10eb55f..3d87f9e 100644 --- a/src/ds/redblacktree.h +++ b/src/ds/redblacktree.h @@ -11,16 +11,40 @@ namespace snmalloc { #ifdef __cpp_concepts + /** + * The representation must define two types. `Contents` defines some + * identifier that can be mapped to a node as a value type. `Handle` defines + * a reference to the storage, which can be used to update it. + * + * Conceptually, `Contents` is a node ID and `Handle` is a pointer to a node + * ID. + */ template concept RBRepTypes = requires() { - typename Rep::Holder; + typename Rep::Handle; typename Rep::Contents; }; + /** + * The representation must define operations on the holder and contents + * types. It must be able to 'dereference' a holder with `get`, assign to it + * with `set`, set and query the red/black colour of a node with `set_red` and + * `is_red`. + * + * The `ref` method provides uniform access to the children of a node, + * returning a holder pointing to either the left or right child, depending on + * the direction parameter. + * + * The backend must also provide two constant values. + * `Rep::null` defines a value that, if returned from `get`, indicates a null + * value. `Rep::root` defines a value that, if constructed directly, indicates + * a null value and can therefore be used as the initial raw bit pattern of + * the root node. + */ template concept RBRepMethods = - requires(typename Rep::Holder* hp, typename Rep::Contents k, bool b) + requires(typename Rep::Handle hp, typename Rep::Contents k, bool b) { { Rep::get(hp) @@ -41,7 +65,20 @@ namespace snmalloc { Rep::ref(b, k) } - ->ConceptSame; + ->ConceptSame; + { + Rep::null + } + ->ConceptSameModRef; + { + typename Rep::Handle + { + const_cast< + std::remove_const_t>*>( + &Rep::root) + } + } + ->ConceptSame; }; template @@ -70,33 +107,44 @@ namespace snmalloc bool TRACE = false> class RBTree { - using H = typename Rep::Holder; + using H = typename Rep::Handle; using K = typename Rep::Contents; // Container that behaves like a C++ Ref type to enable assignment // to treat left, right and root uniformly. class ChildRef { - H* ptr; + H ptr; public: ChildRef() : ptr(nullptr) {} - ChildRef(H& p) : ptr(&p) {} + ChildRef(H p) : ptr(p) {} + + ChildRef(const ChildRef& other) = default; operator K() { return Rep::get(ptr); } - ChildRef& operator=(const K& t) + ChildRef& operator=(const ChildRef& other) = default; + + ChildRef& operator=(const K t) { // Use representations assigment, so we update the correct bits - // color and other things way also be stored in the Holder. + // color and other things way also be stored in the Handle. Rep::set(ptr, t); return *this; } + /** + * Comparison operators. Note that these are nominal comparisons: + * they compare the identities of the references rather than the values + * referenced. + * comparison of the values held in these child references. + * @{ + */ bool operator==(const ChildRef t) const { return ptr == t.ptr; @@ -106,20 +154,26 @@ namespace snmalloc { return ptr != t.ptr; } - - H* addr() - { - return ptr; - } + ///@} bool is_null() { return Rep::get(ptr) == Rep::null; } + + /** + * Return the reference in some printable format defined by the + * representation. + */ + auto printable() + { + return Rep::printable(ptr); + } }; // Root field of the tree - H root{}; + typename std::remove_const_t> + root{Rep::root}; static ChildRef get_dir(bool direction, K k) { @@ -128,7 +182,7 @@ namespace snmalloc ChildRef get_root() { - return {root}; + return {H{&root}}; } void invariant() @@ -194,6 +248,23 @@ namespace snmalloc { ChildRef node; bool dir = false; + + /** + * Update the step to point to a new node and direction. + */ + void set(ChildRef r, bool direction) + { + node = r; + dir = direction; + } + + /** + * Update the step to point to a new node and direction. + */ + void set(typename Rep::Handle r, bool direction) + { + set(ChildRef(r), direction); + } }; public: @@ -207,9 +278,9 @@ namespace snmalloc std::array path; size_t length = 0; - RBPath(typename Rep::Holder& root) : path{} + RBPath(typename Rep::Handle root) : path{} { - path[0] = {root, false}; + path[0].set(root, false); length = 1; } @@ -258,7 +329,7 @@ namespace snmalloc auto next = get_dir(direction, curr()); if (next.is_null()) return false; - path[length] = {next, direction}; + path[length].set(next, direction); length++; return true; } @@ -269,7 +340,7 @@ namespace snmalloc bool move_inc_null(bool direction) { auto next = get_dir(direction, curr()); - path[length] = {next, direction}; + path[length].set(next, direction); length++; return !(next.is_null()); } @@ -319,8 +390,8 @@ namespace snmalloc { message<1024>( " -> {} @ {} ({})", - K(path[i].node), - path[i].node.addr(), + Rep::printable(K(path[i].node)), + path[i].node.printable(), path[i].dir); } } @@ -337,7 +408,7 @@ namespace snmalloc { if constexpr (TRACE) { - message<100>("-------"); + message<100>("------- {}", Rep::name()); message<1024>(msg); path.print(); print(base); @@ -378,11 +449,11 @@ namespace snmalloc "{}\\_{}{}{}@{} ({})", indent, colour, - curr, + Rep::printable((K(curr))), reset, - curr.addr(), + curr.printable(), depth); - if ((get_dir(true, curr) != 0) || (get_dir(false, curr) != 0)) + if (!(get_dir(true, curr).is_null() && get_dir(false, curr).is_null())) { auto s_indent = std::string(indent); print(get_dir(true, curr), (s_indent + "|").c_str(), depth + 1); @@ -465,7 +536,7 @@ namespace snmalloc // we are searching for nearby red elements so we can rotate the tree to // rebalance. The following slides nicely cover the case analysis below // https://www.cs.purdue.edu/homes/ayg/CS251/slides/chap13c.pdf - while (path.curr() != ChildRef(root)) + while (path.curr() != ChildRef(H{&root})) { K parent = path.parent(); bool cur_dir = path.curr_dir(); @@ -574,7 +645,7 @@ namespace snmalloc } // Insert an element at the given path. - void insert_path(RBPath path, K value) + void insert_path(RBPath& path, K value) { SNMALLOC_ASSERT(path.curr().is_null()); path.curr() = value; @@ -704,7 +775,7 @@ namespace snmalloc RBPath get_root_path() { - return RBPath(root); + return RBPath(H{&root}); } }; } // namespace snmalloc diff --git a/src/ds/seqset.h b/src/ds/seqset.h index da5eb46..510961c 100644 --- a/src/ds/seqset.h +++ b/src/ds/seqset.h @@ -20,12 +20,32 @@ namespace snmalloc template class SeqSet { + /** + * This sequence structure is intrusive, in that it requires the use of a + * `next` field in the elements it manages, but, unlike some other intrusive + * designs, it does not require the use of a `container_of`-like construct, + * because its pointers point to the element, not merely the intrusive + * member. + * + * In some cases, the next pointer is provided by a superclass but the list + * is templated over the subclass. The `SeqSet` enforces the invariant that + * only instances of the subclass can be added to the list and so can safely + * down-cast the type of `.next` to `T*`. As such, we require only that the + * `next` field is a pointer to `T` or some superclass of `T`. + * %{ + */ + using NextPtr = decltype(std::declval().next); + static_assert( + std::is_base_of_v, T>, + "T->next must be a queue pointer to T"); + ///@} + /** * Field representation for Fifo behaviour. */ struct FieldFifo { - T* head{nullptr}; + NextPtr head{nullptr}; }; /** @@ -33,14 +53,10 @@ namespace snmalloc */ struct FieldLifo { - T* head{nullptr}; - T** end{&head}; + NextPtr head{nullptr}; + NextPtr* end{&head}; }; - static_assert( - std::is_same::value, - "T->next must be a queue pointer to T"); - /** * Field indirection to actual representation. * Different numbers of fields are required for the @@ -90,7 +106,10 @@ namespace snmalloc else v.head = v.head->next; } - return result; + // This cast is safe if the ->next pointers in all of the objects in the + // list are managed by this class because object types are checked on + // insertion. + return static_cast(result); } /** @@ -107,7 +126,7 @@ namespace snmalloc if (is_empty()) return; - T** prev = &(v.head); + NextPtr* prev = &(v.head); while (true) { @@ -117,11 +136,11 @@ namespace snmalloc break; } - T* curr = *prev; + NextPtr curr = *prev; // Note must read curr->next before calling `f` as `f` is allowed to // mutate that field. - T* next = curr->next; - if (f(curr)) + NextPtr next = curr->next; + if (f(static_cast(curr))) { // Remove element; *prev = next; @@ -165,7 +184,7 @@ namespace snmalloc */ SNMALLOC_FAST_PATH const T* peek() { - return v.head; + return static_cast(v.head); } }; } // namespace snmalloc diff --git a/src/mem/corealloc.h b/src/mem/corealloc.h index 40e08a3..a1fc5c5 100644 --- a/src/mem/corealloc.h +++ b/src/mem/corealloc.h @@ -3,7 +3,7 @@ #include "../ds/defines.h" #include "allocconfig.h" #include "localcache.h" -#include "metaslab.h" +#include "metadata.h" #include "pool.h" #include "remotecache.h" #include "sizeclasstable.h" @@ -11,16 +11,6 @@ namespace snmalloc { - /** - * Empty class used as the superclass for `CoreAllocator` when it does not - * opt into pool allocation. This class exists because `std::conditional` - * (or other equivalent features in C++) can choose between options for - * superclasses but they cannot choose whether a class has a superclass. - * Setting the superclass to an empty class is equivalent to no superclass. - */ - class NotPoolAllocated - {}; - /** * The core, stateful, part of a memory allocator. Each `LocalAllocator` * owns one `CoreAllocator` once it is initialised. @@ -43,19 +33,39 @@ namespace snmalloc * provided externally, then it must be set explicitly with * `init_message_queue`. */ - template + template class CoreAllocator : public std::conditional_t< - SharedStateHandle::Options.CoreAllocIsPoolAllocated, - Pooled>, - NotPoolAllocated> + Backend::Options.CoreAllocIsPoolAllocated, + Pooled>, + Empty> { - template + template friend class LocalAllocator; + /** + * Define local names for specialised versions of various types that are + * specialised for the back-end that we are using. + * @{ + */ + using BackendSlabMetadata = typename Backend::SlabMetadata; + using PagemapEntry = typename Backend::Pagemap::Entry; + /// }@ + /** * Per size class list of active slabs for this allocator. */ - MetaslabCache alloc_classes[NUM_SMALL_SIZECLASSES]; + struct SlabMetadataCache + { +#ifdef SNMALLOC_CHECK_CLIENT + SeqSet available; +#else + // This is slightly faster in some cases, + // but makes memory reuse more predictable. + SeqSet available; +#endif + uint16_t unused = 0; + uint16_t length = 0; + } alloc_classes[NUM_SMALL_SIZECLASSES]; /** * Local entropy source and current version of keys for @@ -68,7 +78,7 @@ namespace snmalloc * allocator */ std::conditional_t< - SharedStateHandle::Options.IsQueueInline, + Backend::Options.IsQueueInline, RemoteAllocator, RemoteAllocator*> remote_alloc; @@ -76,7 +86,7 @@ namespace snmalloc /** * The type used local state. This is defined by the back end. */ - using LocalState = typename SharedStateHandle::LocalState; + using LocalState = typename Backend::LocalState; /** * A local area of address space managed by this allocator. @@ -85,7 +95,7 @@ namespace snmalloc * externally. */ std::conditional_t< - SharedStateHandle::Options.CoreAllocOwnsLocalState, + Backend::Options.CoreAllocOwnsLocalState, LocalState, LocalState*> backend_state; @@ -99,7 +109,7 @@ namespace snmalloc /** * Ticker to query the clock regularly at a lower cost. */ - Ticker ticker; + Ticker ticker; /** * The message queue needs to be accessible from other threads @@ -109,7 +119,7 @@ namespace snmalloc */ auto* public_state() { - if constexpr (SharedStateHandle::Options.IsQueueInline) + if constexpr (Backend::Options.IsQueueInline) { return &remote_alloc; } @@ -124,7 +134,7 @@ namespace snmalloc */ LocalState* backend_state_ptr() { - if constexpr (SharedStateHandle::Options.CoreAllocOwnsLocalState) + if constexpr (Backend::Options.CoreAllocOwnsLocalState) { return &backend_state; } @@ -186,10 +196,10 @@ namespace snmalloc SNMALLOC_ASSERT(attached_cache != nullptr); auto domesticate = [this](freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { - return capptr_domesticate(backend_state_ptr(), p); + return capptr_domesticate(backend_state_ptr(), p); }; // Use attached cache, and fill it if it is empty. - return attached_cache->template alloc( + return attached_cache->template alloc( domesticate, size, [&](smallsizeclass_t sizeclass, freelist::Iter<>* fl) { @@ -199,7 +209,7 @@ namespace snmalloc static SNMALLOC_FAST_PATH void alloc_new_list( capptr::Chunk& bumpptr, - Metaslab* meta, + BackendSlabMetadata* meta, size_t rsize, size_t slab_size, LocalEntropy& entropy) @@ -281,17 +291,18 @@ namespace snmalloc bumpptr = slab_end; } - void clear_slab(Metaslab* meta, smallsizeclass_t sizeclass) + capptr::Alloc + clear_slab(BackendSlabMetadata* meta, smallsizeclass_t sizeclass) { auto& key = entropy.get_free_list_key(); freelist::Iter<> fl; auto more = meta->free_queue.close(fl, key); UNUSED(more); auto local_state = backend_state_ptr(); - auto domesticate = - [local_state](freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { - return capptr_domesticate(local_state, p); - }; + auto domesticate = [local_state](freelist::QueuePtr p) + SNMALLOC_FAST_PATH_LAMBDA { + return capptr_domesticate(local_state, p); + }; capptr::Alloc p = finish_alloc_no_zero(fl.take(key, domesticate), sizeclass); @@ -328,17 +339,13 @@ namespace snmalloc auto start_of_slab = pointer_align_down( p, snmalloc::sizeclass_to_slab_size(sizeclass)); - SNMALLOC_ASSERT( - address_cast(start_of_slab) == meta->meta_common.chunk_address()); - #if defined(__CHERI_PURE_CAPABILITY__) && !defined(SNMALLOC_CHECK_CLIENT) // Zero the whole slab. For CHERI we at least need to clear the freelist // pointers to avoid leaking capabilities but we do not need to do it in // the freelist order as for SNMALLOC_CHECK_CLIENT. Zeroing the whole slab // may be more friendly to hw because it does not involve pointer chasing // and is amenable to prefetching. - meta->meta_common.template zero_chunk( - snmalloc::sizeclass_to_slab_size(sizeclass)); + // FIXME: This should be a back-end method guarded on a feature flag. #endif #ifdef SNMALLOC_TRACING @@ -346,21 +353,18 @@ namespace snmalloc "Slab {} is unused, Object sizeclass {}", start_of_slab.unsafe_ptr(), sizeclass); -#else - UNUSED(start_of_slab); #endif + return start_of_slab; } template SNMALLOC_SLOW_PATH void dealloc_local_slabs(smallsizeclass_t sizeclass) { // Return unused slabs of sizeclass_t back to global allocator - alloc_classes[sizeclass].available.filter([this, - sizeclass](Metaslab* meta) { + alloc_classes[sizeclass].available.filter([this, sizeclass](auto* meta) { auto domesticate = [this](freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { - auto res = - capptr_domesticate(backend_state_ptr(), p); + auto res = capptr_domesticate(backend_state_ptr(), p); #ifdef SNMALLOC_TRACING if (res.unsafe_ptr() != p.unsafe_ptr()) printf( @@ -383,11 +387,12 @@ namespace snmalloc // TODO delay the clear to the next user of the slab, or teardown so // don't touch the cache lines at this point in snmalloc_check_client. - clear_slab(meta, sizeclass); + auto start = clear_slab(meta, sizeclass); - SharedStateHandle::dealloc_chunk( + Backend::dealloc_chunk( get_backend_local_state(), - meta->meta_common, + *meta, + start, sizeclass_to_slab_size(sizeclass)); return true; @@ -400,17 +405,17 @@ namespace snmalloc * by this thread, or handling the final deallocation onto a slab, * so it can be reused by other threads. */ - SNMALLOC_SLOW_PATH void dealloc_local_object_slow(const MetaEntry& entry) + SNMALLOC_SLOW_PATH void + dealloc_local_object_slow(capptr::Alloc p, const PagemapEntry& entry) { // TODO: Handle message queue on this path? - Metaslab* meta = FrontendMetaEntry::get_metaslab(entry); + auto* meta = entry.get_slab_metadata(); if (meta->is_large()) { // Handle large deallocation here. - size_t entry_sizeclass = - FrontendMetaEntry::get_sizeclass(entry).as_large(); + size_t entry_sizeclass = entry.get_sizeclass().as_large(); size_t size = bits::one_at_bit(entry_sizeclass); #ifdef SNMALLOC_TRACING @@ -419,14 +424,12 @@ namespace snmalloc UNUSED(size); #endif - SharedStateHandle::dealloc_chunk( - get_backend_local_state(), meta->meta_common, size); + Backend::dealloc_chunk(get_backend_local_state(), *meta, p, size); return; } - smallsizeclass_t sizeclass = - FrontendMetaEntry::get_sizeclass(entry).as_small(); + smallsizeclass_t sizeclass = entry.get_sizeclass().as_small(); UNUSED(entropy); if (meta->is_sleeping()) @@ -479,25 +482,25 @@ namespace snmalloc { bool need_post = false; auto local_state = backend_state_ptr(); - auto domesticate = - [local_state](freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { - return capptr_domesticate(local_state, p); - }; + auto domesticate = [local_state](freelist::QueuePtr p) + SNMALLOC_FAST_PATH_LAMBDA { + return capptr_domesticate(local_state, p); + }; auto cb = [this, &need_post](freelist::HeadPtr msg) SNMALLOC_FAST_PATH_LAMBDA { #ifdef SNMALLOC_TRACING message<1024>("Handling remote"); #endif - auto& entry = SharedStateHandle::Pagemap::template get_metaentry( - snmalloc::address_cast(msg)); + auto& entry = + Backend::Pagemap::template get_metaentry(snmalloc::address_cast(msg)); handle_dealloc_remote(entry, msg.as_void(), need_post); return true; }; - if constexpr (SharedStateHandle::Options.QueueHeadsAreTame) + if constexpr (Backend::Options.QueueHeadsAreTame) { /* * The front of the queue has already been validated; just change the @@ -529,7 +532,7 @@ namespace snmalloc * need_post will be set to true, if capacity is exceeded. */ void handle_dealloc_remote( - const MetaEntry& entry, + const PagemapEntry& entry, CapPtr p, bool& need_post) { @@ -537,14 +540,13 @@ namespace snmalloc // TODO this needs to not double revoke if using MTE // TODO thread capabilities? - if (SNMALLOC_LIKELY( - FrontendMetaEntry::get_remote(entry) == public_state())) + if (SNMALLOC_LIKELY(entry.get_remote() == public_state())) { if (SNMALLOC_LIKELY( dealloc_local_object_fast(entry, p.as_void(), entropy))) return; - dealloc_local_object_slow(entry); + dealloc_local_object_slow(p, entry); } else { @@ -554,9 +556,7 @@ namespace snmalloc need_post = true; attached_cache->remote_dealloc_cache .template dealloc( - FrontendMetaEntry::get_remote(entry)->trunc_id(), - p.as_void(), - key_global); + entry.get_remote()->trunc_id(), p.as_void(), key_global); } } @@ -572,12 +572,12 @@ namespace snmalloc // Entropy must be first, so that all data-structures can use the key // it generates. // This must occur before any freelists are constructed. - entropy.init(); + entropy.init(); // Ignoring stats for now. // stats().start(); - if constexpr (SharedStateHandle::Options.IsQueueInline) + if constexpr (Backend::Options.IsQueueInline) { init_message_queue(); message_queue().invariant(); @@ -607,7 +607,7 @@ namespace snmalloc * SFINAE disabled if the allocator does not own the local state. */ template< - typename Config = SharedStateHandle, + typename Config = Backend, typename = std::enable_if_t> CoreAllocator(LocalCache* cache) : attached_cache(cache) { @@ -619,7 +619,7 @@ namespace snmalloc * state. SFINAE disabled if the allocator does own the local state. */ template< - typename Config = SharedStateHandle, + typename Config = Backend, typename = std::enable_if_t> CoreAllocator(LocalCache* cache, LocalState* backend = nullptr) : backend_state(backend), attached_cache(cache) @@ -631,7 +631,7 @@ namespace snmalloc * If the message queue is not inline, provide it. This will then * configure the message queue for use. */ - template + template std::enable_if_t init_message_queue(RemoteAllocator* q) { remote_alloc = q; @@ -650,7 +650,7 @@ namespace snmalloc // stats().remote_post(); // TODO queue not in line! bool sent_something = attached_cache->remote_dealloc_cache - .post( + .post( backend_state_ptr(), public_state()->trunc_id(), key_global); return sent_something; @@ -672,27 +672,27 @@ namespace snmalloc SNMALLOC_FAST_PATH void dealloc_local_object(CapPtr p) { - // MetaEntry-s seen here are expected to have meaningful Remote pointers - auto& entry = SharedStateHandle::Pagemap::template get_metaentry( - snmalloc::address_cast(p)); + // PagemapEntry-s seen here are expected to have meaningful Remote + // pointers + auto& entry = + Backend::Pagemap::template get_metaentry(snmalloc::address_cast(p)); if (SNMALLOC_LIKELY(dealloc_local_object_fast(entry, p, entropy))) return; - dealloc_local_object_slow(entry); + dealloc_local_object_slow(p, entry); } SNMALLOC_FAST_PATH static bool dealloc_local_object_fast( - const MetaEntry& entry, + const PagemapEntry& entry, CapPtr p, LocalEntropy& entropy) { - auto meta = FrontendMetaEntry::get_metaslab(entry); + auto meta = entry.get_slab_metadata(); SNMALLOC_ASSERT(!meta->is_unused()); snmalloc_check_client( - is_start_of_object( - FrontendMetaEntry::get_sizeclass(entry), address_cast(p)), + is_start_of_object(entry.get_sizeclass(), address_cast(p)), "Not deallocating start of an object"); auto cp = p.as_static>(); @@ -734,11 +734,11 @@ namespace snmalloc if (meta->needed() == 0) alloc_classes[sizeclass].unused--; - auto domesticate = [this]( - freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { - return capptr_domesticate(backend_state_ptr(), p); - }; - auto [p, still_active] = Metaslab::alloc_free_list( + auto domesticate = + [this](freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { + return capptr_domesticate(backend_state_ptr(), p); + }; + auto [p, still_active] = BackendSlabMetadata::alloc_free_list( domesticate, meta, fast_free_list, entropy, sizeclass); if (still_active) @@ -747,7 +747,7 @@ namespace snmalloc sl.insert(meta); } - auto r = finish_alloc(p, sizeclass); + auto r = finish_alloc(p, sizeclass); return ticker.check_tick(r); } return small_alloc_slow(sizeclass, fast_free_list); @@ -760,7 +760,7 @@ namespace snmalloc SNMALLOC_FAST_PATH LocalState& get_backend_local_state() { - if constexpr (SharedStateHandle::Options.CoreAllocOwnsLocalState) + if constexpr (Backend::Options.CoreAllocOwnsLocalState) { return backend_state; } @@ -784,10 +784,10 @@ namespace snmalloc message<1024>("small_alloc_slow rsize={} slab size={}", rsize, slab_size); #endif - auto [slab, meta] = SharedStateHandle::alloc_chunk( + auto [slab, meta] = Backend::alloc_chunk( get_backend_local_state(), slab_size, - FrontendMetaEntry::encode( + PagemapEntry::encode( public_state(), sizeclass_t::from_small_class(sizeclass))); if (slab == nullptr) @@ -803,9 +803,9 @@ namespace snmalloc auto domesticate = [this](freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { - return capptr_domesticate(backend_state_ptr(), p); + return capptr_domesticate(backend_state_ptr(), p); }; - auto [p, still_active] = Metaslab::alloc_free_list( + auto [p, still_active] = BackendSlabMetadata::alloc_free_list( domesticate, meta, fast_free_list, entropy, sizeclass); if (still_active) @@ -814,7 +814,7 @@ namespace snmalloc alloc_classes[sizeclass].available.insert(meta); } - auto r = finish_alloc(p, sizeclass); + auto r = finish_alloc(p, sizeclass); return ticker.check_tick(r); } @@ -827,10 +827,10 @@ namespace snmalloc { SNMALLOC_ASSERT(attached_cache != nullptr); auto local_state = backend_state_ptr(); - auto domesticate = - [local_state](freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { - return capptr_domesticate(local_state, p); - }; + auto domesticate = [local_state](freelist::QueuePtr p) + SNMALLOC_FAST_PATH_LAMBDA { + return capptr_domesticate(local_state, p); + }; if (destroy_queue) { @@ -841,8 +841,8 @@ namespace snmalloc { bool need_post = true; // Always going to post, so ignore. auto n_tame = p_tame->atomic_read_next(key_global, domesticate); - const MetaEntry& entry = SharedStateHandle::Pagemap::get_metaentry( - snmalloc::address_cast(p_tame)); + const PagemapEntry& entry = + Backend::Pagemap::get_metaentry(snmalloc::address_cast(p_tame)); handle_dealloc_remote(entry, p_tame.as_void(), need_post); p_tame = n_tame; } @@ -855,10 +855,9 @@ namespace snmalloc handle_message_queue([]() {}); } - auto posted = - attached_cache->flush( - backend_state_ptr(), - [&](capptr::Alloc p) { dealloc_local_object(p); }); + auto posted = attached_cache->flush( + backend_state_ptr(), + [&](capptr::Alloc p) { dealloc_local_object(p); }); // We may now have unused slabs, return to the global allocator. for (smallsizeclass_t sizeclass = 0; sizeclass < NUM_SMALL_SIZECLASSES; @@ -896,8 +895,8 @@ namespace snmalloc bool debug_is_empty_impl(bool* result) { auto test = [&result](auto& queue) { - queue.filter([&result](auto metaslab) { - if (metaslab->needed() != 0) + queue.filter([&result](auto slab_metadata) { + if (slab_metadata->needed() != 0) { if (result != nullptr) *result = false; @@ -963,9 +962,6 @@ namespace snmalloc /** * Use this alias to access the pool of allocators throughout snmalloc. */ - template - using AllocPool = Pool< - CoreAllocator, - SharedStateHandle, - SharedStateHandle::pool>; + template + using AllocPool = Pool, Backend, Backend::pool>; } // namespace snmalloc diff --git a/src/mem/localalloc.h b/src/mem/localalloc.h index 2760488..bbe0781 100644 --- a/src/mem/localalloc.h +++ b/src/mem/localalloc.h @@ -59,20 +59,27 @@ namespace snmalloc * core allocator must be provided externally by invoking the `init` method * on this class *before* any allocation-related methods are called. */ - template + template class LocalAllocator { public: - using StateHandle = SharedStateHandle; + using StateHandle = Backend; private: - using CoreAlloc = CoreAllocator; + /** + * Define local names for specialised versions of various types that are + * specialised for the back-end that we are using. + * @{ + */ + using CoreAlloc = CoreAllocator; + using PagemapEntry = typename Backend::Pagemap::Entry; + /// }@ // Free list per small size class. These are used for // allocation on the fast path. This part of the code is inspired by // mimalloc. // Also contains remote deallocation cache. - LocalCache local_cache{&SharedStateHandle::unused_remote}; + LocalCache local_cache{&Backend::unused_remote}; // Underlying allocator for most non-fast path operations. CoreAlloc* core_alloc{nullptr}; @@ -116,7 +123,7 @@ namespace snmalloc SNMALLOC_SLOW_PATH decltype(auto) lazy_init(Action action, Args... args) { SNMALLOC_ASSERT(core_alloc == nullptr); - if constexpr (!SharedStateHandle::Options.LocalAllocSupportsLazyInit) + if constexpr (!Backend::Options.LocalAllocSupportsLazyInit) { SNMALLOC_CHECK( false && @@ -129,7 +136,7 @@ namespace snmalloc else { // Initialise the thread local allocator - if constexpr (SharedStateHandle::Options.CoreAllocOwnsLocalState) + if constexpr (Backend::Options.CoreAllocOwnsLocalState) { init(); } @@ -141,7 +148,7 @@ namespace snmalloc // Must be called at least once per thread. // A pthread implementation only calls the thread destruction handle // if the key has been set. - SharedStateHandle::register_clean_up(); + Backend::register_clean_up(); // Perform underlying operation auto r = action(core_alloc, args...); @@ -180,10 +187,10 @@ namespace snmalloc return check_init([&](CoreAlloc* core_alloc) { // Grab slab of correct size // Set remote as large allocator remote. - auto [chunk, meta] = SharedStateHandle::alloc_chunk( + auto [chunk, meta] = Backend::alloc_chunk( core_alloc->get_backend_local_state(), large_size_to_chunk_size(size), - FrontendMetaEntry::encode( + PagemapEntry::encode( core_alloc->public_state(), size_to_sizeclass_full(size))); // set up meta data so sizeclass is correct, and hence alloc size, and // external pointer. @@ -197,7 +204,7 @@ namespace snmalloc if (zero_mem == YesZero && chunk.unsafe_ptr() != nullptr) { - SharedStateHandle::Pal::template zero( + Backend::Pal::template zero( chunk.unsafe_ptr(), bits::next_pow2(size)); } @@ -208,11 +215,10 @@ namespace snmalloc template SNMALLOC_FAST_PATH capptr::Alloc small_alloc(size_t size) { - auto domesticate = [this](freelist::QueuePtr p) - SNMALLOC_FAST_PATH_LAMBDA { - return capptr_domesticate( - core_alloc->backend_state_ptr(), p); - }; + auto domesticate = [this]( + freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { + return capptr_domesticate(core_alloc->backend_state_ptr(), p); + }; auto slowpath = [&]( smallsizeclass_t sizeclass, freelist::Iter<>* fl) SNMALLOC_FAST_PATH_LAMBDA { @@ -236,7 +242,7 @@ namespace snmalloc sizeclass); }; - return local_cache.template alloc( + return local_cache.template alloc( domesticate, size, slowpath); } @@ -267,10 +273,10 @@ namespace snmalloc p.unsafe_ptr(), alloc_size(p.unsafe_ptr())); #endif - const MetaEntry& entry = - SharedStateHandle::Pagemap::get_metaentry(address_cast(p)); + const PagemapEntry& entry = + Backend::Pagemap::get_metaentry(address_cast(p)); local_cache.remote_dealloc_cache.template dealloc( - FrontendMetaEntry::get_remote(entry)->trunc_id(), p, key_global); + entry.get_remote()->trunc_id(), p, key_global); post_remote_cache(); return; } @@ -297,13 +303,13 @@ namespace snmalloc } /** - * Call `SharedStateHandle::is_initialised()` if it is implemented, + * Call `Backend::is_initialised()` if it is implemented, * unconditionally returns true otherwise. */ SNMALLOC_FAST_PATH bool is_initialised() { - return call_is_initialised(nullptr, 0); + return call_is_initialised(nullptr, 0); } /** @@ -326,13 +332,13 @@ namespace snmalloc {} /** - * Call `SharedStateHandle::ensure_init()` if it is implemented, do + * Call `Backend::ensure_init()` if it is implemented, do * nothing otherwise. */ SNMALLOC_FAST_PATH void ensure_init() { - call_ensure_init(nullptr, 0); + call_ensure_init(nullptr, 0); } public: @@ -377,7 +383,7 @@ namespace snmalloc // Initialise the global allocator structures ensure_init(); // Grab an allocator for this thread. - init(AllocPool::acquire(&(this->local_cache))); + init(AllocPool::acquire(&(this->local_cache))); } // Return all state in the fast allocator and release the underlying @@ -397,9 +403,9 @@ namespace snmalloc // Detach underlying allocator core_alloc->attached_cache = nullptr; // Return underlying allocator to the system. - if constexpr (SharedStateHandle::Options.CoreAllocOwnsLocalState) + if constexpr (Backend::Options.CoreAllocOwnsLocalState) { - AllocPool::release(core_alloc); + AllocPool::release(core_alloc); } // Set up thread local allocator to look like @@ -408,7 +414,7 @@ namespace snmalloc #ifdef SNMALLOC_TRACING message<1024>("flush(): core_alloc={}", core_alloc); #endif - local_cache.remote_allocator = &SharedStateHandle::unused_remote; + local_cache.remote_allocator = &Backend::unused_remote; local_cache.remote_dealloc_cache.capacity = 0; } } @@ -621,14 +627,12 @@ namespace snmalloc * well-formedness) of this pointer. The remainder of the logic will * deal with the object's extent. */ - capptr::Alloc p_tame = capptr_domesticate( - core_alloc->backend_state_ptr(), p_wild); + capptr::Alloc p_tame = + capptr_domesticate(core_alloc->backend_state_ptr(), p_wild); - const MetaEntry& entry = - SharedStateHandle::Pagemap::get_metaentry(address_cast(p_tame)); - if (SNMALLOC_LIKELY( - local_cache.remote_allocator == - FrontendMetaEntry::get_remote(entry))) + const PagemapEntry& entry = + Backend::Pagemap::get_metaentry(address_cast(p_tame)); + if (SNMALLOC_LIKELY(local_cache.remote_allocator == entry.get_remote())) { # if defined(__CHERI_PURE_CAPABILITY__) && defined(SNMALLOC_CHECK_CLIENT) dealloc_cheri_checks(p_tame.unsafe_ptr()); @@ -636,11 +640,11 @@ namespace snmalloc if (SNMALLOC_LIKELY(CoreAlloc::dealloc_local_object_fast( entry, p_tame, local_cache.entropy))) return; - core_alloc->dealloc_local_object_slow(entry); + core_alloc->dealloc_local_object_slow(p_tame, entry); return; } - RemoteAllocator* remote = FrontendMetaEntry::get_remote(entry); + RemoteAllocator* remote = entry.get_remote(); if (SNMALLOC_LIKELY(remote != nullptr)) { # if defined(__CHERI_PURE_CAPABILITY__) && defined(SNMALLOC_CHECK_CLIENT) @@ -706,7 +710,7 @@ namespace snmalloc #else // TODO What's the domestication policy here? At the moment we just // probe the pagemap with the raw address, without checks. There could - // be implicit domestication through the `SharedStateHandle::Pagemap` or + // be implicit domestication through the `Backend::Pagemap` or // we could just leave well enough alone. // Note that alloc_size should return 0 for nullptr. @@ -716,10 +720,10 @@ namespace snmalloc // To handle this case we require the uninitialised pagemap contain an // entry for the first chunk of memory, that states it represents a // large object, so we can pull the check for null off the fast path. - const MetaEntry& entry = - SharedStateHandle::Pagemap::get_metaentry(address_cast(p_raw)); + const PagemapEntry& entry = + Backend::Pagemap::get_metaentry(address_cast(p_raw)); - return sizeclass_full_to_size(FrontendMetaEntry::get_sizeclass(entry)); + return sizeclass_full_to_size(entry.get_sizeclass()); #endif } @@ -761,11 +765,10 @@ namespace snmalloc size_t remaining_bytes(const void* p) { #ifndef SNMALLOC_PASS_THROUGH - const MetaEntry& entry = - SharedStateHandle::Pagemap::template get_metaentry( - address_cast(p)); + const PagemapEntry& entry = + Backend::Pagemap::template get_metaentry(address_cast(p)); - auto sizeclass = FrontendMetaEntry::get_sizeclass(entry); + auto sizeclass = entry.get_sizeclass(); return snmalloc::remaining_bytes(sizeclass, address_cast(p)); #else return pointer_diff(p, reinterpret_cast(UINTPTR_MAX)); @@ -774,7 +777,7 @@ namespace snmalloc bool check_bounds(const void* p, size_t s) { - if (SNMALLOC_LIKELY(SharedStateHandle::Pagemap::is_initialised())) + if (SNMALLOC_LIKELY(Backend::Pagemap::is_initialised())) { return remaining_bytes(p) >= s; } @@ -790,11 +793,10 @@ namespace snmalloc size_t index_in_object(const void* p) { #ifndef SNMALLOC_PASS_THROUGH - const MetaEntry& entry = - SharedStateHandle::Pagemap::template get_metaentry( - address_cast(p)); + const PagemapEntry& entry = + Backend::Pagemap::template get_metaentry(address_cast(p)); - auto sizeclass = FrontendMetaEntry::get_sizeclass(entry); + auto sizeclass = entry.get_sizeclass(); return snmalloc::index_in_object(sizeclass, address_cast(p)); #else return reinterpret_cast(p); diff --git a/src/mem/metadata.h b/src/mem/metadata.h new file mode 100644 index 0000000..765e298 --- /dev/null +++ b/src/mem/metadata.h @@ -0,0 +1,636 @@ +#pragma once + +#include "../backend/backend_concept.h" +#include "../ds/helpers.h" +#include "../ds/seqset.h" +#include "freelist.h" +#include "sizeclasstable.h" + +namespace snmalloc +{ + struct RemoteAllocator; + + /** + * Remotes need to be aligned enough that the bottom bits have enough room for + * all the size classes, both large and small. An additional bit is required + * to separate backend uses. + */ + static constexpr size_t REMOTE_MIN_ALIGN = + bits::max(CACHELINE_SIZE, SIZECLASS_REP_SIZE) << 1; + + /** + * Base class for the templated FrontendMetaEntry. This exists to avoid + * needing a template parameter to access constants that are independent of + * the template parameter and contains all of the state that is agnostic to + * the types used for storing per-slab metadata. This class should never be + * instantiated directly (and its protected constructor guarantees that), + * only the templated subclass should be use. The subclass provides + * convenient accessors. + * + * A back end may also subclass `FrontendMetaEntry` to provide other + * back-end-specific information. The front end never directly instantiates + * these. + */ + class MetaEntryBase + { + protected: + /** + * This bit is set in remote_and_sizeclass to discriminate between the case + * that it is in use by the frontend (0) or by the backend (1). For the + * former case, see other methods on this and the subclass + * `FrontendMetaEntry`; for the latter, see backend/backend.h and + * backend/largebuddyrange.h. + * + * This value is statically checked by the frontend to ensure that its + * bit packing does not conflict; see mem/remoteallocator.h + */ + static constexpr address_t REMOTE_BACKEND_MARKER = 1 << 7; + + /** + * Bit used to indicate this should not be considered part of the previous + * PAL allocation. + * + * Some platforms cannot treat different PalAllocs as a single allocation. + * This is true on CHERI as the combined permission might not be + * representable. It is also true on Windows as you cannot Commit across + * multiple continuous VirtualAllocs. + */ + static constexpr address_t META_BOUNDARY_BIT = 1 << 0; + + /** + * The bit above the sizeclass is always zero unless this is used + * by the backend to represent another datastructure such as the buddy + * allocator entries. + */ + static constexpr size_t REMOTE_WITH_BACKEND_MARKER_ALIGN = + MetaEntryBase::REMOTE_BACKEND_MARKER; + static_assert( + (REMOTE_MIN_ALIGN >> 1) == MetaEntryBase::REMOTE_BACKEND_MARKER); + + /** + * In common cases, the pointer to the slab metadata. See + * docs/AddressSpace.md for additional details. + * + * The bottom bit is used to indicate if this is the first chunk in a PAL + * allocation, that cannot be combined with the preceeding chunk. + */ + uintptr_t meta{0}; + + /** + * In common cases, a bit-packed pointer to the owning allocator (if any), + * and the sizeclass of this chunk. See `encode` for + * details of this case and docs/AddressSpace.md for further details. + */ + uintptr_t remote_and_sizeclass{0}; + + /** + * Constructor from two pointer-sized words. The subclass is responsible + * for ensuring that accesses to these are type-safe. + */ + constexpr MetaEntryBase(uintptr_t m, uintptr_t ras) + : meta(m), remote_and_sizeclass(ras) + {} + + /** + * Default constructor, zero initialises. + */ + constexpr MetaEntryBase() : MetaEntryBase(0, 0) {} + + /** + * When a meta entry is in use by the back end, it exposes two words of + * state. The low bits in both are reserved. Bits in this bitmask must + * not be set by the back end in either word. + * + * During a major release, this constraint may be weakened, allowing the + * back end to set more bits. We don't currently use all of these bits in + * both words, but we reserve them all to make access uniform. If more + * bits are required by a back end then we could make this asymmetric. + * + * `REMOTE_BACKEND_MARKER` is the highest bit that we reserve, so this is + * currently every bit including that bit and all lower bits. + */ + static constexpr address_t BACKEND_RESERVED_MASK = + (REMOTE_BACKEND_MARKER << 1) - 1; + + public: + /** + * Does the back end currently own this entry? Note that freshly + * allocated entries are owned by the front end until explicitly + * claimed by the back end and so this will return `false` if neither + * the front nor back end owns this entry. + */ + [[nodiscard]] bool is_backend_owned() const + { + return (REMOTE_BACKEND_MARKER & remote_and_sizeclass) == + REMOTE_BACKEND_MARKER; + } + + /** + * Returns true if this metaentry has not been claimed by the front or back + * ends. + */ + [[nodiscard]] bool is_unowned() const + { + return (meta == 0) && (remote_and_sizeclass == 0); + } + + /** + * Encode the remote and the sizeclass. + */ + [[nodiscard]] static SNMALLOC_FAST_PATH uintptr_t + encode(RemoteAllocator* remote, sizeclass_t sizeclass) + { + /* remote might be nullptr; cast to uintptr_t before offsetting */ + return pointer_offset( + reinterpret_cast(remote), sizeclass.raw()); + } + + /** + * Return the remote and sizeclass in an implementation-defined encoding. + * This is not guaranteed to be stable across snmalloc releases and so the + * only safe use for this is to pass it to the two-argument constructor of + * this class. + */ + [[nodiscard]] SNMALLOC_FAST_PATH uintptr_t get_remote_and_sizeclass() const + { + return remote_and_sizeclass; + } + + /** + * Explicit assignment operator, copies the data preserving the boundary bit + * in the target if it is set. + */ + MetaEntryBase& operator=(const MetaEntryBase& other) + { + // Don't overwrite the boundary bit with the other's + meta = (other.meta & ~META_BOUNDARY_BIT) | + address_cast(meta & META_BOUNDARY_BIT); + remote_and_sizeclass = other.remote_and_sizeclass; + return *this; + } + + /** + * On some platforms, allocations originating from the OS may not be + * combined. The boundary bit indicates whether this is meta entry + * corresponds to the first chunk in such a range and so may not be combined + * with anything before it in the address space. + * @{ + */ + void set_boundary() + { + meta |= META_BOUNDARY_BIT; + } + + [[nodiscard]] bool is_boundary() const + { + return meta & META_BOUNDARY_BIT; + } + + bool clear_boundary_bit() + { + return meta &= ~META_BOUNDARY_BIT; + } + ///@} + + /** + * Returns the remote. + * + * If the meta entry is owned by the back end then this returns an + * undefined value and will abort in debug builds. + */ + [[nodiscard]] SNMALLOC_FAST_PATH RemoteAllocator* get_remote() const + { + SNMALLOC_ASSERT(!is_backend_owned()); + return reinterpret_cast( + pointer_align_down( + get_remote_and_sizeclass())); + } + + /** + * Return the sizeclass. + * + * This can be called irrespective of whether the corresponding meta entry + * is owned by the front or back end (and is, for example, called by + * `external_pointer`). In the future, it may provide some stronger + * guarantees on the value that is returned in this case. + */ + [[nodiscard]] SNMALLOC_FAST_PATH sizeclass_t get_sizeclass() const + { + // TODO: perhaps remove static_cast with resolution of + // https://github.com/CTSRD-CHERI/llvm-project/issues/588 + return sizeclass_t::from_raw( + static_cast(get_remote_and_sizeclass()) & + (REMOTE_WITH_BACKEND_MARKER_ALIGN - 1)); + } + + /** + * Claim the meta entry for use by the back end. This preserves the + * boundary bit, if it is set, but otherwise resets the meta entry to a + * pristine state. + */ + void claim_for_backend() + { + meta = is_boundary() ? META_BOUNDARY_BIT : 0; + remote_and_sizeclass = REMOTE_BACKEND_MARKER; + } + + /** + * When used by the back end, the two words in a meta entry have no + * semantics defined by the front end and are identified by enumeration + * values. + */ + enum class Word + { + /** + * The first word. + */ + One, + + /** + * The second word. + */ + Two + }; + + static constexpr bool is_backend_allowed_value(Word, uintptr_t val) + { + return (val & BACKEND_RESERVED_MASK) == 0; + } + + /** + * Proxy class that allows setting and reading back the bits in each word + * that are exposed for the back end. + * + * The back end must not keep instances of this class after returning the + * corresponding meta entry to the front end. + */ + class BackendStateWordRef + { + /** + * A pointer to the relevant word. + */ + uintptr_t* val; + + public: + /** + * Constructor, wraps a `uintptr_t`. Note that this may be used outside + * of the meta entry by code wishing to provide uniform storage to things + * that are either in a meta entry or elsewhere. + */ + constexpr BackendStateWordRef(uintptr_t* v) : val(v) {} + + /** + * Copy constructor. Aliases the underlying storage. Note that this is + * not thread safe: two `BackendStateWordRef` instances sharing access to + * the same storage must not be used from different threads without + * explicit synchronisation. + */ + constexpr BackendStateWordRef(const BackendStateWordRef& other) = default; + + /** + * Read the value. This zeroes any bits in the underlying storage that + * the back end is not permitted to access. + */ + [[nodiscard]] uintptr_t get() const + { + return (*val) & ~BACKEND_RESERVED_MASK; + } + + /** + * Default copy assignment. See the copy constructor for constraints on + * using this. + */ + BackendStateWordRef& + operator=(const BackendStateWordRef& other) = default; + + /** + * Assignment operator. Zeroes the bits in the provided value that the + * back end is not permitted to use and then stores the result in the + * value that this class manages. + */ + BackendStateWordRef& operator=(uintptr_t v) + { + SNMALLOC_ASSERT_MSG( + ((v & BACKEND_RESERVED_MASK) == 0), + "The back end is not permitted to use the low bits in the meta " + "entry. ({} & {}) == {}.", + v, + BACKEND_RESERVED_MASK, + (v & BACKEND_RESERVED_MASK)); + *val = v | (static_cast(*val) & BACKEND_RESERVED_MASK); + return *this; + } + + /** + * Comparison operator. Performs address comparison *not* value + * comparison. + */ + bool operator!=(const BackendStateWordRef& other) const + { + return val != other.val; + } + + /** + * Returns the address of the underlying storage in a form that can be + * passed to `snmalloc::message` for printing. + */ + address_t printable_address() + { + return address_cast(val); + } + }; + + /** + * Get a proxy that allows the back end to read from and write to (some bits + * of) a word in the meta entry. The meta entry must either be unowned or + * explicitly claimed by the back end before calling this. + */ + BackendStateWordRef get_backend_word(Word w) + { + if (!is_backend_owned()) + { + SNMALLOC_ASSERT_MSG( + is_unowned(), + "Meta entry is owned by the front end. Meta: {}, " + "remote_and_sizeclass:{}", + meta, + remote_and_sizeclass); + claim_for_backend(); + } + return {w == Word::One ? &meta : &remote_and_sizeclass}; + } + }; + + /** + * The FrontendSlabMetadata represent the metadata associated with a single + * slab. + */ + class alignas(CACHELINE_SIZE) FrontendSlabMetadata + { + public: + /** + * Used to link slab metadata together in various other data-structures. + * This is intended to be used with `SeqSet` and so may actually hold a + * subclass of this class provided by the back end. The `SeqSet` is + * responsible for maintaining that invariant. While an instance of this + * class is in a `SeqSet`, the `next` field should not be assigned to by + * anything that doesn't enforce the invariant that `next` stores a `T*`, + * where `T` is a subclass of `FrontendSlabMetadata`. + */ + FrontendSlabMetadata* next{nullptr}; + + constexpr FrontendSlabMetadata() = default; + + /** + * Data-structure for building the free list for this slab. + */ +#ifdef SNMALLOC_CHECK_CLIENT + freelist::Builder free_queue; +#else + freelist::Builder free_queue; +#endif + + /** + * The number of deallocation required until we hit a slow path. This + * counts down in two different ways that are handled the same on the + * fast path. The first is + * - deallocations until the slab has sufficient entries to be considered + * useful to allocate from. This could be as low as 1, or when we have + * a requirement for entropy then it could be much higher. + * - deallocations until the slab is completely unused. This is needed + * to be detected, so that the statistics can be kept up to date, and + * potentially return memory to the a global pool of slabs/chunks. + */ + uint16_t needed_ = 0; + + /** + * Flag that is used to indicate that the slab is currently not active. + * I.e. it is not in a CoreAllocator cache for the appropriate sizeclass. + */ + bool sleeping_ = false; + + /** + * Flag to indicate this is actually a large allocation rather than a slab + * of small allocations. + */ + bool large_ = false; + + uint16_t& needed() + { + return needed_; + } + + bool& sleeping() + { + return sleeping_; + } + + /** + * Initialise FrontendSlabMetadata for a slab. + */ + void initialise(smallsizeclass_t sizeclass) + { + free_queue.init(); + // Set up meta data as if the entire slab has been turned into a free + // list. This means we don't have to check for special cases where we have + // returned all the elements, but this is a slab that is still being bump + // allocated from. Hence, the bump allocator slab will never be returned + // for use in another size class. + set_sleeping(sizeclass, 0); + + large_ = false; + } + + /** + * Make this a chunk represent a large allocation. + * + * Set needed so immediately moves to slow path. + */ + void initialise_large() + { + // We will push to this just to make the fast path clean. + free_queue.init(); + + // Flag to detect that it is a large alloc on the slow path + large_ = true; + + // Jump to slow path on first deallocation. + needed() = 1; + } + + /** + * Updates statistics for adding an entry to the free list, if the + * slab is either + * - empty adding the entry to the free list, or + * - was full before the subtraction + * this returns true, otherwise returns false. + */ + bool return_object() + { + return (--needed()) == 0; + } + + bool is_unused() + { + return needed() == 0; + } + + bool is_sleeping() + { + return sleeping(); + } + + bool is_large() + { + return large_; + } + + /** + * Try to set this slab metadata to sleep. If the remaining elements are + * fewer than the threshold, then it will actually be set to the sleeping + * state, and will return true, otherwise it will return false. + */ + SNMALLOC_FAST_PATH bool + set_sleeping(smallsizeclass_t sizeclass, uint16_t remaining) + { + auto threshold = threshold_for_waking_slab(sizeclass); + if (remaining >= threshold) + { + // Set needed to at least one, possibly more so we only use + // a slab when it has a reasonable amount of free elements + auto allocated = sizeclass_to_slab_object_count(sizeclass); + needed() = allocated - remaining; + sleeping() = false; + return false; + } + + sleeping() = true; + needed() = threshold - remaining; + return true; + } + + SNMALLOC_FAST_PATH void set_not_sleeping(smallsizeclass_t sizeclass) + { + auto allocated = sizeclass_to_slab_object_count(sizeclass); + needed() = allocated - threshold_for_waking_slab(sizeclass); + + // Design ensures we can't move from full to empty. + // There are always some more elements to free at this + // point. This is because the threshold is always less + // than the count for the slab + SNMALLOC_ASSERT(needed() != 0); + + sleeping() = false; + } + + /** + * Allocates a free list from the meta data. + * + * Returns a freshly allocated object of the correct size, and a bool that + * specifies if the slab metadata should be placed in the queue for that + * sizeclass. + * + * If Randomisation is not used, it will always return false for the second + * component, but with randomisation, it may only return part of the + * available objects for this slab metadata. + */ + template + static SNMALLOC_FAST_PATH std::pair + alloc_free_list( + Domesticator domesticate, + FrontendSlabMetadata* meta, + freelist::Iter<>& fast_free_list, + LocalEntropy& entropy, + smallsizeclass_t sizeclass) + { + auto& key = entropy.get_free_list_key(); + + std::remove_reference_t tmp_fl; + auto remaining = meta->free_queue.close(tmp_fl, key); + auto p = tmp_fl.take(key, domesticate); + fast_free_list = tmp_fl; + +#ifdef SNMALLOC_CHECK_CLIENT + entropy.refresh_bits(); +#else + UNUSED(entropy); +#endif + + // This marks the slab as sleeping, and sets a wakeup + // when sufficient deallocations have occurred to this slab. + // Takes how many deallocations were not grabbed on this call + // This will be zero if there is no randomisation. + auto sleeping = meta->set_sleeping(sizeclass, remaining); + + return {p, !sleeping}; + } + }; + + /** + * Entry stored in the pagemap. See docs/AddressSpace.md for the full + * FrontendMetaEntry lifecycle. + */ + template + class FrontendMetaEntry : public MetaEntryBase + { + /** + * Ensure that the template parameter is valid. + */ + static_assert( + std::is_convertible_v, + "The front end requires that the back end provides slab metadata that is " + "compatible with the front-end's structure"); + + public: + constexpr FrontendMetaEntry() = default; + + /** + * Constructor, provides the remote and sizeclass embedded in a single + * pointer-sized word. This format is not guaranteed to be stable and so + * the second argument of this must always be the return value from + * `get_remote_and_sizeclass`. + */ + SNMALLOC_FAST_PATH + FrontendMetaEntry(BackendSlabMetadata* meta, uintptr_t remote_and_sizeclass) + : MetaEntryBase( + unsafe_to_uintptr(meta), remote_and_sizeclass) + { + SNMALLOC_ASSERT_MSG( + (REMOTE_BACKEND_MARKER & remote_and_sizeclass) == 0, + "Setting a backend-owned value ({}) via the front-end interface is not " + "allowed", + remote_and_sizeclass); + remote_and_sizeclass &= ~REMOTE_BACKEND_MARKER; + } + + /** + * Implicit copying of meta entries is almost certainly a bug and so the + * copy constructor is deleted to statically catch these problems. + */ + FrontendMetaEntry(const FrontendMetaEntry&) = delete; + + /** + * Explicit assignment operator, copies the data preserving the boundary bit + * in the target if it is set. + */ + FrontendMetaEntry& operator=(const FrontendMetaEntry& other) + { + MetaEntryBase::operator=(other); + return *this; + } + + /** + * Return the FrontendSlabMetadata metadata associated with this chunk, + * guarded by an assert that this chunk is being used as a slab (i.e., has + * an associated owning allocator). + */ + [[nodiscard]] SNMALLOC_FAST_PATH BackendSlabMetadata* + get_slab_metadata() const + { + SNMALLOC_ASSERT(get_remote() != nullptr); + return unsafe_from_uintptr( + meta & ~META_BOUNDARY_BIT); + } + }; + +} // namespace snmalloc diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h deleted file mode 100644 index 997784d..0000000 --- a/src/mem/metaslab.h +++ /dev/null @@ -1,280 +0,0 @@ -#pragma once - -#include "../backend/metatypes.h" -#include "../ds/helpers.h" -#include "../ds/seqset.h" -#include "../mem/remoteallocator.h" -#include "freelist.h" -#include "sizeclasstable.h" - -namespace snmalloc -{ - // The Metaslab represent the status of a single slab. - class alignas(CACHELINE_SIZE) Metaslab - { - public: - MetaCommon meta_common; - - // Used to link metaslabs together in various other data-structures. - Metaslab* next{nullptr}; - - constexpr Metaslab() = default; - - /** - * Data-structure for building the free list for this slab. - */ -#ifdef SNMALLOC_CHECK_CLIENT - freelist::Builder free_queue; -#else - freelist::Builder free_queue; -#endif - - /** - * The number of deallocation required until we hit a slow path. This - * counts down in two different ways that are handled the same on the - * fast path. The first is - * - deallocations until the slab has sufficient entries to be considered - * useful to allocate from. This could be as low as 1, or when we have - * a requirement for entropy then it could be much higher. - * - deallocations until the slab is completely unused. This is needed - * to be detected, so that the statistics can be kept up to date, and - * potentially return memory to the a global pool of slabs/chunks. - */ - uint16_t needed_ = 0; - - /** - * Flag that is used to indicate that the slab is currently not active. - * I.e. it is not in a CoreAllocator cache for the appropriate sizeclass. - */ - bool sleeping_ = false; - - /** - * Flag to indicate this is actually a large allocation rather than a slab - * of small allocations. - */ - bool large_ = false; - - uint16_t& needed() - { - return needed_; - } - - bool& sleeping() - { - return sleeping_; - } - - /** - * Initialise Metaslab for a slab. - */ - void initialise(smallsizeclass_t sizeclass) - { - free_queue.init(); - // Set up meta data as if the entire slab has been turned into a free - // list. This means we don't have to check for special cases where we have - // returned all the elements, but this is a slab that is still being bump - // allocated from. Hence, the bump allocator slab will never be returned - // for use in another size class. - set_sleeping(sizeclass, 0); - - large_ = false; - } - - /** - * Make this a chunk represent a large allocation. - * - * Set needed so immediately moves to slow path. - */ - void initialise_large() - { - // We will push to this just to make the fast path clean. - free_queue.init(); - - // Flag to detect that it is a large alloc on the slow path - large_ = true; - - // Jump to slow path on first deallocation. - needed() = 1; - } - - /** - * Updates statistics for adding an entry to the free list, if the - * slab is either - * - empty adding the entry to the free list, or - * - was full before the subtraction - * this returns true, otherwise returns false. - */ - bool return_object() - { - return (--needed()) == 0; - } - - bool is_unused() - { - return needed() == 0; - } - - bool is_sleeping() - { - return sleeping(); - } - - bool is_large() - { - return large_; - } - - /** - * Try to set this metaslab to sleep. If the remaining elements are fewer - * than the threshold, then it will actually be set to the sleeping state, - * and will return true, otherwise it will return false. - */ - SNMALLOC_FAST_PATH bool - set_sleeping(smallsizeclass_t sizeclass, uint16_t remaining) - { - auto threshold = threshold_for_waking_slab(sizeclass); - if (remaining >= threshold) - { - // Set needed to at least one, possibly more so we only use - // a slab when it has a reasonable amount of free elements - auto allocated = sizeclass_to_slab_object_count(sizeclass); - needed() = allocated - remaining; - sleeping() = false; - return false; - } - - sleeping() = true; - needed() = threshold - remaining; - return true; - } - - SNMALLOC_FAST_PATH void set_not_sleeping(smallsizeclass_t sizeclass) - { - auto allocated = sizeclass_to_slab_object_count(sizeclass); - needed() = allocated - threshold_for_waking_slab(sizeclass); - - // Design ensures we can't move from full to empty. - // There are always some more elements to free at this - // point. This is because the threshold is always less - // than the count for the slab - SNMALLOC_ASSERT(needed() != 0); - - sleeping() = false; - } - - /** - * Allocates a free list from the meta data. - * - * Returns a freshly allocated object of the correct size, and a bool that - * specifies if the metaslab should be placed in the queue for that - * sizeclass. - * - * If Randomisation is not used, it will always return false for the second - * component, but with randomisation, it may only return part of the - * available objects for this metaslab. - */ - template - static SNMALLOC_FAST_PATH std::pair - alloc_free_list( - Domesticator domesticate, - Metaslab* meta, - freelist::Iter<>& fast_free_list, - LocalEntropy& entropy, - smallsizeclass_t sizeclass) - { - auto& key = entropy.get_free_list_key(); - - std::remove_reference_t tmp_fl; - auto remaining = meta->free_queue.close(tmp_fl, key); - auto p = tmp_fl.take(key, domesticate); - fast_free_list = tmp_fl; - -#ifdef SNMALLOC_CHECK_CLIENT - entropy.refresh_bits(); -#else - UNUSED(entropy); -#endif - - // This marks the slab as sleeping, and sets a wakeup - // when sufficient deallocations have occurred to this slab. - // Takes how many deallocations were not grabbed on this call - // This will be zero if there is no randomisation. - auto sleeping = meta->set_sleeping(sizeclass, remaining); - - return {p, !sleeping}; - } - }; - -#if defined(USE_METADATA_CONCEPT) - static_assert(ConceptMetadataStruct); -#endif - static_assert( - sizeof(Metaslab) == PAGEMAP_METADATA_STRUCT_SIZE, - "Metaslab is expected to be the largest pagemap metadata record"); - - struct MetaslabCache - { -#ifdef SNMALLOC_CHECK_CLIENT - SeqSet available; -#else - // This is slightly faster in some cases, - // but makes memory reuse more predictable. - SeqSet available; -#endif - uint16_t unused = 0; - uint16_t length = 0; - }; - - /* - * Define the encoding of a RemoteAllocator* and a sizeclass_t into a - * MetaEntry's uintptr_t remote_and_sizeclass field. - * - * There's a little bit of an asymmetry here. Since the backend actually sets - * the entry (when associating a metadata structure), we don't construct a - * full MetaEntry here, but rather use ::encode() to compute its - * remote_and_sizeclass value. On the decode side, we are given read-only - * access to MetaEntry-s so can directly read therefrom rather than having to - * speak in terms of uintptr_t-s. - */ - struct FrontendMetaEntry - { - /// Perform the encoding. - static SNMALLOC_FAST_PATH uintptr_t - encode(RemoteAllocator* remote, sizeclass_t sizeclass) - { - /* remote might be nullptr; cast to uintptr_t before offsetting */ - return pointer_offset( - reinterpret_cast(remote), sizeclass.raw()); - } - - [[nodiscard]] static SNMALLOC_FAST_PATH RemoteAllocator* - get_remote(const MetaEntry& me) - { - return reinterpret_cast( - pointer_align_down( - me.get_remote_and_sizeclass())); - } - - [[nodiscard]] static SNMALLOC_FAST_PATH sizeclass_t - get_sizeclass(const MetaEntry& me) - { - // TODO: perhaps remove static_cast with resolution of - // https://github.com/CTSRD-CHERI/llvm-project/issues/588 - return sizeclass_t::from_raw( - static_cast(me.get_remote_and_sizeclass()) & - (REMOTE_WITH_BACKEND_MARKER_ALIGN - 1)); - } - - /** - * Return the Metaslab metadata associated with this chunk, guarded by an - * assert that this chunk is being used as a slab (i.e., has an associated - * owning allocator). - */ - [[nodiscard]] static SNMALLOC_FAST_PATH Metaslab* - get_metaslab(const MetaEntry& me) - { - SNMALLOC_ASSERT(get_remote(me) != nullptr); - return reinterpret_cast(me.get_meta()); - } - }; -} // namespace snmalloc diff --git a/src/mem/remoteallocator.h b/src/mem/remoteallocator.h index 2853c78..e851a0a 100644 --- a/src/mem/remoteallocator.h +++ b/src/mem/remoteallocator.h @@ -1,8 +1,8 @@ #pragma once -#include "../backend/metatypes.h" #include "../mem/allocconfig.h" #include "../mem/freelist.h" +#include "../mem/metadata.h" #include "../mem/sizeclasstable.h" #include @@ -10,19 +10,6 @@ namespace snmalloc { - // Remotes need to be aligned enough that the bottom bits have enough room for - // all the size classes, both large and small. An additional bit is required - // to separate backend uses. - static constexpr size_t REMOTE_MIN_ALIGN = - bits::max(CACHELINE_SIZE, SIZECLASS_REP_SIZE) << 1; - - // The bit above the sizeclass is always zero unless this is used - // by the backend to represent another datastructure such as the buddy - // allocator entries. - constexpr size_t REMOTE_WITH_BACKEND_MARKER_ALIGN = - MetaEntry::REMOTE_BACKEND_MARKER; - static_assert((REMOTE_MIN_ALIGN >> 1) == MetaEntry::REMOTE_BACKEND_MARKER); - /** * Global key for all remote lists. */ diff --git a/src/mem/remotecache.h b/src/mem/remotecache.h index a258a38..1166656 100644 --- a/src/mem/remotecache.h +++ b/src/mem/remotecache.h @@ -1,10 +1,10 @@ #pragma once -#include "../mem/allocconfig.h" -#include "../mem/freelist.h" -#include "../mem/metaslab.h" -#include "../mem/remoteallocator.h" -#include "../mem/sizeclasstable.h" +#include "allocconfig.h" +#include "freelist.h" +#include "metadata.h" +#include "remoteallocator.h" +#include "sizeclasstable.h" #include #include @@ -52,10 +52,11 @@ namespace snmalloc * * This does not require initialisation to be safely called. */ - SNMALLOC_FAST_PATH bool reserve_space(const MetaEntry& entry) + template + SNMALLOC_FAST_PATH bool reserve_space(const Entry& entry) { - auto size = static_cast( - sizeclass_full_to_size(FrontendMetaEntry::get_sizeclass(entry))); + auto size = + static_cast(sizeclass_full_to_size(entry.get_sizeclass())); bool result = capacity > size; if (result) @@ -75,19 +76,19 @@ namespace snmalloc list[get_slot(target_id, 0)].add(r, key); } - template + template bool post( - typename SharedStateHandle::LocalState* local_state, + typename Backend::LocalState* local_state, RemoteAllocator::alloc_id_t id, const FreeListKey& key) { SNMALLOC_ASSERT(initialised); size_t post_round = 0; bool sent_something = false; - auto domesticate = - [local_state](freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { - return capptr_domesticate(local_state, p); - }; + auto domesticate = [local_state](freelist::QueuePtr p) + SNMALLOC_FAST_PATH_LAMBDA { + return capptr_domesticate(local_state, p); + }; while (true) { @@ -101,16 +102,16 @@ namespace snmalloc if (!list[i].empty()) { auto [first, last] = list[i].extract_segment(key); - const MetaEntry& entry = - SharedStateHandle::Pagemap::get_metaentry(address_cast(first)); - auto remote = FrontendMetaEntry::get_remote(entry); + const auto& entry = + Backend::Pagemap::get_metaentry(address_cast(first)); + auto remote = entry.get_remote(); // If the allocator is not correctly aligned, then the bit that is // set implies this is used by the backend, and we should not be // deallocating memory here. snmalloc_check_client( - (address_cast(remote) & MetaEntry::REMOTE_BACKEND_MARKER) == 0, + !entry.is_backend_owned(), "Delayed detection of attempt to free internal structure."); - if constexpr (SharedStateHandle::Options.QueueHeadsAreTame) + if constexpr (Backend::Options.QueueHeadsAreTame) { auto domesticate_nop = [](freelist::QueuePtr p) { return freelist::HeadPtr(p.unsafe_ptr()); @@ -141,9 +142,8 @@ namespace snmalloc // Use the next N bits to spread out remote deallocs in our own // slot. auto r = resend.take(key, domesticate); - const MetaEntry& entry = - SharedStateHandle::Pagemap::get_metaentry(address_cast(r)); - auto i = FrontendMetaEntry::get_remote(entry)->trunc_id(); + const auto& entry = Backend::Pagemap::get_metaentry(address_cast(r)); + auto i = entry.get_remote()->trunc_id(); size_t slot = get_slot(i, post_round); list[slot].add(r, key); } diff --git a/src/test/func/domestication/domestication.cc b/src/test/func/domestication/domestication.cc index 5108960..64c74dd 100644 --- a/src/test/func/domestication/domestication.cc +++ b/src/test/func/domestication/domestication.cc @@ -146,8 +146,8 @@ int main() * * - RemoteAllocator::dequeue domesticating the stub's next pointer (p) * - * - Metaslab::alloc_free_list, domesticating the successor object in the - * newly minted freelist::Iter (i.e., the thing that would be allocated + * - FrontendMetaData::alloc_free_list, domesticating the successor object + * in the newly minted freelist::Iter (i.e., the thing that would be allocated * after q). */ static constexpr size_t expected_count = diff --git a/src/test/func/redblack/redblack.cc b/src/test/func/redblack/redblack.cc index 5901331..208acea 100644 --- a/src/test/func/redblack/redblack.cc +++ b/src/test/func/redblack/redblack.cc @@ -13,7 +13,7 @@ #include "ds/redblacktree.h" #include "snmalloc.h" -struct Wrapper +struct NodeRef { // The redblack tree is going to be used inside the pagemap, // and the redblack tree cannot use all the bits. Applying an offset @@ -21,7 +21,33 @@ struct Wrapper // the representation. static constexpr size_t offset = 10000; - size_t value = offset << 1; + size_t* ptr; + constexpr NodeRef(size_t* p) : ptr(p) {} + constexpr NodeRef() : ptr(nullptr) {} + constexpr NodeRef(const NodeRef& other) : ptr(other.ptr) {} + constexpr NodeRef(NodeRef&& other) : ptr(other.ptr) {} + + bool operator!=(const NodeRef& other) const + { + return ptr != other.ptr; + } + NodeRef& operator=(const NodeRef& other) + { + ptr = other.ptr; + return *this; + } + void set(uint16_t val) + { + *ptr = ((size_t(val) + offset) << 1) + (*ptr & 1); + } + explicit operator uint16_t() + { + return uint16_t((*ptr >> 1) - offset); + } + explicit operator size_t*() + { + return ptr; + } }; // Simple representation that is like the pagemap. @@ -29,8 +55,8 @@ struct Wrapper // We shift the fields up to make room for the colour. struct node { - Wrapper left; - Wrapper right; + size_t left; + size_t right; }; inline static node array[2048]; @@ -38,40 +64,41 @@ inline static node array[2048]; class Rep { public: - using key = size_t; + using key = uint16_t; static constexpr key null = 0; + static constexpr size_t root{NodeRef::offset << 1}; - using Holder = Wrapper; - using Contents = size_t; + using Handle = NodeRef; + using Contents = uint16_t; - static void set(Holder* ptr, Contents r) + static void set(Handle ptr, Contents r) { - ptr->value = ((r + Wrapper::offset) << 1) + (ptr->value & 1); + ptr.set(r); } - static Contents get(Holder* ptr) + static Contents get(Handle ptr) { - return (ptr->value >> 1) - Wrapper::offset; + return static_cast(ptr); } - static Holder& ref(bool direction, key k) + static Handle ref(bool direction, key k) { if (direction) - return array[k].left; + return {&array[k].left}; else - return array[k].right; + return {&array[k].right}; } static bool is_red(key k) { - return (array[k].left.value & 1) == 1; + return (array[k].left & 1) == 1; } static void set_red(key k, bool new_is_red) { if (new_is_red != is_red(k)) - array[k].left.value ^= 1; + array[k].left ^= 1; } static bool compare(key k1, key k2) @@ -88,6 +115,16 @@ public: { return k; } + + static size_t* printable(NodeRef k) + { + return static_cast(k); + } + + static const char* name() + { + return "TestRep"; + } }; template @@ -112,9 +149,9 @@ void test(size_t size, unsigned int seed) for (auto j = batch; j > 0; j--) { auto index = 1 + rand.next() % size; - if (tree.insert_elem(index)) + if (tree.insert_elem(Rep::key(index))) { - entries.push_back(index); + entries.push_back(Rep::key(index)); } } } @@ -189,4 +226,4 @@ int main(int argc, char** argv) // Trace particular example test(size, seed); return 0; -} \ No newline at end of file +}