Skip to content

make sure executors are only used in the intended context / TestSuppressions cleanups #4963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 28, 2023
Merged

make sure executors are only used in the intended context / TestSuppressions cleanups #4963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d37e9d9
added asserts to make sure executors are only used in the intended co…
firewave Apr 5, 2023
406dd78
TestSuppressions: specify proper job counts in `checkSuppression*()`
firewave Apr 14, 2023
153f88f
TestSuppressions: enabled all asserts in `runChecks()`
firewave Apr 14, 2023
48a85ab
TestSuppressions: removed unnecessary setting from `checkSuppression()`
firewave Apr 14, 2023
d8adf78
TestSuppressions: small cleanup in the way tests are called
firewave Apr 14, 2023
a9199e2
TestSuppressions: use `SingleExecutor`
firewave Apr 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ test/teststring.o: test/teststring.cpp lib/check.h lib/checkstring.h lib/color.h
test/testsummaries.o: test/testsummaries.cpp 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/summaries.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsummaries.cpp

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/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
test/testsuppressions.o: test/testsuppressions.cpp cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h cli/singleexecutor.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/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsuppressions.cpp

test/testsymboldatabase.o: test/testsymboldatabase.cpp 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/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
Expand Down
5 changes: 4 additions & 1 deletion cli/processexecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

#include <algorithm>
#include <numeric>
#include <cassert>
#include <cerrno>
#include <csignal>
#include <cstdlib>
Expand Down Expand Up @@ -61,7 +62,9 @@ using std::memset;

ProcessExecutor::ProcessExecutor(const std::map<std::string, std::size_t> &files, Settings &settings, ErrorLogger &errorLogger)
: Executor(files, settings, errorLogger)
{}
{
assert(mSettings.jobs > 1);
}

ProcessExecutor::~ProcessExecutor()
{}
Expand Down
5 changes: 4 additions & 1 deletion cli/singleexecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "library.h"
#include "settings.h"

#include <cassert>
#include <list>
#include <numeric>
#include <utility>
Expand All @@ -32,7 +33,9 @@ class ErrorLogger;
SingleExecutor::SingleExecutor(CppCheck &cppcheck, const std::map<std::string, std::size_t> &files, Settings &settings, ErrorLogger &errorLogger)
: Executor(files, settings, errorLogger)
, mCppcheck(cppcheck)
{}
{
assert(mSettings.jobs == 1);
}

SingleExecutor::~SingleExecutor()
{}
Expand Down
5 changes: 4 additions & 1 deletion cli/threadexecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "settings.h"

#include <algorithm>
#include <cassert>
#include <cstdlib>
#include <functional>
#include <future>
Expand All @@ -41,7 +42,9 @@ enum class Color;

ThreadExecutor::ThreadExecutor(const std::map<std::string, std::size_t> &files, Settings &settings, ErrorLogger &errorLogger)
: Executor(files, settings, errorLogger)
{}
{
assert(mSettings.jobs > 1);
}

ThreadExecutor::~ThreadExecutor()
{}
Expand Down
93 changes: 42 additions & 51 deletions test/testsuppressions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "fixture.h"
#include "helpers.h"
#include "threadexecutor.h"
#include "singleexecutor.h"

#include <algorithm>
#include <cstddef>
Expand Down Expand Up @@ -178,35 +179,35 @@ class TestSuppressions : public TestFixture {
}

