Skip to content

Commit 505c3ec

Browse files
firewavefirewave
authored
refs #12171 / fixed #12347 - improved handling of (unmatched) local and inline suppressions (danmar#5647)
1 parent 4e724af commit 505c3ec

13 files changed

+352
-228
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,7 @@ cli/singleexecutor.o: cli/singleexecutor.cpp cli/executor.h cli/singleexecutor.h
682682
cli/stacktrace.o: cli/stacktrace.cpp cli/stacktrace.h lib/config.h lib/utils.h
683683
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/stacktrace.cpp
684684

685-
cli/threadexecutor.o: cli/threadexecutor.cpp cli/executor.h cli/threadexecutor.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/settings.h lib/standards.h lib/timer.h lib/utils.h
685+
cli/threadexecutor.o: cli/threadexecutor.cpp cli/executor.h cli/threadexecutor.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/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
686686
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/threadexecutor.cpp
687687

688688
test/fixture.o: test/fixture.cpp externals/simplecpp/simplecpp.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/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/xml.h test/fixture.h test/helpers.h test/options.h test/redirect.h

‎cli/cppcheckexecutor.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,12 @@ bool CppCheckExecutor::reportSuppressions(const Settings &settings, const Suppre
412412
suppressions.getUnmatchedLocalSuppressions(i->file, unusedFunctionCheckEnabled), errorLogger);
413413
}
414414
}
415+
if (settings.inlineSuppressions) {
416+
// report unmatched unusedFunction suppressions
417+
err |= SuppressionList::reportUnmatchedSuppressions(
418+
suppressions.getUnmatchedInlineSuppressions(), errorLogger);
419+
}
420+
415421
err |= SuppressionList::reportUnmatchedSuppressions(suppressions.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled), errorLogger);
416422
return err;
417423
}

