Cheri-research/snmalloc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Stop playing OO games with MetaEntry

Browse Source 
David points out that the downcasts I had introduced were UB.  Instead, go back
to passing MetaEntry-s around and make MetaslabMetaEntry just a namespace of
static methods.

This partially reverts 7940fee00c
This commit is contained in:
Nathaniel Wesley Filardo
2022-03-24 13:47:41 +00:00
committed by Nathaniel Wesley Filardo
parent e77b9e2851
commit 6424edaeaa
6 changed files with 78 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
src/backend/backend.h
View File

@@ -58,14 +58,10 @@ namespace snmalloc
* Set template parameter to true if it not an error
* to access a location that is not backed by a chunk.
*/
template<typename Ret = MetaEntry, bool potentially_out_of_range = false>
SNMALLOC_FAST_PATH static const Ret& get_metaentry(address_t p)
template<bool potentially_out_of_range = false>
SNMALLOC_FAST_PATH static const MetaEntry& get_metaentry(address_t p)
{
static_assert(
std::is_base_of_v<MetaEntry, Ret> && sizeof(MetaEntry) == sizeof(Ret),
"Backend Pagemap get_metaentry return must look like MetaEntry");
return static_cast<const Ret&>(
concretePagemap.template get<potentially_out_of_range>(p));
return concretePagemap.template get<potentially_out_of_range>(p);
}
/**

4
src/backend/backend_concept.h
View File

@@ -24,10 +24,10 @@ namespace snmalloc
{
{ Meta::set_metaentry(addr, sz, t) } -> ConceptSame<void>;
{ Meta::template get_metaentry<MetaEntry, true>(addr) }
{ Meta::template get_metaentry<true>(addr) }
-> ConceptSame<const MetaEntry&>;
{ Meta::template get_metaentry<MetaEntry, false>(addr) }
{ Meta::template get_metaentry<false>(addr) }
-> ConceptSame<const MetaEntry&>;
};

44
src/mem/corealloc.h
View File

@@ -400,17 +400,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 MetaslabMetaEntry& entry)
SNMALLOC_SLOW_PATH void dealloc_local_object_slow(const MetaEntry& entry)
{
// TODO: Handle message queue on this path?
Metaslab* meta = entry.get_metaslab();
Metaslab* meta = FrontendMetaEntry::get_metaslab(entry);
if (meta->is_large())
{
// Handle large deallocation here.
size_t entry_sizeclass = entry.get_sizeclass().as_large();
size_t entry_sizeclass =
FrontendMetaEntry::get_sizeclass(entry).as_large();
size_t size = bits::one_at_bit(entry_sizeclass);
#ifdef SNMALLOC_TRACING
@@ -425,7 +425,8 @@ namespace snmalloc
return;
}
smallsizeclass_t sizeclass = entry.get_sizeclass().as_small();
smallsizeclass_t sizeclass =
FrontendMetaEntry::get_sizeclass(entry).as_small();
UNUSED(entropy);
if (meta->is_sleeping())
@@ -488,9 +489,8 @@ namespace snmalloc
message<1024>("Handling remote");
#endif
auto& entry =
SharedStateHandle::Pagemap::template get_metaentry<MetaslabMetaEntry>(
snmalloc::address_cast(msg));
auto& entry = SharedStateHandle::Pagemap::template get_metaentry(
snmalloc::address_cast(msg));
handle_dealloc_remote(entry, msg.as_void(), need_post);
@@ -529,7 +529,7 @@ namespace snmalloc
* need_post will be set to true, if capacity is exceeded.
*/
void handle_dealloc_remote(
const MetaslabMetaEntry& entry,
const MetaEntry& entry,
CapPtr<void, capptr::bounds::Alloc> p,
bool& need_post)
{
@@ -537,7 +537,8 @@ namespace snmalloc
// TODO this needs to not double revoke if using MTE
// TODO thread capabilities?
if (SNMALLOC_LIKELY(entry.get_remote() == public_state()))
if (SNMALLOC_LIKELY(
FrontendMetaEntry::get_remote(entry) == public_state()))
{
if (SNMALLOC_LIKELY(
dealloc_local_object_fast(entry, p.as_void(), entropy)))
@@ -553,7 +554,9 @@ namespace snmalloc
need_post = true;
attached_cache->remote_dealloc_cache
.template dealloc<sizeof(CoreAllocator)>(
entry.get_remote()->trunc_id(), p.as_void(), key_global);
FrontendMetaEntry::get_remote(entry)->trunc_id(),
p.as_void(),
key_global);
}
}
@@ -670,9 +673,8 @@ namespace snmalloc
dealloc_local_object(CapPtr<void, capptr::bounds::Alloc> p)
{
// MetaEntry-s seen here are expected to have meaningful Remote pointers
auto& entry =
SharedStateHandle::Pagemap::template get_metaentry<MetaslabMetaEntry>(
snmalloc::address_cast(p));
auto& entry = SharedStateHandle::Pagemap::template get_metaentry(
snmalloc::address_cast(p));
if (SNMALLOC_LIKELY(dealloc_local_object_fast(entry, p, entropy)))
return;
@@ -680,16 +682,17 @@ namespace snmalloc
}
SNMALLOC_FAST_PATH static bool dealloc_local_object_fast(
const MetaslabMetaEntry& entry,
const MetaEntry& entry,
CapPtr<void, capptr::bounds::Alloc> p,
LocalEntropy& entropy)
{
auto meta = entry.get_metaslab();
auto meta = FrontendMetaEntry::get_metaslab(entry);
SNMALLOC_ASSERT(!meta->is_unused());
snmalloc_check_client(
is_start_of_object(entry.get_sizeclass(), address_cast(p)),
is_start_of_object(
FrontendMetaEntry::get_sizeclass(entry), address_cast(p)),
"Not deallocating start of an object");
auto cp = p.as_static<freelist::Object::T<>>();
@@ -784,7 +787,7 @@ namespace snmalloc
auto [slab, meta] = SharedStateHandle::alloc_chunk(
get_backend_local_state(),
slab_size,
MetaslabMetaEntry::encode(
FrontendMetaEntry::encode(
public_state(), sizeclass_t::from_small_class(sizeclass)));
if (slab == nullptr)
@@ -838,9 +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 MetaslabMetaEntry& entry =
SharedStateHandle::Pagemap::template get_metaentry<
MetaslabMetaEntry>(snmalloc::address_cast(p_tame));
const MetaEntry& entry = SharedStateHandle::Pagemap::get_metaentry(
snmalloc::address_cast(p_tame));
handle_dealloc_remote(entry, p_tame.as_void(), need_post);
p_tame = n_tame;
}

44
src/mem/localalloc.h
View File

@@ -183,7 +183,7 @@ namespace snmalloc
auto [chunk, meta] = SharedStateHandle::alloc_chunk(
core_alloc->get_backend_local_state(),
large_size_to_chunk_size(size),
MetaslabMetaEntry::encode(
FrontendMetaEntry::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.
@@ -267,11 +267,10 @@ namespace snmalloc
p.unsafe_ptr(),
alloc_size(p.unsafe_ptr()));
#endif
const MetaslabMetaEntry& entry =
SharedStateHandle::Pagemap::template get_metaentry<MetaslabMetaEntry>(
address_cast(p));
const MetaEntry& entry =
SharedStateHandle::Pagemap::get_metaentry(address_cast(p));
local_cache.remote_dealloc_cache.template dealloc<sizeof(CoreAlloc)>(
entry.get_remote()->trunc_id(), p, key_global);
FrontendMetaEntry::get_remote(entry)->trunc_id(), p, key_global);
post_remote_cache();
return;
}
@@ -625,10 +624,11 @@ namespace snmalloc
capptr::Alloc<void> p_tame = capptr_domesticate<SharedStateHandle>(
core_alloc->backend_state_ptr(), p_wild);
const MetaslabMetaEntry& entry =
SharedStateHandle::Pagemap::template get_metaentry<MetaslabMetaEntry>(
address_cast(p_tame));
if (SNMALLOC_LIKELY(local_cache.remote_allocator == entry.get_remote()))
const MetaEntry& entry =
SharedStateHandle::Pagemap::get_metaentry(address_cast(p_tame));
if (SNMALLOC_LIKELY(
local_cache.remote_allocator ==
FrontendMetaEntry::get_remote(entry)))
{
# if defined(__CHERI_PURE_CAPABILITY__) && defined(SNMALLOC_CHECK_CLIENT)
dealloc_cheri_checks(p_tame.unsafe_ptr());
@@ -640,7 +640,8 @@ namespace snmalloc
return;
}
if (SNMALLOC_LIKELY(entry.get_remote() != nullptr))
RemoteAllocator* remote = FrontendMetaEntry::get_remote(entry);
if (SNMALLOC_LIKELY(remote != nullptr))
{
# if defined(__CHERI_PURE_CAPABILITY__) && defined(SNMALLOC_CHECK_CLIENT)
dealloc_cheri_checks(p_tame.unsafe_ptr());
@@ -649,7 +650,7 @@ namespace snmalloc
if (local_cache.remote_dealloc_cache.reserve_space(entry))
{
local_cache.remote_dealloc_cache.template dealloc<sizeof(CoreAlloc)>(
entry.get_remote()->trunc_id(), p_tame, key_global);
remote->trunc_id(), p_tame, key_global);
# ifdef SNMALLOC_TRACING
message<1024>(
"Remote dealloc fast {} ({})", p_raw, alloc_size(p_raw));
@@ -715,11 +716,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 MetaslabMetaEntry& entry =
SharedStateHandle::Pagemap::template get_metaentry<MetaslabMetaEntry>(
address_cast(p_raw));
const MetaEntry& entry =
SharedStateHandle::Pagemap::get_metaentry(address_cast(p_raw));
return sizeclass_full_to_size(entry.get_sizeclass());
return sizeclass_full_to_size(FrontendMetaEntry::get_sizeclass(entry));
#endif
}
@@ -761,10 +761,11 @@ namespace snmalloc
size_t remaining_bytes(const void* p)
{
#ifndef SNMALLOC_PASS_THROUGH
const MetaslabMetaEntry& entry = SharedStateHandle::Pagemap::
template get_metaentry<MetaslabMetaEntry, true>(address_cast(p));
const MetaEntry& entry =
SharedStateHandle::Pagemap::template get_metaentry<true>(
address_cast(p));
auto sizeclass = entry.get_sizeclass();
auto sizeclass = FrontendMetaEntry::get_sizeclass(entry);
return snmalloc::remaining_bytes(sizeclass, address_cast(p));
#else
return pointer_diff(p, reinterpret_cast<void*>(UINTPTR_MAX));
@@ -789,10 +790,11 @@ namespace snmalloc
size_t index_in_object(const void* p)
{
#ifndef SNMALLOC_PASS_THROUGH
const MetaslabMetaEntry& entry = SharedStateHandle::Pagemap::
template get_metaentry<MetaslabMetaEntry, true>(address_cast(p));
const MetaEntry& entry =
SharedStateHandle::Pagemap::template get_metaentry<true>(
address_cast(p));
auto sizeclass = entry.get_sizeclass();
auto sizeclass = FrontendMetaEntry::get_sizeclass(entry);
return snmalloc::index_in_object(sizeclass, address_cast(p));
#else
return reinterpret_cast<size_t>(p);

37
src/mem/metaslab.h
View File

@@ -226,18 +226,17 @@ namespace snmalloc
};
/*
* A convenience wrapper aroun MetaEntry with a meaningful RemoteAllocator
* pointer. This encodes a RemoteAllocator* and a sizeclass_t into a the
* uintptr_t remote_and_sizeclass field.
* 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), MetaslabMetaEntry-s are
* not constructed directly; please use ::encode(). On the other hand, the
* backend's Pagemap::get_metaentry() method is templated on its return type,
* so it is relatively straightforward to view a pagemap entry as a
* MetaslabMetaEntry and then use the accessors here for decoding.
* 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 MetaslabMetaEntry : public MetaEntry
struct FrontendMetaEntry
{
/// Perform the encoding.
static SNMALLOC_FAST_PATH uintptr_t
@@ -248,19 +247,21 @@ namespace snmalloc
reinterpret_cast<uintptr_t>(remote), sizeclass.raw());
}
[[nodiscard]] SNMALLOC_FAST_PATH RemoteAllocator* get_remote() const
[[nodiscard]] static SNMALLOC_FAST_PATH RemoteAllocator*
get_remote(const MetaEntry& me)
{
return reinterpret_cast<RemoteAllocator*>(
pointer_align_down<REMOTE_WITH_BACKEND_MARKER_ALIGN>(
get_remote_and_sizeclass()));
me.get_remote_and_sizeclass()));
}
[[nodiscard]] SNMALLOC_FAST_PATH sizeclass_t get_sizeclass() const
[[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<size_t>(get_remote_and_sizeclass()) &
static_cast<size_t>(me.get_remote_and_sizeclass()) &
(REMOTE_WITH_BACKEND_MARKER_ALIGN - 1));
}
@@ -269,13 +270,11 @@ namespace snmalloc
* assert that this chunk is being used as a slab (i.e., has an associated
* owning allocator).
*/
[[nodiscard]] SNMALLOC_FAST_PATH Metaslab* get_metaslab() const
[[nodiscard]] static SNMALLOC_FAST_PATH Metaslab*
get_metaslab(const MetaEntry& me)
{
SNMALLOC_ASSERT(get_remote() != nullptr);
return reinterpret_cast<Metaslab*>(get_meta());
SNMALLOC_ASSERT(get_remote(me) != nullptr);
return reinterpret_cast<Metaslab*>(me.get_meta());
}
};
static_assert(sizeof(MetaslabMetaEntry) == sizeof(MetaEntry));
} // namespace snmalloc

