From 791815e9bbbc4ad6abaf58dcced12d079e9b2703 Mon Sep 17 00:00:00 2001 From: firewave Date: Wed, 8 Mar 2023 13:54:47 +0100 Subject: [PATCH 01/24] reset timer results before each test --- Makefile | 2 +- lib/cppcheck.cpp | 5 +++++ lib/cppcheck.h | 2 ++ lib/timer.cpp | 6 ++++++ lib/timer.h | 2 ++ test/fixture.cpp | 2 ++ 6 files changed, 18 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2e421a3c1d1..f6119124d99 100644 --- a/Makefile +++ b/Makefile @@ -672,7 +672,7 @@ cli/stacktrace.o: cli/stacktrace.cpp cli/stacktrace.h lib/config.h lib/utils.h cli/threadexecutor.o: cli/threadexecutor.cpp cli/cppcheckexecutor.h cli/executor.h cli/threadexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/threadexecutor.cpp -test/fixture.o: test/fixture.cpp externals/tinyxml2/tinyxml2.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h test/fixture.h test/options.h test/redirect.h +test/fixture.o: test/fixture.cpp externals/tinyxml2/tinyxml2.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h test/fixture.h test/options.h test/redirect.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/fixture.cpp test/helpers.o: test/helpers.cpp cli/filelister.h externals/simplecpp/simplecpp.h lib/config.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/helpers.h diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 7d198f582f3..b83043e7afa 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -1903,3 +1903,8 @@ void CppCheck::removeCtuInfoFiles(const std::map &file } } } + +void CppCheck::resetTimerResults() +{ + s_timerResults.reset(); +} diff --git a/lib/cppcheck.h b/lib/cppcheck.h index fbe73eea1a8..f426e84eef0 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -143,6 +143,8 @@ class CPPCHECKLIB CppCheck : ErrorLogger { /** Remove *.ctu-info files */ void removeCtuInfoFiles(const std::map& files); // cppcheck-suppress functionConst // has side effects + static void resetTimerResults(); + private: #ifdef HAVE_RULES /** Are there "simple" rules */ diff --git a/lib/timer.cpp b/lib/timer.cpp index 73bb3f024bb..8f80957ffd4 100644 --- a/lib/timer.cpp +++ b/lib/timer.cpp @@ -93,6 +93,12 @@ void TimerResults::addResults(const std::string& str, std::clock_t clocks) mResults[str].mNumberOfResults++; } +void TimerResults::reset() +{ + std::lock_guard l(mResultsSync); + mResults.clear(); +} + Timer::Timer(std::string str, SHOWTIME_MODES showtimeMode, TimerResultsIntf* timerResults) : mStr(std::move(str)) , mTimerResults(timerResults) diff --git a/lib/timer.h b/lib/timer.h index 34a5216d8d3..f06e2e0774c 100644 --- a/lib/timer.h +++ b/lib/timer.h @@ -59,6 +59,8 @@ class CPPCHECKLIB TimerResults : public TimerResultsIntf { void showResults(SHOWTIME_MODES mode) const; void addResults(const std::string& str, std::clock_t clocks) override; + void reset(); + private: std::map mResults; mutable std::mutex mResultsSync; diff --git a/test/fixture.cpp b/test/fixture.cpp index bc0c930baa8..ad7c8371481 100644 --- a/test/fixture.cpp +++ b/test/fixture.cpp @@ -18,6 +18,7 @@ #include "fixture.h" +#include "cppcheck.h" #include "errortypes.h" #include "options.h" #include "redirect.h" @@ -90,6 +91,7 @@ bool TestFixture::prepareTest(const char testname[]) mVerbose = false; mTemplateFormat.clear(); mTemplateLocation.clear(); + CppCheck::resetTimerResults(); prepareTestInternal(); From f50a83bfb96171e89883d449ab4d074997a855d4 Mon Sep 17 00:00:00 2001 From: firewave Date: Wed, 8 Mar 2023 14:12:18 +0100 Subject: [PATCH 02/24] TestThreadExecutor: run tests with quiet flag --- test/testthreadexecutor.cpp | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/test/testthreadexecutor.cpp b/test/testthreadexecutor.cpp index 7dccdf51e56..501b9d24cd7 100644 --- a/test/testthreadexecutor.cpp +++ b/test/testthreadexecutor.cpp @@ -49,6 +49,7 @@ class TestThreadExecutor : public TestFixture { struct CheckOptions { CheckOptions() = default; + bool quiet = false; SHOWTIME_MODES showtime = SHOWTIME_MODES::SHOWTIME_NONE; const char* plistOutput = nullptr; std::vector filesList; @@ -80,6 +81,7 @@ class TestThreadExecutor : public TestFixture { Settings settings1 = settings; settings1.jobs = jobs; settings1.showtime = opt.showtime; + settings1.quiet = opt.quiet; if (opt.plistOutput) settings1.plistOutput = opt.plistOutput; // TODO: test with settings.project.fileSettings; @@ -114,16 +116,19 @@ class TestThreadExecutor : public TestFixture { oss << " return 0;\n" << "}\n"; - check(2, 3, 3, oss.str()); + check(2, 3, 3, oss.str(), dinit(CheckOptions, + $.quiet = true)); } + // TODO: check the output void many_threads() { check(16, 100, 100, "int main()\n" "{\n" " char *a = malloc(10);\n" " return 0;\n" - "}"); + "}", dinit(CheckOptions, + $.quiet = true)); } // #11249 - reports TSAN errors - only applies to threads not processes though @@ -134,7 +139,9 @@ class TestThreadExecutor : public TestFixture { "{\n" " char *a = malloc(10);\n" " return 0;\n" - "}", dinit(CheckOptions, $.showtime = SHOWTIME_MODES::SHOWTIME_SUMMARY)); + "}", dinit(CheckOptions, + $.quiet = true, + $.showtime = SHOWTIME_MODES::SHOWTIME_SUMMARY)); } void many_threads_plist() { @@ -146,7 +153,9 @@ class TestThreadExecutor : public TestFixture { "{\n" " char *a = malloc(10);\n" " return 0;\n" - "}", dinit(CheckOptions, $.plistOutput = plistOutput)); + "}", dinit(CheckOptions, + $.quiet = true, + $.plistOutput = plistOutput)); } void no_errors_more_files() { @@ -154,7 +163,8 @@ class TestThreadExecutor : public TestFixture { "int main()\n" "{\n" " return 0;\n" - "}"); + "}", dinit(CheckOptions, + $.quiet = true)); } void no_errors_less_files() { @@ -162,7 +172,8 @@ class TestThreadExecutor : public TestFixture { "int main()\n" "{\n" " return 0;\n" - "}"); + "}", dinit(CheckOptions, + $.quiet = true)); } void no_errors_equal_amount_files() { @@ -170,7 +181,8 @@ class TestThreadExecutor : public TestFixture { "int main()\n" "{\n" " return 0;\n" - "}"); + "}", dinit(CheckOptions, + $.quiet = true)); } void one_error_less_files() { @@ -179,7 +191,8 @@ class TestThreadExecutor : public TestFixture { "{\n" " {char *a = malloc(10);}\n" " return 0;\n" - "}"); + "}", dinit(CheckOptions, + $.quiet = true)); } void one_error_several_files() { @@ -188,7 +201,8 @@ class TestThreadExecutor : public TestFixture { "{\n" " {char *a = malloc(10);}\n" " return 0;\n" - "}"); + "}", dinit(CheckOptions, + $.quiet = true)); } void markup() { From b3f83a6366ac733f59c7906348310bc8f801a6a7 Mon Sep 17 00:00:00 2001 From: firewave Date: Wed, 8 Mar 2023 14:38:28 +0100 Subject: [PATCH 03/24] TimerResults: avoid multiple threads writing their results over each other --- lib/timer.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/timer.cpp b/lib/timer.cpp index 8f80957ffd4..d68c7dc603d 100644 --- a/lib/timer.cpp +++ b/lib/timer.cpp @@ -45,17 +45,18 @@ void TimerResults::showResults(SHOWTIME_MODES mode) const if (mode == SHOWTIME_MODES::SHOWTIME_NONE || mode == SHOWTIME_MODES::SHOWTIME_FILE_TOTAL) return; - std::cout << std::endl; TimerResultsData overallData; - std::vector data; - { - std::lock_guard l(mResultsSync); - data.reserve(mResults.size()); - data.insert(data.begin(), mResults.cbegin(), mResults.cend()); - } + + // lock the whole logging operation to avoid multiple threads printing their results at the same time + std::lock_guard l(mResultsSync); + + data.reserve(mResults.size()); + data.insert(data.begin(), mResults.cbegin(), mResults.cend()); std::sort(data.begin(), data.end(), more_second_sec); + std::cout << std::endl; + size_t ordinal = 1; // maybe it would be nice to have an ordinal in output later! for (std::vector::const_iterator iter=data.cbegin(); iter!=data.cend(); ++iter) { const double sec = iter->second.seconds(); From a9b382609cd1ae687d08c950ce5fda531d844fc6 Mon Sep 17 00:00:00 2001 From: firewave Date: Wed, 8 Mar 2023 14:38:54 +0100 Subject: [PATCH 04/24] TestThreadExecutor: added some `showtime` tests --- test/testthreadexecutor.cpp | 39 +++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/testthreadexecutor.cpp b/test/testthreadexecutor.cpp index 501b9d24cd7..87952d22ef2 100644 --- a/test/testthreadexecutor.cpp +++ b/test/testthreadexecutor.cpp @@ -105,6 +105,8 @@ class TestThreadExecutor : public TestFixture { TEST_CASE(one_error_less_files); TEST_CASE(one_error_several_files); TEST_CASE(markup); + TEST_CASE(showtime_top5); + TEST_CASE(showtime_file); } void deadlock_with_many_errors() { @@ -243,6 +245,43 @@ class TestThreadExecutor : public TestFixture { settings = settingsOld; } + + // TODO: provide data which actually shows values above 0 + + template + std::size_t find_all_of(const std::string& str, T sub) { + std::size_t n = 0; + std::string::size_type pos = 0; + while ((pos = str.find(sub, pos)) != std::string::npos) { + ++pos; + ++n; + } + return n; + } + + void showtime_top5() { + REDIRECT; // should not cause TSAN failures as the showtime logging is synchronized + check(2, 2, 0, + "int main() {}", + dinit(CheckOptions, + $.quiet = true, + $.showtime = SHOWTIME_MODES::SHOWTIME_TOP5)); + // for each file: top5 results + overall + empty line + const std::string output_s = GET_REDIRECT_OUTPUT; + ASSERT_EQUALS((5 + 1 + 1) * 2, find_all_of(output_s, '\n')); + } + + void showtime_file() { + REDIRECT; // should not cause TSAN failures as the showtime logging is synchronized + check(2, 2, 0, + "int main() {}", + dinit(CheckOptions, + $.quiet = true, + $.showtime = SHOWTIME_MODES::SHOWTIME_FILE)); + const std::string output_s = GET_REDIRECT_OUTPUT; + ASSERT_EQUALS(2, find_all_of(output_s, "Overall time:")); + } + // TODO: test clang-tidy // TODO: test whole program analysis }; From 61d6c2e0b1b1a0208ab5d763cf652aec57b21b51 Mon Sep 17 00:00:00 2001 From: firewave Date: Wed, 8 Mar 2023 14:40:08 +0100 Subject: [PATCH 05/24] actually only print the summary with `--showtime=summary` --- Makefile | 2 +- cli/threadexecutor.cpp | 8 +++++++- lib/cppcheck.cpp | 9 ++++++++- lib/cppcheck.h | 1 + releasenotes.txt | 3 ++- test/testthreadexecutor.cpp | 14 ++++++++++++++ 6 files changed, 33 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index f6119124d99..85f072a7b7a 100644 --- a/Makefile +++ b/Makefile @@ -669,7 +669,7 @@ cli/singleexecutor.o: cli/singleexecutor.cpp cli/executor.h cli/singleexecutor.h cli/stacktrace.o: cli/stacktrace.cpp cli/stacktrace.h lib/config.h lib/utils.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/stacktrace.cpp -cli/threadexecutor.o: cli/threadexecutor.cpp cli/cppcheckexecutor.h cli/executor.h cli/threadexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h +cli/threadexecutor.o: cli/threadexecutor.cpp cli/cppcheckexecutor.h cli/executor.h cli/threadexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/threadexecutor.cpp test/fixture.o: test/fixture.cpp externals/tinyxml2/tinyxml2.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h test/fixture.h test/options.h test/redirect.h diff --git a/cli/threadexecutor.cpp b/cli/threadexecutor.cpp index 96f35ac4015..6b82b655a8f 100644 --- a/cli/threadexecutor.cpp +++ b/cli/threadexecutor.cpp @@ -24,6 +24,7 @@ #include "errorlogger.h" #include "importproject.h" #include "settings.h" +#include "timer.h" #include #include @@ -191,7 +192,12 @@ unsigned int ThreadExecutor::check() } } - return std::accumulate(threadFutures.begin(), threadFutures.end(), 0U, [](unsigned int v, std::future& f) { + unsigned int result = std::accumulate(threadFutures.begin(), threadFutures.end(), 0U, [](unsigned int v, std::future& f) { return v + f.get(); }); + + if (mSettings.showtime == SHOWTIME_MODES::SHOWTIME_SUMMARY) + CppCheck::printTimerResultsSummary(); + + return result; } diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index b83043e7afa..61706411cb2 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -467,7 +467,9 @@ CppCheck::~CppCheck() delete mFileInfo.back(); mFileInfo.pop_back(); } - s_timerResults.showResults(mSettings.showtime); + + if (mSettings.showtime != SHOWTIME_MODES::SHOWTIME_SUMMARY) + s_timerResults.showResults(mSettings.showtime); if (mPlistFile.is_open()) { mPlistFile << ErrorLogger::plistFooter(); @@ -1908,3 +1910,8 @@ void CppCheck::resetTimerResults() { s_timerResults.reset(); } + +void CppCheck::printTimerResultsSummary() +{ + s_timerResults.showResults(SHOWTIME_MODES::SHOWTIME_SUMMARY); +} diff --git a/lib/cppcheck.h b/lib/cppcheck.h index f426e84eef0..bca5512c63b 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -144,6 +144,7 @@ class CPPCHECKLIB CppCheck : ErrorLogger { void removeCtuInfoFiles(const std::map& files); // cppcheck-suppress functionConst // has side effects static void resetTimerResults(); + static void printTimerResultsSummary(); private: #ifdef HAVE_RULES diff --git a/releasenotes.txt b/releasenotes.txt index 9967518840f..d0708a2e42c 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -14,4 +14,5 @@ Changed interface: Other: - Windows builds now default to the `native` platform instead of `win32A` or `win64`. Please specify it explicitly if you depedent on it. -- The undocumented and deprecated command-line options `--template