From aea4fd1068edcee9529c42af6fefc34f1e80580f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Tue, 12 Jul 2022 17:36:36 +0200 Subject: [PATCH] some `TestSuppressions` improvements and cleanups (#4271) --- Makefile | 2 +- cli/cppcheckexecutor.cpp | 26 +-- cli/cppcheckexecutor.h | 2 + cli/executor.cpp | 3 - cli/executor.h | 8 +- test/testsuppressions.cpp | 332 ++++++++++++++++++++---------------- test/testthreadexecutor.cpp | 4 - 7 files changed, 201 insertions(+), 176 deletions(-) diff --git a/Makefile b/Makefile index b514e5306..9d0d5cd44 100644 --- a/Makefile +++ b/Makefile @@ -731,7 +731,7 @@ test/testsuite.o: test/testsuite.cpp lib/color.h lib/config.h lib/errorlogger.h test/testsummaries.o: test/testsummaries.cpp 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/summaries.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testsummaries.o test/testsummaries.cpp -test/testsuppressions.o: test/testsuppressions.cpp 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/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h test/testutils.h +test/testsuppressions.o: test/testsuppressions.cpp cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.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/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h test/testutils.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testsuppressions.o test/testsuppressions.cpp test/testsymboldatabase.o: test/testsymboldatabase.cpp externals/tinyxml2/tinyxml2.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/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h test/testutils.h diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 1908be260..33b2a6f0b 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -869,6 +869,18 @@ int CppCheckExecutor::check_wrapper(CppCheck& cppcheck) #endif } +bool CppCheckExecutor::reportSuppressions(const Settings &settings, bool unusedFunctionCheckEnabled, const std::map &files, ErrorLogger& errorLogger) { + bool err = false; + if (settings.jointSuppressionReport) { + for (std::map::const_iterator i = files.begin(); i != files.end(); ++i) { + err |= errorLogger.reportUnmatchedSuppressions( + settings.nomsg.getUnmatchedLocalSuppressions(i->first, unusedFunctionCheckEnabled)); + } + } + err |= errorLogger.reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled)); + return err; +} + /* * That is a method which gets called from check_wrapper * */ @@ -975,8 +987,6 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) } if (cppcheck.analyseWholeProgram()) returnValue++; - } else if (!ThreadExecutor::isEnabled()) { - std::cout << "No thread support yet implemented for this platform." << std::endl; } else { #if defined(THREADING_MODEL_THREAD) ThreadExecutor executor(mFiles, settings, *this); @@ -989,17 +999,7 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) cppcheck.analyseWholeProgram(mSettings->buildDir, mFiles); if (settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) { - const bool enableUnusedFunctionCheck = cppcheck.isUnusedFunctionCheckEnabled(); - - if (settings.jointSuppressionReport) { - for (std::map::const_iterator i = mFiles.begin(); i != mFiles.end(); ++i) { - const bool err = reportUnmatchedSuppressions(settings.nomsg.getUnmatchedLocalSuppressions(i->first, enableUnusedFunctionCheck)); - if (err && returnValue == 0) - returnValue = settings.exitCode; - } - } - - const bool err = reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(enableUnusedFunctionCheck)); + const bool err = reportSuppressions(settings, cppcheck.isUnusedFunctionCheckEnabled(), mFiles, *this); if (err && returnValue == 0) returnValue = settings.exitCode; } diff --git a/cli/cppcheckexecutor.h b/cli/cppcheckexecutor.h index 6d437b275..dd7aaef9c 100644 --- a/cli/cppcheckexecutor.h +++ b/cli/cppcheckexecutor.h @@ -116,6 +116,8 @@ public: */ static bool executeCommand(std::string exe, std::vector args, const std::string &redirect, std::string *output_); + static bool reportSuppressions(const Settings &settings, bool unusedFunctionCheckEnabled, const std::map &files, ErrorLogger& errorLogger); + protected: /** diff --git a/cli/executor.cpp b/cli/executor.cpp index 762a39847..32424f767 100644 --- a/cli/executor.cpp +++ b/cli/executor.cpp @@ -25,6 +25,3 @@ Executor::Executor(const std::map &files, Settings &se Executor::~Executor() {} -bool Executor::isEnabled() { - return true; -} diff --git a/cli/executor.h b/cli/executor.h index 866097c0b..3ce02a372 100644 --- a/cli/executor.h +++ b/cli/executor.h @@ -37,17 +37,13 @@ class ErrorLogger; class Executor { public: Executor(const std::map &files, Settings &settings, ErrorLogger &errorLogger); - Executor(const Executor &) = delete; virtual ~Executor(); + + Executor(const Executor &) = delete; void operator=(const Executor &) = delete; virtual unsigned int check() = 0; - /** - * @return true if support for threads exist. - */ - static bool isEnabled(); - protected: const std::map &mFiles; Settings &mSettings; diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index 98ae2e29d..f33633bdd 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -18,7 +18,9 @@ #include "config.h" #include "cppcheck.h" +#include "cppcheckexecutor.h" #include "errortypes.h" +#include "processexecutor.h" #include "settings.h" #include "suppressions.h" #include "testsuite.h" @@ -50,6 +52,10 @@ private: TEST_CASE(suppressionsGlob); TEST_CASE(suppressionsFileNameWithExtraPath); TEST_CASE(suppressionsSettings); + TEST_CASE(suppressionsSettingsThreads); +#if !defined(WIN32) && !defined(__MINGW32__) && !defined(__CYGWIN__) + TEST_CASE(suppressionsSettingsProcesses); +#endif TEST_CASE(suppressionsMultiFile); TEST_CASE(suppressionsPathSeparator); TEST_CASE(suppressionsLine0); @@ -162,19 +168,6 @@ private: ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "x/../a.c", 123))); } - void reportSuppressions(const Settings &settings, const std::map &files) { - // make it verbose that this check is disabled - const bool unusedFunctionCheck = false; - - if (settings.jointSuppressionReport) { - for (std::map::const_iterator i = files.begin(); i != files.end(); ++i) { - reportUnmatchedSuppressions(settings.nomsg.getUnmatchedLocalSuppressions(i->first, unusedFunctionCheck)); - } - } - - reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(unusedFunctionCheck)); - } - // Check the suppression unsigned int checkSuppression(const char code[], const std::string &suppression = emptyString) { std::map files; @@ -208,7 +201,11 @@ private: if (cppCheck.analyseWholeProgram()) exitCode |= settings.exitCode; - reportSuppressions(settings, files); + std::map files_for_report; + for (std::map::const_iterator file = files.begin(); file != files.end(); ++file) + files_for_report[file->first] = file->second.size(); + + CppCheckExecutor::reportSuppressions(settings, false, files_for_report, *this); return exitCode; } @@ -235,48 +232,74 @@ private: const unsigned int exitCode = executor.check(); - std::map files_for_report; - for (std::map::const_iterator file = files.begin(); file != files.end(); ++file) - files_for_report[file->first] = ""; - - reportSuppressions(settings, files_for_report); + CppCheckExecutor::reportSuppressions(settings, false, files, *this); return exitCode; } +#if !defined(WIN32) && !defined(__MINGW32__) && !defined(__CYGWIN__) + unsigned int checkSuppressionProcesses(const char code[], const std::string &suppression = emptyString) { + errout.str(""); + output.str(""); + + std::map files; + files["test.cpp"] = strlen(code); + + Settings settings; + settings.jobs = 1; + settings.inlineSuppressions = true; + settings.severity.enable(Severity::information); + if (!suppression.empty()) { + EXPECT_EQ("", settings.nomsg.addSuppressionLine(suppression)); + } + ProcessExecutor executor(files, settings, *this); + std::vector scopedfiles; + scopedfiles.reserve(files.size()); + for (std::map::const_iterator i = files.begin(); i != files.end(); ++i) + scopedfiles.emplace_back(i->first, code); + + const unsigned int exitCode = executor.check(); + + CppCheckExecutor::reportSuppressions(settings, false, files, *this); + + return exitCode; + } +#endif + void runChecks(unsigned int (TestSuppressions::*check)(const char[], const std::string &)) { // check to make sure the appropriate error is present - (this->*check)("void f() {\n" - " int a;\n" - " a++;\n" - "}\n", - ""); + ASSERT_EQUALS(1, (this->*check)("void f() {\n" + " int a;\n" + " a++;\n" + "}\n", + "")); ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: a\n", errout.str()); // suppress uninitvar globally - (this->*check)("void f() {\n" - " int a;\n" - " a++;\n" - "}\n", - "uninitvar"); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " a++;\n" + "}\n", + "uninitvar")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar globally, without error present - (this->*check)("void f() {\n" - " int a;\n" - " b++;\n" - "}\n", - "uninitvar"); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " b++;\n" + "}\n", + "uninitvar")); ASSERT_EQUALS("(information) Unmatched suppression: uninitvar\n", errout.str()); // suppress uninitvar for this file only - (this->*check)("void f() {\n" - " int a;\n" - " a++;\n" - "}\n", - "uninitvar:test.cpp"); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " a++;\n" + "}\n", + "uninitvar:test.cpp")); ASSERT_EQUALS("", errout.str()); + // TODO: add assert - gives different result with threads/processes // suppress uninitvar for this file only, without error present (this->*check)("void f() {\n" " int a;\n" @@ -286,13 +309,14 @@ private: ASSERT_EQUALS("[test.cpp]: (information) Unmatched suppression: uninitvar\n", errout.str()); // suppress all for this file only - (this->*check)("void f() {\n" - " int a;\n" - " a++;\n" - "}\n", - "*:test.cpp"); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " a++;\n" + "}\n", + "*:test.cpp")); ASSERT_EQUALS("", errout.str()); + // TODO: add assert - gives different result with threads/processes // suppress all for this file only, without error present (this->*check)("void f() {\n" " int a;\n" @@ -302,13 +326,14 @@ private: ASSERT_EQUALS("[test.cpp]: (information) Unmatched suppression: *\n", errout.str()); // suppress uninitvar for this file and line - (this->*check)("void f() {\n" - " int a;\n" - " a++;\n" - "}\n", - "uninitvar:test.cpp:3"); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " a++;\n" + "}\n", + "uninitvar:test.cpp:3")); ASSERT_EQUALS("", errout.str()); + // TODO: add assert - gives different result with threads/processes // suppress uninitvar for this file and line, without error present (this->*check)("void f() {\n" " int a;\n" @@ -318,126 +343,127 @@ private: ASSERT_EQUALS("[test.cpp:3]: (information) Unmatched suppression: uninitvar\n", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " // cppcheck-suppress uninitvar\n" - " a++;\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " // cppcheck-suppress uninitvar\n" + " a++;\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " // cppcheck-suppress uninitvar\n" - "\n" - " a++;\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " // cppcheck-suppress uninitvar\n" + "\n" + " a++;\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " a++;// cppcheck-suppress uninitvar\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " a++;// cppcheck-suppress uninitvar\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " /* cppcheck-suppress uninitvar */\n" - " a++;\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " /* cppcheck-suppress uninitvar */\n" + " a++;\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " /* cppcheck-suppress uninitvar */\n" - "\n" - " a++;\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " /* cppcheck-suppress uninitvar */\n" + "\n" + " a++;\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " a++;/* cppcheck-suppress uninitvar */\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " a++;/* cppcheck-suppress uninitvar */\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " // cppcheck-suppress[uninitvar]\n" - " a++;\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " // cppcheck-suppress[uninitvar]\n" + " a++;\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " // cppcheck-suppress[uninitvar]\n" - " a++;\n" - "\n" - " a++;\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " // cppcheck-suppress[uninitvar]\n" + " a++;\n" + "\n" + " a++;\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " a++;// cppcheck-suppress[uninitvar]\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " a++;// cppcheck-suppress[uninitvar]\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " /* cppcheck-suppress[uninitvar]*/\n" - " a++;\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " /* cppcheck-suppress[uninitvar]*/\n" + " a++;\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " /* cppcheck-suppress[uninitvar]*/\n" - "\n" - " a++;\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " /* cppcheck-suppress[uninitvar]*/\n" + "\n" + " a++;\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline - (this->*check)("void f() {\n" - " int a;\n" - " a++;/* cppcheck-suppress[uninitvar]*/\n" - "}\n", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " int a;\n" + " a++;/* cppcheck-suppress[uninitvar]*/\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); // suppress uninitvar inline, with asm before (#6813) - (this->*check)("void f() {\n" - " __asm {\n" - " foo\n" - " }" - " int a;\n" - " // cppcheck-suppress uninitvar\n" - " a++;\n" - "}", - ""); + ASSERT_EQUALS(0, (this->*check)("void f() {\n" + " __asm {\n" + " foo\n" + " }" + " int a;\n" + " // cppcheck-suppress uninitvar\n" + " a++;\n" + "}", + "")); ASSERT_EQUALS("", errout.str()); + // TODO: add assert - gives different result with threads/processes // suppress uninitvar inline, without error present (this->*check)("void f() {\n" " int a;\n" @@ -462,10 +488,18 @@ private: void suppressionsSettings() { runChecks(&TestSuppressions::checkSuppression); - if (ThreadExecutor::isEnabled()) - runChecks(&TestSuppressions::checkSuppressionThreads); } + void suppressionsSettingsThreads() { + runChecks(&TestSuppressions::checkSuppressionThreads); + } + +#if !defined(WIN32) && !defined(__MINGW32__) && !defined(__CYGWIN__) + void suppressionsSettingsProcesses() { + runChecks(&TestSuppressions::checkSuppressionProcesses); + } +#endif + void suppressionsMultiFile() { std::map files; files["abc.cpp"] = "void f() {\n" @@ -476,7 +510,7 @@ private: "}\n"; // suppress uninitvar for this file and line - checkSuppression(files, "uninitvar:xyz.cpp:3"); + ASSERT_EQUALS(0, checkSuppression(files, "uninitvar:xyz.cpp:3")); ASSERT_EQUALS("", errout.str()); } @@ -541,20 +575,20 @@ private: } void inlinesuppress_symbolname() { - checkSuppression("void f() {\n" - " int a;\n" - " /* cppcheck-suppress uninitvar symbolName=a */\n" - " a++;\n" - "}\n", - ""); + ASSERT_EQUALS(0, checkSuppression("void f() {\n" + " int a;\n" + " /* cppcheck-suppress uninitvar symbolName=a */\n" + " a++;\n" + "}\n", + "")); ASSERT_EQUALS("", errout.str()); - checkSuppression("void f() {\n" - " int a,b;\n" - " /* cppcheck-suppress uninitvar symbolName=b */\n" - " a++; b++;\n" - "}\n", - ""); + ASSERT_EQUALS(1, checkSuppression("void f() {\n" + " int a,b;\n" + " /* cppcheck-suppress uninitvar symbolName=b */\n" + " a++; b++;\n" + "}\n", + "")); ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: a\n", errout.str()); } @@ -713,7 +747,7 @@ private: std::map files; files["test.cpp"] = "if if\n"; - checkSuppression(files, "syntaxError:test.cpp:1"); + ASSERT_EQUALS(0, checkSuppression(files, "syntaxError:test.cpp:1")); ASSERT_EQUALS("", errout.str()); } @@ -729,7 +763,7 @@ private: " fstp QWORD PTR result ; store a double (8 bytes)\n" " pop EAX ; restore EAX\n" "}"; - checkSuppression(files, ""); + ASSERT_EQUALS(0, checkSuppression(files, "")); ASSERT_EQUALS("", errout.str()); } @@ -748,7 +782,7 @@ private: "[!VAR \"BC\" = \"$BC + 1\"!][!//\n" "[!ENDIF!][!//\n" "};"; - checkSuppression(files, "syntaxError:test.cpp:4"); + ASSERT_EQUALS(0, checkSuppression(files, "syntaxError:test.cpp:4")); ASSERT_EQUALS("", errout.str()); } diff --git a/test/testthreadexecutor.cpp b/test/testthreadexecutor.cpp index 6eab621d5..a2a9fb8dc 100644 --- a/test/testthreadexecutor.cpp +++ b/test/testthreadexecutor.cpp @@ -43,10 +43,6 @@ private: void check(unsigned int jobs, int files, int result, const std::string &data) { errout.str(""); output.str(""); - if (!ThreadExecutor::isEnabled()) { - // Skip this check on systems which don't use this feature - return; - } std::map filemap; for (int i = 1; i <= files; ++i) {