From e297e80fd0ec6ce4c97ee1b40426c76377b45ecc Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Wed, 8 Dec 2021 17:27:52 +0100
Subject: QVarLengthArray: make reallocation strongly exception safe
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 | 11 +++++++-
 src/corelib/tools/qvarlengtharray.h      | 46 ++++++++++++++------------------
 2 files changed, 30 insertions(+), 27 deletions(-)

(limited to 'src')

diff --git a/src/corelib/tools/qcontainertools_impl.h b/src/corelib/tools/qcontainertools_impl.h
index 2053de6408..723c984d81 100644
--- a/src/corelib/tools/qcontainertools_impl.h
+++ b/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);
     }
diff --git a/src/corelib/tools/qvarlengtharray.h b/src/corelib/tools/qvarlengtharray.h
index 621949f506..dfe15f439c 100644
--- a/src/corelib/tools/qvarlengtharray.h
+++ b/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
-            ptr = newPtr;
-            a = aalloc;
-        } else {
-            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;
-            }
+            newA = aalloc;
         } else {
-            memcpy(static_cast<void *>(data()), static_cast<const void *>(oldPtr), copySize * sizeof(T));
+            newPtr = reinterpret_cast<T *>(array);
+            newA = Prealloc;
         }
+        QtPrivate::q_uninitialized_relocate_n(oldPtr, copySize, newPtr);
+        // commit:
+        ptr = newPtr;
+        guard.release();
+        a = newA;
     }
     s = copySize;
 
@@ -540,6 +533,7 @@ Q_OUTOFLINE_TEMPLATE void QVarLengthArray<T, Prealloc>::reallocate(qsizetype asi
     } else {
         s = asize;
     }
+
 }
 
 template <class T, qsizetype Prealloc>
-- 
cgit v1.2.3