From d438be92dd7068fef94ce98e1ec039fe0ef4f3b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bernhard=20=C3=9Cbelacker?= <bernhardu@mailbox.org>
Date: Sun, 12 Feb 2017 14:10:43 +0100
Subject: 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>
---
 src/qml/qml/qqmldata_p.h          |  8 +++++---
 src/qml/qml/qqmlengine.cpp        | 30 ++++++++++--------------------
 src/qml/qml/qqmlobjectcreator.cpp |  4 +++-
 src/qml/qml/qqmlproperty.cpp      |  2 +-
 4 files changed, 19 insertions(+), 25 deletions(-)

(limited to 'src/qml/qml')

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);
-- 
cgit v1.2.3