Skip to content

Commit b7ad05b

Browse files
ludviggunneLudvig Gunne Lindström
and
Ludvig Gunne Lindström
authored
fix #13227: False positive: legacy uninitvar (#6936)
Co-authored-by: Ludvig Gunne Lindström <[email protected]>
1 parent c2a8e1f commit b7ad05b

File tree

3 files changed

+45
-8
lines changed

3 files changed

+45
-8
lines changed

lib/checkuninitvar.cpp

+27-7
Original file line numberDiff line numberDiff line change
@@ -916,7 +916,7 @@ bool CheckUninitVar::checkIfForWhileHead(const Token *startparentheses, const Va
916916
}
917917

918918
/** recursively check loop, return error token */
919-
const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Variable& var, const Alloc alloc, const std::string &membervar, bool &bailout) const
919+
const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Variable& var, const Alloc alloc, const std::string &membervar, bool &bailout, bool &alwaysReturns) const
920920
{
921921
assert(start->str() == "{");
922922

@@ -941,9 +941,10 @@ const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Va
941941
if (!Token::simpleMatch(top->previous(), "for (") || !Token::simpleMatch(top->link(), ") {"))
942942
continue;
943943
const Token *bodyStart = top->link()->next();
944-
const Token *errorToken1 = checkLoopBodyRecursive(bodyStart, var, alloc, membervar, bailout);
944+
const Token *errorToken1 = checkLoopBodyRecursive(bodyStart, var, alloc, membervar, bailout, alwaysReturns);
945945
if (!errorToken)
946946
errorToken = errorToken1;
947+
bailout |= alwaysReturns;
947948
if (bailout)
948949
return nullptr;
949950
}
@@ -962,11 +963,12 @@ const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Va
962963
return nullptr;
963964
}
964965

965-
const Token *errorToken1 = checkLoopBodyRecursive(tok, var, alloc, membervar, bailout);
966+
bool alwaysReturnsUnused;
967+
const Token *errorToken1 = checkLoopBodyRecursive(tok, var, alloc, membervar, bailout, alwaysReturnsUnused);
966968
tok = tok->link();
967969
if (Token::simpleMatch(tok, "} else {")) {
968970
const Token *elseBody = tok->tokAt(2);
969-
const Token *errorToken2 = checkLoopBodyRecursive(elseBody, var, alloc, membervar, bailout);
971+
const Token *errorToken2 = checkLoopBodyRecursive(elseBody, var, alloc, membervar, bailout, alwaysReturnsUnused);
970972
tok = elseBody->link();
971973
if (errorToken1 && errorToken2)
972974
return errorToken1;
@@ -979,6 +981,23 @@ const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Va
979981
errorToken = errorToken1;
980982
}
981983

984+
if (Token::simpleMatch(tok, "return")) {
985+
bool returnWithoutVar = true;
986+
while (tok && !Token::simpleMatch(tok, ";")) {
987+
if (tok->isVariable() && tok->variable() == &var) {
988+
returnWithoutVar = false;
989+
break;
990+
}
991+
tok = tok->next();
992+
}
993+
if (returnWithoutVar) {
994+
alwaysReturns = true;
995+
return nullptr;
996+
}
997+
}
998+
if (!tok)
999+
break;
1000+
9821001
if (tok->varId() != var.declarationId())
9831002
continue;
9841003

@@ -1068,17 +1087,18 @@ const Token* CheckUninitVar::checkLoopBodyRecursive(const Token *start, const Va
10681087
bool CheckUninitVar::checkLoopBody(const Token *tok, const Variable& var, const Alloc alloc, const std::string &membervar, const bool suppressErrors)
10691088
{
10701089
bool bailout = false;
1071-
const Token *errorToken = checkLoopBodyRecursive(tok, var, alloc, membervar, bailout);
1090+
bool alwaysReturns = false;
1091+
const Token *errorToken = checkLoopBodyRecursive(tok, var, alloc, membervar, bailout, alwaysReturns);
10721092

1073-
if (!suppressErrors && !bailout && errorToken) {
1093+
if (!suppressErrors && !bailout && !alwaysReturns && errorToken) {
10741094
if (membervar.empty())
10751095
uninitvarError(errorToken, errorToken->expressionString(), alloc);
10761096
else
10771097
uninitStructMemberError(errorToken, errorToken->expressionString() + "." + membervar);
10781098
return true;
10791099
}
10801100

1081-
return bailout;
1101+
return bailout || alwaysReturns;
10821102
}
10831103

10841104
void CheckUninitVar::checkRhs(const Token *tok, const Variable &var, Alloc alloc, nonneg int number_of_if, const std::string &membervar)

lib/checkuninitvar.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class CPPCHECKLIB CheckUninitVar : public Check {
9696
const Token* checkExpr(const Token* tok, const Variable& var, Alloc alloc, bool known, bool* bailout = nullptr) const;
9797
bool checkIfForWhileHead(const Token *startparentheses, const Variable& var, bool suppressErrors, bool isuninit, Alloc alloc, const std::string &membervar);
9898
bool checkLoopBody(const Token *tok, const Variable& var, Alloc alloc, const std::string &membervar, bool suppressErrors);
99-
const Token* checkLoopBodyRecursive(const Token *start, const Variable& var, Alloc alloc, const std::string &membervar, bool &bailout) const;
99+
const Token* checkLoopBodyRecursive(const Token *start, const Variable& var, Alloc alloc, const std::string &membervar, bool &bailout, bool &alwaysReturns) const;
100100
void checkRhs(const Token *tok, const Variable &var, Alloc alloc, nonneg int number_of_if, const std::string &membervar);
101101
static int isFunctionParUsage(const Token *vartok, const Library &library, bool pointer, Alloc alloc, int indirect = 0);
102102
int isFunctionParUsage(const Token *vartok, bool pointer, Alloc alloc, int indirect = 0) const;

test/testuninitvar.cpp

+17
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ class TestUninitVar : public TestFixture {
5757
TEST_CASE(uninitvar3); // #3844
5858
TEST_CASE(uninitvar4); // #3869 (reference)
5959
TEST_CASE(uninitvar5); // #3861
60+
TEST_CASE(uninitvar6); // #13227
6061
TEST_CASE(uninitvar2_func); // function calls
6162
TEST_CASE(uninitvar2_value); // value flow
6263
TEST_CASE(valueFlowUninit2_value);
@@ -3071,6 +3072,22 @@ class TestUninitVar : public TestFixture {
30713072
ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: c\n", errout_str());
30723073
}
30733074

3075+
// #13227
3076+
void uninitvar6() {
3077+
checkUninitVar("int foo(int x) {\n"
3078+
" int i;\n"
3079+
" if (x == 1) {\n"
3080+
" i = 3;\n"
3081+
" } else {\n"
3082+
" do {\n"
3083+
" return 2;\n"
3084+
" } while (0);\n"
3085+
" }\n"
3086+
" return i;\n"
3087+
"}\n");
3088+
ASSERT_EQUALS("", errout_str());
3089+
}
3090+
30743091
void uninitvar8() {
30753092
const char code[] = "struct Fred {\n"
30763093
" void Sync(dsmp_t& type, int& len, int limit = 123);\n"

0 commit comments

Comments
 (0)