summaryrefslogtreecommitdiffstats
path: root/src/corelib/kernel/qobjectdefs_impl.h
diff options
context:
space:
mode:
authorMarc Mutz <marc.mutz@qt.io>2023-02-10 09:16:51 +0100
committerMarc Mutz <marc.mutz@qt.io>2023-02-19 22:28:30 +0100
commita13fd415d689be046c13cc562cbbbb5df0e6506a (patch)
treeeec6e805cfe199a82457b673bf3b5b6bea8b7554 /src/corelib/kernel/qobjectdefs_impl.h
parentad63f047731ca4065f334ea961917adc37140d2d (diff)
QVarLengthArray: fix UBs in emplace()/insert() ([basic.life], broken class invariant)
There are two problems in emplace_impl() (the same code exists as rvalue insert() since 5.10): First, the old code updated size() at the end of the function. However, if, after constructing the new end element, one of the subsequent move-assignments fail (throws), then the class invariant that size() be the number of alive elements in the container is broken, with the immediate consequence that the QVLA dtor would not destroy this element, but surely other unpleasantness (UB) that the C++ lifetime rules decide to throw our way. Similarly, in the trivially-relocatable case, the memmove() starts the life-time of the new end object, so if the following placement new fails, we're in the same situation. The latter case is worse, though, since here we leave *b in some weird zombie state: the memmove() effectively ended its lifetime in the sense that one mustn't call the destructor on the source object after trivial relocation, but C++ doesn't agree and QVLA's dtor will happily call b->~T() as part of its cleanup. The other ugly thing is that we're using placement new into an object that C++ says is still alive. QString is trivially relocatable, but not trivially copyable, so we can't end a QString's lifetime by placement-new'ing a new QString instance into it without first having ended the old object's lifetime. The fix for both of these is, fortunately, the same: It's a rotate!™ By using emplace_back() + std::rotate(), we always place the new object in a spot that didn't contain an alive object (in the C++ sense) before, we always update the size() right after doing so, maintaining that invariant, and we then rotate() it into place, which doesn't leave zombie objects around. std::rotate() is such a fundamental algorithm that we should trust the STL implementors to have optimized it well: https://stackoverflow.com/questions/21160875/why-is-stdrotate-so-fast We know we can do better only for trivially-relocatable, but non-trivially-copyable types (ex: QString), so in order to not lose the memmove() optimization, we now fall back to std::rotate on raw memory for that case. Amends dd58ddd5d97f0663d5fafb7e81bff4fc7db13ba7. Manual conflict resolutions: - no q20::construct_at() in 6.4 - no QVLAStorage/Base/QVarLengthArray split in 6.2 - no emplace_back() in 6.2 (use append(rv) instead) - no emplace() in 6.2 (the same code here implements insert(rv)) - qsizetype (Qt 6) → int (Qt 5) - no constexpr-if in C++11 Change-Id: Iacce4488ca649502861e0ed4e084c9fad38cab47 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com> Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io> (cherry picked from commit e24df8bc726d12e80f3f1d14834f9305586fcc98) Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io> Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Diffstat (limited to 'src/corelib/kernel/qobjectdefs_impl.h')
0 files changed, 0 insertions, 0 deletions