Fix danmar#5115, #10093 (False positive: redundantAssignment when usi…

…ng a union) (danmar#6484)
chrchr-github · Jun 7, 2024 · f1ddd1a · f1ddd1a
1 parent ac0bd6d
commit f1ddd1a
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 3 deletions.
diff --git a/lib/checkother.cpp b/lib/checkother.cpp
@@ -510,8 +510,17 @@ void CheckOther::checkRedundantAssignment()
                 else
                     start = tok->findExpressionStartEndTokens().second->next();
 
+                const Token * tokenToCheck = tok->astOperand1();
+
+                // Check if we are working with union
+                for (const Token* tempToken = tokenToCheck; Token::simpleMatch(tempToken, ".");) {
+                    tempToken = tempToken->astOperand1();
+                    if (tempToken && tempToken->variable() && tempToken->variable()->type() && tempToken->variable()->type()->isUnionType())
+                        tokenToCheck = tempToken;
+                }
+
                 // Get next assignment..
-                const Token *nextAssign = fwdAnalysis.reassign(tok->astOperand1(), start, scope->bodyEnd);
+                const Token *nextAssign = fwdAnalysis.reassign(tokenToCheck, start, scope->bodyEnd);
 
                 if (!nextAssign)
                     continue;

diff --git a/lib/fwdanalysis.cpp b/lib/fwdanalysis.cpp
@@ -323,8 +323,25 @@ 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()) {
+                    while (parent && Token::simpleMatch(parent->astParent(), "."))
+                        parent = parent->astParent();
+                    if (parent && parent->valueType() && Token::Match(parent->astParent(), "%assign%") && !Token::Match(parent->astParent()->astParent(), "%assign%") && parent->astParent()->astOperand1() == parent) {
+                        const Token * assignment = parent->astParent()->astOperand2();
+                        while (Token::simpleMatch(assignment, ".") && assignment->varId() != expr->varId())
+                            assignment = assignment->astOperand1();
+                        if (assignment && assignment->varId() != expr->varId()) {
+                            if (assignment->valueType() && assignment->valueType()->pointer) // Bailout
+                                return Result(Result::Type::BAILOUT);
+                            return Result(Result::Type::WRITE, parent->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
@@ -4472,6 +4472,86 @@ class TestOther : public TestFixture {
               "  }\n"
               "}");
         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"
+              "    lTotal += u.b.b1;\n"
+              "    u.l1 = 2;\n" //Should not show RedundantAssignment
+              "}", 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 <stdio.h>\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());
     }
 
     void switchRedundantOperationTest() {