added unusedFunction self check to CI / cleanups (#3526)

.github/workflows/selfcheck.yml

@@ -0,0 +1,75 @@
+# Syntax reference https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions
+# Environment reference https://help.github.com/en/actions/reference/virtual-environments-for-github-hosted-runners
+name: selfcheck
+
+on: [push, pull_request]
+
+jobs:
+  build:
+
+    runs-on: ubuntu-20.04
+
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Install missing software
+        run: |
+          sudo apt-get update
+          sudo apt-get install z3 libz3-dev
+
+      - name: Install Qt
+        uses: jurplel/install-qt-action@v2
+        with:
+          version: '5.15.2'
+          modules: 'qtcharts'
+
+      # TODO: cache this - perform same build as for the other self check
+      - name: Self check (build)
+        run: |
+          make clean
+          make -j$(nproc) -s CXXFLAGS="-O2 -w" MATCHCOMPILER=yes
+
+      - name: CMake
+        run: |
+          mkdir cmake.output
+          pushd cmake.output
+          cmake -G "Unix Makefiles" -DUSE_Z3=On -DHAVE_RULES=On -DBUILD_TESTS=On -DBUILD_GUI=ON -DWITH_QCHART=ON -DCMAKE_GLOBAL_AUTOGEN_TARGET=On ..
+
+      - name: Generate dependencies
+        run: |
+          # make sure the precompiled headers exist
+          make -C cmake.output lib/CMakeFiles/lib_objs.dir/cmake_pch.hxx.cxx
+          make -C cmake.output test/CMakeFiles/testrunner.dir/cmake_pch.hxx.cxx
+          # make sure auto-generated GUI files exist
+          make -C cmake.output autogen
+          make -C cmake.output gui-build-deps
+
+      # TODO: find a way to report unmatched suppressions without need to add information checks
+      - name: Self check (unusedFunction)
+        if: false # TODO: fails with preprocessorErrorDirective - see #10667
+        run: |
+          ./cppcheck -q --template=selfcheck --error-exitcode=1 --library=cppcheck-lib --library=qt -D__GNUC__ -DQT_VERSION=0x050000 -DQ_MOC_OUTPUT_REVISION=67 --inconclusive --enable=unusedFunction --exception-handling -rp=. --project=cmake.output/compile_commands.json --suppressions-list=.selfcheck_unused_suppressions --inline-suppr
+        env:
+          DISABLE_VALUEFLOW: 1
+
+      # the following steps are duplicated from above since setting up the buld node in a parallel step takes longer than the actual steps
+      - name: CMake (no test)
+        run: |
+          mkdir cmake.output.notest
+          pushd cmake.output.notest
+          cmake -G "Unix Makefiles" -DUSE_Z3=On -DHAVE_RULES=On -DBUILD_TESTS=0 -DBUILD_GUI=ON -DWITH_QCHART=ON -DCMAKE_GLOBAL_AUTOGEN_TARGET=On ..
+
+      - name: Generate dependencies (no test)
+        run: |
+          # make sure the precompiled headers exist
+          make -C cmake.output.notest lib/CMakeFiles/lib_objs.dir/cmake_pch.hxx.cxx
+          # make sure auto-generated GUI files exist
+          make -C cmake.output.notest autogen
+          make -C cmake.output.notest gui-build-deps
+
+      # TODO: find a way to report unmatched suppressions without need to add information checks
+      - name: Self check (unusedFunction / no test)
+        run: |
+          ./cppcheck -q --template=selfcheck --error-exitcode=1 --library=cppcheck-lib --library=qt -D__GNUC__ -DQT_VERSION=0x050000 -DQ_MOC_OUTPUT_REVISION=67 --inconclusive --enable=unusedFunction --exception-handling -rp=. --project=cmake.output.notest/compile_commands.json --suppressions-list=.selfcheck_unused_suppressions --inline-suppr
+        env:
+          DISABLE_VALUEFLOW: 1

.selfcheck_unused_suppressions

@@ -0,0 +1,13 @@
+# we are not using all methods of their interfaces
+unusedFunction:externals/tinyxml2/tinyxml2.cpp
+unusedFunction:externals/simplecpp/simplecpp.cpp
+
+# TODO: fix these
+# false positive - # 10660
+unusedFunction:gui/mainwindow.cpp
+unusedFunction:gui/resultstree.cpp
+unusedFunction:gui/codeeditor.cpp
+# usage is disabled
+unusedFunction:lib/symboldatabase.cpp
+# false positive - #10661
+unusedFunction:oss-fuzz/main.cpp

cli/threadexecutor.cpp

@@ -55,7 +55,6 @@ using std::memset;
 
 ThreadExecutor::ThreadExecutor(const std::map<std::string, std::size_t> &files, Settings &settings, ErrorLogger &errorLogger)
     : mFiles(files), mSettings(settings), mErrorLogger(errorLogger), mFileCount(0)
-    // Not initialized mFileSync, mErrorSync, mReportSync
 {
 #if defined(THREADING_MODEL_FORK)
     mWpipe = 0;
@@ -68,10 +67,23 @@ ThreadExecutor::ThreadExecutor(const std::map<std::string, std::size_t> &files,
 }
 
 ThreadExecutor::~ThreadExecutor()
+{}
+
+// cppcheck-suppress unusedFunction - only used in unit tests
+void ThreadExecutor::addFileContent(const std::string &path, const std::string &content)
 {
-    //dtor
+    mFileContents[path] = content;
 }
 
+void ThreadExecutor::reportErr(const ErrorMessage &msg)
+{
+    report(msg, MessageType::REPORT_ERROR);
+}
+
+void ThreadExecutor::reportInfo(const ErrorMessage &msg)
+{
+    report(msg, MessageType::REPORT_INFO);
+}
 
 ///////////////////////////////////////////////////////////////////////////////
 ////// This code is for platforms that support fork() only ////////////////////
@@ -79,11 +91,6 @@ ThreadExecutor::~ThreadExecutor()
 
 #if defined(THREADING_MODEL_FORK)
 
-void ThreadExecutor::addFileContent(const std::string &path, const std::string &content)
-{
-    mFileContents[path] = content;
-}
-
 int ThreadExecutor::handleRead(int rpipe, unsigned int &result)
 {
     char type = 0;
@@ -351,21 +358,26 @@ void ThreadExecutor::reportOut(const std::string &outmsg, Color c)
     writeToPipe(REPORT_OUT, ::toString(c) + outmsg + ::toString(Color::Reset));
 }
 
-void ThreadExecutor::reportErr(const ErrorMessage &msg)
-{
-    writeToPipe(REPORT_ERROR, msg.serialize());
-}
-
-void ThreadExecutor::reportInfo(const ErrorMessage &msg)
-{
-    writeToPipe(REPORT_INFO, msg.serialize());
-}
-
 void ThreadExecutor::bughuntingReport(const std::string &str)
 {
     writeToPipe(REPORT_VERIFICATION, str);
 }
 
+void ThreadExecutor::report(const ErrorMessage &msg, MessageType msgType)
+{
+    PipeSignal pipeSignal;
+    switch (msgType) {
+    case MessageType::REPORT_ERROR:
+        pipeSignal = REPORT_ERROR;
+        break;
+    case MessageType::REPORT_INFO:
+        pipeSignal = REPORT_INFO;
+        break;
+    }
+
+    writeToPipe(pipeSignal, msg.serialize());
+}
+
 void ThreadExecutor::reportInternalChildErr(const std::string &childname, const std::string &msg)
 {
     std::list<ErrorMessage::FileLocation> locations;
@@ -383,11 +395,6 @@ void ThreadExecutor::reportInternalChildErr(const std::string &childname, const
 
 #elif defined(THREADING_MODEL_WIN)
 
-void ThreadExecutor::addFileContent(const std::string &path, const std::string &content)
-{
-    mFileContents[path] = content;
-}
-
 unsigned int ThreadExecutor::check()
 {
     std::vector<std::future<unsigned int>> threadFutures;
@@ -481,15 +488,6 @@ void ThreadExecutor::reportOut(const std::string &outmsg, Color c)
 
     mErrorLogger.reportOut(outmsg, c);
 }
-void ThreadExecutor::reportErr(const ErrorMessage &msg)
-{
-    report(msg, MessageType::REPORT_ERROR);
-}
-
-void ThreadExecutor::reportInfo(const ErrorMessage &msg)
-{
-    report(msg, MessageType::REPORT_INFO);
-}
 
 void ThreadExecutor::bughuntingReport(const std::string  & /*str*/)
 {
@@ -513,7 +511,6 @@ void ThreadExecutor::report(const ErrorMessage &msg, MessageType msgType)
         }
     }
 
-
     if (reportError) {
         std::lock_guard<std::mutex> lg(mReportSync);
 
@@ -527,26 +524,4 @@ void ThreadExecutor::report(const ErrorMessage &msg, MessageType msgType)
         }
     }
 }
-
-#else
-
-void ThreadExecutor::addFileContent(const std::string & /*path*/, const std::string & /*content*/)
-{}
-
-unsigned int ThreadExecutor::check()
-{
-    return 0;
-}
-
-void ThreadExecutor::reportOut(const std::string & /*outmsg*/, Color)
-{}
-void ThreadExecutor::reportErr(const ErrorMessage & /*msg*/)
-{}
-
-void ThreadExecutor::reportInfo(const ErrorMessage & /*msg*/)
-{}
-
-void ThreadExecutor::bughuntingReport(const std::string & /*str*/)
-{}
-
 #endif

cli/threadexecutor.h

@@ -34,6 +34,8 @@
 #define THREADING_MODEL_WIN
 #include "importproject.h"
 #include <mutex>
+#else
+#error "No threading moodel defined"
 #endif
 
 class Settings;
@@ -68,11 +70,15 @@ public:
     void addFileContent(const std::string &path, const std::string &content);
 
 private:
+    enum class MessageType {REPORT_ERROR, REPORT_INFO};
+
     const std::map<std::string, std::size_t> &mFiles;
     Settings &mSettings;
     ErrorLogger &mErrorLogger;
     unsigned int mFileCount;
 
+    void report(const ErrorMessage &msg, MessageType msgType);
+
 #if defined(THREADING_MODEL_FORK)
 
     /** @brief Key is file name, and value is the content of the file */
@@ -119,8 +125,6 @@ public:
 #elif defined(THREADING_MODEL_WIN)
 
 private:
-    enum class MessageType {REPORT_ERROR, REPORT_INFO};
-
     std::map<std::string, std::string> mFileContents;
     std::map<std::string, std::size_t>::const_iterator mItNextFile;
     std::list<ImportProject::FileSettings>::const_iterator mItNextFileSettings;
@@ -135,8 +139,6 @@ private:
 
     std::mutex mReportSync;
 
-    void report(const ErrorMessage &msg, MessageType msgType);
-
     static unsigned __stdcall threadProc(ThreadExecutor *threadExecutor);
 
 public:

gui/checkthread.cpp

@@ -83,6 +83,7 @@ void CheckThread::analyseWholeProgram(const QStringList &files)
     start();
 }
 
