From 634d45a210e28e924019b379c3829724557b836b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludvig=20Gunne=20Lindstr=C3=B6m?= Date: Sun, 15 Jun 2025 15:53:22 +0200 Subject: [PATCH 1/2] account for bitfields when computing struct size --- lib/valueflow.cpp | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index e42a3020878..81e65c8347f 100644 --- a/lib/valueflow.cpp +++ b/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) From 465f07f85a561841ba7da3386994ab8ed2a6b4ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludvig=20Gunne=20Lindstr=C3=B6m?= Date: Sun, 15 Jun 2025 12:10:42 +0200 Subject: [PATCH 2/2] add tests --- test/testother.cpp | 9 +++++++++ test/testvalueflow.cpp | 43 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/test/testother.cpp b/test/testother.cpp index 5cdbef0feb5..464889ce012 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -11240,6 +11240,15 @@ class TestOther : public TestFixture { "};\n" "void f(S 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() { diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 4ee4827f0a8..d5a2b4f019b 100644 --- a/test/testvalueflow.cpp +++ b/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)