From 519d2d8f442409e86a0ee2fa16bd543342180861 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 14 Feb 2023 09:26:04 +0100 Subject: QVarLengthArray: fix memory leak in (qsizetype) ctor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of duplicating the logic of resize() to avoid three stores to (ptr, a, s), and getting the memory management wrong (malloc()ed ptr leaked when a following default-construction failed), simply call the QVLA default ctor followed by std::uninitialized_default_construct_n(). After we called the QVLA default ctor, the lifetime of this object has begun, so if the following code throws, ~QVLA will be called to clean up. This was not the case when we didn't delegate the ctor: if the body of this ctor threw, ~QVLA would _not_ have been called, since the lifetime of the object had never started. Since the dtor may now run, we need to maintain the class invariants, so we can't just set the size() before we have constructed all elements. This is where std::uninitialized_default_construct_n() comes in: it's strongly exception-safe, so if a constructor throws, it will destroy all previously-constructed elements, so that either we fail and size() == 0 is true or it doesn't, then we can set size() to asize. Change-Id: I03408d8a473e8a31fa0086ef5ac551ce46a758f8 Reviewed-by: Qt CI Bot Reviewed-by: Mårten Nordheim (cherry picked from commit 217f2ab3eb04b7489959e8e1134184e8071dbaf0) --- src/corelib/tools/qvarlengtharray.h | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/corelib/tools/qvarlengtharray.h b/src/corelib/tools/qvarlengtharray.h index 2c88fbef2b..02015be904 100644 --- a/src/corelib/tools/qvarlengtharray.h +++ b/src/corelib/tools/qvarlengtharray.h @@ -653,23 +653,22 @@ QVarLengthArray(InputIterator, InputIterator) -> QVarLengthArray; template Q_INLINE_TEMPLATE QVarLengthArray::QVarLengthArray(qsizetype asize) + : QVarLengthArray() { - this->s = asize; - static_assert(Prealloc > 0, "QVarLengthArray Prealloc must be greater than 0."); - Q_ASSERT_X(size() >= 0, "QVarLengthArray::QVarLengthArray()", "Size must be greater than or equal to 0."); - if (size() > Prealloc) { - this->ptr = malloc(size() * sizeof(T)); - Q_CHECK_PTR(data()); - this->a = size(); - } else { - this->ptr = this->array; - this->a = Prealloc; - } - if constexpr (QTypeInfo::isComplex) { - T *i = end(); - while (i != begin()) - new (--i) T; + Q_ASSERT_X(asize >= 0, "QVarLengthArray::QVarLengthArray(qsizetype)", + "Size must be greater than or equal to 0."); + + // historically, this ctor worked for non-copyable/non-movable T, so keep it working, why not? + // resize(asize) // this requires a movable or copyable T, can't use, need to do it by hand + + if (asize > Prealloc) { + this->ptr = malloc(asize * sizeof(T)); + Q_CHECK_PTR(this->ptr); + this->a = asize; } + if constexpr (QTypeInfo::isComplex) + std::uninitialized_default_construct_n(data(), asize); + this->s = asize; } template -- cgit v1.2.3