summaryrefslogtreecommitdiffstats
path: root/tests
diff options
context:
space:
mode:
authorGiuseppe D'Angelo <giuseppe.dangelo@kdab.com>2022-07-06 02:20:38 +0200
committerGiuseppe D'Angelo <giuseppe.dangelo@kdab.com>2022-07-10 23:40:24 +0200
commit6f852935e0c847f91c7d6ffca455a74307ebe26d (patch)
tree037cd5c792abf48ef86535f7a249bffef84e5913 /tests
parent54b276be0b5885bbaee2c38f472eb39731fd684a (diff)
QString: fix lifetime issues with QRegularExpression APIs
QString has several functions taking a QRegularExpression: indexOf(), contains(), and so on. Some of those have an out-argument of type QRegularExpressionMatch, to report the details of the match (if any). For instance: QRegularExpression re(...); QRegularExpressionMatch match; if (string.contains(re, &match)) use(match); The code used to route the implementation of these functions through QStringView (which has the very same functions). This however opens up a lifetime problem with temporary strings: if (getString().contains(re, &match)) use(match); // match is dangling Here `match` is dangling because it is referencing data into the destroyed temporary -- nothing is keeping the string alive. This is against the rules we've decided for Qt, and it's also asymmetric with the corresponding code that uses QRegularExpression directly instead: match = re.match(getString()); if (match.hasMatch()) use(match); // not dangling ... although we've documented not to do this. (In light of the decision we've made w.r.t. temporaries, the documentation is wrong anyways.) Here QRE takes a copy of the string and stores it in the match object, thus keeping it alive. Hence, extend the implementation of the QString functions to keep a (shallow) copy of the string. To keep the code shared as much as possible with QStringView, in theory one could have a function taking a std::variant<QString, QStringView> and that uses the currently active member. However I've found that std::variant here creates some abysmal codegen, so instead I went for a simpler approach -- pass a QStringView and an optional pointer to a QString. Use the latter if it's loaded. QStringView has some inline code that calls into exported functions, so I can't change the signature of them without breaking BC; I'm instead adding new unexported functions and a Qt 7 note to unify them. Change-Id: I7c65885a84069d0fbb902dcc96ddff543ca84562 Fixes: QTBUG-103940 Pick-to: 6.2 6.3 6.4 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'tests')
-rw-r--r--tests/auto/corelib/text/qstring/tst_qstring.cpp40
1 files changed, 40 insertions, 0 deletions
diff --git a/tests/auto/corelib/text/qstring/tst_qstring.cpp b/tests/auto/corelib/text/qstring/tst_qstring.cpp
index 467d5b0b39..239b3498a9 100644
--- a/tests/auto/corelib/text/qstring/tst_qstring.cpp
+++ b/tests/auto/corelib/text/qstring/tst_qstring.cpp
@@ -537,6 +537,7 @@ private slots:
#if QT_CONFIG(regularexpression)
void split_regularexpression_data();
void split_regularexpression();
+ void regularexpression_lifetime();
#endif
void fromUtf16_data();
void fromUtf16();
@@ -6244,6 +6245,45 @@ void tst_QString::split_regularexpression()
split_regexp<QStringList, QRegularExpression>(string, pattern, result);
split_regexp<QList<QStringView>, QRegularExpression>(string, pattern, result);
}
+
+// Test that rvalue strings (e.g. temporaries) are kept alive in
+// QRegularExpression-related APIs
+void tst_QString::regularexpression_lifetime()
+{
+ const auto getString = [] {
+ // deliberately heap-allocated
+ return QString(QLatin1String("the quick brown fox jumps over the lazy dog"));
+ };
+
+ QRegularExpression re("\\w{5}");
+
+ {
+ QString s = getString();
+ QRegularExpressionMatch match;
+ const bool contains = std::move(s).contains(re, &match);
+ s.fill('X'); // NOLINT(bugprone-use-after-move)
+ QVERIFY(contains);
+ QCOMPARE(match.capturedView(), u"quick");
+ }
+
+ {
+ QString s = getString();
+ QRegularExpressionMatch match;
+ const auto index = std::move(s).indexOf(re, 0, &match);
+ s.fill('X'); // NOLINT(bugprone-use-after-move)
+ QCOMPARE(index, 4);
+ QCOMPARE(match.capturedView(), u"quick");
+ }
+
+ {
+ QString s = getString();
+ QRegularExpressionMatch match;
+ const auto lastIndex = std::move(s).lastIndexOf(re, &match);
+ s.fill('X'); // NOLINT(bugprone-use-after-move)
+ QCOMPARE(lastIndex, 20);
+ QCOMPARE(match.capturedView(), u"jumps");
+ }
+}
#endif
void tst_QString::fromUtf16_data()