From 66aa4c86f4303dba48910c39ed0908818bf5dc6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Wed, 16 Jun 2021 11:40:04 +0200 Subject: [PATCH] QHash: Fix erase() edge-case When the element you want to erase is the last element AND the next element (element 0), when rehashed, would be relocated to the last element, this leads to the state below. Which is similar to a test in tst_qhash for some seeds. auto it = hash.begin + (hash.size - 1) it = hash.erase(it) it != hash.end By forcing the iterator to increment if we were erasing the last element we always end up with a pointer which is equal to hash.end Befriend the tst_qhash class so we can set the seed to a known-bad one Change-Id: Ie0b175003a2acb175ef5e3ab5a984e010f65d986 Reviewed-by: Qt CI Bot Reviewed-by: Lars Knoll (cherry picked from commit ea7d87b5b59ded22b145ecbb4dd648ecfd1abbdd) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/tools/qhash.h | 5 ++++- tests/auto/corelib/tools/qhash/tst_qhash.cpp | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/corelib/tools/qhash.h b/src/corelib/tools/qhash.h index 4fb68ba0b56..1a793e61636 100644 --- a/src/corelib/tools/qhash.h +++ b/src/corelib/tools/qhash.h @@ -51,6 +51,8 @@ #include #include // for std::hash +class tst_QHash; // for befriending + QT_BEGIN_NAMESPACE struct QHashDummyValue @@ -680,7 +682,7 @@ struct Data } // return correct position of the next element - if (!spans[span].hasNode(index)) + if (bucket == numBuckets - 1 || !spans[span].hasNode(index)) ++it; return it; } @@ -740,6 +742,7 @@ class QHash using Data = QHashPrivate::Data; friend class QSet; friend class QMultiHash; + friend tst_QHash; Data *d = nullptr; diff --git a/tests/auto/corelib/tools/qhash/tst_qhash.cpp b/tests/auto/corelib/tools/qhash/tst_qhash.cpp index d77160ba19e..6ad7d15c771 100644 --- a/tests/auto/corelib/tools/qhash/tst_qhash.cpp +++ b/tests/auto/corelib/tools/qhash/tst_qhash.cpp @@ -42,6 +42,7 @@ class tst_QHash : public QObject private slots: void insert1(); void erase(); + void erase_edge_case(); void key(); void swap(); @@ -532,6 +533,24 @@ void tst_QHash::erase() QVERIFY(mit == h2.end()); } +/* + With a specific seed we could end up in a situation where, upon deleting the + last entry in a QHash, the returned iterator would not point to the end() + iterator. +*/ +void tst_QHash::erase_edge_case() +{ + QHash h1; + h1.reserve(2); + h1.d->seed = 10230148258692185509ull; + h1.insert(3, 4); + h1.insert(5, 6); + QHash::iterator it1 = h1.begin(); + ++it1; + it1 = h1.erase(it1); + QVERIFY(it1 == h1.end()); +} + void tst_QHash::key() { {