Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -762,7 +762,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
Expand Down
1 change: 1 addition & 0 deletions cli/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions cli/cli.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
24 changes: 23 additions & 1 deletion cli/cmdlineparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 = ".";
Expand Down Expand Up @@ -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) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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"
Expand Down
160 changes: 159 additions & 1 deletion cli/cppcheckexecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -71,6 +72,153 @@
#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));
properties["problem.severity"] = picojson::value(sarifSeverity(finding));
danmar marked this conversation as resolved.
Show resolved Hide resolved
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));
danmar marked this conversation as resolved.
Show resolved Hide resolved
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:
Expand Down Expand Up @@ -104,6 +252,9 @@ namespace {
}

~StdLogger() override {
if (mSettings.outputFormat == Settings::OutputFormat::sarif) {
reportErr(mSarifReport.serialize(mSettings.cppcheckCfgProductName));
}
delete mErrorOutput;
}

Expand Down Expand Up @@ -182,6 +333,11 @@ namespace {
* CTU information
*/
std::string mCtuInfo;

/**
* SARIF report generator
*/
SarifReport mSarifReport;
};
}

Expand Down Expand Up @@ -455,7 +611,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));
Expand Down
3 changes: 3 additions & 0 deletions lib/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,9 @@ class CPPCHECKLIB WARN_UNUSED Settings {
/** @brief write results (--output-file=&lt;file&gt;) */
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. */
Expand Down
2 changes: 2 additions & 0 deletions releasenotes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions samples/incorrectLogicOperator/bad.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

void foo(int x) {
if (x >= 0 || x <= 10) {}

Check warning

Code scanning / Cppcheck

Logical disjunction always evaluates to true: x >= 0 || x <= 10.

Logical disjunction always evaluates to true: x >= 0 || x <= 10.
}

dummy=foo();
6 changes: 6 additions & 0 deletions samples/incorrectLogicOperator/good.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

void foo(int x) {
if (x >= 0 && x <= 10) {}
}

dummy=foo();
3 changes: 3 additions & 0 deletions samples/incorrectLogicOperator/out.txt
Original file line number Diff line number Diff line change
@@ -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) {}
^
20 changes: 20 additions & 0 deletions test/cli/helloworld_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
import re
import glob
import json

from testutils import create_gui_project_file, cppcheck

Expand Down Expand Up @@ -319,3 +320,22 @@ 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']['problem.severity'] == 'warning'
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'])
Loading
Loading