1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QStringIterator: fix UB [2/2]: use std::less to compare pointers

Browse Source 
The relational operators <, >, <=, >= do not form a total order for
pointers. In particular, they are only defined for pointer pairs that
point into the same array (incl. one past the array's last element).

Other uses are undefined behavior.

The iterators the class works on are user-supplied, so we cannot
assume that they are pointing into the same array. In fact, we have a
check in setPosition() that tries to catch such misuses, but similar
checks are absent from the ctors, e.g.

In addition, the implementation itself is checking for next-but-one
and prev-but-one in an unsafe way.

std::less can be used to provide a total order, so use that
everywhere.

Change-Id: I32fb0ef9f768d9336ae12cc0542ca18d1bf23adf
Reviewed-by: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
(cherry picked from commit 7cf2684e6a11892975946bd57d8cf672fe6dd43e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Marc Mutz 2022-07-20 16:23:50 +02:00 committed by Qt Cherry-pick Bot
parent 7d92a8009d
commit 8922ead397
1 changed files with 8 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/corelib/text/qstringiterator_p.h
View File

@ -25,6 +25,8 @@ class QStringIterator
{
QString::const_iterator i, pos, e;
static_assert((std::is_same<QString::const_iterator, const QChar *>::value));
static bool less(const QChar *lhs, const QChar *rhs) noexcept
{ return std::less{}(lhs, rhs); }
public:
explicit QStringIterator(QStringView string, qsizetype idx = 0)
: i(string.begin()),
@ -59,7 +61,8 @@ public:
inline void setPosition(QString::const_iterator position)
{
Q_ASSERT_X(i <= position && position <= e, Q_FUNC_INFO, "position out of bounds");
Q_ASSERT_X(!less(position, i) && !less(e, position),
Q_FUNC_INFO, "position out of bounds");
pos = position;
}
@ -67,7 +70,7 @@ public:
inline bool hasNext() const
{
return pos < e;
return less(pos, e);
}
inline void advance()
@ -95,7 +98,7 @@ public:
Q_ASSERT_X(hasNext(), Q_FUNC_INFO, "iterator hasn't a next item");
if (Q_UNLIKELY(pos->isHighSurrogate())) {
Q_ASSERT(pos + 1 < e && pos[1].isLowSurrogate());
Q_ASSERT(less(pos + 1, e) && pos[1].isLowSurrogate());
return QChar::surrogateToUcs4(pos[0], pos[1]);
}
@ -148,7 +151,7 @@ public:
inline bool hasPrevious() const
{
return pos > i;
return less(i, pos);
}
inline void recede()
@ -177,7 +180,7 @@ public:
Q_ASSERT_X(hasPrevious(), Q_FUNC_INFO, "iterator hasn't a previous item");
if (Q_UNLIKELY(pos[-1].isLowSurrogate())) {
Q_ASSERT(pos > i + 1 && pos[-2].isHighSurrogate());
Q_ASSERT(less(i + 1, pos) && pos[-2].isHighSurrogate());
return QChar::surrogateToUcs4(pos[-2], pos[-1]);
}
return pos[-1].unicode();