fixed #13087 - store unmatchedSuppression in AnalyzerInformation

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;
 

cli/cppcheckexecutor.cpp

@@ -299,9 +299,10 @@ int CppCheckExecutor::check_wrapper(const Settings& settings, Suppressions& supp
  * @param unmatched list of unmatched suppressions (from Settings::Suppressions::getUnmatched(Local|Global)Suppressions)
  * @return true is returned if errors are reported
  */
-static bool reportUnmatchedSuppressions(const std::list<SuppressionList::Suppression> &unmatched, ErrorLogger &errorLogger, const std::vector<std::string>& filters)
+static std::vector<::ErrorMessage> reportUnmatchedSuppressions(const std::list<SuppressionList::Suppression> &unmatched, const std::vector<std::string>& filters)
 {
-    bool err = false;
+    std::vector<::ErrorMessage> errors;
+
     // Report unmatched suppressions
     for (const SuppressionList::Suppression &s : unmatched) {
         // check if this unmatched suppression is suppressed
@@ -329,10 +330,11 @@ static bool reportUnmatchedSuppressions(const std::list<SuppressionList::Suppres
         if (!s.fileName.empty()) {
             callStack.emplace_back(s.fileName, s.lineNumber, 0);
         }
-        errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal));
-        err = true;
+        auto errmsg = ::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal);
+        errors.emplace_back(std::move(errmsg));
     }
-    return err;
+
+    return errors;
 }
 
 bool CppCheckExecutor::reportUnmatchedSuppressions(const Settings &settings, const SuppressionList& suppressions, const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, ErrorLogger& errorLogger) {
@@ -358,21 +360,55 @@ bool CppCheckExecutor::reportUnmatchedSuppressions(const Settings &settings, con
         supprlist.addSuppression(std::move(s));
     }
 
+    const auto reportErrorsFn = [&](const std::string& sourcefile, std::size_t fsFileId, const std::vector<::ErrorMessage>& errors) -> bool {
+        if (errors.empty())
+            return false;
+
+        // TODO: what if sourcefile is empty?
+
+        AnalyzerInformation analyzerInfo;
+        // FIXME: this is a horrible hack
+        // we need to "re-open" the file so we can add the unmatchedSuppression findings.
+        // we cannot keep it open conditionally because the whole program analysis read the XML.
+        // re-ordering is also not an option because the unmatched suppression reporting needs to be run after all other checks.
+        analyzerInfo.reopen(settings.buildDir, sourcefile, /*cfgname*/"", fsFileId);
+
+        for (const auto& errmsg : errors) {
+            analyzerInfo.reportErr(errmsg);
+            errorLogger.reportErr(errmsg);
+        }
+        return true;
+    };
+
     bool err = false;
 
     for (auto i = files.cbegin(); i != files.cend(); ++i) {
-        err |= ::reportUnmatchedSuppressions(supprlist.getUnmatchedLocalSuppressions(*i), errorLogger, settings.unmatchedSuppressionFilters);
+        const std::vector<ErrorMessage> errors = ::reportUnmatchedSuppressions(supprlist.getUnmatchedLocalSuppressions(*i), settings.unmatchedSuppressionFilters);
+        err |= reportErrorsFn(i->spath(), 0, errors);
     }
 
     for (auto i = fileSettings.cbegin(); i != fileSettings.cend(); ++i) {
-        err |= ::reportUnmatchedSuppressions(supprlist.getUnmatchedLocalSuppressions(i->file), errorLogger, settings.unmatchedSuppressionFilters);
+        const std::vector<ErrorMessage> errors = ::reportUnmatchedSuppressions(supprlist.getUnmatchedLocalSuppressions(i->file), settings.unmatchedSuppressionFilters);
+        err |= reportErrorsFn(i->file.spath(), i->fileIndex, errors);
     }
 
     if (settings.inlineSuppressions) {
-        err |= ::reportUnmatchedSuppressions(supprlist.getUnmatchedInlineSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
+        const std::vector<ErrorMessage> errors = ::reportUnmatchedSuppressions(supprlist.getUnmatchedInlineSuppressions(), settings.unmatchedSuppressionFilters);
+        for (const auto& errmsg : errors) {
+            std::string sourcefile;
+            if (!errmsg.callStack.empty())
+                sourcefile = errmsg.callStack.cbegin()->getfile();
+            err |= reportErrorsFn(sourcefile, 0, {errmsg});
+        }
     }
 
-    err |= ::reportUnmatchedSuppressions(supprlist.getUnmatchedGlobalSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
+    const std::vector<ErrorMessage> errors = ::reportUnmatchedSuppressions(supprlist.getUnmatchedGlobalSuppressions(), settings.unmatchedSuppressionFilters);
+    for (const auto& errmsg : errors) {
+        std::string sourcefile;
+        if (!errmsg.callStack.empty())
+            sourcefile = errmsg.callStack.cbegin()->getfile();
+        err |= reportErrorsFn(sourcefile, 0, {errmsg});
+    }
     return err;
 }
 
@@ -425,9 +461,10 @@ int CppCheckExecutor::check_internal(const Settings& settings, Suppressions& sup
 #endif
     }
 
+    // TODO: is this run again instead of using previously cached results?
     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;

lib/analyzerinfo.cpp

@@ -25,6 +25,7 @@
 
 #include <cstring>
 #include <exception>
+#include <iostream>
 #include <map>
 #include <sstream>
 #include <utility>
@@ -84,45 +85,69 @@ void AnalyzerInformation::close()
     }
 }
 
