[libcxx-commits] [libcxx] [libcxx] Align allocation to match `__set_long_cap` and `__get_long_cap` (PR #90292)

Vitaly Buka via libcxx-commits libcxx-commits at lists.llvm.org
Tue Apr 30 14:37:28 PDT 2024


https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/90292

>From 3d88874bd17fe8d0625566c406fca0f978cf653e Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Fri, 26 Apr 2024 15:03:40 -0700
Subject: [PATCH 1/4] [libcxx] Add assert into __set_long_cap

---
 libcxx/include/string | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libcxx/include/string b/libcxx/include/string
index 883bc1d7e5dc9f..c72241ff00acf8 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1876,6 +1876,7 @@ private:
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __set_long_cap(size_type __s) _NOEXCEPT {
     __r_.first().__l.__cap_     = __s / __endian_factor;
     __r_.first().__l.__is_long_ = true;
+    _LIBCPP_ASSERT_INTERNAL(__s == __get_long_cap(), "Size must be __endian_factor aligned.");
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type __get_long_cap() const _NOEXCEPT {

>From e5845121e74d83468ae0d0ec34406eb57f1d6fe9 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Fri, 26 Apr 2024 16:04:59 -0700
Subject: [PATCH 2/4] [libcxx] Align allocation to __set_long_cap and
 __get_long_cap match

This is detected by asan after #83774

New assert already fails a lot of tests even without asan.
---
 libcxx/include/string | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index c72241ff00acf8..ff9c7b81375c9c 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -868,7 +868,7 @@ private:
       __r_.first() = __rep();
       __set_short_size(__size);
     } else {
-      auto __capacity   = __recommend(__size) + 1;
+      auto __capacity   = __align_it<__endian_factor>(__recommend(__size) + 1);
       auto __allocation = __alloc_traits::allocate(__alloc(), __capacity);
       __begin_lifetime(__allocation, __capacity);
       __set_long_cap(__capacity);
@@ -2202,7 +2202,7 @@ basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s, size_ty
     __set_short_size(__sz);
     __p = __get_short_pointer();
   } else {
-    auto __allocation = std::__allocate_at_least(__alloc(), __recommend(__reserve) + 1);
+    auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__recommend(__reserve) + 1));
     __p               = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
     __set_long_pointer(__p);
@@ -2226,7 +2226,7 @@ basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s, size_ty
     __set_short_size(__sz);
     __p = __get_short_pointer();
   } else {
-    auto __allocation = std::__allocate_at_least(__alloc(), __recommend(__sz) + 1);
+    auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__recommend(__sz) + 1));
     __p               = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
     __set_long_pointer(__p);
@@ -2251,7 +2251,7 @@ basic_string<_CharT, _Traits, _Allocator>::__init_copy_ctor_external(const value
   } else {
     if (__sz > max_size())
       __throw_length_error();
-    auto __allocation = std::__allocate_at_least(__alloc(), __recommend(__sz) + 1);
+    auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__recommend(__sz) + 1));
     __p               = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
     __set_long_pointer(__p);
@@ -2274,7 +2274,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
     __set_short_size(__n);
     __p = __get_short_pointer();
   } else {
-    auto __allocation = std::__allocate_at_least(__alloc(), __recommend(__n) + 1);
+    auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__recommend(__n) + 1));
     __p               = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
     __set_long_pointer(__p);
@@ -2339,7 +2339,7 @@ basic_string<_CharT, _Traits, _Allocator>::__init_with_size(_InputIterator __fir
     __p = __get_short_pointer();
 
   } else {
-    auto __allocation = std::__allocate_at_least(__alloc(), __recommend(__sz) + 1);
+    auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__recommend(__sz) + 1));
     __p               = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
     __set_long_pointer(__p);
@@ -2379,7 +2379,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
   size_type __cap =
       __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
   __annotate_delete();
-  auto __allocation = std::__allocate_at_least(__alloc(), __cap + 1);
+  auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__cap + 1));
   pointer __p       = __allocation.ptr;
   __begin_lifetime(__p, __allocation.count);
   if (__n_copy != 0)
@@ -2422,7 +2422,7 @@ void _LIBCPP_CONSTEXPR_SINCE_CXX20
   size_type __cap =
       __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
   __annotate_delete();
-  auto __allocation = std::__allocate_at_least(__alloc(), __cap + 1);
+  auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__cap + 1));
   pointer __p       = __allocation.ptr;
   __begin_lifetime(__p, __allocation.count);
   if (__n_copy != 0)
@@ -3255,14 +3255,14 @@ basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target
     __p        = __get_long_pointer();
   } else {
     if (__target_capacity > __cap) {
-      auto __allocation = std::__allocate_at_least(__alloc(), __target_capacity + 1);
+      auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__target_capacity + 1));
       __new_data        = __allocation.ptr;
       __target_capacity = __allocation.count - 1;
     } else {
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
       try {
 #endif // _LIBCPP_HAS_NO_EXCEPTIONS
-        auto __allocation = std::__allocate_at_least(__alloc(), __target_capacity + 1);
+        auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__target_capacity + 1));
         __new_data        = __allocation.ptr;
         __target_capacity = __allocation.count - 1;
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS

>From cc2f5193db42357befe51b6e0fd752ce20b71f3c Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Tue, 30 Apr 2024 11:19:51 -0700
Subject: [PATCH 3/4] rev

---
 libcxx/include/string | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index ff9c7b81375c9c..c72241ff00acf8 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -868,7 +868,7 @@ private:
       __r_.first() = __rep();
       __set_short_size(__size);
     } else {
-      auto __capacity   = __align_it<__endian_factor>(__recommend(__size) + 1);
+      auto __capacity   = __recommend(__size) + 1;
       auto __allocation = __alloc_traits::allocate(__alloc(), __capacity);
       __begin_lifetime(__allocation, __capacity);
       __set_long_cap(__capacity);
@@ -2202,7 +2202,7 @@ basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s, size_ty
     __set_short_size(__sz);
     __p = __get_short_pointer();
   } else {
-    auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__recommend(__reserve) + 1));
+    auto __allocation = std::__allocate_at_least(__alloc(), __recommend(__reserve) + 1);
     __p               = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
     __set_long_pointer(__p);
@@ -2226,7 +2226,7 @@ basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s, size_ty
     __set_short_size(__sz);
     __p = __get_short_pointer();
   } else {
-    auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__recommend(__sz) + 1));
+    auto __allocation = std::__allocate_at_least(__alloc(), __recommend(__sz) + 1);
     __p               = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
     __set_long_pointer(__p);
@@ -2251,7 +2251,7 @@ basic_string<_CharT, _Traits, _Allocator>::__init_copy_ctor_external(const value
   } else {
     if (__sz > max_size())
       __throw_length_error();
-    auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__recommend(__sz) + 1));
+    auto __allocation = std::__allocate_at_least(__alloc(), __recommend(__sz) + 1);
     __p               = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
     __set_long_pointer(__p);
@@ -2274,7 +2274,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
     __set_short_size(__n);
     __p = __get_short_pointer();
   } else {
-    auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__recommend(__n) + 1));
+    auto __allocation = std::__allocate_at_least(__alloc(), __recommend(__n) + 1);
     __p               = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
     __set_long_pointer(__p);
@@ -2339,7 +2339,7 @@ basic_string<_CharT, _Traits, _Allocator>::__init_with_size(_InputIterator __fir
     __p = __get_short_pointer();
 
   } else {
-    auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__recommend(__sz) + 1));
+    auto __allocation = std::__allocate_at_least(__alloc(), __recommend(__sz) + 1);
     __p               = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
     __set_long_pointer(__p);
@@ -2379,7 +2379,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
   size_type __cap =
       __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
   __annotate_delete();
-  auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__cap + 1));
+  auto __allocation = std::__allocate_at_least(__alloc(), __cap + 1);
   pointer __p       = __allocation.ptr;
   __begin_lifetime(__p, __allocation.count);
   if (__n_copy != 0)
@@ -2422,7 +2422,7 @@ void _LIBCPP_CONSTEXPR_SINCE_CXX20
   size_type __cap =
       __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
   __annotate_delete();
-  auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__cap + 1));
+  auto __allocation = std::__allocate_at_least(__alloc(), __cap + 1);
   pointer __p       = __allocation.ptr;
   __begin_lifetime(__p, __allocation.count);
   if (__n_copy != 0)
@@ -3255,14 +3255,14 @@ basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target
     __p        = __get_long_pointer();
   } else {
     if (__target_capacity > __cap) {
-      auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__target_capacity + 1));
+      auto __allocation = std::__allocate_at_least(__alloc(), __target_capacity + 1);
       __new_data        = __allocation.ptr;
       __target_capacity = __allocation.count - 1;
     } else {
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
       try {
 #endif // _LIBCPP_HAS_NO_EXCEPTIONS
-        auto __allocation = std::__allocate_at_least(__alloc(), __align_it<__endian_factor>(__target_capacity + 1));
+        auto __allocation = std::__allocate_at_least(__alloc(), __target_capacity + 1);
         __new_data        = __allocation.ptr;
         __target_capacity = __allocation.count - 1;
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS

>From f89e557365baebd5720ef954bb13aca3a72fa500 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Tue, 30 Apr 2024 14:33:57 -0700
Subject: [PATCH 4/4] @philnik777 proposal

---
 libcxx/include/string | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index c72241ff00acf8..aac2b737127ea3 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1960,10 +1960,10 @@ private:
     if (__s < __min_cap) {
       return static_cast<size_type>(__min_cap) - 1;
     }
-    const size_type __boundary = sizeof(value_type) < __alignment ? __alignment / sizeof(value_type) : 1;
+    const size_type __boundary = sizeof(value_type) < __alignment ? __alignment / sizeof(value_type) : __endian_factor;
     size_type __guess          = __align_it<__boundary>(__s + 1) - 1;
     if (__guess == __min_cap)
-      ++__guess;
+      __guess += 2;
     return __guess;
   }
 



More information about the libcxx-commits mailing list