From f057a88ad1e6b8898d2c1cc663dd491d41512377 Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 8 Jan 2026 19:20:31 +0100 Subject: [PATCH] improved errorhandling of analyzer information loading [skip ci] --- cli/cmdlineparser.cpp | 3 ++ cli/cppcheckexecutor.cpp | 2 +- lib/analyzerinfo.cpp | 107 +++++++++++++++++++++++++++---------- lib/analyzerinfo.h | 7 ++- lib/cppcheck.cpp | 4 +- lib/settings.h | 3 ++ test/cli/other_test.py | 63 ++++++++++++++++++++++ test/testcmdlineparser.cpp | 8 +++ 8 files changed, 163 insertions(+), 34 deletions(-) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 92f4c6cef3b..f183b5c646e 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -621,6 +621,9 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a mSettings.cppHeaderProbe = true; } + else if (std::strcmp(argv[i], "--debug-analyzerinfo") == 0) + mSettings.debugainfo = true; + else if (std::strcmp(argv[i], "--debug-ast") == 0) mSettings.debugast = true; diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index a16f76183c0..2a0e0677573 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -427,7 +427,7 @@ int CppCheckExecutor::check_internal(const Settings& settings, Suppressions& sup returnValue |= cppcheck.analyseWholeProgram(settings.buildDir, mFiles, mFileSettings, stdLogger.getCtuInfo()); - if (settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) { + if ((settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) && !supprs.nomsg.getSuppressions().empty()) { const bool err = reportUnmatchedSuppressions(settings, supprs.nomsg, mFiles, mFileSettings, stdLogger); if (err && returnValue == 0) returnValue = settings.exitCode; diff --git a/lib/analyzerinfo.cpp b/lib/analyzerinfo.cpp index c7045252f3d..1e0448981cc 100644 --- a/lib/analyzerinfo.cpp +++ b/lib/analyzerinfo.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -84,29 +85,48 @@ void AnalyzerInformation::close() } } -bool AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list &errors) +bool AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list &errors, bool debug) { const tinyxml2::XMLElement * const rootNode = analyzerInfoDoc.FirstChildElement(); - if (rootNode == nullptr) + if (rootNode == nullptr) { + if (debug) + std::cout << "discarding cached result - no root node found" << std::endl; return false; + } - const char *attr = rootNode->Attribute("hash"); - if (!attr || attr != std::to_string(hash)) + const char * const attr = rootNode->Attribute("hash"); + if (!attr) { + if (debug) + std::cout << "discarding cached result - no 'hash' attribute found" << std::endl; + return false; + } + if (attr != std::to_string(hash)) { + if (debug) + std::cout << "discarding cached result - hash mismatch" << std::endl; return false; - - // Check for invalid license error or internal error, in which case we should retry analysis - for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) { - if (std::strcmp(e->Name(), "error") == 0 && - (e->Attribute("id", "premium-invalidLicense") || - e->Attribute("id", "premium-internalError") || - e->Attribute("id", "internalError") - )) - return false; } for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) { - if (std::strcmp(e->Name(), "error") == 0) - errors.emplace_back(e); + if (std::strcmp(e->Name(), "error") != 0) + continue; + + // Check for invalid license error or internal error, in which case we should retry analysis + static std::array s_ids{ + "premium-invalidLicense", + "premium-internalError", + "internalError" + }; + for (const auto* id : s_ids) + { + if (e->Attribute("id", id)) { + if (debug) + std::cout << "discarding cached result - '" << id << "' encountered" << std::endl; + errors.clear(); + return false; + } + } + + errors.emplace_back(e); } return true; @@ -145,24 +165,39 @@ std::string AnalyzerInformation::getAnalyzerInfoFile(const std::string &buildDir return Path::join(buildDir, std::move(filename)) + ".analyzerinfo"; } -bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex, std::size_t hash, std::list &errors) +bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex, std::size_t hash, std::list &errors, bool debug) { + if (mOutputStream.is_open()) + throw std::runtime_error("analyzer information file is already open"); + if (buildDir.empty() || sourcefile.empty()) return true; - close(); const std::string analyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir,sourcefile,cfg,fileIndex); - tinyxml2::XMLDocument analyzerInfoDoc; - const tinyxml2::XMLError xmlError = analyzerInfoDoc.LoadFile(analyzerInfoFile.c_str()); - if (xmlError == tinyxml2::XML_SUCCESS && skipAnalysis(analyzerInfoDoc, hash, errors)) - return false; + { + tinyxml2::XMLDocument analyzerInfoDoc; + const tinyxml2::XMLError xmlError = analyzerInfoDoc.LoadFile(analyzerInfoFile.c_str()); + if (xmlError == tinyxml2::XML_SUCCESS) { + if (skipAnalysis(analyzerInfoDoc, hash, errors, debug)) { + if (debug) + std::cout << "skipping analysis - loaded " << errors.size() << " cached finding(s) from '" << analyzerInfoFile << "'" << std::endl; + return false; + } + } + else if (xmlError != tinyxml2::XML_ERROR_FILE_NOT_FOUND) { + if (debug) + std::cout << "discarding cached result - failed to load '" << analyzerInfoFile << "' (" << tinyxml2::XMLDocument::ErrorIDToName(xmlError) << ")" << std::endl; + } + else if (debug) + std::cout << "no cached result '" << analyzerInfoFile << "' found" << std::endl; + } mOutputStream.open(analyzerInfoFile); - if (mOutputStream.is_open()) { - mOutputStream << "\n"; - mOutputStream << "\n"; - } + if (!mOutputStream.is_open()) + throw std::runtime_error("failed to open '" + analyzerInfoFile + "'"); + mOutputStream << "\n"; + mOutputStream << "\n"; return true; } @@ -179,6 +214,7 @@ void AnalyzerInformation::setFileInfo(const std::string &check, const std::strin mOutputStream << " \n" << fileInfo << " \n"; } +// TODO: report errors bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) { const std::string::size_type sep1 = filesTxtLine.find(sep); if (sep1 == std::string::npos) @@ -206,6 +242,7 @@ bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) { return true; } +// TODO: how to report errors properly? // TODO: bail out on unexpected data void AnalyzerInformation::processFilesTxt(const std::string& buildDir, const std::function& handler) { @@ -215,6 +252,7 @@ void AnalyzerInformation::processFilesTxt(const std::string& buildDir, const std while (std::getline(fin, filesTxtLine)) { AnalyzerInformation::Info filesTxtInfo; if (!filesTxtInfo.parse(filesTxtLine)) { + std::cerr << "failed to parse '" + filesTxtLine + "' from '" + filesTxt + "'"; return; } @@ -222,21 +260,32 @@ void AnalyzerInformation::processFilesTxt(const std::string& buildDir, const std tinyxml2::XMLDocument doc; const tinyxml2::XMLError error = doc.LoadFile(xmlfile.c_str()); - if (error != tinyxml2::XML_SUCCESS) + if (error == tinyxml2::XML_ERROR_FILE_NOT_FOUND) return; - const tinyxml2::XMLElement * const rootNode = doc.FirstChildElement(); - if (rootNode == nullptr) + if (error != tinyxml2::XML_SUCCESS) { + std::cerr << "failed to load '" + xmlfile + "' from '" + filesTxt + "'"; return; + } + + const tinyxml2::XMLElement * const rootNode = doc.FirstChildElement(); + if (rootNode == nullptr) { + std::cerr << "no root node found in '" + xmlfile + "' from '" + filesTxt + "'"; + continue; + } for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) { if (std::strcmp(e->Name(), "FileInfo") != 0) continue; const char *checkattr = e->Attribute("check"); - if (checkattr == nullptr) + if (checkattr == nullptr) { + std::cerr << "'check' attribute missing in 'FileInfo' in '" + xmlfile + "' from '" + filesTxt + "'"; continue; + } handler(checkattr, e, filesTxtInfo); } } + + // TODO: error on empty file? } diff --git a/lib/analyzerinfo.h b/lib/analyzerinfo.h index 881076c09f4..89545b5bd75 100644 --- a/lib/analyzerinfo.h +++ b/lib/analyzerinfo.h @@ -61,7 +61,10 @@ class CPPCHECKLIB AnalyzerInformation { /** Close current TU.analyzerinfo file */ void close(); - bool analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex, std::size_t hash, std::list &errors); + /** + * @throws std::runtime_error thrown if the output file is already open or the output file cannot be opened + */ + bool analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex, std::size_t hash, std::list &errors, bool debug = false); void reportErr(const ErrorMessage &msg); void setFileInfo(const std::string &check, const std::string &fileInfo); static std::string getAnalyzerInfoFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex); @@ -84,7 +87,7 @@ class CPPCHECKLIB AnalyzerInformation { static std::string getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfg, int fileIndex); - static bool skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list &errors); + static bool skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list &errors, bool debug = false); private: std::ofstream mOutputStream; diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 399eb6d3d6e..77ee7273259 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -974,7 +974,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str mLogger->setAnalyzerInfo(nullptr); std::list errors; - analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, fileIndex, hash, errors); + analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, fileIndex, hash, errors, mSettings.debugainfo); analyzerInformation->setFileInfo("CheckUnusedFunctions", mUnusedFunctionsCheck->analyzerInfo(tokenizer)); analyzerInformation->close(); } @@ -1020,7 +1020,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str // Calculate hash so it can be compared with old hash / future hashes const std::size_t hash = calculateHash(preprocessor, file.spath()); std::list errors; - if (!analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, fileIndex, hash, errors)) { + if (!analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, fileIndex, hash, errors, mSettings.debugainfo)) { while (!errors.empty()) { mErrorLogger.reportErr(errors.front()); errors.pop_front(); diff --git a/lib/settings.h b/lib/settings.h index 04541cda08c..6521ba4581d 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -189,6 +189,9 @@ class CPPCHECKLIB WARN_UNUSED Settings { /** @brief Are we running from DACA script? */ bool daca{}; + /** @brief Is --debug-analyzerinfo given? */ + bool debugainfo{}; + /** @brief Is --debug-ast given? */ bool debugast{}; diff --git a/test/cli/other_test.py b/test/cli/other_test.py index c4ae2fab5cf..5c1c6978b29 100644 --- a/test/cli/other_test.py +++ b/test/cli/other_test.py @@ -4119,3 +4119,66 @@ def test_active_unusedfunction_only_misra_builddir(tmp_path): 'CheckUnusedFunctions::check' ] __test_active_checkers(tmp_path, 1, 1175, use_unusedfunction_only=True, use_misra=True, checkers_exp=checkers_exp) + + +def test_builddir_(tmp_path): + test_file = tmp_path / 'test.c' + with open(test_file, "w") as f: + f.write( +"""void f() +{ + (void)(*((int*)0)); +} +""") + + build_dir = tmp_path / 'b1' + os.makedirs(build_dir) + + test_a1_file = build_dir / 'test.a1' + + args = [ + '-q', + '--debug-analyzerinfo', + '--template=simple', + '--cppcheck-build-dir={}'.format(build_dir), + str(test_file) + ] + + stderr_exp = [ + '{}:3:14: error: Null pointer dereference: (int*)0 [nullPointer]'.format(test_file) + ] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0, stdout + assert stdout.splitlines() == [ + "no cached result '{}' found".format(test_a1_file) + ] + assert stderr.splitlines() == stderr_exp + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0, stdout + assert stdout.splitlines() == [ + "skipping analysis - loaded 1 cached finding(s) from '{}'".format(test_a1_file) + ] + assert stderr.splitlines() == stderr_exp + + with open(test_file, 'a') as f: + f.write('\n#define DEF') + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0, stdout + assert stdout.splitlines() == [ + "discarding cached result - hash mismatch" + ] + assert stderr.splitlines() == stderr_exp + + a_a1_file = build_dir / 'test.a1' + with open(a_a1_file, 'a') as f: + f.write('.') + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0, stdout + assert stdout.splitlines() == [ + "discarding cached result - failed to load '{}' (XML_ERROR_PARSING_TEXT)".format(test_a1_file) + ] + assert stderr.splitlines() == stderr_exp diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp index 3b343cbc92e..b6690f8d5c7 100644 --- a/test/testcmdlineparser.cpp +++ b/test/testcmdlineparser.cpp @@ -493,6 +493,7 @@ class TestCmdlineParser : public TestFixture { TEST_CASE(safetyOverride); TEST_CASE(noSafety); TEST_CASE(noSafetyOverride); + TEST_CASE(debugAnalyzerinfo); TEST_CASE(ignorepaths1); TEST_CASE(ignorepaths2); @@ -3424,6 +3425,13 @@ class TestCmdlineParser : public TestFixture { ASSERT_EQUALS(false, settings->safety); } + void debugAnalyzerinfo() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--debug-analyzerinfo", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv)); + ASSERT_EQUALS(true, settings->debugainfo); + } + void ignorepaths1() { REDIRECT; const char * const argv[] = {"cppcheck", "-isrc", "file.cpp"};