You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While working on #1615, I (thought I) noticed some places in absl::inlined_vector where it was more-or-less memcpying the union of (data, pointer-to-heap-allocation) guarded only under a check that the value type was trivially relocatable, i.e., it was failing to check whether the allocator's pointer type was also trivially relocatable. An example of a non-trivial pointer type is Boost's offset_ptr.
I tried to make a reproducer for that issue (expecting that I'd be able to show inlined_vector actually misbehaving at runtime, because memcpying an offset_ptr would trash its value) — but then I ran into a bigger issue: inlined_vector calls allocator_traits<A>::construct with a Pointer<A> argument instead of a native pointer. This flat-out doesn't compile, when Pointer<A> isn't a native pointer. This effectively prevents people from using inlined_vector with Boost.Interprocess... but that's a good thing, because if the code actually compiled (and if I'm not mistaken), it would Do The Wrong Thing at runtime by memcpying offset_ptrs!
The simple fix would be to add static_assert(std::is_trivially_copyable_v<Pointer<A>>); to inlined_vector's body, just to make the compiler error more user-friendly and "intentional."
The complicated/difficult fix would be to audit the whole inlined_vector codebase and fix any place that isn't offset_ptr-safe. I don't know how many places this would be.
#include <boost/interprocess/offset_ptr.hpp>
#include <boost/interprocess/segment_manager.hpp>
#include <boost/interprocess/managed_shared_memory.hpp>
#include <absl/container/inlined_vector.h>
#include <vector>
namespace bip = boost::interprocess;
template<class T> using shm_allocator = bip::allocator<T, bip::managed_shared_memory::segment_manager>;
template<class T> using shm_std_vector = std::vector<T, shm_allocator<T>>;
template<class T> using shm_absl_vector = absl::InlinedVector<T, 10, shm_allocator<T>>;
static_assert(!absl::is_trivially_relocatable<shm_std_vector<int>::pointer>::value);
static_assert(!absl::is_trivially_relocatable<shm_absl_vector<int>::pointer>::value);
// This is the key problem: absl::InlinedVector implicitly assumes its pointer
// type is always trivially relocatable, but in fact it might not be.
// This should at least be static_asserted/mandated.
// Ideally, you'd do a code audit of InlinedVector and fix all the places that
// need fixing (i.e. any place that assumes Pointer<A> is memcpyable), so that
// the following code would Just Work. I do *NOT* recommend just sprinkling in
// calls to `std::to_address(ptr)` until the `reserve` call compiles; I think
// it's a good thing that it fails noisily at compile time, instead of
// *for all we know* misbehaving at runtime by trying to memcpy an offset_ptr.
void reserve_some(shm_std_vector<int>& v) {
v.reserve(100); // OK
}
void reserve_some(shm_absl_vector<int>& v) {
v.reserve(100);
// Error in call to allocator_traits::construct:
// can't match 'offset_ptr<int>' against 'T*'
}
Describe the issue
https://godbolt.org/z/9zYjbax8v
While working on #1615, I (thought I) noticed some places in
absl::inlined_vector
where it was more-or-less memcpying the union of (data, pointer-to-heap-allocation) guarded only under a check that the value type was trivially relocatable, i.e., it was failing to check whether the allocator's pointer type was also trivially relocatable. An example of a non-trivial pointer type is Boost'soffset_ptr
.I tried to make a reproducer for that issue (expecting that I'd be able to show
inlined_vector
actually misbehaving at runtime, because memcpying anoffset_ptr
would trash its value) — but then I ran into a bigger issue:inlined_vector
callsallocator_traits<A>::construct
with aPointer<A>
argument instead of a native pointer. This flat-out doesn't compile, whenPointer<A>
isn't a native pointer. This effectively prevents people from usinginlined_vector
with Boost.Interprocess... but that's a good thing, because if the code actually compiled (and if I'm not mistaken), it would Do The Wrong Thing at runtime by memcpyingoffset_ptr
s!The simple fix would be to add
static_assert(std::is_trivially_copyable_v<Pointer<A>>);
toinlined_vector
's body, just to make the compiler error more user-friendly and "intentional."The complicated/difficult fix would be to audit the whole
inlined_vector
codebase and fix any place that isn'toffset_ptr
-safe. I don't know how many places this would be.Steps to reproduce the problem
https://godbolt.org/z/9zYjbax8v
What version of Abseil are you using?
trunk, i.e. 4358cb2 as of this writing
What operating system and version are you using?
Any
What compiler and version are you using?
Any
What build system are you using?
Any
Additional context
No response
The text was updated successfully, but these errors were encountered: