New check: Detect unused 'goto' labels (#5124)

lib/checkother.cpp

@@ -2618,3 +2618,31 @@ void CheckOther::raceAfterInterlockedDecrementError(const Token* tok)
     reportError(tok, Severity::error, "raceAfterInterlockedDecrement",
                 "Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.");
 }
+
+void CheckOther::checkUnusedLabel()
+{
+    if (!_settings->isEnabled("style"))
+        return;
+
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+    const std::size_t functions = symbolDatabase->functionScopes.size();
+    for (std::size_t i = 0; i < functions; ++i) {
+        const Scope * scope = symbolDatabase->functionScopes[i];
+
+        for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
+            if (!tok->scope()->isExecutable())
+                tok = tok->scope()->classEnd;
+
+            if (Token::Match(tok, "{|}|; %name% :") && tok->strAt(1) != "default") {
+                if (!Token::findsimplematch(scope->classStart->next(), ("goto " + tok->strAt(1)).c_str(), scope->classEnd->previous()))
+                    unusedLabelError(tok->next());
+            }
+        }
+    }
+}
+
+void CheckOther::unusedLabelError(const Token* tok)
+{
+    reportError(tok, Severity::style, "unusedLabel",
+                "Label '" + (tok?tok->str():emptyString) + "' is not used.");
+}

lib/checkother.h

@@ -69,10 +69,11 @@ public:
         checkOther.checkIgnoredReturnValue();
         checkOther.checkRedundantPointerOp();
         checkOther.checkZeroDivision();
+        checkOther.checkInterlockedDecrement();
+        checkOther.checkUnusedLabel();
 
         // --check-library : functions with nonmatching configuration
         checkOther.checkLibraryMatchFunctions();
-        checkOther.checkInterlockedDecrement();
     }
 
     /** @brief Run checks against the simplified token list */
@@ -222,8 +223,12 @@ public:
     /** @brief --check-library: warn for unconfigured function calls */
     void checkLibraryMatchFunctions();
 
+    /** @brief %Check for race condition with non-interlocked access after InterlockedDecrement() */
     void checkInterlockedDecrement();
 
+    /** @brief %Check for unused labels */
+    void checkUnusedLabel();
+
 private:
     // Error messages..
     void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &strFunctionName, const std::string &varName, const bool result);
@@ -276,6 +281,7 @@ private:
     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 unusedLabelError(const Token* tok);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckOther c(0, settings, errorLogger);
@@ -334,6 +340,7 @@ private:
         c.commaSeparatedReturnError(0);
         c.ignoredReturnValueError(0, "malloc");
         c.redundantPointerOpError(0, "varname", false);
+        c.unusedLabelError(0);
     }
 
     static std::string myName() {
@@ -391,7 +398,8 @@ private:
                "- comma in return statement (the comma can easily be misread as a semicolon).\n"
                "- prefer erfc, expm1 or log1p to avoid loss of precision.\n"
                "- identical code in both branches of if/else or ternary operator.\n"
-               "- redundant pointer operation on pointer like &*some_ptr.\n";
+               "- redundant pointer operation on pointer like &*some_ptr.\n"
+               "- find unused 'goto' labels.\n";
     }
 };
 /// @}

test/testother.cpp

@@ -168,6 +168,8 @@ private:
         TEST_CASE(redundantPointerOp);
         TEST_CASE(test_isSameExpression);
         TEST_CASE(raceAfterInterlockedDecrement);
+
+        TEST_CASE(testUnusedLabel);
     }
 
     void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, Settings* settings = 0) {
@@ -3091,7 +3093,7 @@ private:
               "  label:\n"
               "    throw 0;\n"
               "}", nullptr, false, false, false);
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (style) Label 'label' is not used.\n", errout.str());
 
         check("void foo() {\n"
               "    wxCHECK2(state < 3 && state >= 0, return);\n"
@@ -6447,6 +6449,51 @@ private:
             "}\n");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void testUnusedLabel() {
+        check("void f() {\n"
+              "    label:\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Label 'label' is not used.\n", errout.str());
+
+        check("void f() {\n"
+              "    label:\n"
+              "    foo();\n"
+              "    goto label;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    label:\n"
+              "    foo();\n"
+              "    goto label;\n"
+              "}\n"
+              "void g() {\n"
+              "    label:\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:7]: (style) Label 'label' is not used.\n", errout.str());
+
+        check("void f() {\n"
+              "    switch(a) {\n"
+              "        default:\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    class X {\n"
+              "        protected:\n"
+              "    };\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    class X {\n"
+              "        my_protected:\n"
+              "    };\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
 };
 
 REGISTER_TEST(TestOther)