+// cppcheck-suppress unusedFunction - TODO: false positive
 void CheckThread::run()
 {
     mState = Running;

gui/codeeditstylecontrols.cpp

@@ -57,6 +57,7 @@ void SelectColorButton::setColor(const QColor& color)
     updateColor();
 }
 
+// cppcheck-suppress unusedFunction
 const QColor& SelectColorButton::getColor()
 {
     return mColor;
@@ -114,6 +115,7 @@ void SelectFontWeightCombo::setWeight(const QFont::Weight& weight)
     updateWeight();
 }
 
+// cppcheck-suppress unusedFunction
 const QFont::Weight& SelectFontWeightCombo::getWeight()
 {
     return mWeight;

gui/test/translationhandler/testtranslationhandler.cpp

@@ -20,10 +20,19 @@
 #include "testtranslationhandler.h"
 #include "translationhandler.h"
 
+static const QStringList getTranslationNames(const TranslationHandler& handler)
+{
+    QStringList names;
+    foreach (TranslationInfo translation, handler.getTranslations()) {
+        names.append(translation.mName);
+    }
+    return names;
+}
+
 void TestTranslationHandler::construct()
 {
     TranslationHandler handler;
-    QCOMPARE(handler.getNames().size(), 13);  // 12 translations + english
+    QCOMPARE(getTranslationNames(handler).size(), 13);  // 12 translations + english
     QCOMPARE(handler.getCurrentLanguage(), QString("en"));
 }
 

gui/translationhandler.cpp

@@ -61,15 +61,6 @@ TranslationHandler::TranslationHandler(QObject *parent) :
 TranslationHandler::~TranslationHandler()
 {}
 
-const QStringList TranslationHandler::getNames() const
-{
-    QStringList names;
-    foreach (TranslationInfo translation, mTranslations) {
-        names.append(translation.mName);
-    }
-    return names;
-}
-
 bool TranslationHandler::setLanguage(const QString &code)
 {
     bool failure = false;

gui/translationhandler.h

@@ -65,13 +65,6 @@ public:
     explicit TranslationHandler(QObject *parent = nullptr);
     virtual ~TranslationHandler();
 
-    /**
-     * @brief Get a list of available translation names.
-     * @return List of available translation names.
-     *
-     */
-    const QStringList getNames() const;
-
     /**
      * @brief Get a list of available translations.
      * @return List of available translations.

lib/checkunusedfunctions.cpp

@@ -436,7 +436,10 @@ void CheckUnusedFunctions::analyseWholeProgram(ErrorLogger * const errorLogger,
     for (std::map<std::string, Location>::const_iterator decl = decls.begin(); decl != decls.end(); ++decl) {
         const std::string &functionName = decl->first;
 
-        if (functionName == "main" || functionName == "WinMain" || functionName == "_tmain" ||
+        // TODO: move to configuration files
+        // TODO: WinMain, wmain and _tmain only apply to Windows code
+        // TODO: also skip other known entry functions i.e. annotated with "constructor" and "destructor" attributes
+        if (functionName == "main" || functionName == "WinMain" || functionName == "wmain" || functionName == "_tmain" ||
             functionName == "if")
             continue;
 

lib/forwardanalyzer.cpp

@@ -89,6 +89,7 @@ struct ForwardTraversal {
         return evalCond(tok, ctx).first;
     }
 
+    // cppcheck-suppress unusedFunction
     bool isConditionFalse(const Token* tok, const Token* ctx = nullptr) const {
         return evalCond(tok, ctx).second;
     }

lib/library.cpp

@@ -184,6 +184,7 @@ Library::Container::Action Library::Container::actionFrom(const std::string& act
         return Container::Action::NO_ACTION;
 }
 
+// cppcheck-suppress unusedFunction - only used in unit tests
 bool Library::loadxmldata(const char xmldata[], std::size_t len)
 {
     tinyxml2::XMLDocument doc;
@@ -1577,6 +1578,8 @@ const Token* Library::getContainerFromYield(const Token* tok, Library::Container
     }
     return nullptr;
 }
+
+// cppcheck-suppress unusedFunction
 const Token* Library::getContainerFromAction(const Token* tok, Library::Container::Action action) const
 {
     if (!tok)

lib/mathlib.cpp

@@ -698,6 +698,7 @@ static bool isValidIntegerSuffixIt(std::string::const_iterator it, std::string::
             (state == Status::SUFFIX_UI64));
 }
 
+// cppcheck-suppress unusedFunction
 bool MathLib::isValidIntegerSuffix(const std::string& str, bool supportMicrosoftExtensions)
 {
     return isValidIntegerSuffixIt(str.begin(), str.end(), supportMicrosoftExtensions);
@@ -1180,16 +1181,19 @@ bool MathLib::isNotEqual(const std::string &first, const std::string &second)
     return !isEqual(first, second);
 }
 
+// cppcheck-suppress unusedFunction
 bool MathLib::isGreater(const std::string &first, const std::string &second)
 {
     return toDoubleNumber(first) > toDoubleNumber(second);
 }
 
+// cppcheck-suppress unusedFunction
 bool MathLib::isGreaterEqual(const std::string &first, const std::string &second)
 {
     return toDoubleNumber(first) >= toDoubleNumber(second);
 }
 
+// cppcheck-suppress unusedFunction
 bool MathLib::isLess(const std::string &first, const std::string &second)
 {
     return toDoubleNumber(first) < toDoubleNumber(second);

lib/programmemory.cpp

@@ -28,6 +28,7 @@ const ValueFlow::Value* ProgramMemory::getValue(nonneg int exprid, bool impossib
         return nullptr;
 }
 
+// cppcheck-suppress unusedFunction
 bool ProgramMemory::getIntValue(nonneg int exprid, MathLib::bigint* result) const
 {
     const ValueFlow::Value* value = getValue(exprid);
@@ -56,6 +57,7 @@ bool ProgramMemory::getTokValue(nonneg int exprid, const Token** result) const
     return false;
 }
 
+// cppcheck-suppress unusedFunction
 bool ProgramMemory::getContainerSizeValue(nonneg int exprid, MathLib::bigint* result) const
 {
     const ValueFlow::Value* value = getValue(exprid);

lib/token.cpp

@@ -1185,6 +1185,7 @@ void Token::printOut(const char *title, const std::vector<std::string> &fileName
     std::cout << stringifyList(stringifyOptions::forPrintOut(), &fileNames, nullptr) << std::endl;
 }
 
+// cppcheck-suppress unusedFunction - used for debugging
 void Token::printLines(int lines) const
 {
     const Token *end = this;
@@ -2446,6 +2447,7 @@ const ValueFlow::Value* Token::getMovedValue() const
     return it == mImpl->mValues->end() ? nullptr : &*it;
 }
 
+// cppcheck-suppress unusedFunction
 const ValueFlow::Value* Token::getContainerSizeValue(const MathLib::bigint val) const
 {
     if (!mImpl->mValues)

lib/tokenize.cpp

@@ -2807,11 +2807,17 @@ bool Tokenizer::simplifyTokens1(const std::string &configuration)
     if (!mSettings->buildDir.empty())
         Summaries::create(this, configuration);
 
-    if (mTimerResults) {
-        Timer t("Tokenizer::simplifyTokens1::ValueFlow", mSettings->showtime, mTimerResults);
-        ValueFlow::setValues(&list, mSymbolDatabase, mErrorLogger, mSettings);
-    } else {
-        ValueFlow::setValues(&list, mSymbolDatabase, mErrorLogger, mSettings);
+    // TODO: do not run valueflow if no checks are being performed at all - e.g. unusedFunctions only
+    const char* disableValueflowEnv = std::getenv("DISABLE_VALUEFLOW");
+    const bool doValueFlow = !disableValueflowEnv || (std::strcmp(disableValueflowEnv, "1") != 0);
+
+    if (doValueFlow) {
+        if (mTimerResults) {
+            Timer t("Tokenizer::simplifyTokens1::ValueFlow", mSettings->showtime, mTimerResults);
+            ValueFlow::setValues(&list, mSymbolDatabase, mErrorLogger, mSettings);
+        } else {
+            ValueFlow::setValues(&list, mSymbolDatabase, mErrorLogger, mSettings);
+        }
     }
 
     // Warn about unhandled character literals
@@ -2827,7 +2833,9 @@ bool Tokenizer::simplifyTokens1(const std::string &configuration)
         }
     }
 
-    mSymbolDatabase->setArrayDimensionsUsingValueFlow();
+    if (doValueFlow) {
+        mSymbolDatabase->setArrayDimensionsUsingValueFlow();
+    }
 
     printDebugOutput(1);
 

test/testlibrary.cpp

@@ -665,8 +665,8 @@ private:
                                 "</def>";
 
         Library library;
-        library.loadxmldata(xmldata1, sizeof(xmldata1));
-        library.loadxmldata(xmldata2, sizeof(xmldata2));
+        ASSERT_EQUALS(true, library.loadxmldata(xmldata1, sizeof(xmldata1)));
+        ASSERT_EQUALS(true, library.loadxmldata(xmldata2, sizeof(xmldata2)));
 
         ASSERT_EQUALS(library.deallocId("free"), library.allocId("malloc"));
         ASSERT_EQUALS(library.deallocId("free"), library.allocId("foo"));

test/testuninitvar.cpp

@@ -814,7 +814,7 @@ private:
                                "<def format=\"1\">"
                                "  <podtype name=\"_tm\"/>"
                                "</def>";
-        settings.library.loadxmldata(xmldata, sizeof(xmldata));
+        ASSERT_EQUALS(true, settings.library.loadxmldata(xmldata, sizeof(xmldata)));
         checkUninitVar("void f() {\n"
                        "  Fred _tm;\n"
                        "  _tm.dostuff();\n"

test/testvalueflow.cpp

@@ -50,7 +50,7 @@ private:
                            "  <function name=\"strcpy\"> <arg nr=\"1\"><not-null/></arg> </function>\n"
                            "  <function name=\"abort\"> <noreturn>true</noreturn> </function>\n" // abort is a noreturn function
                            "</def>";
-        settings.library.loadxmldata(cfg, sizeof(cfg));
+        ASSERT_EQUALS(true, settings.library.loadxmldata(cfg, sizeof(cfg)));
         LOAD_LIB_2(settings.library, "std.cfg");
 
         TEST_CASE(valueFlowNumber);