Fix #9972 (Add support for SARIF output format) (#6863)

Co-authored-by: Mario Campos <mario-campos@github.com>

Makefile

@@ -173,7 +173,7 @@ ifndef INCLUDE_FOR_LIB
 endif
 
 ifndef INCLUDE_FOR_CLI
-    INCLUDE_FOR_CLI=-Ilib -isystem externals/simplecpp -isystem externals/tinyxml2
+    INCLUDE_FOR_CLI=-Ilib -isystem externals/picojson -isystem externals/simplecpp -isystem externals/tinyxml2
 endif
 
 ifndef INCLUDE_FOR_TEST
@@ -752,7 +752,7 @@ $(libcppdir)/vfvalue.o: lib/vfvalue.cpp lib/config.h lib/errortypes.h lib/mathli
 cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/filelister.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h lib/xml.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cmdlineparser.cpp
 
-cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/executor.h cli/processexecutor.h cli/signalhandler.h cli/singleexecutor.h cli/threadexecutor.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkersreport.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/settings.h lib/standards.h lib/suppressions.h lib/utils.h
+cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/executor.h cli/processexecutor.h cli/signalhandler.h cli/singleexecutor.h cli/threadexecutor.h externals/picojson/picojson.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkersreport.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutor.cpp
 
 cli/cppcheckexecutorseh.o: cli/cppcheckexecutorseh.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h lib/config.h lib/filesettings.h lib/path.h lib/platform.h lib/standards.h lib/utils.h

cli/CMakeLists.txt

@@ -12,6 +12,7 @@ if (BUILD_CLI)
     else()
         target_include_directories(cli_objs SYSTEM PRIVATE ${tinyxml2_INCLUDE_DIRS})
     endif()
+    target_externals_include_directories(cli_objs PRIVATE ${PROJECT_SOURCE_DIR}/externals/picojson/)
     target_externals_include_directories(cli_objs PRIVATE ${PROJECT_SOURCE_DIR}/externals/simplecpp/)
     if (NOT CMAKE_DISABLE_PRECOMPILE_HEADERS)
         target_precompile_headers(cli_objs PRIVATE precompiled.h)

cli/cli.vcxproj

@@ -85,7 +85,7 @@
   </PropertyGroup>
   <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">
     <ClCompile>
-      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\picojson;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <BufferSecurityCheck>true</BufferSecurityCheck>
       <DebugInformationFormat>ProgramDatabase</DebugInformationFormat>
       <Optimization>Disabled</Optimization>
@@ -114,7 +114,7 @@
   </ItemDefinitionGroup>
   <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug-PCRE|x64'">
     <ClCompile>
-      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\picojson;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <BufferSecurityCheck>true</BufferSecurityCheck>
       <DebugInformationFormat>ProgramDatabase</DebugInformationFormat>
       <Optimization>Disabled</Optimization>
@@ -143,7 +143,7 @@
   </ItemDefinitionGroup>
   <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
     <ClCompile>
-      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\picojson;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <BufferSecurityCheck>false</BufferSecurityCheck>
       <Optimization>MaxSpeed</Optimization>
       <PreprocessorDefinitions>CPPCHECKLIB_IMPORT;TINYXML2_IMPORT;NDEBUG;WIN32;_CRT_SECURE_NO_WARNINGS;WIN32_LEAN_AND_MEAN;_WIN64;%(PreprocessorDefinitions)</PreprocessorDefinitions>
@@ -181,7 +181,7 @@
   </ItemDefinitionGroup>
   <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release-PCRE|x64'">
     <ClCompile>
-      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\picojson;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <BufferSecurityCheck>false</BufferSecurityCheck>
       <Optimization>MaxSpeed</Optimization>
       <PreprocessorDefinitions>CPPCHECKLIB_IMPORT;TINYXML2_IMPORT;NDEBUG;WIN32;HAVE_RULES;_CRT_SECURE_NO_WARNINGS;WIN32_LEAN_AND_MEAN;_WIN64;%(PreprocessorDefinitions)</PreprocessorDefinitions>

cli/cmdlineparser.cpp

@@ -915,6 +915,20 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
             else if (std::strncmp(argv[i], "--output-file=", 14) == 0)
                 mSettings.outputFile = Path::simplifyPath(argv[i] + 14);
 
