Q*Application: fix UB caused by accessing QGuiApplication from QCoreApplication ctor

As reported by ubsan:

  src/gui/kernel/qplatformintegration.cpp:463:10: runtime error: downcast of address 0x7ffdc2942490 which does not point to an object of type 'QGuiApplication'
  0x7ffdc2942490: note: object is of type 'QCoreApplication'

  src/gui/kernel/qplatformintegration.cpp:466:14: runtime error: downcast of address 0x7ffdc2942490 which does not point to an object of type 'QGuiApplication'
  0x7ffdc2942490: note: object is of type 'QCoreApplication'

  src/gui/kernel/qplatformintegration.cpp:466:43: runtime error: member call on address 0x7ffdc2942490 which does not point to an object of type 'QGuiApplication'
  0x7ffdc2942490: note: object is of type 'QCoreApplication'

to name just a few which are reported when running gui and widget
auto-tests; there're definitely more where these came from.

This is caused by QCoreApplication::init() being called from the
QCoreApplication ctor, calling virtual functions on Q*AppPrivate,
which happen to attempt, in this case, to emit QGuiApp signals.
At that point in time, the QGuiApplication ctor has not entered
the constructor body, ergo the object is still a QCoreApplication,
and calling the signal, as a member function on the derived class,
invokes UB.

Fix by cleaning up the wild mix of initialization functions used in
this hierarchy. The cleanup restores the

  1. Q*ApplicationPrivate::Q*ApplicationPrivate()
  2. Q*ApplicationPrivate::init(), calling each base class'
     init() as the first thing

two-stage construction pattern commonly used elsewhere in Qt to make
sure that the public class' object is fully constructed by the time
each level's Private::init() is called.

Change-Id: I290402b3232315d7ed687c97e740bfbdbd3ecd1a
Reviewed-by: Lars Knoll <lars.knoll@theqtcompany.com>

7 files changed, 38 insertions, 32 deletions

diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp
index 6dcd0ed5b4..30a3204d3d 100644
--- a/src/corelib/kernel/qcoreapplication.cpp
+++ b/src/corelib/kernel/qcoreapplication.cpp
@@ -703,7 +703,7 @@ QCoreApplication::QCoreApplication(QCoreApplicationPrivate &p)
     : QObject(p, 0)
 #endif
 {
-    init();
+    d_func()->q_ptr = this;
     // note: it is the subclasses' job to call
     // QCoreApplicationPrivate::eventDispatcher->startingUp();
 }
