From 3a72496b5c43484a94882440993b0ca0cb842d8a Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 16 Jul 2021 09:57:56 +0200 Subject: [PATCH] tests: fix some -Wvolatile C++20 deprecated compound volatile statements such as pre- and post-increments, to stress that they're not atomic. So instead of volatile i; ~~~~; ++i; you're now supposed to write volatile i; ~~~~; int j = i; // volatile load ++j; i = j; // volatile store which matches more closely what hardware does. Instead of fixing every use of volatile pre- or post-increment in this fashion individually, and realising that probably a few more Qt modules will have the same kind of code patterns in them, write QtPrivate functions to do the job centrally. Change-Id: I838097bd484ef2118c071726963f103c080d2ba5 Reviewed-by: Lars Knoll --- src/corelib/CMakeLists.txt | 1 + src/corelib/global/qvolatile_p.h | 89 +++++++++++++++++++ .../auto/corelib/thread/qmutex/CMakeLists.txt | 6 -- .../auto/corelib/thread/qmutex/tst_qmutex.cpp | 9 +- .../thread/qreadwritelock/CMakeLists.txt | 2 + .../qreadwritelock/tst_qreadwritelock.cpp | 9 +- .../tools/qsharedpointer/CMakeLists.txt | 2 + .../qsharedpointer/tst_qsharedpointer.cpp | 3 +- .../corelib/thread/qmutex/CMakeLists.txt | 1 + .../corelib/thread/qmutex/tst_qmutex.cpp | 7 +- 10 files changed, 109 insertions(+), 20 deletions(-) create mode 100644 src/corelib/global/qvolatile_p.h diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt index db261d33cc..b77cf35b92 100644 --- a/src/corelib/CMakeLists.txt +++ b/src/corelib/CMakeLists.txt @@ -60,6 +60,7 @@ qt_internal_add_module(Core global/qsysinfo.h global/qsystemdetection.h global/qtypeinfo.h + global/qvolatile_p.h io/qabstractfileengine.cpp io/qabstractfileengine_p.h io/qbuffer.cpp io/qbuffer.h io/qdataurl.cpp io/qdataurl_p.h diff --git a/src/corelib/global/qvolatile_p.h b/src/corelib/global/qvolatile_p.h new file mode 100644 index 0000000000..211cb3d90c --- /dev/null +++ b/src/corelib/global/qvolatile_p.h @@ -0,0 +1,89 @@ +/**************************************************************************** +** +** Copyright (C) 2021 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com, author Marc Mutz +** Contact: http://www.qt.io/licensing/ +** +** This file is part of the QtCore module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 3 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL3 included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 3 requirements +** will be met: https://www.gnu.org/licenses/lgpl-3.0.html. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 2.0 or (at your option) the GNU General +** Public license version 3 or any later version approved by the KDE Free +** Qt Foundation. The licenses are as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-2.0.html and +** https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#ifndef QVOLATILE_P_H +#define QVOLATILE_P_H + +// +// W A R N I N G +// ------------- +// +// This file is not part of the Qt API. It exists for the convenience +// of a number of Qt sources files. This header file may change from +// version to version without notice, or even be removed. +// +// We mean it. +// + +#include + +QT_BEGIN_NAMESPACE + +namespace QtPrivate { +template +using if_volatile = std::enable_if_t, bool>; + +// +// C++20-deprecated volatile compound operations, rewritten as separated operations +// + +// these functions return `auto`, not `T`, to strip cv-qualifiers without having +// to mention the `volatile` keyword + +template = true> +auto volatilePreIncrement(T &x) { + auto y = x; + ++y; + x = y; + return y; +} + +template = true> +auto volatilePreDecrement(T &x) +{ + auto y = x; + --y; + x = y; + return y; +} +} // namespace QtPrivate + +QT_END_NAMESPACE + +#endif // QVOLATILE_H diff --git a/tests/auto/corelib/thread/qmutex/CMakeLists.txt b/tests/auto/corelib/thread/qmutex/CMakeLists.txt index 41b7b1da68..1412036fc9 100644 --- a/tests/auto/corelib/thread/qmutex/CMakeLists.txt +++ b/tests/auto/corelib/thread/qmutex/CMakeLists.txt @@ -7,12 +7,6 @@ qt_internal_add_test(tst_qmutex SOURCES tst_qmutex.cpp -) - -## Scopes: -##################################################################### - -qt_internal_extend_target(tst_qmutex CONDITION WIN32 PUBLIC_LIBRARIES Qt::CorePrivate ) diff --git a/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp b/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp index fb080a1a12..c00ddea908 100644 --- a/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp +++ b/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp @@ -36,6 +36,7 @@ #include #include #include +#include class tst_QMutex : public QObject { @@ -1191,9 +1192,9 @@ void tst_QMutex::tryLockDeadlock() { for (int i = 0; i < 100000; ++i) { if (mut.tryLock(0)) { - if ((++tryLockDeadlockCounter) != 1) + if (QtPrivate::volatilePreIncrement(tryLockDeadlockCounter) != 1) ++tryLockDeadlockFailureCount; - if ((--tryLockDeadlockCounter) != 0) + if (QtPrivate::volatilePreDecrement(tryLockDeadlockCounter) != 0) ++tryLockDeadlockFailureCount; mut.unlock(); } @@ -1210,9 +1211,9 @@ void tst_QMutex::tryLockDeadlock() for (int i = 0; i < 100000; ++i) { mut.lock(); - if ((++tryLockDeadlockCounter) != 1) + if (QtPrivate::volatilePreIncrement(tryLockDeadlockCounter) != 1) ++tryLockDeadlockFailureCount; - if ((--tryLockDeadlockCounter) != 0) + if (QtPrivate::volatilePreDecrement(tryLockDeadlockCounter) != 0) ++tryLockDeadlockFailureCount; mut.unlock(); } diff --git a/tests/auto/corelib/thread/qreadwritelock/CMakeLists.txt b/tests/auto/corelib/thread/qreadwritelock/CMakeLists.txt index 4b81229024..a99001425e 100644 --- a/tests/auto/corelib/thread/qreadwritelock/CMakeLists.txt +++ b/tests/auto/corelib/thread/qreadwritelock/CMakeLists.txt @@ -7,4 +7,6 @@ qt_internal_add_test(tst_qreadwritelock SOURCES tst_qreadwritelock.cpp + PUBLIC_LIBRARIES + Qt::CorePrivate ) diff --git a/tests/auto/corelib/thread/qreadwritelock/tst_qreadwritelock.cpp b/tests/auto/corelib/thread/qreadwritelock/tst_qreadwritelock.cpp index ca282e2723..a6c0ebae22 100644 --- a/tests/auto/corelib/thread/qreadwritelock/tst_qreadwritelock.cpp +++ b/tests/auto/corelib/thread/qreadwritelock/tst_qreadwritelock.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #ifdef Q_OS_UNIX #include @@ -587,12 +588,8 @@ public: if(count) qFatal("Non-zero count at start of write! (%d)",count ); // printf("."); - int i; - for(i=0; i #include #include +#include #include "forwarddeclared.h" #include "nontracked.h" @@ -1944,7 +1945,7 @@ class ThreadData QAtomicInt * volatile ptr; public: ThreadData(QAtomicInt *p) : ptr(p) { } - ~ThreadData() { ++ptr; } + ~ThreadData() { QtPrivate::volatilePreIncrement(ptr); } void ref() { // if we're called after the destructor, we'll crash diff --git a/tests/benchmarks/corelib/thread/qmutex/CMakeLists.txt b/tests/benchmarks/corelib/thread/qmutex/CMakeLists.txt index 8872a50227..923bfba117 100644 --- a/tests/benchmarks/corelib/thread/qmutex/CMakeLists.txt +++ b/tests/benchmarks/corelib/thread/qmutex/CMakeLists.txt @@ -8,6 +8,7 @@ qt_internal_add_benchmark(tst_bench_qmutex SOURCES tst_qmutex.cpp PUBLIC_LIBRARIES + Qt::CorePrivate Qt::Test ) diff --git a/tests/benchmarks/corelib/thread/qmutex/tst_qmutex.cpp b/tests/benchmarks/corelib/thread/qmutex/tst_qmutex.cpp index 9cea995433..9b14cbca5d 100644 --- a/tests/benchmarks/corelib/thread/qmutex/tst_qmutex.cpp +++ b/tests/benchmarks/corelib/thread/qmutex/tst_qmutex.cpp @@ -28,6 +28,7 @@ #include #include +#include #include @@ -158,7 +159,7 @@ void tst_QMutex::noThread() QBENCHMARK { count = 0; for (int i = 0; i < N; i++) { - count++; + QtPrivate::volatilePreIncrement(count); } } break; @@ -167,7 +168,7 @@ void tst_QMutex::noThread() count = 0; for (int i = 0; i < N; i++) { mtx.lock(); - count++; + QtPrivate::volatilePreIncrement(count); mtx.unlock(); } } @@ -177,7 +178,7 @@ void tst_QMutex::noThread() count = 0; for (int i = 0; i < N; i++) { QMutexLocker locker(&mtx); - count++; + QtPrivate::volatilePreIncrement(count); } } break;