Fix #14150 (Add unionzeroinit check)

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 checker 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

Author: mptre

lib/checkother.cpp

@@ -43,6 +43,7 @@
 #include <map>
 #include <set>
 #include <sstream>
+#include <unordered_map>
 #include <utility>
 
 //---------------------------------------------------------------------------
@@ -4358,6 +4359,135 @@ void CheckOther::checkModuloOfOneError(const Token *tok)
     reportError(tok, Severity::style, "moduloofone", "Modulo of one is always equal to zero");
 }
 
+static const std::string noname;
+
+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<UnionMember> 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<Union> parseUnions(const SymbolDatabase &symbolDatabase,
+                                      const Settings &settings)
+{
+    std::vector<Union> 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::Match(tok, "= { 0| } ;");
+}
+
+
+void CheckOther::checkUnionZeroInit()
+{
+    if (!mSettings->severity.isEnabled(Severity::portability))
+        return;
+
+    logChecker("CheckUnionZeroInit::check"); // portability
+
+    const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
+
+    std::unordered_map<const Scope *, Union> unionsByScopeId;
+    const std::vector<Union> 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 CheckOther::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().");
+}
+
 //-----------------------------------------------------------------------------
 // Overlapping write (undefined behavior)
 //-----------------------------------------------------------------------------
@@ -4576,6 +4706,7 @@ void CheckOther::runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger)
     checkOther.checkAccessOfMovedVariable();
     checkOther.checkModuloOfOne();
     checkOther.checkOverlappingWrite();
+    checkOther.checkUnionZeroInit();
 }
 
 void CheckOther::getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const
@@ -4658,4 +4789,5 @@ void CheckOther::getErrorMessages(ErrorLogger *errorLogger, const Settings *sett
     const std::vector<const Token *> nullvec;
     c.funcArgOrderDifferent("function", nullptr, nullptr, nullvec, nullvec);
     c.checkModuloOfOneError(nullptr);
+    c.unionZeroInitError(nullptr, UnionMember());
 }

lib/checkother.h

@@ -40,6 +40,7 @@ class Function;
 class Variable;
 class ErrorLogger;
 class Tokenizer;
+struct UnionMember;
 
 /// @addtogroup Checks
 /// @{
@@ -194,6 +195,8 @@ class CPPCHECKLIB CheckOther : public Check {
 
     void checkModuloOfOne();
 
+    void checkUnionZeroInit();
+
     void checkOverlappingWrite();
     void overlappingWriteUnion(const Token *tok);
     void overlappingWriteFunction(const Token *tok, const std::string& funcname);
@@ -257,6 +260,7 @@ class CPPCHECKLIB CheckOther : public Check {
     void knownPointerToBoolError(const Token* tok, const ValueFlow::Value* value);
     void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2);
     void checkModuloOfOneError(const Token *tok);
+    void unionZeroInitError(const Token *tok, const UnionMember& largestMember);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override;
 

test/testother.cpp

@@ -27,6 +27,17 @@
 #include <cstddef>
 #include <string>
 
+static std::string unionZeroInitMessage(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 TestOther : public TestFixture {
 public:
     TestOther() : TestFixture("TestOther") {}
@@ -307,6 +318,12 @@ class TestOther : public TestFixture {
 
         TEST_CASE(knownConditionFloating);
         TEST_CASE(knownConditionPrefixed);
+
+        TEST_CASE(unionZeroInitBasic);
+        TEST_CASE(unionZeroInitArrayMember);
+        TEST_CASE(unionZeroInitStructMember);
+        TEST_CASE(unionZeroInitUnknownType);
+        TEST_CASE(unionZeroInitBitfields);
     }
 
 #define check(...) check_(__FILE__, __LINE__, __VA_ARGS__)
@@ -13242,6 +13259,90 @@ class TestOther : public TestFixture {
             "[test.cpp:2:13] -> [test.cpp:3:11]: (style) The comparison 'i > +1' is always false. [knownConditionTrueFalse]\n",
             errout_str());
     }
+
+    void unionZeroInitBasic() {
+        check(
+            "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 = unionZeroInitMessage(20, 28, "bad0", "i") +
+                                unionZeroInitMessage(21, 21, "bad1", "i64") +
+                                unionZeroInitMessage(23, 15, "bad2", "i");
+        ASSERT_EQUALS(exp, errout_str());
+    }
+
+    void unionZeroInitArrayMember() {
+        check(
+            "void foo(void) {\n"
+            " union { int c; char s8[2]; } u = {0};\n"
+            "}");
+        ASSERT_EQUALS("", errout_str());
+    }
+
+    void unionZeroInitStructMember() {
+        check(
+            "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("", errout_str());
+    }
+
+    void unionZeroInitUnknownType() {
+        check(
+            "union u {\n"
+            "  Unknown x;\n"
+            "}");
+        ASSERT_EQUALS("", errout_str());
+    }
+
+    void unionZeroInitBitfields() {
+        check(
+            "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("", errout_str());
+    }
 };
 
 REGISTER_TEST(TestOther)