-bool AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list<ErrorMessage> &errors)
+bool AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list<ErrorMessage> &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 std::array<const char*, 3> 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;
 }
 
 std::string AnalyzerInformation::getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfg, int fileIndex)
 {
-    const std::string id = (fileIndex > 0) ? std::to_string(fileIndex) : "";
     std::string line;
-    const std::string end(sep + cfg + sep + id + sep + Path::simplifyPath(sourcefile));
     while (std::getline(filesTxt,line)) {
-        if (line.size() <= end.size() + 2U)
-            continue;
-        if (!endsWith(line, end.c_str(), end.size()))
-            continue;
-        return line.substr(0,line.find(sep));
+        AnalyzerInformation::Info filesTxtInfo;
+        if (!filesTxtInfo.parse(line))
+            continue; // TODO: report error?
+        if (filesTxtInfo.sourceFile == sourcefile && filesTxtInfo.cfg == cfg && filesTxtInfo.fileIndex == fileIndex)
+            return filesTxtInfo.afile;
     }
     return "";
 }
@@ -145,24 +170,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<ErrorMessage> &errors)
+bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex, std::size_t hash, std::list<ErrorMessage> &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 << "<?xml version=\"1.0\"?>\n";
-        mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
-    }
+    if (!mOutputStream.is_open())
+        throw std::runtime_error("failed to open '" + analyzerInfoFile + "'");
+    mOutputStream << "<?xml version=\"1.0\"?>\n";
+    mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
 
     return true;
 }
@@ -179,6 +219,7 @@ void AnalyzerInformation::setFileInfo(const std::string &check, const std::strin
         mOutputStream << "  <FileInfo check=\"" << check << "\">\n" << fileInfo << "  </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)
@@ -206,37 +247,70 @@ 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<void(const char* checkattr, const tinyxml2::XMLElement* e, const Info& filesTxtInfo)>& handler)
+std::string AnalyzerInformation::processFilesTxt(const std::string& buildDir, const std::function<void(const char* checkattr, const tinyxml2::XMLElement* e, const Info& filesTxtInfo)>& 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)
+            return "'" + xmlfile + "' from '" + filesTxt + "' not found";
+
         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 "";
 }
 
+void AnalyzerInformation::reopen(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex)
+{
+    if (buildDir.empty() || sourcefile.empty())
+        return;
+
+    const std::string analyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir,sourcefile,cfg,fileIndex);
+    std::ifstream ifs(analyzerInfoFile);
+    if (!ifs.is_open())
+        return;
+
+    std::ostringstream iss;
+    iss << ifs.rdbuf();
+    ifs.close();
+
+    std::string content = iss.str();
+    content = content.substr(0, content.find("</analyzerinfo>"));
+
+    mOutputStream.open(analyzerInfoFile, std::ios::trunc);
+    mOutputStream << content;
+}

lib/analyzerinfo.h

@@ -61,11 +61,16 @@ 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<ErrorMessage> &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<ErrorMessage> &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);
 
+    void reopen(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex);
+
     static const char sep = ':';
 
     class CPPCHECKLIB Info {
@@ -77,14 +82,14 @@ class CPPCHECKLIB AnalyzerInformation {
         std::string sourceFile;
     };
 
-    static void processFilesTxt(const std::string& buildDir, const std::function<void(const char* checkattr, const tinyxml2::XMLElement* e, const Info& filesTxtInfo)>& handler);
+    static std::string processFilesTxt(const std::string& buildDir, const std::function<void(const char* checkattr, const tinyxml2::XMLElement* e, const Info& filesTxtInfo)>& handler, bool debug = false);
 
 protected:
     static std::string getFilesTxt(const std::list<std::string> &sourcefiles, const std::string &userDefines, const std::list<FileSettings> &fileSettings);
 
     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<ErrorMessage> &errors);
+    static bool skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list<ErrorMessage> &errors, bool debug = false);
 
 private:
     std::ofstream mOutputStream;

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);