fix #13653 & #13850: Wrong struct size computed for bitfields (danmar#7597)

Author: ludviggunne

lib/valueflow.cpp

@@ -440,6 +440,7 @@ static Result accumulateStructMembers(const Scope* scope, F f, ValueFlow::Accura
     for (const Variable& var : scope->varlist) {
         if (var.isStatic())
             continue;
+        const size_t bits = var.nameToken() ? var.nameToken()->bits() : 0;
         if (const ValueType* vt = var.valueType()) {
             if (vt->type == ValueType::Type::RECORD && vt->typeScope == scope)
                 return {0, false};
@@ -449,12 +450,12 @@ static Result accumulateStructMembers(const Scope* scope, F f, ValueFlow::Accura
             if (var.nameToken()->scope() != scope && var.nameToken()->scope()->definedType) { // anonymous union
                 const auto ret = anonScopes.insert(var.nameToken()->scope());
                 if (ret.second)
-                    total = f(total, *vt, dim);
+                    total = f(total, *vt, dim, bits);
             }
             else
-                total = f(total, *vt, dim);
+                total = f(total, *vt, dim, bits);
         }
-        if (accuracy == ValueFlow::Accuracy::ExactOrZero && total == 0)
+        if (accuracy == ValueFlow::Accuracy::ExactOrZero && total == 0 && bits == 0)
             return {0, false};
     }
     return {total, true};
@@ -485,7 +486,7 @@ static size_t getAlignOf(const ValueType& vt, const Settings& settings, ValueFlo
         return align == 0 ? 0 : bitCeil(align);
     }
     if (vt.type == ValueType::Type::RECORD && vt.typeScope) {
-        auto accHelper = [&](size_t max, const ValueType& vt2, size_t /*dim*/) {
+        auto accHelper = [&](size_t max, const ValueType& vt2, size_t /*dim*/, size_t /*bits*/) {
             size_t a = getAlignOf(vt2, settings, accuracy, ++maxRecursion);
             return std::max(max, a);
         };
@@ -534,17 +535,46 @@ size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings &settings, Accur
     if (vt.type == ValueType::Type::CONTAINER)
         return 3 * settings.platform.sizeof_pointer; // Just guess
     if (vt.type == ValueType::Type::RECORD && vt.typeScope) {
-        auto accHelper = [&](size_t total, const ValueType& vt2, size_t dim) -> size_t {
+        size_t currentBitCount = 0;
+        size_t currentBitfieldAlloc = 0;
+        auto accHelper = [&](size_t total, const ValueType& vt2, size_t dim, size_t bits) -> size_t {
+            const size_t charBit = settings.platform.char_bit;
             size_t n = ValueFlow::getSizeOf(vt2, settings,accuracy, ++maxRecursion);
             size_t a = getAlignOf(vt2, settings, accuracy);
+            if (bits > 0) {
+                size_t ret = total;
+                if (currentBitfieldAlloc == 0) {
+                    currentBitfieldAlloc = n;
+                    currentBitCount = 0;
+                } else if (currentBitCount + bits > charBit * currentBitfieldAlloc) {
+                    ret += currentBitfieldAlloc;
+                    currentBitfieldAlloc = n;
+                    currentBitCount = 0;
+                }
+                currentBitCount += bits;
+                return ret;
+            }
             if (n == 0 || a == 0)
                 return accuracy == Accuracy::ExactOrZero ? 0 : total;
             n *= dim;
             size_t padding = (a - (total % a)) % a;
+            if (currentBitCount > 0) {
+                bool fitsInBitfield = currentBitCount + n * charBit <= currentBitfieldAlloc * charBit;
+                bool isAligned = currentBitCount % (charBit * a) == 0;
+                if (vt2.isIntegral() && fitsInBitfield && isAligned) {
+                    currentBitCount += charBit * n;
+                    return total;
+                }
+                n += currentBitfieldAlloc;
+                currentBitfieldAlloc = 0;
+                currentBitCount = 0;
+            }
             return vt.typeScope->type == ScopeType::eUnion ? std::max(total, n) : total + padding + n;
         };
         Result result = accumulateStructMembers(vt.typeScope, accHelper, accuracy);
         size_t total = result.total;
+        if (currentBitCount > 0)
+            total += currentBitfieldAlloc;
         if (const Type* dt = vt.typeScope->definedType) {
             total = std::accumulate(dt->derivedFrom.begin(), dt->derivedFrom.end(), total, [&](size_t v, const Type::BaseInfo& bi) {
                 if (bi.type && bi.type->classScope)

test/testother.cpp

@@ -11240,6 +11240,15 @@ class TestOther : public TestFixture {
               "};\n"
               "void f(S<char, 3> s) {}\n");
         ASSERT_EQUALS("", errout_str());
+
+        Settings settingsUnix32 = settingsBuilder().platform(Platform::Type::Unix32).build();
+        check("struct S {\n" // #13850
+              "    int i0 : 32;\n"
+              "    int i1 : 16;\n"
+              "    unsigned short u16;\n"
+              "};\n"
+              "void f(S s) {}\n", true, true, true, false, &settingsUnix32);
+        ASSERT_EQUALS("", errout_str());
     }
 
     void checkComparisonFunctionIsAlwaysTrueOrFalse() {

test/testvalueflow.cpp

@@ -175,6 +175,7 @@ class TestValueFlow : public TestFixture {
         mNewTemplate = true;
 
         TEST_CASE(performanceIfCount);
+        TEST_CASE(bitfields);
     }
 
     static bool isNotTokValue(const ValueFlow::Value &val) {
@@ -9063,6 +9064,48 @@ class TestValueFlow : public TestFixture {
                "}\n";
         ASSERT_EQUALS(1U, tokenValues(code, "v .", &s).size());
     }
+
+#define testBitfields(structBody, expectedSize) testBitfields_(__FILE__, __LINE__, structBody, expectedSize)
+    void testBitfields_(const char *file, int line, const std::string &structBody, std::size_t expectedSize) {
+        const std::string code = "struct S { " + structBody + " }; const std::size_t size = sizeof(S);";
+        const auto values = tokenValues(code.c_str(), "( S");
+        ASSERT_LOC(!values.empty(), file, line);
+        ASSERT_EQUALS_LOC(expectedSize, values.back().intvalue, file, line);
+    }
+
+    void bitfields() {
+
+        // #13653
+        testBitfields("unsigned int data_rw: 1;\n"
+                      "unsigned int page_address: 4;\n"
+                      "unsigned int register_address: 3;\n",
+                      4);
+
+        testBitfields("unsigned char data_rw: 1;\n"
+                      "unsigned char page_address: 4;\n"
+                      "unsigned char register_address: 3;\n",
+                      1);
+
+        testBitfields("unsigned int a : 1;\n"
+                      "unsigned int b;\n"
+                      "unsigned int c : 1;\n",
+                      12);
+
+        testBitfields("unsigned int a : 1;\n"
+                      "unsigned char b;\n"
+                      "unsigned int c : 1;\n",
+                      12);
+
+        testBitfields("unsigned int a : 31;\n"
+                      "unsigned int b : 2;\n",
+                      8);
+
+        // #13850
+        testBitfields("int a : 32;\n"
+                      "int b : 16;\n"
+                      "unsigned short c;\n",
+                      8);
+    }
 };
 
 REGISTER_TEST(TestValueFlow)