Skip to content

Commit 5d56ab9

Browse files
firewavefirewave
committed
refs #13982 - cppcheck.cpp: do not crash if mExecuteCommand is empty
1 parent 0eb4dd8 commit 5d56ab9

4 files changed

Lines changed: 81 additions & 7 deletions

File tree

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ test/testcondition.o: test/testcondition.cpp lib/addoninfo.h lib/check.h lib/che
753753
test/testconstructors.o: test/testconstructors.cpp lib/addoninfo.h lib/check.h lib/checkclass.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
754754
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testconstructors.cpp
755755

756-
test/testcppcheck.o: test/testcppcheck.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
756+
test/testcppcheck.o: test/testcppcheck.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h test/redirect.h
757757
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testcppcheck.cpp
758758

759759
test/testerrorlogger.o: test/testerrorlogger.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/xml.h test/fixture.h test/helpers.h

‎lib/cppcheck.cpp‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,9 @@ static std::vector<picojson::value> executeAddon(const AddonInfo &addonInfo,
432432
const std::string &premiumArgs,
433433
const CppCheck::ExecuteCmdFn &executeCommand)
434434
{
435+
if (!executeCommand)
436+
throw InternalError(nullptr, "Failed to execute addon - no command callback provided");
437+
435438
std::string pythonExe;
436439

437440
if (!addonInfo.executable.empty())
@@ -686,6 +689,11 @@ unsigned int CppCheck::checkClang(const FileWithDetails &file, int fileIndex)
686689
mErrorLogger.reportOut(exe + " " + args2, Color::Reset);
687690
}
688691

692+
if (!mExecuteCommand) {
693+
std::cerr << "Failed to execute '" << exe << " " << args2 << " " << redirect2 << "' - (no command callback provided)" << std::endl;
694+
return 0; // TODO: report as failure?
695+
}
696+
689697
std::string output2;
690698
const int exitcode = mExecuteCommand(exe,split(args2),redirect2,output2);
691699
if (mSettings.debugClangOutput) {
@@ -1954,6 +1962,11 @@ void CppCheck::analyseClangTidy(const FileSettings &fileSettings)
19541962
}
19551963
#endif
19561964

1965+
if (!mExecuteCommand) {
1966+
std::cerr << "Failed to execute '" << exe << "' (no command callback provided)" << std::endl;
1967+
return;
1968+
}
1969+
19571970
// TODO: log this call
19581971
// TODO: get rid of hard-coded checks
19591972
const std::string args = "-quiet -checks=*,-clang-analyzer-*,-llvm* \"" + fileSettings.filename() + "\" -- " + allIncludes + allDefines;