20
src/mem/remotecache.h
View File

@@ -52,10 +52,10 @@ namespace snmalloc
*
* This does not require initialisation to be safely called.
*/
SNMALLOC_FAST_PATH bool reserve_space(const MetaslabMetaEntry& entry)
SNMALLOC_FAST_PATH bool reserve_space(const MetaEntry& entry)
{
auto size =
static_cast<int64_t>(sizeclass_full_to_size(entry.get_sizeclass()));
auto size = static_cast<int64_t>(
sizeclass_full_to_size(FrontendMetaEntry::get_sizeclass(entry)));
bool result = capacity > size;
if (result)
@@ -101,10 +101,9 @@ namespace snmalloc
if (!list[i].empty())
{
auto [first, last] = list[i].extract_segment(key);
const MetaslabMetaEntry& entry =
SharedStateHandle::Pagemap::template get_metaentry<
MetaslabMetaEntry>(address_cast(first));
auto remote = entry.get_remote();
const MetaEntry& entry =
SharedStateHandle::Pagemap::get_metaentry(address_cast(first));
auto remote = FrontendMetaEntry::get_remote(entry);
// 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.
@@ -142,10 +141,9 @@ namespace snmalloc
// Use the next N bits to spread out remote deallocs in our own
// slot.
auto r = resend.take(key, domesticate);
const MetaslabMetaEntry& entry =
SharedStateHandle::Pagemap::template get_metaentry<
MetaslabMetaEntry>(address_cast(r));
auto i = entry.get_remote()->trunc_id();
const MetaEntry& entry =
SharedStateHandle::Pagemap::get_metaentry(address_cast(r));
auto i = FrontendMetaEntry::get_remote(entry)->trunc_id();
size_t slot = get_slot<allocator_size>(i, post_round);
list[slot].add(r, key);
}