From 21d728aff3f157750fa25fa1409c01e516a71667 Mon Sep 17 00:00:00 2001
From: Ulf Hermann <ulf.hermann@qt.io>
Date: Wed, 10 May 2023 14:53:11 +0200
Subject: QML: Maintain invariant between QObjectMethod members

If the methodCount is 0, the methods have to be nullptr. Otherwise they
have to point to the actual method(s). This is important for the method
resolution to work correctly.

In particular when cloning a method we have to check for 0.

Amends commit 17bd07cbc5b6cf54716e991765ab3088a710d7b3.

Pick-to: 6.5
Fixes: QTBUG-113484
Change-Id: Ic31d6e391c1d74a162820232f242a19379f5e4df
Reviewed-by: Sami Shalayel <sami.shalayel@qt.io>
---
 src/qml/jsruntime/qv4qobjectwrapper.cpp            | 22 ++++++++++++++++++---
 tests/auto/qml/qqmllanguage/data/UIToolBar.qml     | 10 ++++++++++
 .../qml/qqmllanguage/data/objectMethodClone.qml    | 23 ++++++++++++++++++++++
 tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp   | 12 +++++++++++
 4 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 tests/auto/qml/qqmllanguage/data/UIToolBar.qml
 create mode 100644 tests/auto/qml/qqmllanguage/data/objectMethodClone.qml

diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index ba94a7ad95..f308cd7660 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -2340,14 +2340,25 @@ ReturnedValue QObjectMethod::create(
     else
         method->d()->setObject(object);
 
-    if (cloneFrom->methodCount == 1) {
+    Q_ASSERT(method->d()->methods == nullptr);
+    switch (cloneFrom->methodCount) {
+    case 0:
+        Q_ASSERT(cloneFrom->methods == nullptr);
+        break;
+    case 1:
+        Q_ASSERT(cloneFrom->methods
+                 == reinterpret_cast<QQmlPropertyData *>(&cloneFrom->_singleMethod));
         method->d()->methods = reinterpret_cast<QQmlPropertyData *>(&method->d()->_singleMethod);
         *method->d()->methods = *cloneFrom->methods;
-    } else {
+        break;
+    default:
+        Q_ASSERT(cloneFrom->methods != nullptr);
         method->d()->methods = new QQmlPropertyData[cloneFrom->methodCount];
         memcpy(method->d()->methods, cloneFrom->methods,
                cloneFrom->methodCount * sizeof(QQmlPropertyData));
+        break;
     }
+
     return method.asReturnedValue();
 }
 
@@ -2473,8 +2484,10 @@ QString Heap::QObjectMethod::name() const
 
 void Heap::QObjectMethod::ensureMethodsCache(const QMetaObject *thisMeta)
 {
-    if (methods)
+    if (methods) {
+        Q_ASSERT(methodCount > 0);
         return;
+    }
 
     const QMetaObject *mo = metaObject();
 
@@ -2511,6 +2524,8 @@ void Heap::QObjectMethod::ensureMethodsCache(const QMetaObject *thisMeta)
         *methods = resolvedMethods.at(0);
         methodCount = 1;
     }
+
+    Q_ASSERT(methodCount > 0);
 }
 
 static QObject *qObject(const Value *v)
@@ -2637,6 +2652,7 @@ ReturnedValue QObjectMethod::callInternal(const Value *thisObject, const Value *
     };
 
     if (d()->methodCount != 1) {
+        Q_ASSERT(d()->methodCount > 0);
         method = ResolveOverloaded(object, d()->methods, d()->methodCount, v4, callData);
         if (method == nullptr)
             return Encode::undefined();
diff --git a/tests/auto/qml/qqmllanguage/data/UIToolBar.qml b/tests/auto/qml/qqmllanguage/data/UIToolBar.qml
new file mode 100644
index 0000000000..08a22d2492
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/UIToolBar.qml
@@ -0,0 +1,10 @@
+import QtQml
+
+QtObject {
+    id: root
+    signal doneClicked()
+    signal foo()
+
+    onObjectNameChanged: foo()
+    Component.onCompleted: root.foo.connect(root.doneClicked)
+}
diff --git a/tests/auto/qml/qqmllanguage/data/objectMethodClone.qml b/tests/auto/qml/qqmllanguage/data/objectMethodClone.qml
new file mode 100644
index 0000000000..e21179ea14
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/objectMethodClone.qml
@@ -0,0 +1,23 @@
+import QtQml
+
+QtObject {
+    id: window
+
+    property int doneClicks: 0
+
+    property UIToolBar t1: UIToolBar {
+        objectName: window.objectName
+        onDoneClicked: window.doneClicks++
+    }
+
+    property UIToolBar t2: UIToolBar {
+        objectName: window.objectName
+        onDoneClicked: window.doneClicks++
+    }
+
+    property Timer timer: Timer {
+        interval: 10
+        running: true
+        onTriggered: window.objectName = "bar"
+    }
+}
diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 371179a39e..f171d55d03 100644
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -413,6 +413,8 @@ private slots:
     void typedEnums_data();
     void typedEnums();
 
+    void objectMethodClone();
+
 private:
     QQmlEngine engine;
     QStringList defaultImportPathList;
@@ -7974,6 +7976,16 @@ void tst_qqmllanguage::typedEnums()
     QCOMPARE(o->property("output2").toDouble(), value);
 }
 
+void tst_qqmllanguage::objectMethodClone()
+{
+    QQmlEngine e;
+    QQmlComponent c(&e, testFileUrl("objectMethodClone.qml"));
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    QScopedPointer<QObject> o(c.create());
+    QVERIFY(!o.isNull());
+    QTRY_COMPARE(o->property("doneClicks").toInt(), 2);
+}
+
 QTEST_MAIN(tst_qqmllanguage)
 
 #include "tst_qqmllanguage.moc"
-- 
cgit v1.2.1