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>

3 files changed, 87 insertions, 10 deletions

diff --git a/src/corelib/text/qstring.cpp b/src/corelib/text/qstring.cpp
index 1694ea8dbe..c569bf627d 100644
--- a/src/corelib/text/qstring.cpp
+++ b/src/corelib/text/qstring.cpp
@@ -4560,7 +4560,7 @@ qsizetype QString::count(QStringView str, Qt::CaseSensitivity cs) const
 */
 qsizetype QString::indexOf(const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch) const
 {
-    return QtPrivate::indexOf(QStringView(*this), re, from, rmatch);
+    return QtPrivate::indexOf(QStringView(*this), this, re, from, rmatch);
 }
 
 /*!
@@ -4594,7 +4594,7 @@ qsizetype QString::indexOf(const QRegularExpression &re, qsizetype from, QRegula
 */
 qsizetype QString::lastIndexOf(const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch) const
 {
-    return QtPrivate::lastIndexOf(QStringView(*this), re, from, rmatch);
+    return QtPrivate::lastIndexOf(QStringView(*this), this, re, from, rmatch);
 }
 
 /*!
@@ -4633,7 +4633,7 @@ qsizetype QString::lastIndexOf(const QRegularExpression &re, qsizetype from, QRe
 
 bool QString::contains(const QRegularExpression &re, QRegularExpressionMatch *rmatch) const
 {
-    return QtPrivate::contains(QStringView(*this), re, rmatch);
+    return QtPrivate::contains(QStringView(*this), this, re, rmatch);
 }
 
 /*!
@@ -10798,14 +10798,16 @@ qsizetype QtPrivate::lastIndexOf(QLatin1StringView haystack, qsizetype from, QLa
 }
 
 #if QT_CONFIG(regularexpression)
-qsizetype QtPrivate::indexOf(QStringView haystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
+qsizetype QtPrivate::indexOf(QStringView viewHaystack, const QString *stringHaystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
 {
     if (!re.isValid()) {
         qtWarnAboutInvalidRegularExpression(re.pattern(), "QString(View)::indexOf");
         return -1;
     }
 
-    QRegularExpressionMatch match = re.match(haystack, from);
+    QRegularExpressionMatch match = stringHaystack
+                ? re.match(*stringHaystack, from)
+                : re.match(viewHaystack, from);
     if (match.hasMatch()) {
         const qsizetype ret = match.capturedStart();
         if (rmatch)
@@ -10816,15 +10818,22 @@ qsizetype QtPrivate::indexOf(QStringView haystack, const QRegularExpression &re,
     return -1;
 }
 
-qsizetype QtPrivate::lastIndexOf(QStringView haystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
+qsizetype QtPrivate::indexOf(QStringView haystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
+{
+    return indexOf(haystack, nullptr, re, from, rmatch);
+}
+
+qsizetype QtPrivate::lastIndexOf(QStringView viewHaystack, const QString *stringHaystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
 {
     if (!re.isValid()) {
         qtWarnAboutInvalidRegularExpression(re.pattern(), "QString(View)::lastIndexOf");
         return -1;
     }
 
-    qsizetype endpos = (from < 0) ? (haystack.size() + from + 1) : (from + 1);
-    QRegularExpressionMatchIterator iterator = re.globalMatch(haystack);
+    qsizetype endpos = (from < 0) ? (viewHaystack.size() + from + 1) : (from + 1);
+    QRegularExpressionMatchIterator iterator = stringHaystack
+            ? re.globalMatch(*stringHaystack)
+            : re.globalMatch(viewHaystack);
     qsizetype lastIndex = -1;
     while (iterator.hasNext()) {
         QRegularExpressionMatch match = iterator.next();
@@ -10841,19 +10850,31 @@ qsizetype QtPrivate::lastIndexOf(QStringView haystack, const QRegularExpression
     return lastIndex;
 }
 
-bool QtPrivate::contains(QStringView haystack, const QRegularExpression &re, QRegularExpressionMatch *rmatch)
+qsizetype QtPrivate::lastIndexOf(QStringView haystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
+{
+    return lastIndexOf(haystack, nullptr, re, from, rmatch);
+}
+
+bool QtPrivate::contains(QStringView viewHaystack, const QString *stringHaystack, const QRegularExpression &re, QRegularExpressionMatch *rmatch)
 {
     if (!re.isValid()) {
         qtWarnAboutInvalidRegularExpression(re.pattern(), "QString(View)::contains");
         return false;
     }
-    QRegularExpressionMatch m = re.match(haystack);
+    QRegularExpressionMatch m = stringHaystack
+                ? re.match(*stringHaystack)
+                : re.match(viewHaystack);
     bool hasMatch = m.hasMatch();
     if (hasMatch && rmatch)
         *rmatch = std::move(m);
     return hasMatch;
 }
 
+bool QtPrivate::contains(QStringView haystack, const QRegularExpression &re, QRegularExpressionMatch *rmatch)
+{
+    return contains(haystack, nullptr, re, rmatch);
+}
+
 qsizetype QtPrivate::count(QStringView haystack, const QRegularExpression &re)
 {
     if (!re.isValid()) {
diff --git a/src/corelib/text/qstringalgorithms.h b/src/corelib/text/qstringalgorithms.h
index 52bb9736ba..888bad5dd3 100644
--- a/src/corelib/text/qstringalgorithms.h
+++ b/src/corelib/text/qstringalgorithms.h
@@ -69,14 +69,30 @@ namespace QtPrivate {
 [[nodiscard]] Q_CORE_EXPORT Q_DECL_PURE_FUNCTION qsizetype count(QLatin1StringView haystack, QChar needle, Qt::CaseSensitivity cs = Qt::CaseSensitive) noexcept;
 
 #if QT_CONFIG(regularexpression)
+// ### Qt 7: unify these overloads;
+// remove the ones taking only a QStringView, export the others, adjust callers
+[[nodiscard]] qsizetype indexOf(QStringView viewHaystack,
+                                const QString *stringHaystack,
+                                const QRegularExpression &re,
+                                qsizetype from = 0,
+                                QRegularExpressionMatch *rmatch = nullptr);
 [[nodiscard]] Q_CORE_EXPORT qsizetype indexOf(QStringView haystack,
                                               const QRegularExpression &re,
                                               qsizetype from = 0,
                                               QRegularExpressionMatch *rmatch = nullptr);
+[[nodiscard]] qsizetype lastIndexOf(QStringView viewHaystack,
+                                    const QString *stringHaystack,
+                                    const QRegularExpression &re,
+                                    qsizetype from = -1,
+                                    QRegularExpressionMatch *rmatch = nullptr);
 [[nodiscard]] Q_CORE_EXPORT qsizetype lastIndexOf(QStringView haystack,
                                                   const QRegularExpression &re,
                                                   qsizetype from = -1,
                                                   QRegularExpressionMatch *rmatch = nullptr);
+[[nodiscard]] bool contains(QStringView viewHaystack,
+                            const QString *stringHaystack,
+                            const QRegularExpression &re,
+                            QRegularExpressionMatch *rmatch = nullptr);
 [[nodiscard]] Q_CORE_EXPORT bool contains(QStringView haystack,
                                           const QRegularExpression &re,
                                           QRegularExpressionMatch *rmatch = nullptr);
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()