Fix #14974: [regression] inline supression does not work with build dir anymore (#8801)

Also fixes #14993.

The hash in the AnalyzerInformation file has been changed from an
attribute to a node, this is because the hash is not known at the time
the root node is written. An alternative would be to delay writing
anything at all until checking is complete.

An `includes` node has been added to the AnalyzerInformation file to
keep track of which files were included during the last check, this
allows calculating the correct hash and processing inline suppressions
before any checking is done.

Author: glankk

Makefile

@@ -792,7 +792,7 @@ test/testcondition.o: test/testcondition.cpp lib/check.h lib/checkcondition.h li
 test/testconstructors.o: test/testconstructors.cpp lib/check.h lib/checkclass.h lib/checkers.h lib/checkimpl.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
 	$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testconstructors.cpp
 
-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
+test/testcppcheck.o: test/testcppcheck.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/analyzerinfo.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
 	$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testcppcheck.cpp
 
 test/testerrorlogger.o: test/testerrorlogger.cpp externals/tinyxml2/tinyxml2.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/analyzerinfo.cpp

@@ -58,6 +58,68 @@ void AnalyzerInformation::writeFilesTxt(const std::string &buildDir, const std::
     fout << getFilesTxt(sourcefiles, fileSettings);
 }
 
+void AnalyzerInformation::writeIncludes(const std::set<std::string> &files)
+{
+    if (!files.empty() && mOutputStream.is_open()) {
+        mOutputStream << "  <includes>\n";
+        for (const std::string &file : files) {
+            mOutputStream << "    <filename>" << file << "</filename>\n";
+        }
+        mOutputStream << "  </includes>\n";
+    }
+}
+
+std::set<std::string> AnalyzerInformation::getIncludes(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId) const
+{
+    if (mOutputStream.is_open())
+        throw std::runtime_error("analyzer information file is already open");
+
+    std::set<std::string> files;
+
+    if (buildDir.empty() || sourcefile.empty())
+        return files;
+
+    const std::string analyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir, sourcefile, cfg, fsFileId);
+
+    tinyxml2::XMLDocument analyzerInfoDoc;
+    if (analyzerInfoDoc.LoadFile(analyzerInfoFile.c_str()) != tinyxml2::XML_SUCCESS)
+        return files;
+
+    const tinyxml2::XMLElement *const rootNode = analyzerInfoDoc.FirstChildElement();
+    if (rootNode == nullptr)
+        return files;
+
+    if (strcmp(rootNode->Name(), "analyzerinfo") != 0)
+        return files;
+
+    const tinyxml2::XMLElement *includesNode = nullptr;
+    for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
+        if (strcmp(e->Name(), "includes") == 0) {
+            includesNode = e;
+            break;
+        }
+    }
+
+    if (includesNode == nullptr)
+        return files;
+
+    for (const tinyxml2::XMLElement *e = includesNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
+        if (strcmp(e->Name(), "filename") != 0)
+            continue;
+
+        files.insert(e->GetText());
+    }
+
+    return files;
+}
+
+void AnalyzerInformation::writeHash(std::size_t hash)
+{
+    if (mOutputStream.is_open()) {
+        mOutputStream << "  <hash>" << hash << "</hash>\n";
+    }
+}
+
 std::string AnalyzerInformation::getFilesTxt(const std::list<std::string> &sourcefiles, const std::list<FileSettings> &fileSettings) {
     std::ostringstream ret;
 
@@ -94,10 +156,17 @@ std::string AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analy
     if (strcmp(rootNode->Name(), "analyzerinfo") != 0)
         return "unexpected root node";
 
-    const char * const attr = rootNode->Attribute("hash");
-    if (!attr)
-        return "no 'hash' attribute found";
-    if (attr != std::to_string(hash))
+    const tinyxml2::XMLElement *hashNode = nullptr;
+    for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
+        if (strcmp(e->Name(), "hash") == 0) {
+            hashNode = e;
+            break;
+        }
+    }
+
+    if (!hashNode)
+        return "no 'hash' node found";
+    if (hashNode->GetText() != std::to_string(hash))
         return "hash mismatch";
 
     for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
@@ -194,7 +263,7 @@ bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::st
     if (!mOutputStream.is_open())
         throw std::runtime_error("failed to open '" + analyzerInfoFile + "'");
     mOutputStream << "<?xml version=\"1.0\"?>\n";
-    mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
+    mOutputStream << "<analyzerinfo>\n";
 
     return true;
 }