@@ -752,27 +752,26 @@ QCoreApplication::QCoreApplication(int &argc, char **argv
     : QObject(*new QCoreApplicationPrivate(argc, argv, _internal))
 #endif
 {
-    init();
+    d_func()->q_ptr = this;
+    d_func()->init();
 #ifndef QT_NO_QOBJECT
     QCoreApplicationPrivate::eventDispatcher->startingUp();
 #endif
 }
 
 
-// ### move to QCoreApplicationPrivate constructor?
-void QCoreApplication::init()
+void QCoreApplicationPrivate::init()
 {
-    d_ptr->q_ptr = this;
-    Q_D(QCoreApplication);
+    Q_Q(QCoreApplication);
 
-    QCoreApplicationPrivate::initLocale();
+    initLocale();
 
-    Q_ASSERT_X(!self, "QCoreApplication", "there should be only one application object");
-    QCoreApplication::self = this;
+    Q_ASSERT_X(!QCoreApplication::self, "QCoreApplication", "there should be only one application object");
+    QCoreApplication::self = q;
 
     // Store app name (so it's still available after QCoreApplication is destroyed)
     if (!coreappdata()->applicationNameSet)
-        coreappdata()->application = d_func()->appName();
+        coreappdata()->application = appName();
 
     QLoggingRegistry::instance()->init();
 
@@ -788,7 +787,7 @@ void QCoreApplication::init()
             // anywhere in the list, we can just linearly scan the lists and find the items that
             // have been removed. Once the original list is exhausted we know all the remaining
             // items have been added.
-            QStringList newPaths(libraryPaths());
+            QStringList newPaths(q->libraryPaths());
             for (int i = manualPaths->length(), j = appPaths->length(); i > 0 || j > 0; qt_noop()) {
                 if (--j < 0) {
                     newPaths.prepend((*manualPaths)[--i]);
@@ -808,28 +807,28 @@ void QCoreApplication::init()
 
 #ifndef QT_NO_QOBJECT
     // use the event dispatcher created by the app programmer (if any)
-    if (!QCoreApplicationPrivate::eventDispatcher)
-        QCoreApplicationPrivate::eventDispatcher = d->threadData->eventDispatcher.load();
+    if (!eventDispatcher)
+        eventDispatcher = threadData->eventDispatcher.load();
     // otherwise we create one
-    if (!QCoreApplicationPrivate::eventDispatcher)
-        d->createEventDispatcher();
-    Q_ASSERT(QCoreApplicationPrivate::eventDispatcher != 0);
+    if (!eventDispatcher)
+        createEventDispatcher();
+    Q_ASSERT(eventDispatcher);
 
-    if (!QCoreApplicationPrivate::eventDispatcher->parent()) {
-        QCoreApplicationPrivate::eventDispatcher->moveToThread(d->threadData->thread);
-        QCoreApplicationPrivate::eventDispatcher->setParent(this);
+    if (!eventDispatcher->parent()) {
+        eventDispatcher->moveToThread(threadData->thread);
+        eventDispatcher->setParent(q);
     }
 
-    d->threadData->eventDispatcher = QCoreApplicationPrivate::eventDispatcher;
-    d->eventDispatcherReady();
+    threadData->eventDispatcher = eventDispatcher;
+    eventDispatcherReady();
 #endif
 
 #ifdef QT_EVAL
     extern void qt_core_eval_init(QCoreApplicationPrivate::Type);
-    qt_core_eval_init(d->application_type);
+    qt_core_eval_init(application_type);
 #endif
 
-    d->processCommandLineArguments();
+    processCommandLineArguments();
 
     qt_call_pre_routines();
     qt_startup_hook();
@@ -839,7 +838,7 @@ void QCoreApplication::init()
 #endif
 
 #ifndef QT_NO_QOBJECT
-    QCoreApplicationPrivate::is_app_running = true; // No longer starting up.
+    is_app_running = true; // No longer starting up.
 #endif
 }
 
diff --git a/src/corelib/kernel/qcoreapplication.h b/src/corelib/kernel/qcoreapplication.h
index d865c4e7a8..a008c25c76 100644
--- a/src/corelib/kernel/qcoreapplication.h
+++ b/src/corelib/kernel/qcoreapplication.h
@@ -200,8 +200,6 @@ private:
     static bool notifyInternal2(QObject *receiver, QEvent *);
 #endif
 
-    void init();
-
     static QCoreApplication *self;
 
     Q_DISABLE_COPY(QCoreApplication)
diff --git a/src/corelib/kernel/qcoreapplication_p.h b/src/corelib/kernel/qcoreapplication_p.h
index 9a9e8dd09a..45c34b7df2 100644
--- a/src/corelib/kernel/qcoreapplication_p.h
+++ b/src/corelib/kernel/qcoreapplication_p.h
@@ -74,6 +74,8 @@ public:
     QCoreApplicationPrivate(int &aargc,  char **aargv, uint flags);
     ~QCoreApplicationPrivate();
 
+    void init();
+
     QString appName() const;
 
 #ifdef Q_OS_MAC
diff --git a/src/gui/kernel/qguiapplication.cpp b/src/gui/kernel/qguiapplication.cpp
index 3808bec8a4..385264d70a 100644
--- a/src/gui/kernel/qguiapplication.cpp
+++ b/src/gui/kernel/qguiapplication.cpp
@@ -577,7 +577,7 @@ QGuiApplication::QGuiApplication(int &argc, char **argv, int flags)
 QGuiApplication::QGuiApplication(QGuiApplicationPrivate &p)
     : QCoreApplication(p)
 {
-    d_func()->init(); }
+}
 
 /*!
     Destructs the application.
@@ -1260,6 +1260,8 @@ void QGuiApplicationPrivate::eventDispatcherReady()
 
 void QGuiApplicationPrivate::init()
 {
+    QCoreApplicationPrivate::init();
+
     QCoreApplicationPrivate::is_app_running = false; // Starting up.
 
     bool loadTestability = false;
diff --git a/src/gui/kernel/qguiapplication_p.h b/src/gui/kernel/qguiapplication_p.h
index 9c8b655c71..9e157aad51 100644
--- a/src/gui/kernel/qguiapplication_p.h
+++ b/src/gui/kernel/qguiapplication_p.h
@@ -75,6 +75,8 @@ public:
     QGuiApplicationPrivate(int &argc, char **argv, int flags);
     ~QGuiApplicationPrivate();
 
+    void init();
+
     void createPlatformIntegration();
     void createEventDispatcher() Q_DECL_OVERRIDE;
     void eventDispatcherReady() Q_DECL_OVERRIDE;
@@ -298,8 +300,6 @@ protected:
 private:
     friend class QDragManager;
 
-    void init();
-
     static QGuiApplicationPrivate *self;
     static QTouchDevice *m_fakeTouchDevice;
     static int m_fakeMouseSourcePointId;
diff --git a/src/widgets/kernel/qapplication.cpp b/src/widgets/kernel/qapplication.cpp
index f7d4139ed8..b7de0d7a7e 100644
--- a/src/widgets/kernel/qapplication.cpp
+++ b/src/widgets/kernel/qapplication.cpp
@@ -562,13 +562,18 @@ QApplication::QApplication(int &argc, char **argv)
 QApplication::QApplication(int &argc, char **argv, int _internal)
 #endif
     : QGuiApplication(*new QApplicationPrivate(argc, argv, _internal))
-{ Q_D(QApplication); d->construct(); }
+{
+    Q_D(QApplication);
+    d->init();
+}
 
 /*!
     \internal
 */
-void QApplicationPrivate::construct()
+void QApplicationPrivate::init()
 {
+    QGuiApplicationPrivate::init();
+
     initResources();
 
     qt_is_gui_used = (application_type != QApplicationPrivate::Tty);
diff --git a/src/widgets/kernel/qapplication_p.h b/src/widgets/kernel/qapplication_p.h
index cb158011f0..832d37a329 100644
--- a/src/widgets/kernel/qapplication_p.h
+++ b/src/widgets/kernel/qapplication_p.h
@@ -150,7 +150,7 @@ public:
 
     bool notify_helper(QObject *receiver, QEvent * e);
 
-    void construct(
+    void init(
 #ifdef Q_DEAD_CODE_FROM_QT4_X11
                    Display *dpy = 0, Qt::HANDLE visual = 0, Qt::HANDLE cmap = 0
 #endif