[libcxx-commits] [libcxx] [libc++] Fix strict aliasing violation for `deque::const_iterator` (PR #136067)
A. Jiang via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Jun 5 01:58:43 PDT 2025
https://github.com/frederick-vs-ja updated https://github.com/llvm/llvm-project/pull/136067
>From 608000748c1e1feec392376de74f0b00f2bdd705 Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Thu, 17 Apr 2025 09:09:28 +0800
Subject: [PATCH 1/2] [libc++] Fix strict aliasing violation for
`deque::const_iterator`
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.
---
libcxx/docs/ReleaseNotes/21.rst | 8 ++++++++
libcxx/include/deque | 17 ++++++++---------
2 files changed, 16 insertions(+), 9 deletions(-)
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);
}
>From c62445517a611ca143f64dfb7027cc4102fd130f Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Thu, 5 Jun 2025 16:56:01 +0800
Subject: [PATCH 2/2] Switch to use internally rebinding
---
libcxx/include/deque | 63 +++++++++++++++++++++++++++++++-------------
1 file changed, 44 insertions(+), 19 deletions(-)
diff --git a/libcxx/include/deque b/libcxx/include/deque
index 8df2a046e618d..60a7b44b1500a 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -227,12 +227,14 @@ template <class T, class Allocator, class Predicate>
# include <__type_traits/disjunction.h>
# include <__type_traits/enable_if.h>
# include <__type_traits/is_allocator.h>
+# include <__type_traits/is_const.h>
# include <__type_traits/is_convertible.h>
# include <__type_traits/is_nothrow_assignable.h>
# include <__type_traits/is_nothrow_constructible.h>
# include <__type_traits/is_same.h>
# include <__type_traits/is_swappable.h>
# include <__type_traits/is_trivially_relocatable.h>
+# include <__type_traits/remove_reference.h>
# include <__type_traits/type_identity.h>
# include <__utility/forward.h>
# include <__utility/move.h>
@@ -269,6 +271,15 @@ struct __deque_block_size {
static const _DiffType value = sizeof(_ValueType) < 256 ? 4096 / sizeof(_ValueType) : 16;
};
+// When using fancy pointers, _MapPointer can be FancyPtr<const FancyPtr<const _ValueType>>, which causes strict
+// aliasing violation in direct dereferencing because the internal map stores FancyPtr<_ValueType> objects.
+// We need to transform the type to something like FancyPtr<const FancyPtr<_ValueType>>.
+template <class _ValueType, class _MapPointer>
+using __get_deque_map_iterator _LIBCPP_NODEBUG =
+ conditional_t<is_const<__libcpp_remove_reference_t<decltype(**_MapPointer())> >::value,
+ __rebind_pointer_t<_MapPointer, const __rebind_pointer_t<_MapPointer, _ValueType> >,
+ _MapPointer>;
+
template <class _ValueType,
class _Pointer,
class _Reference,
@@ -284,7 +295,7 @@ template <class _ValueType,
# endif
>
class __deque_iterator {
- typedef _MapPointer __map_iterator;
+ using __map_iterator _LIBCPP_NODEBUG = __get_deque_map_iterator<_ValueType, _MapPointer>;
public:
typedef _Pointer pointer;
@@ -461,7 +472,7 @@ private:
public:
using __is_segmented_iterator _LIBCPP_NODEBUG = true_type;
- using __segment_iterator _LIBCPP_NODEBUG = _MapPointer;
+ using __segment_iterator _LIBCPP_NODEBUG = __get_deque_map_iterator<_ValueType, _MapPointer>;
using __local_iterator _LIBCPP_NODEBUG = _Pointer;
static _LIBCPP_HIDE_FROM_ABI __segment_iterator __segment(_Iterator __iter) { return __iter.__m_iter_; }
@@ -504,18 +515,27 @@ 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 __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 __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_iterator _LIBCPP_NODEBUG = typename __map::const_iterator;
+
+ // Direct dereferencing __map_const_pointer possibly causes strict aliasing violation.
+ // We need to transform it to something like __map_alloc_traits::const_pointer for the const_iterator.
+ using __map_const_pointer _LIBCPP_NODEBUG = typename allocator_traits<__const_pointer_allocator>::const_pointer;
+ using __map_proper_const_pointer _LIBCPP_NODEBUG = typename __map_alloc_traits::const_pointer;
+
+ using __iter_proper_const_pointer _LIBCPP_NODEBUG = __get_deque_map_iterator<_Tp, __map_const_pointer>;
+ static_assert(is_same<decltype(*__iter_proper_const_pointer()), const pointer&>::value,
+ "The fancy pointer types provided by the allocators cannot be restored by pointer_traits::rebind.");
using reference = value_type&;
using const_reference = const value_type&;
- using iterator = __deque_iterator<value_type, pointer, reference, __map_pointer, difference_type>;
- using const_iterator =
+ using iterator = __deque_iterator<value_type, pointer, reference, __map_pointer, difference_type>;
+ using const_iterator = // Use of __map_const_pointer is merely kept for API stability.
__deque_iterator<value_type, const_pointer, const_reference, __map_const_pointer, difference_type>;
using reverse_iterator = std::reverse_iterator<iterator>;
using const_reverse_iterator = std::reverse_iterator<const_iterator>;
@@ -596,6 +616,10 @@ private:
deque* const __base_;
};
+ static __iter_proper_const_pointer __map_const_ptr_to_iter_const_ptr(__map_proper_const_pointer __mp) _NOEXCEPT {
+ return __mp ? pointer_traits<__iter_proper_const_pointer>::pointer_to(*__mp) : nullptr;
+ }
+
static const difference_type __block_size;
__map __map_;
@@ -720,8 +744,9 @@ public:
}
_LIBCPP_HIDE_FROM_ABI const_iterator begin() const _NOEXCEPT {
- __map_const_pointer __mp = __map_.begin() + __start_ / __block_size;
- return const_iterator(__mp, __map_.empty() ? 0 : *__mp + __start_ % __block_size);
+ __map_proper_const_pointer __mp = __map_.begin() + __start_ / __block_size;
+ return const_iterator(
+ __map_const_ptr_to_iter_const_ptr(__mp), __map_.empty() ? 0 : *__mp + __start_ % __block_size);
}
_LIBCPP_HIDE_FROM_ABI iterator end() _NOEXCEPT {
@@ -731,9 +756,9 @@ public:
}
_LIBCPP_HIDE_FROM_ABI const_iterator end() const _NOEXCEPT {
- size_type __p = size() + __start_;
- __map_const_pointer __mp = __map_.begin() + __p / __block_size;
- return const_iterator(__mp, __map_.empty() ? 0 : *__mp + __p % __block_size);
+ size_type __p = size() + __start_;
+ __map_proper_const_pointer __mp = __map_.begin() + __p / __block_size;
+ return const_iterator(__map_const_ptr_to_iter_const_ptr(__mp), __map_.empty() ? 0 : *__mp + __p % __block_size);
}
_LIBCPP_HIDE_FROM_ABI reverse_iterator rbegin() _NOEXCEPT { return reverse_iterator(end()); }
@@ -2330,7 +2355,7 @@ deque<_Tp, _Allocator>::__move_and_check(iterator __f, iterator __l, iterator __
__fe = __fb + __bs;
}
if (__fb <= __vt && __vt < __fe)
- __vt = (const_iterator(static_cast<__map_const_pointer>(__f.__m_iter_), __vt) -= __f - __r).__ptr_;
+ __vt = (const_iterator(__map_const_ptr_to_iter_const_ptr(__f.__m_iter_), __vt) -= __f - __r).__ptr_;
__r = std::move(__fb, __fe, __r);
__n -= __bs;
__f += __bs;
@@ -2357,7 +2382,7 @@ deque<_Tp, _Allocator>::__move_backward_and_check(iterator __f, iterator __l, it
__lb = __le - __bs;
}
if (__lb <= __vt && __vt < __le)
- __vt = (const_iterator(static_cast<__map_const_pointer>(__l.__m_iter_), __vt) += __r - __l - 1).__ptr_;
+ __vt = (const_iterator(__map_const_ptr_to_iter_const_ptr(__l.__m_iter_), __vt) += __r - __l - 1).__ptr_;
__r = std::move_backward(__lb, __le, __r);
__n -= __bs;
__l -= __bs - 1;
@@ -2383,7 +2408,7 @@ void deque<_Tp, _Allocator>::__move_construct_and_check(iterator __f, iterator _
__fe = __fb + __bs;
}
if (__fb <= __vt && __vt < __fe)
- __vt = (const_iterator(static_cast<__map_const_pointer>(__f.__m_iter_), __vt) += __r - __f).__ptr_;
+ __vt = (const_iterator(__map_const_ptr_to_iter_const_ptr(__f.__m_iter_), __vt) += __r - __f).__ptr_;
for (; __fb != __fe; ++__fb, ++__r, ++__size())
__alloc_traits::construct(__a, std::addressof(*__r), std::move(*__fb));
__n -= __bs;
@@ -2415,7 +2440,7 @@ void deque<_Tp, _Allocator>::__move_construct_backward_and_check(
__lb = __le - __bs;
}
if (__lb <= __vt && __vt < __le)
- __vt = (const_iterator(static_cast<__map_const_pointer>(__l.__m_iter_), __vt) -= __l - __r + 1).__ptr_;
+ __vt = (const_iterator(__map_const_ptr_to_iter_const_ptr(__l.__m_iter_), __vt) -= __l - __r + 1).__ptr_;
while (__le != __lb) {
__alloc_traits::construct(__a, std::addressof(*--__r), std::move(*--__le));
--__start_;
More information about the libcxx-commits
mailing list