<deque>: Properly destroy (fancy) pointers to blocks in the interna…

…l map (microsoft#2775) Co-authored-by: Michael Schellenberger Costa <[email protected]> Co-authored-by: Stephan T. Lavavej <[email protected]>
fsb4000 · Aug 13, 2022 · 0024e62 · 0024e62
1 parent 7fe1f99
commit 0024e62
Show file tree

Hide file tree

Showing 4 changed files with 255 additions and 23 deletions.
diff --git a/stl/inc/deque b/stl/inc/deque
@@ -693,7 +693,7 @@ private:
 
     void _Construct_n(size_type _Count, const _Ty& _Val) { // construct from _Count * _Val
         _Tidy_guard<deque> _Guard{this};
-        for (; 0 < _Count; --_Count) {
+        for (; _Count > 0; --_Count) {
             _Emplace_back_internal(_Val);
         }
 
@@ -964,7 +964,7 @@ public:
             _Newcapacity = _Block_size * _Minimum_map_size;
         }
 
-        if ((empty() && 0 < _Mapsize())
+        if ((empty() && _Mapsize() > 0)
             || (!empty() && size() <= _Newcapacity && _Newcapacity < _Oldcapacity)) { // worth shrinking, do it
             deque _Tmp(_STD make_move_iterator(begin()), _STD make_move_iterator(end()));
             swap(_Tmp);
@@ -976,7 +976,7 @@ public:
             emplace_back();
         }
 
-        while (_Newsize < _Mysize()) {
+        while (_Mysize() > _Newsize) {
             pop_back();
         }
     }
@@ -987,7 +987,7 @@ public:
             _Emplace_back_internal(_Val);
         }
 
-        while (_Newsize < _Mysize()) {
+        while (_Mysize() > _Newsize) {
             pop_back();
         }
     }
@@ -1181,15 +1181,15 @@ public:
         auto _Myfirst       = _Unchecked_begin();
         const auto _Oldsize = _Mysize();
         auto _Assign_count  = (_STD min)(_Count, _Oldsize);
-        for (; 0 < _Assign_count; --_Assign_count) {
+        for (; _Assign_count > 0; --_Assign_count) {
             *_Myfirst = _Val;
             ++_Myfirst;
         }
 
         const auto _Shrink_by = _Oldsize - _Assign_count;
         auto _Extend_by       = _Count - _Assign_count;
         _Erase_last_n(_Shrink_by);
-        for (; 0 < _Extend_by; --_Extend_by) {
+        for (; _Extend_by > 0; --_Extend_by) {
             _Emplace_back_internal(_Val);
         }
     }
@@ -1304,7 +1304,7 @@ public:
 
         auto _Off   = static_cast<size_type>(_First - begin());
         auto _Count = static_cast<size_type>(_Last - _First);
-        bool _Moved = 0 < _Off && _Off + _Count < _Mysize();
+        bool _Moved = _Off > 0 && _Off + _Count < _Mysize();
 
 #else // _ITERATOR_DEBUG_LEVEL == 2
         auto _Off   = static_cast<size_type>(_First - begin());
@@ -1317,12 +1317,12 @@ public:
 
         if (_Off < static_cast<size_type>(end() - _Last)) { // closer to front
             _STD move_backward(begin(), _First, _Last); // copy over hole
-            for (; 0 < _Count; --_Count) {
+            for (; _Count > 0; --_Count) {
                 pop_front(); // pop copied elements
             }
         } else { // closer to back
             _STD move(_Last, end(), _First); // copy over hole
-            for (; 0 < _Count; --_Count) {
+            for (; _Count > 0; --_Count) {
                 pop_back(); // pop copied elements
             }
         }
@@ -1338,7 +1338,7 @@ public:
 
 private:
     void _Erase_last_n(size_type _Count) noexcept {
-        for (; 0 < _Count; --_Count) {
+        for (; _Count > 0; --_Count) {
             pop_back();
         }
     }
@@ -1376,17 +1376,17 @@ private:
         if (_Off < _Rem) { // closer to front
             _Restore_old_size_guard<_Pop_direction::_Front> _Guard{this, _Oldsize};
             if (_Off < _Count) { // insert longer than prefix
-                for (_Num = _Count - _Off; 0 < _Num; --_Num) {
+                for (_Num = _Count - _Off; _Num > 0; --_Num) {
                     push_front(_Val); // push excess values
                 }
-                for (_Num = _Off; 0 < _Num; --_Num) {
+                for (_Num = _Off; _Num > 0; --_Num) {
                     push_front(begin()[static_cast<difference_type>(_Count - 1)]); // push prefix
                 }
 
                 _Mid = begin() + static_cast<difference_type>(_Count);
                 _STD fill(_Mid, _Mid + static_cast<difference_type>(_Off), _Val); // fill in rest of values
             } else { // insert not longer than prefix
-                for (_Num = _Count; 0 < _Num; --_Num) {
+                for (_Num = _Count; _Num > 0; --_Num) {
                     push_front(begin()[static_cast<difference_type>(_Count - 1)]); // push part of prefix
                 }
 
@@ -1402,7 +1402,7 @@ private:
             _Restore_old_size_guard<_Pop_direction::_Back> _Guard{this, _Oldsize};
             if (_Rem < _Count) { // insert longer than suffix
                 _Orphan_all();
-                for (_Num = _Count - _Rem; 0 < _Num; --_Num) {
+                for (_Num = _Count - _Rem; _Num > 0; --_Num) {
                     _Emplace_back_internal(_Val); // push excess values
                 }
                 for (_Num = 0; _Num < _Rem; ++_Num) {
@@ -1437,10 +1437,10 @@ private:
     }
 
     void _Growmap(size_type _Count) { // grow map by at least _Count pointers, _Mapsize() a power of 2
-        static_assert(1 < _Minimum_map_size, "The _Xlen() test should always be performed.");
+        static_assert(_Minimum_map_size > 1, "The _Xlen() test should always be performed.");
 
         _Alpty _Almap(_Getal());
-        size_type _Newsize = 0 < _Mapsize() ? _Mapsize() : 1;
+        size_type _Newsize = _Mapsize() > 0 ? _Mapsize() : 1;
         while (_Newsize - _Mapsize() < _Count || _Newsize < _Minimum_map_size) {
             // scale _Newsize to 2^N >= _Mapsize() + _Count
             if (max_size() / _Block_size - _Newsize < _Newsize) {
@@ -1466,8 +1466,8 @@ private:
             _Uninitialized_value_construct_n_unchecked1(_Myptr, _Count); // clear rest to initial block
         }
 
-        _Destroy_range(_Map() + _Myboff, _Map() + _Mapsize());
         if (_Map() != _Mapptr()) {
+            _Destroy_range(_Map(), _Map() + _Mapsize());
             _Almap.deallocate(_Map(), _Mapsize()); // free storage for old
         }
 
@@ -1483,14 +1483,14 @@ private:
             pop_back();
         }
 
-        for (size_type _Block = _Mapsize(); 0 < _Block;) { // free storage for a block and destroy pointer
-            if (_Map()[--_Block]) { // free block and destroy its pointer
-                _Getal().deallocate(_Map()[_Block], _Block_size);
-                _Destroy_in_place(_Map()[_Block]);
+        if (_Map() != _Mapptr()) {
+            for (size_type _Block = _Mapsize(); _Block > 0;) { // free storage for a block and destroy pointer
+                if (_Map()[--_Block]) { // free block
+                    _Getal().deallocate(_Map()[_Block], _Block_size);
+                }
+                _Destroy_in_place(_Map()[_Block]); // destroy pointer to block
             }
-        }
 
-        if (_Map() != _Mapptr()) {
             _Almap.deallocate(_Map(), _Mapsize()); // free storage for map
         }
 

diff --git a/tests/std/test.lst b/tests/std/test.lst
@@ -206,6 +206,7 @@ tests\GH_002581_common_reference_workaround
 tests\GH_002655_alternate_name_broke_linker
 tests\GH_002711_Zc_alignedNew-
 tests\GH_002760_syncstream_memory_leak
+tests\GH_002769_handle_deque_block_pointers
 tests\LWG2597_complex_branch_cut
 tests\LWG3018_shared_ptr_function
 tests\LWG3121_constrained_tuple_forwarding_ctor

diff --git a/tests/std/tests/GH_002769_handle_deque_block_pointers/env.lst b/tests/std/tests/GH_002769_handle_deque_block_pointers/env.lst
@@ -0,0 +1,4 @@
+# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+RUNALL_INCLUDE ..\usual_matrix.lst