[libcxx-commits] [libcxx] [libc++] Fix strict aliasing violation for `deque::const_iterator` (PR #136067)

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/136067.diff


2 Files Affected:

- (modified) libcxx/docs/ReleaseNotes/21.rst (+8) 
- (modified) libcxx/include/deque (+8-9) 


``````````diff
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);
   }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/136067