SuppressionList: fixed data race

Author: firewave

Diff for: lib/suppressions.cpp

@@ -254,6 +254,8 @@ std::string SuppressionList::addSuppressionLine(const std::string &line)
 
 std::string SuppressionList::addSuppression(SuppressionList::Suppression suppression)
 {
+    std::lock_guard<std::mutex> lg(mSuppressionsSync);
+
     // Check if suppression is already in list
     auto foundSuppression = std::find_if(mSuppressions.begin(), mSuppressions.end(),
                                          std::bind(&Suppression::isSameParameters, &suppression, std::placeholders::_1));
@@ -297,6 +299,8 @@ std::string SuppressionList::addSuppressions(std::list<Suppression> suppressions
 // cppcheck-suppress unusedFunction
 bool SuppressionList::updateSuppressionState(const SuppressionList::Suppression& suppression)
 {
+    std::lock_guard<std::mutex> lg(mSuppressionsSync);
+
     // Check if suppression is already in list
     auto foundSuppression = std::find_if(mSuppressions.begin(), mSuppressions.end(),
                                          std::bind(&Suppression::isSameParameters, &suppression, std::placeholders::_1));
@@ -431,6 +435,8 @@ std::string SuppressionList::Suppression::getText() const
 
 bool SuppressionList::isSuppressed(const SuppressionList::ErrorMessage &errmsg, bool global)
 {
+    std::lock_guard<std::mutex> lg(mSuppressionsSync);
+
     const bool unmatchedSuppression(errmsg.errorId == "unmatchedSuppression");
     bool returnValue = false;
     for (Suppression &s : mSuppressions) {
@@ -446,6 +452,8 @@ bool SuppressionList::isSuppressed(const SuppressionList::ErrorMessage &errmsg,
 
 bool SuppressionList::isSuppressedExplicitly(const SuppressionList::ErrorMessage &errmsg, bool global)
 {
+    std::lock_guard<std::mutex> lg(mSuppressionsSync);
+
     for (Suppression &s : mSuppressions) {
         if (!global && !s.isLocal())
             continue;
@@ -459,13 +467,19 @@ bool SuppressionList::isSuppressedExplicitly(const SuppressionList::ErrorMessage
 
 bool SuppressionList::isSuppressed(const ::ErrorMessage &errmsg, const std::set<std::string>& macroNames)
 {
-    if (mSuppressions.empty())
-        return false;
+    {
+        std::lock_guard<std::mutex> lg(mSuppressionsSync);
+
+        if (mSuppressions.empty())
+            return false;
+    }
     return isSuppressed(SuppressionList::ErrorMessage::fromErrorMessage(errmsg, macroNames));
 }
 
 void SuppressionList::dump(std::ostream & out) const
 {
+    std::lock_guard<std::mutex> lg(mSuppressionsSync);
+
     out << "  <suppressions>" << std::endl;
     for (const Suppression &suppression : mSuppressions) {
         out << "    <suppression";
@@ -499,6 +513,8 @@ void SuppressionList::dump(std::ostream & out) const
 
 std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file, const bool unusedFunctionChecking) const
 {
+    std::lock_guard<std::mutex> lg(mSuppressionsSync);
+
     std::list<Suppression> result;
     for (const Suppression &s : mSuppressions) {
         if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked))
@@ -520,6 +536,8 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppre
 
 std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const
 {
+    std::lock_guard<std::mutex> lg(mSuppressionsSync);
+
     std::list<Suppression> result;
     for (const Suppression &s : mSuppressions) {
         if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked))
@@ -537,12 +555,16 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppr
     return result;
 }
 
-const std::list<SuppressionList::Suppression> &SuppressionList::getSuppressions() const
+std::list<SuppressionList::Suppression> SuppressionList::getSuppressions() const
 {
+    std::lock_guard<std::mutex> lg(mSuppressionsSync);
+
     return mSuppressions;
 }
 
 void SuppressionList::markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &tokenizer) {
+    std::lock_guard<std::mutex> lg(mSuppressionsSync);
+
     int currLineNr = -1;
     int currFileIdx = -1;
     for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {

Diff for: lib/suppressions.h

@@ -26,20 +26,21 @@
 #include <cstdint>
 #include <istream>
 #include <list>
+#include <mutex>
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
-/// @addtogroup Core
-/// @{
-
 class Tokenizer;
 class ErrorMessage;
 class ErrorLogger;
 enum class Certainty : std::uint8_t;
 class FileWithDetails;
 
+/// @addtogroup Core
+/// @{
+
 /** @brief class for handling suppressions */
 class CPPCHECKLIB SuppressionList {
 public:
@@ -250,7 +251,7 @@ class CPPCHECKLIB SuppressionList {
      * @brief Returns list of all suppressions.
      * @return list of suppressions
      */
-    const std::list<Suppression> &getSuppressions() const;
+    std::list<Suppression> getSuppressions() const;
 
     /**
      * @brief Marks Inline Suppressions as checked if source line is in the token stream
@@ -265,6 +266,7 @@ class CPPCHECKLIB SuppressionList {
     static bool reportUnmatchedSuppressions(const std::list<SuppressionList::Suppression> &unmatched, ErrorLogger &errorLogger);
 
 private:
+    mutable std::mutex mSuppressionsSync;
     /** @brief List of error which the user doesn't want to see. */
     std::list<Suppression> mSuppressions;
 };

Diff for: test/testcmdlineparser.cpp

@@ -1133,7 +1133,8 @@ class TestCmdlineParser : public TestFixture {
         const char * const argv[] = {"cppcheck", "--exitcode-suppressions=suppr.txt", "file.cpp"};
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
         ASSERT_EQUALS(2, supprs->nofail.getSuppressions().size());
-        auto it = supprs->nofail.getSuppressions().cbegin();
+        const auto supprsNofail = supprs->nofail.getSuppressions();
+        auto it = supprsNofail.cbegin();
         ASSERT_EQUALS("uninitvar", (it++)->errorId);
         ASSERT_EQUALS("unusedFunction", it->errorId);
     }
@@ -1657,7 +1658,8 @@ class TestCmdlineParser : public TestFixture {
         const char * const argv[] = {"cppcheck", "--suppressions-list=suppr.txt", "file.cpp"};
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
         ASSERT_EQUALS(2, supprs->nomsg.getSuppressions().size());
-        auto it = supprs->nomsg.getSuppressions().cbegin();
+        const auto supprsNofail = supprs->nomsg.getSuppressions();
+        auto it = supprsNofail.cbegin();
         ASSERT_EQUALS("uninitvar", (it++)->errorId);
         ASSERT_EQUALS("unusedFunction", it->errorId);
     }