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>
This commit is contained in:
Marc Mutz 2016-03-07 20:26:14 +01:00
parent fb7ef2b9f5
commit 1b441c3941
7 changed files with 38 additions and 32 deletions

View File

@ -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
}

View File

@ -200,8 +200,6 @@ private:
static bool notifyInternal2(QObject *receiver, QEvent *);
#endif
void init();
static QCoreApplication *self;
Q_DISABLE_COPY(QCoreApplication)

View File

@ -74,6 +74,8 @@ public:
QCoreApplicationPrivate(int &aargc, char **aargv, uint flags);
~QCoreApplicationPrivate();
void init();
QString appName() const;
#ifdef Q_OS_MAC

View File

@ -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;

View File

@ -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;

View File

@ -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);

View File

@ -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