[Bug]: inlined_vector implicitly assumes Pointer<A> is a native pointer type
Opened this issue · 0 comments
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'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_ptr
s!
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.
Steps to reproduce the problem
https://godbolt.org/z/9zYjbax8v
#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*'
}
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