+            else if (std::strncmp(argv[i], "--output-format=", 16) == 0) {
+                const std::string format = argv[i] + 16;
+                if (format == "sarif")
+                    mSettings.outputFormat = Settings::OutputFormat::sarif;
+                else if (format == "xml")
+                    mSettings.outputFormat = Settings::OutputFormat::xml;
+                else {
+                    mLogger.printError("argument to '--output-format=' must be 'sarif' or 'xml'.");
+                    return Result::Fail;
+                }
+                mSettings.xml = (mSettings.outputFormat == Settings::OutputFormat::xml);
+            }
+
+
             // Experimental: limit execution time for extended valueflow analysis. basic valueflow analysis
             // is always executed.
             else if (std::strncmp(argv[i], "--performance-valueflow-max-time=", 33) == 0) {
@@ -949,6 +963,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
 
             // Write results in results.plist
             else if (std::strncmp(argv[i], "--plist-output=", 15) == 0) {
+                mSettings.outputFormat = Settings::OutputFormat::plist;
                 mSettings.plistOutput = Path::simplifyPath(argv[i] + 15);
                 if (mSettings.plistOutput.empty())
                     mSettings.plistOutput = ".";
@@ -1361,8 +1376,10 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
                 mSettings.verbose = true;
 
             // Write results in results.xml
-            else if (std::strcmp(argv[i], "--xml") == 0)
+            else if (std::strcmp(argv[i], "--xml") == 0) {
                 mSettings.xml = true;
+                mSettings.outputFormat = Settings::OutputFormat::xml;
+            }
 
             // Define the XML file version (and enable XML output)
             else if (std::strncmp(argv[i], "--xml-version=", 14) == 0) {
@@ -1378,6 +1395,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
                 mSettings.xml_version = tmp;
                 // Enable also XML if version is set
                 mSettings.xml = true;
+                mSettings.outputFormat = Settings::OutputFormat::xml;
             }
 
             else {
@@ -1623,6 +1641,10 @@ void CmdLineParser::printHelp() const
         "                         is 2. A larger value will mean more errors can be found\n"
         "                         but also means the analysis will be slower.\n"
         "    --output-file=<file> Write results to file, rather than standard error.\n"
+        "    --output-format=<format>\n"
+        "                        Specify the output format. The available formats are:\n"
+        "                          * sarif\n"
+        "                          * xml\n"
         "    --platform=<type>, --platform=<file>\n"
         "                         Specifies platform specific types and sizes. The\n"
         "                         available builtin platforms are:\n"

cli/cppcheckexecutor.cpp

@@ -28,6 +28,7 @@
 #include "errorlogger.h"
 #include "errortypes.h"
 #include "filesettings.h"
+#include "json.h"
 #include "settings.h"
 #include "singleexecutor.h"
 #include "suppressions.h"
@@ -71,6 +72,152 @@
 #endif
 
 namespace {
+    class SarifReport {
+    public:
+        void addFinding(ErrorMessage msg) {
+            mFindings.push_back(std::move(msg));
+        }
+
+        picojson::array serializeRules() const {
+            picojson::array ret;
+            std::set<std::string> ruleIds;
+            for (const auto& finding : mFindings) {
+                // github only supports findings with locations
+                if (finding.callStack.empty())
+                    continue;
+                if (ruleIds.insert(finding.id).second) {
+                    picojson::object rule;
+                    rule["id"] = picojson::value(finding.id);
+                    // rule.shortDescription.text
+                    picojson::object shortDescription;
+                    shortDescription["text"] = picojson::value(finding.shortMessage());
+                    rule["shortDescription"] = picojson::value(shortDescription);
+                    // rule.fullDescription.text
+                    picojson::object fullDescription;
+                    fullDescription["text"] = picojson::value(finding.verboseMessage());
+                    rule["fullDescription"] = picojson::value(fullDescription);
+                    // rule.help.text
+                    picojson::object help;
+                    help["text"] = picojson::value(finding.verboseMessage()); // FIXME provide proper help text
+                    rule["help"] = picojson::value(help);
+                    // rule.properties.precision, rule.properties.problem.severity
+                    picojson::object properties;
+                    properties["precision"] = picojson::value(sarifPrecision(finding));
+                    double securitySeverity = 0;
+                    if (finding.severity == Severity::error && !ErrorLogger::isCriticalErrorId(finding.id))
+                        securitySeverity = 9.9; // We see undefined behavior
+                    //else if (finding.severity == Severity::warning)
+                    //    securitySeverity = 5.1; // We see potential undefined behavior
+                    if (securitySeverity > 0.5) {
+                        properties["security-severity"] = picojson::value(securitySeverity);
+                        const picojson::array tags{picojson::value("security")};
+                        properties["tags"] = picojson::value(tags);
+                    }
+                    rule["properties"] = picojson::value(properties);
+
+                    ret.emplace_back(rule);
+                }
+            }
+            return ret;
+        }
+
+        static picojson::array serializeLocations(const ErrorMessage& finding) {
+            picojson::array ret;
+            for (const auto& location : finding.callStack) {
+                picojson::object physicalLocation;
+                picojson::object artifactLocation;
+                artifactLocation["uri"] = picojson::value(location.getfile(false));
+                physicalLocation["artifactLocation"] = picojson::value(artifactLocation);
+                picojson::object region;
+                region["startLine"] = picojson::value(static_cast<int64_t>(location.line));
+                region["startColumn"] = picojson::value(static_cast<int64_t>(location.column));
+                region["endLine"] = region["startLine"];
+                region["endColumn"] = region["startColumn"];
+                physicalLocation["region"] = picojson::value(region);
+                picojson::object loc;
+                loc["physicalLocation"] = picojson::value(physicalLocation);
+                ret.emplace_back(loc);
+            }
+            return ret;
+        }
+
+        picojson::array serializeResults() const {
+            picojson::array results;
+            for (const auto& finding : mFindings) {
+                // github only supports findings with locations
+                if (finding.callStack.empty())
+                    continue;
+                picojson::object res;
+                res["level"] = picojson::value(sarifSeverity(finding));
+                res["locations"] = picojson::value(serializeLocations(finding));
+                picojson::object message;
+                message["text"] = picojson::value(finding.shortMessage());
+                res["message"] = picojson::value(message);
+                res["ruleId"] = picojson::value(finding.id);
+                results.emplace_back(res);
+            }
+            return results;
+        }
+
+        picojson::value serializeRuns(const std::string& productName, const std::string& version) const {
+            picojson::object driver;
+            driver["name"] = picojson::value(productName);
+            driver["semanticVersion"] = picojson::value(version);
+            driver["informationUri"] = picojson::value("https://cppcheck.sourceforge.io");
+            driver["rules"] = picojson::value(serializeRules());
+            picojson::object tool;
+            tool["driver"] = picojson::value(driver);
+            picojson::object run;
+            run["tool"] = picojson::value(tool);
+            run["results"] = picojson::value(serializeResults());
+            picojson::array runs{picojson::value(run)};
+            return picojson::value(runs);
+        }
+
+        std::string serialize(std::string productName) const {
+            const auto nameAndVersion = Settings::getNameAndVersion(productName);
+            productName = nameAndVersion.first.empty() ? "Cppcheck" : nameAndVersion.first;
+            std::string version = nameAndVersion.first.empty() ? CppCheck::version() : nameAndVersion.second;
+            if (version.find(' ') != std::string::npos)
+                version.erase(version.find(' '), std::string::npos);
+
+            picojson::object doc;
+            doc["version"] = picojson::value("2.1.0");
+            doc["$schema"] = picojson::value("https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/schemas/sarif-schema-2.1.0.json");
+            doc["runs"] = serializeRuns(productName, version);
+
+            return picojson::value(doc).serialize(true);
+        }
+    private:
+
+        static std::string sarifSeverity(const ErrorMessage& errmsg) {
+            if (ErrorLogger::isCriticalErrorId(errmsg.id))
+                return "error";
+            switch (errmsg.severity) {
+            case Severity::error:
+            case Severity::warning:
+            case Severity::style:
+            case Severity::portability:
+            case Severity::performance:
+                return "warning";
+            case Severity::information:
+            case Severity::internal:
+            case Severity::debug:
+            case Severity::none:
+                return "note";
+            }
+            return "note";
+        }
+
+        static std::string sarifPrecision(const ErrorMessage& errmsg) {
+            if (errmsg.certainty == Certainty::inconclusive)
+                return "medium";
+            return "high";
+        }
+
+        std::vector<ErrorMessage> mFindings;
+    };
+
     class CmdLineLoggerStd : public CmdLineLogger
     {
     public:
@@ -104,6 +251,9 @@ namespace {
         }
 
         ~StdLogger() override {
+            if (mSettings.outputFormat == Settings::OutputFormat::sarif) {
+                reportErr(mSarifReport.serialize(mSettings.cppcheckCfgProductName));
+            }
             delete mErrorOutput;
         }
 
@@ -182,6 +332,11 @@ namespace {
          * CTU information
          */
         std::string mCtuInfo;
+
+        /**
+         * SARIF report generator
+         */
+        SarifReport mSarifReport;
     };
 }
 
@@ -455,7 +610,9 @@ void StdLogger::reportErr(const ErrorMessage &msg)
     if (!mShownErrors.insert(msg.toString(mSettings.verbose)).second)
         return;
 
-    if (mSettings.xml)
+    if (mSettings.outputFormat == Settings::OutputFormat::sarif)
+        mSarifReport.addFinding(msg);
+    else if (mSettings.xml)
         reportErr(msg.toXML());
     else
         reportErr(msg.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation));

lib/settings.h

@@ -273,6 +273,9 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     /** @brief write results (--output-file=<file>) */
     std::string outputFile;
 
+    enum class OutputFormat : std::uint8_t {text, plist, sarif, xml};
+    OutputFormat outputFormat = OutputFormat::text;
+
     Platform platform;
 
     /** @brief pid of cppcheck. Intention is that this is set in the main process. */

releasenotes.txt

@@ -10,6 +10,8 @@ GUI:
 -
 
 Changed interface:
+- SARIF output. Use --output-format=sarif to activate this.
+- Add option --output-format=<format>. Allowed formats are sarif and xml.
 -
 
 Deprecations:

samples/incorrectLogicOperator/bad.c

@@ -0,0 +1,6 @@
+
+void foo(int x) {
+    if (x >= 0 || x <= 10) {}
+}
+
+dummy=foo();

samples/incorrectLogicOperator/good.c

@@ -0,0 +1,6 @@
+
+void foo(int x) {
+    if (x >= 0 && x <= 10) {}
+}
+
+dummy=foo();

samples/incorrectLogicOperator/out.txt

@@ -0,0 +1,3 @@
+samples\incorrectLogicOperator\bad.c:3:16: warning: Logical disjunction always evaluates to true: x >= 0 || x <= 10. [incorrectLogicOperator]
+    if (x >= 0 || x <= 10) {}
+               ^

test/cli/helloworld_test.py

@@ -4,6 +4,7 @@
 import os
 import re
 import glob
+import json
 
 from testutils import create_gui_project_file, cppcheck
 
@@ -319,3 +320,21 @@ def test_missing_include_system():  # #11283
 
     _, _, stderr = cppcheck(args, cwd=__script_dir)
     assert stderr.replace('\\', '/') == 'helloworld/main.c:1:0: information: Include file: <stdio.h> not found. Please note: Cppcheck does not need standard library headers to get proper results. [missingIncludeSystem]\n'
+
+
+def test_sarif():
+    args = [
+        '--output-format=sarif',
+        'helloworld'
+    ]
+    ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
+    assert ret == 0, stdout
+    res = json.loads(stderr)
+    assert res['version'] == '2.1.0'
+    assert res['runs'][0]['results'][0]['locations'][0]['physicalLocation']['artifactLocation']['uri'] == 'helloworld/main.c'
+    assert res['runs'][0]['results'][0]['ruleId'] == 'zerodiv'
+    assert res['runs'][0]['tool']['driver']['rules'][0]['id'] == 'zerodiv'
+    assert res['runs'][0]['tool']['driver']['rules'][0]['properties']['precision'] == 'high'
+    assert res['runs'][0]['tool']['driver']['rules'][0]['properties']['security-severity'] > 9.5
+    assert 'security' in res['runs'][0]['tool']['driver']['rules'][0]['properties']['tags']
+    assert re.match(r'[0-9]+(.[0-9]+)+', res['runs'][0]['tool']['driver']['semanticVersion'])