abseil / abseil/abseil-cpp

[Bug]: inlined_vector implicitly assumes Pointer<A> is a native pointer type

Đang mở
#1,616 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
C++
Star
18.1k
Fork
3.2k
Merge trung bình
20 giờ 36 phút
Pull request đã merge (30 ngày)
1

Mô 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'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::construct` with a `Pointer` argument instead of a native pointer. This flat-out doesn't compile, when `Pointer` 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>);` 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
#include
#include
#include
#include

namespace bip = boost::interprocess;
template using shm_allocator = bip::allocator;
template using shm_std_vector = std::vector>;
template using shm_absl_vector = absl::InlinedVector>;

static_assert(!absl::is_trivially_relocatable::pointer>::value);
static_assert(!absl::is_trivially_relocatable::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
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& v) {
v.reserve(100); // OK
}

void reserve_some(shm_absl_vector& v) {
v.reserve(100);
// Error in call to allocator_traits::construct:
// can't match 'offset_ptr' against 'T*'
}
```

### What version of Abseil are you using?

trunk, i.e. 4358cb2f8cb304e64d9a2d2845f472297724e19f 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_

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.