From fc8c24467583b638c0e8e946ba2441040b1b5fcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 6 Nov 2023 15:31:47 +0100 Subject: [PATCH] CI: Add selfcheck using Cppcheck Premium. Activates Misra C++ 2008 and Cert C++ 2016 checkers. (#5623) --- .github/workflows/cppcheck-premium.yml | 43 ++++++ cppcheckpremium-suppressions | 186 +++++++++++++++++++++++++ lib/analyzer.h | 2 + lib/astutils.cpp | 18 +-- lib/checkcondition.cpp | 2 +- lib/checkers.h | 5 +- lib/checkersreport.h | 5 +- lib/symboldatabase.cpp | 14 +- lib/tokenize.cpp | 2 - lib/valueflow.cpp | 16 +-- lib/version.h | 5 + 11 files changed, 268 insertions(+), 30 deletions(-) create mode 100644 .github/workflows/cppcheck-premium.yml create mode 100644 cppcheckpremium-suppressions diff --git a/.github/workflows/cppcheck-premium.yml b/.github/workflows/cppcheck-premium.yml new file mode 100644 index 000000000..11d6f3b8c --- /dev/null +++ b/.github/workflows/cppcheck-premium.yml @@ -0,0 +1,43 @@ + +name: cppcheck-premium + +on: + push: + branches: + - 'main' + - 'releases/**' + tags: + - '2.*' + pull_request: + +permissions: + contents: read + +jobs: + + build: + runs-on: ubuntu-22.04 # run on the latest image only + + env: + PREMIUM_VERSION: devdrop-20231105 + + steps: + - uses: actions/checkout@v3 + + - name: Download cppcheckpremium + run: | + wget https://files.cppchecksolutions.com/cppcheckpremium-${{ env.PREMIUM_VERSION }}-amd64.tar.gz + tar xzf cppcheckpremium-${{ env.PREMIUM_VERSION }}-amd64.tar.gz + + - name: Generate a license file + run: | + echo cppcheck > cppcheck.lic + echo 231231 >> cppcheck.lic + echo 80000 >> cppcheck.lic + echo 57e08c39523ab54d >> cppcheck.lic + echo path:lib >> cppcheck.lic + + - name: Check + run: | + cppcheckpremium-${{ env.PREMIUM_VERSION }}/premiumaddon --check-loc-license cppcheck.lic > cppcheck-premium-loc + cppcheckpremium-${{ env.PREMIUM_VERSION }}/cppcheck -j$(nproc) -D__GNUC__ -D__CPPCHECK__ --suppressions-list=cppcheckpremium-suppressions --platform=unix64 --enable=style --premium=misra-c++-2008 --premium=cert-c++-2016 --error-exitcode=1 lib diff --git a/cppcheckpremium-suppressions b/cppcheckpremium-suppressions new file mode 100644 index 000000000..8ca95c7ec --- /dev/null +++ b/cppcheckpremium-suppressions @@ -0,0 +1,186 @@ + +# False positives +premium-misra-cpp-2008-5-17-1 +premium-misra-cpp-2008-5-0-6 +premium-misra-cpp-2008-7-2-1 +premium-misra-cpp-2008-3-3-2 + +# open source warnings are handled by the selfcheck.yml +noExplicitConstructor +postfixOperator +shadowFunction +useStlAlgorithm + +# we need to declare reserved identifier _CRTDBG_MAP_ALLOC +premium-cert-dcl51-cpp + +# TODO: Is there unsafe allocations, in case of exceptions) in cppcheck +premium-cert-err58-cpp + +# we have global objects +premium-cert-err58-cpp + +# TODO: Exception objects must be nothrow copy constructible. +premium-cert-err60-cpp + +# TODO should we throw Token? +premium-cert-err61-cpp + +# TODO: Detect errors when converting a string to a number. The library function 'atoi()' shall not be used. +premium-cert-err62-cpp + +# TODO: Can we reduce some const_cast? +premium-cert-exp55-cpp + +# sometimes a void function does not have side effects +premium-misra-cpp-2008-0-1-8 + +# unused arguments, misra rules are too strict +premium-misra-cpp-2008-0-1-11 +premium-misra-cpp-2008-0-1-12 + +# we sometimes don't care about return value from functions +premium-misra-cpp-2008-0-1-7 + +# TODO: can we prevent commented out code? +premium-misra-cpp-2008-2-7-2 +premium-misra-cpp-2008-2-7-3 + +# NA +premium-misra-cpp-2008-2-10-1 + +# objects of a class often has the lowercase name of the class. +premium-misra-cpp-2008-2-10-4 + +# flag |= .. +premium-misra-cpp-2008-4-5-1 + +# Token/Variable flags are enum constants and we use those in bitwise operations by intention. +premium-misra-cpp-2008-4-5-2 + +# intentional addition of char to string: const std::string end(':' + cfg + ':' + Path::simplifyPath(sourcefile)); +premium-misra-cpp-2008-4-5-3 + +# too strict operator precedence warnings +premium-misra-cpp-2008-5-0-2 + +# we are less strict about signedness. what bug is there here: unsigned int col = 0 +premium-misra-cpp-2008-5-0-4 + +# intentional integral-to-float conversion +premium-misra-cpp-2008-5-0-5 + +# intentional addition of char literal: c = 'a' + (temp - 10); +premium-misra-cpp-2008-5-0-11 + +# conversion of char-to-int is intentional sometimes +premium-misra-cpp-2008-5-0-12 + +# pointer-to-bool conversion is common +premium-misra-cpp-2008-5-0-14 + +# pointer arithmetic is not uncommon in cppcheck code +premium-misra-cpp-2008-5-0-15 + +# it's only a problem if signed expression is negative +premium-misra-cpp-2008-5-0-21 + +# Intentional safe operands of &&: return !stdValue.empty() && str == getCPP(); +premium-misra-cpp-2008-5-2-1 + +# const_cast performs intentional const casting +premium-misra-cpp-2008-5-2-5 + +# safe code: const char *next = static_cast(std::memchr(pattern, ' ', pattern_len)); +premium-misra-cpp-2008-5-2-8 + +# we intentionally cast pointer to integer when creating id for dumpfile +premium-misra-cpp-2008-5-2-9 + +# we intentionally mix increment with other operators in expressions +premium-misra-cpp-2008-5-2-10 + +# intentional array-to-pointer decay +premium-misra-cpp-2008-5-2-12 + +# we write !pointer by intention +premium-misra-cpp-2008-5-3-1 + +# for (;;) +premium-misra-cpp-2008-6-2-3 + +# it's not a bug to not put default at the end of a switch body +premium-misra-cpp-2008-6-4-6 + +# looping linked list => not well formed for loop +premium-misra-cpp-2008-6-5-1 +premium-misra-cpp-2008-6-5-2 +premium-misra-cpp-2008-6-5-3 +premium-misra-cpp-2008-6-5-4 +premium-misra-cpp-2008-6-5-5 +premium-misra-cpp-2008-6-5-6 + +# we like early returns +premium-misra-cpp-2008-6-6-3 +premium-misra-cpp-2008-6-6-4 +premium-misra-cpp-2008-6-6-5 + +# we have local functions by intention +premium-misra-cpp-2008-7-3-1 + +# intentional: return reference from method to non-const reference parameter +premium-misra-cpp-2008-7-5-3 + +# intentional declaration of multiple variables +premium-misra-cpp-2008-8-0-1 + +# we intentionally don't use & before function names +premium-misra-cpp-2008-8-4-4 + +# cppcheck does not care about this enumerator rule +premium-misra-cpp-2008-8-5-3 + +# TODO Fix these +premium-misra-cpp-2008-9-3-1 + +# we use unions by intention sometimes +premium-misra-cpp-2008-9-5-1 + +# overridden methods is safe +premium-misra-cpp-2008-10-3-1 + +# some classes have public members by intention +premium-misra-cpp-2008-11-0-1 + +# rule should not apply to deleted copy assignment operator +premium-misra-cpp-2008-12-8-2 + +# TODO: this can be fixed by refactoring the code. +premium-misra-cpp-2008-14-6-2 + +# function specializations: TODO check if we should refactor +premium-misra-cpp-2008-14-8-2 + +# we use preprocessor when it makes sense +premium-misra-cpp-2008-16-0-1 +premium-misra-cpp-2008-16-2-1 +premium-misra-cpp-2008-16-2-2 +premium-misra-cpp-2008-16-3-2 + +# TODO do we need to catch string conversion errors (using atoi)? +premium-misra-cpp-2008-18-0-2 + +# what standard alternative is there for std::getenv +premium-misra-cpp-2008-18-0-3 + +# is used by intention +premium-misra-cpp-2008-18-0-4 + +# code is safe. we use std::strcmp by intention +premium-misra-cpp-2008-18-0-5 + +# we do avoid using new/delete +premium-misra-cpp-2008-18-4-1 + +# is used by intention +premium-misra-cpp-2008-27-0-1 diff --git a/lib/analyzer.h b/lib/analyzer.h index 3ceab711b..475c5f28c 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -33,6 +33,8 @@ struct Analyzer { struct Action { Action() = default; + Action(const Action&) = default; + Action& operator=(const Action& rhs) = default; template ), diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 64c5a74a3..2214b5365 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2241,7 +2241,7 @@ bool isScopeBracket(const Token* tok) } template )> -T* getTokenArgumentFunctionImpl(T* tok, int& argn) +static T* getTokenArgumentFunctionImpl(T* tok, int& argn) { argn = -1; { @@ -2892,13 +2892,13 @@ const Token* findThisChanged(const Token* start, const Token* end, int indirect, } template -const Token* findExpressionChangedImpl(const Token* expr, - const Token* start, - const Token* end, - const Settings* settings, - bool cpp, - int depth, - Find find) +static const Token* findExpressionChangedImpl(const Token* expr, + const Token* start, + const Token* end, + const Settings* settings, + bool cpp, + int depth, + Find find) { if (depth < 0) return start; @@ -3095,7 +3095,7 @@ const Token *findLambdaStartToken(const Token *last) } template -T* findLambdaEndTokenGeneric(T* first) +static T* findLambdaEndTokenGeneric(T* first) { auto maybeLambda = [](T* tok) -> bool { while (Token::Match(tok, "*|%name%|::|>")) { diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index a9813297a..8e5e3f134 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -995,7 +995,7 @@ static bool checkFloatRelation(const std::string &op, const double value1, const } template -T getvalue3(const T value1, const T value2) +static T getvalue3(const T value1, const T value2) { const T min = std::min(value1, value2); if (min== std::numeric_limits::max()) diff --git a/lib/checkers.h b/lib/checkers.h index 218ce262f..4297665b0 100644 --- a/lib/checkers.h +++ b/lib/checkers.h @@ -16,7 +16,8 @@ * along with this program. If not, see . */ -#pragma once +#ifndef checkersH +#define checkersH #include #include @@ -44,4 +45,4 @@ namespace checkers { extern CPPCHECKLIB const std::map misraRuleSeverity; } - +#endif diff --git a/lib/checkersreport.h b/lib/checkersreport.h index 5353f74b9..e0c8654e0 100644 --- a/lib/checkersreport.h +++ b/lib/checkersreport.h @@ -16,7 +16,8 @@ * along with this program. If not, see . */ -#pragma once +#ifndef checkersReportH +#define checkersReportH #include "config.h" @@ -44,4 +45,4 @@ private: int mAllCheckersCount = 0; }; - +#endif diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 6098b8dc7..9281b54a3 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -2178,6 +2178,10 @@ Variable& Variable::operator=(const Variable &var) if (this == &var) return *this; + ValueType* vt = nullptr; + if (var.mValueType) + vt = new ValueType(*var.mValueType); + mNameToken = var.mNameToken; mTypeStartToken = var.mTypeStartToken; mTypeEndToken = var.mTypeEndToken; @@ -2188,9 +2192,7 @@ Variable& Variable::operator=(const Variable &var) mScope = var.mScope; mDimensions = var.mDimensions; delete mValueType; - mValueType = nullptr; - if (var.mValueType) - mValueType = new ValueType(*var.mValueType); + mValueType = vt; return *this; } @@ -2344,9 +2346,9 @@ void Variable::setValueType(const ValueType &valueType) if (declType && !declType->next()->valueType()) return; } + ValueType* vt = new ValueType(valueType); delete mValueType; - mValueType = nullptr; - mValueType = new ValueType(valueType); + mValueType = vt; if ((mValueType->pointer > 0) && (!isArray() || Token::Match(mNameToken->previous(), "( * %name% )"))) setFlag(fIsPointer, true); setFlag(fIsConst, mValueType->constness & (1U << mValueType->pointer)); @@ -6359,7 +6361,7 @@ static void setAutoTokenProperties(Token * const autoTok) autoTok->isStandardType(true); } -bool isContainerYieldElement(Library::Container::Yield yield) +static bool isContainerYieldElement(Library::Container::Yield yield) { return yield == Library::Container::Yield::ITEM || yield == Library::Container::Yield::AT_INDEX || yield == Library::Container::Yield::BUFFER || yield == Library::Container::Yield::BUFFER_NT; diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 2d8a5eb9a..c3c33f9eb 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -618,7 +618,6 @@ namespace { std::pair mRangeType; std::pair mRangeTypeQualifiers; std::pair mRangeAfterVar; - std::string mTypedefName; // Name of typedef type Token* mNameToken{nullptr}; bool mFail = false; bool mReplaceFailed = false; @@ -643,7 +642,6 @@ namespace { if (Token::Match(nameToken, "%name% ;")) { mRangeType = rangeBefore; mRangeTypeQualifiers = rangeQualifiers; - mTypedefName = nameToken->str(); Token* typeName = rangeBefore.second->previous(); if (typeName->isKeyword()) { (void)num; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index b0f8baa0c..ecda35927 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2237,7 +2237,7 @@ struct SingleRange { }; template -SingleRange MakeSingleRange(T& x) +static SingleRange MakeSingleRange(T& x) { return {&x}; } @@ -6804,7 +6804,7 @@ ValuePtr ValueFlow::makeIntegralInferModel() { return IntegralInferModel{}; } -ValueFlow::Value inferCondition(const std::string& op, const Token* varTok, MathLib::bigint val) +static ValueFlow::Value inferCondition(const std::string& op, const Token* varTok, MathLib::bigint val) { if (!varTok) return ValueFlow::Value{}; @@ -7374,7 +7374,7 @@ struct MultiValueFlowAnalyzer : ValueFlowAnalyzer { }; template -bool productParams(const Settings* settings, const std::unordered_map>& vars, F f) +static bool productParams(const Settings* settings, const std::unordered_map>& vars, F f) { using Args = std::vector>; Args args(1); @@ -7607,7 +7607,7 @@ struct IteratorRange }; template -IteratorRange MakeIteratorRange(Iterator start, Iterator last) +static IteratorRange MakeIteratorRange(Iterator start, Iterator last) { return {start, last}; } @@ -8260,7 +8260,7 @@ static const Token* solveExprValue(const Token* expr, ValueFlow::Value& value) value); } -ValuePtr makeAnalyzer(const Token* exprTok, ValueFlow::Value value, const TokenList& tokenlist, const Settings* settings) +static ValuePtr makeAnalyzer(const Token* exprTok, ValueFlow::Value value, const TokenList& tokenlist, const Settings* settings) { if (value.isContainerSizeValue()) return ContainerExpressionAnalyzer(exprTok, std::move(value), tokenlist, settings); @@ -8268,7 +8268,7 @@ ValuePtr makeAnalyzer(const Token* exprTok, ValueFlow::Value value, co return ExpressionAnalyzer(expr, std::move(value), tokenlist, settings); } -ValuePtr makeReverseAnalyzer(const Token* exprTok, ValueFlow::Value value, const TokenList& tokenlist, const Settings* settings) +static ValuePtr makeReverseAnalyzer(const Token* exprTok, ValueFlow::Value value, const TokenList& tokenlist, const Settings* settings) { if (value.isContainerSizeValue()) return ContainerExpressionAnalyzer(exprTok, std::move(value), tokenlist, settings); @@ -9380,7 +9380,7 @@ struct ValueFlowPassAdaptor : ValueFlowPass { const char* mName = nullptr; bool mCPP = false; F mRun; - ValueFlowPassAdaptor(const char* pname, bool pcpp, F prun) : mName(pname), mCPP(pcpp), mRun(prun) {} + ValueFlowPassAdaptor(const char* pname, bool pcpp, F prun) : ValueFlowPass(), mName(pname), mCPP(pcpp), mRun(prun) {} const char* name() const override { return mName; } @@ -9394,7 +9394,7 @@ struct ValueFlowPassAdaptor : ValueFlowPass { }; template -ValueFlowPassAdaptor makeValueFlowPassAdaptor(const char* name, bool cpp, F run) +static ValueFlowPassAdaptor makeValueFlowPassAdaptor(const char* name, bool cpp, F run) { return {name, cpp, run}; } diff --git a/lib/version.h b/lib/version.h index 3aff0ffb2..85bb17033 100644 --- a/lib/version.h +++ b/lib/version.h @@ -1,6 +1,9 @@ // For a release version x.y.z the MAJOR should be x and both MINOR and DEVMINOR should be y. // After a release the DEVMINOR is incremented. MAJOR=x MINOR=y, DEVMINOR=y+1 +#ifndef versionH +#define versionH + #define CPPCHECK_MAJOR_VERSION 2 #define CPPCHECK_MINOR_VERSION 12 #define CPPCHECK_DEVMINOR_VERSION 13 @@ -16,3 +19,5 @@ #define CPPCHECK_VERSION CPPCHECK_MAJOR_VERSION,CPPCHECK_MINOR_VERSION,99,0 #endif #define LEGALCOPYRIGHT L"Copyright (C) 2007-2023 Cppcheck team." + +#endif