Orphan before reallocation in vector<:ghost:> (#2441)

Co-authored-by: Stephan T. Lavavej <[email protected]>
microsoft · Mar 17, 2022 · 18b3c67 · 18b3c67
1 parent e6fa258
commit 18b3c67
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 11 deletions.
diff --git a/stl/inc/deque b/stl/inc/deque
@@ -722,7 +722,6 @@ public:
         _Take_contents(_Right);
     }
 
-public:
     deque& operator=(deque&& _Right) noexcept(_Alty_traits::is_always_equal::value) {
         if (this == _STD addressof(_Right)) {
             return *this;

diff --git a/stl/inc/vector b/stl/inc/vector
@@ -2727,11 +2727,6 @@ public:
 
     template <class _Iter, enable_if_t<_Is_iterator_v<_Iter>, int> = 0>
     _CONSTEXPR20 vector(_Iter _First, _Iter _Last, const _Alloc& _Al = _Alloc()) : _Mybase(_Al) {
-        _BConstruct(_First, _Last);
-    }
-
-    template <class _Iter>
-    _CONSTEXPR20 void _BConstruct(_Iter _First, _Iter _Last) {
         insert(begin(), _First, _Last);
     }
 
@@ -3197,7 +3192,9 @@ public:
                 _Xlen(); // result too long
             }
 
-            // worth doing
+#if _ITERATOR_DEBUG_LEVEL == 2
+            _Orphan_range(static_cast<size_type>(_Realloc ? 0 : _Off), this->_Mysize);
+#endif // _ITERATOR_DEBUG_LEVEL == 2
             this->_Myvec.resize(this->_Nw(size() + _Count), 0);
             if (empty()) {
                 this->_Mysize += _Count;
@@ -3206,10 +3203,6 @@ public:
                 this->_Mysize += _Count;
                 _STD copy_backward(begin() + _Off, _Oldend, end());
             }
-
-#if _ITERATOR_DEBUG_LEVEL == 2
-            _Orphan_range(static_cast<size_type>(_Realloc ? 0 : _Off), this->_Mysize);
-#endif // _ITERATOR_DEBUG_LEVEL == 2
         }
 
         return static_cast<size_type>(_Off);

diff --git a/tests/std/tests/P1004R2_constexpr_vector_bool/test.cpp b/tests/std/tests/P1004R2_constexpr_vector_bool/test.cpp
@@ -16,6 +16,39 @@ using namespace std;
 static constexpr bool input[]         = {true, false, true, true, false, true};
 static constexpr bool input_flipped[] = {false, true, false, false, true, false};
 
+struct demoterator { // demote pointer to input iterator
+    using iterator_category = input_iterator_tag;
+    using value_type        = int;
+    using difference_type   = ptrdiff_t;
+    using reference         = const int&;
+    using pointer           = void;
+
+    constexpr bool operator==(const demoterator& that) const {
+        return ptr == that.ptr;
+    }
+
+    constexpr reference operator*() const {
+        return *ptr;
+    }
+
+    constexpr demoterator& operator++() {
+        ++ptr;
+        return *this;
+    }
+    constexpr void operator++(int) {
+        ++*this;
+    }
+
+    const int* ptr;
+};
+
+// Just long enough to force a reallocation when inserting
+static constexpr int numbers[33] = { //
+    0, 1, 2, 3, 4, 5, 6, 7, 8, 9, //
+    0, 1, 2, 3, 4, 5, 6, 7, 8, 9, //
+    0, 1, 2, 3, 4, 5, 6, 7, 8, 9, //
+    0, 1, 2};
+
 template <typename T>
 struct soccc_allocator {
     using value_type = T;
@@ -462,6 +495,18 @@ constexpr bool test_interface() {
 
         emplaced.erase(emplaced.begin(), emplaced.begin() + 2);
         assert(emplaced.size() == 22);
+
+        {
+            // GH-2440: we were incorrectly reallocating _before_ orphaning iterators
+            // (resulting in UB) while inserting ranges of unknown length
+
+            vector<bool> input_inserted;
+            const auto result =
+                input_inserted.insert(input_inserted.end(), demoterator{begin(numbers)}, demoterator{end(numbers)});
+            static_assert(is_same_v<decltype(result), const vector<bool>::iterator>);
+            assert(result == input_inserted.begin());
+            assert(input_inserted.size() == size(numbers));
+        }
 #endif // __EDG__
     }