‎cli/processexecutor.cpp‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ ProcessExecutor::ProcessExecutor(const std::list<FileWithDetails> &files, const
7878
namespace {
7979
class PipeWriter : public ErrorLogger {
8080
public:
81-
enum PipeSignal : std::uint8_t {REPORT_OUT='1',REPORT_ERROR='2', CHILD_END='5'};
81+
enum PipeSignal : std::uint8_t {REPORT_OUT='1',REPORT_ERROR='2',REPORT_SUPPR_INLINE='3',CHILD_END='5'};
8282

8383
explicit PipeWriter(int pipe) : mWpipe(pipe) {}
8484

@@ -90,11 +90,32 @@ namespace {
9090
writeToPipe(REPORT_ERROR, msg.serialize());
9191
}
9292

93+
void writeSuppr(const SuppressionList &supprs) const {
94+
for (const auto& suppr : supprs.getSuppressions())
95+
{
96+
if (!suppr.isInline)
97+
continue;
98+
99+
writeToPipe(REPORT_SUPPR_INLINE, suppressionToString(suppr));
100+
}
101+
// TODO: update suppression states?
102+
}
103+
93104
void writeEnd(const std::string& str) const {
94105
writeToPipe(CHILD_END, str);
95106
}
96107

97108
private:
109+
static std::string suppressionToString(const SuppressionList::Suppression &suppr)
110+
{
111+
std::string suppr_str = suppr.toString();
112+
suppr_str += ";";
113+
suppr_str += suppr.checked ? "1" : "0";
114+
suppr_str += ";";
115+
suppr_str += suppr.matched ? "1" : "0";
116+
return suppr_str;
117+
}
118+
98119
// TODO: how to log file name in error?
99120
void writeToPipeInternal(PipeSignal type, const void* data, std::size_t to_write) const
100121
{
@@ -124,7 +145,7 @@ namespace {
124145
writeToPipeInternal(type, &len, l_size);
125146
}
126147

127-
if (len > 0)
148+
if (len > 0) // TODO: unexpected - write a warning?
128149
writeToPipeInternal(type, data.c_str(), len);
129150
}
130151

@@ -155,7 +176,10 @@ bool ProcessExecutor::handleRead(int rpipe, unsigned int &result, const std::str
155176
std::exit(EXIT_FAILURE);
156177
}
157178

158-
if (type != PipeWriter::REPORT_OUT && type != PipeWriter::REPORT_ERROR && type != PipeWriter::CHILD_END) {
179+
if (type != PipeWriter::REPORT_OUT &&
180+
type != PipeWriter::REPORT_ERROR &&
181+
type != PipeWriter::REPORT_SUPPR_INLINE &&
182+
type != PipeWriter::CHILD_END) {
159183
std::cerr << "#### ThreadExecutor::handleRead(" << filename << ") invalid type " << int(type) << std::endl;
160184
std::exit(EXIT_FAILURE);
161185
}
@@ -174,7 +198,7 @@ bool ProcessExecutor::handleRead(int rpipe, unsigned int &result, const std::str
174198
}
175199

176200
std::string buf(len, '\0');
177-
if (len > 0) {
201+
if (len > 0) { // TODO: unexpected - write a warning?
178202
char *data_start = &buf[0];
179203
bytes_to_read = len;
180204
do {
@@ -206,6 +230,29 @@ bool ProcessExecutor::handleRead(int rpipe, unsigned int &result, const std::str
206230

207231
if (hasToLog(msg))
208232
mErrorLogger.reportErr(msg);
233+
} else if (type == PipeWriter::REPORT_SUPPR_INLINE) {
234+
if (!buf.empty()) {
235+
// TODO: avoid string splitting
236+
auto parts = splitString(buf, ';');
237+
if (parts.size() != 3)
238+
{
239+
// TODO: make this non-fatal
240+
std::cerr << "#### ThreadExecutor::handleRead(" << filename << ") adding of inline suppression failed - insufficient data" << std::endl;
241+
std::exit(EXIT_FAILURE);
242+
}
243+
auto suppr = SuppressionList::parseLine(parts[0]);
244+
suppr.isInline = true;
245+
suppr.checked = parts[1] == "1";
246+
suppr.matched = parts[2] == "1";
247+
const std::string err = mSuppressions.nomsg.addSuppression(suppr);
248+
if (!err.empty()) {
249+
// TODO: only update state if it doesn't exist - otherwise propagate error
250+
mSuppressions.nomsg.updateSuppressionState(suppr); // TODO: check result
251+
// TODO: make this non-fatal
252+
//std::cerr << "#### ThreadExecutor::handleRead(" << filename << ") adding of inline suppression failed - " << err << std::endl;
253+
//std::exit(EXIT_FAILURE);
254+
}
255+
}
209256
} else if (type == PipeWriter::CHILD_END) {
210257
result += std::stoi(buf);
211258
res = false;
@@ -297,6 +344,8 @@ unsigned int ProcessExecutor::check()
297344
// TODO: call analyseClangTidy()?
298345
}
299346

347+
pipewriter.writeSuppr(mSuppressions.nomsg);
348+
300349
pipewriter.writeEnd(std::to_string(resultOfCheck));
301350
std::exit(EXIT_SUCCESS);
302351
}

‎cli/threadexecutor.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "errorlogger.h"
2424
#include "filesettings.h"
2525
#include "settings.h"
26+
#include "suppressions.h"
2627
#include "timer.h"
2728

2829
#include <cassert>
@@ -125,6 +126,23 @@ class ThreadData
125126
result = fileChecker.check(*file);
126127
// TODO: call analyseClangTidy()?
127128
}
129+
for (const auto& suppr : mSuppressions.nomsg.getSuppressions()) {
130+
// need to transfer all inline suppressions because these are used later on
131+
if (suppr.isInline) {
132+
const std::string err = mSuppressions.nomsg.addSuppression(suppr);
133+
if (!err.empty()) {
134+
// TODO: only update state if it doesn't exist - otherwise propagate error
135+
mSuppressions.nomsg.updateSuppressionState(suppr); // TODO: check result
136+
}
137+
continue;
138+
}
139+
140+
// propagate state of global suppressions
141+
if (!suppr.isLocal()) {
142+
mSuppressions.nomsg.updateSuppressionState(suppr); // TODO: check result
143+
continue;
144+
}
145+
}
128146
return result;
129147
}
130148

‎lib/cppcheck.cpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,8 +1168,10 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
11681168
fdump << "</dump>" << std::endl;
11691169
}
11701170

1171-
// Need to call this even if the hash will skip this configuration
1172-
mSuppressions.nomsg.markUnmatchedInlineSuppressionsAsChecked(tokenizer);
1171+
if (mSettings.inlineSuppressions) {
1172+
// Need to call this even if the hash will skip this configuration
1173+
mSuppressions.nomsg.markUnmatchedInlineSuppressionsAsChecked(tokenizer);
1174+
}
11731175

11741176
// Skip if we already met the same simplified token list
11751177
if (mSettings.force || mSettings.maxConfigs > 1) {
@@ -1258,10 +1260,14 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
12581260
mErrorLogger.reportErr(errmsg);
12591261
}
12601262

1261-
// In jointSuppressionReport mode, unmatched suppressions are
1262-
// collected after all files are processed
1263-
if (!mSettings.useSingleJob() && (mSettings.severity.isEnabled(Severity::information) || mSettings.checkConfiguration)) {
1264-
SuppressionList::reportUnmatchedSuppressions(mSuppressions.nomsg.getUnmatchedLocalSuppressions(file, (bool)mUnusedFunctionsCheck), mErrorLogger);
1263+
// TODO: this is done too early causing the whole program analysis suppressions to be reported as unmatched
1264+
if (mSettings.severity.isEnabled(Severity::information) || mSettings.checkConfiguration) {
1265+
// In jointSuppressionReport mode, unmatched suppressions are
1266+
// collected after all files are processed
1267+
if (!mSettings.useSingleJob()) {
1268+
// TODO: check result?
1269+
SuppressionList::reportUnmatchedSuppressions(mSuppressions.nomsg.getUnmatchedLocalSuppressions(file, (bool)mUnusedFunctionsCheck), mErrorLogger);
1270+
}
12651271
}
12661272

12671273
if (analyzerInformation) {

‎lib/preprocessor.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
135135
std::vector<SuppressionList::Suppression> suppressions = SuppressionList::parseMultiSuppressComment(comment, &errmsg);
136136

137137
for (SuppressionList::Suppression &s : suppressions) {
138+
s.isInline = true;
138139
s.type = errorType;
139140
s.lineNumber = tok->location.line;
140141
}
@@ -152,6 +153,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
152153
if (!s.parseComment(comment, &errmsg))
153154
return false;
154155

156+
s.isInline = true;
155157
s.type = errorType;
156158
s.lineNumber = tok->location.line;
157159

‎lib/suppressions.cpp‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ std::vector<SuppressionList::Suppression> SuppressionList::parseMultiSuppressCom
204204
return suppressions;
205205
}
206206

207-
std::string SuppressionList::addSuppressionLine(const std::string &line)
207+
SuppressionList::Suppression SuppressionList::parseLine(const std::string &line)
208208
{
209209
std::istringstream lineStream;
210210
SuppressionList::Suppression suppression;
@@ -249,7 +249,12 @@ std::string SuppressionList::addSuppressionLine(const std::string &line)
249249

250250
suppression.fileName = Path::simplifyPath(suppression.fileName);
251251

252-
return addSuppression(std::move(suppression));
252+
return suppression;
253+
}
254+
255+
std::string SuppressionList::addSuppressionLine(const std::string &line)
256+
{
257+
return addSuppression(parseLine(line));
253258
}
254259

255260
std::string SuppressionList::addSuppression(SuppressionList::Suppression suppression)
@@ -511,12 +516,14 @@ void SuppressionList::dump(std::ostream & out) const
511516
out << " </suppressions>" << std::endl;
512517
}
513518

514-
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file, const bool unusedFunctionChecking) const
519+
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file, const bool includeUnusedFunction) const
515520
{
516521
std::lock_guard<std::mutex> lg(mSuppressionsSync);
517522

518523
std::list<Suppression> result;
519524
for (const Suppression &s : mSuppressions) {
525+
if (s.isInline)
526+
continue;
520527
if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked))
521528
continue;
522529
if (s.type == SuppressionList::Type::macro)
@@ -525,7 +532,7 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppre
525532
continue;
526533
if (s.errorId == ID_CHECKERSREPORT)
527534
continue;
528-
if (!unusedFunctionChecking && s.errorId == ID_UNUSEDFUNCTION)
535+
if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
529536
continue;
530537
if (!s.isLocal() || s.fileName != file.spath())
531538
continue;
@@ -534,17 +541,19 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppre
534541
return result;
535542
}
536543

