Don't keep raw pointers to SparseArrayNode

The nodes are owned by the SparseArrayData and will be freed whenever an
item is deleted from the array. Therefore, we have to look up the node
for each iteration. This is slightly slower, but at least it doesn't
crash.

Fixes: QTBUG-74188
Change-Id: Id24324a8c83b00b3ad1212cdaabccabd6c8a999f
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

4 files changed, 28 insertions, 5 deletions

diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp
index 3d2d54f651..9d6cb730a9 100644
--- a/src/qml/jsruntime/qv4object.cpp
+++ b/src/qml/jsruntime/qv4object.cpp
@@ -320,8 +320,11 @@ bool Object::virtualDeleteProperty(Managed *m, PropertyKey id)
 PropertyKey ObjectOwnPropertyKeyIterator::next(const Object *o, Property *pd, PropertyAttributes *attrs)
 {
     if (arrayIndex != UINT_MAX && o->arrayData()) {
-        if (!arrayIndex)
-            arrayNode = o->sparseBegin();
+        SparseArrayNode *arrayNode = nullptr;
+        if (o->arrayType() == Heap::ArrayData::Sparse) {
+            SparseArray *sparse = o->arrayData()->sparse;
+            arrayNode = arrayIndex ? sparse->lowerBound(arrayIndex) : sparse->begin();
+        }
 
         // sparse arrays
         if (arrayNode) {
@@ -339,7 +342,6 @@ PropertyKey ObjectOwnPropertyKeyIterator::next(const Object *o, Property *pd, Pr
                     *attrs = a;
                 return PropertyKey::fromArrayIndex(k);
             }
-            arrayNode = nullptr;
             arrayIndex = UINT_MAX;
         }
         // dense arrays
diff --git a/src/qml/jsruntime/qv4object_p.h b/src/qml/jsruntime/qv4object_p.h
index ff47810994..9f24449c91 100644
--- a/src/qml/jsruntime/qv4object_p.h
+++ b/src/qml/jsruntime/qv4object_p.h
@@ -408,7 +408,6 @@ struct ObjectOwnPropertyKeyIterator : OwnPropertyKeyIterator
     uint arrayIndex = 0;
     uint memberIndex = 0;
     bool iterateOverSymbols = false;
-    SparseArrayNode *arrayNode = nullptr;
     ~ObjectOwnPropertyKeyIterator() override = default;
     PropertyKey next(const Object *o, Property *pd = nullptr, PropertyAttributes *attrs = nullptr) override;
 
diff --git a/src/qml/jsruntime/qv4stringobject.cpp b/src/qml/jsruntime/qv4stringobject.cpp
index 8186153ba4..dee6a67792 100644
--- a/src/qml/jsruntime/qv4stringobject.cpp
+++ b/src/qml/jsruntime/qv4stringobject.cpp
@@ -130,7 +130,7 @@ PropertyKey StringObjectOwnPropertyKeyIterator::next(const QV4::Object *o, Prope
         return PropertyKey::fromArrayIndex(index);
     } else if (arrayIndex == slen) {
         if (s->arrayData()) {
-            arrayNode = s->sparseBegin();
+            SparseArrayNode *arrayNode = s->sparseBegin();
             // iterate until we're past the end of the string
             while (arrayNode && arrayNode->key() < slen)
                 arrayNode = arrayNode->nextNode();
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index eb9b05e764..a67a503f82 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -364,6 +364,7 @@ private slots:
     void arrayAndException();
     void numberToStringWithRadix();
     void tailCallWithArguments();
+    void deleteSparseInIteration();
 
 private:
 //    static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter);
@@ -8907,6 +8908,27 @@ void tst_qqmlecmascript::tailCallWithArguments()
     QCOMPARE(value.toInt(), 1);
 }
 
+void tst_qqmlecmascript::deleteSparseInIteration()
+{
+    QJSEngine engine;
+    const QJSValue value = engine.evaluate(
+            "(function() {\n"
+            "    var obj = { 1: null, 2: null, 4096: null };\n"
+            "    var iterated = [];\n"
+            "    for (var t in obj) {\n"
+            "        if (t == 2)\n"
+            "            delete obj[t];\n"
+            "        iterated.push(t);\n"
+            "    }\n"
+            "    return iterated;"
+            "})()");
+    QVERIFY(value.isArray());
+    QCOMPARE(value.property("length").toInt(), 3);
+    QCOMPARE(value.property("0").toInt(), 1);
+    QCOMPARE(value.property("1").toInt(), 2);
+    QCOMPARE(value.property("2").toInt(), 4096);
+}
+
 QTEST_MAIN(tst_qqmlecmascript)
 
 #include "tst_qqmlecmascript.moc"