QVarLengthArray: make reallocation strongly exception safe

The old code had several bugs:

- it immediately clobbered *this with new state, before having copied
  over the elements from the old to the new buffer

- when buffer relocation threw, it would keep the new (partially-filled)
  buffer and throw away the old

- it unconditionally used std::move() for non-relocatable types, making
  it impossible to restore the original buffer when a move throws

Instead of clobbering *this with new state, do all the work on the
side and change *this only once the reallocation has happened
successfully.

Also use q_uninitialized_relocate_n() and unique_ptr in the
implementation to simplify the code. The former got the necessary
update to use std::move_if_noexcept() instead of an unconditional
std::move() for the non-relocatable case.

[ChangeLog][QtCore][QVarLengthArray] The append()-like functions are
now strongly exception safe. This means reallocation will now use
copies instead of moves, unless the value_type has a noexcept move
constructor.

Fixes: QTBUG-99039
Change-Id: I031251b8d14ac045592d01caed59d4638c3d9892
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

src/corelib/tools/qcontainertools_impl.h

@@ -73,6 +73,15 @@ static constexpr bool q_points_into_range(const T *p, const T *b, const T *e,
     return !less(p, b) && less(p, e);
 }
 
+template <typename T, typename N>
+void q_uninitialized_move_if_noexcept_n(T* first, N n, T* out)
+{
+    if constexpr (std::is_nothrow_move_constructible_v<T> || !std::is_copy_constructible_v<T>)
+        std::uninitialized_move_n(first, n, out);
+    else
+        std::uninitialized_copy_n(first, n, out);
+}
+
 template <typename T, typename N>
 void q_uninitialized_relocate_n(T* first, N n, T* out)
 {
@@ -83,7 +92,7 @@ void q_uninitialized_relocate_n(T* first, N n, T* out)
                          n * sizeof(T));
         }
     } else {
-        std::uninitialized_move_n(first, n, out);
+        q_uninitialized_move_if_noexcept_n(first, n, out);
         if constexpr (QTypeInfo<T>::isComplex)
             std::destroy_n(first, n);
     }

src/corelib/tools/qvarlengtharray.h

@@ -49,7 +49,9 @@
 #include <algorithm>
 #include <initializer_list>
 #include <iterator>
+#include <memory>
 #include <new>
+
 #include <string.h>
 #include <stdlib.h>
 
@@ -487,38 +489,29 @@ Q_OUTOFLINE_TEMPLATE void QVarLengthArray<T, Prealloc>::reallocate(qsizetype asi
 
     const qsizetype copySize = qMin(asize, osize);
     Q_ASSUME(copySize >= 0);
+
     if (aalloc != capacity()) {
+        struct free_deleter {
+            void operator()(void *p) const noexcept { free(p); }
+        };
+        std::unique_ptr<void, free_deleter> guard;
+        T *newPtr;
+        qsizetype newA;
         if (aalloc > Prealloc) {
-            T *newPtr = reinterpret_cast<T *>(malloc(aalloc * sizeof(T)));
+            newPtr = reinterpret_cast<T *>(malloc(aalloc * sizeof(T)));
+            guard.reset(newPtr);
             Q_CHECK_PTR(newPtr); // could throw
             // by design: in case of QT_NO_EXCEPTIONS malloc must not fail or it crashes here
+            newA = aalloc;
+        } else {
+            newPtr = reinterpret_cast<T *>(array);
+            newA = Prealloc;
+        }
+        QtPrivate::q_uninitialized_relocate_n(oldPtr, copySize, newPtr);
+        // commit:
         ptr = newPtr;
-            a = aalloc;
+        guard.release();
-        } else {
+        a = newA;
-            ptr = reinterpret_cast<T *>(array);
-            a = Prealloc;
-        }
-        s = 0;
-        if constexpr (!QTypeInfo<T>::isRelocatable) {
-            QT_TRY {
-                // move all the old elements
-                while (size() < copySize) {
-                    new (end()) T(std::move(*(oldPtr+size())));
-                    (oldPtr+size())->~T();
-                    s++;
-                }
-            } QT_CATCH(...) {
-                // clean up all the old objects and then free the old ptr
-                qsizetype sClean = size();
-                while (sClean < osize)
-                    (oldPtr+(sClean++))->~T();
-                if (oldPtr != reinterpret_cast<T *>(array) && oldPtr != data())
-                    free(oldPtr);
-                QT_RETHROW;
-            }
-        } else {
-            memcpy(static_cast<void *>(data()), static_cast<const void *>(oldPtr), copySize * sizeof(T));
-        }
     }
     s = copySize;
 
@@ -540,6 +533,7 @@ Q_OUTOFLINE_TEMPLATE void QVarLengthArray<T, Prealloc>::reallocate(qsizetype asi
     } else {
         s = asize;
     }
+
 }
 
 template <class T, qsizetype Prealloc>

tests/auto/corelib/tools/qvarlengtharray/tst_qvarlengtharray.cpp

@@ -429,8 +429,6 @@ void tst_QVarLengthArray::appendIsStronglyExceptionSafe()
     };
 
     {
-        // ### TODO: QVLA isn't exception-safe when throwing during reallocation,
-        // ### so check with size() < capacity() for now
         QVarLengthArray<Thrower, 2> vla(1);
         {
             Thrower t;
@@ -443,6 +441,28 @@ void tst_QVarLengthArray::appendIsStronglyExceptionSafe()
             QVERIFY_THROWS_EXCEPTION(int, vla.push_back({}));
             QCOMPARE(vla.size(), 1);
         }
+        vla.push_back({});
+        QCOMPARE(vla.size(), 2);
+        {
+            Thrower t;
+            {
+                // tests the copy inside append()
+                const QScopedValueRollback rb(throwOnCopyNow, true);
+                QVERIFY_THROWS_EXCEPTION(int, vla.push_back(t));
+                QCOMPARE(vla.size(), 2);
+            }
+            {
+                // tests the move inside reallocate()
+                const QScopedValueRollback rb(throwOnMoveNow, true);
+                QVERIFY_THROWS_EXCEPTION(int, vla.push_back(t));
+                QCOMPARE(vla.size(), 2);
+            }
+        }
+        {
+            const QScopedValueRollback rb(throwOnMoveNow, true);
+            QVERIFY_THROWS_EXCEPTION(int, vla.push_back({}));
+            QCOMPARE(vla.size(), 2);
+        }
     }
 #endif
 }