Skip to content

refs #12232/#7772 - added script to detect missing --errorlist entries and test coverage / added some missing --errorlist entries #7439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ $(libcppdir)/clangimport.o: lib/clangimport.cpp lib/addoninfo.h lib/checkers.h l
$(libcppdir)/color.o: lib/color.cpp lib/color.h lib/config.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/color.cpp

$(libcppdir)/cppcheck.o: lib/cppcheck.cpp externals/picojson/picojson.h externals/simplecpp/simplecpp.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkers.h lib/checkunusedfunctions.h lib/clangimport.h lib/color.h lib/config.h lib/cppcheck.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/version.h lib/vfvalue.h lib/xml.h
$(libcppdir)/cppcheck.o: lib/cppcheck.cpp externals/picojson/picojson.h externals/simplecpp/simplecpp.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkers.h lib/checkunusedfunctions.h lib/clangimport.h lib/color.h lib/config.h lib/cppcheck.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/version.h lib/vfvalue.h lib/xml.h
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/cppcheck.cpp

$(libcppdir)/ctu.o: lib/ctu.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/check.h lib/checkers.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h lib/xml.h
Expand Down
3 changes: 3 additions & 0 deletions lib/checkbufferoverrun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1235,4 +1235,7 @@ void CheckBufferOverrun::getErrorMessages(ErrorLogger *errorLogger, const Settin
c.argumentSizeError(nullptr, "function", 1, "buffer", nullptr, nullptr);
c.negativeMemoryAllocationSizeError(nullptr, nullptr);
c.negativeArraySizeError(nullptr);
c.terminateStrncpyError(nullptr, "var_name");
// TODO: ctuArrayIndex
// TODO: ctuPointerArith
}
1 change: 1 addition & 0 deletions lib/checkclass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3819,4 +3819,5 @@ void CheckClass::getErrorMessages(ErrorLogger *errorLogger, const Settings *sett
c.virtualFunctionCallInConstructorError(nullptr, std::list<const Token *>(), "f");
c.thisUseAfterFree(nullptr, nullptr, nullptr);
c.unsafeClassRefMemberError(nullptr, "UnsafeClass::var");
// TODO: ctuOneDefinitionRuleViolation
}
2 changes: 2 additions & 0 deletions lib/checkfunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,7 @@ void CheckFunctions::getErrorMessages(ErrorLogger *errorLogger, const Settings *
c.invalidFunctionArgBoolError(nullptr, "func_name", 1);
c.invalidFunctionArgStrError(nullptr, "func_name", 1);
c.ignoredReturnValueError(nullptr, "malloc");
c.ignoredReturnErrorCode(nullptr, "func_name");
c.mathfunctionCallWarning(nullptr);
c.mathfunctionCallWarning(nullptr, "1 - erf(x)", "erfc(x)");
c.memsetZeroBytesError(nullptr);
Expand All @@ -873,4 +874,5 @@ void CheckFunctions::getErrorMessages(ErrorLogger *errorLogger, const Settings *
c.missingReturnError(nullptr);
c.copyElisionError(nullptr);
c.useStandardLibraryError(nullptr, "memcpy");
// TODO: allocaCalled, <funcname>Called
}
4 changes: 4 additions & 0 deletions lib/checknullpointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -719,5 +719,9 @@ void CheckNullPointer::getErrorMessages(ErrorLogger *errorLogger, const Settings
CheckNullPointer c(nullptr, settings, errorLogger);
c.nullPointerError(nullptr, "pointer", nullptr, false);
c.pointerArithmeticError(nullptr, nullptr, false);
// TODO: nullPointerArithmeticOutOfMemory
c.redundantConditionWarning(nullptr, nullptr, nullptr, false);
// TODO: ctunullpointer
// TODO: ctunullpointerOutOfMemory
// TODO: ctunullpointerOutOfResources
}
4 changes: 4 additions & 0 deletions lib/checkother.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4580,6 +4580,8 @@ void CheckOther::getErrorMessages(ErrorLogger *errorLogger, const Settings *sett
c.intToPointerCastError(nullptr, "decimal");
c.suspiciousFloatingPointCastError(nullptr);
c.passedByValueError(nullptr, false);
// TODO: iterateByValue
// TODO: passedByValueCallback
c.constVariableError(nullptr, nullptr);
c.constStatementError(nullptr, "type", false);
c.signedCharArrayIndexError(nullptr);
Expand Down Expand Up @@ -4623,8 +4625,10 @@ void CheckOther::getErrorMessages(ErrorLogger *errorLogger, const Settings *sett
c.knownArgumentError(nullptr, nullptr, nullptr, "x", false);
c.knownPointerToBoolError(nullptr, nullptr);
c.comparePointersError(nullptr, nullptr, nullptr);
// TODO: subtractPointers
c.redundantAssignmentError(nullptr, nullptr, "var", false);
c.redundantInitializationError(nullptr, nullptr, "var", false);
c.redundantContinueError(nullptr);

const std::vector<const Token *> nullvec;
c.funcArgOrderDifferent("function", nullptr, nullptr, nullvec, nullvec);
Expand Down
11 changes: 10 additions & 1 deletion lib/checkstl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2345,7 +2345,8 @@ void CheckStl::uselessCallsSubstrError(const Token *tok, SubstrErrorType type)

void CheckStl::uselessCallsConstructorError(const Token *tok)
{
const std::string msg = "Inefficient constructor call: container '" + tok->str() + "' is assigned a partial copy of itself. Use erase() or resize() instead.";
const std::string container = tok ? tok->str() : "";
const std::string msg = "Inefficient constructor call: container '" + container + "' is assigned a partial copy of itself. Use erase() or resize() instead.";
reportError(tok, Severity::performance, "uselessCallsConstructor", msg, CWE398, Certainty::normal);
}

Expand Down Expand Up @@ -3451,6 +3452,7 @@ void CheckStl::getErrorMessages(ErrorLogger* errorLogger, const Settings* settin
c.iteratorsError(nullptr, nullptr, "container");
c.invalidContainerLoopError(nullptr, nullptr, ErrorPath{});
c.invalidContainerError(nullptr, nullptr, nullptr, ErrorPath{});
c.invalidContainerReferenceError(nullptr, nullptr, ErrorPath{});
c.mismatchingContainerIteratorError(nullptr, nullptr, nullptr);
c.mismatchingContainersError(nullptr, nullptr);
c.mismatchingContainerExpressionError(nullptr, nullptr);
Expand All @@ -3465,19 +3467,26 @@ void CheckStl::getErrorMessages(ErrorLogger* errorLogger, const Settings* settin
c.string_c_strError(nullptr);
c.string_c_strReturn(nullptr);
c.string_c_strParam(nullptr, 0);
c.string_c_strConstructor(nullptr);
c.string_c_strAssignment(nullptr);
c.string_c_strConcat(nullptr);
c.string_c_strStream(nullptr);
c.string_c_strThrowError(nullptr);
c.sizeError(nullptr);
c.missingComparisonError(nullptr, nullptr);
c.redundantIfRemoveError(nullptr);
c.uselessCallsReturnValueError(nullptr, "str", "find");
c.uselessCallsSwapError(nullptr, "str");
c.uselessCallsSubstrError(nullptr, SubstrErrorType::COPY);
c.uselessCallsConstructorError(nullptr);
c.uselessCallsEmptyError(nullptr);
c.uselessCallsRemoveError(nullptr, "remove");
c.dereferenceInvalidIteratorError(nullptr, "i");
// TODO: derefInvalidIteratorRedundantCheck
c.eraseIteratorOutOfBoundsError(nullptr, nullptr);
c.useStlAlgorithmError(nullptr, "");
c.knownEmptyContainerError(nullptr, "");
c.globalLockGuardError(nullptr);
c.localMutexError(nullptr);
c.outOfBoundsIndexExpressionError(nullptr, nullptr);
}
2 changes: 2 additions & 0 deletions lib/checktype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,9 @@ void CheckType::getErrorMessages(ErrorLogger *errorLogger, const Settings *setti
c.tooBigBitwiseShiftError(nullptr, 32, ValueFlow::Value(64));
c.tooBigSignedBitwiseShiftError(nullptr, 31, ValueFlow::Value(31));
c.integerOverflowError(nullptr, ValueFlow::Value(1LL<<32));
// TODO: integerOverflowCond
c.signConversionError(nullptr, nullptr, false);
// TODO: signConversionCond
c.longCastAssignError(nullptr);
c.longCastReturnError(nullptr);
ValueFlow::Value f;
Expand Down
2 changes: 1 addition & 1 deletion lib/checkuninitvar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1813,7 +1813,7 @@ void CheckUninitVar::getErrorMessages(ErrorLogger* errorLogger, const Settings*

ValueFlow::Value v{};

c.uninitvarError(nullptr, v);
c.uninitvarError(nullptr, v); // TODO: does not produce any output
c.uninitdataError(nullptr, "varname");
c.uninitStructMemberError(nullptr, "a.b");
}
9 changes: 3 additions & 6 deletions lib/checkunusedfunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -341,12 +341,9 @@ static bool isOperatorFunction(const std::string & funcName)
return std::find(additionalOperators.cbegin(), additionalOperators.cend(), funcName.substr(operatorPrefix.length())) != additionalOperators.cend();
}

static void staticFunctionError(ErrorLogger& errorLogger,
const std::string &filename,
nonneg int fileIndex,
nonneg int lineNumber,
nonneg int column,
const std::string &funcname)
void CheckUnusedFunctions::staticFunctionError(ErrorLogger& errorLogger,
const std::string &filename, nonneg int fileIndex, nonneg int lineNumber, nonneg int column,
const std::string &funcname)
{
std::list<ErrorMessage::FileLocation> locationList;
if (!filename.empty()) {
Expand Down
5 changes: 5 additions & 0 deletions lib/checkunusedfunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class CPPCHECKLIB CheckUnusedFunctions {

static void getErrorMessages(ErrorLogger &errorLogger) {
unusedFunctionError(errorLogger, "", 0, 0, 0, "funcName");
staticFunctionError(errorLogger, "", 0, 0, 0, "funcName");
}

// Return true if an error is reported.
Expand All @@ -70,6 +71,10 @@ class CPPCHECKLIB CheckUnusedFunctions {
const std::string &filename, nonneg int fileIndex, nonneg int lineNumber, nonneg int column,
const std::string &funcname);

static void staticFunctionError(ErrorLogger& errorLogger,
const std::string &filename, nonneg int fileIndex, nonneg int lineNumber, nonneg int column,
const std::string &funcname);

struct CPPCHECKLIB FunctionUsage {
std::string filename;
nonneg int lineNumber{};
Expand Down
2 changes: 2 additions & 0 deletions lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "settings.h"
#include "standards.h"
#include "suppressions.h"
#include "symboldatabase.h"
#include "timer.h"
#include "token.h"
#include "tokenize.h"
Expand Down Expand Up @@ -1929,6 +1930,7 @@ void CppCheck::getErrorMessages(ErrorLogger &errorlogger)
CheckUnusedFunctions::getErrorMessages(errorlogger);
Preprocessor::getErrorMessages(errorlogger, s);
Tokenizer::getErrorMessages(errorlogger, s);
SymbolDatabase::getErrorMessages(errorlogger);
}

void CppCheck::analyseClangTidy(const FileSettings &fileSettings)
Expand Down
7 changes: 7 additions & 0 deletions lib/symboldatabase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8390,3 +8390,10 @@ ValueType::MatchResult ValueType::matchParameter(const ValueType *call, const Va
}
return res;
}

void SymbolDatabase::getErrorMessages(ErrorLogger& /*errorLogger*/)
{
// TODO
//SymbolDatabase symdb;
//symdb.returnImplicitIntError(nullptr);
}
2 changes: 2 additions & 0 deletions lib/symboldatabase.h
Original file line number Diff line number Diff line change
Expand Up @@ -1418,6 +1418,8 @@ class CPPCHECKLIB SymbolDatabase {
/* returns the opening { if tok points to enum */
static const Token* isEnumDefinition(const Token* tok);

static void getErrorMessages(ErrorLogger &errorLogger);

private:
friend class Scope;
friend class Function;
Expand Down
2 changes: 1 addition & 1 deletion oss-fuzz/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ $(libcppdir)/clangimport.o: ../lib/clangimport.cpp ../lib/addoninfo.h ../lib/che
$(libcppdir)/color.o: ../lib/color.cpp ../lib/color.h ../lib/config.h
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/color.cpp

$(libcppdir)/cppcheck.o: ../lib/cppcheck.cpp ../externals/picojson/picojson.h ../externals/simplecpp/simplecpp.h ../externals/tinyxml2/tinyxml2.h ../lib/addoninfo.h ../lib/analyzerinfo.h ../lib/check.h ../lib/checkers.h ../lib/checkunusedfunctions.h ../lib/clangimport.h ../lib/color.h ../lib/config.h ../lib/cppcheck.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/filesettings.h ../lib/json.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/preprocessor.h ../lib/settings.h ../lib/standards.h ../lib/suppressions.h ../lib/templatesimplifier.h ../lib/timer.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/version.h ../lib/vfvalue.h ../lib/xml.h
$(libcppdir)/cppcheck.o: ../lib/cppcheck.cpp ../externals/picojson/picojson.h ../externals/simplecpp/simplecpp.h ../externals/tinyxml2/tinyxml2.h ../lib/addoninfo.h ../lib/analyzerinfo.h ../lib/check.h ../lib/checkers.h ../lib/checkunusedfunctions.h ../lib/clangimport.h ../lib/color.h ../lib/config.h ../lib/cppcheck.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/filesettings.h ../lib/json.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/preprocessor.h ../lib/settings.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/timer.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/version.h ../lib/vfvalue.h ../lib/xml.h
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/cppcheck.cpp

$(libcppdir)/ctu.o: ../lib/ctu.cpp ../externals/tinyxml2/tinyxml2.h ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkers.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h ../lib/xml.h
Expand Down
31 changes: 31 additions & 0 deletions tools/errorid.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/bin/bash
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the filename is pretty generic so it's not easy to understand exactly what it does. And there is no comment that explains it neither. can it be clarified..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Will adjust the name and document it in the related markdown file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some documentation in the readme.md of the tools folder.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think the filename is too generic but I don't feel I have a very good suggestion..

how about:

  • compare-errorlist-testsuite
  • get-missing-ids-errorlist-testsuite

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried coming up with something but couldn't. The suggestions are equally missing the point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will give it another thought tomorrow as I am also not satisfied with the name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • compare-errorids
  • check-errorids
  • check-errorid-coverage

Definitely something containing "errorid".

Copy link
Owner

@danmar danmar Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like "check-errorids" . but no strong opinion about those other. they are better than the existing filename.

imho I would add testsuite-errorlist into your names.

  • compare-errorids-testsuite-errorlist
  • check-errorid-coverage-testsuite-errorlist

the names are a bit longish but imho this is fine.


#set -x

SCRIPT_DIR="$(dirname "$(realpath "$0")")"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imho it would be easier to maintain a python script than a bash script.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I started out with these shell commands and I didn't feel like try to replicate it in Python for now. It is also sufficient until we want to integrate it into the CI.


# TODO: exclude testinternal.cpp
echo 'no --errorlist entry:'
grep -h -o -P '\[[a-zA-Z0-9_]+\]\\n\"' $SCRIPT_DIR/../test/*.cpp | tr -d '[]\"' | sed 's/\\n$//' | sort -u | \
while read -r id; do
if [ ${#id} -lt 4 ]; then
continue
fi
$SCRIPT_DIR/../cppcheck --errorlist | grep "id=\"$id\"" > /dev/null
# shellcheck disable=SC2181
if [ $? -ne 0 ]; then
echo $id
fi
done

echo ''

echo 'no test coverage:'
$SCRIPT_DIR/../cppcheck --errorlist | grep -h -o -P 'id=\"[a-zA-Z0-9_]*\"' | sed 's/\id=//' | tr -d '\"' | sort -u | \
while read -r id; do
grep -h -o -P "\[$id\]\\\\n\"" $SCRIPT_DIR/../test/*.cpp > /dev/null
# shellcheck disable=SC2181
if [ $? -ne 0 ]; then
echo $id
fi
done
5 changes: 5 additions & 0 deletions tools/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,8 @@ Script to compare result of working Cppcheck from your branch with main branch.
This tool lets you comfortably look at Cppcheck analysis results for daca packages. It automatically
downloads the package, extracts it and jumps to the corresponding source code for a Cppcheck
message.

### * tools/errorid.sh

Script to compare the error IDs in the expected `testrunner` output (without executing it) with the `--errorlist` output.
It will report missing test coverage for an ID and missing IDs in the `--errorlist` output.
Loading