Skip to content

Commit 4823852

Browse files
authored
Fix #14150 (Add unionzeroinit check) (danmar#7843)
This has bitten me before and is definitely a foot gun. GCC 15[1] changed their semantics related to zero initialization of unions; from initializing the complete union (sizeof union) to only zero initializing the first member. If the same first member is not the largest one, the state of the remaining storage is considered undefined and in practice most likely stack garbage. The unionzeroinit addon can detect such scenarios and emit a warning. It does not cover the designated initializers as I would interpret those as being intentional. Example output from one of my projects: ``` x86-decoder.c:294:7: warning: Zero initializing union Evex does not guarantee its complete storage to be zero initialized as its largest member is not declared as the first member. Consider making u32 the first member or favor memset(). [unionzeroinit-unionzeroinit] Evex evex = {0}; ^ ``` [1] https://trofi.github.io/posts/328-c-union-init-and-gcc-15.html
1 parent 67cef47 commit 4823852

File tree

9 files changed

+454
-0
lines changed

9 files changed

+454
-0
lines changed

.selfcheck_unused_suppressions

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,6 @@ unusedFunction:lib/symboldatabase.cpp
66

77
# Q_OBJECT functions which are not called in our code
88
unusedFunction:cmake.output.notest/gui/cppcheck-gui_autogen/*/moc_aboutdialog.cpp
9+
10+
# CheckUnionZeroInit::generateTestMessage only used in tests.
11+
unusedFunction:lib/checkunionzeroinit.cpp

Makefile

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ LIBOBJ = $(libcppdir)/valueflow.o \
224224
$(libcppdir)/checkstring.o \
225225
$(libcppdir)/checktype.o \
226226
$(libcppdir)/checkuninitvar.o \
227+
$(libcppdir)/checkunionzeroinit.o \
227228
$(libcppdir)/checkunusedfunctions.o \
228229
$(libcppdir)/checkunusedvar.o \
229230
$(libcppdir)/checkvaarg.o \
@@ -346,6 +347,7 @@ TESTOBJ = test/fixture.o \
346347
test/testtokenrange.o \
347348
test/testtype.o \
348349
test/testuninitvar.o \
350+
test/testunionzeroinit.o \
349351
test/testunusedfunctions.o \
350352
test/testunusedprivfunc.o \
351353
test/testunusedvar.o \
@@ -561,6 +563,9 @@ $(libcppdir)/checktype.o: lib/checktype.cpp lib/addoninfo.h lib/astutils.h lib/c
561563
$(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
562564
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkuninitvar.cpp
563565

566+
$(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
567+
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkunionzeroinit.cpp
568+
564569
$(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
565570
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkunusedfunctions.cpp
566571

@@ -909,6 +914,9 @@ test/testtype.o: test/testtype.cpp lib/addoninfo.h lib/check.h lib/checkers.h li
909914
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
910915
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testuninitvar.cpp
911916

917+
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
918+
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testunionzeroinit.cpp
919+
912920
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
913921
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testunusedfunctions.cpp
914922

lib/checkers.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ namespace checkers {
200200
{"CheckType::checkLongCast","style"},
201201
{"CheckType::checkSignConversion","warning"},
202202
{"CheckType::checkTooBigBitwiseShift","platform"},
203+
{"CheckUninitVar::check",""},
203204
{"CheckUninitVar::analyseWholeProgram",""},
204205
{"CheckUninitVar::check",""},
205206
{"CheckUninitVar::valueFlowUninit",""},

lib/checkunionzeroinit.cpp

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
/*
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2025 Cppcheck team.
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
#include "checkunionzeroinit.h"
20+
21+
#include <sstream>
22+
#include <unordered_map>
23+
#include <vector>
24+
25+
#include "errortypes.h"
26+
#include "settings.h"
27+
#include "symboldatabase.h"
28+
#include "token.h"
29+
#include "tokenize.h"
30+
#include "valueflow.h"
31+
32+
static const CWE CWEXXX(0U); /* TODO */
33+
34+
static const std::string noname;
35+
36+
// Register this check class (by creating a static instance of it)
37+
namespace {
38+
CheckUnionZeroInit instance;
39+
}
40+
41+
struct UnionMember {
42+
UnionMember()
43+
: name(noname)
44+
, size(0) {}
45+
46+
UnionMember(const std::string &name_, size_t size_)
47+
: name(name_)
48+
, size(size_) {}
49+
50+
const std::string &name;
51+
size_t size;
52+
};
53+
54+
struct Union {
55+
Union(const Scope *scope_, const std::string &name_)
56+
: scope(scope_)
57+
, name(name_) {}
58+
59+
const Scope *scope;
60+
const std::string &name;
61+
std::vector<UnionMember> members;
62+
63+
const UnionMember *largestMember() const {
64+
const UnionMember *largest = nullptr;
65+
for (const UnionMember &m : members) {
66+
if (m.size == 0)
67+
return nullptr;
68+
if (largest == nullptr || m.size > largest->size)
69+
largest = &m;
70+
}
71+
return largest;
72+
}
73+
74+
bool isLargestMemberFirst() const {
75+
const UnionMember *largest = largestMember();
76+
return largest == nullptr || largest == &members[0];
77+
}
78+
};
79+
80+
static UnionMember parseUnionMember(const Variable &var,
81+
const Settings &settings)
82+
{
83+
const Token *nameToken = var.nameToken();
84+
if (nameToken == nullptr)
85+
return UnionMember();
86+
87+
const ValueType *vt = nameToken->valueType();
88+
size_t size = 0;
89+
if (var.isArray()) {
90+
size = var.dimension(0);
91+
} else if (vt != nullptr) {
92+
size = ValueFlow::getSizeOf(*vt, settings,
93+
ValueFlow::Accuracy::ExactOrZero);
94+
}
95+
return UnionMember(nameToken->str(), size);
96+
}
97+
98+
static std::vector<Union> parseUnions(const SymbolDatabase &symbolDatabase,
99+
const Settings &settings)
100+
{
101+
std::vector<Union> unions;
102+
103+
for (const Scope &scope : symbolDatabase.scopeList) {
104+
if (scope.type != ScopeType::eUnion)
105+
continue;
106+
107+
Union u(&scope, scope.className);
108+
for (const Variable &var : scope.varlist) {
109+
u.members.push_back(parseUnionMember(var, settings));
110+
}
111+
unions.push_back(u);
112+
}
113+
114+
return unions;
115+
}
116+
117+
static bool isZeroInitializer(const Token *tok)
118+
{
119+
return Token::simpleMatch(tok, "= { 0 } ;") ||
120+
Token::simpleMatch(tok, "= { } ;");
121+
}
122+
123+
void CheckUnionZeroInit::check()
124+
{
125+
if (!mSettings->severity.isEnabled(Severity::portability))
126+
return;
127+
128+
logChecker("CheckUnionZeroInit::check"); // portability
129+
130+
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
131+
132+
std::unordered_map<const Scope *, Union> unionsByScopeId;
133+
const std::vector<Union> unions = parseUnions(*symbolDatabase, *mSettings);
134+
for (const Union &u : unions) {
135+
unionsByScopeId.insert({u.scope, u});
136+
}
137+
138+
for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) {
139+
if (!tok->isName() || !isZeroInitializer(tok->next()))
140+
continue;
141+
142+
const ValueType *vt = tok->valueType();
143+
if (vt == nullptr || vt->typeScope == nullptr)
144+
continue;
145+
auto it = unionsByScopeId.find(vt->typeScope);
146+
if (it == unionsByScopeId.end())
147+
continue;
148+
const Union &u = it->second;
149+
if (!u.isLargestMemberFirst()) {
150+
const UnionMember *largestMember = u.largestMember();
151+
assert(largestMember != nullptr);
152+
unionZeroInitError(tok, *largestMember);
153+
}
154+
}
155+
}
156+
157+
void CheckUnionZeroInit::runChecks(const Tokenizer &tokenizer,
158+
ErrorLogger *errorLogger)
159+
{
160+
CheckUnionZeroInit(&tokenizer, &tokenizer.getSettings(), errorLogger).check();
161+
}
162+
163+
void CheckUnionZeroInit::unionZeroInitError(const Token *tok,
164+
const UnionMember& largestMember)
165+
{
166+
reportError(tok, Severity::portability, "UnionZeroInit",
167+
"$symbol:" + (tok != nullptr ? tok->str() : "") + "\n"
168+
"Zero initializing union '$symbol' does not guarantee " +
169+
"its complete storage to be zero initialized as its largest member " +
170+
"is not declared as the first member. Consider making " +
171+
largestMember.name + " the first member or favor memset().",
172+
CWEXXX, Certainty::normal);
173+
}
174+
175+
void CheckUnionZeroInit::getErrorMessages(ErrorLogger *errorLogger,
176+
const Settings *settings) const
177+
{
178+
CheckUnionZeroInit c(nullptr, settings, errorLogger);
179+
c.unionZeroInitError(nullptr, UnionMember());
180+
}
181+
182+
std::string CheckUnionZeroInit::generateTestMessage(const Tokenizer &tokenizer,
183+
const Settings &settings)
184+
{
185+
std::stringstream ss;
186+
187+
const std::vector<Union> unions = parseUnions(*tokenizer.getSymbolDatabase(),
188+
settings);
189+
for (const Union &u : unions) {
190+
ss << "Union{";
191+
ss << "name=\"" << u.name << "\", ";
192+
ss << "scope=" << u.scope << ", ";
193+
ss << "isLargestMemberFirst=" << u.isLargestMemberFirst();
194+
ss << "}" << std::endl;
195+
196+
const UnionMember *largest = u.largestMember();
197+
for (const UnionMember &m : u.members) {
198+
ss << " Member{";
199+
ss << "name=\"" << m.name << "\", ";
200+
ss << "size=" << m.size;
201+
ss << "}";
202+
if (&m == largest)
203+
ss << " (largest)";
204+
ss << std::endl;
205+
}
206+
}
207+
208+
return ss.str();
209+
}

