diff options
author | Marc Mutz <marc.mutz@qt.io> | 2021-12-08 17:27:52 +0100 |
---|---|---|
committer | Marc Mutz <marc.mutz@qt.io> | 2021-12-09 18:59:55 +0000 |
commit | e297e80fd0ec6ce4c97ee1b40426c76377b45ecc (patch) | |
tree | cf6ee3f6f0a2bcb2ac57a63035d1f0ceba914a50 /src | |
parent | c1f510b3596eaefb54cec83206cccbfecd25fc3b (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.h | 11 | ||||
-rw-r--r-- | src/corelib/tools/qvarlengtharray.h | 46 |
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> |