From afe5bcdbd1dbf8c8a229d75d16b614f5e645d32f Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 27 Sep 2016 11:25:10 +0200 Subject: tst_QWidget: Fix UB (invalid member access) in EnterTestMainDialog::eventFilter() Found by UBSan: tst_qwidget.cpp:10207:29: runtime error: member access within address 0x6060000e8880 which does not point to an object of type 'EnterTestModalDialog' 0x6060000e8880: note: object is of type 'QWidget' eb 00 80 45 10 4b 32 ab 11 2b 00 00 80 df 08 00 60 61 00 00 c0 4c 32 ab 11 2b 00 00 00 00 be be ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'QWidget' #0 0x6ca13f in EnterTestMainDialog::eventFilter(QObject*, QEvent*) tst_qwidget.cpp:10207 #1 0x2b11b8bc90c3 in QCoreApplicationPrivate::sendThroughApplicationEventFilters(QObject*, QEvent*) qcoreapplication.cpp:1081 #2 0x2b11a3c49b4a in QApplicationPrivate::notify_helper(QObject*, QEvent*) qapplication.cpp:3716 #3 0x2b11a3c8ec72 in QApplication::notify(QObject*, QEvent*) qapplication.cpp:3704 #4 0x2b11b8bccd0f in QCoreApplication::notifyInternal2(QObject*, QEvent*) qcoreapplication.cpp:988 #5 0x2b11aea5c34d in QCoreApplication::sendEvent(QObject*, QEvent*) qcoreapplication.h:231 #6 0x2b11aea5c34d in QGuiApplicationPrivate::_q_updateFocusObject(QObject*) qguiapplication.cpp:3690 #7 0x2b11aea61360 in QGuiApplication::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) .moc/moc_qguiapplication.cpp:177 #8 0x2b11b8d1dc86 in QMetaObject::activate(QObject*, int, int, void**) qobject.cpp:3787 #9 0x2b11aea784a3 in QWindow::focusObjectChanged(QObject*) .moc/moc_qwindow.cpp:760 #10 0x2b11a3fb24f2 in QWidget::clearFocus() qwidget.cpp:6705 #11 0x2b11a3fc87b1 in QWidget::~QWidget() qwidget.cpp:1608 #12 0x2b11a526688c in QDialog::~QDialog() qdialog.cpp:352 #13 0x6c43e2 in EnterTestModalDialog::~EnterTestModalDialog() tst_qwidget.cpp:10160 #14 0x6c43e2 in EnterTestModalDialog::~EnterTestModalDialog() tst_qwidget.cpp:10160 #15 0x492be3 in EnterTestMainDialog::buttonPressed() tst_qwidget.cpp:10188 #16 0x492be3 in EnterTestMainDialog::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) .moc/tst_qwidget.moc:2056 #17 0x2b11b8d1dc86 in QMetaObject::activate(QObject*, int, int, void**) qobject.cpp:3787 #18 0x2b11a45cb833 in QAbstractButton::clicked(bool) .moc/moc_qabstractbutton.cpp:307 #19 0x2b11a45cd54b in QAbstractButtonPrivate::emitClicked() qabstractbutton.cpp:411 #20 0x2b11a45df73a in QAbstractButtonPrivate::click() qabstractbutton.cpp:404 [...] #41 0x6bb2cf in tst_QWidget::taskQTBUG_27643_enterEvents() tst_qwidget.cpp:10249 [...] Fix by checking the event type first, and accessing modal->button only if it's QEvent::Enter. Change-Id: I2c7df3a1f43ecbfe14741b5861729078a91a32d6 Reviewed-by: Friedemann Kleint --- tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'tests/auto/widgets/kernel') diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index e00e575c36..cc1aaa7d51 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -10430,14 +10430,13 @@ public slots: bool eventFilter(QObject *o, QEvent *e) { - if (modal && modal->button && o == modal->button) { - switch (e->type()) { - case QEvent::Enter: + switch (e->type()) { + case QEvent::Enter: + if (modal && modal->button && o == modal->button) enters++; - break; - default: - break; - } + break; + default: + break; } return QDialog::eventFilter(o, e); } -- cgit v1.2.3 From 686c44a69b13f6e884dd2b6d9991f4cd94597c5a Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Wed, 28 Sep 2016 10:49:57 +0200 Subject: Plug remaining leaks in tests/auto/widgets/kernel The usual: - delete return values of QLayout::takeAt(), replaceWidget() - delete styles - delete top-level widgets - delete actions Either by naked delete, QScopedPointer or allocation on the stack instead of the heap. This fixes the remaining errors in GCC 6.1 Linux ASan runs of tests/auto/widgets/kernel. Change-Id: I8cc217be114b2e0edf34ad8d60dbf722f900bb7f Reviewed-by: Thiago Macieira --- tests/auto/widgets/kernel/qaction/tst_qaction.cpp | 6 +++--- .../auto/widgets/kernel/qapplication/tst_qapplication.cpp | 12 ++++++------ tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp | 15 +++++++++------ 3 files changed, 18 insertions(+), 15 deletions(-) (limited to 'tests/auto/widgets/kernel') diff --git a/tests/auto/widgets/kernel/qaction/tst_qaction.cpp b/tests/auto/widgets/kernel/qaction/tst_qaction.cpp index 71b55d71ea..b496550f85 100644 --- a/tests/auto/widgets/kernel/qaction/tst_qaction.cpp +++ b/tests/auto/widgets/kernel/qaction/tst_qaction.cpp @@ -329,7 +329,7 @@ void tst_QAction::enabledVisibleInteraction() void tst_QAction::task200823_tooltip() { - QAction *action = new QAction("foo", 0); + const QScopedPointer action(new QAction("foo", Q_NULLPTR)); QString shortcut("ctrl+o"); action->setShortcut(shortcut); @@ -343,8 +343,8 @@ void tst_QAction::task200823_tooltip() void tst_QAction::task229128TriggeredSignalWithoutActiongroup() { // test without a group - QAction *actionWithoutGroup = new QAction("Test", qApp); - QSignalSpy spyWithoutGroup(actionWithoutGroup, SIGNAL(triggered(bool))); + const QScopedPointer actionWithoutGroup(new QAction("Test", Q_NULLPTR)); + QSignalSpy spyWithoutGroup(actionWithoutGroup.data(), SIGNAL(triggered(bool))); QCOMPARE(spyWithoutGroup.count(), 0); actionWithoutGroup->trigger(); // signal should be emitted diff --git a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp b/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp index 87a189fc87..424069c8ae 100644 --- a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp +++ b/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp @@ -2211,8 +2211,8 @@ void tst_QApplication::noQuitOnHide() { int argc = 0; QApplication app(argc, 0); - QWidget *window1 = new NoQuitOnHideWidget; - window1->show(); + NoQuitOnHideWidget window1; + window1.show(); QCOMPARE(app.exec(), 1); } @@ -2246,12 +2246,12 @@ void tst_QApplication::abortQuitOnShow() { int argc = 0; QApplication app(argc, 0); - QWidget *window1 = new ShowCloseShowWidget(false); - window1->show(); + ShowCloseShowWidget window1(false); + window1.show(); QCOMPARE(app.exec(), 0); - QWidget *window2 = new ShowCloseShowWidget(true); - window2->show(); + ShowCloseShowWidget window2(true); + window2.show(); QCOMPARE(app.exec(), 1); } diff --git a/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp b/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp index e1b494c9f1..5703d7e114 100644 --- a/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp +++ b/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp @@ -395,7 +395,8 @@ void tst_QFormLayout::setFormStyle() QCOMPARE(layout.rowWrapPolicy(), QFormLayout::DontWrapRows); #endif - widget.setStyle(QStyleFactory::create("windows")); + const QScopedPointer windowsStyle(QStyleFactory::create("windows")); + widget.setStyle(windowsStyle.data()); QCOMPARE(layout.labelAlignment(), Qt::AlignLeft); QVERIFY(layout.formAlignment() == (Qt::AlignLeft | Qt::AlignTop)); @@ -406,14 +407,16 @@ void tst_QFormLayout::setFormStyle() this test is cross platform.. so create dummy styles that return all the right stylehints. */ - widget.setStyle(new DummyMacStyle()); + DummyMacStyle macStyle; + widget.setStyle(&macStyle); QCOMPARE(layout.labelAlignment(), Qt::AlignRight); QVERIFY(layout.formAlignment() == (Qt::AlignHCenter | Qt::AlignTop)); QCOMPARE(layout.fieldGrowthPolicy(), QFormLayout::FieldsStayAtSizeHint); QCOMPARE(layout.rowWrapPolicy(), QFormLayout::DontWrapRows); - widget.setStyle(new DummyQtopiaStyle()); + DummyQtopiaStyle qtopiaStyle; + widget.setStyle(&qtopiaStyle); QCOMPARE(layout.labelAlignment(), Qt::AlignRight); QVERIFY(layout.formAlignment() == (Qt::AlignLeft | Qt::AlignTop)); @@ -891,7 +894,7 @@ void tst_QFormLayout::takeAt() QCOMPARE(layout->count(), 7); for (int i = 6; i >= 0; --i) { - layout->takeAt(0); + delete layout->takeAt(0); QCOMPARE(layout->count(), i); } } @@ -983,7 +986,7 @@ void tst_QFormLayout::replaceWidget() QFormLayout::ItemRole role; // replace editor - layout->replaceWidget(edit1, edit3); + delete layout->replaceWidget(edit1, edit3); edit1->hide(); // Not strictly needed for the test, but for normal usage it is. QCOMPARE(layout->indexOf(edit1), -1); QCOMPARE(layout->indexOf(edit3), editIndex); @@ -994,7 +997,7 @@ void tst_QFormLayout::replaceWidget() QCOMPARE(rownum, 0); QCOMPARE(role, QFormLayout::FieldRole); - layout->replaceWidget(label1, label2); + delete layout->replaceWidget(label1, label2); label1->hide(); QCOMPARE(layout->indexOf(label1), -1); QCOMPARE(layout->indexOf(label2), labelIndex); -- cgit v1.2.3