lib/checkunionzeroinit.h

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/* -*- C++ -*-
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2025 Cppcheck team.
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
20+
//---------------------------------------------------------------------------
21+
#ifndef checkunionzeroinitH
22+
#define checkunionzeroinitH
23+
//---------------------------------------------------------------------------
24+
25+
#include "check.h"
26+
#include "config.h"
27+
28+
#include <string>
29+
30+
class ErrorLogger;
31+
class Settings;
32+
class Token;
33+
class Tokenizer;
34+
struct UnionMember;
35+
36+
/// @addtogroup Checks
37+
/// @{
38+
39+
/**
40+
* @brief Check for error-prone zero initialization of unions.
41+
*/
42+
43+
class CPPCHECKLIB CheckUnionZeroInit : public Check {
44+
friend class TestUnionZeroInit;
45+
46+
public:
47+
/** This constructor is used when registering the CheckUnionZeroInit */
48+
CheckUnionZeroInit() : Check(myName()) {}
49+
50+
private:
51+
/** This constructor is used when running checks. */
52+
CheckUnionZeroInit(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
53+
: Check(myName(), tokenizer, settings, errorLogger) {}
54+
55+
/** @brief Run checks against the normal token list */
56+
void runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger) override;
57+
58+
/** Check for error-prone zero initialization of unions. */
59+
void check();
60+
61+
void unionZeroInitError(const Token *tok, const UnionMember &largestMember);
62+
63+
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override;
64+
65+
static std::string generateTestMessage(const Tokenizer &tokenizer, const Settings &settings);
66+
67+
static std::string myName() {
68+
return "CheckUnionZeroInit";
69+
}
70+
71+
std::string classInfo() const override {
72+
return "Check for error-prone zero initialization of unions.\n";
73+
}
74+
};
75+
/// @}
76+
//---------------------------------------------------------------------------
77+
#endif // checkunionzeroinitH

