From c07c4eb068a586c033b190794ee7f31a1ed70375 Mon Sep 17 00:00:00 2001 From: Christian Stenger Date: Tue, 8 Nov 2016 09:46:22 +0100 Subject: [PATCH] AutoTest: Avoid concurrent access on static function members First thread using the helper function initialized the list object another thread accesses it afterwards. This could be problematic under some circumstances. Issue was detected using Helgrind. Change-Id: I1520b1f7364742cb02630af1fd9d98960dec1f41 Reviewed-by: David Schulz --- src/plugins/autotest/autotest.pro | 3 + src/plugins/autotest/gtest/gtest_utils.cpp | 56 ++++++++++++++ src/plugins/autotest/gtest/gtest_utils.h | 29 ++----- src/plugins/autotest/qtest/qttest_utils.cpp | 68 ++++++++++++++++ src/plugins/autotest/qtest/qttest_utils.h | 40 ++-------- .../autotest/quick/quicktest_utils.cpp | 77 +++++++++++++++++++ src/plugins/autotest/quick/quicktest_utils.h | 49 ++---------- 7 files changed, 223 insertions(+), 99 deletions(-) create mode 100644 src/plugins/autotest/gtest/gtest_utils.cpp create mode 100644 src/plugins/autotest/qtest/qttest_utils.cpp create mode 100644 src/plugins/autotest/quick/quicktest_utils.cpp diff --git a/src/plugins/autotest/autotest.pro b/src/plugins/autotest/autotest.pro index 9cf411299d4..a22a584ff3a 100644 --- a/src/plugins/autotest/autotest.pro +++ b/src/plugins/autotest/autotest.pro @@ -32,6 +32,7 @@ SOURCES += \ gtest/gtestframework.cpp \ gtest/gtestsettings.cpp \ gtest/gtestsettingspage.cpp \ + gtest/gtest_utils.cpp \ qtest/qttesttreeitem.cpp \ qtest/qttestvisitors.cpp \ qtest/qttestconfiguration.cpp \ @@ -41,11 +42,13 @@ SOURCES += \ qtest/qttestframework.cpp \ qtest/qttestsettings.cpp \ qtest/qttestsettingspage.cpp \ + qtest/qttest_utils.cpp \ quick/quicktestconfiguration.cpp \ quick/quicktestparser.cpp \ quick/quicktesttreeitem.cpp \ quick/quicktestvisitors.cpp \ quick/quicktestframework.cpp \ + quick/quicktest_utils.cpp \ testframeworkmanager.cpp diff --git a/src/plugins/autotest/gtest/gtest_utils.cpp b/src/plugins/autotest/gtest/gtest_utils.cpp new file mode 100644 index 00000000000..4ce68760a25 --- /dev/null +++ b/src/plugins/autotest/gtest/gtest_utils.cpp @@ -0,0 +1,56 @@ +/**************************************************************************** +** +** Copyright (C) 2016 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** 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 General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** 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-3.0.html. +** +****************************************************************************/ + +#include "gtest_utils.h" + +#include + +namespace Autotest { +namespace Internal { +namespace GTestUtils { + +static const QStringList valid = { + QStringLiteral("TEST"), QStringLiteral("TEST_F"), QStringLiteral("TEST_P"), + QStringLiteral("TYPED_TEST"), QStringLiteral("TYPED_TEST_P") +}; + +bool isGTestMacro(const QString ¯o) +{ + return valid.contains(macro); +} + +bool isGTestParameterized(const QString ¯o) +{ + return macro == QStringLiteral("TEST_P") || macro == QStringLiteral("TYPED_TEST_P"); +} + +bool isGTestTyped(const QString ¯o) +{ + return macro == QStringLiteral("TYPED_TEST") || macro == QStringLiteral("TYPED_TEST_P"); +} + +} // namespace GTestUtils +} // namespace Internal +} // namespace Autotest diff --git a/src/plugins/autotest/gtest/gtest_utils.h b/src/plugins/autotest/gtest/gtest_utils.h index 3a46a4e52a6..93dd1ad6215 100644 --- a/src/plugins/autotest/gtest/gtest_utils.h +++ b/src/plugins/autotest/gtest/gtest_utils.h @@ -25,31 +25,16 @@ #pragma once +#include + namespace Autotest { namespace Internal { +namespace GTestUtils { -class GTestUtils -{ -public: - static bool isGTestMacro(const QString ¯o) - { - static QStringList valid = { - QStringLiteral("TEST"), QStringLiteral("TEST_F"), QStringLiteral("TEST_P"), - QStringLiteral("TYPED_TEST"), QStringLiteral("TYPED_TEST_P") - }; - return valid.contains(macro); - } - - static bool isGTestParameterized(const QString ¯o) - { - return macro == QStringLiteral("TEST_P") || macro == QStringLiteral("TYPED_TEST_P"); - } - - static bool isGTestTyped(const QString ¯o) - { - return macro == QStringLiteral("TYPED_TEST") || macro == QStringLiteral("TYPED_TEST_P"); - } -}; +bool isGTestMacro(const QString ¯o); +bool isGTestParameterized(const QString ¯o); +bool isGTestTyped(const QString ¯o); +} // namespace GTestUtils } // namespace Internal } // namespace Autotest diff --git a/src/plugins/autotest/qtest/qttest_utils.cpp b/src/plugins/autotest/qtest/qttest_utils.cpp new file mode 100644 index 00000000000..bb80570ed12 --- /dev/null +++ b/src/plugins/autotest/qtest/qttest_utils.cpp @@ -0,0 +1,68 @@ +/**************************************************************************** +** +** Copyright (C) 2016 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** 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 General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** 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-3.0.html. +** +****************************************************************************/ + +#include "qttest_utils.h" +#include "../testframeworkmanager.h" +#include "../testtreeitem.h" + +#include + +#include + +namespace Autotest { +namespace Internal { +namespace QTestUtils { + +static const QByteArrayList valid = {"QTEST_MAIN", "QTEST_APPLESS_MAIN", "QTEST_GUILESS_MAIN"}; + +bool isQTestMacro(const QByteArray ¯o) +{ + return valid.contains(macro); +} + +QHash testCaseNamesForFiles(const Core::Id &id, + const QStringList &files) +{ + QHash result; + TestTreeItem *rootNode = TestFrameworkManager::instance()->rootNodeForTestFramework(id); + QTC_ASSERT(rootNode, return result); + + for (int row = 0, rootCount = rootNode->childCount(); row < rootCount; ++row) { + const TestTreeItem *child = rootNode->childItem(row); + if (files.contains(child->filePath())) { + result.insert(child->filePath(), child->name()); + } + for (int childRow = 0, count = child->childCount(); childRow < count; ++childRow) { + const TestTreeItem *grandChild = child->childItem(childRow); + if (files.contains(grandChild->filePath())) + result.insert(grandChild->filePath(), child->name()); + } + } + return result; +} + +} // namespace QTestUtils +} // namespace Internal +} // namespace Autotest diff --git a/src/plugins/autotest/qtest/qttest_utils.h b/src/plugins/autotest/qtest/qttest_utils.h index 75024b9bc9f..0d4f661a357 100644 --- a/src/plugins/autotest/qtest/qttest_utils.h +++ b/src/plugins/autotest/qtest/qttest_utils.h @@ -25,45 +25,17 @@ #pragma once -#include "../testframeworkmanager.h" +#include -#include - -#include +namespace Core { class Id; } namespace Autotest { namespace Internal { +namespace QTestUtils { -class QTestUtils -{ -public: - static bool isQTestMacro(const QByteArray ¯o) - { - static QByteArrayList valid = {"QTEST_MAIN", "QTEST_APPLESS_MAIN", "QTEST_GUILESS_MAIN"}; - return valid.contains(macro); - } - - static QHash testCaseNamesForFiles(const Core::Id &id, - const QStringList &files) - { - QHash result; - TestTreeItem *rootNode = TestFrameworkManager::instance()->rootNodeForTestFramework(id); - QTC_ASSERT(rootNode, return result); - - for (int row = 0, rootCount = rootNode->childCount(); row < rootCount; ++row) { - const TestTreeItem *child = rootNode->childItem(row); - if (files.contains(child->filePath())) { - result.insert(child->filePath(), child->name()); - } - for (int childRow = 0, count = child->childCount(); childRow < count; ++childRow) { - const TestTreeItem *grandChild = child->childItem(childRow); - if (files.contains(grandChild->filePath())) - result.insert(grandChild->filePath(), child->name()); - } - } - return result; - } -}; +bool isQTestMacro(const QByteArray ¯o); +QHash testCaseNamesForFiles(const Core::Id &id, const QStringList &files); +} // namespace QTestUtils } // namespace Internal } // namespace Autotest diff --git a/src/plugins/autotest/quick/quicktest_utils.cpp b/src/plugins/autotest/quick/quicktest_utils.cpp new file mode 100644 index 00000000000..2e7c4f1594e --- /dev/null +++ b/src/plugins/autotest/quick/quicktest_utils.cpp @@ -0,0 +1,77 @@ +/**************************************************************************** +** +** Copyright (C) 2016 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** 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 General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** 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-3.0.html. +** +****************************************************************************/ + +#include "quicktest_utils.h" +#include "../testframeworkmanager.h" +#include "../testtreeitem.h" + +#include + +#include + +namespace Autotest { +namespace Internal { +namespace QuickTestUtils { + +static const QByteArrayList valid = {"QUICK_TEST_MAIN", "QUICK_TEST_OPENGL_MAIN"}; + +bool isQuickTestMacro(const QByteArray ¯o) +{ + return valid.contains(macro); +} + +QHash proFilesForQmlFiles(const Core::Id &id, const QStringList &files) +{ + QHash result; + TestTreeItem *rootNode = TestFrameworkManager::instance()->rootNodeForTestFramework(id); + QTC_ASSERT(rootNode, return result); + + if (files.isEmpty()) + return result; + + for (int row = 0, rootCount = rootNode->childCount(); row < rootCount; ++row) { + const TestTreeItem *child = rootNode->childItem(row); + const QString &file = child->filePath(); + if (!file.isEmpty() && files.contains(file)) { + const QString &proFile = child->proFile(); + if (!proFile.isEmpty()) + result.insert(file, proFile); + } + for (int subRow = 0, subCount = child->childCount(); subRow < subCount; ++subRow) { + const TestTreeItem *grandChild = child->childItem(subRow); + const QString &file = grandChild->filePath(); + if (!file.isEmpty() && files.contains(file)) { + const QString &proFile = grandChild->proFile(); + if (!proFile.isEmpty()) + result.insert(file, proFile); + } + } + } + return result; +} + +} // namespace QuickTestUtils +} // namespace Internal +} // namespace Autotest diff --git a/src/plugins/autotest/quick/quicktest_utils.h b/src/plugins/autotest/quick/quicktest_utils.h index f7a17ad538c..af5cfdb7bf0 100644 --- a/src/plugins/autotest/quick/quicktest_utils.h +++ b/src/plugins/autotest/quick/quicktest_utils.h @@ -25,54 +25,17 @@ #pragma once -#include "../testframeworkmanager.h" +#include -#include - -#include +namespace Core { class Id; } namespace Autotest { namespace Internal { +namespace QuickTestUtils { -class QuickTestUtils -{ -public: - static bool isQuickTestMacro(const QByteArray ¯o) - { - static const QByteArrayList valid = {"QUICK_TEST_MAIN", "QUICK_TEST_OPENGL_MAIN"}; - return valid.contains(macro); - } - - static QHash proFilesForQmlFiles(const Core::Id &id, const QStringList &files) - { - QHash result; - TestTreeItem *rootNode = TestFrameworkManager::instance()->rootNodeForTestFramework(id); - QTC_ASSERT(rootNode, return result); - - if (files.isEmpty()) - return result; - - for (int row = 0, rootCount = rootNode->childCount(); row < rootCount; ++row) { - const TestTreeItem *child = rootNode->childItem(row); - const QString &file = child->filePath(); - if (!file.isEmpty() && files.contains(file)) { - const QString &proFile = child->proFile(); - if (!proFile.isEmpty()) - result.insert(file, proFile); - } - for (int subRow = 0, subCount = child->childCount(); subRow < subCount; ++subRow) { - const TestTreeItem *grandChild = child->childItem(subRow); - const QString &file = grandChild->filePath(); - if (!file.isEmpty() && files.contains(file)) { - const QString &proFile = grandChild->proFile(); - if (!proFile.isEmpty()) - result.insert(file, proFile); - } - } - } - return result; - } -}; +bool isQuickTestMacro(const QByteArray ¯o); +QHash proFilesForQmlFiles(const Core::Id &id, const QStringList &files); +} // namespace QuickTestUtils } // namespace Internal } // namespace Autotest