From ddd21a260f5567d1c94a805b7430ae4869aeae9e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Tue, 30 Jun 2020 18:26:24 +0200
Subject: [PATCH] Fixed #6978 (False positive: unusedLabel shown for labels
 that are used in some preprocessor configurations)

---
 lib/checkother.cpp | 37 ++++++++++++++++++++++++-------------
 lib/checkother.h   |  8 +++++---
 lib/tokenize.cpp   | 17 +++++++++++++++++
 lib/tokenize.h     |  2 ++
 4 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index c48e6b735..c3a492fa9 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -2637,6 +2637,7 @@ void CheckOther::checkUnusedLabel()
 
     const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
     for (const Scope * scope : symbolDatabase->functionScopes) {
+        const bool hasIfdef = mTokenizer->hasIfdef(scope->bodyStart, scope->bodyEnd);
         for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
             if (!tok->scope()->isExecutable())
                 tok = tok->scope()->bodyEnd;
@@ -2644,25 +2645,35 @@ void CheckOther::checkUnusedLabel()
             if (Token::Match(tok, "{|}|; %name% :") && tok->strAt(1) != "default") {
                 const std::string tmp("goto " + tok->strAt(1));
                 if (!Token::findsimplematch(scope->bodyStart->next(), tmp.c_str(), tmp.size(), scope->bodyEnd->previous()))
-                    unusedLabelError(tok->next(), tok->next()->scope()->type == Scope::eSwitch);
+                    unusedLabelError(tok->next(), tok->next()->scope()->type == Scope::eSwitch, hasIfdef);
             }
         }
     }
 }
 
-void CheckOther::unusedLabelError(const Token* tok, bool inSwitch)
+void CheckOther::unusedLabelError(const Token* tok, bool inSwitch, bool hasIfdef)
 {
-    if (inSwitch) {
-        if (!tok || mSettings->isEnabled(Settings::WARNING))
-            reportError(tok, Severity::warning, "unusedLabelSwitch",
-                        "$symbol:" + (tok ? tok->str() : emptyString) + "\n"
-                        "Label '$symbol' is not used. Should this be a 'case' of the enclosing switch()?", CWE398, false);
-    } else {
-        if (!tok || mSettings->isEnabled(Settings::STYLE))
-            reportError(tok, Severity::style, "unusedLabel",
-                        "$symbol:" + (tok ? tok->str() : emptyString) + "\n"
-                        "Label '$symbol' is not used.", CWE398, false);
-    }
+    if (tok && !mSettings->isEnabled(inSwitch ? Settings::WARNING : Settings::STYLE))
+        return;
+
+    std::string id = "unusedLabel";
+    if (inSwitch)
+        id += "Switch";
+    if (hasIfdef)
+        id += "Configuration";
+
+    std::string msg = "$symbol:" + (tok ? tok->str() : emptyString) + "\nLabel '$symbol' is not used.";
+    if (hasIfdef)
+        msg += " There is #if in function body so the label might be used in code that is removed by the preprocessor.";
+    if (inSwitch)
+        msg += " Should this be a 'case' of the enclosing switch()?";
+
+    reportError(tok,
+                inSwitch ? Severity::warning : Severity::style,
+                id,
+                msg,
+                CWE398,
+                false);
 }
 
 
diff --git a/lib/checkother.h b/lib/checkother.h
index 0d1e1a323..4dc418986 100644
--- a/lib/checkother.h
+++ b/lib/checkother.h
@@ -266,7 +266,7 @@ private:
     void commaSeparatedReturnError(const Token *tok);
     void redundantPointerOpError(const Token* tok, const std::string& varname, bool inconclusive);
     void raceAfterInterlockedDecrementError(const Token* tok);
-    void unusedLabelError(const Token* tok, bool inSwitch);
+    void unusedLabelError(const Token* tok, bool inSwitch, bool hasIfdef);
     void unknownEvaluationOrder(const Token* tok);
     static bool isMovedParameterAllowedForInconclusiveFunction(const Token * tok);
     void accessMovedError(const Token *tok, const std::string &varname, const ValueFlow::Value *value, bool inconclusive);
@@ -331,8 +331,10 @@ private:
         c.nanInArithmeticExpressionError(nullptr);
         c.commaSeparatedReturnError(nullptr);
         c.redundantPointerOpError(nullptr,  "varname", false);
-        c.unusedLabelError(nullptr,  true);
-        c.unusedLabelError(nullptr,  false);
+        c.unusedLabelError(nullptr, false, false);
+        c.unusedLabelError(nullptr, false, true);
+        c.unusedLabelError(nullptr, true, false);
+        c.unusedLabelError(nullptr, true, true);
         c.unknownEvaluationOrder(nullptr);
         c.accessMovedError(nullptr, "v", nullptr, false);
         c.funcArgNamesDifferent("function", 1, nullptr, nullptr);
diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp
index a92301a78..a9fa35bea 100644
--- a/lib/tokenize.cpp
+++ b/lib/tokenize.cpp
@@ -24,6 +24,7 @@
 #include "library.h"
 #include "mathlib.h"
 #include "platform.h"
+#include "preprocessor.h"
 #include "settings.h"
 #include "standards.h"
 #include "symboldatabase.h"
@@ -11773,3 +11774,19 @@ bool Tokenizer::VariableMap::hasVariable(const std::string &varname) const
 {
     return mVariableId.find(varname) != mVariableId.end();
 }
+
+bool Tokenizer::hasIfdef(const Token *start, const Token *end) const
+{
+    if (!mPreprocessor)
+        return false;
+    for (const Directive &d: mPreprocessor->getDirectives()) {
+        if (d.str.compare(0,3,"#if") == 0 &&
+            d.linenr >= start->linenr() &&
+            d.linenr <= end->linenr() &&
+            start->fileIndex() < list.getFiles().size() &&
+            d.file == list.getFiles()[start->fileIndex()])
+            return true;
+    }
+    return false;
+}
+
diff --git a/lib/tokenize.h b/lib/tokenize.h
index 628628ae6..543a6cb74 100644
--- a/lib/tokenize.h
+++ b/lib/tokenize.h
@@ -568,6 +568,8 @@ public:
         return mPreprocessor;
     }
 
+    bool hasIfdef(const Token *start, const Token *end) const;
+
 private:
 
     /**