whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix UB in QFutureInterface: invalid casts from ResultStoreBase to ResultStore<> 

ResultStore never actually exists, only ResutStoreBase does. So casting to
ResultStore<T> and calling its  member functions is UB. Put the type dependent
function as template member functions within ResultStoreBase and so we don't
need QtPrivate::ResultStore anymore.
Same goes for the iterator.

Change-Id: I739b9d234ba2238977863df77fde3a4471a9abd2
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
bb10
Olivier Goffart 2017-02-27 12:13:13 +01:00 committed by Olivier Goffart (Woboq GmbH)
parent a170c974a5
commit 6797570a59
5 changed files with 122 additions and 145 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/corelib/thread/qfutureinterface.h
View File

@ -159,7 +159,7 @@ public:
~QFutureInterface()
{
if (!derefT())
resultStore().clear();
resultStoreBase().template clear<T>();
}
static QFutureInterface canceledResult()
@ -169,7 +169,7 @@ public:
{
other.refT();
if (!derefT())
resultStore().clear();
resultStoreBase().template clear<T>();
QFutureInterfaceBase::operator=(other);
return *this;
}
@ -184,11 +184,6 @@ public:
inline const T &resultReference(int index) const;
inline const T *resultPointer(int index) const;
inline QList<T> results();
private:
QtPrivate::ResultStore<T> &resultStore()
{ return static_cast<QtPrivate::ResultStore<T> &>(resultStoreBase()); }
const QtPrivate::ResultStore<T> &resultStore() const
{ return static_cast<const QtPrivate::ResultStore<T> &>(resultStoreBase()); }
};
template <typename T>
@ -199,15 +194,14 @@ inline void QFutureInterface<T>::reportResult(const T *result, int index)
return;
}
QtPrivate::ResultStore<T> &store = resultStore();
QtPrivate::ResultStoreBase &store = resultStoreBase();
if (store.filterMode()) {
const int resultCountBefore = store.count();
store.addResult(index, result);
store.addResult<T>(index, result);
this->reportResultsReady(resultCountBefore, resultCountBefore + store.count());
} else {
const int insertIndex = store.addResult(index, result);
const int insertIndex = store.addResult<T>(index, result);
this->reportResultsReady(insertIndex, insertIndex + 1);
}
}
@ -226,7 +220,7 @@ inline void QFutureInterface<T>::reportResults(const QVector<T> &_results, int b
return;
}
QtPrivate::ResultStore<T> &store = resultStore();
auto &store = resultStoreBase();
if (store.filterMode()) {
const int resultCountBefore = store.count();
@ -250,14 +244,14 @@ template <typename T>
inline const T &QFutureInterface<T>::resultReference(int index) const
{
QMutexLocker lock(mutex());
return resultStore().resultAt(index).value();
return resultStoreBase().resultAt(index).template value<T>();
}
template <typename T>
inline const T *QFutureInterface<T>::resultPointer(int index) const
{
QMutexLocker lock(mutex());
return resultStore().resultAt(index).pointer();
return resultStoreBase().resultAt(index).template pointer<T>();
}
template <typename T>
@ -272,9 +266,9 @@ inline QList<T> QFutureInterface<T>::results()
QList<T> res;
QMutexLocker lock(mutex());
QtPrivate::ResultIterator<T> it = resultStore().begin();
while (it != resultStore().end()) {
res.append(it.value());
QtPrivate::ResultIteratorBase it = resultStoreBase().begin();
while (it != resultStoreBase().end()) {
res.append(it.value<T>());
++it;
}

6
src/corelib/thread/qresultstore.cpp
View File

@ -98,6 +98,12 @@ bool ResultIteratorBase::canIncrementVectorIndex() const
ResultStoreBase::ResultStoreBase()
: insertIndex(0), resultCount(0), m_filterMode(false), filteredResults(0) { }
ResultStoreBase::~ResultStoreBase()
{
// QFutureInterface's dtor must delete the contents of m_results.
Q_ASSERT(m_results.isEmpty());
}
void ResultStoreBase::setFilterMode(bool enable)
{
m_filterMode = enable;

64
src/corelib/thread/qresultstore.h
View File

@ -93,20 +93,14 @@ public:
protected:
QMap<int, ResultItem>::const_iterator mapIterator;
int m_vectorIndex;
};
template <typename T>
class ResultIterator : public ResultIteratorBase
{
public:
ResultIterator(const ResultIteratorBase &base)
: ResultIteratorBase(base) { }
template <typename T>
const T &value() const
{
return *pointer();
return *pointer<T>();
}
template <typename T>
const T *pointer() const
{
if (mapIterator.value().isVector())
@ -130,7 +124,7 @@ public:
ResultIteratorBase resultAt(int index) const;
bool contains(int index) const;
int count() const;
virtual ~ResultStoreBase() { }
virtual ~ResultStoreBase();
protected:
int insertResultItem(int index, ResultItem &resultItem);
@ -147,64 +141,44 @@ protected:
QMap<int, ResultItem> pendingResults;
int filteredResults;
};
template <typename T>
class ResultStore : public ResultStoreBase
{
public:
ResultStore() { }
ResultStore(const ResultStoreBase &base)
: ResultStoreBase(base) { }
int addResult(int index, const T *result)
template <typename T>
int addResult(int index, const T *result)
{
if (result == 0)
return ResultStoreBase::addResult(index, result);
return addResult(index, static_cast<void *>(nullptr));
else
return ResultStoreBase::addResult(index, new T(*result));
return addResult(index, static_cast<void *>(new T(*result)));
}
template <typename T>
int addResults(int index, const QVector<T> *results)
{
return ResultStoreBase::addResults(index, new QVector<T>(*results), results->count(), results->count());
return addResults(index, new QVector<T>(*results), results->count(), results->count());
}
template <typename T>
int addResults(int index, const QVector<T> *results, int totalCount)
{
if (m_filterMode == true && results->count() != totalCount && 0 == results->count())
return ResultStoreBase::addResults(index, 0, 0, totalCount);
return addResults(index, 0, 0, totalCount);
else
return ResultStoreBase::addResults(index, new QVector<T>(*results), results->count(), totalCount);
return addResults(index, new QVector<T>(*results), results->count(), totalCount);
}
int addCanceledResult(int index)
{
return addResult(index, 0);
return addResult(index, static_cast<void *>(nullptr));
}
template <typename T>
int addCanceledResults(int index, int _count)
{
QVector<T> empty;
return addResults(index, &empty, _count);
}
ResultIterator<T> begin() const
{
return static_cast<ResultIterator<T> >(ResultStoreBase::begin());
}
ResultIterator<T> end() const
{
return static_cast<ResultIterator<T> >(ResultStoreBase::end());
}
ResultIterator<T> resultAt(int index) const
{
return static_cast<ResultIterator<T> >(ResultStoreBase::resultAt(index));
}
template <typename T>
void clear()
{
QMap<int, ResultItem>::const_iterator mapIterator = m_results.constBegin();
@ -218,12 +192,6 @@ public:
resultCount = 0;
m_results.clear();
}
~ResultStore()
{
clear();
}
};
} // namespace QtPrivate

87
tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
View File

@ -43,6 +43,11 @@
# undef interface
#endif
struct ResultStoreInt : QtPrivate::ResultStoreBase
{
~ResultStoreInt() { clear<int>(); }
};
class tst_QFuture: public QObject
{
Q_OBJECT
@ -78,7 +83,7 @@ void tst_QFuture::resultStore()
int int2 = 2;
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
QCOMPARE(store.begin(), store.end());
QCOMPARE(store.resultAt(0), store.end());
QCOMPARE(store.resultAt(1), store.end());
@ -86,9 +91,9 @@ void tst_QFuture::resultStore()
{
QtPrivate::ResultStoreBase store;
store.addResult(-1, &int0); // note to self: adding a pointer to the stack here is ok since
store.addResult(1, &int1); // ResultStoreBase does not take ownership, only ResultStore<> does.
ResultStoreInt store;
store.addResult(-1, &int0);
store.addResult(1, &int1);
QtPrivate::ResultIteratorBase it = store.begin();
QCOMPARE(it.resultIndex(), 0);
QVERIFY(it == store.begin());
@ -108,9 +113,9 @@ void tst_QFuture::resultStore()
QVector<int> vec1 = QVector<int>() << 4 << 5;
{
QtPrivate::ResultStoreBase store;
store.addResults(-1, &vec0, 2, 2);
store.addResults(-1, &vec1, 2, 2);
ResultStoreInt store;
store.addResults(-1, &vec0, 2);
store.addResults(-1, &vec1, 2);
QtPrivate::ResultIteratorBase it = store.begin();
QCOMPARE(it.resultIndex(), 0);
QCOMPARE(it, store.begin());
@ -131,9 +136,9 @@ void tst_QFuture::resultStore()
QCOMPARE(it, store.end());
}
{
QtPrivate::ResultStoreBase store;
ResultStoreInt store;
store.addResult(-1, &int0);
store.addResults(-1, &vec1, 2, 2);
store.addResults(-1, &vec1, 2);
store.addResult(-1, &int1);
QtPrivate::ResultIteratorBase it = store.begin();
@ -162,7 +167,7 @@ void tst_QFuture::resultStore()
QCOMPARE(store.resultAt(4), store.end());
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.addResult(-1, &int0);
store.addResults(-1, &vec0);
store.addResult(-1, &int1);
@ -186,36 +191,36 @@ void tst_QFuture::resultStore()
++it;
QVERIFY(it == store.end());
QCOMPARE(store.resultAt(0).value(), int0);
QCOMPARE(store.resultAt(1).value(), vec0[0]);
QCOMPARE(store.resultAt(2).value(), vec0[1]);
QCOMPARE(store.resultAt(3).value(), int1);
QCOMPARE(store.resultAt(0).value<int>(), int0);
QCOMPARE(store.resultAt(1).value<int>(), vec0[0]);
QCOMPARE(store.resultAt(2).value<int>(), vec0[1]);
QCOMPARE(store.resultAt(3).value<int>(), int1);
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.addResult(-1, &int0);
store.addResults(-1, &vec0);
store.addResult(200, &int1);
QCOMPARE(store.resultAt(0).value(), int0);
QCOMPARE(store.resultAt(1).value(), vec0[0]);
QCOMPARE(store.resultAt(2).value(), vec0[1]);
QCOMPARE(store.resultAt(200).value(), int1);
QCOMPARE(store.resultAt(0).value<int>(), int0);
QCOMPARE(store.resultAt(1).value<int>(), vec0[0]);
QCOMPARE(store.resultAt(2).value<int>(), vec0[1]);
QCOMPARE(store.resultAt(200).value<int>(), int1);
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.addResult(1, &int1);
store.addResult(0, &int0);
store.addResult(-1, &int2);
QCOMPARE(store.resultAt(0).value(), int0);
QCOMPARE(store.resultAt(1).value(), int1);
QCOMPARE(store.resultAt(2).value(), int2);
QCOMPARE(store.resultAt(0).value<int>(), int0);
QCOMPARE(store.resultAt(1).value<int>(), int1);
QCOMPARE(store.resultAt(2).value<int>(), int2);
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
QCOMPARE(store.contains(0), false);
QCOMPARE(store.contains(1), false);
QCOMPARE(store.contains(INT_MAX), false);
@ -223,7 +228,7 @@ void tst_QFuture::resultStore()
{
// Test filter mode, where "gaps" in the result array aren't allowed.
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResult(0, &int0);
@ -257,7 +262,7 @@ void tst_QFuture::resultStore()
{
// test canceled results
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResult(0, &int0);
@ -294,7 +299,7 @@ void tst_QFuture::resultStore()
{
// test addResult return value
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResult(0, &int0);
@ -340,7 +345,7 @@ void tst_QFuture::resultStore()
{
// test resultCount in non-filtered mode. It should always be possible
// to iterate through the results 0 to resultCount.
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.addResult(0, &int0);
QCOMPARE(store.count(), 1);
@ -354,7 +359,7 @@ void tst_QFuture::resultStore()
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.addResult(2, &int0);
QCOMPARE(store.count(), 0);
@ -366,7 +371,7 @@ void tst_QFuture::resultStore()
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.addResults(2, &vec1);
QCOMPARE(store.count(), 0);
@ -378,7 +383,7 @@ void tst_QFuture::resultStore()
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.addResults(2, &vec1);
QCOMPARE(store.count(), 0);
@ -386,7 +391,7 @@ void tst_QFuture::resultStore()
QCOMPARE(store.count(), 4);
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.addResults(3, &vec1);
QCOMPARE(store.count(), 0);
@ -398,7 +403,7 @@ void tst_QFuture::resultStore()
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResults(3, &vec1);
QCOMPARE(store.count(), 0);
@ -411,22 +416,22 @@ void tst_QFuture::resultStore()
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResults(3, &vec1);
QCOMPARE(store.count(), 0);
store.addCanceledResults(0, 3);
store.addCanceledResults<int>(0, 3);
QCOMPARE(store.count(), 2);
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResults(3, &vec1);
QCOMPARE(store.count(), 0);
store.addCanceledResults(0, 3);
store.addCanceledResults<int>(0, 3);
QCOMPARE(store.count(), 2); // results at 3 and 4 become available at index 0, 1
store.addResult(5, &int0);
@ -434,7 +439,7 @@ void tst_QFuture::resultStore()
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.addResult(1, &int0);
store.addResult(3, &int0);
store.addResults(6, &vec0);
@ -449,7 +454,7 @@ void tst_QFuture::resultStore()
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResult(1, &int0);
store.addResult(3, &int0);
@ -465,7 +470,7 @@ void tst_QFuture::resultStore()
store.addCanceledResult(0);
store.addCanceledResult(2);
store.addCanceledResults(4, 2);
store.addCanceledResults<int>(4, 2);
QCOMPARE(store.contains(0), true);
QCOMPARE(store.contains(1), true);
@ -477,7 +482,7 @@ void tst_QFuture::resultStore()
QCOMPARE(store.contains(7), false);
}
{
QtPrivate::ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addCanceledResult(0);
QCOMPARE(store.contains(0), false);

82
tests/auto/corelib/thread/qresultstore/tst_qresultstore.cpp
View File

@ -32,6 +32,11 @@
using namespace QtPrivate;
struct ResultStoreInt : ResultStoreBase
{
~ResultStoreInt() { clear<int>(); }
};
class tst_QtConcurrentResultStore : public QObject
{
Q_OBJECT
@ -67,20 +72,20 @@ void tst_QtConcurrentResultStore::init()
void tst_QtConcurrentResultStore::construction()
{
ResultStore<int> store;
ResultStoreBase store;
QCOMPARE(store.count(), 0);
}
void tst_QtConcurrentResultStore::iterators()
{
{
ResultStore<int> store;
ResultStoreBase store;
QCOMPARE(store.begin(), store.end());
QCOMPARE(store.resultAt(0), store.end());
QCOMPARE(store.resultAt(1), store.end());
}
{
ResultStoreBase storebase;
ResultStoreInt storebase;
storebase.addResult(-1, &int0); // note to self: adding a pointer to the stack here is ok since
storebase.addResult(1, &int1); // ResultStoreBase does not take ownership, only ResultStore<> does.
ResultIteratorBase it = storebase.begin();
@ -103,7 +108,7 @@ void tst_QtConcurrentResultStore::addResult()
{
{
// test addResult return value
ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
QCOMPARE(store.addResult(0, &int0), 0);
@ -149,10 +154,9 @@ void tst_QtConcurrentResultStore::addResult()
void tst_QtConcurrentResultStore::addResults()
{
ResultStoreBase store;
store.addResults(-1, &vec0, 2, 2);
store.addResults(-1, &vec1, 2, 2);
ResultStoreInt store;
store.addResults(-1, &vec0);
store.addResults(-1, &vec1);
ResultIteratorBase it = store.begin();
QCOMPARE(it.resultIndex(), 0);
QCOMPARE(it, store.begin());
@ -175,7 +179,7 @@ void tst_QtConcurrentResultStore::addResults()
void tst_QtConcurrentResultStore::resultIndex()
{
ResultStore<int> store;
ResultStoreInt store;
store.addResult(-1, &int0);
store.addResults(-1, &vec0);
store.addResult(-1, &int1);
@ -199,41 +203,41 @@ void tst_QtConcurrentResultStore::resultIndex()
++it;
QVERIFY(it == store.end());
QCOMPARE(store.resultAt(0).value(), int0);
QCOMPARE(store.resultAt(1).value(), vec0[0]);
QCOMPARE(store.resultAt(2).value(), vec0[1]);
QCOMPARE(store.resultAt(3).value(), int1);
QCOMPARE(store.resultAt(0).value<int>(), int0);
QCOMPARE(store.resultAt(1).value<int>(), vec0[0]);
QCOMPARE(store.resultAt(2).value<int>(), vec0[1]);
QCOMPARE(store.resultAt(3).value<int>(), int1);
}
void tst_QtConcurrentResultStore::resultAt()
{
{
ResultStore<int> store;
ResultStoreInt store;
store.addResult(-1, &int0);
store.addResults(-1, &vec0);
store.addResult(200, &int1);
QCOMPARE(store.resultAt(0).value(), int0);
QCOMPARE(store.resultAt(1).value(), vec0[0]);
QCOMPARE(store.resultAt(2).value(), vec0[1]);
QCOMPARE(store.resultAt(200).value(), int1);
QCOMPARE(store.resultAt(0).value<int>(), int0);
QCOMPARE(store.resultAt(1).value<int>(), vec0[0]);
QCOMPARE(store.resultAt(2).value<int>(), vec0[1]);
QCOMPARE(store.resultAt(200).value<int>(), int1);
}
{
ResultStore<int> store;
ResultStoreInt store;
store.addResult(1, &int1);
store.addResult(0, &int0);
store.addResult(-1, &int2);
QCOMPARE(store.resultAt(0).value(), int0);
QCOMPARE(store.resultAt(1).value(), int1);
QCOMPARE(store.resultAt(2).value(), int2);
QCOMPARE(store.resultAt(0).value<int>(), int0);
QCOMPARE(store.resultAt(1).value<int>(), int1);
QCOMPARE(store.resultAt(2).value<int>(), int2);
}
}
void tst_QtConcurrentResultStore::contains()
{
{
ResultStore<int> store;
ResultStoreInt store;
QCOMPARE(store.contains(0), false);
QCOMPARE(store.contains(1), false);
QCOMPARE(store.contains(INT_MAX), false);
@ -245,7 +249,7 @@ void tst_QtConcurrentResultStore::contains()
QVERIFY(store.contains(int2));
}
{
ResultStore<int> store;
ResultStoreInt store;
store.addResult(1, &int0);
store.addResult(3, &int0);
store.addResults(6, &vec0);
@ -260,7 +264,7 @@ void tst_QtConcurrentResultStore::contains()
}
{
ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResult(1, &int0);
store.addResult(3, &int0);
@ -276,7 +280,7 @@ void tst_QtConcurrentResultStore::contains()
store.addCanceledResult(0);
store.addCanceledResult(2);
store.addCanceledResults(4, 2);
store.addCanceledResults<int>(4, 2);
QCOMPARE(store.contains(0), true);
QCOMPARE(store.contains(1), true);
@ -288,7 +292,7 @@ void tst_QtConcurrentResultStore::contains()
QCOMPARE(store.contains(7), false);
}
{
ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addCanceledResult(0);
QCOMPARE(store.contains(0), false);
@ -302,7 +306,7 @@ void tst_QtConcurrentResultStore::contains()
void tst_QtConcurrentResultStore::filterMode()
{
// Test filter mode, where "gaps" in the result array aren't allowed.
ResultStore<int> store;
ResultStoreInt store;
QCOMPARE(store.filterMode(), false);
store.setFilterMode(true);
QVERIFY(store.filterMode());
@ -339,7 +343,7 @@ void tst_QtConcurrentResultStore::filterMode()
void tst_QtConcurrentResultStore::addCanceledResult()
{
// test canceled results
ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResult(0, &int0);
@ -379,7 +383,7 @@ void tst_QtConcurrentResultStore::count()
{
// test resultCount in non-filtered mode. It should always be possible
// to iterate through the results 0 to resultCount.
ResultStore<int> store;
ResultStoreInt store;
store.addResult(0, &int0);
QCOMPARE(store.count(), 1);
@ -393,7 +397,7 @@ void tst_QtConcurrentResultStore::count()
}
{
ResultStore<int> store;
ResultStoreInt store;
store.addResult(2, &int0);
QCOMPARE(store.count(), 0);
@ -405,7 +409,7 @@ void tst_QtConcurrentResultStore::count()
}
{
ResultStore<int> store;
ResultStoreInt store;
store.addResults(2, &vec1);
QCOMPARE(store.count(), 0);
@ -417,7 +421,7 @@ void tst_QtConcurrentResultStore::count()
}
{
ResultStore<int> store;
ResultStoreInt store;
store.addResults(2, &vec1);
QCOMPARE(store.count(), 0);
@ -425,7 +429,7 @@ void tst_QtConcurrentResultStore::count()
QCOMPARE(store.count(), 4);
}
{
ResultStore<int> store;
ResultStoreInt store;
store.addResults(3, &vec1);
QCOMPARE(store.count(), 0);
@ -437,7 +441,7 @@ void tst_QtConcurrentResultStore::count()
}
{
ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResults(3, &vec1);
QCOMPARE(store.count(), 0);
@ -450,22 +454,22 @@ void tst_QtConcurrentResultStore::count()
}
{
ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResults(3, &vec1);
QCOMPARE(store.count(), 0);
store.addCanceledResults(0, 3);
store.addCanceledResults<int>(0, 3);
QCOMPARE(store.count(), 2);
}
{
ResultStore<int> store;
ResultStoreInt store;
store.setFilterMode(true);
store.addResults(3, &vec1);
QCOMPARE(store.count(), 0);
store.addCanceledResults(0, 3);
store.addCanceledResults<int>(0, 3);
QCOMPARE(store.count(), 2); // results at 3 and 4 become available at index 0, 1
store.addResult(5, &int0);