Fix some bugs in string

* We were only allocating `_Newcapacity` in `_Copy_assign`. This lead to memory corruption * We were not calling `_Tidy_deallocate` in `_Move_assign` which leaked memory of lhs * We did not swap proxies in `swap` if one of the strings was in SSO. Breaks with stateful allocators * We always performed work in `swap` even if self swapping
microsoft · Aug 27, 2021 · ea452a2 · ea452a2
1 parent 3a281cd
commit ea452a2
Show file tree

Hide file tree

Showing 2 changed files with 154 additions and 189 deletions.
diff --git a/stl/inc/xstring b/stl/inc/xstring
@@ -2780,38 +2780,7 @@ public:
         _Mypair._Myval2._Alloc_proxy(_GET_PROXY_ALLOCATOR(_Alty, _Getal()));
         _Tidy_init();
     }
-#endif // _HAS_CXX20
-
-private:
-    _CONSTEXPR20 void _Move_assign(basic_string& _Right, _Equal_allocators) noexcept {
-        _Tidy_deallocate();
-        _Pocma(_Getal(), _Right._Getal());
-        _Take_contents(_Right);
-    }
-
-    _CONSTEXPR20 void _Move_assign(basic_string& _Right, _Propagate_allocators) noexcept {
-        if (_Getal() == _Right._Getal()) {
-            _Move_assign(_Right, _Equal_allocators{});
-        } else {
-            // intentionally slams into noexcept on OOM, TRANSITION, VSO-466800
-            _Mypair._Myval2._Orphan_all();
-            _Mypair._Myval2._Reload_proxy(
-                _GET_PROXY_ALLOCATOR(_Alty, _Getal()), _GET_PROXY_ALLOCATOR(_Alty, _Right._Getal()));
-            _Pocma(_Getal(), _Right._Getal());
-            _Take_contents(_Right);
-        }
-    }
 
-    _CONSTEXPR20 void _Move_assign(basic_string& _Right, _No_propagate_allocators) {
-        if (_Getal() == _Right._Getal()) {
-            _Move_assign(_Right, _Equal_allocators{});
-        } else {
-            assign(_Right._Mypair._Myval2._Myptr(), _Right._Mypair._Myval2._Mysize);
-        }
-    }
-
-public:
-#if _HAS_CXX20
     _NODISCARD bool _Move_assign_from_buffer(_Elem* const _Right, const size_type _Size, const size_type _Res) {
         // Move assign from a buffer, used exclusively by basic_stringbuf; returns _Large_string_engaged()
         _Tidy_deallocate();
@@ -2857,11 +2826,30 @@ public:
 #endif // _HAS_CXX20
 
     _CONSTEXPR20 basic_string& operator=(basic_string&& _Right) noexcept(
-        noexcept(_Move_assign(_Right, _Choose_pocma<_Alty>{}))) {
-        if (this != _STD addressof(_Right)) {
-            _Move_assign(_Right, _Choose_pocma<_Alty>{});
+        !is_same_v<_Choose_pocma<_Alty>, _No_propagate_allocators>) {
+        if (this == _STD addressof(_Right)) {
+            return *this;
         }
 
+        using _POCMA = _Choose_pocma<_Alty>;
+        if constexpr (is_same_v<_POCMA, _Propagate_allocators>) {
+            if (_Getal() != _Right._Getal()) {
+                // intentionally slams into noexcept on OOM, TRANSITION, VSO-466800
+                _Mypair._Myval2._Orphan_all();
+                _Mypair._Myval2._Reload_proxy(
+                    _GET_PROXY_ALLOCATOR(_Alty, _Getal()), _GET_PROXY_ALLOCATOR(_Alty, _Right._Getal()));
+            }
+
+        } else if constexpr (is_same_v<_POCMA, _No_propagate_allocators>) {
+            if (_Getal() != _Right._Getal()) {
+                assign(_Right._Mypair._Myval2._Myptr(), _Right._Mypair._Myval2._Mysize);
+                return *this;
+            }
+        }
+
+        _Tidy_deallocate();
+        _Pocma(_Getal(), _Right._Getal());
+        _Take_contents(_Right);
         return *this;
     }
 
@@ -3064,7 +3052,7 @@ private:
             const auto _New_size     = _Right._Mypair._Myval2._Mysize;
             const auto _New_capacity = _Calculate_growth(_New_size, 0, _Right.max_size());
             auto _Right_al_non_const = _Right_al;
-            const auto _New_ptr      = _Right_al_non_const.allocate(_New_capacity); // throws
+            const auto _New_ptr      = _Right_al_non_const.allocate(_New_capacity + 1); // throws
 
 #if _HAS_CXX20
             if (_STD is_constant_evaluated()) { // Begin the lifetimes of the objects before copying to avoid UB
@@ -4111,13 +4099,11 @@ public:
                 _Right._Mypair._Myval2._Orphan_all();
             }
 
-            if (_My_large || _Right_large) {
-                _Mypair._Myval2._Swap_proxy_and_iterators(_Right._Mypair._Myval2);
-            }
+            _Mypair._Myval2._Swap_proxy_and_iterators(_Right._Mypair._Myval2);
 #endif // _ITERATOR_DEBUG_LEVEL != 0
-        }
 
-        _Swap_data(_Right);
+            _Swap_data(_Right);
+        }
     }
 
 #if _HAS_CXX17