[libc++] Take the ABI break for `std::list`'s pointer UB unconditionally #100585

philnik777 · 2024-07-25T15:14:31Z

This ABI break only affects fancy pointer which have a different value representation when pointing to a base of T instead of T itself. This seems like a rather small set of fancy pointers, which themselves already represent a very small niche. This patch swaps a pointer to T with a pointer to base of T in a few library-internal types.

llvmbot · 2024-08-15T15:15:12Z

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

Patch is 21.11 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/100585.diff

2 Files Affected:

(modified) libcxx/include/__configuration/abi.h (-2)
(modified) libcxx/include/list (+61-73)

diff --git a/libcxx/include/__configuration/abi.h b/libcxx/include/__configuration/abi.h
index cbde7887becf1e..01e903417d9e88 100644
--- a/libcxx/include/__configuration/abi.h
+++ b/libcxx/include/__configuration/abi.h
@@ -24,8 +24,6 @@
 #  define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 // Fix deque iterator type in order to support incomplete types.
 #  define _LIBCPP_ABI_INCOMPLETE_TYPES_IN_DEQUE
-// Fix undefined behavior in how std::list stores its linked nodes.
-#  define _LIBCPP_ABI_LIST_REMOVE_NODE_POINTER_UB
 // Fix undefined behavior in  how __tree stores its end and parent nodes.
 #  define _LIBCPP_ABI_TREE_REMOVE_NODE_POINTER_UB
 // Fix undefined behavior in how __hash_table stores its pointer types.
