QObject/QWidget::setParent: add assertions to prevent loops

It is perfectly possible to accidentally create a parent/child
loop. This can happens by direct means (a->setParent(b);
b->setParent(a);), or some more subtle means, e.g.

class MyClass : public QObject {
   MyClass() : QObject(this) {}
};

Since this is UB, add a few robustness checks to make sure the
code above crashes right away (at least in debug builds).

Change-Id: I6583c8514b4c1f8a90677b04c77b8e8f0c15dba3
Reviewed-by: Liang Qi <liang.qi@qt.io>
Reviewed-by: Sérgio Martins <sergio.martins@kdab.com>
 
 
 
 
 
 
 

3 files changed, 45 insertions, 0 deletions

diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index 965857d408..b4e7568a23 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -58,6 +58,7 @@
 #include <qdebug.h>
 #include <qpair.h>
 #include <qvarlengtharray.h>
+#include <qscopeguard.h>
 #include <qset.h>
 #if QT_CONFIG(thread)
 #include <qsemaphore.h>
@@ -851,6 +852,8 @@ static bool check_parent_thread(QObject *parent,
 QObject::QObject(QObject *parent)
     : d_ptr(new QObjectPrivate)
 {
+    Q_ASSERT_X(this != parent, Q_FUNC_INFO, "Cannot parent a QObject to itself");
+
     Q_D(QObject);
     d_ptr->q_ptr = this;
     d->threadData = (parent && !parent->thread()) ? parent->d_func()->threadData : QThreadData::current();
@@ -879,6 +882,8 @@ QObject::QObject(QObject *parent)
 QObject::QObject(QObjectPrivate &dd, QObject *parent)
     : d_ptr(&dd)
 {
+    Q_ASSERT_X(this != parent, Q_FUNC_INFO, "Cannot parent a QObject to itself");
+
     Q_D(QObject);
     d_ptr->q_ptr = this;
     d->threadData = (parent && !parent->thread()) ? parent->d_func()->threadData : QThreadData::current();
@@ -2069,8 +2074,25 @@ void QObjectPrivate::deleteChildren()
 void QObjectPrivate::setParent_helper(QObject *o)
 {
     Q_Q(QObject);
+    Q_ASSERT_X(q != o, Q_FUNC_INFO, "Cannot parent a QObject to itself");
+#ifdef QT_DEBUG
+    const auto checkForParentChildLoops = qScopeGuard([&](){
+        int depth = 0;
+        auto p = parent;
+        while (p) {
+            if (++depth == CheckForParentChildLoopsWarnDepth) {
+                qWarning("QObject %p (class: '%s', object name: '%s') may have a loop in its parent-child chain; "
+                         "this is undefined behavior",
+                         q, q->metaObject()->className(), qPrintable(q->objectName()));
+            }
+            p = p->parent();
+        }
+    });
+#endif
+
     if (o == parent)
         return;
+
     if (parent) {
         QObjectPrivate *parentD = parent->d_func();
         if (parentD->isDeletingChildren && wasDeleted
diff --git a/src/corelib/kernel/qobject.h b/src/corelib/kernel/qobject.h
index ad5e1163bf..7f72b69c1a 100644
--- a/src/corelib/kernel/qobject.h
+++ b/src/corelib/kernel/qobject.h
@@ -113,6 +113,10 @@ public:
     int postedEvents;
     QDynamicMetaObjectData *metaObject;
     QMetaObject *dynamicMetaObject() const;
+
+#ifdef QT_DEBUG
+    enum { CheckForParentChildLoopsWarnDepth = 4096 };
+#endif
 };
 
 
diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp
index 8f927e8bee..58963d5eec 100644
--- a/src/widgets/kernel/qwidget.cpp
+++ b/src/widgets/kernel/qwidget.cpp
@@ -79,6 +79,7 @@
 #include "private/qstylesheetstyle_p.h"
 #include "private/qstyle_p.h"
 #include "qfileinfo.h"
+#include "qscopeguard.h"
 #include <QtGui/private/qhighdpiscaling_p.h>
 #include <QtGui/qinputmethod.h>
 #include <QtGui/qopenglcontext.h>
@@ -1123,6 +1124,8 @@ void QWidgetPrivate::adjustFlags(Qt::WindowFlags &flags, QWidget *w)
 void QWidgetPrivate::init(QWidget *parentWidget, Qt::WindowFlags f)
 {
     Q_Q(QWidget);
+    Q_ASSERT_X(q != parentWidget, Q_FUNC_INFO, "Cannot parent a QWidget to itself");
+
     if (Q_UNLIKELY(!qobject_cast<QApplication *>(QCoreApplication::instance())))
         qFatal("QWidget: Cannot create a QWidget without QApplication");
 
@@ -10677,6 +10680,22 @@ static void sendWindowChangeToTextureChildrenRecursively(QWidget *widget)
 void QWidget::setParent(QWidget *parent, Qt::WindowFlags f)
 {
     Q_D(QWidget);
+    Q_ASSERT_X(this != parent, Q_FUNC_INFO, "Cannot parent a QWidget to itself");
+#ifdef QT_DEBUG
+    const auto checkForParentChildLoops = qScopeGuard([&](){
+        int depth = 0;
+        auto p = parentWidget();
+        while (p) {
+            if (++depth == QObjectPrivate::CheckForParentChildLoopsWarnDepth) {
+                qWarning("QWidget %p (class: '%s', object name: '%s') may have a loop in its parent-child chain; "
+                         "this is undefined behavior",
+                         this, metaObject()->className(), qPrintable(objectName()));
+            }
+            p = p->parentWidget();
+        }
+    });
+#endif
+
     bool resized = testAttribute(Qt::WA_Resized);
     bool wasCreated = testAttribute(Qt::WA_WState_Created);
     QWidget *oldtlw = window();