‎lib/cppcheck.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class CPPCHECKLIB CppCheck {
6060
friend class TestCppcheck;
6161

6262
public:
63+
// exe, args, redirect, output_
6364
using ExecuteCmdFn = std::function<int (std::string,std::vector<std::string>,std::string,std::string&)>;
6465

6566
/**

‎test/testcppcheck.cpp‎

Lines changed: 66 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "helpers.h"
2626
#include "path.h"
2727
#include "preprocessor.h"
28+
#include "redirect.h"
2829
#include "settings.h"
2930
#include "standards.h"
3031
#include "suppressions.h"
@@ -66,7 +67,11 @@ class TestCppcheck : public TestFixture {
6667
void run() override {
6768
TEST_CASE(getErrorMessages);
6869
TEST_CASE(checkWithFile);
70+
TEST_CASE(checkWithFileWithTidy);
71+
TEST_CASE(checkWithFileWithTidyNoCommand);
6972
TEST_CASE(checkWithFS);
73+
TEST_CASE(checkWithFSWithTidy);
74+
TEST_CASE(checkWithFSWithTidyNoCommand);
7075
TEST_CASE(suppress_error_library);
7176
TEST_CASE(unique_errors);
7277
TEST_CASE(unique_errors_2);
@@ -114,41 +119,81 @@ class TestCppcheck : public TestFixture {
114119
ASSERT(foundMissingIncludeSystem);
115120
}
116121

117-
void checkWithFile() const
122+
void checkWithFileInternal(bool tidy, bool nocmd = false) const
118123
{
124+
REDIRECT;
119125
ScopedFile file("test.cpp",
120126
"int main()\n"
121127
"{\n"
122128
" int i = *((int*)0);\n"
123129
" return 0;\n"
124130
"}");
125131

126-
const auto s = dinit(Settings, $.templateFormat = templateFormat);
132+
bool called = false;
133+
const auto s = dinit(Settings, $.templateFormat = templateFormat, $.clangTidy = true);
127134
Suppressions supprs;
128135
ErrorLogger2 errorLogger;
129-
CppCheck cppcheck(s, supprs, errorLogger, false, {});
136+
CppCheck::ExecuteCmdFn f;
137+
if (tidy && !nocmd) {
138+
// NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature
139+
f = [&](std::string exe, std::vector<std::string> /*args*/, std::string /*redirect*/, std::string& /*output*/) -> int {
140+
ASSERT_EQUALS("clang-tidy", exe);
141+
called = true;
142+
return 0;
143+
};
144+
}
145+
CppCheck cppcheck(s, supprs, errorLogger, false, f);
130146
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(file.path(), Path::identify(file.path(), false), 0)));
131147
// TODO: how to properly disable these warnings?
132148
errorLogger.ids.erase(std::remove_if(errorLogger.ids.begin(), errorLogger.ids.end(), [](const std::string& id) {
133149
return id == "logChecker";
134150
}), errorLogger.ids.end());
135151
ASSERT_EQUALS(1, errorLogger.ids.size());
136152
ASSERT_EQUALS("nullPointer", *errorLogger.ids.cbegin());
153+
// TODO: clang-tidy is currently not invoked with file inputs - see #12053
154+
#if 0
155+
ASSERT_EQUALS(called, tidy && !nocmd);
156+
if (tidy && nocmd)
157+
ASSERT_EQUALS("Failed to execute 'clang-tidy' (no command callback provided)\n", GET_REDIRECT_ERROUT);
158+
#endif
159+
}
160+
161+
void checkWithFile() {
162+
checkWithFileInternal(false);
163+
}
164+
165+
void checkWithFileWithTidy() {
166+
checkWithFileInternal(true);
167+
}
168+
169+
void checkWithFileWithTidyNoCommand() {
170+
checkWithFileInternal(true, true);
137171
}
138172

139-
void checkWithFS() const
173+
void checkWithFSInternal(bool tidy, bool nocmd = false) const
140174
{
175+
REDIRECT;
141176
ScopedFile file("test.cpp",
142177
"int main()\n"
143178
"{\n"
144179
" int i = *((int*)0);\n"
145180
" return 0;\n"
146181
"}");
147182

148-
const auto s = dinit(Settings, $.templateFormat = templateFormat);
183+
bool called = false;
184+
const auto s = dinit(Settings, $.templateFormat = templateFormat, $.clangTidy = tidy);
149185
Suppressions supprs;
150186
ErrorLogger2 errorLogger;
151-
CppCheck cppcheck(s, supprs, errorLogger, false, {});
187+
CppCheck::ExecuteCmdFn f;
188+
if (tidy && !nocmd) {
189+
// NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature
190+
f = [&](std::string exe, std::vector<std::string> /*args*/, std::string /*redirect*/, std::string& /*output*/) -> int {
191+
ASSERT_EQUALS("clang-tidy", exe);
192+
called = true;
193+
return 0;
194+
};
195+
}
196+
CppCheck cppcheck(s, supprs, errorLogger, false, f);
152197
FileSettings fs{file.path(), Path::identify(file.path(), false), 0};
153198
ASSERT_EQUALS(1, cppcheck.check(fs));
154199
// TODO: how to properly disable these warnings?
@@ -157,6 +202,21 @@ class TestCppcheck : public TestFixture {
157202
}), errorLogger.ids.end());
158203
ASSERT_EQUALS(1, errorLogger.ids.size());
159204
ASSERT_EQUALS("nullPointer", *errorLogger.ids.cbegin());
205+
ASSERT_EQUALS(called, tidy && !nocmd);
206+
if (tidy && nocmd)
207+
ASSERT_EQUALS("Failed to execute 'clang-tidy' (no command callback provided)\n", GET_REDIRECT_ERROUT);
208+
}
209+
210+
void checkWithFS() {
211+
checkWithFSInternal(false);
212+
}
213+
214+
void checkWithFSWithTidy() {
215+
checkWithFSInternal(true);
216+
}
217+
218+
void checkWithFSWithTidyNoCommand() {
219+
checkWithFSInternal(true, true);
160220
}
161221

162222
void suppress_error_library() const

0 commit comments

Comments
 (0)