aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBernhard Übelacker <bernhardu@mailbox.org>2017-02-12 14:10:43 +0100
committerEdward Welbourne <edward.welbourne@qt.io>2017-04-10 17:05:50 +0000
commitd438be92dd7068fef94ce98e1ec039fe0ef4f3b3 (patch)
treebc78d2190469c83f656cf789ea647e5ed81bbeba
parent617d6dc2017f49a84e4aeb15a40d78462be62326 (diff)
Avoid access to declarativeData when isDeletingChildren is set
QObject's members declarativeData and currentChildBeingDeleted share the same memory because they are inside a union. This leads to a problem when destructing mixed Widgets and QML objects. Then in QObjectPrivate::deleteChildren the member currentChildBeingDeleted is set. But unfortunatley QObjectWrapper::destroyObject retrieves the same pointer via declarativeData. This patch should avoid this by disallowing retrieval of declarativeData when isDeletingChildren is set (or at least adds a Q_ASSERT). Task-number: QTBUG-57714 Change-Id: I9ee02f79be3e8226c30076c24859b49b8dcfaecf Reviewed-by: Simon Hausmann <simon.hausmann@qt.io> Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>
-rw-r--r--src/qml/jsruntime/qv4qobjectwrapper_p.h11
-rw-r--r--src/qml/qml/qqmldata_p.h8
-rw-r--r--src/qml/qml/qqmlengine.cpp30
-rw-r--r--src/qml/qml/qqmlobjectcreator.cpp4
-rw-r--r--src/qml/qml/qqmlproperty.cpp2
-rw-r--r--src/qml/types/qqmldelegatemodel.cpp11
6 files changed, 27 insertions, 39 deletions
diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h
index c7c4f4dd77..3764943499 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper_p.h
+++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h
@@ -209,13 +209,10 @@ inline ReturnedValue QObjectWrapper::wrap(ExecutionEngine *engine, QObject *obje
if (Q_UNLIKELY(QQmlData::wasDeleted(object)))
return QV4::Encode::null();
- QObjectPrivate *priv = QObjectPrivate::get(const_cast<QObject *>(object));
- if (Q_LIKELY(priv->declarativeData)) {
- auto ddata = static_cast<QQmlData *>(priv->declarativeData);
- if (Q_LIKELY(ddata->jsEngineId == engine->m_engineId && !ddata->jsWrapper.isUndefined())) {
- // We own the JS object
- return ddata->jsWrapper.value();
- }
+ auto ddata = QQmlData::get(object);
+ if (Q_LIKELY(ddata && ddata->jsEngineId == engine->m_engineId && !ddata->jsWrapper.isUndefined())) {
+ // We own the JS object
+ return ddata->jsWrapper.value();
}
return wrap_slowPath(engine, object);
diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h
index e271598c2d..2083326cd5 100644
--- a/src/qml/qml/qqmldata_p.h
+++ b/src/qml/qml/qqmldata_p.h
@@ -201,7 +201,9 @@ public:
static QQmlData *get(const QObject *object, bool create = false) {
QObjectPrivate *priv = QObjectPrivate::get(const_cast<QObject *>(object));
- if (priv->wasDeleted) {
+ // If QObjectData::isDeletingChildren is set then access to QObjectPrivate::declarativeData has
+ // to be avoided because QObjectPrivate::currentChildBeingDeleted is in use.
+ if (priv->isDeletingChildren || priv->wasDeleted) {
Q_ASSERT(!create);
return 0;
} else if (priv->declarativeData) {
@@ -269,8 +271,8 @@ bool QQmlData::wasDeleted(QObject *object)
if (!priv || priv->wasDeleted)
return true;
- return priv->declarativeData &&
- static_cast<QQmlData *>(priv->declarativeData)->isQueuedForDeletion;
+ QQmlData *ddata = QQmlData::get(object);
+ return ddata && ddata->isQueuedForDeletion;
}
QQmlNotifierEndpoint *QQmlData::notify(int index)
diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index f1c592b632..1eb892a0de 100644
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -692,9 +692,7 @@ QQmlEnginePrivate::~QQmlEnginePrivate()
void QQmlPrivate::qdeclarativeelement_destructor(QObject *o)
{
- QObjectPrivate *p = QObjectPrivate::get(o);
- if (p->declarativeData) {
- QQmlData *d = static_cast<QQmlData*>(p->declarativeData);
+ if (QQmlData *d = QQmlData::get(o)) {
if (d->ownContext && d->context) {
d->context->destroy();
d->context = 0;
@@ -864,13 +862,10 @@ void QQmlData::markAsDeleted(QObject *o)
void QQmlData::setQueuedForDeletion(QObject *object)
{
if (object) {
- if (QObjectPrivate *priv = QObjectPrivate::get(object)) {
- if (!priv->wasDeleted && priv->declarativeData) {
- QQmlData *ddata = QQmlData::get(object, false);
- if (ddata->ownContext && ddata->context)
- ddata->context->emitDestruction();
- ddata->isQueuedForDeletion = true;
- }
+ if (QQmlData *ddata = QQmlData::get(object)) {
+ if (ddata->ownContext && ddata->context)
+ ddata->context->emitDestruction();
+ ddata->isQueuedForDeletion = true;
}
}
}
@@ -1319,17 +1314,11 @@ QQmlContext *QQmlEngine::contextForObject(const QObject *object)
if(!object)
return 0;
- QObjectPrivate *priv = QObjectPrivate::get(const_cast<QObject *>(object));
-
- QQmlData *data =
- static_cast<QQmlData *>(priv->declarativeData);
-
- if (!data)
- return 0;
- else if (data->outerContext)
+ QQmlData *data = QQmlData::get(object);
+ if (data && data->outerContext)
return data->outerContext->asQQmlContext();
- else
- return 0;
+
+ return 0;
}
/*!
@@ -1864,6 +1853,7 @@ void QQmlData::setPendingBindingBit(QObject *obj, int coreIndex)
QQmlData *QQmlData::createQQmlData(QObjectPrivate *priv)
{
Q_ASSERT(priv);
+ Q_ASSERT(!priv->isDeletingChildren);
priv->declarativeData = new QQmlData;
return static_cast<QQmlData *>(priv->declarativeData);
}
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index 09936f6e7a..3ed3ce5460 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -1075,7 +1075,9 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent, bool isCo
{
QQmlData *ddata = new (ddataMemory) QQmlData;
ddata->ownMemory = false;
- QObjectPrivate::get(instance)->declarativeData = ddata;
+ QObjectPrivate* p = QObjectPrivate::get(instance);
+ Q_ASSERT(!p->isDeletingChildren);
+ p->declarativeData = ddata;
}
const int parserStatusCast = type->parserStatusCast();
diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp
index 7df8336f51..cec04d25a1 100644
--- a/src/qml/qml/qqmlproperty.cpp
+++ b/src/qml/qml/qqmlproperty.cpp
@@ -1628,7 +1628,7 @@ QMetaMethod QQmlPropertyPrivate::findSignalByName(const QMetaObject *mo, const Q
*/
static inline void flush_vme_signal(const QObject *object, int index, bool indexInSignalRange)
{
- QQmlData *data = static_cast<QQmlData *>(QObjectPrivate::get(const_cast<QObject *>(object))->declarativeData);
+ QQmlData *data = QQmlData::get(object);
if (data && data->propertyCache) {
QQmlPropertyData *property = indexInSignalRange ? data->propertyCache->signal(index)
: data->propertyCache->method(index);
diff --git a/src/qml/types/qqmldelegatemodel.cpp b/src/qml/types/qqmldelegatemodel.cpp
index d9a8b1d179..62a9c40c07 100644
--- a/src/qml/types/qqmldelegatemodel.cpp
+++ b/src/qml/types/qqmldelegatemodel.cpp
@@ -1959,9 +1959,8 @@ void QQmlDelegateModelItem::destroyObject()
Q_ASSERT(object);
Q_ASSERT(contextData);
- QObjectPrivate *p = QObjectPrivate::get(object);
- Q_ASSERT(p->declarativeData);
- QQmlData *data = static_cast<QQmlData*>(p->declarativeData);
+ QQmlData *data = QQmlData::get(object);
+ Q_ASSERT(data);
if (data->ownContext && data->context)
data->context->clearContext();
object->deleteLater();
@@ -1978,10 +1977,8 @@ void QQmlDelegateModelItem::destroyObject()
QQmlDelegateModelItem *QQmlDelegateModelItem::dataForObject(QObject *object)
{
- QObjectPrivate *p = QObjectPrivate::get(object);
- QQmlContextData *context = p->declarativeData
- ? static_cast<QQmlData *>(p->declarativeData)->context
- : 0;
+ QQmlData *d = QQmlData::get(object);
+ QQmlContextData *context = d ? d->context : 0;
for (context = context ? context->parent : 0; context; context = context->parent) {
if (QQmlDelegateModelItem *cacheItem = qobject_cast<QQmlDelegateModelItem *>(
context->contextObject)) {