diff --git a/lib/fwdanalysis.cpp b/lib/fwdanalysis.cpp index d735e6771e9..713d8aa228b 100644 --- a/lib/fwdanalysis.cpp +++ b/lib/fwdanalysis.cpp @@ -323,8 +323,23 @@ FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const Token * return Result(Result::Type::WRITE, parent->astParent()); return Result(Result::Type::READ); } - if (mWhat == What::Reassign && parent->valueType() && parent->valueType()->pointer && Token::Match(parent->astParent(), "%assign%") && parent == parent->astParent()->astOperand1()) - return Result(Result::Type::READ); + if (mWhat == What::Reassign) { + if (parent->variable() && parent->variable()->type() && parent->variable()->type()->isUnionType() && parent->varId() == expr->varId()) { + const Token * tempToken = parent; + while (tempToken && Token::Match(tempToken->astParent(), ".|->")) + tempToken = tempToken->astParent(); + if (tempToken->valueType() && Token::Match(tempToken->astParent(), "%assign%") && !Token::Match(tempToken->astParent()->astParent(), "%assign%") && tempToken->astParent()->astOperand1() == tempToken) { + const Token * assignment = tempToken->astParent()->astOperand2(); + while (Token::Match(assignment, ".|->") && assignment->varId() != expr->varId()) + assignment = assignment->astOperand1(); + if (assignment && assignment->varId() != expr->varId()) + return Result(Result::Type::WRITE, tempToken->astParent()); + } + return Result(Result::Type::READ); + } + if (parent->valueType() && parent->valueType()->pointer && Token::Match(parent->astParent(), "%assign%")) + return Result(Result::Type::READ); + } if (Token::Match(parent->astParent(), "%assign%") && !parent->astParent()->astParent() && parent == parent->astParent()->astOperand1()) { if (mWhat == What::Reassign) diff --git a/test/testother.cpp b/test/testother.cpp index 9a76768a1b9..462b32247fd 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -4476,24 +4476,80 @@ class TestOther : public TestFixture { // Ticket #5115 "redundantAssignment when using a union" check("void main(void)\n" "{\n" - " unsigned long lTotal = 0;\n" + " short lTotal = 0;\n" " union\n" " {\n" - " unsigned long l1;\n" + " short l1;\n" " struct\n" " {\n" " unsigned char b1;\n" " unsigned char b2;\n" - " unsigned char b3;\n" - " unsigned char b4;\n" " } b;\n" " } u;\n" " u.l1 = 1;\n" " lTotal += u.b.b1;\n" " u.l1 = 2;\n" //Should not show RedundantAssignment - " lTotal += u.b.b1;\n" - " printf(\"b1 %u, b2 %u, b3 %u b4 %u, l1 %lu, lTotal %lu\",\n" - " u.b.b1, u.b.b2, u.b.b3, u.b.b4, u.l1, lTotal);\n" + "}", true, false, false); + ASSERT_EQUALS("", errout_str()); + + // Ticket #5115 "redundantAssignment when using a union" + check("void main(void)\n" + "{\n" + " short lTotal = 0;\n" + " union\n" + " {\n" + " short l1;\n" + " struct\n" + " {\n" + " unsigned char b1;\n" + " unsigned char b2;\n" + " } b;\n" + " } u;\n" + " u.l1 = 1;\n" + " u.l1 = 2;\n" + "}", true, false, false); + ASSERT_EQUALS("[test.cpp:13] -> [test.cpp:14]: (style) Variable 'u.l1' is reassigned a value before the old one has been used.\n", errout_str()); + + // Ticket #10093 "redundantAssignment when using a union" + check("typedef union fixed32_union {\n" + " struct {\n" + " unsigned32 abcd;\n" + " } u32;\n" + " struct {\n" + " unsigned16 ab;\n" + " unsigned16 cd;\n" + " } u16;" + " struct {\n" + " unsigned8 a;\n" + " unsigned8 b;\n" + " unsigned8 c;\n" + " unsigned8 d;\n" + " } b;\n" + "} fixed32;\n" + "void func1(void) {\n" + " fixed32 m;\n" + " m.u16.ab = 47;\n" + " m.u16.cd = 0;\n" + " m.u16.ab = m.u32.abcd / 53;\n" + "}", true, false, false); + ASSERT_EQUALS("", errout_str()); + + // Ticket #10093 "redundantAssignment when using a union" + check("#include \n" + "typedef union{\n" + " char as_char[4];\n" + " int as_int;\n" + "} union_t;\n" + "void fn(char *data, int len) {\n" + " int i;\n" + " for (i = 0; i < len; i++)\n" + " data[i] = 'a';\n" + "}\n" + "int main(int argc, char *argv[]) {\n" + " union_t u;\n" + " u.as_int = 42;\n" + " fn(&u.as_char[0], 4);\n" + " u.as_int = 0;\n" "}", true, false, false); ASSERT_EQUALS("", errout_str()); }