summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorMarc Mutz <marc.mutz@qt.io>2021-12-08 17:27:52 +0100
committerMarc Mutz <marc.mutz@qt.io>2021-12-09 18:59:55 +0000
commite297e80fd0ec6ce4c97ee1b40426c76377b45ecc (patch)
treecf6ee3f6f0a2bcb2ac57a63035d1f0ceba914a50 /src
parentc1f510b3596eaefb54cec83206cccbfecd25fc3b (diff)
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>
Diffstat (limited to 'src')
-rw-r--r--src/corelib/tools/qcontainertools_impl.h11
-rw-r--r--src/corelib/tools/qvarlengtharray.h46
2 files changed, 30 insertions, 27 deletions
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
@@ -74,6 +74,15 @@ static constexpr bool q_points_into_range(const T *p, const T *b, const T *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)
{
if constexpr (QTypeInfo<T>::isRelocatable) {
@@ -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>