// Check the suppression for multiple files
unsigned int checkSuppression(std::map<std::string, std::string> &files, const std::string &suppression = emptyString) {
unsigned int checkSuppression(std::map<std::string, std::string> &f, const std::string &suppression = emptyString) {
// Clear the error log
errout.str("");
output.str("");

std::map<std::string, std::size_t> files;
for (std::map<std::string, std::string>::const_iterator i = f.cbegin(); i != f.cend(); ++i) {
files[i->first] = i->second.size();
}

CppCheck cppCheck(*this, true, nullptr);
Settings& settings = cppCheck.settings();
settings.exitCode = 1;
settings.jobs = 1;
settings.inlineSuppressions = true;
settings.severity.enable(Severity::information);
if (suppression == "unusedFunction")
settings.checks.setEnabled(Checks::unusedFunction, true);
settings.severity.enable(Severity::information);
settings.jobs = 1;
if (!suppression.empty()) {
std::string r = settings.nomsg.addSuppressionLine(suppression);
EXPECT_EQ("", r);
EXPECT_EQ("", settings.nomsg.addSuppressionLine(suppression));
}
SingleExecutor executor(cppCheck, files, settings, *this);
std::vector<std::unique_ptr<ScopedFile>> scopedfiles;
scopedfiles.reserve(files.size());
for (std::map<std::string, std::string>::const_iterator i = f.cbegin(); i != f.cend(); ++i)
scopedfiles.emplace_back(new ScopedFile(i->first, i->second));

unsigned int exitCode = std::accumulate(files.cbegin(), files.cend(), 0U, [&](unsigned int v, const std::pair<std::string, std::string>& f) {
return v | cppCheck.check(f.first, f.second);
});

if (cppCheck.analyseWholeProgram())
exitCode |= settings.exitCode;

std::map<std::string, std::size_t> files_for_report;
for (std::map<std::string, std::string>::const_iterator file = files.cbegin(); file != files.cend(); ++file)
files_for_report[file->first] = file->second.size();
const unsigned int exitCode = executor.check();

CppCheckExecutor::reportSuppressions(settings, false, files_for_report, *this);
CppCheckExecutor::reportSuppressions(settings, false, files, *this);

return exitCode;
}
Expand All @@ -219,7 +220,7 @@ class TestSuppressions : public TestFixture {
files["test.cpp"] = strlen(code);

Settings settings;
settings.jobs = 1;
settings.jobs = 2;
settings.inlineSuppressions = true;
settings.severity.enable(Severity::information);
if (!suppression.empty()) {
Expand Down Expand Up @@ -247,7 +248,7 @@ class TestSuppressions : public TestFixture {
files["test.cpp"] = strlen(code);

Settings settings;
settings.jobs = 1;
settings.jobs = 2;
settings.inlineSuppressions = true;
settings.severity.enable(Severity::information);
if (!suppression.empty()) {
Expand Down Expand Up @@ -298,9 +299,8 @@ class TestSuppressions : public TestFixture {
" a++;\n"
"}\n",
"uninitvar:test.cpp"));
//TODO_ASSERT_EQUALS("", "[test.cpp]: (information) Unmatched suppression: uninitvar\n", errout.str());
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"
Expand All @@ -315,9 +315,8 @@ class TestSuppressions : public TestFixture {
" a++;\n"
"}\n",
"*:test.cpp"));
//TODO_ASSERT_EQUALS("", "[test.cpp]: (information) Unmatched suppression: *\n", errout.str());
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"
Expand All @@ -334,14 +333,13 @@ class TestSuppressions : public TestFixture {
"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"
" b++;\n"
"}\n",
"uninitvar:test.cpp:3");
//TODO_ASSERT_EQUALS("[test.cpp:3]: (information) Unmatched suppression: uninitvar\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (information) Unmatched suppression: uninitvar\n", errout.str());

// suppress uninitvar inline
ASSERT_EQUALS(0, (this->*check)("void f() {\n"
Expand Down Expand Up @@ -464,15 +462,14 @@ class TestSuppressions : public TestFixture {
""));
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"
" // cppcheck-suppress uninitvar\n"
" b++;\n"
"}\n",
"");
//TODO_ASSERT_EQUALS("[test.cpp:4]: (information) Unmatched suppression: uninitvar\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (information) Unmatched suppression: uninitvar\n", errout.str());

// #5746 - exitcode
ASSERT_EQUALS(1U,
Expand Down Expand Up @@ -745,16 +742,14 @@ class TestSuppressions : public TestFixture {
}

void suppressingSyntaxErrors() { // syntaxErrors should be suppressible (#7076)
std::map<std::string, std::string> files;
files["test.cpp"] = "if if\n";
const char code[] = "if if\n";

ASSERT_EQUALS(0, checkSuppression(files, "syntaxError:test.cpp:1"));
ASSERT_EQUALS(0, checkSuppression(code, "syntaxError:test.cpp:1"));
ASSERT_EQUALS("", errout.str());
}

void suppressingSyntaxErrorsInline() { // syntaxErrors should be suppressible (#5917)
std::map<std::string, std::string> files;
files["test.cpp"] = "double result(0.0);\n"
const char code[] = "double result(0.0);\n"
"_asm\n"
"{\n"
" // cppcheck-suppress syntaxError\n"
Expand All @@ -764,13 +759,12 @@ class TestSuppressions : public TestFixture {
" fstp QWORD PTR result ; store a double (8 bytes)\n"
" pop EAX ; restore EAX\n"
"}";
ASSERT_EQUALS(0, checkSuppression(files, ""));
ASSERT_EQUALS(0, checkSuppression(code, ""));
ASSERT_EQUALS("", errout.str());
}

void suppressingSyntaxErrorsWhileFileRead() { // syntaxError while file read should be suppressible (PR #1333)
std::map<std::string, std::string> files;
files["test.cpp"] = "CONST (genType, KS_CONST) genService[KS_CFG_NR_OF_NVM_BLOCKS] =\n"
const char code[] = "CONST (genType, KS_CONST) genService[KS_CFG_NR_OF_NVM_BLOCKS] =\n"
"{\n"
"[!VAR \"BC\" = \"$BC + 1\"!][!//\n"
"[!IF \"(as:modconf('Ks')[1]/KsGeneral/KsType = 'KS_CFG_TYPE_KS_MASTER') and\n"
Expand All @@ -783,7 +777,7 @@ class TestSuppressions : public TestFixture {
"[!VAR \"BC\" = \"$BC + 1\"!][!//\n"
"[!ENDIF!][!//\n"
"};";
ASSERT_EQUALS(0, checkSuppression(files, "syntaxError:test.cpp:4"));
ASSERT_EQUALS(0, checkSuppression(code, "syntaxError:test.cpp:4"));
ASSERT_EQUALS("", errout.str());
}

Expand Down Expand Up @@ -813,34 +807,31 @@ class TestSuppressions : public TestFixture {
}

void suppressingSyntaxErrorAndExitCode() {
std::map<std::string, std::string> files;
files["test.cpp"] = "fi if;";
const char code[] = "fi if;";

ASSERT_EQUALS(0, checkSuppression(files, "*:test.cpp"));
ASSERT_EQUALS(0, checkSuppression(code, "*:test.cpp"));
ASSERT_EQUALS("", errout.str());

// multi files, but only suppression one
std::map<std::string, std::string> mfiles;
mfiles["test.cpp"] = "fi if;";
mfiles["test2.cpp"] = "fi if";
ASSERT_EQUALS(1, checkSuppression(mfiles, "*:test.cpp"));
ASSERT_EQUALS(2, checkSuppression(mfiles, "*:test.cpp"));
ASSERT_EQUALS("[test2.cpp:1]: (error) syntax error\n", errout.str());

// multi error in file, but only suppression one error
std::map<std::string, std::string> file2;
file2["test.cpp"] = "fi fi\n"
"if if;";
ASSERT_EQUALS(1, checkSuppression(file2, "*:test.cpp:1")); // suppress all error at line 1 of test.cpp
const char code2[] = "fi fi\n"
"if if;";
ASSERT_EQUALS(2, checkSuppression(code2, "*:test.cpp:1")); // suppress all error at line 1 of test.cpp
ASSERT_EQUALS("[test.cpp:2]: (error) syntax error\n", errout.str());

// multi error in file, but only suppression one error (2)
std::map<std::string, std::string> file3;
file3["test.cpp"] = "void f(int x, int y){\n"
" int a = x/0;\n"
" int b = y/0;\n"
"}\n"
"f(0, 1);\n";
ASSERT_EQUALS(1, checkSuppression(file3, "zerodiv:test.cpp:3")); // suppress 'errordiv' at line 3 of test.cpp
const char code3[] = "void f(int x, int y){\n"
" int a = x/0;\n"
" int b = y/0;\n"
"}\n"
"f(0, 1);\n";
ASSERT_EQUALS(2, checkSuppression(code3, "zerodiv:test.cpp:3")); // suppress 'errordiv' at line 3 of test.cpp
}

};
Expand Down