Refactor capptr_domesticate SFINAE to make more statically safe.

This refactoring was provided by David.  Previously if a backend
provided a capptr_domesticate function with the wrong type it would be
silently ignored.  This change requires backends to explicitly opt in
to domestication via a new Backend::Option and ensures the compiler
will loudly complain if there is a mismatch.
This commit is contained in:
Robert Norton
2022-04-27 11:56:16 +01:00
committed by David Chisnall
parent bf54eeb7be
commit f277cf2f00
4 changed files with 72 additions and 19 deletions

View File

@@ -24,7 +24,26 @@ namespace snmalloc
return alloc_pool; return alloc_pool;
} }
static constexpr Flags Options{}; /*
* The obvious
* `static constexpr Flags Options{.HasDomesticate = true};` fails on
* Ubuntu 18.04 with an error "sorry, unimplemented: non-trivial
* designated initializers not supported".
* The following was copied from domestication.cc test with the following
* comment:
* C++, even as late as C++20, has some really quite strict limitations on
* designated initializers. However, as of C++17, we can have constexpr
* lambdas and so can use more of the power of the statement fragment of
* C++, and not just its initializer fragment, to initialize a non-prefix
* subset of the flags (in any order, at that).
*/
static constexpr Flags Options = []() constexpr
{
Flags opts = {};
opts.HasDomesticate = true;
return opts;
}
();
// This needs to be a forward reference as the // This needs to be a forward reference as the
// thread local state will need to know about this. // thread local state will need to know about this.

View File

@@ -86,6 +86,13 @@ namespace snmalloc
* the queue nodes themselves (which are always considered Wild)? * the queue nodes themselves (which are always considered Wild)?
*/ */
bool QueueHeadsAreTame = true; bool QueueHeadsAreTame = true;
/**
* Does the backend provide a capptr_domesticate function to sanity check
* pointers? If so it will be called when untrusted pointers are consumed
* (on dealloc and in freelists) otherwise a no-op version is provided.
*/
bool HasDomesticate = false;
}; };
/** /**

View File

@@ -36,41 +36,46 @@ namespace snmalloc
namespace detail namespace detail
{ {
/** /**
* SFINAE helper, calls capptr_domesticate in the backend if it exists. * SFINAE helper to detect the presence of capptr_domesticate function in
* backend. Returns true if there is a function with correct name and type.
*/ */
template< template<
SNMALLOC_CONCEPT(ConceptBackendDomestication) Backend, SNMALLOC_CONCEPT(ConceptBackendDomestication) Backend,
typename T, typename T,
SNMALLOC_CONCEPT(capptr::ConceptBound) B> SNMALLOC_CONCEPT(capptr::ConceptBound) B>
SNMALLOC_FAST_PATH_INLINE auto constexpr SNMALLOC_FAST_PATH_INLINE auto has_domesticate(int)
capptr_domesticate(typename Backend::LocalState* ls, CapPtr<T, B> p, int) -> std::enable_if_t<
-> decltype(Backend::capptr_domesticate(ls, p)) std::is_same_v<
decltype(Backend::capptr_domesticate(
std::declval<typename Backend::LocalState*>(),
std::declval<CapPtr<T, B>>())),
CapPtr<
T,
typename B::template with_wildness<
capptr::dimension::Wildness::Tame>>>,
bool>
{ {
return Backend::capptr_domesticate(ls, p); return true;
} }
/** /**
* SFINAE helper. If the back end does not provide special handling for * SFINAE helper to detect the presence of capptr_domesticate function in
* domestication then assume all wild pointers can be domesticated. * backend. Returns false in case where above template does not match.
*/ */
template< template<
SNMALLOC_CONCEPT(ConceptBackendGlobals) Backend, SNMALLOC_CONCEPT(ConceptBackendGlobals) Backend,
typename T, typename T,
SNMALLOC_CONCEPT(capptr::ConceptBound) B> SNMALLOC_CONCEPT(capptr::ConceptBound) B>
SNMALLOC_FAST_PATH_INLINE auto constexpr SNMALLOC_FAST_PATH_INLINE bool has_domesticate(long)
capptr_domesticate(typename Backend::LocalState*, CapPtr<T, B> p, long)
{ {
return CapPtr< return false;
T,
typename B::template with_wildness<capptr::dimension::Wildness::Tame>>(
p.unsafe_ptr());
} }
} // namespace detail } // namespace detail
/** /**
* Wrapper that calls `Backend::capptr_domesticate` if and only if it is * Wrapper that calls `Backend::capptr_domesticate` if and only if
* implemented. If it is not implemented then this assumes that any wild * Backend::Options.HasDomesticate is true. If it is not implemented then
* pointer can be domesticated. * this assumes that any wild pointer can be domesticated.
*/ */
template< template<
SNMALLOC_CONCEPT(ConceptBackendGlobals) Backend, SNMALLOC_CONCEPT(ConceptBackendGlobals) Backend,
@@ -79,7 +84,28 @@ namespace snmalloc
SNMALLOC_FAST_PATH_INLINE auto SNMALLOC_FAST_PATH_INLINE auto
capptr_domesticate(typename Backend::LocalState* ls, CapPtr<T, B> p) capptr_domesticate(typename Backend::LocalState* ls, CapPtr<T, B> p)
{ {
return detail::capptr_domesticate<Backend>(ls, p, 0); static_assert(
} !detail::has_domesticate<Backend, T, B>(0) ||
(detail::has_domesticate<Backend, T, B>(0) &&
Backend::Options.HasDomesticate),
"Back end provides domesticate function but opts out of using it ");
static_assert(
detail::has_domesticate<Backend, T, B>(0) ||
!(detail::has_domesticate<Backend, T, B>(0) &&
Backend::Options.HasDomesticate),
"Back end does not provide capptr_domesticate and requests its use");
if constexpr (Backend::Options.HasDomesticate)
{
return Backend::capptr_domesticate(ls, p);
}
else
{
UNUSED(ls);
return CapPtr<
T,
typename B::template with_wildness<capptr::dimension::Wildness::Tame>>(
p.unsafe_ptr());
}
}
} // namespace snmalloc } // namespace snmalloc

View File

@@ -40,6 +40,7 @@ namespace snmalloc
{ {
Flags opts = {}; Flags opts = {};
opts.QueueHeadsAreTame = false; opts.QueueHeadsAreTame = false;
opts.HasDomesticate = true;
return opts; return opts;
} }
(); ();