QmlCompiler: Guard against disappearing arrow functions

You can override a QObject method with a JavaScript function and take
away the JavaScript function later by swapping out objects. This should
not crash.

Pick-to: 6.10 6.9
Fixes: QTBUG-140074
Change-Id: I85b17f4f619235024d0f1a27b4ff4128c7a57083
Reviewed-by: Sami Shalayel <sami.shalayel@qt.io>

src/qml/qml/qqml.cpp

@@ -2139,8 +2139,13 @@ static bool callQObjectMethodAsVariant(
     QV4::Scope scope(engine);
     QV4::ScopedValue wrappedObject(scope, QV4::QObjectWrapper::wrap(scope.engine, thisObject));
     QV4::ScopedFunctionObject function(scope, lookup->getter(scope.engine, wrappedObject));
-    Q_ASSERT(function);
+
-    Q_ASSERT(lookup->asVariant); // The getter mustn't reset the isVariant flag
+    // The getter mustn't reset the isVariant flag
+    Q_ASSERT(lookup->asVariant);
+
+    // Since we have an asVariant lookup, the function may have been overridden in the mean time.
+    if (!function)
+        return false;
 
     Q_ALLOCA_VAR(QMetaType, types, (argc + 1) * sizeof(QMetaType));
     std::fill(types, types + argc + 1, QMetaType::fromType<QVariant>());
@@ -2232,31 +2237,6 @@ static bool callArrowFunction(
     Q_UNREACHABLE_RETURN(false);
 }
 
-static bool callArrowFunctionAsVariant(
-        QV4::ExecutionEngine *engine, QV4::ArrowFunction *function,
-        QObject *thisObject, void **args, int argc)
-{
-    QV4::Function *v4Function = function->function();
-    Q_ASSERT(v4Function);
-
-    switch (v4Function->kind) {
-    case QV4::Function::JsUntyped:
-        // We cannot assert anything here because the method can be shadowed.
-        // That's why we wrap everything in QVariant.
-    case QV4::Function::AotCompiled:
-    case QV4::Function::JsTyped: {
-        Q_ALLOCA_VAR(QMetaType, types, (argc + 1) * sizeof(QMetaType));
-        std::fill(types, types + argc + 1, QMetaType::fromType<QVariant>());
-        function->call(thisObject, args, types, argc);
-        return !engine->hasException;
-    }
-    case QV4::Function::Eval:
-        break;
-    }
-
-    Q_UNREACHABLE_RETURN(false);
-}
-
 bool AOTCompiledContext::callQmlContextPropertyLookup(uint index, void **args, int argc) const
 {
     QV4::Lookup *lookup = compilationUnit->runtimeLookups + index;
@@ -2436,16 +2416,25 @@ bool AOTCompiledContext::callObjectPropertyLookup(
                 : callQObjectMethod(engine->handle(), lookup, object, args, argc);
     case QV4::Lookup::Call::GetterQObjectProperty:
     case QV4::Lookup::Call::GetterQObjectPropertyFallback: {
-        const bool asVariant = lookup->asVariant;
+        if (lookup->asVariant) {
-        // Here we always retrieve a fresh method via the getter. No need to re-init.
+            // If the method can be shadowed, the overridden method can be taken away, too.
+            // In that case we might end up with a QObjectMethod or random other values instead.
+            // callQObjectMethodAsVariant is flexible enough to handle that.
+            return callQObjectMethodAsVariant(engine->handle(), lookup, object, args, argc);
+        }
+
+        // Here we always retrieve a fresh ArrowFunction via the getter.
         QV4::Scope scope(engine->handle());
         QV4::ScopedValue thisObject(scope, QV4::QObjectWrapper::wrap(scope.engine, object));
         QV4::Scoped<QV4::ArrowFunction> function(scope, lookup->getter(scope.engine, thisObject));
+
+        // The getter mustn't touch the asVariant bit
+        Q_ASSERT(!lookup->asVariant);
+
+        // If the method can't be shadowed, it has to stay the same.
         Q_ASSERT(function);
-        Q_ASSERT(lookup->asVariant == asVariant); // The getter mustn't touch the asVariant bit
+
-        return asVariant
+        return callArrowFunction(scope.engine, function, qmlScopeObject, args, argc);
-                ? callArrowFunctionAsVariant(scope.engine, function, qmlScopeObject, args, argc)
-                : callArrowFunction(scope.engine, function, qmlScopeObject, args, argc);
     }
     default:
         break;
@@ -2464,16 +2453,16 @@ void AOTCompiledContext::initCallObjectPropertyLookupAsVariant(uint index, QObje
     QV4::Lookup *lookup = compilationUnit->runtimeLookups + index;
     QV4::Scope scope(engine->handle());
 
-    const auto throwInvalidObjectError = [&]() {
+    const auto throwInvalidObjectError = [&](const QString &object) {
         scope.engine->throwTypeError(
-                QStringLiteral("Property '%1' of object [object Object] is not a function")
+                QStringLiteral("Property '%1' of object %2 is not a function").arg(
-                        .arg(compilationUnit->runtimeStrings[lookup->nameIndex]->toQString()));
+                        compilationUnit->runtimeStrings[lookup->nameIndex]->toQString(), object));
     };
 
     const auto *ddata = QQmlData::get(object, false);
     if (ddata && ddata->hasVMEMetaObject && ddata->jsWrapper.isNullOrUndefined()) {
         // We cannot lookup functions on an object with VME metaobject but no QObjectWrapper
-        throwInvalidObjectError();
+        throwInvalidObjectError(QStringLiteral("[object Object]"));
         return;
     }
 
@@ -2491,7 +2480,7 @@ void AOTCompiledContext::initCallObjectPropertyLookupAsVariant(uint index, QObje
         return;
     }
 
-    throwInvalidObjectError();
+    throwInvalidObjectError(thisObject->toQStringNoThrow());
 }
 
 void AOTCompiledContext::initCallObjectPropertyLookup(

tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt

@@ -151,6 +151,7 @@ set(qml_files
     detachedreferences.qml
     dialog.qml
     dialogButtonBox.qml
+    disappearingArrowFunction.qml
     dynamicscene.qml
     enforceSignature.qml
     enumConversion.qml

tests/auto/qml/qmlcppcodegen/data/disappearingArrowFunction.qml

@@ -0,0 +1,28 @@
+pragma Strict
+import QtQml
+
+QtObject {
+    property Person inner: Person {
+        function getName() : int { return 5 }
+    }
+
+    property Person none: Person {}
+
+    property Person evil: Person {
+        property string getName: "not a function"
+    }
+
+    onObjectNameChanged: console.log(inner.getName())
+
+    function swapNone() {
+        let t = inner;
+        inner = none;
+        none = t;
+    }
+
+    function swapEvil() {
+        let t = inner;
+        inner = evil;
+        evil = t;
+    }
+}

tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp

@@ -103,6 +103,7 @@ private slots:
     void detachOnAssignment();
     void detachedReferences();
     void dialogButtonBox();
+    void disappearingArrowFunction();
     void enumConversion();
     void enumFromBadSingleton();
     void enumLookup();
@@ -1866,6 +1867,56 @@ void tst_QmlCppCodegen::dialogButtonBox()
              QPlatformDialogHelper::Ok | QPlatformDialogHelper::Cancel);
 }
 
+void tst_QmlCppCodegen::disappearingArrowFunction()
+{
+    QQmlEngine engine;
+    const QUrl url(u"qrc:/qt/qml/TestTypes/disappearingArrowFunction.qml"_s);
+    QQmlComponent c(&engine, url);
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    QScopedPointer<QObject> o(c.create());
+    QVERIFY(!o.isNull());
+
+    QTest::ignoreMessage(QtDebugMsg, "5");
+    o->setObjectName("no");
+
+    QMetaObject::invokeMethod(o.data(), "swapNone");
+    QTest::ignoreMessage(QtDebugMsg, "Bart");
+    o->setObjectName("nono");
+
+    QMetaObject::invokeMethod(o.data(), "swapNone");
+    QTest::ignoreMessage(QtDebugMsg, "5");
+    o->setObjectName("nonono");
+
+    const QRegularExpression warning(
+                QRegularExpression::escape(url.toString())
+                + u"\\:15\\: TypeError\\: Property 'getName' of object "
+                  "Person_QML_[0-9]+\\(0x[0-9a-f]+\\) is not a function"_s);
+
+    QMetaObject::invokeMethod(o.data(), "swapEvil");
+    QTest::ignoreMessage(QtWarningMsg, warning);
+    o->setObjectName("nononono");
+
+    QMetaObject::invokeMethod(o.data(), "swapEvil");
+    QTest::ignoreMessage(QtDebugMsg, "5");
+    o->setObjectName("nonononono");
+
+    QMetaObject::invokeMethod(o.data(), "swapNone");
+    QTest::ignoreMessage(QtDebugMsg, "Bart");
+    o->setObjectName("nononononono");
+
+    QMetaObject::invokeMethod(o.data(), "swapEvil");
+    QTest::ignoreMessage(QtWarningMsg, warning);
+    o->setObjectName("nonononononono");
+
+    QMetaObject::invokeMethod(o.data(), "swapEvil");
+    QTest::ignoreMessage(QtDebugMsg, "Bart");
+    o->setObjectName("nononononononono");
+
+    QMetaObject::invokeMethod(o.data(), "swapNone");
+    QTest::ignoreMessage(QtDebugMsg, "5");
+    o->setObjectName("nonononononononono");
+}
+
 void tst_QmlCppCodegen::enumConversion()
 {
     QQmlEngine engine;