-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[libc++] Fix strict aliasing violation for deque::const_iterator
#136067
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[libc++] Fix strict aliasing violation for deque::const_iterator
#136067
Conversation
When the allocators use fancy pointers, the internal map of `deque` stores `fancy_ptr<T>` objects, and the previous strategy accessed these objects via `const fancy_ptr<const T>` lvalues, which usually caused core language undefined behavior. Now `const_iterator` stores `fancy_ptr<const fancy_ptr<T>>` instead of `fancy_ptr<const fancy_ptr<const T>>`, and ABI break can happen when such two types have incompatible layouts. This is necessary for reducing undefined behavior and `constexpr` support for `deque` in C++26, and I currently don't want to provide any way to opt-out of that behavior. Also removes redundant identity `static_cast` before and after type change. The existing test coverage seems to be sufficient.
@llvm/pr-subscribers-libcxx Author: A. Jiang (frederick-vs-ja) ChangesWhen the allocators use fancy pointers, the internal map of This is necessary for reducing undefined behavior and Also removes redundant identity The existing test coverage seems to be sufficient. Full diff: https://github.com/llvm/llvm-project/pull/136067.diff 2 Files Affected:
diff --git a/libcxx/docs/ReleaseNotes/21.rst b/libcxx/docs/ReleaseNotes/21.rst
index 2091a713ea200..84d7d8cf7aab3 100644
--- a/libcxx/docs/ReleaseNotes/21.rst
+++ b/libcxx/docs/ReleaseNotes/21.rst
@@ -114,6 +114,14 @@ ABI Affecting Changes
comparison between shared libraries, since all RTTI has the correct visibility now. There is no behaviour change on
Clang.
+- The ``const_iterator`` member type of ``std::deque`` is now corrected to hold a (possibly fancy) pointer to the
+ (possibly fancy) pointer allocated in the internal map. E.g. when the allocators use fancy pointers, the internal map
+ stores ``fancy_ptr<T>`` objects, and the previous strategy accessed these objects via ``const fancy_ptr<const T>``
+ lvalues, which usually caused core language undefined behavior. Now ``const_iterator`` stores
+ ``fancy_ptr<const fancy_ptr<T>>`` instead of ``fancy_ptr<const fancy_ptr<const T>>``, and ABI break can happen when
+ such two types have incompatible layouts. This is necessary for reducing undefined behavior and ``constexpr`` support
+ for ``deque`` in C++26, so we do not provide any way to opt-out of that behavior.
+
Build System Changes
--------------------
diff --git a/libcxx/include/deque b/libcxx/include/deque
index e9846af5e5848..8df2a046e618d 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -504,13 +504,12 @@ public:
using pointer = typename __alloc_traits::pointer;
using const_pointer = typename __alloc_traits::const_pointer;
- using __pointer_allocator _LIBCPP_NODEBUG = __rebind_alloc<__alloc_traits, pointer>;
- using __const_pointer_allocator _LIBCPP_NODEBUG = __rebind_alloc<__alloc_traits, const_pointer>;
- using __map _LIBCPP_NODEBUG = __split_buffer<pointer, __pointer_allocator>;
- using __map_alloc_traits _LIBCPP_NODEBUG = allocator_traits<__pointer_allocator>;
- using __map_pointer _LIBCPP_NODEBUG = typename __map_alloc_traits::pointer;
- using __map_const_pointer _LIBCPP_NODEBUG = typename allocator_traits<__const_pointer_allocator>::const_pointer;
- using __map_const_iterator _LIBCPP_NODEBUG = typename __map::const_iterator;
+ using __pointer_allocator _LIBCPP_NODEBUG = __rebind_alloc<__alloc_traits, pointer>;
+ using __map _LIBCPP_NODEBUG = __split_buffer<pointer, __pointer_allocator>;
+ using __map_alloc_traits _LIBCPP_NODEBUG = allocator_traits<__pointer_allocator>;
+ using __map_pointer _LIBCPP_NODEBUG = typename __map_alloc_traits::pointer;
+ using __map_const_pointer _LIBCPP_NODEBUG = typename allocator_traits<__pointer_allocator>::const_pointer;
+ using __map_const_iterator _LIBCPP_NODEBUG = typename __map::const_iterator;
using reference = value_type&;
using const_reference = const value_type&;
@@ -721,7 +720,7 @@ public:
}
_LIBCPP_HIDE_FROM_ABI const_iterator begin() const _NOEXCEPT {
- __map_const_pointer __mp = static_cast<__map_const_pointer>(__map_.begin() + __start_ / __block_size);
+ __map_const_pointer __mp = __map_.begin() + __start_ / __block_size;
return const_iterator(__mp, __map_.empty() ? 0 : *__mp + __start_ % __block_size);
}
@@ -733,7 +732,7 @@ public:
_LIBCPP_HIDE_FROM_ABI const_iterator end() const _NOEXCEPT {
size_type __p = size() + __start_;
- __map_const_pointer __mp = static_cast<__map_const_pointer>(__map_.begin() + __p / __block_size);
+ __map_const_pointer __mp = __map_.begin() + __p / __block_size;
return const_iterator(__mp, __map_.empty() ? 0 : *__mp + __p % __block_size);
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can do this as-is. AFAICT this changes the type of deque::const_iterator
, which is way too big of an ABI break.
When the allocators use fancy pointers, the internal map of
deque
storesfancy_ptr<T>
objects, and the previous strategy accessed these objects viaconst fancy_ptr<const T>
lvalues, which usually caused core language undefined behavior. Nowconst_iterator
storesfancy_ptr<const fancy_ptr<T>>
instead offancy_ptr<const fancy_ptr<const T>>
, and ABI break can happen when such two types have incompatible layouts.This is necessary for reducing undefined behavior and
constexpr
support fordeque
in C++26, and I currently don't want to provide any way to opt-out of that behavior.Also removes redundant identity
static_cast
before and after type change.The existing test coverage seems to be sufficient.