From 7447e2b337f12b4d04935d0f30fc673e4327d5a0 Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Mon, 24 Feb 2020 16:23:27 +0100 Subject: [PATCH 01/15] QTextMarkdownImporter: fix use after free; add fuzz-generated tests It was possible to end up with a dangling pointer in m_listStack. This is now avoided by using QPointer and doing nullptr checks before accessing any QTextList pointer stored there. We have 2 specimens of garbage that caused crashes before; now they don't. But only fuzz20450 triggered the dangling pointer in the list stack. The crash caused by fuzz20580 was fixed by updating md4c from upstream: 4b0fc030777cd541604f5ebaaad47a2b76d61ff9 Change-Id: I8e1eca23b281256a03aea0f55e9ae20f1bdd2a38 Reviewed-by: Robert Loehning --- src/gui/text/qtextmarkdownimporter.cpp | 7 ++++-- src/gui/text/qtextmarkdownimporter_p.h | 2 +- .../qtextmarkdownimporter/data/fuzz20450.md | 5 ++++ .../qtextmarkdownimporter/data/fuzz20580.md | 1 + .../qtextmarkdownimporter.pro | 2 ++ .../tst_qtextmarkdownimporter.cpp | 24 +++++++++++++++++++ 6 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20450.md create mode 100644 tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20580.md diff --git a/src/gui/text/qtextmarkdownimporter.cpp b/src/gui/text/qtextmarkdownimporter.cpp index 7e18a108955..5e75e7816b5 100644 --- a/src/gui/text/qtextmarkdownimporter.cpp +++ b/src/gui/text/qtextmarkdownimporter.cpp @@ -577,7 +577,10 @@ void QTextMarkdownImporter::insertBlock() QTextBlockFormat blockFormat; if (!m_listStack.isEmpty() && !m_needsInsertList && m_listItem) { QTextList *list = m_listStack.top(); - blockFormat = list->item(list->count() - 1).blockFormat(); + if (list) + blockFormat = list->item(list->count() - 1).blockFormat(); + else + qWarning() << "attempted to insert into a list that no longer exists"; } if (m_blockQuoteDepth) { blockFormat.setProperty(QTextFormat::BlockQuoteLevel, m_blockQuoteDepth); @@ -607,7 +610,7 @@ void QTextMarkdownImporter::insertBlock() } if (m_needsInsertList) { m_listStack.push(m_cursor->createList(m_listFormat)); - } else if (!m_listStack.isEmpty() && m_listItem) { + } else if (!m_listStack.isEmpty() && m_listItem && m_listStack.top()) { m_listStack.top()->add(m_cursor->block()); } m_needsInsertList = false; diff --git a/src/gui/text/qtextmarkdownimporter_p.h b/src/gui/text/qtextmarkdownimporter_p.h index f450da5eb35..e3b4bcd0f2b 100644 --- a/src/gui/text/qtextmarkdownimporter_p.h +++ b/src/gui/text/qtextmarkdownimporter_p.h @@ -113,7 +113,7 @@ private: #endif QString m_blockCodeLanguage; QVector m_nonEmptyTableCells; // in the current row - QStack m_listStack; + QStack> m_listStack; QStack m_spanFormatStack; QFont m_monoFont; QPalette m_palette; diff --git a/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20450.md b/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20450.md new file mode 100644 index 00000000000..d7005cb01eb --- /dev/null +++ b/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20450.md @@ -0,0 +1,5 @@ +ÿ +* ÿ + + ÿ +* ÿ \ No newline at end of file diff --git a/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20580.md b/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20580.md new file mode 100644 index 00000000000..22006f58768 --- /dev/null +++ b/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20580.md @@ -0,0 +1 @@ +| --:| ("warning"); + QTest::newRow("fuzz20450") << "attempted to insert into a list that no longer exists"; + QTest::newRow("fuzz20580") << ""; +} + +void tst_QTextMarkdownImporter::pathological() // avoid crashing on crazy input +{ + QFETCH(QString, warning); + QString filename = QLatin1String("data/") + QTest::currentDataTag() + QLatin1String(".md"); + QFile f(QFINDTESTDATA(filename)); + QVERIFY(f.open(QFile::ReadOnly)); +#ifdef QT_NO_DEBUG + Q_UNUSED(warning) +#else + if (!warning.isEmpty()) + QTest::ignoreMessage(QtWarningMsg, warning.toLatin1()); +#endif + QTextDocument().setMarkdown(f.readAll()); +} + QTEST_MAIN(tst_QTextMarkdownImporter) #include "tst_qtextmarkdownimporter.moc" From 08d5059320334223ff1f9009342324f25c231f0b Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Mon, 14 Oct 2019 14:54:44 +0200 Subject: [PATCH 02/15] Fix geometry handling for native child windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't move the native child window position for native child windows. Initial-patch-by: BÅ‚ażej SzczygieÅ‚ Task-number: QTBUG-82312 Fixes: QTBUG-79166 Change-Id: I117ef08da13c8e90ff60cf034126c9efdc17b836 Reviewed-by: BÅ‚ażej SzczygieÅ‚ Reviewed-by: Morten Johan Sørvig Reviewed-by: Oliver Wolff --- src/gui/kernel/qwindow.cpp | 10 +++++++--- src/gui/kernel/qwindowsysteminterface.cpp | 13 ++++++++++--- .../platforms/windows/qwindowsintegration.cpp | 4 +++- src/plugins/platforms/xcb/qxcbwindow.cpp | 4 +++- 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/gui/kernel/qwindow.cpp b/src/gui/kernel/qwindow.cpp index 0a4277c118f..9cb851a7a20 100644 --- a/src/gui/kernel/qwindow.cpp +++ b/src/gui/kernel/qwindow.cpp @@ -1666,7 +1666,7 @@ void QWindow::setGeometry(const QRect &rect) if (newScreen && isTopLevel()) nativeRect = QHighDpi::toNativePixels(rect, newScreen); else - nativeRect = QHighDpi::toNativePixels(rect, this); + nativeRect = QHighDpi::toNativeLocalPosition(rect, newScreen); d->platformWindow->setGeometry(nativeRect); } else { d->geometry = rect; @@ -1717,8 +1717,12 @@ QScreen *QWindowPrivate::screenForGeometry(const QRect &newGeometry) const QRect QWindow::geometry() const { Q_D(const QWindow); - if (d->platformWindow) - return QHighDpi::fromNativePixels(d->platformWindow->geometry(), this); + if (d->platformWindow) { + const auto nativeGeometry = d->platformWindow->geometry(); + return isTopLevel() + ? QHighDpi::fromNativePixels(nativeGeometry, this) + : QHighDpi::fromNativeLocalPosition(nativeGeometry, this); + } return d->geometry; } diff --git a/src/gui/kernel/qwindowsysteminterface.cpp b/src/gui/kernel/qwindowsysteminterface.cpp index 8457282bed1..8f2927901a2 100644 --- a/src/gui/kernel/qwindowsysteminterface.cpp +++ b/src/gui/kernel/qwindowsysteminterface.cpp @@ -296,14 +296,21 @@ QWindowSystemInterfacePrivate::GeometryChangeEvent::GeometryChangeEvent(QWindow , window(window) , newGeometry(newGeometry) { - if (const QPlatformWindow *pw = window->handle()) - requestedGeometry = QHighDpi::fromNativePixels(pw->QPlatformWindow::geometry(), window); + if (const QPlatformWindow *pw = window->handle()) { + const auto nativeGeometry = pw->QPlatformWindow::geometry(); + requestedGeometry = window->isTopLevel() + ? QHighDpi::fromNativePixels(nativeGeometry, window) + : QHighDpi::fromNativeLocalPosition(nativeGeometry, window); + } } QT_DEFINE_QPA_EVENT_HANDLER(void, handleGeometryChange, QWindow *window, const QRect &newRect) { Q_ASSERT(window); - QWindowSystemInterfacePrivate::GeometryChangeEvent *e = new QWindowSystemInterfacePrivate::GeometryChangeEvent(window, QHighDpi::fromNativePixels(newRect, window)); + const auto newRectDi = window->isTopLevel() + ? QHighDpi::fromNativePixels(newRect, window) + : QHighDpi::fromNativeLocalPosition(newRect, window); + auto e = new QWindowSystemInterfacePrivate::GeometryChangeEvent(window, newRectDi); if (window->handle()) { // Persist the new geometry so that QWindow::geometry() can be queried in the resize event window->handle()->QPlatformWindow::setGeometry(newRect); diff --git a/src/plugins/platforms/windows/qwindowsintegration.cpp b/src/plugins/platforms/windows/qwindowsintegration.cpp index 09117f663d9..d37b405db8d 100644 --- a/src/plugins/platforms/windows/qwindowsintegration.cpp +++ b/src/plugins/platforms/windows/qwindowsintegration.cpp @@ -334,7 +334,9 @@ QPlatformWindow *QWindowsIntegration::createPlatformWindow(QWindow *window) cons QWindowsWindowData requested; requested.flags = window->flags(); - requested.geometry = QHighDpi::toNativePixels(window->geometry(), window); + requested.geometry = window->isTopLevel() + ? QHighDpi::toNativePixels(window->geometry(), window) + : QHighDpi::toNativeLocalPosition(window->geometry(), window); // Apply custom margins (see QWindowsWindow::setCustomMargins())). const QVariant customMarginsV = window->property("_q_windowsCustomMargins"); if (customMarginsV.isValid()) diff --git a/src/plugins/platforms/xcb/qxcbwindow.cpp b/src/plugins/platforms/xcb/qxcbwindow.cpp index cfe048d5c4d..c9d1cfe50ae 100644 --- a/src/plugins/platforms/xcb/qxcbwindow.cpp +++ b/src/plugins/platforms/xcb/qxcbwindow.cpp @@ -276,7 +276,9 @@ void QXcbWindow::create() QXcbScreen *currentScreen = xcbScreen(); QXcbScreen *platformScreen = parent() ? parentScreen() : initialScreen(); - QRect rect = QHighDpi::toNativePixels(window()->geometry(), platformScreen); + QRect rect = parent() + ? QHighDpi::toNativeLocalPosition(window()->geometry(), platformScreen) + : QHighDpi::toNativePixels(window()->geometry(), platformScreen); if (type == Qt::Desktop) { m_window = platformScreen->root(); From 4a1de178c9cc891560f38d64d89074799b0fa0e1 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Fri, 28 Feb 2020 12:33:35 +0100 Subject: [PATCH 03/15] Try again to make QDeadlineTimer test robust against context switches Instead of comparing to absolute values, compare the result from QDeadlineTimer with the reference clock types from std::chrono. Pass the test as long as we are within 10% of that reference. In addition, handle the case where QTest::qSleep sleeps for more than 10% longer or shorter than what is requested, and if so, abort the test. Change-Id: If8b77aea55a8c5c53e96427b2fff2f78281d0f82 Reviewed-by: Lars Knoll --- .../qdeadlinetimer/tst_qdeadlinetimer.cpp | 50 +++++++++++++------ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/tests/auto/corelib/kernel/qdeadlinetimer/tst_qdeadlinetimer.cpp b/tests/auto/corelib/kernel/qdeadlinetimer/tst_qdeadlinetimer.cpp index 35c5e7cb751..db53c3f20ce 100644 --- a/tests/auto/corelib/kernel/qdeadlinetimer/tst_qdeadlinetimer.cpp +++ b/tests/auto/corelib/kernel/qdeadlinetimer/tst_qdeadlinetimer.cpp @@ -573,12 +573,32 @@ void tst_QDeadlineTimer::stdchrono() QCOMPARE(deadline.remainingTimeAsDuration(), nanoseconds::zero()); + /* + Call QTest::qSleep, and return true if the time actually slept is + within \a deviationPercent percent of the requested sleep time. + Otherwise, return false, in which case the test should to abort. + */ + auto sleepHelper = [](int ms, int deviationPercent = 10) -> bool { + auto before = steady_clock::now(); + QTest::qSleep(ms); + auto after = steady_clock::now(); + auto diff = duration_cast(after - before).count(); + bool inRange = qAbs(diff - ms) < ms * deviationPercent/100.0; + if (!inRange) + qWarning() << "sleeping" << diff << "instead of" << ms << inRange; + return inRange; + }; + auto steady_before = steady_clock::now(); auto system_before = system_clock::now(); - QTest::qSleep(minResolution); + if (!sleepHelper(minResolution)) + QSKIP("Slept too long"); auto now = QDeadlineTimer::current(timerType); - QTest::qSleep(minResolution); + auto steady_reference = steady_clock::now(); + auto system_reference = system_clock::now(); + if (!sleepHelper(minResolution)) + QSKIP("Slept too long"); auto sampling_start = steady_clock::now(); auto steady_deadline = now.deadline(); @@ -599,35 +619,33 @@ void tst_QDeadlineTimer::stdchrono() } { - auto diff = duration_cast(steady_after - steady_deadline); - QVERIFY2(diff.count() > minResolution/2, QByteArray::number(qint64(diff.count()))); - QVERIFY2(diff.count() < 3*minResolution/2, QByteArray::number(qint64(diff.count()))); + auto reference = duration_cast(steady_after - steady_reference).count(); + auto diff = duration_cast(steady_after - steady_deadline).count(); + QVERIFY2(diff > reference * 0.9 && diff < reference*1.1, QByteArray::number(qint64(diff))); QDeadlineTimer dt_after(steady_after, timerType); QVERIFY2(now < dt_after, ("now = " + QLocale().toString(now.deadlineNSecs()) + "; after = " + QLocale().toString(dt_after.deadlineNSecs())).toLatin1()); - diff = duration_cast(steady_deadline - steady_before); - QVERIFY2(diff.count() > minResolution/2, QByteArray::number(qint64(diff.count()))); - QVERIFY2(diff.count() < 3*minResolution/2, QByteArray::number(qint64(diff.count()))); + reference = duration_cast(steady_reference - steady_before).count(); + diff = duration_cast(steady_deadline - steady_before).count(); + QVERIFY2(diff > reference * 0.9 && diff < reference*1.1, QByteArray::number(qint64(diff))); QDeadlineTimer dt_before(steady_before, timerType); QVERIFY2(now > dt_before, ("now = " + QLocale().toString(now.deadlineNSecs()) + "; before = " + QLocale().toString(dt_before.deadlineNSecs())).toLatin1()); } { - auto diff = duration_cast(system_after - system_deadline); - QVERIFY2(diff.count() > minResolution/2, QByteArray::number(qint64(diff.count()))); - QVERIFY2(diff.count() < 3*minResolution/2, QByteArray::number(qint64(diff.count()))); - QDeadlineTimer dt_after(system_after, timerType); + auto reference = duration_cast(system_after - system_reference).count(); + auto diff = duration_cast(system_after - system_deadline).count(); + QVERIFY2(diff > reference * 0.9 && diff < reference*1.1, QByteArray::number(qint64(diff))); QDeadlineTimer dt_after(system_after, timerType); QVERIFY2(now < dt_after, ("now = " + QLocale().toString(now.deadlineNSecs()) + "; after = " + QLocale().toString(dt_after.deadlineNSecs())).toLatin1()); - diff = duration_cast(system_deadline - system_before); - QVERIFY2(diff.count() > minResolution/2, QByteArray::number(qint64(diff.count()))); - QVERIFY2(diff.count() < 3*minResolution/2, QByteArray::number(qint64(diff.count()))); - QDeadlineTimer dt_before(system_before, timerType); + reference = duration_cast(system_reference - system_before).count(); + diff = duration_cast(steady_deadline - steady_before).count(); + QVERIFY2(diff > reference * 0.9 && diff < reference*1.1, QByteArray::number(qint64(diff))); QDeadlineTimer dt_before(system_before, timerType); QVERIFY2(now > dt_before, ("now = " + QLocale().toString(now.deadlineNSecs()) + "; before = " + QLocale().toString(dt_before.deadlineNSecs())).toLatin1()); From 3f7d087d3394a0cd80042f50edd9abcc53febe18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Fri, 28 Feb 2020 10:42:47 +0100 Subject: [PATCH 04/15] uic: Add pass to empty functions When there are no translations on the UI file, the function was left empty but without a 'pass' statement, generating a SyntaxError, this change includes it to avoid problems while using the generated Python file. Fixes: PYSIDE-1234 Change-Id: I30482a95c95fb4b4f4456531946a79c960d76318 Reviewed-by: Friedemann Kleint --- src/tools/uic/cpp/cppwriteinitialization.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/tools/uic/cpp/cppwriteinitialization.cpp b/src/tools/uic/cpp/cppwriteinitialization.cpp index 1212c410ff0..92fd6471a8e 100644 --- a/src/tools/uic/cpp/cppwriteinitialization.cpp +++ b/src/tools/uic/cpp/cppwriteinitialization.cpp @@ -546,12 +546,19 @@ void WriteInitialization::acceptUI(DomUI *node) m_output << m_option.indent << language::endFunctionDefinition("setupUi"); - if (!m_mainFormUsedInRetranslateUi && language::language() == Language::Cpp) { - // Mark varName as unused to avoid compiler warnings. - m_refreshInitialization += m_indent; - m_refreshInitialization += QLatin1String("(void)"); - m_refreshInitialization += varName ; - m_refreshInitialization += language::eol; + if (!m_mainFormUsedInRetranslateUi) { + if (language::language() == Language::Cpp) { + // Mark varName as unused to avoid compiler warnings. + m_refreshInitialization += m_indent; + m_refreshInitialization += QLatin1String("(void)"); + m_refreshInitialization += varName ; + m_refreshInitialization += language::eol; + } else if (language::language() == Language::Python) { + // output a 'pass' to have an empty function + m_refreshInitialization += m_indent; + m_refreshInitialization += QLatin1String("pass"); + m_refreshInitialization += language::eol; + } } m_output << m_option.indent From 4f370d36ec5caa02f78545ceee5704d94edf0530 Mon Sep 17 00:00:00 2001 From: JiDe Zhang Date: Sat, 1 Jun 2019 16:56:49 +0800 Subject: [PATCH 05/15] xcb: Fix logic for minimized state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When _NET_WM_STATE_HIDDEN is not contains in the _NET_WM_STATE window property, the window should not be considered to be minimized According to https://specifications.freedesktop.org/wm-spec/1.3/ar01s05.html _NET_WM_STATE_HIDDEN should be set by the Window Manager to indicate that a window would not be visible on the screen if its desktop/viewport were active and its coordinates were within the screen bounds. The canonical example is that minimized windows should be in the _NET_WM_STATE_HIDDEN state. Pagers and similar applications should use _NET_WM_STATE_HIDDEN instead of WM_STATE to decide whether to display a window in miniature representations of the windows on a desktop. For mutter/GNOME Shell, without _NET_WM_STATE_HIDDEN, window manager will not reply XCB_ICCCM_WM_STATE_ICONIC settings in WM_CHANGE_STATE client message. Task-number: QTBUG-76147 Task-number: QTBUG-76354 Task-number: QTBUG-68864 Done-With: Liang Qi Change-Id: Ic9d26d963979b7f0ef4d1cf322c54ef8c40fa004 Reviewed-by: Uli Schlachter Reviewed-by: Tor Arne Vestbø --- src/plugins/platforms/xcb/qxcbatom.cpp | 1 + src/plugins/platforms/xcb/qxcbatom.h | 1 + src/plugins/platforms/xcb/qxcbwindow.cpp | 17 +++++++++++++++-- src/plugins/platforms/xcb/qxcbwindow.h | 3 ++- .../widgets/kernel/qwidget_window/BLACKLIST | 4 ---- 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/plugins/platforms/xcb/qxcbatom.cpp b/src/plugins/platforms/xcb/qxcbatom.cpp index ecb73cb90b2..d366564dd60 100644 --- a/src/plugins/platforms/xcb/qxcbatom.cpp +++ b/src/plugins/platforms/xcb/qxcbatom.cpp @@ -122,6 +122,7 @@ static const char *xcb_atomnames = { "_NET_WM_STATE_MODAL\0" "_NET_WM_STATE_STAYS_ON_TOP\0" "_NET_WM_STATE_DEMANDS_ATTENTION\0" + "_NET_WM_STATE_HIDDEN\0" "_NET_WM_USER_TIME\0" "_NET_WM_USER_TIME_WINDOW\0" diff --git a/src/plugins/platforms/xcb/qxcbatom.h b/src/plugins/platforms/xcb/qxcbatom.h index 233d2eadb79..80b5887395a 100644 --- a/src/plugins/platforms/xcb/qxcbatom.h +++ b/src/plugins/platforms/xcb/qxcbatom.h @@ -123,6 +123,7 @@ public: _NET_WM_STATE_MODAL, _NET_WM_STATE_STAYS_ON_TOP, _NET_WM_STATE_DEMANDS_ATTENTION, + _NET_WM_STATE_HIDDEN, _NET_WM_USER_TIME, _NET_WM_USER_TIME_WINDOW, diff --git a/src/plugins/platforms/xcb/qxcbwindow.cpp b/src/plugins/platforms/xcb/qxcbwindow.cpp index c9d1cfe50ae..a5974ec36e2 100644 --- a/src/plugins/platforms/xcb/qxcbwindow.cpp +++ b/src/plugins/platforms/xcb/qxcbwindow.cpp @@ -907,6 +907,8 @@ QXcbWindow::NetWmStates QXcbWindow::netWmStates() result |= NetWmStateStaysOnTop; if (statesEnd != std::find(states, statesEnd, atom(QXcbAtom::_NET_WM_STATE_DEMANDS_ATTENTION))) result |= NetWmStateDemandsAttention; + if (statesEnd != std::find(states, statesEnd, atom(QXcbAtom::_NET_WM_STATE_HIDDEN))) + result |= NetWmStateHidden; } else { qCDebug(lcQpaXcb, "getting net wm state (%x), empty\n", m_window); } @@ -1078,6 +1080,9 @@ void QXcbWindow::setNetWmStateOnUnmappedWindow() states |= NetWmStateBelow; } + if (window()->windowStates() & Qt::WindowMinimized) + states |= NetWmStateHidden; + if (window()->windowStates() & Qt::WindowFullScreen) states |= NetWmStateFullScreen; @@ -1111,6 +1116,8 @@ void QXcbWindow::setNetWmStateOnUnmappedWindow() atoms.push_back(atom(QXcbAtom::_NET_WM_STATE_ABOVE)); if (states & NetWmStateBelow && !atoms.contains(atom(QXcbAtom::_NET_WM_STATE_BELOW))) atoms.push_back(atom(QXcbAtom::_NET_WM_STATE_BELOW)); + if (states & NetWmStateHidden && !atoms.contains(atom(QXcbAtom::_NET_WM_STATE_HIDDEN))) + atoms.push_back(atom(QXcbAtom::_NET_WM_STATE_HIDDEN)); if (states & NetWmStateFullScreen && !atoms.contains(atom(QXcbAtom::_NET_WM_STATE_FULLSCREEN))) atoms.push_back(atom(QXcbAtom::_NET_WM_STATE_FULLSCREEN)); if (states & NetWmStateMaximizedHorz && !atoms.contains(atom(QXcbAtom::_NET_WM_STATE_MAXIMIZED_HORZ))) @@ -2219,10 +2226,16 @@ void QXcbWindow::handlePropertyNotifyEvent(const xcb_property_notify_event_t *ev || (data[0] == XCB_ICCCM_WM_STATE_WITHDRAWN && m_minimized)); } } - if (m_minimized) - newState = Qt::WindowMinimized; const NetWmStates states = netWmStates(); + // _NET_WM_STATE_HIDDEN should be set by the Window Manager to indicate that a window would + // not be visible on the screen if its desktop/viewport were active and its coordinates were + // within the screen bounds. The canonical example is that minimized windows should be in + // the _NET_WM_STATE_HIDDEN state. + if (m_minimized && (!connection()->wmSupport()->isSupportedByWM(NetWmStateHidden) + || states.testFlag(NetWmStateHidden))) + newState = Qt::WindowMinimized; + if (states & NetWmStateFullScreen) newState |= Qt::WindowFullScreen; if ((states & NetWmStateMaximizedHorz) && (states & NetWmStateMaximizedVert)) diff --git a/src/plugins/platforms/xcb/qxcbwindow.h b/src/plugins/platforms/xcb/qxcbwindow.h index a808437c5aa..b84250d109a 100644 --- a/src/plugins/platforms/xcb/qxcbwindow.h +++ b/src/plugins/platforms/xcb/qxcbwindow.h @@ -68,7 +68,8 @@ public: NetWmStateMaximizedVert = 0x10, NetWmStateModal = 0x20, NetWmStateStaysOnTop = 0x40, - NetWmStateDemandsAttention = 0x80 + NetWmStateDemandsAttention = 0x80, + NetWmStateHidden = 0x100 }; Q_DECLARE_FLAGS(NetWmStates, NetWmState) diff --git a/tests/auto/widgets/kernel/qwidget_window/BLACKLIST b/tests/auto/widgets/kernel/qwidget_window/BLACKLIST index 39d7b695f6c..d3bfaba4330 100644 --- a/tests/auto/widgets/kernel/qwidget_window/BLACKLIST +++ b/tests/auto/widgets/kernel/qwidget_window/BLACKLIST @@ -2,7 +2,3 @@ # QTBUG-66345 opensuse-42.3 ubuntu-16.04 -[setWindowState] -ubuntu-18.04 -rhel-7.6 - From 8c883c8da32faf1245d257f1fc1fb39fb2b63efc Mon Sep 17 00:00:00 2001 From: Christian Ehrlicher Date: Wed, 19 Feb 2020 18:48:32 +0100 Subject: [PATCH 06/15] QDom: use correct precision when converting float/double values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit d7cb21ac085117f879a8aa1d7727b2ca52d3353d change the way a double is converted which resulted in less precision. Fix it by explictily setting the precision in QString::setNum() Task-number: QTBUG-80068 Change-Id: I1fd9d00837155ceb707e84bfeb9deff03b5ab57e Reviewed-by: André Hartmann Reviewed-by: Konstantin Shegunov Reviewed-by: Lars Knoll --- src/xml/dom/qdom.cpp | 7 ++++--- tests/auto/xml/dom/qdom/tst_qdom.cpp | 24 +++++++++++++++++------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/xml/dom/qdom.cpp b/src/xml/dom/qdom.cpp index 04151c3f31d..04331527f9f 100644 --- a/src/xml/dom/qdom.cpp +++ b/src/xml/dom/qdom.cpp @@ -62,6 +62,7 @@ #include #include #include +#include QT_BEGIN_NAMESPACE @@ -4866,7 +4867,7 @@ void QDomElement::setAttribute(const QString& name, float value) if (!impl) return; QString x; - x.setNum(value); + x.setNum(value, 'g', 8); IMPL->setAttribute(name, x); } @@ -4880,7 +4881,7 @@ void QDomElement::setAttribute(const QString& name, double value) if (!impl) return; QString x; - x.setNum(value); + x.setNum(value, 'g', 17); IMPL->setAttribute(name, x); } @@ -5049,7 +5050,7 @@ void QDomElement::setAttributeNS(const QString nsURI, const QString& qName, doub if (!impl) return; QString x; - x.setNum(value); + x.setNum(value, 'g', 17); IMPL->setAttributeNS(nsURI, qName, x); } diff --git a/tests/auto/xml/dom/qdom/tst_qdom.cpp b/tests/auto/xml/dom/qdom/tst_qdom.cpp index 99639df5b01..229d2ba6e5d 100644 --- a/tests/auto/xml/dom/qdom/tst_qdom.cpp +++ b/tests/auto/xml/dom/qdom/tst_qdom.cpp @@ -38,6 +38,7 @@ #include #include #include +#include QT_FORWARD_DECLARE_CLASS(QDomDocument) QT_FORWARD_DECLARE_CLASS(QDomNode) @@ -408,7 +409,9 @@ void tst_QDom::setGetAttributes() const int intVal = std::numeric_limits::min(); const uint uintVal = std::numeric_limits::max(); const float floatVal = 0.1234f; - const double doubleVal = 0.1234; + const double doubleVal1 = 1./6.; + const double doubleVal2 = std::nextafter(doubleVal1, 1.); + const double doubleVal3 = std::nextafter(doubleVal2, 1.); rootNode.setAttribute("qstringVal", qstringVal); rootNode.setAttribute("qlonglongVal", qlonglongVal); @@ -416,7 +419,9 @@ void tst_QDom::setGetAttributes() rootNode.setAttribute("intVal", intVal); rootNode.setAttribute("uintVal", uintVal); rootNode.setAttribute("floatVal", floatVal); - rootNode.setAttribute("doubleVal", doubleVal); + rootNode.setAttribute("doubleVal1", doubleVal1); + rootNode.setAttribute("doubleVal2", doubleVal2); + rootNode.setAttribute("doubleVal3", doubleVal3); QDomElement nsNode = doc.createElement("NS"); rootNode.appendChild(nsNode); @@ -426,7 +431,9 @@ void tst_QDom::setGetAttributes() nsNode.setAttributeNS("namespace", "intVal", intVal); nsNode.setAttributeNS("namespace", "uintVal", uintVal); nsNode.setAttributeNS("namespace", "floatVal", floatVal); // not available atm - nsNode.setAttributeNS("namespace", "doubleVal", doubleVal); + nsNode.setAttributeNS("namespace", "doubleVal1", doubleVal1); + nsNode.setAttributeNS("namespace", "doubleVal2", doubleVal2); + nsNode.setAttributeNS("namespace", "doubleVal3", doubleVal3); bool bOk; QCOMPARE(rootNode.attribute("qstringVal"), qstringVal); @@ -440,8 +447,10 @@ void tst_QDom::setGetAttributes() QVERIFY(bOk); QCOMPARE(rootNode.attribute("floatVal").toFloat(&bOk), floatVal); QVERIFY(bOk); - QCOMPARE(rootNode.attribute("doubleVal").toDouble(&bOk), doubleVal); - QVERIFY(bOk); + + QVERIFY(rootNode.attribute("doubleVal1").toDouble(&bOk) == doubleVal1 && bOk); + QVERIFY(rootNode.attribute("doubleVal2").toDouble(&bOk) == doubleVal2 && bOk); + QVERIFY(rootNode.attribute("doubleVal3").toDouble(&bOk) == doubleVal3 && bOk); QCOMPARE(nsNode.attributeNS("namespace", "qstringVal"), qstringVal); QCOMPARE(nsNode.attributeNS("namespace", "qlonglongVal").toLongLong(&bOk), qlonglongVal); @@ -454,8 +463,9 @@ void tst_QDom::setGetAttributes() QVERIFY(bOk); QCOMPARE(nsNode.attributeNS("namespace", "floatVal").toFloat(&bOk), floatVal); QVERIFY(bOk); - QCOMPARE(nsNode.attributeNS("namespace", "doubleVal").toDouble(&bOk), doubleVal); - QVERIFY(bOk); + QVERIFY(nsNode.attributeNS("namespace", "doubleVal1").toDouble(&bOk) == doubleVal1 && bOk); + QVERIFY(nsNode.attributeNS("namespace", "doubleVal2").toDouble(&bOk) == doubleVal2 && bOk); + QVERIFY(nsNode.attributeNS("namespace", "doubleVal3").toDouble(&bOk) == doubleVal3 && bOk); QLocale::setDefault(oldLocale); } From d9ca61bf0f6e544ad1fe2908e84c4a4966e5d0a1 Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Mon, 2 Mar 2020 17:40:11 +0100 Subject: [PATCH 07/15] Add the include for QPointer to avoid MSVC compilation error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It has been reported that 7447e2b337f12b4d04935d0f30fc673e4327d5a0 needs amending to build on MSVC 2019, although it was OK in CI. Change-Id: Id22c2a3608529abebd66c0e8f401bc6f26f45e18 Reviewed-by: MÃ¥rten Nordheim --- src/gui/text/qtextmarkdownimporter_p.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gui/text/qtextmarkdownimporter_p.h b/src/gui/text/qtextmarkdownimporter_p.h index e3b4bcd0f2b..f12b725d8e7 100644 --- a/src/gui/text/qtextmarkdownimporter_p.h +++ b/src/gui/text/qtextmarkdownimporter_p.h @@ -56,6 +56,7 @@ #include #include #include +#include #include QT_BEGIN_NAMESPACE From f6c1cebe42193a62fa0b9c6a881bb1a973b1b8a9 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 18 Dec 2019 18:17:38 -0800 Subject: [PATCH 08/15] QPluginLoader: rework the loading and the caching of instance There was a race condition in accessing the cached instance factory member, so rework loadPlugin() to return the cached or newly discovered instance, with proper, atomic caching. Because I had to change that, I took the opportunity to fix the QFactoryLoader code that calls loadPlugin(). Note that QLibraryPrivate::loadPlugin() returns non-nullptr now if the instance is known, which means the last return in QPluginLoader::load() will convert to true, not false, if the instance got cached between the earlier check and the call to loadPlugin(). That's probably what was intended. Task-number: QTBUG-39642 Change-Id: I46bf1f65e8db46afbde5fffd15e1a42d2b6cbf2c Reviewed-by: David Faure --- src/corelib/plugin/qfactoryloader.cpp | 15 +++------ src/corelib/plugin/qlibrary.cpp | 45 +++++++++++++++++++++------ src/corelib/plugin/qlibrary_p.h | 6 ++-- 3 files changed, 44 insertions(+), 22 deletions(-) diff --git a/src/corelib/plugin/qfactoryloader.cpp b/src/corelib/plugin/qfactoryloader.cpp index 18f10c9b43a..9a6a69676d7 100644 --- a/src/corelib/plugin/qfactoryloader.cpp +++ b/src/corelib/plugin/qfactoryloader.cpp @@ -389,17 +389,12 @@ QObject *QFactoryLoader::instance(int index) const QMutexLocker lock(&d->mutex); if (index < d->libraryList.size()) { QLibraryPrivate *library = d->libraryList.at(index); - if (library->instance || library->loadPlugin()) { - if (!library->inst) - library->inst = library->instance(); - QObject *obj = library->inst.data(); - if (obj) { - if (!obj->parent()) - obj->moveToThread(QCoreApplicationPrivate::mainThread()); - return obj; - } + if (QObject *obj = library->pluginInstance()) { + if (!obj->parent()) + obj->moveToThread(QCoreApplicationPrivate::mainThread()); + return obj; } - return 0; + return nullptr; } index -= d->libraryList.size(); lock.unlock(); diff --git a/src/corelib/plugin/qlibrary.cpp b/src/corelib/plugin/qlibrary.cpp index eeaa3c18ecc..7ce959cc8fb 100644 --- a/src/corelib/plugin/qlibrary.cpp +++ b/src/corelib/plugin/qlibrary.cpp @@ -498,7 +498,7 @@ inline void QLibraryStore::releaseLibrary(QLibraryPrivate *lib) } QLibraryPrivate::QLibraryPrivate(const QString &canonicalFileName, const QString &version, QLibrary::LoadHints loadHints) - : pHnd(0), fileName(canonicalFileName), fullVersion(version), instance(0), + : pHnd(0), fileName(canonicalFileName), fullVersion(version), libraryRefCount(0), libraryUnloadCount(0), pluginState(MightBeAPlugin) { loadHintsInt.storeRelaxed(loadHints); @@ -539,6 +539,32 @@ void QLibraryPrivate::setLoadHints(QLibrary::LoadHints lh) mergeLoadHints(lh); } +QObject *QLibraryPrivate::pluginInstance() +{ + // first, check if the instance is cached and hasn't been deleted + QObject *obj = inst.data(); + if (obj) + return obj; + + // We need to call the plugin's factory function. Is that cached? + // skip increasing the reference count (why? -Thiago) + QtPluginInstanceFunction factory = instanceFactory.loadAcquire(); + if (!factory) + factory = loadPlugin(); + + if (!factory) + return nullptr; + + obj = factory(); + + // cache again + if (inst) + obj = inst; + else + inst = obj; + return obj; +} + bool QLibraryPrivate::load() { if (pHnd) { @@ -585,7 +611,7 @@ bool QLibraryPrivate::unload(UnloadFlag flag) //can get deleted libraryRefCount.deref(); pHnd = 0; - instance = 0; + instanceFactory.storeRelaxed(nullptr); } } @@ -597,22 +623,23 @@ void QLibraryPrivate::release() QLibraryStore::releaseLibrary(this); } -bool QLibraryPrivate::loadPlugin() +QtPluginInstanceFunction QLibraryPrivate::loadPlugin() { - if (instance) { + if (auto ptr = instanceFactory.loadAcquire()) { libraryUnloadCount.ref(); - return true; + return ptr; } if (pluginState == IsNotAPlugin) - return false; + return nullptr; if (load()) { - instance = (QtPluginInstanceFunction)resolve("qt_plugin_instance"); - return instance; + auto ptr = reinterpret_cast(resolve("qt_plugin_instance")); + instanceFactory.storeRelease(ptr); // two threads may store the same value + return ptr; } if (qt_debug_component()) qWarning() << "QLibraryPrivate::loadPlugin failed on" << fileName << ":" << errorString; pluginState = IsNotAPlugin; - return false; + return nullptr; } /*! diff --git a/src/corelib/plugin/qlibrary_p.h b/src/corelib/plugin/qlibrary_p.h index db5afac98e9..29724655a39 100644 --- a/src/corelib/plugin/qlibrary_p.h +++ b/src/corelib/plugin/qlibrary_p.h @@ -86,7 +86,7 @@ public: QString fullVersion; bool load(); - bool loadPlugin(); // loads and resolves instance + QtPluginInstanceFunction loadPlugin(); // loads and resolves instance bool unload(UnloadFlag flag = UnloadSys); void release(); QFunctionPointer resolve(const char *); @@ -100,8 +100,8 @@ public: static QStringList suffixes_sys(const QString &fullVersion); static QStringList prefixes_sys(); - QPointer inst; - QtPluginInstanceFunction instance; + QPointer inst; // used by QFactoryLoader + QAtomicPointer::type> instanceFactory; QJsonObject metaData; QString errorString; From ef92ac5636c2f0407fba8141c35ea61d391fd479 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 18 Dec 2019 18:31:55 -0800 Subject: [PATCH 09/15] QLibrary: stop setting errorString after resolve() resolve() is technically thread-safe if the library has been loadaed. We don't promise that, but it's there. More importantly, because QLibraryPrivate is shared among QPluginLoader and QLibrary that point to the same file, we can't thread-safely set the error string. [ChangeLog][Important Behavior Changes] QLibrary::resolve() will no longer set or clear the error string based on the success of finding the symbol. The error string will reflect the result of loading the library. Change-Id: I46bf1f65e8db46afbde5fffd15e1a4f4c2713c17 Reviewed-by: David Faure --- src/corelib/plugin/qlibrary_unix.cpp | 6 ------ src/corelib/plugin/qlibrary_win.cpp | 6 ------ tests/auto/corelib/plugin/qlibrary/tst_qlibrary.cpp | 2 +- 3 files changed, 1 insertion(+), 13 deletions(-) diff --git a/src/corelib/plugin/qlibrary_unix.cpp b/src/corelib/plugin/qlibrary_unix.cpp index 135b82cd378..23f96dbdaff 100644 --- a/src/corelib/plugin/qlibrary_unix.cpp +++ b/src/corelib/plugin/qlibrary_unix.cpp @@ -317,12 +317,6 @@ Q_CORE_EXPORT QFunctionPointer qt_mac_resolve_sys(void *handle, const char *symb QFunctionPointer QLibraryPrivate::resolve_sys(const char* symbol) { QFunctionPointer address = QFunctionPointer(dlsym(pHnd, symbol)); - if (!address) { - errorString = QLibrary::tr("Cannot resolve symbol \"%1\" in %2: %3").arg( - QString::fromLatin1(symbol), fileName, qdlerror()); - } else { - errorString.clear(); - } return address; } diff --git a/src/corelib/plugin/qlibrary_win.cpp b/src/corelib/plugin/qlibrary_win.cpp index 05a93d467e7..c8309104aa2 100644 --- a/src/corelib/plugin/qlibrary_win.cpp +++ b/src/corelib/plugin/qlibrary_win.cpp @@ -162,12 +162,6 @@ bool QLibraryPrivate::unload_sys() QFunctionPointer QLibraryPrivate::resolve_sys(const char* symbol) { FARPROC address = GetProcAddress(pHnd, symbol); - if (!address) { - errorString = QLibrary::tr("Cannot resolve symbol \"%1\" in %2: %3").arg( - QString::fromLatin1(symbol), QDir::toNativeSeparators(fileName), qt_error_string()); - } else { - errorString.clear(); - } return QFunctionPointer(address); } QT_END_NAMESPACE diff --git a/tests/auto/corelib/plugin/qlibrary/tst_qlibrary.cpp b/tests/auto/corelib/plugin/qlibrary/tst_qlibrary.cpp index e0d09b08132..50c4d9b4677 100644 --- a/tests/auto/corelib/plugin/qlibrary/tst_qlibrary.cpp +++ b/tests/auto/corelib/plugin/qlibrary/tst_qlibrary.cpp @@ -368,7 +368,7 @@ void tst_QLibrary::errorString_data() QTest::newRow("bad load()") << (int)Load << QString("nosuchlib") << false << QString("Cannot load library nosuchlib: .*"); QTest::newRow("call errorString() on QLibrary with no d-pointer (crashtest)") << (int)(Load | DontSetFileName) << QString() << false << QString("Unknown error"); - QTest::newRow("bad resolve") << (int)Resolve << appDir + "/mylib" << false << QString("Cannot resolve symbol \"nosuchsymbol\" in \\S+: .*"); + QTest::newRow("bad resolve") << (int)Resolve << appDir + "/mylib" << false << QString("Unknown error"); QTest::newRow("good resolve") << (int)Resolve << appDir + "/mylib" << true << QString("Unknown error"); #ifdef Q_OS_WIN From ae6f73e8566fa76470937aca737141183929a5ec Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 18 Dec 2019 18:45:36 -0800 Subject: [PATCH 10/15] QLibrary: introduce a mutex to protect non-atomic internals And make pHnd atomic. The majority of the variables is updated in QLibraryPrivate::load_sys and updatePluginState(), which get the mutex protection. QLibraryPrivate::unload_sys() doesn't need a mutex protection because we have the refcounting. [ChangeLog][QtCore][QLibrary & QPluginLoader] Fixed a number of race conditions caused by having two QLibrary objects pointing to the same library being operated in different threads. Fixes: QTBUG-39642 Change-Id: I46bf1f65e8db46afbde5fffd15e1a5b3f5e74ea4 Reviewed-by: Lars Knoll --- src/corelib/plugin/qlibrary.cpp | 42 ++++++++++++++++++---------- src/corelib/plugin/qlibrary_p.h | 22 ++++++++------- src/corelib/plugin/qlibrary_unix.cpp | 24 ++++++++-------- src/corelib/plugin/qlibrary_win.cpp | 24 ++++++++-------- src/corelib/plugin/qpluginloader.cpp | 8 ++---- 5 files changed, 68 insertions(+), 52 deletions(-) diff --git a/src/corelib/plugin/qlibrary.cpp b/src/corelib/plugin/qlibrary.cpp index 7ce959cc8fb..ddb053c26fa 100644 --- a/src/corelib/plugin/qlibrary.cpp +++ b/src/corelib/plugin/qlibrary.cpp @@ -407,7 +407,7 @@ inline void QLibraryStore::cleanup() QLibraryPrivate *lib = it.value(); if (lib->libraryRefCount.loadRelaxed() == 1) { if (lib->libraryUnloadCount.loadRelaxed() > 0) { - Q_ASSERT(lib->pHnd); + Q_ASSERT(lib->pHnd.loadRelaxed()); lib->libraryUnloadCount.storeRelaxed(1); #ifdef __GLIBC__ // glibc has a bug in unloading from global destructors @@ -498,8 +498,7 @@ inline void QLibraryStore::releaseLibrary(QLibraryPrivate *lib) } QLibraryPrivate::QLibraryPrivate(const QString &canonicalFileName, const QString &version, QLibrary::LoadHints loadHints) - : pHnd(0), fileName(canonicalFileName), fullVersion(version), - libraryRefCount(0), libraryUnloadCount(0), pluginState(MightBeAPlugin) + : fileName(canonicalFileName), fullVersion(version), pluginState(MightBeAPlugin) { loadHintsInt.storeRelaxed(loadHints); if (canonicalFileName.isEmpty()) @@ -519,7 +518,7 @@ QLibraryPrivate::~QLibraryPrivate() void QLibraryPrivate::mergeLoadHints(QLibrary::LoadHints lh) { // if the library is already loaded, we can't change the load hints - if (pHnd) + if (pHnd.loadRelaxed()) return; loadHintsInt.storeRelaxed(lh); @@ -527,7 +526,7 @@ void QLibraryPrivate::mergeLoadHints(QLibrary::LoadHints lh) QFunctionPointer QLibraryPrivate::resolve(const char *symbol) { - if (!pHnd) + if (!pHnd.loadRelaxed()) return 0; return resolve_sys(symbol); } @@ -542,7 +541,7 @@ void QLibraryPrivate::setLoadHints(QLibrary::LoadHints lh) QObject *QLibraryPrivate::pluginInstance() { // first, check if the instance is cached and hasn't been deleted - QObject *obj = inst.data(); + QObject *obj = (QMutexLocker(&mutex), inst.data()); if (obj) return obj; @@ -558,6 +557,7 @@ QObject *QLibraryPrivate::pluginInstance() obj = factory(); // cache again + QMutexLocker locker(&mutex); if (inst) obj = inst; else @@ -567,7 +567,7 @@ QObject *QLibraryPrivate::pluginInstance() bool QLibraryPrivate::load() { - if (pHnd) { + if (pHnd.loadRelaxed()) { libraryUnloadCount.ref(); return true; } @@ -576,7 +576,9 @@ bool QLibraryPrivate::load() Q_TRACE(QLibraryPrivate_load_entry, fileName); + mutex.lock(); bool ret = load_sys(); + mutex.unlock(); if (qt_debug_component()) { if (ret) { qDebug() << "loaded library" << fileName; @@ -599,9 +601,10 @@ bool QLibraryPrivate::load() bool QLibraryPrivate::unload(UnloadFlag flag) { - if (!pHnd) + if (!pHnd.loadRelaxed()) return false; if (libraryUnloadCount.loadRelaxed() > 0 && !libraryUnloadCount.deref()) { // only unload if ALL QLibrary instance wanted to + QMutexLocker locker(&mutex); delete inst.data(); if (flag == NoUnloadSys || unload_sys()) { if (qt_debug_component()) @@ -610,12 +613,13 @@ bool QLibraryPrivate::unload(UnloadFlag flag) //when the library is unloaded, we release the reference on it so that 'this' //can get deleted libraryRefCount.deref(); - pHnd = 0; + pHnd.storeRelaxed(nullptr); instanceFactory.storeRelaxed(nullptr); + return true; } } - return (pHnd == 0); + return false; } void QLibraryPrivate::release() @@ -746,6 +750,7 @@ bool QLibraryPrivate::isPlugin() void QLibraryPrivate::updatePluginState() { + QMutexLocker locker(&mutex); errorString.clear(); if (pluginState != MightBeAPlugin) return; @@ -766,7 +771,7 @@ void QLibraryPrivate::updatePluginState() } #endif - if (!pHnd) { + if (!pHnd.loadRelaxed()) { // scan for the plugin metadata without loading success = findPatternUnloaded(fileName, this); } else { @@ -830,7 +835,7 @@ bool QLibrary::load() if (!d) return false; if (did_load) - return d->pHnd; + return d->pHnd.loadRelaxed(); did_load = true; return d->load(); } @@ -866,7 +871,7 @@ bool QLibrary::unload() */ bool QLibrary::isLoaded() const { - return d && d->pHnd; + return d && d->pHnd.loadRelaxed(); } @@ -977,8 +982,10 @@ void QLibrary::setFileName(const QString &fileName) QString QLibrary::fileName() const { - if (d) + if (d) { + QMutexLocker locker(&d->mutex); return d->qualifiedFileName.isEmpty() ? d->fileName : d->qualifiedFileName; + } return QString(); } @@ -1119,7 +1126,12 @@ QFunctionPointer QLibrary::resolve(const QString &fileName, const QString &versi */ QString QLibrary::errorString() const { - return (!d || d->errorString.isEmpty()) ? tr("Unknown error") : d->errorString; + QString str; + if (d) { + QMutexLocker locker(&d->mutex); + str = d->errorString; + } + return str.isEmpty() ? tr("Unknown error") : str; } /*! diff --git a/src/corelib/plugin/qlibrary_p.h b/src/corelib/plugin/qlibrary_p.h index 29724655a39..d756126c644 100644 --- a/src/corelib/plugin/qlibrary_p.h +++ b/src/corelib/plugin/qlibrary_p.h @@ -54,10 +54,10 @@ #include #include "QtCore/qlibrary.h" +#include "QtCore/qmutex.h" #include "QtCore/qpointer.h" #include "QtCore/qstringlist.h" #include "QtCore/qplugin.h" -#include "QtCore/qsharedpointer.h" #ifdef Q_OS_WIN # include "QtCore/qt_windows.h" #endif @@ -72,18 +72,15 @@ class QLibraryStore; class QLibraryPrivate { public: - #ifdef Q_OS_WIN - HINSTANCE + using Handle = HINSTANCE; #else - void * + using Handle = void *; #endif - pHnd; - enum UnloadFlag { UnloadSys, NoUnloadSys }; - QString fileName, qualifiedFileName; - QString fullVersion; + const QString fileName; + const QString fullVersion; bool load(); QtPluginInstanceFunction loadPlugin(); // loads and resolves instance @@ -94,17 +91,22 @@ public: QLibrary::LoadHints loadHints() const { return QLibrary::LoadHints(loadHintsInt.loadRelaxed()); } void setLoadHints(QLibrary::LoadHints lh); + QObject *pluginInstance(); static QLibraryPrivate *findOrCreate(const QString &fileName, const QString &version = QString(), QLibrary::LoadHints loadHints = nullptr); static QStringList suffixes_sys(const QString &fullVersion); static QStringList prefixes_sys(); - QPointer inst; // used by QFactoryLoader QAtomicPointer::type> instanceFactory; - QJsonObject metaData; + QAtomicPointer::type> pHnd; + // the mutex protects the fields below + QMutex mutex; + QPointer inst; // used by QFactoryLoader + QJsonObject metaData; QString errorString; + QString qualifiedFileName; void updatePluginState(); bool isPlugin(); diff --git a/src/corelib/plugin/qlibrary_unix.cpp b/src/corelib/plugin/qlibrary_unix.cpp index 23f96dbdaff..29813e5863b 100644 --- a/src/corelib/plugin/qlibrary_unix.cpp +++ b/src/corelib/plugin/qlibrary_unix.cpp @@ -214,8 +214,9 @@ bool QLibraryPrivate::load_sys() #endif bool retry = true; - for(int prefix = 0; retry && !pHnd && prefix < prefixes.size(); prefix++) { - for(int suffix = 0; retry && !pHnd && suffix < suffixes.size(); suffix++) { + Handle hnd = nullptr; + for (int prefix = 0; retry && !hnd && prefix < prefixes.size(); prefix++) { + for (int suffix = 0; retry && !hnd && suffix < suffixes.size(); suffix++) { if (!prefixes.at(prefix).isEmpty() && name.startsWith(prefixes.at(prefix))) continue; if (path.isEmpty() && prefixes.at(prefix).contains(QLatin1Char('/'))) @@ -232,7 +233,7 @@ bool QLibraryPrivate::load_sys() attempt = path + prefixes.at(prefix) + name + suffixes.at(suffix); } - pHnd = dlopen(QFile::encodeName(attempt), dlFlags); + hnd = dlopen(QFile::encodeName(attempt), dlFlags); #ifdef Q_OS_ANDROID if (!pHnd) { auto attemptFromBundle = attempt; @@ -248,7 +249,7 @@ bool QLibraryPrivate::load_sys() } #endif - if (!pHnd && fileName.startsWith(QLatin1Char('/')) && QFile::exists(attempt)) { + if (!hnd && fileName.startsWith(QLatin1Char('/')) && QFile::exists(attempt)) { // We only want to continue if dlopen failed due to that the shared library did not exist. // However, we are only able to apply this check for absolute filenames (since they are // not influenced by the content of LD_LIBRARY_PATH, /etc/ld.so.cache, DT_RPATH etc...) @@ -259,7 +260,7 @@ bool QLibraryPrivate::load_sys() } #ifdef Q_OS_MAC - if (!pHnd) { + if (!hnd) { QByteArray utf8Bundle = fileName.toUtf8(); QCFType bundleUrl = CFURLCreateFromFileSystemRepresentation(NULL, reinterpret_cast(utf8Bundle.data()), utf8Bundle.length(), true); QCFType bundle = CFBundleCreate(NULL, bundleUrl); @@ -268,23 +269,24 @@ bool QLibraryPrivate::load_sys() char executableFile[FILENAME_MAX]; CFURLGetFileSystemRepresentation(url, true, reinterpret_cast(executableFile), FILENAME_MAX); attempt = QString::fromUtf8(executableFile); - pHnd = dlopen(QFile::encodeName(attempt), dlFlags); + hnd = dlopen(QFile::encodeName(attempt), dlFlags); } } #endif - if (!pHnd) { + if (!hnd) { errorString = QLibrary::tr("Cannot load library %1: %2").arg(fileName, qdlerror()); } - if (pHnd) { + if (hnd) { qualifiedFileName = attempt; errorString.clear(); } - return (pHnd != 0); + pHnd.storeRelaxed(hnd); + return (hnd != nullptr); } bool QLibraryPrivate::unload_sys() { - if (dlclose(pHnd)) { + if (dlclose(pHnd.loadAcquire())) { #if defined (Q_OS_QNX) // Workaround until fixed in QNX; fixes crash in char *error = dlerror(); // QtDeclarative auto test "qqmlenginecleanup" for instance if (!qstrcmp(error, "Shared objects still referenced")) // On QNX that's only "informative" @@ -316,7 +318,7 @@ Q_CORE_EXPORT QFunctionPointer qt_mac_resolve_sys(void *handle, const char *symb QFunctionPointer QLibraryPrivate::resolve_sys(const char* symbol) { - QFunctionPointer address = QFunctionPointer(dlsym(pHnd, symbol)); + QFunctionPointer address = QFunctionPointer(dlsym(pHnd.loadAcquire(), symbol)); return address; } diff --git a/src/corelib/plugin/qlibrary_win.cpp b/src/corelib/plugin/qlibrary_win.cpp index c8309104aa2..000bf762763 100644 --- a/src/corelib/plugin/qlibrary_win.cpp +++ b/src/corelib/plugin/qlibrary_win.cpp @@ -95,26 +95,27 @@ bool QLibraryPrivate::load_sys() attempts.prepend(QDir::rootPath() + fileName); #endif + Handle hnd = nullptr; for (const QString &attempt : qAsConst(attempts)) { #ifndef Q_OS_WINRT - pHnd = LoadLibrary(reinterpret_cast(QDir::toNativeSeparators(attempt).utf16())); + hnd = LoadLibrary(reinterpret_cast(QDir::toNativeSeparators(attempt).utf16())); #else // Q_OS_WINRT QString path = QDir::toNativeSeparators(QDir::current().relativeFilePath(attempt)); - pHnd = LoadPackagedLibrary(reinterpret_cast(path.utf16()), 0); - if (pHnd) + hnd = LoadPackagedLibrary(reinterpret_cast(path.utf16()), 0); + if (hnd) qualifiedFileName = attempt; #endif // !Q_OS_WINRT // If we have a handle or the last error is something other than "unable // to find the module", then bail out - if (pHnd || ::GetLastError() != ERROR_MOD_NOT_FOUND) + if (hnd || ::GetLastError() != ERROR_MOD_NOT_FOUND) break; } #ifndef Q_OS_WINRT SetErrorMode(oldmode); #endif - if (!pHnd) { + if (!hnd) { errorString = QLibrary::tr("Cannot load library %1: %2").arg( QDir::toNativeSeparators(fileName), qt_error_string()); } else { @@ -123,7 +124,7 @@ bool QLibraryPrivate::load_sys() #ifndef Q_OS_WINRT wchar_t buffer[MAX_PATH]; - ::GetModuleFileName(pHnd, buffer, MAX_PATH); + ::GetModuleFileName(hnd, buffer, MAX_PATH); QString moduleFileName = QString::fromWCharArray(buffer); moduleFileName.remove(0, 1 + moduleFileName.lastIndexOf(QLatin1Char('\\'))); @@ -138,19 +139,20 @@ bool QLibraryPrivate::load_sys() HMODULE hmod; bool ok = GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_PIN | GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, - reinterpret_cast(pHnd), + reinterpret_cast(hnd), &hmod); - Q_ASSERT(!ok || hmod == pHnd); + Q_ASSERT(!ok || hmod == hnd); Q_UNUSED(ok); } #endif // !Q_OS_WINRT } - return (pHnd != 0); + pHnd.storeRelaxed(hnd); + return (pHnd != nullptr); } bool QLibraryPrivate::unload_sys() { - if (!FreeLibrary(pHnd)) { + if (!FreeLibrary(pHnd.loadAcquire())) { errorString = QLibrary::tr("Cannot unload library %1: %2").arg( QDir::toNativeSeparators(fileName), qt_error_string()); return false; @@ -161,7 +163,7 @@ bool QLibraryPrivate::unload_sys() QFunctionPointer QLibraryPrivate::resolve_sys(const char* symbol) { - FARPROC address = GetProcAddress(pHnd, symbol); + FARPROC address = GetProcAddress(pHnd.loadAcquire(), symbol); return QFunctionPointer(address); } QT_END_NAMESPACE diff --git a/src/corelib/plugin/qpluginloader.cpp b/src/corelib/plugin/qpluginloader.cpp index dac8502daea..0e12793d5eb 100644 --- a/src/corelib/plugin/qpluginloader.cpp +++ b/src/corelib/plugin/qpluginloader.cpp @@ -196,9 +196,7 @@ QObject *QPluginLoader::instance() { if (!isLoaded() && !load()) return 0; - if (!d->inst && d->instance) - d->inst = d->instance(); - return d->inst.data(); + return d->pluginInstance(); } /*! @@ -233,7 +231,7 @@ bool QPluginLoader::load() if (!d || d->fileName.isEmpty()) return false; if (did_load) - return d->pHnd && d->instance; + return d->pHnd && d->instanceFactory.loadAcquire(); if (!d->isPlugin()) return false; did_load = true; @@ -275,7 +273,7 @@ bool QPluginLoader::unload() */ bool QPluginLoader::isLoaded() const { - return d && d->pHnd && d->instance; + return d && d->pHnd && d->instanceFactory.loadRelaxed(); } #if defined(QT_SHARED) From d678827f11263cb7c53218448c9bcc9afbd1439f Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Thu, 27 Feb 2020 09:58:59 +0100 Subject: [PATCH 11/15] Un-blacklist QElapsedTimer::elapsed test The test was fixed and metrics show no flaky failures anymore. Task-number: QTBUG-58713 Change-Id: I50c0844db099f45bb5b7ca51a510bf0318554c44 Reviewed-by: Lars Knoll --- tests/auto/corelib/kernel/qelapsedtimer/BLACKLIST | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 tests/auto/corelib/kernel/qelapsedtimer/BLACKLIST diff --git a/tests/auto/corelib/kernel/qelapsedtimer/BLACKLIST b/tests/auto/corelib/kernel/qelapsedtimer/BLACKLIST deleted file mode 100644 index 2317cf886cd..00000000000 --- a/tests/auto/corelib/kernel/qelapsedtimer/BLACKLIST +++ /dev/null @@ -1,3 +0,0 @@ -[elapsed] -macos -windows-10 From 7c7bd44669babe5eb9cbc0b803d43e8e2cf0518b Mon Sep 17 00:00:00 2001 From: Oliver Wolff Date: Thu, 5 Mar 2020 10:06:24 +0100 Subject: [PATCH 12/15] examples: Fix build for configurations that do not have printsupport printsupport features are only available if the module is available for the configuration. Every printer feature check has to be coupled with a check for the module itself. Change-Id: Id2ca84e71d5d74463d0ff35e8b18b252a779a883 Reviewed-by: Friedemann Kleint --- examples/widgets/itemviews/spreadsheet/printview.cpp | 12 +++++++++--- .../widgets/itemviews/spreadsheet/spreadsheet.cpp | 2 +- examples/widgets/richtext/textedit/textedit.cpp | 12 ++++++------ examples/widgets/tutorials/notepad/notepad.pro | 2 ++ examples/widgets/widgets/imageviewer/imageviewer.cpp | 9 +++++---- examples/widgets/widgets/imageviewer/imageviewer.h | 10 +++++++--- 6 files changed, 30 insertions(+), 17 deletions(-) diff --git a/examples/widgets/itemviews/spreadsheet/printview.cpp b/examples/widgets/itemviews/spreadsheet/printview.cpp index 7db1a6bad9d..7700b4ed6a4 100644 --- a/examples/widgets/itemviews/spreadsheet/printview.cpp +++ b/examples/widgets/itemviews/spreadsheet/printview.cpp @@ -50,8 +50,12 @@ #include "printview.h" -#ifndef QT_NO_PRINTER -#include +#if defined(QT_PRINTSUPPORT_LIB) +# include + +# if QT_CONFIG(printer) +# include +# endif #endif PrintView::PrintView() @@ -62,9 +66,11 @@ PrintView::PrintView() void PrintView::print(QPrinter *printer) { -#ifndef QT_NO_PRINTER +#if defined(QT_PRINTSUPPORT_LIB) && QT_CONFIG(printer) resize(printer->width(), printer->height()); render(printer); +#else + Q_UNUSED(printer) #endif } diff --git a/examples/widgets/itemviews/spreadsheet/spreadsheet.cpp b/examples/widgets/itemviews/spreadsheet/spreadsheet.cpp index fc7fbb872c7..ac8ea7d4374 100644 --- a/examples/widgets/itemviews/spreadsheet/spreadsheet.cpp +++ b/examples/widgets/itemviews/spreadsheet/spreadsheet.cpp @@ -638,7 +638,7 @@ QString encode_pos(int row, int col) void SpreadSheet::print() { -#if QT_CONFIG(printpreviewdialog) +#if defined(QT_PRINTSUPPORT_LIB) && QT_CONFIG(printpreviewdialog) QPrinter printer(QPrinter::ScreenResolution); QPrintPreviewDialog dlg(&printer); PrintView view; diff --git a/examples/widgets/richtext/textedit/textedit.cpp b/examples/widgets/richtext/textedit/textedit.cpp index 7708b25a241..c9173bcb999 100644 --- a/examples/widgets/richtext/textedit/textedit.cpp +++ b/examples/widgets/richtext/textedit/textedit.cpp @@ -197,7 +197,7 @@ void TextEdit::setupFileActions() a->setPriority(QAction::LowPriority); menu->addSeparator(); -#ifndef QT_NO_PRINTER +#if defined(QT_PRINTSUPPORT_LIB) && QT_CONFIG(printer) const QIcon printIcon = QIcon::fromTheme("document-print", QIcon(rsrcPath + "/fileprint.png")); a = menu->addAction(printIcon, tr("&Print..."), this, &TextEdit::filePrint); a->setPriority(QAction::LowPriority); @@ -559,7 +559,7 @@ void TextEdit::filePrint() void TextEdit::filePrintPreview() { -#if QT_CONFIG(printpreviewdialog) +#if defined(QT_PRINTSUPPORT_LIB) && QT_CONFIG(printpreviewdialog) QPrinter printer(QPrinter::HighResolution); QPrintPreviewDialog preview(&printer, this); connect(&preview, &QPrintPreviewDialog::paintRequested, this, &TextEdit::printPreview); @@ -569,17 +569,17 @@ void TextEdit::filePrintPreview() void TextEdit::printPreview(QPrinter *printer) { -#ifdef QT_NO_PRINTER - Q_UNUSED(printer); -#else +#if defined(QT_PRINTSUPPORT_LIB) && QT_CONFIG(printer) textEdit->print(printer); +#else + Q_UNUSED(printer) #endif } void TextEdit::filePrintPdf() { -#ifndef QT_NO_PRINTER +#if defined(QT_PRINTSUPPORT_LIB) && QT_CONFIG(printer) //! [0] QFileDialog fileDialog(this, tr("Export PDF")); fileDialog.setAcceptMode(QFileDialog::AcceptSave); diff --git a/examples/widgets/tutorials/notepad/notepad.pro b/examples/widgets/tutorials/notepad/notepad.pro index 6451f22735e..efe16fc2a2d 100644 --- a/examples/widgets/tutorials/notepad/notepad.pro +++ b/examples/widgets/tutorials/notepad/notepad.pro @@ -1,6 +1,8 @@ TEMPLATE = app TARGET = notepad +QT += widgets + qtHaveModule(printsupport): QT += printsupport requires(qtConfig(fontdialog)) diff --git a/examples/widgets/widgets/imageviewer/imageviewer.cpp b/examples/widgets/widgets/imageviewer/imageviewer.cpp index 70a4cda9844..1ed55ca6cbf 100644 --- a/examples/widgets/widgets/imageviewer/imageviewer.cpp +++ b/examples/widgets/widgets/imageviewer/imageviewer.cpp @@ -69,10 +69,11 @@ #include #if defined(QT_PRINTSUPPORT_LIB) -#include -#if QT_CONFIG(printdialog) -#include -#endif +# include + +# if QT_CONFIG(printdialog) +# include +# endif #endif //! [0] diff --git a/examples/widgets/widgets/imageviewer/imageviewer.h b/examples/widgets/widgets/imageviewer/imageviewer.h index 49c7ac205b9..9c8388d4709 100644 --- a/examples/widgets/widgets/imageviewer/imageviewer.h +++ b/examples/widgets/widgets/imageviewer/imageviewer.h @@ -53,8 +53,12 @@ #include #include -#ifndef QT_NO_PRINTER -#include +#if defined(QT_PRINTSUPPORT_LIB) +# include + +# if QT_CONFIG(printer) +# include +# endif #endif QT_BEGIN_NAMESPACE @@ -100,7 +104,7 @@ private: QScrollArea *scrollArea; double scaleFactor = 1; -#ifndef QT_NO_PRINTER +#if defined(QT_PRINTSUPPORT_LIB) && QT_CONFIG(printer) QPrinter printer; #endif From 08b9e663470137e04b1f19522606707a652b3ae5 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Tue, 3 Mar 2020 12:30:59 +0100 Subject: [PATCH 13/15] Fix QDeviceDiscoveryUDev on FreeBSD Include input.h from the right path. Change-Id: I016027ab5d6372a8584e04043444a355e7e2d539 Reviewed-by: Joerg Bornemann --- src/platformsupport/devicediscovery/qdevicediscovery_udev.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/platformsupport/devicediscovery/qdevicediscovery_udev.cpp b/src/platformsupport/devicediscovery/qdevicediscovery_udev.cpp index f601a196ca1..c2413c131d0 100644 --- a/src/platformsupport/devicediscovery/qdevicediscovery_udev.cpp +++ b/src/platformsupport/devicediscovery/qdevicediscovery_udev.cpp @@ -46,7 +46,11 @@ #include #include +#ifdef Q_OS_FREEBSD +#include +#else #include +#endif QT_BEGIN_NAMESPACE From c1bb6285720513b9b22b552ca30af3c1501a7761 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 5 Mar 2020 15:57:07 +0100 Subject: [PATCH 14/15] Port from deprecated std::is_pod to is_trivial + is_standard_layout The std::is_pod trait is deprecated in C++20; is_trivial and is_standard_layout exist since C++11. Change-Id: I4b901d8edf1a55001764445aee9c338d3dc23b21 Reviewed-by: Lars Knoll --- src/corelib/kernel/qmetatype.cpp | 3 ++- src/corelib/serialization/qcborvalue_p.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/corelib/kernel/qmetatype.cpp b/src/corelib/kernel/qmetatype.cpp index 356a675517a..9fc0659cf2c 100644 --- a/src/corelib/kernel/qmetatype.cpp +++ b/src/corelib/kernel/qmetatype.cpp @@ -589,7 +589,8 @@ QMetaTypeComparatorRegistry; typedef QMetaTypeFunctionRegistry QMetaTypeDebugStreamRegistry; -Q_STATIC_ASSERT(std::is_pod::value); +Q_STATIC_ASSERT(std::is_trivial::value); +Q_STATIC_ASSERT(std::is_standard_layout::value); Q_DECLARE_TYPEINFO(QCustomTypeInfo, Q_MOVABLE_TYPE); Q_GLOBAL_STATIC(QVector, customTypes) diff --git a/src/corelib/serialization/qcborvalue_p.h b/src/corelib/serialization/qcborvalue_p.h index 48818e4c63d..a74ac2ba10e 100644 --- a/src/corelib/serialization/qcborvalue_p.h +++ b/src/corelib/serialization/qcborvalue_p.h @@ -115,7 +115,8 @@ struct ByteData QStringView asStringView() const{ return QStringView(utf16(), len / 2); } QString asQStringRaw() const { return QString::fromRawData(utf16(), len / 2); } }; -Q_STATIC_ASSERT(std::is_pod::value); +Q_STATIC_ASSERT(std::is_trivial::value); +Q_STATIC_ASSERT(std::is_standard_layout::value); } // namespace QtCbor Q_DECLARE_TYPEINFO(QtCbor::Element, Q_PRIMITIVE_TYPE); From 3702a4c37e06a989cdd1cf19a26bc0d1c3fba07c Mon Sep 17 00:00:00 2001 From: Christian Ehrlicher Date: Sun, 8 Mar 2020 19:52:18 +0100 Subject: [PATCH 15/15] QSortFilterProxyModel doc: do not mention deprecated function reset() QSFPM::reset() is deprecated and begin/endResetModel() should be used. Therefore adjust the documentation to reflect this. Fixes: QTBUG-82470 Change-Id: I786b3f25e5674d97d0ef6a0c91342973d5e952e9 Reviewed-by: Sze Howe Koh Reviewed-by: Paul Wicking --- src/corelib/itemmodels/qsortfilterproxymodel.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/corelib/itemmodels/qsortfilterproxymodel.cpp b/src/corelib/itemmodels/qsortfilterproxymodel.cpp index 61d37d5062a..122b95ed57e 100644 --- a/src/corelib/itemmodels/qsortfilterproxymodel.cpp +++ b/src/corelib/itemmodels/qsortfilterproxymodel.cpp @@ -1949,8 +1949,9 @@ void QSortFilterProxyModelPrivate::_q_sourceColumnsMoved( example.) If you are working with large amounts of filtering and have to invoke - invalidateFilter() repeatedly, using reset() may be more efficient, - depending on the implementation of your model. However, reset() returns the + invalidateFilter() repeatedly, using beginResetModel() / endResetModel() may + be more efficient, depending on the implementation of your model. However, + beginResetModel() / endResetModel() returns the proxy model to its original state, losing selection information, and will cause the proxy model to be repopulated.