diff options
Diffstat (limited to 'src/qml')
-rw-r--r-- | src/qml/animations/qcontinuinganimationgroupjob.cpp | 4 | ||||
-rw-r--r-- | src/qml/animations/qparallelanimationgroupjob.cpp | 4 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4context.cpp | 18 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4identifier.cpp | 4 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4identifiertable.cpp | 24 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4identifiertable_p.h | 5 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4internalclass.cpp | 12 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4internalclass_p.h | 2 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4module.cpp | 9 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4sequenceobject.cpp | 28 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4string_p.h | 22 | ||||
-rw-r--r-- | src/qml/qml/qqmldata_p.h | 64 | ||||
-rw-r--r-- | src/qml/qml/qqmlengine.cpp | 84 | ||||
-rw-r--r-- | src/qml/qml/qqmlvmemetaobject.cpp | 3 |
14 files changed, 198 insertions, 85 deletions
diff --git a/src/qml/animations/qcontinuinganimationgroupjob.cpp b/src/qml/animations/qcontinuinganimationgroupjob.cpp index 88c0e9e60e..61a9dc36f8 100644 --- a/src/qml/animations/qcontinuinganimationgroupjob.cpp +++ b/src/qml/animations/qcontinuinganimationgroupjob.cpp @@ -82,9 +82,9 @@ void QContinuingAnimationGroupJob::updateState(QAbstractAnimationJob::State newS return; } for (QAbstractAnimationJob *animation = firstChild(); animation; animation = animation->nextSibling()) { - resetUncontrolledAnimationFinishTime(animation); + RETURN_IF_DELETED(resetUncontrolledAnimationFinishTime(animation)); animation->setDirection(m_direction); - animation->start(); + RETURN_IF_DELETED(animation->start()); } break; } diff --git a/src/qml/animations/qparallelanimationgroupjob.cpp b/src/qml/animations/qparallelanimationgroupjob.cpp index 420a934ba2..a828d0e234 100644 --- a/src/qml/animations/qparallelanimationgroupjob.cpp +++ b/src/qml/animations/qparallelanimationgroupjob.cpp @@ -144,10 +144,10 @@ void QParallelAnimationGroupJob::updateState(QAbstractAnimationJob::State newSta animation->stop(); m_previousLoop = m_direction == Forward ? 0 : m_loopCount - 1; } - resetUncontrolledAnimationFinishTime(animation); + RETURN_IF_DELETED(resetUncontrolledAnimationFinishTime(animation)); animation->setDirection(m_direction); if (shouldAnimationStart(animation, oldState == Stopped)) - animation->start(); + RETURN_IF_DELETED(animation->start()); } break; } diff --git a/src/qml/jsruntime/qv4context.cpp b/src/qml/jsruntime/qv4context.cpp index 018571e325..c547c0692d 100644 --- a/src/qml/jsruntime/qv4context.cpp +++ b/src/qml/jsruntime/qv4context.cpp @@ -334,9 +334,14 @@ ReturnedValue ExecutionContext::getProperty(String *name) case Heap::ExecutionContext::Type_CallContext: { Heap::CallContext *c = static_cast<Heap::CallContext *>(ctx); - uint index = c->internalClass->indexOfValueOrGetter(id); - if (index < UINT_MAX) + const uint index = c->internalClass->indexOfValueOrGetter(id); + if (index < c->locals.alloc) return c->locals[index].asReturnedValue(); + + // TODO: We should look up the module imports here, but those are part of the CU: + // imports[index - c->locals.size]; + // See QTBUG-118478 + Q_FALLTHROUGH(); } case Heap::ExecutionContext::Type_WithContext: @@ -384,9 +389,14 @@ ReturnedValue ExecutionContext::getPropertyAndBase(String *name, Value *base) case Heap::ExecutionContext::Type_CallContext: { Heap::CallContext *c = static_cast<Heap::CallContext *>(ctx); - uint index = c->internalClass->indexOfValueOrGetter(id); - if (index < UINT_MAX) + const uint index = c->internalClass->indexOfValueOrGetter(id); + if (index < c->locals.alloc) return c->locals[index].asReturnedValue(); + + // TODO: We should look up the module imports here, but those are part of the CU: + // imports[index - c->locals.size]; + // See QTBUG-118478 + Q_FALLTHROUGH(); } case Heap::ExecutionContext::Type_GlobalContext: { diff --git a/src/qml/jsruntime/qv4identifier.cpp b/src/qml/jsruntime/qv4identifier.cpp index c3d7165f71..266ed17c18 100644 --- a/src/qml/jsruntime/qv4identifier.cpp +++ b/src/qml/jsruntime/qv4identifier.cpp @@ -152,7 +152,7 @@ const IdentifierHashEntry *IdentifierHash::lookup(const QString &str) const if (!d) return nullptr; - PropertyKey id = d->identifierTable->asPropertyKey(str); + PropertyKey id = d->identifierTable->asPropertyKey(str, IdentifierTable::ForceConversionToId); return lookup(id); } @@ -169,7 +169,7 @@ const IdentifierHashEntry *IdentifierHash::lookup(String *str) const const PropertyKey IdentifierHash::toIdentifier(const QString &str) const { Q_ASSERT(d); - return d->identifierTable->asPropertyKey(str); + return d->identifierTable->asPropertyKey(str, IdentifierTable::ForceConversionToId); } const PropertyKey IdentifierHash::toIdentifier(Heap::String *str) const diff --git a/src/qml/jsruntime/qv4identifiertable.cpp b/src/qml/jsruntime/qv4identifiertable.cpp index 21b47c3909..ad6b39e0b1 100644 --- a/src/qml/jsruntime/qv4identifiertable.cpp +++ b/src/qml/jsruntime/qv4identifiertable.cpp @@ -132,16 +132,23 @@ void IdentifierTable::addEntry(Heap::StringOrSymbol *str) -Heap::String *IdentifierTable::insertString(const QString &s) +Heap::String *IdentifierTable::insertString( + const QString &s, IdentifierTable::KeyConversionBehavior conversionBehavior) { uint subtype; - uint hash = String::createHashValue(s.constData(), s.length(), &subtype); + + uint hash = String::createHashValue(s.constData(), s.size(), &subtype); if (subtype == Heap::String::StringType_ArrayIndex) { - Heap::String *str = engine->newString(s); - str->stringHash = hash; - str->subtype = subtype; - return str; + if (Q_UNLIKELY(conversionBehavior == ForceConversionToId)) { + hash = String::createHashValueDisallowingArrayIndex(s.constData(), s.size(), &subtype); + } else { + Heap::String *str = engine->newString(s); + str->stringHash = hash; + str->subtype = subtype; + return str; + } } + uint idx = hash % alloc; while (Heap::StringOrSymbol *e = entriesByHash[idx]) { if (e->stringHash == hash && e->toQString() == s) @@ -278,9 +285,10 @@ void IdentifierTable::sweep() size -= freed; } -PropertyKey IdentifierTable::asPropertyKey(const QString &s) +PropertyKey IdentifierTable::asPropertyKey( + const QString &s, IdentifierTable::KeyConversionBehavior conversionBehavior) { - return insertString(s)->identifier; + return insertString(s, conversionBehavior)->identifier; } PropertyKey IdentifierTable::asPropertyKey(const char *s, int len) diff --git a/src/qml/jsruntime/qv4identifiertable_p.h b/src/qml/jsruntime/qv4identifiertable_p.h index 78e2b6620e..1dda65f2ed 100644 --- a/src/qml/jsruntime/qv4identifiertable_p.h +++ b/src/qml/jsruntime/qv4identifiertable_p.h @@ -75,11 +75,12 @@ struct Q_QML_PRIVATE_EXPORT IdentifierTable void addEntry(Heap::StringOrSymbol *str); public: + enum KeyConversionBehavior { Default, ForceConversionToId }; IdentifierTable(ExecutionEngine *engine, int numBits = 8); ~IdentifierTable(); - Heap::String *insertString(const QString &s); + Heap::String *insertString(const QString &s, KeyConversionBehavior conversionBehavior); Heap::Symbol *insertSymbol(const QString &s); PropertyKey asPropertyKey(const Heap::String *str) { @@ -91,7 +92,7 @@ public: return asPropertyKey(str->d()); } - PropertyKey asPropertyKey(const QString &s); + PropertyKey asPropertyKey(const QString &s, KeyConversionBehavior conversionBehavior = Default); PropertyKey asPropertyKey(const char *s, int len); PropertyKey asPropertyKeyImpl(const Heap::String *str); diff --git a/src/qml/jsruntime/qv4internalclass.cpp b/src/qml/jsruntime/qv4internalclass.cpp index 904c6a5eaf..d6211bab1a 100644 --- a/src/qml/jsruntime/qv4internalclass.cpp +++ b/src/qml/jsruntime/qv4internalclass.cpp @@ -329,9 +329,15 @@ void InternalClass::destroy() Base::destroy(); } -QString InternalClass::keyAt(uint index) const -{ - return nameMap.at(index).toQString(); +ReturnedValue InternalClass::keyAt(uint index) const +{ + PropertyKey key = nameMap.at(index); + if (!key.isValid()) + return Encode::undefined(); + if (key.isArrayIndex()) + return Encode(key.asArrayIndex()); + Q_ASSERT(key.isStringOrSymbol()); + return key.asStringOrSymbol()->asReturnedValue(); } void InternalClass::changeMember(QV4::Object *object, PropertyKey id, PropertyAttributes data, InternalClassEntry *entry) diff --git a/src/qml/jsruntime/qv4internalclass_p.h b/src/qml/jsruntime/qv4internalclass_p.h index a5a1471cf1..5fe97e8029 100644 --- a/src/qml/jsruntime/qv4internalclass_p.h +++ b/src/qml/jsruntime/qv4internalclass_p.h @@ -344,7 +344,7 @@ struct InternalClass : Base { void init(InternalClass *other); void destroy(); - Q_QML_PRIVATE_EXPORT QString keyAt(uint index) const; + Q_QML_PRIVATE_EXPORT ReturnedValue keyAt(uint index) const; Q_REQUIRED_RESULT InternalClass *nonExtensible(); static void addMember(QV4::Object *object, PropertyKey id, PropertyAttributes data, InternalClassEntry *entry); diff --git a/src/qml/jsruntime/qv4module.cpp b/src/qml/jsruntime/qv4module.cpp index 08a1900383..8f148994e3 100644 --- a/src/qml/jsruntime/qv4module.cpp +++ b/src/qml/jsruntime/qv4module.cpp @@ -258,9 +258,12 @@ OwnPropertyKeyIterator *Module::virtualOwnPropertyKeys(const Object *o, Value *t if (module->d()->unit->isESModule()) { names = module->d()->unit->exportedNames(); } else { - Heap::InternalClass *scopeClass = module->d()->scope->internalClass; - for (uint i = 0; i < scopeClass->size; ++i) - names << scopeClass->keyAt(i); + QV4::Scope scope(module->engine()); + QV4::Scoped<InternalClass> scopeClass(scope, module->d()->scope->internalClass); + for (uint i = 0, end = scopeClass->d()->size; i < end; ++i) { + QV4::ScopedValue key(scope, scopeClass->d()->keyAt(i)); + names << key->toQString(); + } } return new ModuleNamespaceIterator(names); diff --git a/src/qml/jsruntime/qv4sequenceobject.cpp b/src/qml/jsruntime/qv4sequenceobject.cpp index f84718b48f..04b6cfc921 100644 --- a/src/qml/jsruntime/qv4sequenceobject.cpp +++ b/src/qml/jsruntime/qv4sequenceobject.cpp @@ -261,7 +261,33 @@ struct QQmlSequence : Object { template <typename Container> struct QQmlSequence : public QV4::Object { - V4_OBJECT2(QQmlSequence<Container>, QV4::Object) + // Unroll V4_OBJECT2(QQmlSequence<Container>, QV4::Object) here. + // Newer C++ versions don't tolerate the template argument in Q_DISABLE_COPY. +private: + QQmlSequence() Q_DECL_EQ_DELETE; + Q_DISABLE_COPY(QQmlSequence) +public: + Q_MANAGED_CHECK + typedef QV4::Heap::QQmlSequence<Container> Data; + typedef QV4::Object SuperClass; + static const QV4::VTable static_vtbl; + static inline const QV4::VTable *staticVTable() { return &static_vtbl; } + V4_MANAGED_SIZE_TEST + + QV4::Heap::QQmlSequence<Container> *d_unchecked() const + { + return static_cast<QV4::Heap::QQmlSequence<Container> *>(m()); + } + + QV4::Heap::QQmlSequence<Container> *d() const + { + QV4::Heap::QQmlSequence<Container> *dptr = d_unchecked(); + dptr->_checkIsInitialized(); + return dptr; + } + + Q_STATIC_ASSERT(std::is_trivial< QV4::Heap::QQmlSequence<Container>>::value); + Q_MANAGED_TYPE(QmlSequence) V4_PROTOTYPE(sequencePrototype) V4_NEEDS_DESTROY diff --git a/src/qml/jsruntime/qv4string_p.h b/src/qml/jsruntime/qv4string_p.h index 52fe09cd72..55a0ff4316 100644 --- a/src/qml/jsruntime/qv4string_p.h +++ b/src/qml/jsruntime/qv4string_p.h @@ -222,6 +222,12 @@ struct Q_QML_PRIVATE_EXPORT String : public StringOrSymbol { return calculateHashValue(ch, end, subtype); } + static uint createHashValueDisallowingArrayIndex(const QChar *ch, int length, uint *subtype) + { + const QChar *end = ch + length; + return calculateHashValue<String::DisallowArrayIndex>(ch, end, subtype); + } + static uint createHashValue(const char *ch, int length, uint *subtype) { const char *end = ch + length; @@ -235,15 +241,19 @@ protected: static qint64 virtualGetLength(const Managed *m); public: - template <typename T> + enum IndicesBehavior {Default, DisallowArrayIndex}; + template <IndicesBehavior Behavior = Default, typename T> static inline uint calculateHashValue(const T *ch, const T* end, uint *subtype) { // array indices get their number as hash value - uint h = stringToArrayIndex(ch, end); - if (h != UINT_MAX) { - if (subtype) - *subtype = Heap::StringOrSymbol::StringType_ArrayIndex; - return h; + uint h = UINT_MAX; + if (Behavior != DisallowArrayIndex) { + h = stringToArrayIndex(ch, end); + if (h != UINT_MAX) { + if (subtype) + *subtype = Heap::StringOrSymbol::StringType_ArrayIndex; + return h; + } } while (ch < end) { diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h index ee31cb38d9..c44fb8608e 100644 --- a/src/qml/qml/qqmldata_p.h +++ b/src/qml/qml/qqmldata_p.h @@ -176,24 +176,24 @@ public: }; struct NotifyList { - quint64 connectionMask; - - quint16 maximumTodoIndex; - quint16 notifiesSize; - - QQmlNotifierEndpoint *todo; - QQmlNotifierEndpoint**notifies; + quint64 connectionMask = 0; + QQmlNotifierEndpoint *todo = nullptr; + QQmlNotifierEndpoint**notifies = nullptr; + quint16 maximumTodoIndex = 0; + quint16 notifiesSize = 0; void layout(); private: void layout(QQmlNotifierEndpoint*); }; - NotifyList *notifyList; + QAtomicPointer<NotifyList> notifyList; - inline QQmlNotifierEndpoint *notify(int index); + inline QQmlNotifierEndpoint *notify(int index) const; void addNotify(int index, QQmlNotifierEndpoint *); int endpointCount(int index); bool signalHasEndpoint(int index) const; - void disconnectNotifiers(); + + enum class DeleteNotifyList { Yes, No }; + void disconnectNotifiers(DeleteNotifyList doDelete); // The context that created the C++ object QQmlContextData *context = nullptr; @@ -342,23 +342,31 @@ bool QQmlData::wasDeleted(const QObject *object) return ddata && ddata->isQueuedForDeletion; } -QQmlNotifierEndpoint *QQmlData::notify(int index) +inline bool isIndexInConnectionMask(quint64 connectionMask, int index) +{ + return connectionMask & (1ULL << quint64(index % 64)); +} + +QQmlNotifierEndpoint *QQmlData::notify(int index) const { + // Can only happen on "home" thread. We apply relaxed semantics when loading the atomics. + Q_ASSERT(index <= 0xFFFF); - if (!notifyList || !(notifyList->connectionMask & (1ULL << quint64(index % 64)))) { + NotifyList *list = notifyList.loadRelaxed(); + if (!list || !isIndexInConnectionMask(list->connectionMask, index)) return nullptr; - } else if (index < notifyList->notifiesSize) { - return notifyList->notifies[index]; - } else if (index <= notifyList->maximumTodoIndex) { - notifyList->layout(); - } - if (index < notifyList->notifiesSize) { - return notifyList->notifies[index]; - } else { - return nullptr; + if (index < list->notifiesSize) + return list->notifies[index]; + + if (index <= list->maximumTodoIndex) { + list->layout(); + if (index < list->notifiesSize) + return list->notifies[index]; } + + return nullptr; } /* @@ -367,7 +375,19 @@ QQmlNotifierEndpoint *QQmlData::notify(int index) */ inline bool QQmlData::signalHasEndpoint(int index) const { - return notifyList && (notifyList->connectionMask & (1ULL << quint64(index % 64))); + // This can be called from any thread. + // We still use relaxed semantics. If we're on a thread different from the "home" thread + // of the QQmlData, two interesting things might happen: + // + // 1. The list might go away while we hold it. In that case we are dealing with an object whose + // QObject dtor is being executed concurrently. This is UB already without the notify lists. + // Therefore, we don't need to consider it. + // 2. The connectionMask may be amended or zeroed while we are looking at it. In that case + // we "misreport" the endpoint. Since ordering of events across threads is inherently + // nondeterministic, either result is correct in that case. We can accept it. + + NotifyList *list = notifyList.loadRelaxed(); + return list && isIndexInConnectionMask(list->connectionMask, index); } bool QQmlData::hasBindingBit(int coreIndex) const diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp index 852a673ebd..2325c4c1e0 100644 --- a/src/qml/qml/qqmlengine.cpp +++ b/src/qml/qml/qqmlengine.cpp @@ -718,7 +718,7 @@ void QQmlPrivate::qdeclarativeelement_destructor(QObject *o) // Disconnect the notifiers now - during object destruction this would be too late, since // the disconnect call wouldn't be able to call disconnectNotify(), as it isn't possible to // get the metaobject anymore. - d->disconnectNotifiers(); + d->disconnectNotifiers(QQmlData::DeleteNotifyList::No); } } @@ -789,7 +789,10 @@ void QQmlData::signalEmitted(QAbstractDeclarativeData *, QObject *object, int in // QQmlEngine to emit signals from a different thread. These signals are then automatically // marshalled back onto the QObject's thread and handled by QML from there. This is tested // by the qqmlecmascript::threadSignal() autotest. - if (!ddata->notifyList) + + // Relaxed semantics here. If we're on a different thread we might schedule a useless event, + // but that should be rare. + if (!ddata->notifyList.loadRelaxed()) return; auto objectThreadData = QObjectPrivate::get(object)->threadData.loadRelaxed(); @@ -1835,49 +1838,72 @@ void QQmlData::releaseDeferredData() void QQmlData::addNotify(int index, QQmlNotifierEndpoint *endpoint) { - if (!notifyList) { - notifyList = (NotifyList *)malloc(sizeof(NotifyList)); - notifyList->connectionMask = 0; - notifyList->maximumTodoIndex = 0; - notifyList->notifiesSize = 0; - notifyList->todo = nullptr; - notifyList->notifies = nullptr; + // Can only happen on "home" thread. We apply relaxed semantics when loading the atomics. + + NotifyList *list = notifyList.loadRelaxed(); + + if (!list) { + list = new NotifyList; + // We don't really care when this change takes effect on other threads. The notifyList can + // only become non-null once in the life time of a QQmlData. It becomes null again when the + // underlying QObject is deleted. At that point any interaction with the QQmlData is UB + // anyway. So, for all intents and purposese, the list becomes non-null once and then stays + // non-null "forever". We can apply relaxed semantics. + notifyList.storeRelaxed(list); } Q_ASSERT(!endpoint->isConnected()); index = qMin(index, 0xFFFF - 1); - notifyList->connectionMask |= (1ULL << quint64(index % 64)); - if (index < notifyList->notifiesSize) { + // Likewise, we don't really care _when_ the change in the connectionMask is propagated to other + // threads. Cross-thread event ordering is inherently nondeterministic. Therefore, when querying + // the conenctionMask in the presence of concurrent modification, any result is correct. + list->connectionMask |= (1ULL << quint64(index % 64)); - endpoint->next = notifyList->notifies[index]; + if (index < list->notifiesSize) { + endpoint->next = list->notifies[index]; if (endpoint->next) endpoint->next->prev = &endpoint->next; - endpoint->prev = ¬ifyList->notifies[index]; - notifyList->notifies[index] = endpoint; - + endpoint->prev = &list->notifies[index]; + list->notifies[index] = endpoint; } else { - notifyList->maximumTodoIndex = qMax(int(notifyList->maximumTodoIndex), index); + list->maximumTodoIndex = qMax(int(list->maximumTodoIndex), index); - endpoint->next = notifyList->todo; + endpoint->next = list->todo; if (endpoint->next) endpoint->next->prev = &endpoint->next; - endpoint->prev = ¬ifyList->todo; - notifyList->todo = endpoint; + endpoint->prev = &list->todo; + list->todo = endpoint; } } -void QQmlData::disconnectNotifiers() +void QQmlData::disconnectNotifiers(QQmlData::DeleteNotifyList doDelete) { - if (notifyList) { - while (notifyList->todo) - notifyList->todo->disconnect(); - for (int ii = 0; ii < notifyList->notifiesSize; ++ii) { - while (QQmlNotifierEndpoint *ep = notifyList->notifies[ii]) + // Can only happen on "home" thread. We apply relaxed semantics when loading the atomics. + if (NotifyList *list = notifyList.loadRelaxed()) { + while (QQmlNotifierEndpoint *todo = list->todo) + todo->disconnect(); + for (int ii = 0; ii < list->notifiesSize; ++ii) { + while (QQmlNotifierEndpoint *ep = list->notifies[ii]) ep->disconnect(); } - free(notifyList->notifies); - free(notifyList); - notifyList = nullptr; + free(list->notifies); + + if (doDelete == DeleteNotifyList::Yes) { + // We can only get here from QQmlData::destroyed(), and that can only come from the + // the QObject dtor. If you're still sending signals at that point you have UB already + // without any threads. Therefore, it's enough to apply relaxed semantics. + notifyList.storeRelaxed(nullptr); + delete list; + } else { + // We can use relaxed semantics here. The worst thing that can happen is that some + // signal is falsely reported as connected. Signal connectedness across threads + // is not quite deterministic anyway. + list->connectionMask = 0; + list->maximumTodoIndex = 0; + list->notifiesSize = 0; + list->notifies = nullptr; + + } } } @@ -1961,7 +1987,7 @@ void QQmlData::destroyed(QObject *object) guard->objectDestroyed(object); } - disconnectNotifiers(); + disconnectNotifiers(DeleteNotifyList::Yes); if (extendedData) delete extendedData; diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp index 1e0e4e419f..4fd2092fd3 100644 --- a/src/qml/qml/qqmlvmemetaobject.cpp +++ b/src/qml/qml/qqmlvmemetaobject.cpp @@ -231,6 +231,9 @@ void QQmlVMEMetaObjectEndpoint::tryConnect() const QV4::CompiledData::Alias *aliasData = &metaObject->compiledObject->aliasTable()[aliasId]; if (!aliasData->isObjectAlias()) { QQmlContextData *ctxt = metaObject->ctxt; + if (!ctxt->engine) + return; + QObject *target = ctxt->idValues[aliasData->targetObjectId()].data(); if (!target) return; |