lib/analyzerinfo.h

@@ -27,6 +27,7 @@
 #include <fstream>
 #include <functional>
 #include <list>
+#include <set>
 #include <string>
 
 class ErrorMessage;
@@ -67,6 +68,9 @@ class CPPCHECKLIB AnalyzerInformation {
     bool analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId, std::size_t hash, std::list<ErrorMessage> &errors, bool debug = false);
     void reportErr(const ErrorMessage &msg);
     void setFileInfo(const std::string &check, const std::string &fileInfo);
+    void writeIncludes(const std::set<std::string> &files);
+    std::set<std::string> getIncludes(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId) const;
+    void writeHash(std::size_t hash);
     static std::string getAnalyzerInfoFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId);
 
     void reopen(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId);

lib/cppcheck.cpp

@@ -1026,25 +1026,6 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
         preprocessor.inlineSuppressions(mSuppressions.nomsg);
         preprocessor.removeComments();
 
-        if (!mSettings.buildDir.empty()) {
-            analyzerInformation.reset(new AnalyzerInformation);
-            mLogger->setAnalyzerInfo(analyzerInformation.get());
-        }
-
-        if (analyzerInformation) {
-            // Calculate hash so it can be compared with old hash / future hashes
-            const std::size_t hash = calculateHash(preprocessor, file.spath());
-            std::list<ErrorMessage> 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();
-                }
-                mLogger->setAnalyzerInfo(nullptr);
-                return mLogger->exitcode();  // known results => no need to reanalyze file
-            }
-        }
-
         // Get directives
         std::list<Directive> directives;
         preprocessor.createDirectives(directives);
@@ -1062,7 +1043,13 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
                        std::inserter(configDefines, configDefines.end()),
                        getDefineName);
 
