Skip to content

Commit 92fffee

Browse files
authored
Fix first instance rule being used as rule description for all violations of that rule and other SARIF improvements (#7640)
* Fixed issue where the first instance of a rule violation short description would get used for all subsequent rule violations, found that if you make all the rule name and descriptions empty strings, github will default to the instance descriptions * Added setting for problem.severity * Changed defaultConfiguration.level to use problem.severity instead of security-severity * Added more levels for security-severity * Added reporting of cwe ID * Fixed issue with uncrustify version detection * Added unit tests for sarif output Before: ![cppcheck_original_problem](https://github.com/user-attachments/assets/badb787a-0f2c-4809-a41f-a1d1b66b39d3) After: <img width="925" height="1636" alt="image" src="https://github.com/user-attachments/assets/d40f5f9f-186c-4367-9290-9d73cbd23d55" />
1 parent 1023d9b commit 92fffee

File tree

13 files changed

+1817
-157
lines changed

13 files changed

+1817
-157
lines changed

AUTHORS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,7 @@ Tommy Bergman
411411
Toralf Förster
412412
Troshin V.S.
413413
Tyson Nottingham
414+
Usman Majid
414415
Valentin Batz
415416
Valerii Lashmanov
416417
Vasily Maslyukov

Makefile

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ ifndef INCLUDE_FOR_CLI
176176
endif
177177

178178
ifndef INCLUDE_FOR_TEST
179-
INCLUDE_FOR_TEST=-Ilib -Ifrontend -Icli -isystem externals/simplecpp -isystem externals/tinyxml2
179+
INCLUDE_FOR_TEST=-Ilib -Ifrontend -Icli -isystem externals/picojson -isystem externals/simplecpp -isystem externals/tinyxml2
180180
endif
181181

182182
ifndef CFLAGS_FOR_TEST
@@ -249,6 +249,7 @@ LIBOBJ = $(libcppdir)/valueflow.o \
249249
$(libcppdir)/programmemory.o \
250250
$(libcppdir)/regex.o \
251251
$(libcppdir)/reverseanalyzer.o \
252+
$(libcppdir)/sarifreport.o \
252253
$(libcppdir)/settings.o \
253254
$(libcppdir)/standards.o \
254255
$(libcppdir)/summaries.o \
@@ -327,6 +328,7 @@ TESTOBJ = test/fixture.o \
327328
test/testprocessexecutor.o \
328329
test/testprogrammemory.o \
329330
test/testregex.o \
331+
test/testsarifreport.o \
330332
test/testsettings.o \
331333
test/testsimplifytemplate.o \
332334
test/testsimplifytokens.o \
@@ -638,6 +640,9 @@ $(libcppdir)/regex.o: lib/regex.cpp lib/config.h lib/regex.h
638640
$(libcppdir)/reverseanalyzer.o: lib/reverseanalyzer.cpp lib/addoninfo.h lib/analyzer.h lib/astutils.h lib/checkers.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/library.h lib/mathlib.h lib/platform.h lib/reverseanalyzer.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueptr.h lib/vfvalue.h
639641
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/reverseanalyzer.cpp
640642

643+
$(libcppdir)/sarifreport.o: lib/sarifreport.cpp externals/picojson/picojson.h lib/addoninfo.h lib/check.h lib/checkers.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/json.h lib/library.h lib/mathlib.h lib/platform.h lib/sarifreport.h lib/settings.h lib/standards.h lib/utils.h
644+
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/sarifreport.cpp
645+
641646
$(libcppdir)/settings.o: lib/settings.cpp externals/picojson/picojson.h lib/addoninfo.h lib/checkers.h lib/config.h lib/errortypes.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/summaries.h lib/suppressions.h lib/utils.h lib/vfvalue.h
642647
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/settings.cpp
643648

@@ -683,7 +688,7 @@ frontend/frontend.o: frontend/frontend.cpp frontend/frontend.h lib/addoninfo.h l
683688
cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/filelister.h externals/tinyxml2/tinyxml2.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/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/regex.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h lib/xml.h
684689
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cmdlineparser.cpp
685690

686-
cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h cli/sehwrapper.h cli/signalhandler.h cli/singleexecutor.h cli/threadexecutor.h externals/picojson/picojson.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkers.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
691+
cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h cli/sehwrapper.h cli/signalhandler.h cli/singleexecutor.h cli/threadexecutor.h externals/picojson/picojson.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkers.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/sarifreport.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
687692
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutor.cpp
688693

689694
cli/executor.o: cli/executor.cpp cli/executor.h lib/addoninfo.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
@@ -854,6 +859,9 @@ test/testprogrammemory.o: test/testprogrammemory.cpp lib/addoninfo.h lib/check.h
854859
test/testregex.o: test/testregex.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/regex.h lib/settings.h lib/standards.h lib/utils.h test/fixture.h
855860
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testregex.cpp
856861

862+
test/testsarifreport.o: test/testsarifreport.cpp externals/picojson/picojson.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/sarifreport.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
863+
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsarifreport.cpp
864+
857865
test/testsettings.o: test/testsettings.cpp lib/addoninfo.h lib/check.h lib/checkers.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/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
858866
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsettings.cpp
859867

cli/cppcheckexecutor.cpp

Lines changed: 7 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "filesettings.h"
3535
#include "json.h"
3636
#include "path.h"
37+
#include "sarifreport.h"
3738
#include "settings.h"
3839
#include "singleexecutor.h"
3940
#include "suppressions.h"
@@ -79,156 +80,6 @@
7980
#endif
8081

8182
namespace {
82-
class SarifReport {
83-
public:
84-
void addFinding(ErrorMessage msg) {
85-
mFindings.push_back(std::move(msg));
86-
}
87-
88-
picojson::array serializeRules() const {
89-
picojson::array ret;
90-
std::set<std::string> ruleIds;
91-
for (const auto& finding : mFindings) {
92-
// github only supports findings with locations
93-
if (finding.callStack.empty())
94-
continue;
95-
if (ruleIds.insert(finding.id).second) {
96-
picojson::object rule;
97-
rule["id"] = picojson::value(finding.id);
98-
// rule.shortDescription.text
99-
picojson::object shortDescription;
100-
shortDescription["text"] = picojson::value(finding.shortMessage());
101-
rule["shortDescription"] = picojson::value(shortDescription);
102-
// rule.fullDescription.text
103-
picojson::object fullDescription;
104-
fullDescription["text"] = picojson::value(finding.verboseMessage());
105-
rule["fullDescription"] = picojson::value(fullDescription);
106-
// rule.help.text
107-
picojson::object help;
108-
help["text"] = picojson::value(finding.verboseMessage()); // FIXME provide proper help text
109-
rule["help"] = picojson::value(help);
110-
// rule.properties.precision, rule.properties.problem.severity
111-
picojson::object properties;
112-
properties["precision"] = picojson::value(sarifPrecision(finding));
113-
const char* securitySeverity = nullptr;
114-
if (finding.severity == Severity::error && !ErrorLogger::isCriticalErrorId(finding.id))
115-
securitySeverity = "9.9"; // We see undefined behavior
116-
//else if (finding.severity == Severity::warning)
117-
// securitySeverity = 5.1; // We see potential undefined behavior
118-
if (securitySeverity) {
119-
properties["security-severity"] = picojson::value(securitySeverity);
120-
const picojson::array tags{picojson::value("security")};
121-
properties["tags"] = picojson::value(tags);
122-
}
123-
rule["properties"] = picojson::value(properties);
124-
// rule.defaultConfiguration.level
125-
picojson::object defaultConfiguration;
126-
defaultConfiguration["level"] = picojson::value(sarifSeverity(finding));
127-
rule["defaultConfiguration"] = picojson::value(defaultConfiguration);
128-
129-
ret.emplace_back(rule);
130-
}
131-
}
132-
return ret;
133-
}
134-
135-
static picojson::array serializeLocations(const ErrorMessage& finding) {
136-
picojson::array ret;
137-
for (const auto& location : finding.callStack) {
138-
picojson::object physicalLocation;
139-
picojson::object artifactLocation;
140-
artifactLocation["uri"] = picojson::value(location.getfile(false));
141-
physicalLocation["artifactLocation"] = picojson::value(artifactLocation);
142-
picojson::object region;
143-
region["startLine"] = picojson::value(static_cast<int64_t>(location.line < 1 ? 1 : location.line));
144-
region["startColumn"] = picojson::value(static_cast<int64_t>(location.column < 1 ? 1 : location.column));
145-
region["endLine"] = region["startLine"];
146-
region["endColumn"] = region["startColumn"];
147-
physicalLocation["region"] = picojson::value(region);
148-
picojson::object loc;
149-
loc["physicalLocation"] = picojson::value(physicalLocation);
150-
ret.emplace_back(loc);
151-
}
152-
return ret;
153-
}
154-
155-
picojson::array serializeResults() const {
156-
picojson::array results;
157-
for (const auto& finding : mFindings) {
158-
// github only supports findings with locations
159-
if (finding.callStack.empty())
160-
continue;
161-
picojson::object res;
162-
res["level"] = picojson::value(sarifSeverity(finding));
163-
res["locations"] = picojson::value(serializeLocations(finding));
164-
picojson::object message;
165-
message["text"] = picojson::value(finding.shortMessage());
166-
res["message"] = picojson::value(message);
167-
res["ruleId"] = picojson::value(finding.id);
168-
results.emplace_back(res);
169-
}
170-
return results;
171-
}
172-
173-
picojson::value serializeRuns(const std::string& productName, const std::string& version) const {
174-
picojson::object driver;
175-
driver["name"] = picojson::value(productName);
176-
driver["semanticVersion"] = picojson::value(version);
177-
driver["informationUri"] = picojson::value("https://cppcheck.sourceforge.io");
178-
driver["rules"] = picojson::value(serializeRules());
179-
picojson::object tool;
180-
tool["driver"] = picojson::value(driver);
181-
picojson::object run;
182-
run["tool"] = picojson::value(tool);
183-
run["results"] = picojson::value(serializeResults());
184-
picojson::array runs{picojson::value(run)};
185-
return picojson::value(runs);
186-
}
187-
188-
std::string serialize(std::string productName) const {
189-
const auto nameAndVersion = Settings::getNameAndVersion(productName);
190-
productName = nameAndVersion.first.empty() ? "Cppcheck" : nameAndVersion.first;
191-
std::string version = nameAndVersion.first.empty() ? CppCheck::version() : nameAndVersion.second;
192-
if (version.find(' ') != std::string::npos)
193-
version.erase(version.find(' '), std::string::npos);
194-
195-
picojson::object doc;
196-
doc["version"] = picojson::value("2.1.0");
197-
doc["$schema"] = picojson::value("https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/schemas/sarif-schema-2.1.0.json");
198-
doc["runs"] = serializeRuns(productName, version);
199-
200-
return picojson::value(doc).serialize(true);
201-
}
202-
private:
203-
204-
static std::string sarifSeverity(const ErrorMessage& errmsg) {
205-
if (ErrorLogger::isCriticalErrorId(errmsg.id))
206-
return "error";
207-
switch (errmsg.severity) {
208-
case Severity::error:
209-
case Severity::warning:
210-
case Severity::style:
211-
case Severity::portability:
212-
case Severity::performance:
213-
return "warning";
214-
case Severity::information:
215-
case Severity::internal:
216-
case Severity::debug:
217-
case Severity::none:
218-
return "note";
219-
}
220-
return "note";
221-
}
222-
223-
static std::string sarifPrecision(const ErrorMessage& errmsg) {
224-
if (errmsg.certainty == Certainty::inconclusive)
225-
return "medium";
226-
return "high";
227-
}
228-
229-
std::vector<ErrorMessage> mFindings;
230-
};
231-
23283
class CmdLineLoggerStd : public CmdLineLogger
23384
{
23485
public:
@@ -712,18 +563,20 @@ void StdLogger::reportErr(const ErrorMessage &msg)
712563
msgCopy.classification = getClassification(msgCopy.guideline, mSettings.reportType);
713564

714565
// TODO: there should be no need for verbose and default messages here
715-
const std::string msgStr = msgCopy.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation);
566+
const std::string msgStr =
567+
msgCopy.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation);
716568

717569
// Alert only about unique errors
718570
if (!mSettings.emitDuplicates && !mShownErrors.insert(msgStr).second)
719571
return;
720572

721-
if (mSettings.outputFormat == Settings::OutputFormat::sarif)
573+
if (mSettings.outputFormat == Settings::OutputFormat::sarif) {
722574
mSarifReport.addFinding(std::move(msgCopy));
723-
else if (mSettings.outputFormat == Settings::OutputFormat::xml)
575+
} else if (mSettings.outputFormat == Settings::OutputFormat::xml) {
724576
reportErr(msgCopy.toXML());
725-
else
577+
} else {
726578
reportErr(msgStr);
579+
}
727580
}
728581

729582
/**

lib/cppcheck.vcxproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
<ClCompile Include="programmemory.cpp" />
8282
<ClCompile Include="regex.cpp" />
8383
<ClCompile Include="reverseanalyzer.cpp" />
84+
<ClCompile Include="sarifreport.cpp" />
8485
<ClCompile Include="settings.cpp" />
8586
<ClCompile Include="standards.cpp" />
8687
<ClCompile Include="summaries.cpp" />
@@ -158,6 +159,7 @@
158159
<ClInclude Include="programmemory.h" />
159160
<ClInclude Include="regex.h" />
160161
<ClInclude Include="reverseanalyzer.h" />
162+
<ClInclude Include="sarifreport.h" />
161163
<ClInclude Include="settings.h" />
162164
<ClInclude Include="smallvector.h" />
163165
<ClInclude Include="sourcelocation.h" />

0 commit comments

Comments
 (0)