537-
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const
544+
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions(const bool includeUnusedFunction) const
538545
{
539546
std::lock_guard<std::mutex> lg(mSuppressionsSync);
540547

541548
std::list<Suppression> result;
542549
for (const Suppression &s : mSuppressions) {
550+
if (s.isInline)
551+
continue;
543552
if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked))
544553
continue;
545554
if (s.hash > 0)
546555
continue;
547-
if (!unusedFunctionChecking && s.errorId == ID_UNUSEDFUNCTION)
556+
if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
548557
continue;
549558
if (s.errorId == ID_CHECKERSREPORT)
550559
continue;
@@ -555,6 +564,23 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppr
555564
return result;
556565
}
557566

567+
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppressions() const
568+
{
569+
std::list<SuppressionList::Suppression> result;
570+
for (const SuppressionList::Suppression &s : SuppressionList::mSuppressions) {
571+
if (!s.isInline)
572+
continue;
573+
if (!s.checked)
574+
continue;
575+
if (s.matched)
576+
continue;
577+
if (s.hash > 0)
578+
continue;
579+
result.push_back(s);
580+
}
581+
return result;
582+
}
583+
558584
std::list<SuppressionList::Suppression> SuppressionList::getSuppressions() const
559585
{
560586
std::lock_guard<std::mutex> lg(mSuppressionsSync);

‎lib/suppressions.h‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ class CPPCHECKLIB SuppressionList {
151151
bool thisAndNextLine{}; // Special case for backwards compatibility: { // cppcheck-suppress something
152152
bool matched{};
153153
bool checked{}; // for inline suppressions, checked or not
154+
bool isInline{};
154155

155156
enum : std::int8_t { NO_LINE = -1 };
156157
};
@@ -177,6 +178,13 @@ class CPPCHECKLIB SuppressionList {
177178
*/
178179
static std::vector<Suppression> parseMultiSuppressComment(const std::string &comment, std::string *errorMessage);
179180

181+
/**
182+
* Create a Suppression object from a suppression line
183+
* @param line The line to parse.
184+
* @return a suppression object
185+
*/
186+
static Suppression parseLine(const std::string &line);
187+
180188
/**
181189
* @brief Don't show the given error.
182190
* @param line Description of error to suppress (in id:file:line format).
@@ -239,13 +247,19 @@ class CPPCHECKLIB SuppressionList {
239247
* @brief Returns list of unmatched local (per-file) suppressions.
240248
* @return list of unmatched suppressions
241249
*/
242-
std::list<Suppression> getUnmatchedLocalSuppressions(const FileWithDetails &file, bool unusedFunctionChecking) const;
250+
std::list<Suppression> getUnmatchedLocalSuppressions(const FileWithDetails &file, bool includeUnusedFunction) const;
243251

244252
/**
245253
* @brief Returns list of unmatched global (glob pattern) suppressions.
246254
* @return list of unmatched suppressions
247255
*/
248-
std::list<Suppression> getUnmatchedGlobalSuppressions(bool unusedFunctionChecking) const;
256+
std::list<Suppression> getUnmatchedGlobalSuppressions(bool includeUnusedFunction) const;
257+
258+
/**
259+
* @brief Returns list of unmatched inline suppressions.
260+
* @return list of unmatched suppressions
261+
*/
262+
std::list<Suppression> getUnmatchedInlineSuppressions() const;
249263

250264
/**
251265
* @brief Returns list of all suppressions.

0 commit comments

Comments
 (0)