lib/cppcheck.vcxproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
<ClCompile Include="checkstring.cpp" />
5757
<ClCompile Include="checktype.cpp" />
5858
<ClCompile Include="checkuninitvar.cpp" />
59+
<ClCompile Include="checkunionzeroinit.cpp" />
5960
<ClCompile Include="checkunusedfunctions.cpp" />
6061
<ClCompile Include="checkunusedvar.cpp" />
6162
<ClCompile Include="checkvaarg.cpp" />
@@ -127,6 +128,7 @@
127128
<ClInclude Include="checkstring.h" />
128129
<ClInclude Include="checktype.h" />
129130
<ClInclude Include="checkuninitvar.h" />
131+
<ClInclude Include="checkunionzeroinit.h" />
130132
<ClInclude Include="checkunusedfunctions.h" />
131133
<ClInclude Include="checkunusedvar.h" />
132134
<ClInclude Include="checkvaarg.h" />

oss-fuzz/Makefile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ LIBOBJ = $(libcppdir)/valueflow.o \
7070
$(libcppdir)/checkstring.o \
7171
$(libcppdir)/checktype.o \
7272
$(libcppdir)/checkuninitvar.o \
73+
$(libcppdir)/checkunionzeroinit.o \
7374
$(libcppdir)/checkunusedfunctions.o \
7475
$(libcppdir)/checkunusedvar.o \
7576
$(libcppdir)/checkvaarg.o \
@@ -243,6 +244,9 @@ $(libcppdir)/checktype.o: ../lib/checktype.cpp ../lib/addoninfo.h ../lib/astutil
243244
$(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
244245
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkuninitvar.cpp
245246

247+
$(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
248+
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkunionzeroinit.cpp
249+
246250
$(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
247251
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkunusedfunctions.cpp
248252

test/testrunner.vcxproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
<ClCompile Include="testtokenrange.cpp" />
108108
<ClCompile Include="testtype.cpp" />
109109
<ClCompile Include="testuninitvar.cpp" />
110+
<ClCompile Include="testunionzeroinit.cpp" />
110111
<ClCompile Include="testunusedfunctions.cpp" />
111112
<ClCompile Include="testunusedprivfunc.cpp" />
112113
<ClCompile Include="testunusedvar.cpp" />

0 commit comments

Comments
 (0)