diff options
author | Marc Mutz <marc.mutz@kdab.com> | 2017-11-22 01:05:52 +0100 |
---|---|---|
committer | Marc Mutz <marc.mutz@kdab.com> | 2017-11-28 21:58:50 +0000 |
commit | 67391f0a572ddc9c53bdff35dac893b07a862fe5 (patch) | |
tree | ecbbd9bdabea8cda10bf3d8bb7caadd313e582fe | |
parent | a0871ad225bc0a7ceed67fa2eb5ed16e475ebd93 (diff) | |
download | qtbase-67391f0a572ddc9c53bdff35dac893b07a862fe5.tar.gz |
Fix aliasing problem in QVector::removeAll()
Since removeAll() takes its argument by cref, if passing a reference
to an element of the container to removeAll(), the element may be
deleted (overwritten) by anyother value, leading to UB.
Add a test that actually happens to fail for me without the patch,
even though that might not be guaranteed (we may invoke UB).
Change-Id: If8c795113aeb515f4a9bdf1e072395b932295667
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
-rw-r--r-- | src/corelib/tools/qvector.h | 5 | ||||
-rw-r--r-- | tests/auto/corelib/tools/qvector/tst_qvector.cpp | 8 |
2 files changed, 11 insertions, 2 deletions
diff --git a/src/corelib/tools/qvector.h b/src/corelib/tools/qvector.h index da15d401e3..74c37faad0 100644 --- a/src/corelib/tools/qvector.h +++ b/src/corelib/tools/qvector.h @@ -164,9 +164,10 @@ public: const const_iterator ce = this->cend(), cit = std::find(this->cbegin(), ce, t); if (cit == ce) return 0; - // next operation detaches, so ce, cit may become invalidated: + // next operation detaches, so ce, cit, t may become invalidated: + const T tCopy = t; const int firstFoundIdx = std::distance(this->cbegin(), cit); - const iterator e = end(), it = std::remove(begin() + firstFoundIdx, e, t); + const iterator e = end(), it = std::remove(begin() + firstFoundIdx, e, tCopy); const int result = std::distance(it, e); erase(it, e); return result; diff --git a/tests/auto/corelib/tools/qvector/tst_qvector.cpp b/tests/auto/corelib/tools/qvector/tst_qvector.cpp index 374fec221e..6975452d76 100644 --- a/tests/auto/corelib/tools/qvector/tst_qvector.cpp +++ b/tests/auto/corelib/tools/qvector/tst_qvector.cpp @@ -245,6 +245,7 @@ private slots: void qhashInt() const { qhash<int>(); } void qhashMovable() const { qhash<Movable>(); } void qhashCustom() const { qhash<Custom>(); } + void removeAllWithAlias() const; void removeInt() const; void removeMovable() const; void removeCustom() const; @@ -1722,6 +1723,13 @@ void tst_QVector::prependCustom() const QCOMPARE(instancesCount, Custom::counter.loadAcquire()); } +void tst_QVector::removeAllWithAlias() const +{ + QVector<QString> strings; + strings << "One" << "Two" << "Three" << "One" /* must be distinct, but equal */; + QCOMPARE(strings.removeAll(strings.front()), 2); // will trigger asan/ubsan +} + template<typename T> void tst_QVector::remove() const { |