diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 041c051dc..b16c6ff7f 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -692,6 +692,12 @@ bool CppCheckExecutor::executeCommand(std::string exe, std::vector { output_.clear(); +#ifdef _WIN32 + // Extra quoutes are needed in windows if filename has space + if (exe.find(" ") != std::string::npos) + exe = "\"" + exe + "\""; +#endif + std::string joinedArgs; for (const std::string &arg : args) { if (!joinedArgs.empty()) @@ -702,21 +708,34 @@ bool CppCheckExecutor::executeCommand(std::string exe, std::vector joinedArgs += arg; } + const std::string cmd = exe + " " + joinedArgs + " " + redirect; + #ifdef _WIN32 - // Extra quoutes are needed in windows if filename has space - if (exe.find(" ") != std::string::npos) - exe = "\"" + exe + "\""; - const std::string cmd = exe + " " + joinedArgs + " " + redirect; - std::unique_ptr pipe(_popen(cmd.c_str(), "r"), _pclose); + FILE* p = _popen(cmd.c_str(), "r"); #else - const std::string cmd = exe + " " + joinedArgs + " " + redirect; - std::unique_ptr pipe(popen(cmd.c_str(), "r"), pclose); + FILE *p = popen(cmd.c_str(), "r"); #endif - if (!pipe) + if (!p) { + // TODO: read errno return false; + } char buffer[1024]; - while (fgets(buffer, sizeof(buffer), pipe.get()) != nullptr) + while (fgets(buffer, sizeof(buffer), p) != nullptr) output_ += buffer; + +#ifdef _WIN32 + const int res = _pclose(p); +#else + const int res = pclose(p); +#endif + if (res == -1) { // error occured + // TODO: read errno + return false; + } + if (res != 0) { // process failed + // TODO: need to get error details + return false; + } return true; } diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index af13e04f7..e9fc037dd 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -352,6 +352,14 @@ static std::string executeAddon(const AddonInfo &addonInfo, #endif for (const char* py_exe : py_exes) { std::string out; +#ifdef _MSC_VER + // FIXME: hack to avoid debug assertion with _popen() in executeCommand() for non-existing commands + const std::string cmd = std::string(py_exe) + " --version >NUL"; + if (system(cmd.c_str()) != 0) { + // TODO: get more detailed error? + break; + } +#endif if (executeCommand(py_exe, split("--version"), redirect, out) && out.compare(0, 7, "Python ") == 0 && std::isdigit(out[7])) { pythonExe = py_exe; break; @@ -373,7 +381,7 @@ static std::string executeAddon(const AddonInfo &addonInfo, std::string result; if (!executeCommand(pythonExe, split(args), redirect, result)) { - std::string message("Failed to execute addon (command: '" + pythonExe + " " + args + "'). Exitcode is nonzero."); + std::string message("Failed to execute addon '" + addonInfo.name + "' (command: '" + pythonExe + " " + args + "'). Exitcode is nonzero."); if (result.size() > 2) { message = message + "\n" + message + "\nOutput:\n" + result; message.resize(message.find_last_not_of("\n\r")); @@ -588,13 +596,12 @@ unsigned int CppCheck::check(const std::string &path) executeAddons(dumpFile); } catch (const InternalError &e) { - internalError(path, e.errorMessage); - mExitCode = 1; // e.g. reflect a syntax error + internalError(path, "Processing Clang AST dump failed: " + e.errorMessage); } catch (const TerminateException &) { // Analysis is terminated return mExitCode; } catch (const std::exception &e) { - internalError(path, e.what()); + internalError(path, std::string("Processing Clang AST dump failed: ") + e.what()); } return mExitCode; @@ -975,22 +982,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string return mExitCode; } catch (const InternalError &e) { - std::list locationList; - if (e.token) { - locationList.emplace_back(e.token, &tokenizer.list); - } else { - locationList.emplace_back(filename, 0, 0); - if (filename != tokenizer.list.getSourceFilePath()) { - locationList.emplace_back(tokenizer.list.getSourceFilePath(), 0, 0); - } - } - ErrorMessage errmsg(std::move(locationList), - tokenizer.list.getSourceFilePath(), - Severity::error, - e.errorMessage, - e.id, - Certainty::normal); - + ErrorMessage errmsg = ErrorMessage::fromInternalError(e, &tokenizer.list, filename); reportErr(errmsg); } } @@ -1026,12 +1018,11 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string // Analysis is terminated return mExitCode; } catch (const std::runtime_error &e) { - internalError(filename, e.what()); - } catch (const std::bad_alloc &e) { - internalError(filename, e.what()); + internalError(filename, std::string("Checking file failed: ") + e.what()); + } catch (const std::bad_alloc &) { + internalError(filename, "Checking file failed: out of memory"); } catch (const InternalError &e) { - internalError(filename, e.errorMessage); - mExitCode=1; // e.g. reflect a syntax error + internalError(filename, "Checking file failed: " + e.errorMessage); } if (!mSettings.buildDir.empty()) { @@ -1050,9 +1041,10 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string return mExitCode; } +// TODO: replace with ErrorMessage::fromInternalError() void CppCheck::internalError(const std::string &filename, const std::string &msg) { - const std::string fullmsg("Bailing out from checking since there was an internal error: " + msg); + const std::string fullmsg("Bailing out from analysis: " + msg); const ErrorMessage::FileLocation loc1(filename, 0, 0); std::list callstack(1, loc1); @@ -1525,8 +1517,7 @@ void CppCheck::executeAddonsWholeProgram(const std::map locationList; + if (tokenList && internalError.token) { + ErrorMessage::FileLocation loc(internalError.token, tokenList); + locationList.push_back(std::move(loc)); + } else { + ErrorMessage::FileLocation loc2(filename, 0, 0); + locationList.push_back(std::move(loc2)); + if (tokenList && (filename != tokenList->getSourceFilePath())) { + ErrorMessage::FileLocation loc(tokenList->getSourceFilePath(), 0, 0); + locationList.push_back(std::move(loc)); + } + } + ErrorMessage errmsg(std::move(locationList), + tokenList ? tokenList->getSourceFilePath() : filename, + Severity::error, + internalError.errorMessage, + internalError.id, + Certainty::normal); + return errmsg; +} + std::string ErrorMessage::serialize() const { // Serialize this message into a simple string diff --git a/lib/errorlogger.h b/lib/errorlogger.h index ae0466ce5..2d1bed3c4 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -198,6 +198,8 @@ public: return mSymbolNames; } + static ErrorMessage fromInternalError(const InternalError &internalError, const TokenList *tokenList, const std::string &filename); + private: static std::string fixInvalidChars(const std::string& raw); diff --git a/releasenotes.txt b/releasenotes.txt index c54c20094..0e69003df 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -20,3 +20,4 @@ Deprecations: Other: - "USE_QT6=On" will no longer fallback to Qt5 when Qt6 is not found. +- When the execution of an addon fails with an exitcode it will now result in an 'internalError' instead of being silently ignored. \ No newline at end of file diff --git a/test/cli/test-other.py b/test/cli/test-other.py index 360637ad5..56a45af53 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -154,3 +154,56 @@ def test_slow_array_many_strings(tmpdir): f.write(' "abc",\n') f.write("};\n") cppcheck([filename]) # should not take more than ~1 second + + +def test_execute_addon_failure(tmpdir): + test_file = os.path.join(tmpdir, 'test.cpp') + with open(test_file, 'wt') as f: + f.write(""" + void f(); + """) + + args = ['--addon=naming', test_file] + + # provide empty PATH environment variable so python is not found and execution of addon fails + env = {'PATH': ''} + _, _, stderr = cppcheck(args, env) + assert stderr == '{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to auto detect python [internalError]\n\n^\n'.format(test_file) + + +def test_execute_addon_failure_2(tmpdir): + test_file = os.path.join(tmpdir, 'test.cpp') + with open(test_file, 'wt') as f: + f.write(""" + void f(); + """) + + # specify non-existent python executbale so execution of addon fails + args = ['--addon=naming', '--addon-python=python5.x', test_file] + + _, _, stderr = cppcheck(args) + # /tmp/pytest-of-sshuser/pytest-215/test_execute_addon_failure_20/test.cpp:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'naming' (command: 'python5.x /mnt/s/GitHub/cppcheck-fw/addons/runaddon.py /mnt/s/GitHub/cppcheck-fw/addons/naming.py --cli /tmp/pytest-of-sshuser/pytest-215/test_execute_addon_failure_20/test.cpp.7306.dump'). Exitcode is nonzero. [internalError]\n\n^\n + # "C:\\Users\\Quotenjugendlicher\\AppData\\Local\\Temp\\pytest-of-Quotenjugendlicher\\pytest-15\\test_execute_addon_failure_20\\test.cpp:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon (command: 'python5.x S:\\GitHub\\cppcheck-fw\\bin\\debug\\addons\\runaddon.py S:\\GitHub\\cppcheck-fw\\bin\\debug\\addons\\naming.py --cli C:\\Users\\Quotenjugendlicher\\AppData\\Local\\Temp\\pytest-of-Quotenjugendlicher\\pytest-15\\test_execute_addon_failure_20\\test.cpp.9892.dump'). Exitcode is nonzero. [internalError]\n\n^\n + assert stderr.startswith('{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon \'naming\' (command: \'python5.x '.format(test_file)) + assert stderr.endswith(' [internalError]\n\n^\n') + + +# TODO: find a test case which always fails +@pytest.mark.skip +def test_internal_error(tmpdir): + test_file = os.path.join(tmpdir, 'test.cpp') + with open(test_file, 'wt') as f: + f.write(""" +#include + +void f() { + double gc = 3333.3333; + char stat[80]; + sprintf(stat,"'%2.1f'",gc); +} + """) + + args = [test_file] + + _, _, stderr = cppcheck(args) + assert stderr == '{}:0:0: error: Bailing from out analysis: Checking file failed: converting \'1f\' to integer failed - not an integer [internalError]\n\n^\n'.format(test_file) \ No newline at end of file diff --git a/test/cli/testutils.py b/test/cli/testutils.py index c1a1455ea..1e53c65db 100644 --- a/test/cli/testutils.py +++ b/test/cli/testutils.py @@ -63,12 +63,12 @@ def lookup_cppcheck_exe(): # Run Cppcheck with args -def cppcheck(args): +def cppcheck(args, env=None): exe = lookup_cppcheck_exe() assert exe is not None, 'no cppcheck binary found' logging.info(exe + ' ' + ' '.join(args)) - p = subprocess.Popen([exe] + args, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + p = subprocess.Popen([exe] + args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env) comm = p.communicate() stdout = comm[0].decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n') stderr = comm[1].decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n')