Skip to content

Fix #5115 (False positive: redundantAssignment when using a union) #6484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 7, 2024
Merged

Fix #5115 (False positive: redundantAssignment when using a union) #6484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0ee7816
Fix #5115: redundantAssignment when using a union
olabetskyi Jun 4, 2024
6a71b26
Fix #10093: False positive: Redundant assignment with union
olabetskyi Jun 5, 2024
05a4cb4
Ci fix
olabetskyi Jun 5, 2024
33b3d8c
Small fix
olabetskyi Jun 6, 2024
48e987b
Bailout
olabetskyi Jun 6, 2024
0e8e004
Scope fix
olabetskyi Jun 7, 2024
ff580f9
Make Ci happy
olabetskyi Jun 7, 2024
57d2dfa
Small nit
olabetskyi Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion lib/checkother.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
21 changes: 19 additions & 2 deletions lib/fwdanalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
80 changes: 80 additions & 0 deletions test/testother.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading