[libcxx-commits] [libcxx] 6a9c41f - [libc++] Set correct size at the end of growing std::string

Advenam Tacet via libcxx-commits libcxx-commits at lists.llvm.org
Fri Jul 14 19:00:47 PDT 2023


Author: Advenam Tacet
Date: 2023-07-15T04:00:30+02:00
New Revision: 6a9c41fdd4ca834a46fd866278c90a35f2375333

URL: https://github.com/llvm/llvm-project/commit/6a9c41fdd4ca834a46fd866278c90a35f2375333
DIFF: https://github.com/llvm/llvm-project/commit/6a9c41fdd4ca834a46fd866278c90a35f2375333.diff

LOG: [libc++] Set correct size at the end of growing std::string

This commit deprecates `std::basic_string::__grow_by`, which is part of ABIv1. The function is replaced by `std::basic_string:__grow_by_without_replace`, which is not part of ABI.

- The original function `__grow_by` is deprecated because it does not set the string size,  therefore it may not update the size when the size is changed, and it may also not set the size at all when the string was short initially. This leads to unpredictable size value. It is not removed or changed to avoid breaking the ABI.
- The commit adds `_LIBCPP_HIDE_FROM_ABI`  guarded by `_LIBCPP_ABI_VERSION >= 2` to `__grow_by`. This allows the function to be used in the dylib in ABIv1 without raising the `[abi:v170000]` error and removes it from future ABIs. `_LIBCPP_HIDE_FROM_ABI_AFTER_V1` cannot be used.
- Additionally, `__grow_by` has been removed from `_LIBCPP_STRING_UNSTABLE_EXTERN_TEMPLATE_LIST` in `libcxx/include/__string/extern_template_lists.h`.

This bugfix is necessary to implement string ASan annotations, because it mitigates the problems encountered in D132769.

Reviewed By: ldionne, #libc, philnik

Differential Revision: https://reviews.llvm.org/D148693

Added: 
    

Modified: 
    libcxx/include/__string/extern_template_lists.h
    libcxx/include/string

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__string/extern_template_lists.h b/libcxx/include/__string/extern_template_lists.h
index 41b71f342010a7..a833c000a2896b 100644
--- a/libcxx/include/__string/extern_template_lists.h
+++ b/libcxx/include/__string/extern_template_lists.h
@@ -104,7 +104,6 @@
   _Func(_LIBCPP_EXPORTED_FROM_ABI void basic_string<_CharType>::__init(size_type, value_type)) \
   _Func(_LIBCPP_EXPORTED_FROM_ABI basic_string<_CharType>& basic_string<_CharType>::insert(size_type, value_type const*)) \
   _Func(_LIBCPP_EXPORTED_FROM_ABI basic_string<_CharType>::size_type basic_string<_CharType>::find_last_of(value_type const*, size_type, size_type) const) \
-  _Func(_LIBCPP_EXPORTED_FROM_ABI void basic_string<_CharType>::__grow_by(size_type, size_type, size_type, size_type, size_type, size_type)) \
   _Func(_LIBCPP_EXPORTED_FROM_ABI void basic_string<_CharType>::__grow_by_and_replace(size_type, size_type, size_type, size_type, size_type, size_type, value_type const*)) \
   _Func(_LIBCPP_EXPORTED_FROM_ABI basic_string<_CharType>& basic_string<_CharType>::__assign_no_alias<false>(value_type const*, size_type)) \
   _Func(_LIBCPP_EXPORTED_FROM_ABI basic_string<_CharType>& basic_string<_CharType>::__assign_no_alias<true>(value_type const*, size_type)) \

diff  --git a/libcxx/include/string b/libcxx/include/string
index 69dcc8ea92e327..d4442507034a2b 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1791,7 +1791,7 @@ private:
         }
         else
         {
-            __grow_by(__cap, __sz + __n - __cap, __sz, __ip, 0, __n);
+            __grow_by_without_replace(__cap, __sz + __n - __cap, __sz, __ip, 0, __n);
             __p = std::__to_address(__get_long_pointer());
         }
         __sz += __n;
@@ -1919,8 +1919,15 @@ private:
     void __init_with_size(_InputIterator __first, _Sentinel __last, size_type __sz);
 
     _LIBCPP_CONSTEXPR_SINCE_CXX20
+#if _LIBCPP_ABI_VERSION >= 2 //  We want to use the function in the dylib in ABIv1
+    _LIBCPP_HIDE_FROM_ABI
+#endif
+    _LIBCPP_DEPRECATED_("use __grow_by_without_replace")
     void __grow_by(size_type __old_cap, size_type __delta_cap, size_type __old_sz,
                    size_type __n_copy,  size_type __n_del,     size_type __n_add = 0);
+    _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI
+    void __grow_by_without_replace(size_type __old_cap, size_type __delta_cap, size_type __old_sz,
+                   size_type __n_copy,  size_type __n_del,     size_type __n_add = 0);
     _LIBCPP_CONSTEXPR_SINCE_CXX20
     void __grow_by_and_replace(size_type __old_cap, size_type __delta_cap, size_type __old_sz,
                                size_type __n_copy,  size_type __n_del,
@@ -2336,9 +2343,16 @@ basic_string<_CharT, _Traits, _Allocator>::__grow_by_and_replace
     traits_type::assign(__p[__old_sz], value_type());
 }
 
+// __grow_by is deprecated because it does not set the size. It may not update the size when the size is changed, and it
+// may also not set the size at all when the string was short initially. This leads to unpredictable size value. It is
+// not removed or changed to avoid breaking the ABI.
 template <class _CharT, class _Traits, class _Allocator>
 void
 _LIBCPP_CONSTEXPR_SINCE_CXX20
+#if _LIBCPP_ABI_VERSION >= 2 // We want to use the function in the dylib in ABIv1
+    _LIBCPP_HIDE_FROM_ABI
+#endif
+    _LIBCPP_DEPRECATED_("use __grow_by_without_replace")
 basic_string<_CharT, _Traits, _Allocator>::__grow_by(size_type __old_cap, size_type __delta_cap, size_type __old_sz,
                                                      size_type __n_copy,  size_type __n_del,     size_type __n_add)
 {
@@ -2366,6 +2380,21 @@ basic_string<_CharT, _Traits, _Allocator>::__grow_by(size_type __old_cap, size_t
     __set_long_cap(__allocation.count);
 }
 
+template <class _CharT, class _Traits, class _Allocator>
+void _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI
+basic_string<_CharT, _Traits, _Allocator>::__grow_by_without_replace(
+    size_type __old_cap,
+    size_type __delta_cap,
+    size_type __old_sz,
+    size_type __n_copy,
+    size_type __n_del,
+    size_type __n_add) {
+    _LIBCPP_SUPPRESS_DEPRECATED_PUSH
+    __grow_by(__old_cap, __delta_cap, __old_sz, __n_copy, __n_del, __n_add);
+    _LIBCPP_SUPPRESS_DEPRECATED_POP
+    __set_long_size(__old_sz - __n_del + __n_add);
+}
+
 // assign
 
 template <class _CharT, class _Traits, class _Allocator>
@@ -2424,7 +2453,7 @@ basic_string<_CharT, _Traits, _Allocator>::assign(size_type __n, value_type __c)
     if (__cap < __n)
     {
         size_type __sz = size();
-        __grow_by(__cap, __n - __cap, __sz, 0, __sz);
+        __grow_by_without_replace(__cap, __n - __cap, __sz, 0, __sz);
     }
     value_type* __p = std::__to_address(__get_pointer());
     traits_type::assign(__p, __n, __c);
@@ -2568,7 +2597,7 @@ basic_string<_CharT, _Traits, _Allocator>::__assign_trivial(_Iterator __first, _
     // 2. In the exotic case where the input range is the byte representation of the string itself, the string
     //    object itself stays valid even if reallocation happens.
     size_type __sz = size();
-    __grow_by(__cap, __n - __cap, __sz, 0, __sz);
+    __grow_by_without_replace(__cap, __n - __cap, __sz, 0, __sz);
     }
     pointer __p = __get_pointer();
     for (; __first != __last; ++__p, (void) ++__first)
@@ -2657,7 +2686,7 @@ basic_string<_CharT, _Traits, _Allocator>::append(size_type __n, value_type __c)
         size_type __cap = capacity();
         size_type __sz = size();
         if (__cap - __sz < __n)
-            __grow_by(__cap, __sz + __n - __cap, __sz, __sz, 0);
+            __grow_by_without_replace(__cap, __sz + __n - __cap, __sz, __sz, 0);
         pointer __p = __get_pointer();
         traits_type::assign(std::__to_address(__p) + __sz, __n, __c);
         __sz += __n;
@@ -2676,7 +2705,7 @@ basic_string<_CharT, _Traits, _Allocator>::__append_default_init(size_type __n)
         size_type __cap = capacity();
         size_type __sz = size();
         if (__cap - __sz < __n)
-            __grow_by(__cap, __sz + __n - __cap, __sz, __sz, 0);
+            __grow_by_without_replace(__cap, __sz + __n - __cap, __sz, __sz, 0);
         pointer __p = __get_pointer();
         __sz += __n;
         __set_size(__sz);
@@ -2704,7 +2733,7 @@ basic_string<_CharT, _Traits, _Allocator>::push_back(value_type __c)
     }
     if (__sz == __cap)
     {
-        __grow_by(__cap, 1, __sz, __sz, 0);
+        __grow_by_without_replace(__cap, 1, __sz, __sz, 0);
         __is_short = false; // the string is always long after __grow_by
     }
     pointer __p = __get_pointer();
@@ -2737,7 +2766,7 @@ basic_string<_CharT, _Traits, _Allocator>::append(
             !__addr_in_range(*__first))
         {
             if (__cap - __sz < __n)
-                __grow_by(__cap, __sz + __n - __cap, __sz, __sz, 0);
+              __grow_by_without_replace(__cap, __sz + __n - __cap, __sz, __sz, 0);
             pointer __p = __get_pointer() + __sz;
             for (; __first != __last; ++__p, (void) ++__first)
                 traits_type::assign(*__p, *__first);
@@ -2850,7 +2879,7 @@ basic_string<_CharT, _Traits, _Allocator>::insert(size_type __pos, size_type __n
         }
         else
         {
-            __grow_by(__cap, __sz + __n - __cap, __sz, __pos, 0, __n);
+            __grow_by_without_replace(__cap, __sz + __n - __cap, __sz, __pos, 0, __n);
             __p = std::__to_address(__get_long_pointer());
         }
         traits_type::assign(__p + __pos, __n, __c);
@@ -2946,7 +2975,7 @@ basic_string<_CharT, _Traits, _Allocator>::insert(const_iterator __pos, value_ty
     value_type* __p;
     if (__cap == __sz)
     {
-        __grow_by(__cap, 1, __sz, __ip, 0, 1);
+        __grow_by_without_replace(__cap, 1, __sz, __ip, 0, 1);
         __p = std::__to_address(__get_long_pointer());
     }
     else
@@ -3037,7 +3066,7 @@ basic_string<_CharT, _Traits, _Allocator>::replace(size_type __pos, size_type __
     }
     else
     {
-        __grow_by(__cap, __sz - __n1 + __n2 - __cap, __sz, __pos, __n1, __n2);
+        __grow_by_without_replace(__cap, __sz - __n1 + __n2 - __cap, __sz, __pos, __n1, __n2);
         __p = std::__to_address(__get_long_pointer());
     }
     traits_type::assign(__p + __pos, __n2, __c);


        


More information about the libcxx-commits mailing list