From f17168ef874f05b4ea57088cadcc2d5a8fd2c5a0 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Mon, 6 Mar 2023 09:24:39 +0100
Subject: [PATCH] setInternalClass: Correctly handle deleted properties

When an IC was rebuilt, we must not set members that were deleted,
otherwise we'll trigger an assertion. Since the nameMap has to match the
data we still allocate memory for such members and fill it with
undefined. This could be avoided with some deeper refactoring, but a
simple solution is to be preferred because this needs to be picked back
all the way to 6.2.

Pick-to: 6.5 6.2
Fixes: QTBUG-111729
Change-Id: I730d6b4634d989191434225600a08cf0208e72f8
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
(cherry picked from commit 642d531e42fb233709155f8c8feb7d429c48db38)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 308f785c7842267a8744381148bd82b21c536033)
---
 src/qml/jsruntime/qv4object.cpp            |  7 +++++--
 tests/auto/qml/qjsengine/tst_qjsengine.cpp | 22 ++++++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp
index 6475ec6e06..4d55b15cfa 100644
--- a/src/qml/jsruntime/qv4object.cpp
+++ b/src/qml/jsruntime/qv4object.cpp
@@ -41,8 +41,11 @@ void Object::setInternalClass(Heap::InternalClass *ic)
         // Pick the members of the old IC that are still valid in the new IC.
         // Order them by index in memberData (or inline data).
         Scoped<MemberData> newMembers(scope, MemberData::allocate(scope.engine, ic->size));
-        for (uint i = 0; i < ic->size; ++i)
-            newMembers->set(scope.engine, i, get(ic->nameMap.at(i)));
+        for (uint i = 0; i < ic->size; ++i) {
+            // Note that some members might have been deleted. The key may be invalid.
+            const PropertyKey key = ic->nameMap.at(i);
+            newMembers->set(scope.engine, i, key.isValid() ? get(key) : Encode::undefined());
+        }
 
         p->internalClass.set(scope.engine, ic);
         const uint nInline = p->vtable()->nInlineProperties;
diff --git a/tests/auto/qml/qjsengine/tst_qjsengine.cpp b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
index b6cb4ae0f3..c1c755d843 100644
--- a/tests/auto/qml/qjsengine/tst_qjsengine.cpp
+++ b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
@@ -310,6 +310,9 @@ private slots:
     void symbolToVariant();
 
     void garbageCollectedObjectMethodBase();
+
+    void deleteDefineCycle();
+
 public:
     Q_INVOKABLE QJSValue throwingCppMethod1();
     Q_INVOKABLE void throwingCppMethod2();
@@ -6302,6 +6305,25 @@ void tst_QJSEngine::garbageCollectedObjectMethodBase()
     }
 }
 
+void tst_QJSEngine::deleteDefineCycle()
+{
+  QJSEngine engine;
+  QStringList stackTrace;
+
+  QJSValue result = engine.evaluate(QString::fromLatin1(R"(
+  let global = ({})
+
+  for (let j = 0; j < 1000; j++) {
+    for (let i = 0; i < 2; i++) {
+      const name = "test" + i
+      delete global[name]
+      Object.defineProperty(global, name, { get() { return 0 }, configurable: true })
+    }
+  }
+  )"), {}, 1, &stackTrace);
+  QVERIFY(stackTrace.isEmpty());
+}
+
 QTEST_MAIN(tst_QJSEngine)
 
 #include "tst_qjsengine.moc"