diff --git a/libcxx/include/list b/libcxx/include/list
index 929c84de7be449..69651260f52059 100644
--- a/libcxx/include/list
+++ b/libcxx/include/list
@@ -271,19 +271,10 @@ struct __list_node_pointer_traits {
   typedef __rebind_pointer_t<_VoidPtr, __list_node<_Tp, _VoidPtr> > __node_pointer;
   typedef __rebind_pointer_t<_VoidPtr, __list_node_base<_Tp, _VoidPtr> > __base_pointer;
 
-#if defined(_LIBCPP_ABI_LIST_REMOVE_NODE_POINTER_UB)
-  typedef __base_pointer __link_pointer;
-#else
-  typedef __conditional_t<is_pointer<_VoidPtr>::value, __base_pointer, __node_pointer> __link_pointer;
-#endif
-
-  typedef __conditional_t<is_same<__link_pointer, __node_pointer>::value, __base_pointer, __node_pointer>
-      __non_link_pointer;
+  static _LIBCPP_HIDE_FROM_ABI __base_pointer __unsafe_link_pointer_cast(__base_pointer __p) { return __p; }
 
-  static _LIBCPP_HIDE_FROM_ABI __link_pointer __unsafe_link_pointer_cast(__link_pointer __p) { return __p; }
-
-  static _LIBCPP_HIDE_FROM_ABI __link_pointer __unsafe_link_pointer_cast(__non_link_pointer __p) {
-    return static_cast<__link_pointer>(static_cast<_VoidPtr>(__p));
+  static _LIBCPP_HIDE_FROM_ABI __base_pointer __unsafe_link_pointer_cast(__node_pointer __p) {
+    return static_cast<__base_pointer>(static_cast<_VoidPtr>(__p));
   }
 };
 
@@ -292,16 +283,13 @@ struct __list_node_base {
   typedef __list_node_pointer_traits<_Tp, _VoidPtr> _NodeTraits;
   typedef typename _NodeTraits::__node_pointer __node_pointer;
   typedef typename _NodeTraits::__base_pointer __base_pointer;
-  typedef typename _NodeTraits::__link_pointer __link_pointer;
 
-  __link_pointer __prev_;
-  __link_pointer __next_;
+  __base_pointer __prev_;
+  __base_pointer __next_;
 
-  _LIBCPP_HIDE_FROM_ABI __list_node_base()
-      : __prev_(_NodeTraits::__unsafe_link_pointer_cast(__self())),
-        __next_(_NodeTraits::__unsafe_link_pointer_cast(__self())) {}
+  _LIBCPP_HIDE_FROM_ABI __list_node_base() : __prev_(__self()), __next_(__self()) {}
 
-  _LIBCPP_HIDE_FROM_ABI explicit __list_node_base(__link_pointer __prev, __link_pointer __next)
+  _LIBCPP_HIDE_FROM_ABI explicit __list_node_base(__base_pointer __prev, __base_pointer __next)
       : __prev_(__prev), __next_(__next) {}
 
   _LIBCPP_HIDE_FROM_ABI __base_pointer __self() { return pointer_traits<__base_pointer>::pointer_to(*this); }
@@ -332,12 +320,12 @@ public:
 #endif
 
   typedef __list_node_base<_Tp, _VoidPtr> __base;
-  typedef typename __base::__link_pointer __link_pointer;
+  typedef typename __base::__base_pointer __base_pointer;
 
-  _LIBCPP_HIDE_FROM_ABI explicit __list_node(__link_pointer __prev, __link_pointer __next) : __base(__prev, __next) {}
+  _LIBCPP_HIDE_FROM_ABI explicit __list_node(__base_pointer __prev, __base_pointer __next) : __base(__prev, __next) {}
   _LIBCPP_HIDE_FROM_ABI ~__list_node() {}
 
-  _LIBCPP_HIDE_FROM_ABI __link_pointer __as_link() { return static_cast<__link_pointer>(__base::__self()); }
+  _LIBCPP_HIDE_FROM_ABI __base_pointer __as_link() { return __base::__self(); }
 };
 
 template <class _Tp, class _Alloc = allocator<_Tp> >
@@ -350,11 +338,11 @@ class _LIBCPP_TEMPLATE_VIS __list_const_iterator;
 template <class _Tp, class _VoidPtr>
 class _LIBCPP_TEMPLATE_VIS __list_iterator {
   typedef __list_node_pointer_traits<_Tp, _VoidPtr> _NodeTraits;
-  typedef typename _NodeTraits::__link_pointer __link_pointer;
+  typedef typename _NodeTraits::__base_pointer __base_pointer;
 
-  __link_pointer __ptr_;
+  __base_pointer __ptr_;
 
-  _LIBCPP_HIDE_FROM_ABI explicit __list_iterator(__link_pointer __p) _NOEXCEPT : __ptr_(__p) {}
+  _LIBCPP_HIDE_FROM_ABI explicit __list_iterator(__base_pointer __p) _NOEXCEPT : __ptr_(__p) {}
 
   template <class, class>
   friend class list;
@@ -408,11 +396,11 @@ public:
 template <class _Tp, class _VoidPtr>
 class _LIBCPP_TEMPLATE_VIS __list_const_iterator {
   typedef __list_node_pointer_traits<_Tp, _VoidPtr> _NodeTraits;
-  typedef typename _NodeTraits::__link_pointer __link_pointer;
+  typedef typename _NodeTraits::__base_pointer __base_pointer;
 
-  __link_pointer __ptr_;
+  __base_pointer __ptr_;
 
-  _LIBCPP_HIDE_FROM_ABI explicit __list_const_iterator(__link_pointer __p) _NOEXCEPT : __ptr_(__p) {}
+  _LIBCPP_HIDE_FROM_ABI explicit __list_const_iterator(__base_pointer __p) _NOEXCEPT : __ptr_(__p) {}
 
   template <class, class>
   friend class list;
@@ -485,8 +473,8 @@ protected:
   typedef typename __node_alloc_traits::pointer __node_pointer;
   typedef typename __node_alloc_traits::pointer __node_const_pointer;
   typedef __list_node_pointer_traits<value_type, __void_pointer> __node_pointer_traits;
-  typedef typename __node_pointer_traits::__link_pointer __link_pointer;
-  typedef __link_pointer __link_const_pointer;
+  typedef typename __node_pointer_traits::__base_pointer __base_pointer;
+  typedef __base_pointer __link_const_pointer;
   typedef typename __alloc_traits::pointer pointer;
   typedef typename __alloc_traits::const_pointer const_pointer;
   typedef typename __alloc_traits::difference_type difference_type;
@@ -499,7 +487,7 @@ protected:
   __node_base __end_;
   __compressed_pair<size_type, __node_allocator> __size_alloc_;
 
-  _LIBCPP_HIDE_FROM_ABI __link_pointer __end_as_link() const _NOEXCEPT {
+  _LIBCPP_HIDE_FROM_ABI __base_pointer __end_as_link() const _NOEXCEPT {
     return __node_pointer_traits::__unsafe_link_pointer_cast(const_cast<__node_base&>(__end_).__self());
   }
 
@@ -511,7 +499,7 @@ protected:
   _LIBCPP_HIDE_FROM_ABI size_type __node_alloc_max_size() const _NOEXCEPT {
     return __node_alloc_traits::max_size(__node_alloc());
   }
-  _LIBCPP_HIDE_FROM_ABI static void __unlink_nodes(__link_pointer __f, __link_pointer __l) _NOEXCEPT;
+  _LIBCPP_HIDE_FROM_ABI static void __unlink_nodes(__base_pointer __f, __base_pointer __l) _NOEXCEPT;
 
   _LIBCPP_HIDE_FROM_ABI __list_imp() _NOEXCEPT_(is_nothrow_default_constructible<__node_allocator>::value);
   _LIBCPP_HIDE_FROM_ABI __list_imp(const allocator_type& __a);
@@ -548,7 +536,7 @@ protected:
   }
 
   template <class... _Args>
-  _LIBCPP_HIDE_FROM_ABI __node_pointer __create_node(__link_pointer __prev, __link_pointer __next, _Args&&... __args) {
+  _LIBCPP_HIDE_FROM_ABI __node_pointer __create_node(__base_pointer __prev, __base_pointer __next, _Args&&... __args) {
     __node_allocator& __alloc = __node_alloc();
     __allocation_guard<__node_allocator> __guard(__alloc, 1);
     // Begin the lifetime of the node itself. Note that this doesn't begin the lifetime of the value
@@ -593,7 +581,7 @@ private:
 
 // Unlink nodes [__f, __l]
 template <class _Tp, class _Alloc>
-inline void __list_imp<_Tp, _Alloc>::__unlink_nodes(__link_pointer __f, __link_pointer __l) _NOEXCEPT {
+inline void __list_imp<_Tp, _Alloc>::__unlink_nodes(__base_pointer __f, __base_pointer __l) _NOEXCEPT {
   __f->__prev_->__next_ = __l->__next_;
   __l->__next_->__prev_ = __f->__prev_;
 }
@@ -621,8 +609,8 @@ __list_imp<_Tp, _Alloc>::~__list_imp() {
 template <class _Tp, class _Alloc>
 void __list_imp<_Tp, _Alloc>::clear() _NOEXCEPT {
   if (!empty()) {
-    __link_pointer __f = __end_.__next_;
-    __link_pointer __l = __end_as_link();
+    __base_pointer __f = __end_.__next_;
+    __base_pointer __l = __end_as_link();
     __unlink_nodes(__f, __l->__prev_);
     __sz() = 0;
     while (__f != __l) {
@@ -668,7 +656,7 @@ class _LIBCPP_TEMPLATE_VIS list : private __list_imp<_Tp, _Alloc> {
   typedef typename base::__node_alloc_traits __node_alloc_traits;
   typedef typename base::__node_base __node_base;
   typedef typename base::__node_base_pointer __node_base_pointer;
-  typedef typename base::__link_pointer __link_pointer;
+  typedef typename base::__base_pointer __base_pointer;
 
 public:
   typedef _Tp value_type;
@@ -917,9 +905,9 @@ private:
   template <class _Iterator, class _Sentinel>
   _LIBCPP_HIDE_FROM_ABI iterator __insert_with_sentinel(const_iterator __p, _Iterator __f, _Sentinel __l);
 
-  _LIBCPP_HIDE_FROM_ABI static void __link_nodes(__link_pointer __p, __link_pointer __f, __link_pointer __l);
-  _LIBCPP_HIDE_FROM_ABI void __link_nodes_at_front(__link_pointer __f, __link_pointer __l);
-  _LIBCPP_HIDE_FROM_ABI void __link_nodes_at_back(__link_pointer __f, __link_pointer __l);
+  _LIBCPP_HIDE_FROM_ABI static void __link_nodes(__base_pointer __p, __base_pointer __f, __base_pointer __l);
+  _LIBCPP_HIDE_FROM_ABI void __link_nodes_at_front(__base_pointer __f, __base_pointer __l);
+  _LIBCPP_HIDE_FROM_ABI void __link_nodes_at_back(__base_pointer __f, __base_pointer __l);
   _LIBCPP_HIDE_FROM_ABI iterator __iterator(size_type __n);
   // TODO: Make this _LIBCPP_HIDE_FROM_ABI
   template <class _Comp>
@@ -953,7 +941,7 @@ list(from_range_t, _Range&&, _Alloc = _Alloc()) -> list<ranges::range_value_t<_R
 
 // Link in nodes [__f, __l] just prior to __p
 template <class _Tp, class _Alloc>
-inline void list<_Tp, _Alloc>::__link_nodes(__link_pointer __p, __link_pointer __f, __link_pointer __l) {
+inline void list<_Tp, _Alloc>::__link_nodes(__base_pointer __p, __base_pointer __f, __base_pointer __l) {
   __p->__prev_->__next_ = __f;
   __f->__prev_          = __p->__prev_;
   __p->__prev_          = __l;
@@ -962,7 +950,7 @@ inline void list<_Tp, _Alloc>::__link_nodes(__link_pointer __p, __link_pointer _
 
 // Link in nodes [__f, __l] at the front of the list
 template <class _Tp, class _Alloc>
-inline void list<_Tp, _Alloc>::__link_nodes_at_front(__link_pointer __f, __link_pointer __l) {
+inline void list<_Tp, _Alloc>::__link_nodes_at_front(__base_pointer __f, __base_pointer __l) {
   __f->__prev_          = base::__end_as_link();
   __l->__next_          = base::__end_.__next_;
   __l->__next_->__prev_ = __l;
@@ -971,7 +959,7 @@ inline void list<_Tp, _Alloc>::__link_nodes_at_front(__link_pointer __f, __link_
 
 // Link in nodes [__f, __l] at the back of the list
 template <class _Tp, class _Alloc>
-inline void list<_Tp, _Alloc>::__link_nodes_at_back(__link_pointer __f, __link_pointer __l) {
+inline void list<_Tp, _Alloc>::__link_nodes_at_back(__base_pointer __f, __base_pointer __l) {
   __l->__next_          = base::__end_as_link();
   __f->__prev_          = base::__end_.__prev_;
   __f->__prev_->__next_ = __f;
@@ -1163,7 +1151,7 @@ list<_Tp, _Alloc>::insert(const_iterator __p, size_type __n, const value_type& _
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
     } catch (...) {
       while (true) {
-        __link_pointer __prev    = __e.__ptr_->__prev_;
+        __base_pointer __prev    = __e.__ptr_->__prev_;
         __node_pointer __current = __e.__ptr_->__as_node();
         this->__delete_node(__current);
         if (__prev == 0)
@@ -1205,7 +1193,7 @@ list<_Tp, _Alloc>::__insert_with_sentinel(const_iterator __p, _Iterator __f, _Se
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
     } catch (...) {
       while (true) {
-        __link_pointer __prev    = __e.__ptr_->__prev_;
+        __base_pointer __prev    = __e.__ptr_->__prev_;
         __node_pointer __current = __e.__ptr_->__as_node();
         this->__delete_node(__current);
         if (__prev == 0)
@@ -1224,7 +1212,7 @@ list<_Tp, _Alloc>::__insert_with_sentinel(const_iterator __p, _Iterator __f, _Se
 template <class _Tp, class _Alloc>
 void list<_Tp, _Alloc>::push_front(const value_type& __x) {
   __node_pointer __node = this->__create_node(/* prev = */ nullptr, /* next = */ nullptr, __x);
-  __link_pointer __nl   = __node->__as_link();
+  __base_pointer __nl   = __node->__as_link();
   __link_nodes_at_front(__nl, __nl);
   ++base::__sz();
 }
@@ -1232,7 +1220,7 @@ void list<_Tp, _Alloc>::push_front(const value_type& __x) {
 template <class _Tp, class _Alloc>
 void list<_Tp, _Alloc>::push_back(const value_type& __x) {
   __node_pointer __node = this->__create_node(/* prev = */ nullptr, /* next = */ nullptr, __x);
-  __link_pointer __nl   = __node->__as_link();
+  __base_pointer __nl   = __node->__as_link();
   __link_nodes_at_back(__nl, __nl);
   ++base::__sz();
 }
@@ -1242,7 +1230,7 @@ void list<_Tp, _Alloc>::push_back(const value_type& __x) {
 template <class _Tp, class _Alloc>
 void list<_Tp, _Alloc>::push_front(value_type&& __x) {
   __node_pointer __node = this->__create_node(/* prev = */ nullptr, /* next = */ nullptr, std::move(__x));
-  __link_pointer __nl   = __node->__as_link();
+  __base_pointer __nl   = __node->__as_link();
   __link_nodes_at_front(__nl, __nl);
   ++base::__sz();
 }
@@ -1250,7 +1238,7 @@ void list<_Tp, _Alloc>::push_front(value_type&& __x) {
 template <class _Tp, class _Alloc>
 void list<_Tp, _Alloc>::push_back(value_type&& __x) {
   __node_pointer __node = this->__create_node(/* prev = */ nullptr, /* next = */ nullptr, std::move(__x));
-  __link_pointer __nl   = __node->__as_link();
+  __base_pointer __nl   = __node->__as_link();
   __link_nodes_at_back(__nl, __nl);
   ++base::__sz();
 }
@@ -1265,7 +1253,7 @@ void
 list<_Tp, _Alloc>::emplace_front(_Args&&... __args) {
   __node_pointer __node =
       this->__create_node(/* prev = */ nullptr, /* next = */ nullptr, std::forward<_Args>(__args)...);
-  __link_pointer __nl = __node->__as_link();
+  __base_pointer __nl = __node->__as_link();
   __link_nodes_at_front(__nl, __nl);
   ++base::__sz();
 #  if _LIBCPP_STD_VER >= 17
@@ -1283,7 +1271,7 @@ void
 list<_Tp, _Alloc>::emplace_back(_Args&&... __args) {
   __node_pointer __node =
       this->__create_node(/* prev = */ nullptr, /* next = */ nullptr, std::forward<_Args>(__args)...);
-  __link_pointer __nl = __node->__as_link();
+  __base_pointer __nl = __node->__as_link();
   __link_nodes_at_back(__nl, __nl);
   ++base::__sz();
 #  if _LIBCPP_STD_VER >= 17
@@ -1296,7 +1284,7 @@ template <class... _Args>
 typename list<_Tp, _Alloc>::iterator list<_Tp, _Alloc>::emplace(const_iterator __p, _Args&&... __args) {
   __node_pointer __node =
       this->__create_node(/* prev = */ nullptr, /* next = */ nullptr, std::forward<_Args>(__args)...);
-  __link_pointer __nl = __node->__as_link();
+  __base_pointer __nl = __node->__as_link();
   __link_nodes(__p.__ptr_, __nl, __nl);
   ++base::__sz();
   return iterator(__nl);
@@ -1305,7 +1293,7 @@ typename list<_Tp, _Alloc>::iterator list<_Tp, _Alloc>::emplace(const_iterator _
 template <class _Tp, class _Alloc>
 typename list<_Tp, _Alloc>::iterator list<_Tp, _Alloc>::insert(const_iterator __p, value_type&& __x) {
   __node_pointer __node = this->__create_node(/* prev = */ nullptr, /* next = */ nullptr, std::move(__x));
-  __link_pointer __nl   = __node->__as_link();
+  __base_pointer __nl   = __node->__as_link();
   __link_nodes(__p.__ptr_, __nl, __nl);
   ++base::__sz();
   return iterator(__nl);
@@ -1316,7 +1304,7 @@ typename list<_Tp, _Alloc>::iterator list<_Tp, _Alloc>::insert(const_iterator __
 template <class _Tp, class _Alloc>
 void list<_Tp, _Alloc>::pop_front() {
   _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!empty(), "list::pop_front() called with empty list");
-  __link_pointer __n = base::__end_.__next_;
+  __base_pointer __n = base::__end_.__next_;
   base::__unlink_nodes(__n, __n);
   --base::__sz();
   this->__delete_node(__n->__as_node());
@@ -1325,7 +1313,7 @@ void list<_Tp, _Alloc>::pop_front() {
 template <class _Tp, class _Alloc>
 void list<_Tp, _Alloc>::pop_back() {
   _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!empty(), "list::pop_back() called on an empty list");
-  __link_pointer __n = base::__end_.__prev_;
+  __base_pointer __n = base::__end_.__prev_;
   base::__unlink_nodes(__n, __n);
   --base::__sz();
   this->__delete_node(__n->__as_node());
@@ -1334,8 +1322,8 @@ void list<_Tp, _Alloc>::pop_back() {
 template <class _Tp, class _Alloc>
 typename list<_Tp, _Alloc>::iterator list<_Tp, _Alloc>::erase(const_iterator __p) {
   _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__p != end(), "list::erase(iterator) called with a non-dereferenceable iterator");
-  __link_pointer __n = __p.__ptr_;
-  __link_pointer __r = __n->__next_;
+  __base_pointer __n = __p.__ptr_;
+  __base_pointer __r = __n->__next_;
   base::__unlink_nodes(__n, __n);
   --base::__sz();
   this->__delete_node(__n->__as_node());
@@ -1347,7 +1335,7 @@ typename list<_Tp, _Alloc>::iterator list<_Tp, _Alloc>::erase(const_iterator __f
   if (__f != __l) {
     base::__unlink_nodes(__f.__ptr_, __l.__ptr_->__prev_);
     while (__f != __l) {
-      __link_pointer __n = __f.__ptr_;
+      __base_pointer __n = __f.__ptr_;
       ++__f;
       --base::__sz();
       this->__delete_node(__n->__as_node());
@@ -1376,7 +1364,7 @@ void list<_Tp, _Alloc>::resize(size_type __n) {
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
     } catch (...) {
       while (true) {
-        __link_pointer __prev    = __e.__ptr_->__prev_;
+        __base_pointer __prev    = __e.__ptr_->__prev_;
         __node_pointer __current = __e.__ptr_->__as_node();
         this->__delete_node(__current);
         if (__prev == 0)
@@ -1400,7 +1388,7 @@ void list<_Tp, _Alloc>::resize(size_type __n, const value_type& __x) {
     size_type __ds        = 0;
     __node_pointer __node = this->__create_node(/* prev = */ nullptr, /* next = */ nullptr, __x);
     ++__ds;
-    __link_pointer __nl = __node->__as_link();
+    __base_pointer __nl = __node->__as_link();
     iterator __r        = iterator(__nl);
     iterator __e        = __r;
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
@@ -1412,7 +1400,7 @@ void list<_Tp, _Alloc>::resize(size_type __n, const value_type& __x) {
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
     } catch (...) {
       while (true) {
-        __link_pointer __prev    = __e.__ptr_->__prev_;
+        __base_pointer __prev    = __e.__ptr_->__prev_;
         __node_pointer __current = __e.__ptr_->__as_node();
         this->__delete_node(__current);
         if (__prev == 0)
@@ -1432,8 +1420,8 @@ void list<_Tp, _Alloc>::splice(const_iterator __p, list& __c) {
   _LIBCPP_ASSERT_VALID_INPUT_RANGE(
       this != std::addressof(__c), "list::splice(iterator, list) called with this == &list");
   if (!__c.empty()) {
-    __link_pointer __f = __c.__end_.__next_;
-    __link_pointer __l = __c.__end_.__prev_;
+    __base_pointer __f = __c.__end_.__next_;
+    __base_pointer __l = __c.__end_.__prev_;
     base::__unlink_nodes(__f, __l);
     __link_nodes(__p.__ptr_, __f, __l);
     base::__sz() += __c.__sz();
@@ -1444,7 +1432,7 @@ void list<_Tp, _Alloc>::splice(const_iterator __p, list& __c) {
 template <class _Tp, class _Alloc>
 void list<_Tp, _Alloc>::splice(const_iterator __p, list& __c, const_iterator __i) {
   if (__p.__ptr_ != __i.__ptr_ && __p.__ptr_ != __i.__ptr_->__next_) {
-    __link_pointer __f = __i.__ptr_;
+    __base_pointer __f = __i.__ptr_;
     base::__unlink_nodes(__f, __f);
     __link_nodes(__p.__ptr_, __f, __f);
     --__c.__sz();
@@ -1455,9 +1443,9 @@ void list<_Tp, _Alloc>::splice(const_iterator __p, list& __c, const_iterator __i
 template <class _Tp, class _Alloc>
 void list<_Tp, _Alloc>::splice(const_iterator __p, list& __c, const_iterator __f, const_iterator __l) {
   if (__f != __l) {
-    __link_pointer __first = __f.__ptr_;
+    __base_pointer __first = __f.__ptr_;
     --__l;
-    __link_pointer __last = __l.__ptr_;
+    __base_pointer __last = __l.__ptr_;
     if (this != std::addressof(__c)) {
       size_type __s = std::distance(__f, __l) + 1;
       __c.__sz() -= __s;
@@ -1545,8 +1533,8 @@ void list<_Tp, _Alloc>::merge(list& __c, _Comp __comp) {
           ;
         base::__sz() += __ds;
         __c.__sz() -= __ds;
-        __link_pointer __f = __f2.__ptr_;
-        __link_pointer __l = __m2.__ptr_->__prev_;
+        __base_pointer __f = __f2.__ptr_;
+        __base_pointer __l = __m2.__ptr_->__prev_;
         __f2               = __m2;
         base::__unlink_nodes(__f, __l);
         __m2 = std::next(__f1);
@@ -1580,7 +1568,7 @@ list<_Tp, _Alloc>::__sort(iterator __f1, iterator __e2, size_type __n, _Comp& __
     return __f1;
   case 2:
     if (__comp(*--...
[truncated]

ldionne

I think this ABI break is very unlikely to actually cause issues. However, as explained in the comments, I think there's a few things we can do to go out of our way to

catch unintended breakages for a bit of time
properly communicate the change being done to users and vendors

So overall, I think we can move forward with this after applying a few suggested changes.

libcxx/include/__configuration/abi.h

libcxx/include/list

ldionne · 2024-08-15T15:45:13Z

libcxx/include/list

  _LIBCPP_HIDE_FROM_ABI ~__list_node() {}

-  _LIBCPP_HIDE_FROM_ABI __link_pointer __as_link() { return static_cast<__link_pointer>(__base::__self()); }
+  _LIBCPP_HIDE_FROM_ABI __base_pointer __as_link() { return __base::__self(); }


It would be nice to clean up this __as_link function as a follow-up.

libcxx/include/list

libcxx/include/__configuration/abi.h

ldionne

LGTM with minor comments.

CC @llvm/libcxx-vendors for awareness.

libcxx/docs/ReleaseNotes/20.rst

libcxx/include/__hash_table

ldionne · 2024-09-12T16:46:58Z

libcxx/include/__hash_table

-#else
-  typedef __conditional_t<is_pointer<__node_pointer>::value, __node_base_pointer, __node_pointer> __next_pointer;
+
+#ifndef _LIBCPP_ABI_FIX_UNORDERED_NODE_POINTER_UB


Suggested change

#ifndef _LIBCPP_ABI_FIX_UNORDERED_NODE_POINTER_UB

// TODO(LLVM 22): Remove

#ifndef _LIBCPP_ABI_FIX_UNORDERED_NODE_POINTER_UB

Here and below.

philnik777 changed the title ~~[libc++] Take the ABI break for unconditionally~~ [libc++] Take the ABI break for std::node's pointer UB unconditionally Jul 25, 2024

philnik777 added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Aug 15, 2024

philnik777 changed the title ~~[libc++] Take the ABI break for std::node's pointer UB unconditionally~~ [libc++] Take the ABI break for std::list's pointer UB unconditionally Aug 15, 2024

ldionne marked this pull request as ready for review August 15, 2024 15:53

ldionne requested a review from a team as a code owner August 15, 2024 15:53

ldionne requested changes Aug 15, 2024

View reviewed changes

philnik777 force-pushed the remove_pointer_ub_abi_flag branch from 39601ef to 8ce53cb Compare August 17, 2024 08:47

ldionne reviewed Sep 12, 2024

View reviewed changes

ldionne approved these changes Sep 12, 2024

View reviewed changes

[libc++] Take the ABI break for unconditionally

0b78609

philnik777 force-pushed the remove_pointer_ub_abi_flag branch from 8ce53cb to 0b78609 Compare September 14, 2024 08:42

Merge branch 'main' into remove_pointer_ub_abi_flag

d5b54c6

ldionne merged commit 01df775 into llvm:main Sep 16, 2024
10 of 12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc++] Take the ABI break for `std::list`'s pointer UB unconditionally #100585

[libc++] Take the ABI break for `std::list`'s pointer UB unconditionally #100585

philnik777 commented Jul 25, 2024 •

edited

Loading

llvmbot commented Aug 15, 2024

ldionne left a comment

ldionne Aug 15, 2024

ldionne left a comment

ldionne Sep 12, 2024

	#ifndef _LIBCPP_ABI_FIX_UNORDERED_NODE_POINTER_UB
	// TODO(LLVM 22): Remove
	#ifndef _LIBCPP_ABI_FIX_UNORDERED_NODE_POINTER_UB

[libc++] Take the ABI break for std::list's pointer UB unconditionally #100585

[libc++] Take the ABI break for std::list's pointer UB unconditionally #100585

Conversation

philnik777 commented Jul 25, 2024 • edited Loading

llvmbot commented Aug 15, 2024

ldionne left a comment

Choose a reason for hiding this comment

ldionne Aug 15, 2024

Choose a reason for hiding this comment

ldionne left a comment

Choose a reason for hiding this comment

ldionne Sep 12, 2024

Choose a reason for hiding this comment

[libc++] Take the ABI break for `std::list`'s pointer UB unconditionally #100585

[libc++] Take the ABI break for `std::list`'s pointer UB unconditionally #100585

philnik777 commented Jul 25, 2024 •

edited

Loading