diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 7c18e428023..c017e238409 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/lib/analyzerinfo.cpp b/lib/analyzerinfo.cpp index ea6c414b7b0..67fcc1f7fe9 100644 --- a/lib/analyzerinfo.cpp +++ b/lib/analyzerinfo.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -82,29 +83,56 @@ 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)) + if (strcmp(rootNode->Name(), "analyzerinfo") != 0) { + if (debug) + std::cout << "discarding cached result - unexpected root node" << 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; + 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; } 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; + + // TODO: discarding results on internalError doesn't make sense since that won't fix itself + // Check for invalid license error or internal error, in which case we should retry analysis + static const std::array s_ids{ + "premium-invalidLicense", + "premium-internalError", + "internalError" + }; + for (const auto* id : s_ids) + { + // cppcheck-suppress useStlAlgorithm + 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; @@ -138,27 +166,43 @@ std::string AnalyzerInformation::getAnalyzerInfoFile(const std::string &buildDir filename = sourcefile; else filename = sourcefile.substr(pos + 1); + // TODO: is this correct? the above code will return files ending in '.aN'. Also does not consider the ID return Path::join(buildDir, std::move(filename)) + ".analyzerinfo"; } -bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId, std::size_t hash, std::list &errors) +bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId, 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,fsFileId); - 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; } @@ -175,6 +219,7 @@ void AnalyzerInformation::setFileInfo(const std::string &check, const std::strin mOutputStream << " \n" << fileInfo << " \n"; } +// TODO: report detailed errors? bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) { const std::string::size_type sep1 = filesTxtLine.find(sep); if (sep1 == std::string::npos) @@ -202,37 +247,58 @@ bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) { return true; } -// TODO: bail out on unexpected data -void AnalyzerInformation::processFilesTxt(const std::string& buildDir, const std::function& handler) +std::string AnalyzerInformation::processFilesTxt(const std::string& buildDir, const std::function& handler, bool debug) { const std::string filesTxt(buildDir + "/files.txt"); std::ifstream fin(filesTxt.c_str()); std::string filesTxtLine; while (std::getline(fin, filesTxtLine)) { AnalyzerInformation::Info filesTxtInfo; - if (!filesTxtInfo.parse(filesTxtLine)) { - return; - } + if (!filesTxtInfo.parse(filesTxtLine)) + return "failed to parse '" + filesTxtLine + "' from '" + filesTxt + "'"; + + if (filesTxtInfo.afile.empty()) + return "empty afile from '" + filesTxt + "'"; const std::string xmlfile = buildDir + '/' + filesTxtInfo.afile; tinyxml2::XMLDocument doc; const tinyxml2::XMLError error = doc.LoadFile(xmlfile.c_str()); + if (error == tinyxml2::XML_ERROR_FILE_NOT_FOUND) { + /* FIXME: this can currently not be reported as an error because: + * - --clang does not generate any analyzer information - see #14456 + * - markup files might not generate analyzer information + * - files with preprocessor errors might not generate analyzer information + */ + if (debug) + std::cout << "'" + xmlfile + "' from '" + filesTxt + "' not found"; + return ""; + } + if (error != tinyxml2::XML_SUCCESS) - return; + return "failed to load '" + xmlfile + "' from '" + filesTxt + "'"; const tinyxml2::XMLElement * const rootNode = doc.FirstChildElement(); if (rootNode == nullptr) - return; + return "no root node found in '" + xmlfile + "' from '" + filesTxt + "'"; + + if (strcmp(rootNode->Name(), "analyzerinfo") != 0) + return "unexpected root node in '" + xmlfile + "' from '" + filesTxt + "'"; 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) { + if (debug) + std::cout << "'check' attribute missing in 'FileInfo' in '" << xmlfile << "' from '" << filesTxt + "'"; continue; + } handler(checkattr, e, filesTxtInfo); } } + + // TODO: error on empty file? + return ""; } diff --git a/lib/analyzerinfo.h b/lib/analyzerinfo.h index 1547ef50f16..5435be62560 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, std::size_t fsFileId, 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, std::size_t fsFileId, 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, std::size_t fsFileId); @@ -77,14 +80,14 @@ class CPPCHECKLIB AnalyzerInformation { std::string sourceFile; }; - static void processFilesTxt(const std::string& buildDir, const std::function& handler); + static std::string processFilesTxt(const std::string& buildDir, const std::function& handler, bool debug = false); protected: static std::string getFilesTxt(const std::list &sourcefiles, const std::list &fileSettings); static std::string getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfg, int fsFileId); - 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/checkunusedfunctions.cpp b/lib/checkunusedfunctions.cpp index 7edaff6c241..eb06a4691f1 100644 --- a/lib/checkunusedfunctions.cpp +++ b/lib/checkunusedfunctions.cpp @@ -482,7 +482,12 @@ void CheckUnusedFunctions::analyseWholeProgram(const Settings &settings, ErrorLo } }; - AnalyzerInformation::processFilesTxt(buildDir, handler); + const std::string err = AnalyzerInformation::processFilesTxt(buildDir, handler, settings.debugainfo); + if (!err.empty()) { + const ErrorMessage errmsg({}, "", Severity::error, err, "internalError", Certainty::normal); + errorLogger.reportErr(errmsg); + return; + } for (auto decl = decls.cbegin(); decl != decls.cend(); ++decl) { const std::string &functionName = stripTemplateParameters(decl->first); diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index de3bcd1530a..ac335373784 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -973,7 +973,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str mLogger->setAnalyzerInfo(nullptr); std::list errors; - analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, file.fsFileId(), hash, errors); + analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, file.fsFileId(), hash, errors, mSettings.debugainfo); analyzerInformation->setFileInfo("CheckUnusedFunctions", mUnusedFunctionsCheck->analyzerInfo(tokenizer)); analyzerInformation->close(); } @@ -1019,7 +1019,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, file.fsFileId(), hash, errors)) { + if (!analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, file.fsFileId(), hash, errors, mSettings.debugainfo)) { while (!errors.empty()) { mErrorLogger.reportErr(errors.front()); errors.pop_front(); @@ -1860,12 +1860,17 @@ unsigned int CppCheck::analyseWholeProgram(const std::string &buildDir, const st } }; - AnalyzerInformation::processFilesTxt(buildDir, handler); - - // Analyse the tokens - // cppcheck-suppress shadowFunction - TODO: fix this - for (Check *check : Check::instances()) - check->analyseWholeProgram(ctuFileInfo, fileInfoList, mSettings, mErrorLogger); + const std::string err = AnalyzerInformation::processFilesTxt(buildDir, handler, mSettings.debugainfo); + if (!err.empty()) { + const ErrorMessage errmsg({}, "", Severity::error, err, "internalError", Certainty::normal); + mErrorLogger.reportErr(errmsg); + } + else { + // Analyse the tokens + // cppcheck-suppress shadowFunction - TODO: fix this + for (Check *check : Check::instances()) + check->analyseWholeProgram(ctuFileInfo, fileInfoList, mSettings, mErrorLogger); + } for (Check::FileInfo *fi : fileInfoList) delete fi; diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index e62ed6b806d..13f66c13d1a 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -162,6 +162,7 @@ ErrorMessage::ErrorMessage(ErrorPath errorPath, const TokenList *tokenList, Seve // hash = calculateWarningHash(tokenList, hashWarning.str()); } +// TODO: improve errorhandling? ErrorMessage::ErrorMessage(const tinyxml2::XMLElement * const errmsg) : severity(Severity::none), cwe(0U), 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 744a68aac60..7f882268754 100644 --- a/test/cli/other_test.py +++ b/test/cli/other_test.py @@ -4081,3 +4081,103 @@ 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_analyzerinfo(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), + '--enable=all', + str(test_file) + ] + + stderr_exp = [ + '{}:3:14: error: Null pointer dereference: (int*)0 [nullPointer]'.format(test_file), + "{}:1:6: style: The function 'f' is never used. [unusedFunction]".format(test_file) + ] + + def run_and_assert_cppcheck(stdout_exp): + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0, stdout + assert stdout.splitlines() == stdout_exp + assert stderr.splitlines() == stderr_exp + + test_a1_file_s = str(test_a1_file).replace('\\', '/') + + # no cached results + run_and_assert_cppcheck([ + "no cached result '{}' found".format(test_a1_file_s) + ]) + + # cached results + run_and_assert_cppcheck([ + "skipping analysis - loaded 1 cached finding(s) from '{}'".format(test_a1_file_s) + ]) + + # modified file + with open(test_file, 'a') as f: + f.write('\n#define DEF') + + run_and_assert_cppcheck([ + "discarding cached result - hash mismatch" # TODO: add filename + ]) + + # invalid XML + with open(test_a1_file, 'a') as f: + f.write('.') + + run_and_assert_cppcheck([ + "discarding cached result - failed to load '{}' (XML_ERROR_PARSING_TEXT)".format(test_a1_file_s) + ]) + + # missing root node + with open(test_a1_file, 'w') as f: + f.write('') + + run_and_assert_cppcheck([ + "discarding cached result - no root node found" # TODO: add filename + ]) + + # mismatched root node + with open(test_a1_file, 'w') as f: + f.write('') + + run_and_assert_cppcheck([ + "discarding cached result - unexpected root node" # TODO: add filename + ]) + + # missing 'hash' attribute + with open(test_a1_file, 'w') as f: + f.write('') + + run_and_assert_cppcheck([ + "discarding cached result - no 'hash' attribute found" # TODO: add filename + ]) + + # invalid 'hash' attribute + with open(test_a1_file, 'w') as f: + f.write('') + + run_and_assert_cppcheck([ + "discarding cached result - hash mismatch" # TODO: add filename + ]) + + # TODO: + # - invalid error + # - internalError 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"};