StackView: fix heap-use-after-free when pushing after clear

This patch extends the work done in aaec25a7 to cover all operations.

Note also that b94889f4 does a similar thing to this patch and
aaec25a7, in that it explicitly ignores operations that are done during
the removal of elements.

Fixes: QTBUG-84381
Pick-to: 5.15
Change-Id: Id8bbbded39d8e58bcf0e8eedeb2dde794952333f
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>

src/quicktemplates2/qquickstackview.cpp

@@ -558,7 +558,14 @@ QQuickItem *QQuickStackView::find(const QJSValue &callback, LoadBehavior behavio
 void QQuickStackView::push(QQmlV4Function *args)
 {
     Q_D(QQuickStackView);
-    QScopedValueRollback<QString> rollback(d->operation, QStringLiteral("push"));
+    const QString operationName = QStringLiteral("push");
+    if (d->modifyingElements) {
+        d->warnOfInterruption(operationName);
+        return;
+    }
+
+    QScopedValueRollback<bool> modifyingElements(d->modifyingElements, true);
+    QScopedValueRollback<QString> operationNameRollback(d->operation, operationName);
     if (args->length() <= 0) {
         d->warn(QStringLiteral("missing arguments"));
         args->setReturnValue(QV4::Encode::null());
@@ -651,14 +658,15 @@ void QQuickStackView::push(QQmlV4Function *args)
 void QQuickStackView::pop(QQmlV4Function *args)
 {
     Q_D(QQuickStackView);
-    if (d->removingElements) {
-        d->warn(QStringLiteral("cannot pop while already in the process of removing elements"));
+    const QString operationName = QStringLiteral("pop");
+    if (d->modifyingElements) {
+        d->warnOfInterruption(operationName);
         args->setReturnValue(QV4::Encode::null());
         return;
     }
 
-    QScopedValueRollback<bool> removingElements(d->removingElements, true);
-    QScopedValueRollback<QString> rollback(d->operation, QStringLiteral("pop"));
+    QScopedValueRollback<bool> modifyingElements(d->modifyingElements, true);
+    QScopedValueRollback<QString> operationNameRollback(d->operation, operationName);
     int argc = args->length();
     if (d->elements.count() <= 1 || argc > 2) {
         if (argc > 2)
@@ -813,14 +821,15 @@ void QQuickStackView::pop(QQmlV4Function *args)
 void QQuickStackView::replace(QQmlV4Function *args)
 {
     Q_D(QQuickStackView);
-    if (d->removingElements) {
-        d->warn(QStringLiteral("cannot replace while already in the process of removing elements"));
+    const QString operationName = QStringLiteral("replace");
+    if (d->modifyingElements) {
+        d->warnOfInterruption(operationName);
         args->setReturnValue(QV4::Encode::null());
         return;
     }
 
-    QScopedValueRollback<bool> removingElements(d->removingElements, true);
-    QScopedValueRollback<QString> rollback(d->operation, QStringLiteral("replace"));
+    QScopedValueRollback<bool> modifyingElements(d->modifyingElements, true);
+    QScopedValueRollback<QString> operationNameRollback(d->operation, operationName);
     if (args->length() <= 0) {
         d->warn(QStringLiteral("missing arguments"));
         args->setReturnValue(QV4::Encode::null());
@@ -916,12 +925,14 @@ void QQuickStackView::clear(Operation operation)
     if (d->elements.isEmpty())
         return;
 
-    if (d->removingElements) {
-        d->warn(QStringLiteral("cannot clear while already in the process of removing elements"));
+    const QString operationName = QStringLiteral("clear");
+    if (d->modifyingElements) {
+        d->warnOfInterruption(operationName);
         return;
     }
 
-    QScopedValueRollback<bool> removingElements(d->removingElements, true);
+    QScopedValueRollback<bool> modifyingElements(d->modifyingElements, true);
+    QScopedValueRollback<QString> operationNameRollback(d->operation, operationName);
     if (operation != Immediate) {
         QQuickStackElement *exit = d->elements.pop();
         exit->removal = true;
@@ -1128,7 +1139,7 @@ void QQuickStackView::componentComplete()
     QQuickControl::componentComplete();
 
     Q_D(QQuickStackView);
-    QScopedValueRollback<QString> rollback(d->operation, QStringLiteral("initialItem"));
+    QScopedValueRollback<QString> operationNameRollback(d->operation, QStringLiteral("initialItem"));
     QQuickStackElement *element = nullptr;
     QString error;
     int oldDepth = d->elements.count();

src/quicktemplates2/qquickstackview_p.cpp

@@ -56,6 +56,12 @@ void QQuickStackViewPrivate::warn(const QString &error)
         qmlWarning(q) << operation << ": " << error;
 }
 
+void QQuickStackViewPrivate::warnOfInterruption(const QString &attemptedOperation)
+{
+    Q_Q(QQuickStackView);
+    qmlWarning(q) << "cannot " << attemptedOperation << " while already in the process of completing a " << operation;
+}
+
 void QQuickStackViewPrivate::setCurrentItem(QQuickStackElement *element)
 {
     Q_Q(QQuickStackView);

src/quicktemplates2/qquickstackview_p_p.h

@@ -73,6 +73,7 @@ public:
     }
 
     void warn(const QString &error);
+    void warnOfInterruption(const QString &attemptedOperation);
 
     void setCurrentItem(QQuickStackElement *element);
 
@@ -94,7 +95,7 @@ public:
     void depthChange(int newDepth, int oldDepth);
 
     bool busy = false;
-    bool removingElements = false;
+    bool modifyingElements = false;
     QString operation;
     QJSValue initialItem;
     QQuickItem *currentItem = nullptr;

tests/auto/controls/data/tst_stackview.qml

@@ -804,7 +804,10 @@ TestCase {
         var item = control.push(component, StackView.Immediate)
         verify(item)
 
-        item.StackView.onRemoved.connect(function() { control.push(component, StackView.Immediate) } )
+        item.StackView.onRemoved.connect(function() {
+            ignoreWarning(/.*QML StackView: cannot push while already in the process of completing a pop/)
+            control.push(component, StackView.Immediate)
+        })
 
         // don't crash (QTBUG-62153)
         control.pop(StackView.Immediate)
@@ -1287,7 +1290,7 @@ TestCase {
         control.push(container.clearUponDestructionComponent, StackView.Immediate)
 
         // Shouldn't crash.
-        ignoreWarning(new RegExp(".*cannot clear while already in the process of removing elements"))
+        ignoreWarning(/.*cannot clear while already in the process of completing a clear/)
         control.clear(StackView.Immediate)
     }
 
@@ -1301,7 +1304,7 @@ TestCase {
 
         // Pop all items except the first, removing the second item we pushed in the process.
         // Shouldn't crash.
-        ignoreWarning(new RegExp(".*cannot clear while already in the process of removing elements"))
+        ignoreWarning(/.*cannot clear while already in the process of completing a pop/)
         control.pop(null, StackView.Immediate)
     }
 
@@ -1316,7 +1319,7 @@ TestCase {
         control.push(container.clearUponDestructionComponent, StackView.Immediate)
 
         // Pop the top item, then pop down to the first item in response.
-        ignoreWarning(new RegExp(".*cannot pop while already in the process of removing elements"))
+        ignoreWarning(/.*cannot pop while already in the process of completing a pop/)
         control.pop(StackView.Immediate)
     }
 
@@ -1329,7 +1332,7 @@ TestCase {
         control.push(container.clearUponDestructionComponent, StackView.Immediate)
 
         // Replace the top item, then clear in response.
-        ignoreWarning(new RegExp(".*cannot clear while already in the process of removing elements"))
+        ignoreWarning(/.*cannot clear while already in the process of completing a replace/)
         control.replace(component, StackView.Immediate)
     }
 
@@ -1342,7 +1345,7 @@ TestCase {
         control.push(container.clearUponDestructionComponent, StackView.Immediate)
 
         // Replace the top item, then clear in response.
-        ignoreWarning(new RegExp(".*cannot replace while already in the process of removing elements"))
+        ignoreWarning(/.*cannot replace while already in the process of completing a clear/)
         control.clear(StackView.Immediate)
     }
 
@@ -1404,4 +1407,27 @@ TestCase {
         tryCompare(control, "busy", false)
         compare(redRect.visible, true)
     }
+
+    // QTBUG-84381
+    function test_clearAndPushAfterDepthChange() {
+        var control = createTemporaryObject(stackView, testCase, {
+            popEnter: null, popExit: null, pushEnter: null,
+            pushExit: null, replaceEnter: null, replaceExit: null
+        })
+        verify(control)
+
+        control.depthChanged.connect(function() {
+            if (control.depth === 2) {
+                // Shouldn't assert.
+                ignoreWarning(/.*QML StackView: cannot clear while already in the process of completing a push/)
+                control.clear()
+                // Shouldn't crash.
+                ignoreWarning(/.*QML StackView: cannot push while already in the process of completing a push/)
+                control.push(component)
+            }
+        })
+
+        control.push(component)
+        control.push(component)
+    }
 }