diff --git a/.selfcheck_unused_suppressions b/.selfcheck_unused_suppressions index ce685d8ab10..66a3c3a57c1 100644 --- a/.selfcheck_unused_suppressions +++ b/.selfcheck_unused_suppressions @@ -6,3 +6,6 @@ unusedFunction:lib/symboldatabase.cpp # Q_OBJECT functions which are not called in our code unusedFunction:cmake.output.notest/gui/cppcheck-gui_autogen/*/moc_aboutdialog.cpp + +# CheckUnionZeroInit::generateTestMessage only used in tests. +unusedFunction:lib/checkunionzeroinit.cpp diff --git a/Makefile b/Makefile index 8dda00a2096..a22405b8da1 100644 --- a/Makefile +++ b/Makefile @@ -224,6 +224,7 @@ LIBOBJ = $(libcppdir)/valueflow.o \ $(libcppdir)/checkstring.o \ $(libcppdir)/checktype.o \ $(libcppdir)/checkuninitvar.o \ + $(libcppdir)/checkunionzeroinit.o \ $(libcppdir)/checkunusedfunctions.o \ $(libcppdir)/checkunusedvar.o \ $(libcppdir)/checkvaarg.o \ @@ -346,6 +347,7 @@ TESTOBJ = test/fixture.o \ test/testtokenrange.o \ test/testtype.o \ test/testuninitvar.o \ + test/testunionzeroinit.o \ test/testunusedfunctions.o \ test/testunusedprivfunc.o \ test/testunusedvar.o \ @@ -561,6 +563,9 @@ $(libcppdir)/checktype.o: lib/checktype.cpp lib/addoninfo.h lib/astutils.h lib/c $(libcppdir)/checkuninitvar.o: lib/checkuninitvar.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkers.h lib/checknullpointer.h lib/checkuninitvar.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.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 $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkuninitvar.cpp +$(libcppdir)/checkunionzeroinit.o: lib/checkunionzeroinit.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/checkunionzeroinit.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.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/valueflow.h lib/vfvalue.h + $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkunionzeroinit.cpp + $(libcppdir)/checkunusedfunctions.o: lib/checkunusedfunctions.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/checkers.h lib/checkunusedfunctions.h lib/config.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 $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkunusedfunctions.cpp @@ -909,6 +914,9 @@ test/testtype.o: test/testtype.cpp lib/addoninfo.h lib/check.h lib/checkers.h li test/testuninitvar.o: test/testuninitvar.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/checkuninitvar.h lib/color.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/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h $(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testuninitvar.cpp +test/testunionzeroinit.o: test/testunionzeroinit.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/checkunionzeroinit.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h + $(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testunionzeroinit.cpp + test/testunusedfunctions.o: test/testunusedfunctions.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h $(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testunusedfunctions.cpp diff --git a/lib/checkers.cpp b/lib/checkers.cpp index a5a47b867c2..24a68b23664 100644 --- a/lib/checkers.cpp +++ b/lib/checkers.cpp @@ -200,6 +200,7 @@ namespace checkers { {"CheckType::checkLongCast","style"}, {"CheckType::checkSignConversion","warning"}, {"CheckType::checkTooBigBitwiseShift","platform"}, + {"CheckUninitVar::check",""}, {"CheckUninitVar::analyseWholeProgram",""}, {"CheckUninitVar::check",""}, {"CheckUninitVar::valueFlowUninit",""}, diff --git a/lib/checkunionzeroinit.cpp b/lib/checkunionzeroinit.cpp new file mode 100644 index 00000000000..2c1863129d3 --- /dev/null +++ b/lib/checkunionzeroinit.cpp @@ -0,0 +1,209 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2025 Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "checkunionzeroinit.h" + +#include +#include +#include + +#include "errortypes.h" +#include "settings.h" +#include "symboldatabase.h" +#include "token.h" +#include "tokenize.h" +#include "valueflow.h" + +static const CWE CWEXXX(0U); /* TODO */ + +static const std::string noname; + +// Register this check class (by creating a static instance of it) +namespace { + CheckUnionZeroInit instance; +} + +struct UnionMember { + UnionMember() + : name(noname) + , size(0) {} + + UnionMember(const std::string &name_, size_t size_) + : name(name_) + , size(size_) {} + + const std::string &name; + size_t size; +}; + +struct Union { + Union(const Scope *scope_, const std::string &name_) + : scope(scope_) + , name(name_) {} + + const Scope *scope; + const std::string &name; + std::vector members; + + const UnionMember *largestMember() const { + const UnionMember *largest = nullptr; + for (const UnionMember &m : members) { + if (m.size == 0) + return nullptr; + if (largest == nullptr || m.size > largest->size) + largest = &m; + } + return largest; + } + + bool isLargestMemberFirst() const { + const UnionMember *largest = largestMember(); + return largest == nullptr || largest == &members[0]; + } +}; + +static UnionMember parseUnionMember(const Variable &var, + const Settings &settings) +{ + const Token *nameToken = var.nameToken(); + if (nameToken == nullptr) + return UnionMember(); + + const ValueType *vt = nameToken->valueType(); + size_t size = 0; + if (var.isArray()) { + size = var.dimension(0); + } else if (vt != nullptr) { + size = ValueFlow::getSizeOf(*vt, settings, + ValueFlow::Accuracy::ExactOrZero); + } + return UnionMember(nameToken->str(), size); +} + +static std::vector parseUnions(const SymbolDatabase &symbolDatabase, + const Settings &settings) +{ + std::vector unions; + + for (const Scope &scope : symbolDatabase.scopeList) { + if (scope.type != ScopeType::eUnion) + continue; + + Union u(&scope, scope.className); + for (const Variable &var : scope.varlist) { + u.members.push_back(parseUnionMember(var, settings)); + } + unions.push_back(u); + } + + return unions; +} + +static bool isZeroInitializer(const Token *tok) +{ + return Token::simpleMatch(tok, "= { 0 } ;") || + Token::simpleMatch(tok, "= { } ;"); +} + +void CheckUnionZeroInit::check() +{ + if (!mSettings->severity.isEnabled(Severity::portability)) + return; + + logChecker("CheckUnionZeroInit::check"); // portability + + const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); + + std::unordered_map unionsByScopeId; + const std::vector unions = parseUnions(*symbolDatabase, *mSettings); + for (const Union &u : unions) { + unionsByScopeId.insert({u.scope, u}); + } + + for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) { + if (!tok->isName() || !isZeroInitializer(tok->next())) + continue; + + const ValueType *vt = tok->valueType(); + if (vt == nullptr || vt->typeScope == nullptr) + continue; + auto it = unionsByScopeId.find(vt->typeScope); + if (it == unionsByScopeId.end()) + continue; + const Union &u = it->second; + if (!u.isLargestMemberFirst()) { + const UnionMember *largestMember = u.largestMember(); + assert(largestMember != nullptr); + unionZeroInitError(tok, *largestMember); + } + } +} + +void CheckUnionZeroInit::runChecks(const Tokenizer &tokenizer, + ErrorLogger *errorLogger) +{ + CheckUnionZeroInit(&tokenizer, &tokenizer.getSettings(), errorLogger).check(); +} + +void CheckUnionZeroInit::unionZeroInitError(const Token *tok, + const UnionMember& largestMember) +{ + reportError(tok, Severity::portability, "UnionZeroInit", + "$symbol:" + (tok != nullptr ? tok->str() : "") + "\n" + "Zero initializing union '$symbol' does not guarantee " + + "its complete storage to be zero initialized as its largest member " + + "is not declared as the first member. Consider making " + + largestMember.name + " the first member or favor memset().", + CWEXXX, Certainty::normal); +} + +void CheckUnionZeroInit::getErrorMessages(ErrorLogger *errorLogger, + const Settings *settings) const +{ + CheckUnionZeroInit c(nullptr, settings, errorLogger); + c.unionZeroInitError(nullptr, UnionMember()); +} + +std::string CheckUnionZeroInit::generateTestMessage(const Tokenizer &tokenizer, + const Settings &settings) +{ + std::stringstream ss; + + const std::vector unions = parseUnions(*tokenizer.getSymbolDatabase(), + settings); + for (const Union &u : unions) { + ss << "Union{"; + ss << "name=\"" << u.name << "\", "; + ss << "scope=" << u.scope << ", "; + ss << "isLargestMemberFirst=" << u.isLargestMemberFirst(); + ss << "}" << std::endl; + + const UnionMember *largest = u.largestMember(); + for (const UnionMember &m : u.members) { + ss << " Member{"; + ss << "name=\"" << m.name << "\", "; + ss << "size=" << m.size; + ss << "}"; + if (&m == largest) + ss << " (largest)"; + ss << std::endl; + } + } + + return ss.str(); +} diff --git a/lib/checkunionzeroinit.h b/lib/checkunionzeroinit.h new file mode 100644 index 00000000000..3b1dd869905 --- /dev/null +++ b/lib/checkunionzeroinit.h @@ -0,0 +1,77 @@ +/* -*- C++ -*- + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2025 Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + + +//--------------------------------------------------------------------------- +#ifndef checkunionzeroinitH +#define checkunionzeroinitH +//--------------------------------------------------------------------------- + +#include "check.h" +#include "config.h" + +#include + +class ErrorLogger; +class Settings; +class Token; +class Tokenizer; +struct UnionMember; + +/// @addtogroup Checks +/// @{ + +/** + * @brief Check for error-prone zero initialization of unions. + */ + +class CPPCHECKLIB CheckUnionZeroInit : public Check { + friend class TestUnionZeroInit; + +public: + /** This constructor is used when registering the CheckUnionZeroInit */ + CheckUnionZeroInit() : Check(myName()) {} + +private: + /** This constructor is used when running checks. */ + CheckUnionZeroInit(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) + : Check(myName(), tokenizer, settings, errorLogger) {} + + /** @brief Run checks against the normal token list */ + void runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger) override; + + /** Check for error-prone zero initialization of unions. */ + void check(); + + void unionZeroInitError(const Token *tok, const UnionMember &largestMember); + + void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override; + + static std::string generateTestMessage(const Tokenizer &tokenizer, const Settings &settings); + + static std::string myName() { + return "CheckUnionZeroInit"; + } + + std::string classInfo() const override { + return "Check for error-prone zero initialization of unions.\n"; + } +}; +/// @} +//--------------------------------------------------------------------------- +#endif // checkunionzeroinitH diff --git a/lib/cppcheck.vcxproj b/lib/cppcheck.vcxproj index 3d830e2894c..f2594a26ca4 100644 --- a/lib/cppcheck.vcxproj +++ b/lib/cppcheck.vcxproj @@ -56,6 +56,7 @@ + @@ -127,6 +128,7 @@ + diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 9c6db66a4c6..df4fe6333e5 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -10043,7 +10043,7 @@ void Tokenizer::simplifyBitfields() !Token::Match(tok->next(), "case|public|protected|private|class|struct") && !Token::simpleMatch(tok->tokAt(2), "default :")) { Token *tok1 = typeTok->next(); - if (Token::Match(tok1, "%name% : %num% [;=]")) + if (Token::Match(tok1, "%name% : %num% [;=,]")) if (!tok1->setBits(MathLib::toBigNumber(tok1->tokAt(2)))) tooLargeError(tok1->tokAt(2)); if (tok1 && tok1->tokAt(2) && diff --git a/oss-fuzz/Makefile b/oss-fuzz/Makefile index 5d4658790aa..8d1ea962754 100644 --- a/oss-fuzz/Makefile +++ b/oss-fuzz/Makefile @@ -70,6 +70,7 @@ LIBOBJ = $(libcppdir)/valueflow.o \ $(libcppdir)/checkstring.o \ $(libcppdir)/checktype.o \ $(libcppdir)/checkuninitvar.o \ + $(libcppdir)/checkunionzeroinit.o \ $(libcppdir)/checkunusedfunctions.o \ $(libcppdir)/checkunusedvar.o \ $(libcppdir)/checkvaarg.o \ @@ -243,6 +244,9 @@ $(libcppdir)/checktype.o: ../lib/checktype.cpp ../lib/addoninfo.h ../lib/astutil $(libcppdir)/checkuninitvar.o: ../lib/checkuninitvar.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkers.h ../lib/checknullpointer.h ../lib/checkuninitvar.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.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 $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkuninitvar.cpp +$(libcppdir)/checkunionzeroinit.o: ../lib/checkunionzeroinit.cpp ../lib/addoninfo.h ../lib/check.h ../lib/checkers.h ../lib/checkunionzeroinit.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.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/valueflow.h ../lib/vfvalue.h + $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkunionzeroinit.cpp + $(libcppdir)/checkunusedfunctions.o: ../lib/checkunusedfunctions.cpp ../externals/tinyxml2/tinyxml2.h ../lib/addoninfo.h ../lib/astutils.h ../lib/checkers.h ../lib/checkunusedfunctions.h ../lib/config.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 $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkunusedfunctions.cpp diff --git a/test/testrunner.vcxproj b/test/testrunner.vcxproj index 8e1ce01f07e..997644dd6a0 100755 --- a/test/testrunner.vcxproj +++ b/test/testrunner.vcxproj @@ -107,6 +107,7 @@ + diff --git a/test/testunionzeroinit.cpp b/test/testunionzeroinit.cpp new file mode 100644 index 00000000000..d41a5edeee8 --- /dev/null +++ b/test/testunionzeroinit.cpp @@ -0,0 +1,149 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2025 Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "checkunionzeroinit.h" +#include "errortypes.h" +#include "fixture.h" +#include "helpers.h" +#include "settings.h" + +static std::string expectedErrorMessage(int lno, int cno, const std::string &varName, const std::string &largestMemberName) +{ + std::stringstream ss; + ss << "[test.cpp:" << lno << ":" << cno << "]: (portability) "; + ss << "Zero initializing union '" << varName << "' "; + ss << "does not guarantee its complete storage to be zero initialized as its largest member is not declared as the first member. "; + ss << "Consider making " << largestMemberName << " the first member or favor memset(). [UnionZeroInit]"; + ss << std::endl; + return ss.str(); +} + +class TestUnionZeroInit : public TestFixture { +public: + TestUnionZeroInit() : TestFixture("TestUnionZeroInit") {} + +private: + const Settings mSettings = settingsBuilder().severity(Severity::portability).library("std.cfg").build(); + std::string mMessage; + + void run() override { + mNewTemplate = true; + TEST_CASE(basic); + TEST_CASE(arrayMember); + TEST_CASE(structMember); + TEST_CASE(unknownType); + TEST_CASE(bitfields); + } + +#define checkUnionZeroInit(...) checkUnionZeroInit_(__FILE__, __LINE__, __VA_ARGS__) + template + void checkUnionZeroInit_(const char* file, int line, const char (&code)[size], bool cpp = true) { + // Tokenize.. + SimpleTokenizer tokenizer(mSettings, *this, cpp); + ASSERT_LOC(tokenizer.tokenize(code), file, line); + + CheckUnionZeroInit(&tokenizer, &mSettings, this).check(); + + mMessage = CheckUnionZeroInit::generateTestMessage(tokenizer, mSettings); + } + + void basic() { + checkUnionZeroInit( + "union bad_union_0 {\n" + " char c;\n" + " long long i64;\n" + " void *p;\n" + "};\n" + "\n" + "typedef union {\n" + " char c;\n" + " int i;\n" + "} bad_union_1;\n" + "\n" + "extern void e(union bad_union_0 *);\n" + "\n" + "void\n" + "foo(void)\n" + "{\n" + " union { int i; char c; } good0 = {0};\n" + " union { int i; char c; } good1 = {};\n" + "\n" + " union { char c; int i; } bad0 = {0};\n" + " union bad_union_0 bad1 = {0};\n" + " e(&bad1);\n" + " bad_union_1 bad2 = {0};\n" + "}"); + const std::string exp = expectedErrorMessage(20, 28, "bad0", "i") + + expectedErrorMessage(21, 21, "bad1", "i64") + + expectedErrorMessage(23, 15, "bad2", "i"); + ASSERT_EQUALS_MSG(exp, errout_str(), mMessage); + } + + void arrayMember() { + checkUnionZeroInit( + "void foo(void) {\n" + " union { int c; char s8[2]; } u = {0};\n" + "}"); + ASSERT_EQUALS_MSG("", errout_str(), mMessage); + } + + void structMember() { + checkUnionZeroInit( + "void foo(void) {\n" + " union {\n" + " int c;\n" + " struct {\n" + " char x;\n" + " struct {\n" + " char y;\n" + " } s1;\n" + " } s0;\n" + " } u = {0};\n" + "}"); + ASSERT_EQUALS_MSG("", errout_str(), mMessage); + } + + void unknownType() { + checkUnionZeroInit( + "union u {\n" + " Unknown x;\n" + "}"); + ASSERT_EQUALS_MSG("", errout_str(), mMessage); + } + + void bitfields() { + checkUnionZeroInit( + "typedef union Evex {\n" + " int u32;\n" + " struct {\n" + " char mmm:3,\n" + " b4:1,\n" + " r4:1,\n" + " b3:1,\n" + " x3:1,\n" + " r3:1;\n" + " } extended;\n" + "} Evex;\n" + "\n" + "void foo(void) {\n" + " Evex evex = {0};\n" + "}"); + ASSERT_EQUALS_MSG("", errout_str(), mMessage); + } +}; +REGISTER_TEST(TestUnionZeroInit)