From 0800947d7d2127eacaabf66ee1702d4980897041 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 9 Dec 2021 22:22:04 +0100 Subject: [PATCH] QVarLengthArray: fix UB (precondition violation) in range-erase() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the range-erase() function is called with an empty, valid range, it passed equal iterators to both the first and the last arguments of std::move(f, l, d) (the algorithm, not the cast). This is UB, since it violates the algorithm's precondition that d ∉ [f,l). For non-empty ranges, f > d, thus d < f, hence d ∉ [f,l), so everything is ok _there_. Fix the empty range case by returning early. Reviewers may question the precondition, expecting that std::move(f, l, f) just be a no-op, but the algorithm, as specified, performs self-move-assignments in that case, which need not be supported. In fact, the Hinnant criterion, itself only applicable if one calls for self-swap-safety, asks for self-move-assignment-safety only for objects in the moved-from state. QVarLengthArray, itself, meets only the Hinnant criterion; self-move-assignment of non-empty QVLAs invokes UB. So, the UB here is real. Qt 5.15 uses std::copy() here, which has the same precondition as std::move(), and the same fix applies there, too. [ChangeLog][QtCore][QVarLengthArray] Fixed a bug where range-erase() could invoke undefined behavior when called with an empty range. Pick-to: 6.2 5.15 Change-Id: I656aa09d025168d6d9ef088ad4c6954d216f0d54 Reviewed-by: Thiago Macieira --- src/corelib/tools/qvarlengtharray.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/corelib/tools/qvarlengtharray.h b/src/corelib/tools/qvarlengtharray.h index de13f2c8295..80326630458 100644 --- a/src/corelib/tools/qvarlengtharray.h +++ b/src/corelib/tools/qvarlengtharray.h @@ -682,6 +682,9 @@ Q_OUTOFLINE_TEMPLATE typename QVarLengthArray::iterator QVarLengthA qsizetype l = qsizetype(aend - cbegin()); qsizetype n = l - f; + if (n == 0) // avoid UB in std::move() below + return data() + f; + if constexpr (QTypeInfo::isComplex) { std::move(begin() + l, end(), QT_MAKE_CHECKED_ARRAY_ITERATOR(begin() + f, size() - f)); std::destroy(end() - n, end());