QVector: fix use of invalid iterators in removeAll()

The c2m() function which converts a const_iterator into an iterator
is a broken concept for an implicitly shared container such as
QVector, because the act of calling begin() as the starting
point already detaches and invalidates the c2m argument.

This could be fixed in c2m, but the bug wasn't even in c2m,
but in removeAll(), which called end() before c2m, so the c2m
argument was already invalidated when entering c2m.

The solution is to store the positions as indices instead of
iterators before calling the first detaching function.

Task-number: QTBUG-44592
Change-Id: I66cf4f1277e71148a4d5b5bbfb6a3369ad02db68
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
This commit is contained in:
Marc Mutz 2015-02-21 09:57:09 +01:00
parent 9a950655fe
commit 6716fe8cfd
2 changed files with 10 additions and 1 deletions

View File

@ -153,7 +153,9 @@ public:
const const_iterator ce = this->cend(), cit = std::find(this->cbegin(), ce, t); const const_iterator ce = this->cend(), cit = std::find(this->cbegin(), ce, t);
if (cit == ce) if (cit == ce)
return 0; return 0;
const iterator e = end(), it = std::remove(c2m(cit), e, t); // next operation detaches, so ce, cit may become invalidated:
const int firstFoundIdx = std::distance(this->cbegin(), cit);
const iterator e = end(), it = std::remove(begin() + firstFoundIdx, e, t);
const int result = std::distance(it, e); const int result = std::distance(it, e);
erase(it, e); erase(it, e);
return result; return result;

View File

@ -1500,11 +1500,18 @@ void tst_QVector::remove() const
QVERIFY(myvec.removeOne(val2)); QVERIFY(myvec.removeOne(val2));
QCOMPARE(myvec, QVector<T>() << val1 << val3 << val1 << val3 << val1 << val2 << val3); QCOMPARE(myvec, QVector<T>() << val1 << val3 << val1 << val3 << val1 << val2 << val3);
QVector<T> myvecCopy = myvec;
QVERIFY(myvecCopy.isSharedWith(myvec));
// removeAll() // removeAll()
QCOMPARE(myvec.removeAll(val4), 0); QCOMPARE(myvec.removeAll(val4), 0);
QVERIFY(myvecCopy.isSharedWith(myvec));
QCOMPARE(myvec.removeAll(val1), 3); QCOMPARE(myvec.removeAll(val1), 3);
QVERIFY(!myvecCopy.isSharedWith(myvec));
QCOMPARE(myvec, QVector<T>() << val3 << val3 << val2 << val3); QCOMPARE(myvec, QVector<T>() << val3 << val3 << val2 << val3);
myvecCopy = myvec;
QVERIFY(myvecCopy.isSharedWith(myvec));
QCOMPARE(myvec.removeAll(val2), 1); QCOMPARE(myvec.removeAll(val2), 1);
QVERIFY(!myvecCopy.isSharedWith(myvec));
QCOMPARE(myvec, QVector<T>() << val3 << val3 << val3); QCOMPARE(myvec, QVector<T>() << val3 << val3 << val3);
// remove rest // remove rest