Skip to content

[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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions libcxx/docs/ReleaseNotes/22.rst
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,13 @@ Announcements About Future Releases
ABI Affecting Changes
---------------------

- 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, caused 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
--------------------
53 changes: 39 additions & 14 deletions libcxx/include/deque
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,15 @@ 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_replaceable.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>
Expand Down Expand Up @@ -270,6 +272,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,
Expand All @@ -285,7 +296,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;
Expand Down Expand Up @@ -461,7 +472,7 @@ private:
__deque_iterator<_ValueType, _Pointer, _Reference, _MapPointer, _DiffType, _BlockSize>;

public:
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_; }
Expand Down Expand Up @@ -509,14 +520,22 @@ public:
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;

// 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 stability of const_iterator's true name.
__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>;
Expand Down Expand Up @@ -601,6 +620,11 @@ private:
deque* const __base_;
};

static _LIBCPP_HIDE_FROM_ABI __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_;
Expand Down Expand Up @@ -725,8 +749,9 @@ public:
}

_LIBCPP_HIDE_FROM_ABI const_iterator begin() const _NOEXCEPT {
__map_const_pointer __mp = static_cast<__map_const_pointer>(__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 {
Expand All @@ -736,9 +761,9 @@ 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);
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()); }
Expand Down Expand Up @@ -2335,7 +2360,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;
Expand All @@ -2362,7 +2387,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;
Expand All @@ -2388,7 +2413,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;
Expand Down Expand Up @@ -2420,7 +2445,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_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
template <class T>
class small_pointer {
std::uint16_t offset;

public:
T& operator*() const;
};

template <class T>
Expand Down
Loading