Fix QMetaObject::connect and disconnect with "dynamic signals"

QML might pass index that are larger that the method cound.
We must not call QMetaObjectPrivate::originalClone in that case as
this would read invalid memory

Reviewed-by: brad

3 files changed, 168 insertions, 20 deletions

diff --git a/src/corelib/kernel/qmetaobject.cpp b/src/corelib/kernel/qmetaobject.cpp
index 6e6da19..72d6786 100644
--- a/src/corelib/kernel/qmetaobject.cpp
+++ b/src/corelib/kernel/qmetaobject.cpp
@@ -2648,6 +2648,7 @@ const char* QMetaClassInfo::value() const
  */
 int QMetaObjectPrivate::originalClone(const QMetaObject *mobj, int local_method_index)
 {
+    Q_ASSERT(local_method_index < get(mobj)->methodCount);
     int handle = get(mobj)->methodData + 5 * local_method_index;
     while (mobj->d.data[handle + 4] & MethodCloned) {
         Q_ASSERT(local_method_index > 0);
diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index 85915c2..4370ee0 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -2850,6 +2850,27 @@ void QObject::disconnectNotify(const char *)
 {
 }
 
+/* \internal
+    convert a signal index from the method range to the signal range
+ */
+static int methodIndexToSignalIndex(const QMetaObject *metaObject, int signal_index)
+{
+    if (signal_index < 0)
+        return signal_index;
+    while (metaObject && metaObject->methodOffset() > signal_index)
+        metaObject = metaObject->superClass();
+
+    if (metaObject) {
+        int signalOffset, methodOffset;
+        computeOffsets(metaObject, &signalOffset, &methodOffset);
+        if (signal_index < metaObject->methodCount())
+            signal_index = QMetaObjectPrivate::originalClone(metaObject, signal_index - methodOffset) + signalOffset;
+        else
+            signal_index = signal_index - methodOffset + signalOffset;
+    }
+    return signal_index;
+}
+
 /*!\internal
    \a types is a 0-terminated vector of meta types for queued
    connections.
@@ -2860,16 +2881,7 @@ void QObject::disconnectNotify(const char *)
 bool QMetaObject::connect(const QObject *sender, int signal_index,
                           const QObject *receiver, int method_index, int type, int *types)
 {
-    if (signal_index > 0) {
-        const QMetaObject *mo = sender->metaObject();
-        while (mo && mo->methodOffset() > signal_index)
-            mo = mo->superClass();
-        if (mo) {
-            int signalOffset, methodOffset;
-            computeOffsets(mo, &signalOffset, &methodOffset);
-            signal_index = QMetaObjectPrivate::originalClone(mo, signal_index - methodOffset) + signalOffset;
-        }
-    }
+    signal_index = methodIndexToSignalIndex(sender->metaObject(), signal_index);
     return QMetaObjectPrivate::connect(sender, signal_index,
                                        receiver, method_index, type, types);
 }
@@ -2938,16 +2950,7 @@ bool QMetaObjectPrivate::connect(const QObject *sender, int signal_index,
 bool QMetaObject::disconnect(const QObject *sender, int signal_index,
                              const QObject *receiver, int method_index)
 {
-    if (signal_index > 0) {
-        const QMetaObject *mo = sender->metaObject();
-        while (mo && mo->methodOffset() > signal_index)
-            mo = mo->superClass();
-        if (mo) {
-            int signalOffset, methodOffset;
-            computeOffsets(mo, &signalOffset, &methodOffset);
-            signal_index = QMetaObjectPrivate::originalClone(mo, signal_index - methodOffset) + signalOffset;
-        }
-    }
+    signal_index = methodIndexToSignalIndex(sender->metaObject(), signal_index);
     return QMetaObjectPrivate::disconnect(sender, signal_index,
                                           receiver, method_index);
 }
diff --git a/tests/auto/qobject/tst_qobject.cpp b/tests/auto/qobject/tst_qobject.cpp
index a2524aa..75d7a0a 100644
--- a/tests/auto/qobject/tst_qobject.cpp
+++ b/tests/auto/qobject/tst_qobject.cpp
@@ -126,6 +126,7 @@ private slots:
     void deleteQObjectWhenDeletingEvent();
     void overloads();
     void isSignalConnected();
+    void qMetaObjectConnect();
 protected:
 };
 
@@ -3125,5 +3126,148 @@ void tst_QObject::isSignalConnected()
     QCOMPARE(o.rec, 2);
 }
 
+void tst_QObject::qMetaObjectConnect()
+{
+    SenderObject *s = new SenderObject;
+    ReceiverObject *r1 = new ReceiverObject;
+    ReceiverObject *r2 = new ReceiverObject;
+    r1->reset();
+    r2->reset();
+    ReceiverObject::sequence = 0;
+
+    int signal1Index = s->metaObject()->indexOfSignal("signal1()");
+    int signal3Index = s->metaObject()->indexOfSignal("signal3()");
+    int slot1Index = r1->metaObject()->indexOfSlot("slot1()");
+    int slot2Index = r1->metaObject()->indexOfSlot("slot2()");
+    int slot3Index = r1->metaObject()->indexOfSlot("slot3()");
+
+    QVERIFY(slot1Index > 0);
+    QVERIFY(slot2Index > 0);
+    QVERIFY(slot3Index > 0);
+
+    QVERIFY( QMetaObject::connect( s, signal1Index, r1, slot1Index) );
+    QVERIFY( QMetaObject::connect( s, signal3Index, r2, slot3Index) );
+    QVERIFY( QMetaObject::connect( s, -1, r2, slot2Index) );
+
+    QCOMPARE( r1->count_slot1, 0 );
+    QCOMPARE( r1->count_slot2, 0 );
+    QCOMPARE( r1->count_slot3, 0 );
+    QCOMPARE( r2->count_slot1, 0 );
+    QCOMPARE( r2->count_slot2, 0 );
+    QCOMPARE( r2->count_slot3, 0 );
+
+    s->emitSignal1();
+
+    QCOMPARE( r1->count_slot1, 1 );
+    QCOMPARE( r1->count_slot2, 0 );
+    QCOMPARE( r1->count_slot3, 0 );
+    QCOMPARE( r2->count_slot1, 0 );
+    QCOMPARE( r2->count_slot2, 1 );
+    QCOMPARE( r2->count_slot3, 0 );
+
+    s->emitSignal2();
+    s->emitSignal3();
+    s->emitSignal4();
+
+    QCOMPARE( r1->count_slot1, 1 );
+    QCOMPARE( r1->count_slot2, 0 );
+    QCOMPARE( r1->count_slot3, 0 );
+    QCOMPARE( r2->count_slot1, 0 );
+    QCOMPARE( r2->count_slot2, 4 );
+    QCOMPARE( r2->count_slot3, 1 );
+
+    QVERIFY( QMetaObject::disconnect( s, signal1Index, r1, slot1Index) );
+    QVERIFY( QMetaObject::disconnect( s, signal3Index, r2, slot3Index) );
+    QVERIFY( QMetaObject::disconnect( s, -1, r2, slot2Index) );
+
+    s->emitSignal1();
+    s->emitSignal2();
+    s->emitSignal3();
+    s->emitSignal4();
+
+    QCOMPARE( r1->count_slot1, 1 );
+    QCOMPARE( r1->count_slot2, 0 );
+    QCOMPARE( r1->count_slot3, 0 );
+    QCOMPARE( r2->count_slot1, 0 );
+    QCOMPARE( r2->count_slot2, 4 );
+    QCOMPARE( r2->count_slot3, 1 );
+
+    //some "dynamic" signal
+    QVERIFY( QMetaObject::connect( s, s->metaObject()->methodOffset() + 20, r1, slot3Index) );
+    QVERIFY( QMetaObject::connect( s, s->metaObject()->methodOffset() + 35, r2, slot1Index) );
+    QVERIFY( QMetaObject::connect( s, -1, r1, slot2Index) );
+
+    r1->reset();
+    r2->reset();
+
+    void *args[] = { 0 , 0 };
+    QMetaObject::activate(s, s->metaObject()->methodOffset() + 20, args);
+    QMetaObject::activate(s, s->metaObject()->methodOffset() + 48, args);
+    QCOMPARE( r1->count_slot1, 0 );
+    QCOMPARE( r1->count_slot2, 2 );
+    QCOMPARE( r1->count_slot3, 1 );
+    QCOMPARE( r2->count_slot1, 0 );
+    QCOMPARE( r2->count_slot2, 0 );
+    QCOMPARE( r2->count_slot3, 0 );
+
+    QMetaObject::activate(s, s->metaObject()->methodOffset() + 35, args);
+    s->emitSignal1();
+    s->emitSignal2();
+
+    QCOMPARE( r1->count_slot1, 0 );
+    QCOMPARE( r1->count_slot2, 5 );
+    QCOMPARE( r1->count_slot3, 1 );
+    QCOMPARE( r2->count_slot1, 1 );
+    QCOMPARE( r2->count_slot2, 0 );
+    QCOMPARE( r2->count_slot3, 0 );
+
+    delete s;
+    r1->reset();
+    r2->reset();
+
+#define SIGNAL_INDEX(S)  obj1.metaObject()->indexOfSignal(QMetaObject::normalizedSignature(#S))
+    OverloadObject obj1;
+    QObject obj2, obj3;
+
+    QMetaObject::connect(&obj1, SIGNAL_INDEX(sig(int)) , r1, slot1Index);
+    QMetaObject::connect(&obj1, SIGNAL_INDEX(sig(QObject *, QObject *, QObject *)) , r2, slot1Index);
+
+    QMetaObject::connect(&obj1, SIGNAL_INDEX(sig(QObject *, QObject *, QObject *, QObject *)) , r1, slot2Index);
+    QMetaObject::connect(&obj1, SIGNAL_INDEX(sig(QObject *)) , r2, slot2Index);
+    QMetaObject::connect(&obj1, SIGNAL_INDEX(sig(int, int)) , r1, slot3Index);
+
+    emit obj1.sig(0.5); //connected to nothing
+    emit obj1.sig(1, 'a'); //connected to nothing
+    QCOMPARE( r1->count_slot1, 0 );
+    QCOMPARE( r1->count_slot2, 0 );
+    QCOMPARE( r1->count_slot3, 0 );
+    QCOMPARE( r2->count_slot1, 0 );
+    QCOMPARE( r2->count_slot2, 0 );
+    QCOMPARE( r2->count_slot3, 0 );
+
+    emit obj1.sig(1); //this signal is connected
+    emit obj1.sig(&obj2);
+
+    QCOMPARE( r1->count_slot1, 1 );
+    QCOMPARE( r1->count_slot2, 0 );
+    QCOMPARE( r1->count_slot3, 1 );
+    QCOMPARE( r2->count_slot1, 0 );
+    QCOMPARE( r2->count_slot2, 1 );
+    QCOMPARE( r2->count_slot3, 0 );
+
+    emit obj1.sig(&obj2, &obj3); //this signal is connected
+
+    QCOMPARE( r1->count_slot1, 1 );
+    QCOMPARE( r1->count_slot2, 1 );
+    QCOMPARE( r1->count_slot3, 1 );
+    QCOMPARE( r2->count_slot1, 1 );
+    QCOMPARE( r2->count_slot2, 1 );
+    QCOMPARE( r2->count_slot3, 0 );
+
+    delete r1;
+    delete r2;
+
+}
+
 QTEST_MAIN(tst_QObject)
 #include "tst_qobject.moc"