From 2f4479857d2b68f8cd3267b2f2b3c652ada431ed Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Mon, 20 May 2019 16:09:13 +0200 Subject: Fix lookups of properties in QML singletons An unqualified name that points to a QML singleton will evaluate to a QQmlTypeWrapper JS object. A member lookup in such an object is not guaranteed to always produce the same property. The property cache check may protect us from that, but we must still retrieve the QObject singleton for every lookup. Task-number: QTBUG-75896 Change-Id: Ibd9bac6e5c2047f838758811790b299ace636446 Reviewed-by: Ulf Hermann --- src/qml/jsruntime/qv4lookup_p.h | 2 +- src/qml/jsruntime/qv4qmlcontext.cpp | 2 - src/qml/jsruntime/qv4qobjectwrapper.cpp | 1 - src/qml/jsruntime/qv4qobjectwrapper_p.h | 3 +- src/qml/qml/qqmltypewrapper.cpp | 47 +++++++++++++++++--- src/qml/qml/qqmltypewrapper_p.h | 2 + .../qqmlecmascript/data/SingletonLookupTest.qml | 14 ++++++ .../auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp | 50 ++++++++++++++++++++++ 8 files changed, 110 insertions(+), 11 deletions(-) create mode 100644 tests/auto/qml/qqmlecmascript/data/SingletonLookupTest.qml diff --git a/src/qml/jsruntime/qv4lookup_p.h b/src/qml/jsruntime/qv4lookup_p.h index 03dc5f6d3c..4fd5e133f7 100644 --- a/src/qml/jsruntime/qv4lookup_p.h +++ b/src/qml/jsruntime/qv4lookup_p.h @@ -120,7 +120,7 @@ struct Lookup { } indexedLookup; struct { Heap::InternalClass *ic; - Heap::QObjectWrapper *staticQObject; + Heap::InternalClass *qmlTypeIc; // only used when lookup goes through QQmlTypeWrapper QQmlPropertyCache *propertyCache; QQmlPropertyData *propertyData; } qobjectLookup; diff --git a/src/qml/jsruntime/qv4qmlcontext.cpp b/src/qml/jsruntime/qv4qmlcontext.cpp index 0c5226d46c..6aa0130188 100644 --- a/src/qml/jsruntime/qv4qmlcontext.cpp +++ b/src/qml/jsruntime/qv4qmlcontext.cpp @@ -293,7 +293,6 @@ ReturnedValue QQmlContextWrapper::getPropertyAndBase(const QQmlContextWrapper *r ScopedValue val(scope, base ? *base : Value::fromReturnedValue(QV4::QObjectWrapper::wrap(v4, scopeObject))); const QObjectWrapper *That = static_cast(val->objectValue()); lookup->qobjectLookup.ic = That->internalClass(); - lookup->qobjectLookup.staticQObject = nullptr; lookup->qobjectLookup.propertyCache = ddata->propertyCache; lookup->qobjectLookup.propertyCache->addref(); lookup->qobjectLookup.propertyData = propertyData; @@ -326,7 +325,6 @@ ReturnedValue QQmlContextWrapper::getPropertyAndBase(const QQmlContextWrapper *r ScopedValue val(scope, base ? *base : Value::fromReturnedValue(QV4::QObjectWrapper::wrap(v4, context->contextObject))); const QObjectWrapper *That = static_cast(val->objectValue()); lookup->qobjectLookup.ic = That->internalClass(); - lookup->qobjectLookup.staticQObject = nullptr; lookup->qobjectLookup.propertyCache = ddata->propertyCache; lookup->qobjectLookup.propertyCache->addref(); lookup->qobjectLookup.propertyData = propertyData; diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp index 6fed538fa8..63b6435112 100644 --- a/src/qml/jsruntime/qv4qobjectwrapper.cpp +++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp @@ -870,7 +870,6 @@ ReturnedValue QObjectWrapper::virtualResolveLookupGetter(const Object *object, E } lookup->qobjectLookup.ic = This->internalClass(); - lookup->qobjectLookup.staticQObject = nullptr; lookup->qobjectLookup.propertyCache = ddata->propertyCache; lookup->qobjectLookup.propertyCache->addref(); lookup->qobjectLookup.propertyData = property; diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h index 2558ede401..5543c4d5a6 100644 --- a/src/qml/jsruntime/qv4qobjectwrapper_p.h +++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h @@ -233,8 +233,7 @@ inline ReturnedValue QObjectWrapper::lookupGetterImpl(Lookup *lookup, ExecutionE if (!o || o->internalClass != lookup->qobjectLookup.ic) return revertLookup(); - const Heap::QObjectWrapper *This = lookup->qobjectLookup.staticQObject ? lookup->qobjectLookup.staticQObject : - static_cast(o); + const Heap::QObjectWrapper *This = static_cast(o); QObject *qobj = This->object(); if (QQmlData::wasDeleted(qobj)) return QV4::Encode::undefined(); diff --git a/src/qml/qml/qqmltypewrapper.cpp b/src/qml/qml/qqmltypewrapper.cpp index 24c5aecc00..c5fa4a04ec 100644 --- a/src/qml/qml/qqmltypewrapper.cpp +++ b/src/qml/qml/qqmltypewrapper.cpp @@ -459,16 +459,16 @@ ReturnedValue QQmlTypeWrapper::virtualResolveLookupGetter(const Object *object, if (!includeEnums || !name->startsWithUpper()) { QQmlData *ddata = QQmlData::get(qobjectSingleton, false); if (ddata && ddata->propertyCache) { - ScopedValue val(scope, Value::fromReturnedValue(QV4::QObjectWrapper::wrap(engine, qobjectSingleton))); QQmlPropertyData *property = ddata->propertyCache->property(name.getPointer(), qobjectSingleton, qmlContext); if (property) { - lookup->qobjectLookup.ic = This->internalClass(); - lookup->qobjectLookup.staticQObject = static_cast(val->heapObject()); + ScopedValue val(scope, Value::fromReturnedValue(QV4::QObjectWrapper::wrap(engine, qobjectSingleton))); + lookup->qobjectLookup.qmlTypeIc = This->internalClass(); + lookup->qobjectLookup.ic = val->objectValue()->internalClass(); lookup->qobjectLookup.propertyCache = ddata->propertyCache; lookup->qobjectLookup.propertyCache->addref(); lookup->qobjectLookup.propertyData = property; - lookup->getter = QV4::QObjectWrapper::lookupGetter; - return lookup->getter(lookup, engine, *This); + lookup->getter = QQmlTypeWrapper::lookupSingletonProperty; + return lookup->getter(lookup, engine, *object); } // Fall through to base implementation } @@ -488,6 +488,43 @@ bool QQmlTypeWrapper::virtualResolveLookupSetter(Object *object, ExecutionEngine return Object::virtualResolveLookupSetter(object, engine, lookup, value); } +ReturnedValue QQmlTypeWrapper::lookupSingletonProperty(Lookup *l, ExecutionEngine *engine, const Value &object) +{ + const auto revertLookup = [l, engine, &object]() { + l->qobjectLookup.propertyCache->release(); + l->qobjectLookup.propertyCache = nullptr; + l->getter = Lookup::getterGeneric; + return Lookup::getterGeneric(l, engine, object); + }; + + // we can safely cast to a QV4::Object here. If object is something else, + // the internal class won't match + Heap::Object *o = static_cast(object.heapObject()); + if (!o || o->internalClass != l->qobjectLookup.qmlTypeIc) + return revertLookup(); + + Heap::QQmlTypeWrapper *This = static_cast(o); + + QQmlType type = This->type(); + if (!type.isValid()) + return revertLookup(); + + if (!type.isSingleton()) + return revertLookup(); + + QQmlEngine *e = engine->qmlEngine(); + QQmlType::SingletonInstanceInfo *siinfo = type.singletonInstanceInfo(); + siinfo->init(e); + + QObject *qobjectSingleton = siinfo->qobjectApi(e); + if (!qobjectSingleton) + return revertLookup(); + + Scope scope(engine); + ScopedValue obj(scope, QV4::QObjectWrapper::wrap(engine, qobjectSingleton)); + return QObjectWrapper::lookupGetterImpl(l, engine, obj, /*useOriginalProperty*/ true, revertLookup); +} + void Heap::QQmlScopedEnumWrapper::destroy() { QQmlType::derefHandle(typePrivate); diff --git a/src/qml/qml/qqmltypewrapper_p.h b/src/qml/qml/qqmltypewrapper_p.h index 44e82dec2b..5f3cb2523f 100644 --- a/src/qml/qml/qqmltypewrapper_p.h +++ b/src/qml/qml/qqmltypewrapper_p.h @@ -114,6 +114,8 @@ struct Q_QML_EXPORT QQmlTypeWrapper : Object static ReturnedValue virtualResolveLookupGetter(const Object *object, ExecutionEngine *engine, Lookup *lookup); static bool virtualResolveLookupSetter(Object *object, ExecutionEngine *engine, Lookup *lookup, const Value &value); + static ReturnedValue lookupSingletonProperty(Lookup *l, ExecutionEngine *engine, const Value &base); + protected: static ReturnedValue virtualGet(const Managed *m, PropertyKey id, const Value *receiver, bool *hasProperty); static bool virtualPut(Managed *m, PropertyKey id, const Value &value, Value *receiver); diff --git a/tests/auto/qml/qqmlecmascript/data/SingletonLookupTest.qml b/tests/auto/qml/qqmlecmascript/data/SingletonLookupTest.qml new file mode 100644 index 0000000000..3166ab647d --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/SingletonLookupTest.qml @@ -0,0 +1,14 @@ +import QtQml 2.0 +import Test.Singletons 1.0 + +QtObject { + property Component singletonAccessor : Component { + QtObject { + property var singletonHolder; + property int result: singletonHolder.testVar + } + } + + property int firstLookup: singletonAccessor.createObject(this, { singletonHolder: CppSingleton1 }).result; + property int secondLookup: singletonAccessor.createObject(this, { singletonHolder: CppSingleton2 }).result; +} diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp index 007ad99655..c24c495b13 100644 --- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp +++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp @@ -369,6 +369,7 @@ private slots: void intMinDividedByMinusOne(); void undefinedPropertiesInObjectWrapper(); void hugeRegexpQuantifiers(); + void singletonTypeWrapperLookup(); private: // static void propertyVarWeakRefCallback(v8::Persistent object, void* parameter); @@ -8979,6 +8980,55 @@ void tst_qqmlecmascript::hugeRegexpQuantifiers() QVERIFY(value.isRegExp()); } +struct CppSingleton1 : public QObject +{ + Q_OBJECT + Q_PROPERTY(int testVar MEMBER testVar CONSTANT) +public: + const int testVar = 0; +}; + +struct CppSingleton2 : public QObject +{ + Q_OBJECT + Q_PROPERTY(int testVar MEMBER testVar CONSTANT) +public: + const int testVar = 1; +}; + +void tst_qqmlecmascript::singletonTypeWrapperLookup() +{ + QQmlEngine engine; + + auto singletonTypeId1 = qmlRegisterSingletonType("Test.Singletons", 1, 0, "CppSingleton1", + [](QQmlEngine *, QJSEngine *) -> QObject * { + return new CppSingleton1; + }); + + auto singletonTypeId2 = qmlRegisterSingletonType("Test.Singletons", 1, 0, "CppSingleton2", + [](QQmlEngine *, QJSEngine *) -> QObject * { + return new CppSingleton2; + }); + + auto cleanup = qScopeGuard([&]() { + qmlUnregisterType(singletonTypeId1); + qmlUnregisterType(singletonTypeId2); + }); + + QQmlComponent component(&engine, testFileUrl("SingletonLookupTest.qml")); + QScopedPointer test(component.create()); + QVERIFY2(!test.isNull(), qPrintable(component.errorString())); + + auto singleton1 = engine.singletonInstance(singletonTypeId1); + QVERIFY(singleton1); + + auto singleton2 = engine.singletonInstance(singletonTypeId2); + QVERIFY(singleton2); + + QCOMPARE(test->property("firstLookup").toInt(), singleton1->testVar); + QCOMPARE(test->property("secondLookup").toInt(), singleton2->testVar); +} + QTEST_MAIN(tst_qqmlecmascript) #include "tst_qqmlecmascript.moc" -- cgit v1.2.3