Fix #10093: False positive: Redundant assignment with union

danmar · Jun 5, 2024 · 6a71b26 · 6a71b26
1 parent 0ee7816
commit 6a71b26
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 9 deletions.
diff --git 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
@@ -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 <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());
     }