+        // Keep track of all included files when using build dir
+        std::set<std::string> includedFiles;
+
         preprocessor.setLoadCallback([&](simplecpp::FileData &data, bool loaded) {
+            if (analyzerInformation) {
+                includedFiles.insert(data.filename);
+            }
             if (loaded) {
                 // Do preprocessing on included file
                 mLogger->addRemarkComments(preprocessor.getRemarkComments(data.tokens));
@@ -1078,12 +1065,37 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
 
         preprocessor.setPlatformInfo();
 
+        if (!mSettings.buildDir.empty()) {
+            analyzerInformation.reset(new AnalyzerInformation);
+            mLogger->setAnalyzerInfo(analyzerInformation.get());
+        }
+
+        if (analyzerInformation) {
+            // Load all included files to get correct hashes and suppressions
+            for (const std::string &filename : analyzerInformation->getIncludes(mSettings.buildDir, file.spath(), cfgname, file.fsFileId()))
+                preprocessor.loadFile(files, filename);
+            // Calculate hash so it can be compared with old hash / future hashes
+            const std::size_t hash = calculateHash(preprocessor, file.spath());
+            std::list<ErrorMessage> 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();
+                }
+                mLogger->setAnalyzerInfo(nullptr);
+                return mLogger->exitcode();  // known results => no need to reanalyze file
+            }
+            // Clear included file list; we don't want to keep includes that have been removed from the source
+            // Any includes that are still present will be readded
+            includedFiles.clear();
+        }
+
         // Get configurations..
         if (maxConfigs > 1) {
             Timer::run("Preprocessor::getConfigs", mTimerResults, [&]() {
                 configurations = { "" };
                 preprocessor.getConfigs(configDefines, configurations);
-                preprocessor.loadFiles(files);
+                preprocessor.loadAllIncludes(files);
             });
         } else {
             configurations = { mSettings.userDefines };
@@ -1306,6 +1318,11 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
             mLogger->setPlistFilenames(std::move(files));
         }
 
+        if (analyzerInformation) {
+            analyzerInformation->writeIncludes(includedFiles);
+            analyzerInformation->writeHash(calculateHash(preprocessor, file.spath()));
+        }
+
         executeAddons(dumpFile, file);
     } catch (const TerminateException &) {
         // Analysis is terminated

lib/preprocessor.cpp

@@ -833,7 +833,7 @@ const simplecpp::Output* Preprocessor::handleErrors(const simplecpp::OutputList&
     return reportOutput(outputList, showerror);
 }
 
-bool Preprocessor::loadFiles(std::vector<std::string> &files)
+bool Preprocessor::loadAllIncludes(std::vector<std::string> &files)
 {
     const simplecpp::DUI dui = createDUI(mSettings, "", mLang);
 
@@ -842,6 +842,13 @@ bool Preprocessor::loadFiles(std::vector<std::string> &files)
     return !handleErrors(outputList);
 }
 
+simplecpp::FileData *Preprocessor::loadFile(std::vector<std::string> &files, const std::string &file)
+{
+    const simplecpp::DUI dui = createDUI(mSettings, "", mLang);
+
+    return mFileCache.get("", file, dui, false, files, nullptr).first;
+}
+
 void Preprocessor::removeComments()
 {
     removeComments(mTokens);

lib/preprocessor.h

@@ -122,7 +122,9 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
 
     std::vector<RemarkComment> getRemarkComments(const simplecpp::TokenList &tokens) const;
 
-    bool loadFiles(std::vector<std::string> &files);
+    bool loadAllIncludes(std::vector<std::string> &files);
+
+    simplecpp::FileData *loadFile(std::vector<std::string> &files, const std::string &file);
 
     void removeComments();
 

test/cli/inline-suppress_test.py

@@ -248,6 +248,30 @@ def test_build_dir(tmpdir):
     assert stdout == ''
     assert ret == 0, stdout
 
+
+def test_build_dir_include(tmpdir):
+    args = [
+        '-q',
+        '--template=simple',
+        '--cppcheck-build-dir={}'.format(tmpdir),
+        '--enable=all',
+        '--inline-suppr',
+        '{}5.cpp'.format(__proj_inline_suppres_path)
+    ]
+
+    ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
+    lines = stderr.splitlines()
+    assert lines == []
+    assert stdout == ''
+    assert ret == 0, stdout
+
+    ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
+    lines = stderr.splitlines()
+    assert lines == []
+    assert stdout == ''
+    assert ret == 0, stdout
+
+
 def test_build_dir_jobs_suppressions(tmpdir): #14064
     args = [
         '-q',

test/cli/other_test.py

@@ -2320,7 +2320,7 @@ def test_builddir_hash_check_level(tmp_path):  # #13376
     cache_file = (build_dir / 'test.a1')
 
     root = ElementTree.fromstring(cache_file.read_text())
-    hash_1 = root.get('hash')
+    hash_1 = root.findtext('hash')
 
     args += ['--check-level=exhaustive']
 
@@ -2329,7 +2329,7 @@ def test_builddir_hash_check_level(tmp_path):  # #13376
     assert stderr == ''
 
     root = ElementTree.fromstring(cache_file.read_text())
-    hash_2 = root.get('hash')
+    hash_2 = root.findtext('hash')
 
     assert hash_1 != hash_2
 
@@ -4531,17 +4531,17 @@ def run_and_assert_cppcheck(stdout_exp):
         "discarding cached result from '{}' for '{}' - unexpected root node".format(test_a1_file_s, test_file_s)
     ])
 
-    # missing 'hash' attribute
+    # missing 'hash' node
     with open(test_a1_file, 'w') as f:
         f.write('<?xml version="1.0"?><analyzerinfo/>')
 
     run_and_assert_cppcheck([
-        "discarding cached result from '{}' for '{}' - no 'hash' attribute found".format(test_a1_file_s, test_file_s)
+        "discarding cached result from '{}' for '{}' - no 'hash' node found".format(test_a1_file_s, test_file_s)
     ])
 
-    # invalid 'hash' attribute
+    # invalid 'hash' node
     with open(test_a1_file, 'w') as f:
-        f.write('<?xml version="1.0"?><analyzerinfo hash="hash"/>')
+        f.write('<?xml version="1.0"?><analyzerinfo><hash>hash</hash></analyzerinfo>')
 
     run_and_assert_cppcheck([
         "discarding cached result from '{}' for '{}' - hash mismatch".format(test_a1_file_s, test_file_s)

test/cli/premium_test.py

@@ -88,11 +88,11 @@ def test_build_dir_hash_cppcheck_product(tmpdir):
     assert exitcode == 0
 
     def _get_hash(s:str):
-        i = s.find(' hash="')
+        i = s.find('<hash>')
         if i <= -1:
             return ''
         i += 7
-        return s[i:s.find('"', i)]
+        return s[i:s.find('</hash>', i)]
 
     with open(build_dir.join('test.a1'), 'rt') as f:
         f1 = f.read()

test/cli/proj-inline-suppress/5.cpp

@@ -0,0 +1 @@
+#include "5.h"