Skip to content

Commit 38fb579

Browse files
firewavefirewave
committed
SuppressionList: fixed data race
1 parent 0bf52f0 commit 38fb579

3 files changed

Lines changed: 35 additions & 9 deletions

File tree

‎lib/suppressions.cpp‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,8 @@ std::string SuppressionList::addSuppressionLine(const std::string &line)
254254

255255
std::string SuppressionList::addSuppression(SuppressionList::Suppression suppression)
256256
{
257+
std::lock_guard<std::mutex> lg(mSuppressionsSync);
258+
257259
// Check if suppression is already in list
258260
auto foundSuppression = std::find_if(mSuppressions.begin(), mSuppressions.end(),
259261
std::bind(&Suppression::isSameParameters, &suppression, std::placeholders::_1));
@@ -297,6 +299,8 @@ std::string SuppressionList::addSuppressions(std::list<Suppression> suppressions
297299
// cppcheck-suppress unusedFunction
298300
bool SuppressionList::updateSuppressionState(const SuppressionList::Suppression& suppression)
299301
{
302+
std::lock_guard<std::mutex> lg(mSuppressionsSync);
303+
300304
// Check if suppression is already in list
301305
auto foundSuppression = std::find_if(mSuppressions.begin(), mSuppressions.end(),
302306
std::bind(&Suppression::isSameParameters, &suppression, std::placeholders::_1));
@@ -431,6 +435,8 @@ std::string SuppressionList::Suppression::getText() const
431435

432436
bool SuppressionList::isSuppressed(const SuppressionList::ErrorMessage &errmsg, bool global)
433437
{
438+
std::lock_guard<std::mutex> lg(mSuppressionsSync);
439+
434440
const bool unmatchedSuppression(errmsg.errorId == "unmatchedSuppression");
435441
bool returnValue = false;
436442
for (Suppression &s : mSuppressions) {
@@ -446,6 +452,8 @@ bool SuppressionList::isSuppressed(const SuppressionList::ErrorMessage &errmsg,
446452

447453
bool SuppressionList::isSuppressedExplicitly(const SuppressionList::ErrorMessage &errmsg, bool global)
448454
{
455+
std::lock_guard<std::mutex> lg(mSuppressionsSync);
456+
449457
for (Suppression &s : mSuppressions) {
450458
if (!global && !s.isLocal())
451459
continue;
@@ -459,13 +467,19 @@ bool SuppressionList::isSuppressedExplicitly(const SuppressionList::ErrorMessage
459467

460468
bool SuppressionList::isSuppressed(const ::ErrorMessage &errmsg, const std::set<std::string>& macroNames)
461469
{
462-
if (mSuppressions.empty())
463-
return false;
470+
{
471+
std::lock_guard<std::mutex> lg(mSuppressionsSync);
472+
473+
if (mSuppressions.empty())
474+
return false;
475+
}
464476
return isSuppressed(SuppressionList::ErrorMessage::fromErrorMessage(errmsg, macroNames));
465477
}
466478

467479
void SuppressionList::dump(std::ostream & out) const
468480
{
481+
std::lock_guard<std::mutex> lg(mSuppressionsSync);
482+
469483
out << " <suppressions>" << std::endl;
470484
for (const Suppression &suppression : mSuppressions) {
471485
out << " <suppression";
@@ -499,6 +513,8 @@ void SuppressionList::dump(std::ostream & out) const
499513

500514
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file, const bool unusedFunctionChecking) const
501515
{
516+
std::lock_guard<std::mutex> lg(mSuppressionsSync);
517+
502518
std::list<Suppression> result;
503519
for (const Suppression &s : mSuppressions) {
504520
if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked))
@@ -520,6 +536,8 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppre
520536

521537
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const
522538
{
539+
std::lock_guard<std::mutex> lg(mSuppressionsSync);
540+
523541
std::list<Suppression> result;
524542
for (const Suppression &s : mSuppressions) {
525543
if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked))
@@ -537,12 +555,16 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppr
537555
return result;
538556
}
539557

540-
const std::list<SuppressionList::Suppression> &SuppressionList::getSuppressions() const
558+
std::list<SuppressionList::Suppression> SuppressionList::getSuppressions() const
541559
{
560+
std::lock_guard<std::mutex> lg(mSuppressionsSync);
561+
542562
return mSuppressions;
543563
}
544564

545565
void SuppressionList::markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &tokenizer) {
566+
std::lock_guard<std::mutex> lg(mSuppressionsSync);
567+
546568
int currLineNr = -1;
547569
int currFileIdx = -1;
548570
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {

‎lib/suppressions.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,21 @@
2626
#include <cstdint>
2727
#include <istream>
2828
#include <list>
29+
#include <mutex>
2930
#include <set>
3031
#include <string>
3132
#include <utility>
3233
#include <vector>
3334

34-
/// @addtogroup Core
35-
/// @{
36-
3735
class Tokenizer;
3836
class ErrorMessage;
3937
class ErrorLogger;
4038
enum class Certainty : std::uint8_t;
4139
class FileWithDetails;
4240

41+
/// @addtogroup Core
42+
/// @{
43+
4344
/** @brief class for handling suppressions */
4445
class CPPCHECKLIB SuppressionList {
4546
public:
@@ -250,7 +251,7 @@ class CPPCHECKLIB SuppressionList {
250251
* @brief Returns list of all suppressions.
251252
* @return list of suppressions
252253
*/
253-
const std::list<Suppression> &getSuppressions() const;
254+
std::list<Suppression> getSuppressions() const;
254255

255256
/**
256257
* @brief Marks Inline Suppressions as checked if source line is in the token stream
@@ -265,6 +266,7 @@ class CPPCHECKLIB SuppressionList {
265266
static bool reportUnmatchedSuppressions(const std::list<SuppressionList::Suppression> &unmatched, ErrorLogger &errorLogger);
266267

267268
private:
269+
mutable std::mutex mSuppressionsSync;
268270
/** @brief List of error which the user doesn't want to see. */
269271
std::list<Suppression> mSuppressions;
270272
};

‎test/testcmdlineparser.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,7 +1133,8 @@ class TestCmdlineParser : public TestFixture {
11331133
const char * const argv[] = {"cppcheck", "--exitcode-suppressions=suppr.txt", "file.cpp"};
11341134
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
11351135
ASSERT_EQUALS(2, supprs->nofail.getSuppressions().size());
1136-
auto it = supprs->nofail.getSuppressions().cbegin();
1136+
const auto supprsNofail = supprs->nofail.getSuppressions();
1137+
auto it = supprsNofail.cbegin();
11371138
ASSERT_EQUALS("uninitvar", (it++)->errorId);
11381139
ASSERT_EQUALS("unusedFunction", it->errorId);
11391140
}
@@ -1657,7 +1658,8 @@ class TestCmdlineParser : public TestFixture {
16571658
const char * const argv[] = {"cppcheck", "--suppressions-list=suppr.txt", "file.cpp"};
16581659
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
16591660
ASSERT_EQUALS(2, supprs->nomsg.getSuppressions().size());
1660-
auto it = supprs->nomsg.getSuppressions().cbegin();
1661+
const auto supprsNofail = supprs->nomsg.getSuppressions();
1662+
auto it = supprsNofail.cbegin();
16611663
ASSERT_EQUALS("uninitvar", (it++)->errorId);
16621664
ASSERT_EQUALS("unusedFunction", it->errorId);
16631665
}

0 commit comments

Comments
 (0)