From 992a37234c5ec7494fc2f24217d36400f6c1630a Mon Sep 17 00:00:00 2001 From: "Bradley T. Hughes" Date: Mon, 3 Oct 2011 15:21:02 +0200 Subject: [PATCH] Don't crash by modifying read-only shared_null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Functions that modify the d-pointer must detach or otherwise take measures to not modify the const, read-only shared_null. The setSharable(bool) function takes care to detach when setting sharable to false, but should avoid setting the sharable data member unless d is not the shared null. Similarly, QMap::setInsertInOrder() needs to detach if it is shared with the shared_null (the logic has been updated to be the same as setSharable()). Change-Id: Ida5cb9818b86695f1b9f0264418b955c56424898 Reviewed-on: http://codereview.qt-project.org/5929 Reviewed-by: Qt Sanity Bot Reviewed-by: Bradley T. Hughes Reviewed-by: Jan-Arve Sæther --- src/corelib/tools/qhash.h | 2 +- src/corelib/tools/qlinkedlist.h | 2 +- src/corelib/tools/qlist.h | 2 +- src/corelib/tools/qmap.h | 4 ++-- src/corelib/tools/qvector.h | 2 +- tests/auto/corelib/tools/qhash/tst_qhash.cpp | 13 +++++++++++++ tests/auto/corelib/tools/qlist/tst_qlist.cpp | 13 +++++++++++++ tests/auto/corelib/tools/qmap/tst_qmap.cpp | 17 +++++++++++++++++ .../auto/corelib/tools/qvector/tst_qvector.cpp | 13 +++++++++++++ 9 files changed, 62 insertions(+), 6 deletions(-) diff --git a/src/corelib/tools/qhash.h b/src/corelib/tools/qhash.h index 25cece4673..cc120aa09e 100644 --- a/src/corelib/tools/qhash.h +++ b/src/corelib/tools/qhash.h @@ -291,7 +291,7 @@ public: inline void detach() { if (d->ref != 1) detach_helper(); } inline bool isDetached() const { return d->ref == 1; } - inline void setSharable(bool sharable) { if (!sharable) detach(); d->sharable = sharable; } + inline void setSharable(bool sharable) { if (!sharable) detach(); if (d != &QHashData::shared_null) d->sharable = sharable; } inline bool isSharedWith(const QHash &other) const { return d == other.d; } void clear(); diff --git a/src/corelib/tools/qlinkedlist.h b/src/corelib/tools/qlinkedlist.h index bcad210d30..36cbc68eb8 100644 --- a/src/corelib/tools/qlinkedlist.h +++ b/src/corelib/tools/qlinkedlist.h @@ -97,7 +97,7 @@ public: inline void detach() { if (d->ref != 1) detach_helper(); } inline bool isDetached() const { return d->ref == 1; } - inline void setSharable(bool sharable) { if (!sharable) detach(); d->sharable = sharable; } + inline void setSharable(bool sharable) { if (!sharable) detach(); if (d != &QLinkedListData::shared_null) d->sharable = sharable; } inline bool isSharedWith(const QLinkedList &other) const { return d == other.d; } inline bool isEmpty() const { return d->size == 0; } diff --git a/src/corelib/tools/qlist.h b/src/corelib/tools/qlist.h index e9e44dbb10..5c8a58a196 100644 --- a/src/corelib/tools/qlist.h +++ b/src/corelib/tools/qlist.h @@ -143,7 +143,7 @@ public: } inline bool isDetached() const { return d->ref == 1; } - inline void setSharable(bool sharable) { if (!sharable) detach(); d->sharable = sharable; } + inline void setSharable(bool sharable) { if (!sharable) detach(); if (d != &QListData::shared_null) d->sharable = sharable; } inline bool isSharedWith(const QList &other) const { return d == other.d; } inline bool isEmpty() const { return p.isEmpty(); } diff --git a/src/corelib/tools/qmap.h b/src/corelib/tools/qmap.h index ff181d5071..8beae03a6e 100644 --- a/src/corelib/tools/qmap.h +++ b/src/corelib/tools/qmap.h @@ -204,9 +204,9 @@ public: inline void detach() { if (d->ref != 1) detach_helper(); } inline bool isDetached() const { return d->ref == 1; } - inline void setSharable(bool sharable) { if (!sharable) detach(); d->sharable = sharable; } + inline void setSharable(bool sharable) { if (!sharable) detach(); if (d != &QMapData::shared_null) d->sharable = sharable; } inline bool isSharedWith(const QMap &other) const { return d == other.d; } - inline void setInsertInOrder(bool ordered) { d->insertInOrder = ordered; } + inline void setInsertInOrder(bool ordered) { if (ordered) detach(); if (d != &QMapData::shared_null) d->insertInOrder = ordered; } void clear(); diff --git a/src/corelib/tools/qvector.h b/src/corelib/tools/qvector.h index 34d1ed3717..eab9311eb3 100644 --- a/src/corelib/tools/qvector.h +++ b/src/corelib/tools/qvector.h @@ -146,7 +146,7 @@ public: inline void detach() { if (d->ref != 1) detach_helper(); } inline bool isDetached() const { return d->ref == 1; } - inline void setSharable(bool sharable) { if (!sharable) detach(); d->sharable = sharable; } + inline void setSharable(bool sharable) { if (!sharable) detach(); if (d != &QVectorData::shared_null) d->sharable = sharable; } inline bool isSharedWith(const QVector &other) const { return d == other.d; } inline T *data() { detach(); return p->array; } diff --git a/tests/auto/corelib/tools/qhash/tst_qhash.cpp b/tests/auto/corelib/tools/qhash/tst_qhash.cpp index 16fb123256..fc669628b5 100644 --- a/tests/auto/corelib/tools/qhash/tst_qhash.cpp +++ b/tests/auto/corelib/tools/qhash/tst_qhash.cpp @@ -79,6 +79,8 @@ private slots: void iterators(); // sligthly modified from tst_QMap void keys_values_uniqueKeys(); // slightly modified from tst_QMap void noNeedlessRehashes(); + + void const_shared_null(); }; struct Foo { @@ -1237,5 +1239,16 @@ void tst_QHash::noNeedlessRehashes() } } +void tst_QHash::const_shared_null() +{ + QHash hash1; + hash1.setSharable(false); + QVERIFY(hash1.isDetached()); + + QHash hash2; + hash2.setSharable(true); + QVERIFY(!hash2.isDetached()); +} + QTEST_APPLESS_MAIN(tst_QHash) #include "tst_qhash.moc" diff --git a/tests/auto/corelib/tools/qlist/tst_qlist.cpp b/tests/auto/corelib/tools/qlist/tst_qlist.cpp index 3901b6ffe8..3d062919da 100644 --- a/tests/auto/corelib/tools/qlist/tst_qlist.cpp +++ b/tests/auto/corelib/tools/qlist/tst_qlist.cpp @@ -91,6 +91,8 @@ private slots: void testOperators() const; void initializeList() const; + + void const_shared_null() const; }; void tst_QList::length() const @@ -688,5 +690,16 @@ void tst_QList::initializeList() const #endif } +void tst_QList::const_shared_null() const +{ + QList list1; + list1.setSharable(false); + QVERIFY(list1.isDetached()); + + QList list2; + list2.setSharable(true); + QVERIFY(!list2.isDetached()); +} + QTEST_APPLESS_MAIN(tst_QList) #include "tst_qlist.moc" diff --git a/tests/auto/corelib/tools/qmap/tst_qmap.cpp b/tests/auto/corelib/tools/qmap/tst_qmap.cpp index 141e693b16..2b4cbe9b66 100644 --- a/tests/auto/corelib/tools/qmap/tst_qmap.cpp +++ b/tests/auto/corelib/tools/qmap/tst_qmap.cpp @@ -80,6 +80,8 @@ private slots: void iterators(); void keys_values_uniqueKeys(); void qmultimap_specific(); + + void const_shared_null(); }; tst_QMap::tst_QMap() @@ -871,5 +873,20 @@ void tst_QMap::qmultimap_specific() } } +void tst_QMap::const_shared_null() +{ + QMap map1; + map1.setSharable(false); + QVERIFY(map1.isDetached()); + + QMap map2; + map2.setSharable(true); + QVERIFY(!map2.isDetached()); + + QMap map3; + map3.setInsertInOrder(true); + map3.setInsertInOrder(false); +} + QTEST_APPLESS_MAIN(tst_QMap) #include "tst_qmap.moc" diff --git a/tests/auto/corelib/tools/qvector/tst_qvector.cpp b/tests/auto/corelib/tools/qvector/tst_qvector.cpp index 4cc2bc481a..2b615bc28c 100644 --- a/tests/auto/corelib/tools/qvector/tst_qvector.cpp +++ b/tests/auto/corelib/tools/qvector/tst_qvector.cpp @@ -92,6 +92,8 @@ private slots: void QTBUG11763_data(); void QTBUG11763(); void initializeList(); + + void const_shared_null(); }; void tst_QVector::constructors() const @@ -940,5 +942,16 @@ void tst_QVector::initializeList() #endif } +void tst_QVector::const_shared_null() +{ + QVector v1; + v1.setSharable(false); + QVERIFY(v1.isDetached()); + + QVector v2; + v2.setSharable(true); + QVERIFY(!v2.isDetached()); +} + QTEST_APPLESS_MAIN(tst_QVector) #include "tst_qvector.moc"