Detect non-interlocked check after InterlockedDecrement()

lib/checkother.cpp

@@ -2770,3 +2770,30 @@ void CheckOther::checkLibraryMatchFunctions()
         }
     }
 }
+
+void CheckOther::checkInterlockedDecrement()
+{
+    if (!_settings->isWindowsPlatform()) {
+        return;
+    }
+
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        if (Token::Match(tok, "InterlockedDecrement ( & %name% ) ; if ( %name%|!|0")) {
+            const Token* interlockedVarTok = tok->tokAt(3);
+            const Token* checkStartTok =  interlockedVarTok->tokAt(5);
+            if ((Token::Match(checkStartTok, "0 %comp% %name% )") && checkStartTok->strAt(2) == interlockedVarTok->str()) ||
+                (Token::Match(checkStartTok, "! %name% )") && checkStartTok->strAt(1) == interlockedVarTok->str()) ||
+                (Token::Match(checkStartTok, "%name% )") && checkStartTok->str() == interlockedVarTok->str()) ||
+                (Token::Match(checkStartTok, "%name% %comp% 0 )") && checkStartTok->str() == interlockedVarTok->str())) {
+                raceAfterInterlockedDecrementError(checkStartTok);
+            }
+        }
+    }
+}
+
+void CheckOther::raceAfterInterlockedDecrementError(const Token* tok)
+{
+    reportError(tok, Severity::error, "raceAfterInterlockedDecrement",
+        "Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.");
+}
+

lib/checkother.h

@@ -83,6 +83,7 @@ public:
 
         // --check-library : functions with nonmatching configuration
         checkOther.checkLibraryMatchFunctions();
+        checkOther.checkInterlockedDecrement();
     }
 
     /** @brief Run checks against the simplified token list */
@@ -238,6 +239,10 @@ public:
     /** @brief --check-library: warn for unconfigured function calls */
     void checkLibraryMatchFunctions();
 
+    void checkInterlockedDecrement();
+
+	void checkNewAfterDelete();
+
 private:
     // Error messages..
     void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &strFunctionName, const std::string &varName, const bool result);
@@ -293,6 +298,7 @@ private:
     void commaSeparatedReturnError(const Token *tok);
     void ignoredReturnValueError(const Token* tok, const std::string& function);
     void redundantPointerOpError(const Token* tok, const std::string& varname, bool inconclusive);
+    void raceAfterInterlockedDecrementError(const Token* tok);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckOther c(0, settings, errorLogger);
@@ -306,6 +312,7 @@ private:
         c.invalidPointerCastError(0, "float", "double", false);
         c.negativeBitwiseShiftError(0);
         c.checkPipeParameterSizeError(0, "varname", "dimension");
+        c.raceAfterInterlockedDecrementError(0);
 
         //performance
         c.redundantCopyError(0, "varname");
@@ -368,6 +375,7 @@ private:
                "- provide wrong dimensioned array to pipe() system command (--std=posix)\n"
                "- cast the return values of getc(),fgetc() and getchar() to character and compare it to EOF\n"
                "- invalid input values for functions\n"
+               "- race condition with non-interlocked access after InterlockedDecrement() call\n"
 
                // warning
                "- either division by zero or useless condition\n"

test/testother.cpp

@@ -166,6 +166,7 @@ private:
 
         TEST_CASE(redundantPointerOp);
         TEST_CASE(test_isSameExpression);
+        TEST_CASE(raceAfterInterlockedDecrement);
     }
 
     void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, Settings* settings = 0) {
@@ -211,6 +212,13 @@ private:
               &settings);
     }
 
+    void checkInterlockedDecrement(const char code[]) {
+        Settings settings;
+        settings.platformType = Settings::Win32A;
+
+        check(code, nullptr, false, false, true, &settings);
+    }
+
     void check_preprocess_suppress(const char precode[], const char *filename = nullptr) {
         // Clear the error buffer..
         errout.str("");
@@ -6228,6 +6236,206 @@ private:
               "};", "test.cpp", false, false);
         TODO_ASSERT_EQUALS("", "[test.cpp:1] -> [test.cpp:1]: (style) Same expression on both sides of '||'.\n", errout.str());
     }
+
+    void raceAfterInterlockedDecrement() {
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    InterlockedDecrement(&counter);\n"
+              "    whatever();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    InterlockedDecrement(&counter);\n"
+              "    if (counter)\n"
+              "        return;\n"
+              "    destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    InterlockedDecrement(&counter);\n"
+              "    if (!counter)\n"
+              "        destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    InterlockedDecrement(&counter);\n"
+              "    if (counter > 0)\n"
+              "        return;\n"
+              "    destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    InterlockedDecrement(&counter);\n"
+              "    if (0 < counter)\n"
+              "        return;\n"
+              "    destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    InterlockedDecrement(&counter);\n"
+              "    if (counter == 0)\n"
+              "        destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    InterlockedDecrement(&counter);\n"
+              "    if (0 == counter)\n"
+              "        destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    InterlockedDecrement(&counter);\n"
+              "    if (0 != counter)\n"
+              "        return;\n"
+              "    destroy()\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    InterlockedDecrement(&counter);\n"
+              "    if (counter != 0)\n"
+              "        return;\n"
+              "    destroy()\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    InterlockedDecrement(&counter);\n"
+              "    if (counter <= 0)\n"
+              "        destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    InterlockedDecrement(&counter);\n"
+              "    if (0 >= counter)\n"
+              "        destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    int newCount = InterlockedDecrement(&counter);\n"
+              "    if (newCount)\n"
+              "        return;\n"
+              "    destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    int newCount = InterlockedDecrement(&counter);\n"
+              "    if (!newCount)\n"
+              "        destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    int newCount = InterlockedDecrement(&counter);\n"
+              "    if (newCount > 0)\n"
+              "        return;\n"
+              "    destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    int newCount = InterlockedDecrement(&counter);\n"
+              "    if (0 < newCount)\n"
+              "        return;\n"
+              "    destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    int newCount = InterlockedDecrement(&counter);\n"
+              "    if (newCount == 0)\n"
+              "        destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    int newCount = InterlockedDecrement(&counter);\n"
+              "    if (0 == newCount)\n"
+              "        destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    int newCount = InterlockedDecrement(&counter);\n"
+              "    if (0 != newCount)\n"
+              "        return;\n"
+              "    destroy()\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    int newCount = InterlockedDecrement(&counter);\n"
+              "    if (newCount != 0)\n"
+              "        return;\n"
+              "    destroy()\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    int newCount = InterlockedDecrement(&counter);\n"
+              "    if (newCount <= 0)\n"
+              "        destroy();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkInterlockedDecrement(
+              "void f() {\n"
+              "    int counter = 0;\n"
+              "    int newCount = InterlockedDecrement(&counter);\n"
+              "    if (0 >= newCount)\n"
+              "        destroy;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
 };
 
 REGISTER_TEST(TestOther)