From 1b441c3941efc56f9b0ead35a4501056a74a77e1 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Mon, 7 Mar 2016 20:26:14 +0100 Subject: 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 --- src/corelib/kernel/qcoreapplication.cpp | 47 ++++++++++++++++----------------- src/corelib/kernel/qcoreapplication.h | 2 -- src/corelib/kernel/qcoreapplication_p.h | 2 ++ 3 files changed, 25 insertions(+), 26 deletions(-) (limited to 'src/corelib') 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 -- cgit v1.2.3